diff --git a/.changeset/rm-rf-temp-paths.md b/.changeset/rm-rf-temp-paths.md new file mode 100644 index 000000000..255bb4af3 --- /dev/null +++ b/.changeset/rm-rf-temp-paths.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": patch +--- + +Skip the confirmation prompt for rm -rf commands that target only /tmp or /temp paths. diff --git a/docs/configuration/config-files.md b/docs/configuration/config-files.md index bca968310..7af29eb00 100644 --- a/docs/configuration/config-files.md +++ b/docs/configuration/config-files.md @@ -513,7 +513,7 @@ api_key = "sk-xxx" `permission` sets permission rules that are automatically loaded when a session starts, controlling whether the Agent needs user confirmation before calling a tool. Rules are written as a `[[permission.rules]]` array of tables, matched in order — the first matching rule takes effect. -The dangerous-command guard is enabled by default. In Always Ask and Ask When Needed modes, it requests confirmation for dangerous or unanalyzable `Bash` commands. Allow rules cannot bypass this guard. Never Ask mode and non-interactive execution skip the guard; explicit deny rules still apply. Set `dangerous_command_guard = false`, or `PYTHINKER_CODE_DANGEROUS_COMMAND_GUARD=false`, to disable it in other modes. +The dangerous-command guard is enabled by default. In Always Ask and Ask When Needed modes, it requests confirmation for dangerous or unanalyzable `Bash` commands. Allow rules cannot bypass this guard. Recursive force `rm` (`rm -rf`) of literal paths under `/tmp` or `/temp` that still resolve inside those directories does not require confirmation; mixed, non-literal, escaped, or out-of-prefix targets still do. If that resolution is unavailable, confirmation is required. Never Ask mode and non-interactive execution skip the guard; explicit deny rules still apply. Set `dangerous_command_guard = false`, or `PYTHINKER_CODE_DANGEROUS_COMMAND_GUARD=false`, to disable it in other modes. ```toml [permission] diff --git a/packages/agent-core-v2/src/agent/permissionPolicy/policies/dangerous-command-ask.ts b/packages/agent-core-v2/src/agent/permissionPolicy/policies/dangerous-command-ask.ts index 6bf4cd2af..8d95dd6bd 100644 --- a/packages/agent-core-v2/src/agent/permissionPolicy/policies/dangerous-command-ask.ts +++ b/packages/agent-core-v2/src/agent/permissionPolicy/policies/dangerous-command-ask.ts @@ -4,6 +4,9 @@ import { type BashSyntaxNode, } from '#/app/bashParser/bashParser'; import { IConfigService } from '#/app/config/config'; +import { IHostEnvironment, type PathClass } from '#/os/interface/hostEnvironment'; +import { IHostFileSystem } from '#/os/interface/hostFileSystem'; +import { isWithinDirectory, resolveRealTarget } from '#/tool/path-access'; import { IAgentPermissionModeService } from '#/agent/permissionMode/permissionMode'; import { isDangerousCommandGuardEnabled } from '#/agent/permissionRules/configSection'; import type { ResolvedToolExecutionHookContext } from '#/agent/toolExecutor/toolHooks'; @@ -104,9 +107,41 @@ const DD_SAFE_DEVICE_TARGETS: ReadonlySet = new Set([ '/dev/stderr', ]); +const RM_SAFE_TEMP_ROOTS: readonly string[] = ['/tmp', '/temp']; + +function isSafeTempRmOperand(operand: string): boolean { + for (const segment of operand.split('/')) { + if (segment === '..') return false; + } + return RM_SAFE_TEMP_ROOTS.some((root) => operand === root || operand.startsWith(`${root}/`)); +} + +async function operandResolvesInsideTemp( + fs: Pick, + pathClass: PathClass, + operand: string, +): Promise { + let resolved: string; + try { + resolved = await resolveRealTarget(fs, operand); + } catch { + return false; + } + for (const root of RM_SAFE_TEMP_ROOTS) { + try { + const realRoot = await fs.realpath(root); + if (isWithinDirectory(resolved, realRoot, pathClass)) return true; + } catch { + continue; + } + } + return false; +} + type DangerousVerdict = | { readonly kind: 'dangerous'; readonly command: string } - | { readonly kind: 'unanalyzable' }; + | { readonly kind: 'unanalyzable' } + | { readonly kind: 'temp-rm'; readonly operands: readonly string[] }; export class DangerousCommandAskPermissionPolicyService implements PermissionPolicy { readonly name = 'dangerous-command-ask'; @@ -115,9 +150,11 @@ export class DangerousCommandAskPermissionPolicyService implements PermissionPol @IBashParserService private readonly bashParser: IBashParserService, @IAgentPermissionModeService private readonly modeService: IAgentPermissionModeService, @IConfigService private readonly config: IConfigService, + @IHostFileSystem private readonly hostFs: IHostFileSystem, + @IHostEnvironment private readonly env: IHostEnvironment, ) {} - evaluate(context: ResolvedToolExecutionHookContext): PermissionPolicyResult | undefined { + async evaluate(context: ResolvedToolExecutionHookContext): Promise { if (!isDangerousCommandGuardEnabled(this.config)) return undefined; if (this.modeService.mode === 'auto') return undefined; if (context.toolCall.name !== 'Bash') return undefined; @@ -129,6 +166,22 @@ export class DangerousCommandAskPermissionPolicyService implements PermissionPol this.bashParser.parse(source, PARSE_OPTIONS), ); if (verdict === undefined) return undefined; + if (verdict.kind === 'temp-rm') { + let contained = false; + try { + contained = ( + await Promise.all( + verdict.operands.map((operand) => + operandResolvesInsideTemp(this.hostFs, this.env.pathClass, operand), + ), + ) + ).every(Boolean); + } catch { + contained = false; + } + if (contained) return undefined; + return { kind: 'ask', reason: { dangerous_command: 'rm -rf' } }; + } if (verdict.kind === 'dangerous') { return { kind: 'ask', reason: { dangerous_command: verdict.command } }; } @@ -151,10 +204,17 @@ function analyzeSource( if (!parsed.ok || parsed.hasError) return { kind: 'unanalyzable' }; const commands: BashSyntaxNode[] = []; collectCommands(parsed.root, commands); + const tempOperands: string[] = []; for (const command of commands) { const verdict = analyzeCommand(command, depth, parse); - if (verdict !== undefined) return verdict; + if (verdict === undefined) continue; + if (verdict.kind === 'temp-rm') { + tempOperands.push(...verdict.operands); + continue; + } + return verdict; } + if (tempOperands.length > 0) return { kind: 'temp-rm', operands: tempOperands }; return undefined; } @@ -273,8 +333,17 @@ function analyzeInvocation( if (name === 'rm') { let recursive = false; let force = false; + const operands: string[] = []; + let optionsEnded = false; for (const arg of args) { - if (arg === '--') break; + if (!optionsEnded && arg === '--') { + optionsEnded = true; + continue; + } + if (optionsEnded) { + operands.push(arg); + continue; + } if (arg === '--recursive') { recursive = true; } else if (arg === '--force') { @@ -282,9 +351,16 @@ function analyzeInvocation( } else if (/^-[a-zA-Z]+$/.test(arg)) { if (/[rR]/.test(arg)) recursive = true; if (arg.includes('f')) force = true; + } else { + operands.push(arg); + } + } + if (recursive && force) { + if (!dropped && operands.length > 0 && operands.every(isSafeTempRmOperand)) { + return { kind: 'temp-rm', operands }; } + return { kind: 'dangerous', command: 'rm -rf' }; } - if (recursive && force) return { kind: 'dangerous', command: 'rm -rf' }; return dropped ? { kind: 'unanalyzable' } : undefined; } return undefined; diff --git a/packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts b/packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts index b53e40be4..3a691b51f 100644 --- a/packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts +++ b/packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts @@ -15,6 +15,7 @@ import { } from '#/tool/rule-match'; import type { ResolvedToolExecutionHookContext } from '#/agent/toolExecutor/toolHooks'; import { IHostEnvironment, type IHostEnvironment as HostEnvironmentService } from '#/os/interface/hostEnvironment'; +import { IHostFileSystem } from '#/os/interface/hostFileSystem'; import { IAgentPermissionModeService } from '#/agent/permissionMode/permissionMode'; import { IAgentPermissionPolicyService, type PermissionPolicyEvaluation } from '#/agent/permissionPolicy/permissionPolicy'; import type { PermissionMode } from '#/agent/permissionPolicy/types'; @@ -54,6 +55,7 @@ describe('AgentPermissionPolicyService chain', () => { let workspace: ReturnType; let hostArgs: HostArgs; let dangerousCommandGuardEnabled: boolean; + let resolveRealpath: (path: string) => Promise; beforeEach(() => { disposables = new DisposableStore(); @@ -63,6 +65,12 @@ describe('AgentPermissionPolicyService chain', () => { workspace = workspaceStub('/workspace'); hostArgs = { requestHeaders: {}, nonInteractive: false }; dangerousCommandGuardEnabled = true; + resolveRealpath = async (path) => { + for (const root of ['/tmp', '/temp'] as const) { + if (path === root || path.startsWith(`${root}/`)) return path; + } + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }; ix = createServices(disposables, { additionalServices: (reg) => { reg.defineInstance(IAgentPermissionModeService, stubPermissionModeService(() => mode)); @@ -88,6 +96,9 @@ describe('AgentPermissionPolicyService chain', () => { })); reg.defineInstance(ISessionWorkspaceContext, workspace.stub); reg.defineInstance(IHostEnvironment, pyaosStub()); + reg.definePartialInstance(IHostFileSystem, { + realpath: (path: string) => resolveRealpath(path), + }); reg.defineInstance(IAgentRuntimeService, { _serviceBrand: undefined, onDidChange: () => ({ dispose: () => {} }), @@ -274,7 +285,10 @@ describe('AgentPermissionPolicyService chain', () => { ['systemctl poweroff', 'systemctl poweroff'], ['systemctl --user reboot', 'systemctl reboot'], ['bash -c "shutdown now"', 'shutdown'], - ['rm -rf /tmp/build', 'rm -rf'], + ['rm -rf /tmp/build /root', 'rm -rf'], + ['rm -rf /tmp/build && rm -rf /root', 'rm -rf'], + ['rm -rf /tmp/../etc', 'rm -rf'], + ['rm -rf /tmpfoo', 'rm -rf'], ['rm -fr dir', 'rm -rf'], ['rm -r -f dir', 'rm -rf'], ['rm -R --force dir', 'rm -rf'], @@ -311,6 +325,58 @@ describe('AgentPermissionPolicyService chain', () => { }); }); + it.each([ + 'rm -rf /tmp/build', + 'rm -rf /temp/cache', + 'rm -rf -- /tmp/build', + 'rm -rf /tmp/build && rm -rf /temp/cache', + ])( + 'approves `%s` in yolo mode', + async (command) => { + mode = 'yolo'; + + await expect(evaluate({ + toolName: 'Bash', + args: { command, timeout: 60 }, + })).resolves.toMatchObject({ + policyName: 'yolo-mode-approve', + result: { kind: 'approve' }, + }); + }, + ); + + it('asks for rm -rf of a temp path that realpath-escapes in yolo mode', async () => { + mode = 'yolo'; + resolveRealpath = async (path) => { + if (path === '/tmp/build') return '/etc'; + if (path === '/tmp' || path.startsWith('/tmp/')) return path; + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }; + + await expect(evaluate({ + toolName: 'Bash', + args: { command: 'rm -rf /tmp/build', timeout: 60 }, + })).resolves.toMatchObject({ + policyName: 'dangerous-command-ask', + result: { kind: 'ask', reason: { dangerous_command: 'rm -rf' } }, + }); + }); + + it('asks for rm -rf of a temp path when filesystem realpath fails in yolo mode', async () => { + mode = 'yolo'; + resolveRealpath = async () => { + throw new Error('unavailable'); + }; + + await expect(evaluate({ + toolName: 'Bash', + args: { command: 'rm -rf /tmp/build', timeout: 60 }, + })).resolves.toMatchObject({ + policyName: 'dangerous-command-ask', + result: { kind: 'ask', reason: { dangerous_command: 'rm -rf' } }, + }); + }); + it.each([ 'init 3', 'dd if=/dev/zero of=/dev/null bs=1M count=1', @@ -482,6 +548,7 @@ describe('AgentPermissionPolicyService git cwd write approval', () => { reg.definePartialInstance(IAgentPermissionRulesService, permissionRulesStub()); reg.defineInstance(ISessionWorkspaceContext, workspace.stub); reg.defineInstance(IHostEnvironment, pyaosStub()); + reg.defineInstance(IHostFileSystem, hostFs); reg.defineInstance(IAgentRuntimeService, { _serviceBrand: undefined, onDidChange: () => ({ dispose: () => {} }),