mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 11:41:04 +00:00
fix(core): add getDefaultPermission and allowExternalPaths to ripGrep tool
Add getDefaultPermission() override to GrepToolInvocation in ripGrep.ts to match the behavior of grep.ts, returning "ask" for paths outside the workspace and "allow" for workspace-internal paths. Also pass allowExternalPaths: true to resolveAndValidatePath in both the execute() and validateToolParamValues() methods, so external paths are not rejected at the validation layer (permission is deferred to getDefaultPermission as designed). Fixes issue where grep searches in arbitrary workspace paths would fail with "Path is not within workspace" even when the user intended to search external directories. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
9b22c9fa7c
commit
80b0c6baec
2 changed files with 60 additions and 6 deletions
|
|
@ -590,11 +590,12 @@ describe('RipGrepTool', () => {
|
|||
});
|
||||
|
||||
describe('error handling and edge cases', () => {
|
||||
it('should handle workspace boundary violations', () => {
|
||||
it('should handle workspace boundary violations', async () => {
|
||||
const params: RipGrepToolParams = { pattern: 'test', path: '../outside' };
|
||||
expect(() => grepTool.build(params)).toThrow(
|
||||
/Path is not within workspace/,
|
||||
);
|
||||
// External paths are allowed; permission is deferred to getDefaultPermission()
|
||||
const invocation = grepTool.build(params);
|
||||
const permission = await invocation.getDefaultPermission();
|
||||
expect(permission).toBe('ask');
|
||||
});
|
||||
|
||||
it('should handle empty directories gracefully', async () => {
|
||||
|
|
@ -864,4 +865,34 @@ describe('RipGrepTool', () => {
|
|||
expect(invocation.getDescription()).toBe("'testPattern' in path '.'");
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDefaultPermission', () => {
|
||||
it('should return allow when no path is specified', async () => {
|
||||
const params: RipGrepToolParams = { pattern: 'hello' };
|
||||
const invocation = grepTool.build(params);
|
||||
const permission = await invocation.getDefaultPermission();
|
||||
expect(permission).toBe('allow');
|
||||
});
|
||||
|
||||
it('should return allow for paths within workspace', async () => {
|
||||
const params: RipGrepToolParams = { pattern: 'hello', path: '.' };
|
||||
const invocation = grepTool.build(params);
|
||||
const permission = await invocation.getDefaultPermission();
|
||||
expect(permission).toBe('allow');
|
||||
});
|
||||
|
||||
it('should return allow for subdirectories within workspace', async () => {
|
||||
const params: RipGrepToolParams = { pattern: 'hello', path: 'sub' };
|
||||
const invocation = grepTool.build(params);
|
||||
const permission = await invocation.getDefaultPermission();
|
||||
expect(permission).toBe('allow');
|
||||
});
|
||||
|
||||
it('should return ask for paths outside workspace', async () => {
|
||||
const params: RipGrepToolParams = { pattern: 'hello', path: '/tmp' };
|
||||
const invocation = grepTool.build(params);
|
||||
const permission = await invocation.getDefaultPermission();
|
||||
expect(permission).toBe('ask');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import { SchemaValidator } from '../utils/schemaValidator.js';
|
|||
import type { FileFilteringOptions } from '../config/constants.js';
|
||||
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
|
||||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import type { PermissionDecision } from '../permissions/types.js';
|
||||
|
||||
const debugLogger = createDebugLogger('RIPGREP');
|
||||
|
||||
|
|
@ -56,6 +57,25 @@ class GrepToolInvocation extends BaseToolInvocation<
|
|||
super(params);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns 'ask' for paths outside the workspace, so that external grep
|
||||
* searches require user confirmation.
|
||||
*/
|
||||
override async getDefaultPermission(): Promise<PermissionDecision> {
|
||||
if (!this.params.path) {
|
||||
return 'allow'; // Default workspace directory
|
||||
}
|
||||
const workspaceContext = this.config.getWorkspaceContext();
|
||||
const resolvedPath = path.resolve(
|
||||
this.config.getTargetDir(),
|
||||
this.params.path,
|
||||
);
|
||||
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
|
||||
return 'allow';
|
||||
}
|
||||
return 'ask';
|
||||
}
|
||||
|
||||
async execute(signal: AbortSignal): Promise<ToolResult> {
|
||||
try {
|
||||
// Determine which paths to search
|
||||
|
|
@ -67,7 +87,7 @@ class GrepToolInvocation extends BaseToolInvocation<
|
|||
const searchDirAbs = resolveAndValidatePath(
|
||||
this.config,
|
||||
this.params.path,
|
||||
{ allowFiles: true },
|
||||
{ allowFiles: true, allowExternalPaths: true },
|
||||
);
|
||||
searchPaths.push(searchDirAbs);
|
||||
searchDirDisplay = this.params.path;
|
||||
|
|
@ -364,7 +384,10 @@ export class RipGrepTool extends BaseDeclarativeTool<
|
|||
// Only validate path if one is provided
|
||||
if (params.path) {
|
||||
try {
|
||||
resolveAndValidatePath(this.config, params.path, { allowFiles: true });
|
||||
resolveAndValidatePath(this.config, params.path, {
|
||||
allowFiles: true,
|
||||
allowExternalPaths: true,
|
||||
});
|
||||
} catch (error) {
|
||||
return getErrorMessage(error);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue