Skip to content

Commit a7666f8

Browse files
elkholy90elkaix
andauthored
fix(permissions): skip rm -rf prompts for /tmp and /temp paths (#315)
## Related Issue Internal permission-policy exception for recursive force rm that targets only temp directories. ## Problem The dangerous-command guard asked for every `rm -rf`, including deletes that target only `/tmp` or `/temp`. That blocked routine cleanup of temp directories in Ask When Needed mode. ## What changed Skip the confirmation prompt when every `rm -rf` operand is a literal path under `/tmp` or `/temp` (segment-level prefix, no `..` escape). Mixed, non-literal, or out-of-prefix targets still ask. `rm -rf -- /tmp/...` is included. The permission docs state the exception. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue (external PRs: the issue must have a maintainer's `/approve`). - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. --------- Co-authored-by: elkaix <melkholy@techmatrix.com>
1 parent d18aa91 commit a7666f8

4 files changed

Lines changed: 155 additions & 7 deletions

File tree

.changeset/rm-rf-temp-paths.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Skip the confirmation prompt for rm -rf commands that target only /tmp or /temp paths.

docs/configuration/config-files.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ api_key = "sk-xxx"
513513

514514
`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.
515515

516-
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.
516+
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.
517517

518518
```toml
519519
[permission]

packages/agent-core-v2/src/agent/permissionPolicy/policies/dangerous-command-ask.ts

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import {
44
type BashSyntaxNode,
55
} from '#/app/bashParser/bashParser';
66
import { IConfigService } from '#/app/config/config';
7+
import { IHostEnvironment, type PathClass } from '#/os/interface/hostEnvironment';
8+
import { IHostFileSystem } from '#/os/interface/hostFileSystem';
9+
import { isWithinDirectory, resolveRealTarget } from '#/tool/path-access';
710
import { IAgentPermissionModeService } from '#/agent/permissionMode/permissionMode';
811
import { isDangerousCommandGuardEnabled } from '#/agent/permissionRules/configSection';
912
import type { ResolvedToolExecutionHookContext } from '#/agent/toolExecutor/toolHooks';
@@ -104,9 +107,41 @@ const DD_SAFE_DEVICE_TARGETS: ReadonlySet<string> = new Set([
104107
'/dev/stderr',
105108
]);
106109

110+
const RM_SAFE_TEMP_ROOTS: readonly string[] = ['/tmp', '/temp'];
111+
112+
function isSafeTempRmOperand(operand: string): boolean {
113+
for (const segment of operand.split('/')) {
114+
if (segment === '..') return false;
115+
}
116+
return RM_SAFE_TEMP_ROOTS.some((root) => operand === root || operand.startsWith(`${root}/`));
117+
}
118+
119+
async function operandResolvesInsideTemp(
120+
fs: Pick<IHostFileSystem, 'realpath'>,
121+
pathClass: PathClass,
122+
operand: string,
123+
): Promise<boolean> {
124+
let resolved: string;
125+
try {
126+
resolved = await resolveRealTarget(fs, operand);
127+
} catch {
128+
return false;
129+
}
130+
for (const root of RM_SAFE_TEMP_ROOTS) {
131+
try {
132+
const realRoot = await fs.realpath(root);
133+
if (isWithinDirectory(resolved, realRoot, pathClass)) return true;
134+
} catch {
135+
continue;
136+
}
137+
}
138+
return false;
139+
}
140+
107141
type DangerousVerdict =
108142
| { readonly kind: 'dangerous'; readonly command: string }
109-
| { readonly kind: 'unanalyzable' };
143+
| { readonly kind: 'unanalyzable' }
144+
| { readonly kind: 'temp-rm'; readonly operands: readonly string[] };
110145

111146
export class DangerousCommandAskPermissionPolicyService implements PermissionPolicy {
112147
readonly name = 'dangerous-command-ask';
@@ -115,9 +150,11 @@ export class DangerousCommandAskPermissionPolicyService implements PermissionPol
115150
@IBashParserService private readonly bashParser: IBashParserService,
116151
@IAgentPermissionModeService private readonly modeService: IAgentPermissionModeService,
117152
@IConfigService private readonly config: IConfigService,
153+
@IHostFileSystem private readonly hostFs: IHostFileSystem,
154+
@IHostEnvironment private readonly env: IHostEnvironment,
118155
) {}
119156

120-
evaluate(context: ResolvedToolExecutionHookContext): PermissionPolicyResult | undefined {
157+
async evaluate(context: ResolvedToolExecutionHookContext): Promise<PermissionPolicyResult | undefined> {
121158
if (!isDangerousCommandGuardEnabled(this.config)) return undefined;
122159
if (this.modeService.mode === 'auto') return undefined;
123160
if (context.toolCall.name !== 'Bash') return undefined;
@@ -129,6 +166,22 @@ export class DangerousCommandAskPermissionPolicyService implements PermissionPol
129166
this.bashParser.parse(source, PARSE_OPTIONS),
130167
);
131168
if (verdict === undefined) return undefined;
169+
if (verdict.kind === 'temp-rm') {
170+
let contained = false;
171+
try {
172+
contained = (
173+
await Promise.all(
174+
verdict.operands.map((operand) =>
175+
operandResolvesInsideTemp(this.hostFs, this.env.pathClass, operand),
176+
),
177+
)
178+
).every(Boolean);
179+
} catch {
180+
contained = false;
181+
}
182+
if (contained) return undefined;
183+
return { kind: 'ask', reason: { dangerous_command: 'rm -rf' } };
184+
}
132185
if (verdict.kind === 'dangerous') {
133186
return { kind: 'ask', reason: { dangerous_command: verdict.command } };
134187
}
@@ -151,10 +204,17 @@ function analyzeSource(
151204
if (!parsed.ok || parsed.hasError) return { kind: 'unanalyzable' };
152205
const commands: BashSyntaxNode[] = [];
153206
collectCommands(parsed.root, commands);
207+
const tempOperands: string[] = [];
154208
for (const command of commands) {
155209
const verdict = analyzeCommand(command, depth, parse);
156-
if (verdict !== undefined) return verdict;
210+
if (verdict === undefined) continue;
211+
if (verdict.kind === 'temp-rm') {
212+
tempOperands.push(...verdict.operands);
213+
continue;
214+
}
215+
return verdict;
157216
}
217+
if (tempOperands.length > 0) return { kind: 'temp-rm', operands: tempOperands };
158218
return undefined;
159219
}
160220

@@ -273,18 +333,34 @@ function analyzeInvocation(
273333
if (name === 'rm') {
274334
let recursive = false;
275335
let force = false;
336+
const operands: string[] = [];
337+
let optionsEnded = false;
276338
for (const arg of args) {
277-
if (arg === '--') break;
339+
if (!optionsEnded && arg === '--') {
340+
optionsEnded = true;
341+
continue;
342+
}
343+
if (optionsEnded) {
344+
operands.push(arg);
345+
continue;
346+
}
278347
if (arg === '--recursive') {
279348
recursive = true;
280349
} else if (arg === '--force') {
281350
force = true;
282351
} else if (/^-[a-zA-Z]+$/.test(arg)) {
283352
if (/[rR]/.test(arg)) recursive = true;
284353
if (arg.includes('f')) force = true;
354+
} else {
355+
operands.push(arg);
356+
}
357+
}
358+
if (recursive && force) {
359+
if (!dropped && operands.length > 0 && operands.every(isSafeTempRmOperand)) {
360+
return { kind: 'temp-rm', operands };
285361
}
362+
return { kind: 'dangerous', command: 'rm -rf' };
286363
}
287-
if (recursive && force) return { kind: 'dangerous', command: 'rm -rf' };
288364
return dropped ? { kind: 'unanalyzable' } : undefined;
289365
}
290366
return undefined;

packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '#/tool/rule-match';
1616
import type { ResolvedToolExecutionHookContext } from '#/agent/toolExecutor/toolHooks';
1717
import { IHostEnvironment, type IHostEnvironment as HostEnvironmentService } from '#/os/interface/hostEnvironment';
18+
import { IHostFileSystem } from '#/os/interface/hostFileSystem';
1819
import { IAgentPermissionModeService } from '#/agent/permissionMode/permissionMode';
1920
import { IAgentPermissionPolicyService, type PermissionPolicyEvaluation } from '#/agent/permissionPolicy/permissionPolicy';
2021
import type { PermissionMode } from '#/agent/permissionPolicy/types';
@@ -54,6 +55,7 @@ describe('AgentPermissionPolicyService chain', () => {
5455
let workspace: ReturnType<typeof workspaceStub>;
5556
let hostArgs: HostArgs;
5657
let dangerousCommandGuardEnabled: boolean;
58+
let resolveRealpath: (path: string) => Promise<string>;
5759

5860
beforeEach(() => {
5961
disposables = new DisposableStore();
@@ -63,6 +65,12 @@ describe('AgentPermissionPolicyService chain', () => {
6365
workspace = workspaceStub('/workspace');
6466
hostArgs = { requestHeaders: {}, nonInteractive: false };
6567
dangerousCommandGuardEnabled = true;
68+
resolveRealpath = async (path) => {
69+
for (const root of ['/tmp', '/temp'] as const) {
70+
if (path === root || path.startsWith(`${root}/`)) return path;
71+
}
72+
throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
73+
};
6674
ix = createServices(disposables, {
6775
additionalServices: (reg) => {
6876
reg.defineInstance(IAgentPermissionModeService, stubPermissionModeService(() => mode));
@@ -88,6 +96,9 @@ describe('AgentPermissionPolicyService chain', () => {
8896
}));
8997
reg.defineInstance(ISessionWorkspaceContext, workspace.stub);
9098
reg.defineInstance(IHostEnvironment, pyaosStub());
99+
reg.definePartialInstance(IHostFileSystem, {
100+
realpath: (path: string) => resolveRealpath(path),
101+
});
91102
reg.defineInstance(IAgentRuntimeService, {
92103
_serviceBrand: undefined,
93104
onDidChange: () => ({ dispose: () => {} }),
@@ -274,7 +285,10 @@ describe('AgentPermissionPolicyService chain', () => {
274285
['systemctl poweroff', 'systemctl poweroff'],
275286
['systemctl --user reboot', 'systemctl reboot'],
276287
['bash -c "shutdown now"', 'shutdown'],
277-
['rm -rf /tmp/build', 'rm -rf'],
288+
['rm -rf /tmp/build /root', 'rm -rf'],
289+
['rm -rf /tmp/build && rm -rf /root', 'rm -rf'],
290+
['rm -rf /tmp/../etc', 'rm -rf'],
291+
['rm -rf /tmpfoo', 'rm -rf'],
278292
['rm -fr dir', 'rm -rf'],
279293
['rm -r -f dir', 'rm -rf'],
280294
['rm -R --force dir', 'rm -rf'],
@@ -311,6 +325,58 @@ describe('AgentPermissionPolicyService chain', () => {
311325
});
312326
});
313327

328+
it.each([
329+
'rm -rf /tmp/build',
330+
'rm -rf /temp/cache',
331+
'rm -rf -- /tmp/build',
332+
'rm -rf /tmp/build && rm -rf /temp/cache',
333+
])(
334+
'approves `%s` in yolo mode',
335+
async (command) => {
336+
mode = 'yolo';
337+
338+
await expect(evaluate({
339+
toolName: 'Bash',
340+
args: { command, timeout: 60 },
341+
})).resolves.toMatchObject({
342+
policyName: 'yolo-mode-approve',
343+
result: { kind: 'approve' },
344+
});
345+
},
346+
);
347+
348+
it('asks for rm -rf of a temp path that realpath-escapes in yolo mode', async () => {
349+
mode = 'yolo';
350+
resolveRealpath = async (path) => {
351+
if (path === '/tmp/build') return '/etc';
352+
if (path === '/tmp' || path.startsWith('/tmp/')) return path;
353+
throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
354+
};
355+
356+
await expect(evaluate({
357+
toolName: 'Bash',
358+
args: { command: 'rm -rf /tmp/build', timeout: 60 },
359+
})).resolves.toMatchObject({
360+
policyName: 'dangerous-command-ask',
361+
result: { kind: 'ask', reason: { dangerous_command: 'rm -rf' } },
362+
});
363+
});
364+
365+
it('asks for rm -rf of a temp path when filesystem realpath fails in yolo mode', async () => {
366+
mode = 'yolo';
367+
resolveRealpath = async () => {
368+
throw new Error('unavailable');
369+
};
370+
371+
await expect(evaluate({
372+
toolName: 'Bash',
373+
args: { command: 'rm -rf /tmp/build', timeout: 60 },
374+
})).resolves.toMatchObject({
375+
policyName: 'dangerous-command-ask',
376+
result: { kind: 'ask', reason: { dangerous_command: 'rm -rf' } },
377+
});
378+
});
379+
314380
it.each([
315381
'init 3',
316382
'dd if=/dev/zero of=/dev/null bs=1M count=1',
@@ -482,6 +548,7 @@ describe('AgentPermissionPolicyService git cwd write approval', () => {
482548
reg.definePartialInstance(IAgentPermissionRulesService, permissionRulesStub());
483549
reg.defineInstance(ISessionWorkspaceContext, workspace.stub);
484550
reg.defineInstance(IHostEnvironment, pyaosStub());
551+
reg.defineInstance(IHostFileSystem, hostFs);
485552
reg.defineInstance(IAgentRuntimeService, {
486553
_serviceBrand: undefined,
487554
onDidChange: () => ({ dispose: () => {} }),

0 commit comments

Comments
 (0)