MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-05-14 22:02:59 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): resolve sudo and windows normalize bypasses in commandSafety

Browse Source
This commit is contained in:
Coco Sheng
2026-04-20 11:04:15 -04:00
parent dbbed7fd44
commit 6e13a9f06b
5 changed files with 356 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/sandbox/utils/commandSafety.test.ts
+59 -6
View File
@@ -19,12 +19,10 @@ vi.mock('../../utils/shell-utils.js', () => ({
return '';
},
normalizeCommand: (cmd: string) => {
// Simple mock normalization: /bin/rm -> rm
if (cmd.startsWith('/')) {
const parts = cmd.split('/');
return parts[parts.length - 1];
}
return cmd;
if (!cmd) return '';
const parts = cmd.split(/[\\/]/).filter(Boolean);
const base = parts.length > 0 ? parts[parts.length - 1] : '';
return base.toLowerCase().replace(/\.exe$/, '');
},
}));
@@ -74,6 +72,41 @@ describe('POSIX commandSafety', () => {
it('should identify sudo as dangerous if command is dangerous', () => {
expect(isDangerousCommand(['sudo', 'rm', 'file'])).toBe(true);
expect(isDangerousCommand(['sudo', '-u', 'root', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '--user', 'root', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '-t', 'type', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '-o', 'opt=val', 'rm', 'file'])).toBe(
true,
);
expect(
isDangerousCommand(['sudo', '--option', 'opt=val', 'rm', 'file']),
).toBe(true);
expect(isDangerousCommand(['sudo', '--askpass', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '-D', '/tmp', 'rm', '-rf', '/'])).toBe(
true,
);
expect(
isDangerousCommand(['sudo', '--chroot', '/tmp', 'rm', '-rf', '/']),
).toBe(true);
expect(isDangerousCommand(['sudo', '-r', 'role', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '-t', 'type', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', '--askpass', 'rm', 'file'])).toBe(
true,
);
expect(isDangerousCommand(['sudo', './rm=dangerous'])).toBe(false);
expect(isDangerousCommand(['sudo', './rm'])).toBe(true);
});
it('should identify find -exec as dangerous', () => {
@@ -82,6 +115,26 @@ describe('POSIX commandSafety', () => {
);
});
it('should identify dangerous commands wrapped in env', () => {
expect(isDangerousCommand(['env', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['env', '-i', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['env', '-u', 'USER', 'rm', '-rf', '/'])).toBe(
true,
);
expect(isDangerousCommand(['env', 'VAR=val', 'rm', '-rf', '/'])).toBe(
true,
);
expect(isDangerousCommand(['env', '--', 'rm', '-rf', '/'])).toBe(true);
});
it('should identify dangerous commands wrapped in xargs', () => {
expect(isDangerousCommand(['xargs', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['xargs', '-I', '{}', 'rm', '{}'])).toBe(true);
expect(isDangerousCommand(['xargs', '-n', '1', 'rm'])).toBe(true);
expect(isDangerousCommand(['xargs', '-0', 'rm'])).toBe(true);
expect(isDangerousCommand(['xargs', '--', 'rm'])).toBe(true);
});
it('should identify dangerous rg flags', () => {
expect(isDangerousCommand(['rg', '--hostname-bin', 'something'])).toBe(
true,
packages/core/src/sandbox/utils/commandSafety.ts
+96 -5
View File
@@ -256,7 +256,7 @@ function isSafeToCallWithExec(args: string[]): boolean {
* @param subcommands - A list of subcommands to look for.
* @returns An object containing the index of the subcommand and its name.
*/
function findGitSubcommand(
export function findGitSubcommand(
args: string[],
subcommands: string[],
): { idx: number; subcommand: string | null } {
@@ -318,7 +318,7 @@ function findGitSubcommand(
* @param args - The git command arguments.
* @returns true if config overrides are present.
*/
function gitHasConfigOverrideGlobalOption(args: string[]): boolean {
export function gitHasConfigOverrideGlobalOption(args: string[]): boolean {
return args.some(
(arg) =>
arg === '-c' ||
@@ -336,7 +336,7 @@ function gitHasConfigOverrideGlobalOption(args: string[]): boolean {
* @param args - Arguments passed to the git subcommand.
* @returns true if the arguments only represent read-only operations.
*/
function gitSubcommandArgsAreReadOnly(args: string[]): boolean {
export function gitSubcommandArgsAreReadOnly(args: string[]): boolean {
const unsafeFlags = new Set([
'--output',
'--ext-diff',
@@ -360,7 +360,7 @@ function gitSubcommandArgsAreReadOnly(args: string[]): boolean {
* @param args - Arguments passed to `git branch`.
* @returns true if it's purely a listing/read-only branch command.
*/
function gitBranchIsReadOnly(args: string[]): boolean {
export function gitBranchIsReadOnly(args: string[]): boolean {
if (args.length === 0) return true;
let sawReadOnlyFlag = false;
@@ -436,7 +436,98 @@ export function isDangerousCommand(args: string[]): boolean {
}
if (cmd === 'sudo') {
return isDangerousCommand(args.slice(1));
let nextCmdIdx = 1;
while (nextCmdIdx < args.length) {
const arg = args[nextCmdIdx];
if (arg === '--') {
nextCmdIdx++;
break;
}
if (!arg.startsWith('-')) {
// It could be variable assignment like VAR=value before the command
if (/^[a-zA-Z_][a-zA-Z0-9_]*=/.test(arg)) {
nextCmdIdx++;
continue;
}
break;
}
// Check for options that consume the next argument
if (
/^-[acCDghopRrTtUu]$/.test(arg) ||
[
'--auth-type',
'--login-class',
'--close-from',
'--chdir',
'--group',
'--host',
'--prompt',
'--chroot',
'--role',
'--type',
'--command-timeout',
'--timeout',
'--user',
'--other-user',
'--option',
].includes(arg)
) {
nextCmdIdx += 2;
} else {
nextCmdIdx += 1;
}
}
return isDangerousCommand(args.slice(nextCmdIdx));
}
if (['env', 'xargs'].includes(cmd)) {
let nextCmdIdx = 1;
while (nextCmdIdx < args.length) {
const arg = args[nextCmdIdx];
if (arg === '--') {
nextCmdIdx++;
break;
}
if (!arg.startsWith('-')) {
// It could be variable assignment like VAR=value before the command (for env)
if (cmd === 'env' && /^[a-zA-Z_][a-zA-Z0-9_]*=/.test(arg)) {
nextCmdIdx++;
continue;
}
break;
}
// Check for options that consume the next argument
if (cmd === 'env') {
if (
/^-[uS]$/.test(arg) ||
['--unset', '--split-string'].includes(arg)
) {
nextCmdIdx += 2;
} else {
nextCmdIdx += 1;
}
} else if (cmd === 'xargs') {
if (
/^-[aEeIiLlnPs]$/.test(arg) ||
[
'--arg-file',
'--max-args',
'--max-procs',
'--max-chars',
'--process-slot-var',
].includes(arg)
) {
nextCmdIdx += 2;
} else {
nextCmdIdx += 1;
}
} else {
nextCmdIdx += 1;
}
}
return isDangerousCommand(args.slice(nextCmdIdx));
}
if (cmd === 'find') {
packages/core/src/sandbox/windows/commandSafety.test.ts
+53
View File
@@ -25,6 +25,15 @@ describe('Windows commandSafety', () => {
expect(isKnownSafeCommand(['unknown'])).toBe(false);
expect(isKnownSafeCommand(['npm', 'install'])).toBe(false);
});
it('should reject unsafe git commands', () => {
expect(isKnownSafeCommand(['git', 'commit'])).toBe(false);
expect(isKnownSafeCommand(['git', 'push'])).toBe(false);
expect(isKnownSafeCommand(['git', 'checkout'])).toBe(false);
expect(isKnownSafeCommand(['git', 'log', '--output=file.txt'])).toBe(
false,
);
});
});
describe('isDangerousCommand', () => {
@@ -34,6 +43,15 @@ describe('Windows commandSafety', () => {
expect(isDangerousCommand(['cmd', '/c', 'dir'])).toBe(true);
});
it('should reject unsafe git commands as dangerous', () => {
expect(isDangerousCommand(['git', 'log', '--output=file.txt'])).toBe(
true,
);
expect(isDangerousCommand(['git', '-c', 'alias.foo=!sh', 'status'])).toBe(
true,
);
});
it('should identify path-qualified dangerous commands', () => {
expect(
isDangerousCommand(['C:\\Windows\\System32\\del.exe', 'file.txt']),
@@ -48,6 +66,41 @@ describe('Windows commandSafety', () => {
expect(isDangerousCommand(['cmd.exe', '/c', 'dir'])).toBe(true);
});
it('should strip multiple trailing dots and extensions for dangerous commands', () => {
expect(isDangerousCommand(['powershell.', '-Command', 'echo'])).toBe(
true,
);
expect(isDangerousCommand(['powershell.exe.', '-Command', 'echo'])).toBe(
true,
);
expect(isDangerousCommand(['cmd.bat..', '/c', 'dir'])).toBe(true);
});
it('should flag rm as dangerous', () => {
expect(isDangerousCommand(['rm', 'file'])).toBe(true);
expect(isDangerousCommand(['rm.exe', 'file'])).toBe(true);
});
it('should identify dangerous commands wrapped in env', () => {
expect(isDangerousCommand(['env', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['env', '-i', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['env', '-u', 'USER', 'rm', '-rf', '/'])).toBe(
true,
);
expect(isDangerousCommand(['env', 'VAR=val', 'rm', '-rf', '/'])).toBe(
true,
);
expect(isDangerousCommand(['env', '--', 'rm', '-rf', '/'])).toBe(true);
});
it('should identify dangerous commands wrapped in xargs', () => {
expect(isDangerousCommand(['xargs', 'rm', '-rf', '/'])).toBe(true);
expect(isDangerousCommand(['xargs', '-I', '{}', 'rm', '{}'])).toBe(true);
expect(isDangerousCommand(['xargs', '-n', '1', 'rm'])).toBe(true);
expect(isDangerousCommand(['xargs', '-0', 'rm'])).toBe(true);
expect(isDangerousCommand(['xargs', '--', 'rm'])).toBe(true);
});
it('should not flag safe commands as dangerous', () => {
expect(isDangerousCommand(['dir'])).toBe(false);
expect(isDangerousCommand(['echo', 'hello'])).toBe(false);
packages/core/src/sandbox/windows/commandSafety.ts
+128 -13
View File
@@ -10,7 +10,14 @@ import {
splitCommands,
stripShellWrapper,
normalizeCommand,
hasRedirection,
} from '../../utils/shell-utils.js';
import {
findGitSubcommand,
gitHasConfigOverrideGlobalOption,
gitSubcommandArgsAreReadOnly,
gitBranchIsReadOnly,
} from '../utils/commandSafety.js';
/**
* Determines if a command is strictly approved for execution on Windows.
@@ -34,6 +41,10 @@ export async function isStrictlyApproved(
const fullCmd = [command, ...args].join(' ');
const stripped = stripShellWrapper(fullCmd);
if (hasRedirection(stripped)) {
return false;
}
const pipelineCommands = splitCommands(stripped);
// Fallback for simple commands or parsing failures
@@ -49,10 +60,7 @@ export async function isStrictlyApproved(
const parsedArgs = shellParse(trimmed).map(extractStringFromParseEntry);
if (parsedArgs.length === 0) return true;
let root = parsedArgs[0].toLowerCase();
if (root.endsWith('.exe')) {
root = root.slice(0, -4);
}
const root = normalizeCommand(parsedArgs[0]);
// The segment is approved if the root tool is in the allowlist OR if the whole segment is safe.
return (
tools.some((t) => t.toLowerCase() === root) ||
@@ -66,10 +74,7 @@ export async function isStrictlyApproved(
*/
export function isKnownSafeCommand(args: string[]): boolean {
if (!args || args.length === 0) return false;
let cmd = args[0].toLowerCase();
if (cmd.endsWith('.exe')) {
cmd = cmd.slice(0, -4);
}
const cmd = normalizeCommand(args[0]);
// Native Windows/PowerShell safe commands
const safeCommands = new Set([
@@ -106,10 +111,35 @@ export function isKnownSafeCommand(args: string[]): boolean {
// We allow git on Windows if it's read-only, using the same logic as POSIX
if (cmd === 'git') {
// For simplicity in this branch, we'll allow standard git read operations
// In a full implementation, we'd port the sub-command validation too.
const sub = args[1]?.toLowerCase();
return ['status', 'log', 'diff', 'show', 'branch'].includes(sub);
if (gitHasConfigOverrideGlobalOption(args)) {
return false;
}
const { idx, subcommand } = findGitSubcommand(args, [
'status',
'log',
'diff',
'show',
'branch',
]);
if (!subcommand) {
return false;
}
const subcommandArgs = args.slice(idx + 1);
if (['status', 'log', 'diff', 'show'].includes(subcommand)) {
return gitSubcommandArgsAreReadOnly(subcommandArgs);
}
if (subcommand === 'branch') {
return (
gitSubcommandArgsAreReadOnly(subcommandArgs) &&
gitBranchIsReadOnly(subcommandArgs)
);
}
return false;
}
return false;
@@ -123,6 +153,7 @@ export function isDangerousCommand(args: string[]): boolean {
const cmd = normalizeCommand(args[0]);
const dangerous = new Set([
'rm',
'del',
'erase',
'rd',
@@ -144,5 +175,89 @@ export function isDangerousCommand(args: string[]): boolean {
'new-item',
]);
return dangerous.has(cmd);
if (dangerous.has(cmd)) {
return true;
}
if (cmd === 'git') {
if (gitHasConfigOverrideGlobalOption(args)) {
return true;
}
const { idx, subcommand } = findGitSubcommand(args, [
'status',
'log',
'diff',
'show',
'branch',
]);
if (!subcommand) {
// It's a git command we don't recognize as explicitly safe.
return false;
}
const subcommandArgs = args.slice(idx + 1);
if (['status', 'log', 'diff', 'show'].includes(subcommand)) {
return !gitSubcommandArgsAreReadOnly(subcommandArgs);
}
if (subcommand === 'branch') {
return !(
gitSubcommandArgsAreReadOnly(subcommandArgs) &&
gitBranchIsReadOnly(subcommandArgs)
);
}
return false;
}
if (['env', 'xargs'].includes(cmd)) {
let nextCmdIdx = 1;
while (nextCmdIdx < args.length) {
const arg = args[nextCmdIdx];
if (arg === '--') {
nextCmdIdx++;
break;
}
if (!arg.startsWith('-')) {
if (cmd === 'env' && /^[a-zA-Z_][a-zA-Z0-9_]*=/.test(arg)) {
nextCmdIdx++;
continue;
}
break;
}
if (cmd === 'env') {
if (
/^-[uS]$/.test(arg) ||
['--unset', '--split-string'].includes(arg)
) {
nextCmdIdx += 2;
} else {
nextCmdIdx += 1;
}
} else if (cmd === 'xargs') {
if (
/^-[aEeIiLlnPs]$/.test(arg) ||
[
'--arg-file',
'--max-args',
'--max-procs',
'--max-chars',
'--process-slot-var',
].includes(arg)
) {
nextCmdIdx += 2;
} else {
nextCmdIdx += 1;
}
} else {
nextCmdIdx += 1;
}
}
return isDangerousCommand(args.slice(nextCmdIdx));
}
return false;
}
packages/core/src/utils/shell-utils.ts
+20 -1
View File
@@ -321,7 +321,26 @@ export function normalizeCommand(commandName: string): string {
// Split by both separators and get the last non-empty part
const parts = commandName.split(/[\\/]/).filter(Boolean);
const base = parts.length > 0 ? parts[parts.length - 1] : '';
return base.toLowerCase().replace(/\.exe$/, '');
let result = base.toLowerCase();
const extensions = ['.exe', '.cmd', '.bat', '.com', '.ps1'];
while (true) {
const prev = result;
while (result.endsWith('.')) {
result = result.slice(0, -1);
}
for (const ext of extensions) {
if (result.endsWith(ext)) {
result = result.slice(0, -ext.length);
break;
}
}
if (result === prev) break;
}
return result;
}
function extractNameFromNode(node: Node): string | null {