Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/rm-rf-temp-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@pymodel/pythinker-code": patch
---

Skip the confirmation prompt for rm -rf commands that target only /tmp or /temp paths.
2 changes: 1 addition & 1 deletion docs/configuration/config-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -104,9 +107,41 @@ const DD_SAFE_DEVICE_TARGETS: ReadonlySet<string> = 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}/`));
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
async function operandResolvesInsideTemp(
fs: Pick<IHostFileSystem, 'realpath'>,
pathClass: PathClass,
operand: string,
): Promise<boolean> {
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';
Expand All @@ -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<PermissionPolicyResult | undefined> {
if (!isDangerousCommandGuardEnabled(this.config)) return undefined;
if (this.modeService.mode === 'auto') return undefined;
if (context.toolCall.name !== 'Bash') return undefined;
Expand All @@ -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 } };
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -273,18 +333,34 @@ 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') {
force = true;
} 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -54,6 +55,7 @@ describe('AgentPermissionPolicyService chain', () => {
let workspace: ReturnType<typeof workspaceStub>;
let hostArgs: HostArgs;
let dangerousCommandGuardEnabled: boolean;
let resolveRealpath: (path: string) => Promise<string>;

beforeEach(() => {
disposables = new DisposableStore();
Expand All @@ -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));
Expand All @@ -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: () => {} }),
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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: () => {} }),
Expand Down
Loading