Skip to content

Commit 00bf360

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

File tree

4 files changed

+100
-6
lines changed

4 files changed

+100
-6
lines changed
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
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: unknown) => {
15+
if (typeof entry === 'string') return entry;
16+
if (entry && typeof entry === 'object' && 'content' in entry) {
17+
return (entry as { content: string }).content;
18+
}
19+
return '';
20+
},
21+
normalizeCommand: (cmd: string) => {
22+
// Simple mock normalization: /bin/rm -> rm
23+
if (cmd.startsWith('/')) {
24+
const parts = cmd.split('/');
25+
return parts[parts.length - 1];
26+
}
27+
return cmd;
28+
},
29+
}));
30+
31+
import { isKnownSafeCommand, isDangerousCommand } from './commandSafety.js';
32+
33+
describe('POSIX commandSafety', () => {
34+
describe('isKnownSafeCommand', () => {
35+
it('should identify known safe commands', () => {
36+
expect(isKnownSafeCommand(['ls', '-la'])).toBe(true);
37+
expect(isKnownSafeCommand(['cat', 'file.txt'])).toBe(true);
38+
expect(isKnownSafeCommand(['pwd'])).toBe(true);
39+
expect(isKnownSafeCommand(['echo', 'hello'])).toBe(true);
40+
});
41+
42+
it('should identify safe git commands', () => {
43+
expect(isKnownSafeCommand(['git', 'status'])).toBe(true);
44+
expect(isKnownSafeCommand(['git', 'log'])).toBe(true);
45+
expect(isKnownSafeCommand(['git', 'diff'])).toBe(true);
46+
});
47+
48+
it('should reject unsafe git commands', () => {
49+
expect(isKnownSafeCommand(['git', 'commit'])).toBe(false);
50+
expect(isKnownSafeCommand(['git', 'push'])).toBe(false);
51+
expect(isKnownSafeCommand(['git', 'checkout'])).toBe(false);
52+
});
53+
54+
it('should reject commands with redirection', () => {
55+
// isKnownSafeCommand handles bash -c "..." which can have redirections
56+
// but the simple check for atomic commands doesn't see redirection because it's already parsed
57+
});
58+
});
59+
60+
describe('isDangerousCommand', () => {
61+
it('should identify destructive rm commands', () => {
62+
expect(isDangerousCommand(['rm'])).toBe(true);
63+
expect(isDangerousCommand(['rm', 'file.txt'])).toBe(true);
64+
expect(isDangerousCommand(['rm', '-rf', '/'])).toBe(true);
65+
expect(isDangerousCommand(['rm', '-f', 'file'])).toBe(true);
66+
expect(isDangerousCommand(['rm', '-r', 'dir'])).toBe(true);
67+
expect(isDangerousCommand(['/bin/rm', 'file'])).toBe(true);
68+
});
69+
70+
it('should flag rm help/version as dangerous (strict)', () => {
71+
expect(isDangerousCommand(['rm', '--help'])).toBe(true);
72+
expect(isDangerousCommand(['rm', '--version'])).toBe(true);
73+
});
74+
75+
it('should identify sudo as dangerous if command is dangerous', () => {
76+
expect(isDangerousCommand(['sudo', 'rm', 'file'])).toBe(true);
77+
});
78+
79+
it('should identify find -exec as dangerous', () => {
80+
expect(isDangerousCommand(['find', '.', '-exec', 'rm', '{}', '+'])).toBe(
81+
true,
82+
);
83+
});
84+
85+
it('should identify dangerous rg flags', () => {
86+
expect(isDangerousCommand(['rg', '--hostname-bin', 'something'])).toBe(
87+
true,
88+
);
89+
});
90+
});
91+
});

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

Lines changed: 3 additions & 2 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

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

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

packages/core/src/sandbox/windows/commandSafety.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ describe('Windows commandSafety', () => {
3434
expect(isDangerousCommand(['cmd', '/c', 'dir'])).toBe(true);
3535
});
3636

37+
it('should identify path-qualified dangerous commands', () => {
38+
expect(isDangerousCommand(['C:\\Windows\\System32\\del.exe', 'file.txt'])).toBe(true);
39+
});
40+
3741
it('should strip .exe extension for dangerous commands', () => {
3842
expect(isDangerousCommand(['del.exe', 'file.txt'])).toBe(true);
3943
expect(isDangerousCommand(['POWERSHELL.EXE', '-Command', 'echo'])).toBe(

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

Lines changed: 2 additions & 4 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
/**
@@ -119,10 +120,7 @@ export function isKnownSafeCommand(args: string[]): boolean {
119120
*/
120121
export function isDangerousCommand(args: string[]): boolean {
121122
if (!args || args.length === 0) return false;
122-
let cmd = args[0].toLowerCase();
123-
if (cmd.endsWith('.exe')) {
124-
cmd = cmd.slice(0, -4);
125-
}
123+
const cmd = normalizeCommand(args[0]);
126124

127125
const dangerous = new Set([
128126
'del',

0 commit comments

Comments
 (0)