Skip to content

Commit 11b6bcf

Browse files
committed
security: improve rm detection and clarify UI labels
1 parent 6d7974f commit 11b6bcf

File tree

4 files changed

+97
-13
lines changed

4 files changed

+97
-13
lines changed

packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,15 @@ export const ToolConfirmationMessage: React.FC<
352352
key: 'Allow once',
353353
});
354354
if (isTrustedFolder) {
355+
const rootCommand = confirmationDetails.rootCommand || 'this command';
355356
options.push({
356-
label: `Allow for this session`,
357+
label: `Allow all '${rootCommand}' commands for this session`,
357358
value: ToolConfirmationOutcome.ProceedAlways,
358359
key: `Allow for this session`,
359360
});
360361
if (allowPermanentApproval) {
361362
options.push({
362-
label: `Allow this command for all future sessions`,
363+
label: `Allow all '${rootCommand}' commands for all future sessions`,
363364
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
364365
key: `Allow for all future sessions`,
365366
});

packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ exports[`ToolConfirmationMessage > height allocation and layout > should expand
9696
╰──────────────────────────────────────────────────────────────────────────────╯
9797
Allow execution of [echo]?
9898
99-
● 1. Allow once
100-
2. Allow for this session
99+
● 1. Allow once
100+
2. Allow all 'echo' commands for this session
101101
3. No, suggest changes (esc)
102102
"
103103
`;
@@ -112,8 +112,8 @@ exports[`ToolConfirmationMessage > should display multiple commands for exec typ
112112
╰──────────────────────────────────────────────────────────────────────────────╯
113113
Allow execution of [echo, ls, whoami]?
114114
115-
● 1. Allow once
116-
2. Allow for this session
115+
● 1. Allow once
116+
2. Allow all 'echo' commands for this session
117117
3. No, suggest changes (esc)
118118
"
119119
`;
@@ -150,8 +150,8 @@ exports[`ToolConfirmationMessage > should render multiline shell scripts with co
150150
╰──────────────────────────────────────────────────────────────────────────────╯
151151
Allow execution of [echo]?
152152
153-
● 1. Allow once
154-
2. Allow for this session
153+
● 1. Allow once
154+
2. Allow all 'echo' commands for this session
155155
3. No, suggest changes (esc)"
156156
`;
157157

@@ -213,8 +213,8 @@ exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations'
213213
╰──────────────────────────────────────────────────────────────────────────────╯
214214
Allow execution of [echo]?
215215
216-
● 1. Allow once
217-
2. Allow for this session
216+
● 1. Allow once
217+
2. Allow all 'echo' commands for this session
218218
3. No, suggest changes (esc)
219219
"
220220
`;
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { describe, expect, it, vi } from 'vitest';
8+
9+
// Mock shell-utils to avoid relying on tree-sitter WASM
10+
vi.mock('../../utils/shell-utils.js', () => ({
11+
initializeShellParsers: vi.fn().mockResolvedValue(undefined),
12+
splitCommands: (cmd: string) => [cmd],
13+
stripShellWrapper: (cmd: string) => cmd,
14+
extractStringFromParseEntry: (entry: any) =>
15+
typeof entry === 'string' ? entry : entry.content,
16+
normalizeCommand: (cmd: string) => {
17+
// Simple mock normalization: /bin/rm -> rm
18+
if (cmd.startsWith('/')) {
19+
const parts = cmd.split('/');
20+
return parts[parts.length - 1];
21+
}
22+
return cmd;
23+
},
24+
}));
25+
26+
import { isKnownSafeCommand, isDangerousCommand } from './commandSafety.js';
27+
28+
describe('POSIX commandSafety', () => {
29+
describe('isKnownSafeCommand', () => {
30+
it('should identify known safe commands', () => {
31+
expect(isKnownSafeCommand(['ls', '-la'])).toBe(true);
32+
expect(isKnownSafeCommand(['cat', 'file.txt'])).toBe(true);
33+
expect(isKnownSafeCommand(['pwd'])).toBe(true);
34+
expect(isKnownSafeCommand(['echo', 'hello'])).toBe(true);
35+
});
36+
37+
it('should identify safe git commands', () => {
38+
expect(isKnownSafeCommand(['git', 'status'])).toBe(true);
39+
expect(isKnownSafeCommand(['git', 'log'])).toBe(true);
40+
expect(isKnownSafeCommand(['git', 'diff'])).toBe(true);
41+
});
42+
43+
it('should reject unsafe git commands', () => {
44+
expect(isKnownSafeCommand(['git', 'commit'])).toBe(false);
45+
expect(isKnownSafeCommand(['git', 'push'])).toBe(false);
46+
expect(isKnownSafeCommand(['git', 'checkout'])).toBe(false);
47+
});
48+
49+
it('should reject commands with redirection', () => {
50+
// isKnownSafeCommand handles bash -c "..." which can have redirections
51+
// but the simple check for atomic commands doesn't see redirection because it's already parsed
52+
});
53+
});
54+
55+
describe('isDangerousCommand', () => {
56+
it('should identify destructive rm commands', () => {
57+
expect(isDangerousCommand(['rm'])).toBe(true);
58+
expect(isDangerousCommand(['rm', 'file.txt'])).toBe(true);
59+
expect(isDangerousCommand(['rm', '-rf', '/'])).toBe(true);
60+
expect(isDangerousCommand(['rm', '-f', 'file'])).toBe(true);
61+
expect(isDangerousCommand(['rm', '-r', 'dir'])).toBe(true);
62+
expect(isDangerousCommand(['/bin/rm', 'file'])).toBe(true);
63+
});
64+
65+
it('should flag rm help/version as dangerous (strict)', () => {
66+
expect(isDangerousCommand(['rm', '--help'])).toBe(true);
67+
expect(isDangerousCommand(['rm', '--version'])).toBe(true);
68+
});
69+
70+
it('should identify sudo as dangerous if command is dangerous', () => {
71+
expect(isDangerousCommand(['sudo', 'rm', 'file'])).toBe(true);
72+
});
73+
74+
it('should identify find -exec as dangerous', () => {
75+
expect(isDangerousCommand(['find', '.', '-exec', 'rm', '{}', '+'])).toBe(true);
76+
});
77+
78+
it('should identify dangerous rg flags', () => {
79+
expect(isDangerousCommand(['rg', '--hostname-bin', 'something'])).toBe(true);
80+
});
81+
});
82+
});

packages/core/src/sandbox/utils/commandSafety.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
initializeShellParsers,
1010
splitCommands,
1111
stripShellWrapper,
12+
normalizeCommand,
1213
} from '../../utils/shell-utils.js';
1314

1415
/**
@@ -428,10 +429,10 @@ export function isDangerousCommand(args: string[]): boolean {
428429
return false;
429430
}
430431

431-
const cmd = args[0];
432+
const cmd = normalizeCommand(args[0]);
432433

433-
if (cmd === 'rm') {
434-
return args[1] === '-f' || args[1] === '-rf' || args[1] === '-fr';
434+
if (cmd === 'rm' || cmd === 'del' || cmd === 'erase') {
435+
return true;
435436
}
436437

437438
if (cmd === 'sudo') {

0 commit comments

Comments
 (0)