From 741f44feb6d1312f4891894574ccc6fd5af7825d Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Fri, 12 Dec 2025 17:36:24 -0800 Subject: [PATCH 01/10] feat: enhance shell safety policy with comprehensive tests for command injection and parsing, and refactor test setup. --- packages/core/src/policy/policy-engine.ts | 106 ++++++---- packages/core/src/policy/shell-safety.test.ts | 199 +++++++++++++++++- packages/core/src/policy/toml-loader.ts | 77 +++---- 3 files changed, 298 insertions(+), 84 deletions(-) diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index 71a4ff232b6..da538fa1da3 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -140,7 +140,37 @@ export class PolicyEngine { `[PolicyEngine.check] toolCall.name: ${toolCall.name}, stringifiedArgs: ${stringifiedArgs}`, ); - // Find the first matching rule (already sorted by priority) + // Check for shell commands upfront to handle splitting + let subCommands: string[] | undefined; + let isShellCommand = false; + let command: string | undefined; + + if (toolCall.name && SHELL_TOOL_NAMES.includes(toolCall.name)) { + isShellCommand = true; + command = (toolCall.args as { command?: string })?.command; + if (command) { + // Initialize parser if needed (lazy load) + await initializeShellParsers(); + subCommands = splitCommands(command); + + if (subCommands.length === 0) { + // Parsing failed or empty command -> Unsafe to rely on prefix matching + debugLogger.debug( + `[PolicyEngine.check] Command parsing failed for: ${command}. Falling back to default/safe decision.`, + ); + // If parsing fails, we cannot safely decompose the command. + // We force an ASK_USER decision (or DENY in non-interactive) regardless of rules, + // because we can't guarantee a rule matches the *actual* executed logic if we can't parse it. + // Using defaultDecision might be too lenient if default is ALLOW (though it shouldn't be). + // Safest is ASK_USER. + return { + decision: this.applyNonInteractiveMode(PolicyDecision.ASK_USER), + rule: undefined, + }; + } + } + } + let matchedRule: PolicyRule | undefined; let decision: PolicyDecision | undefined; @@ -150,38 +180,31 @@ export class PolicyEngine { `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`, ); - // Special handling for shell commands: check sub-commands if present - if ( - toolCall.name && - SHELL_TOOL_NAMES.includes(toolCall.name) && - rule.decision === PolicyDecision.ALLOW - ) { - const command = (toolCall.args as { command?: string })?.command; - if (command) { - await initializeShellParsers(); - const subCommands = splitCommands(command); - - // If there are multiple sub-commands, we must verify EACH of them matches an ALLOW rule. - // If any sub-command results in DENY -> the whole thing is DENY. - // If any sub-command results in ASK_USER -> the whole thing is ASK_USER (unless one is DENY). - // Only if ALL sub-commands are ALLOW do we proceed with ALLOW. - if (subCommands.length === 0) { - // This case occurs if the command is non-empty but parsing fails. - // An ALLOW rule for a prefix might have matched, but since the rest of - // the command is un-parseable, it's unsafe to proceed. - // Fall back to a safe decision. - debugLogger.debug( - `[PolicyEngine.check] Command parsing failed for: ${command}. Falling back to safe decision because implicit ALLOW is unsafe.`, - ); - decision = this.applyNonInteractiveMode(PolicyDecision.ASK_USER); - } else if (subCommands.length > 1) { + if (isShellCommand && rule.decision === PolicyDecision.ALLOW) { + // For shell commands, if the matching rule is ALLOW, we must verify ALL subcommands allow it. + // If subsCommands is set (length > 0), we check them. + if (subCommands && subCommands.length > 0) { + if (subCommands.length > 1) { debugLogger.debug( `[PolicyEngine.check] Compound command detected: ${subCommands.length} parts`, ); let aggregateDecision = PolicyDecision.ALLOW; + // We need to check if *any* subcommand fails the check. + // Note: We are recursively checking subcommands against the FULL rule set. + // This ensures that "git log" matches the "git log" rule, + // and "rm -rf /" matches (or doesn't match) its own rules. for (const subCmd of subCommands) { - // Recursively check each sub-command + // Prevent infinite recursion if the subcommand is identical to the original command. + // This happens when splitCommands returns the root command along with its children. + // We use trimmed comparison to be robust against whitespace differences. + if (command && subCmd.trim() === command.trim()) { + debugLogger.debug( + `[PolicyEngine.check] Skipping recursion for self-referential command: "${subCmd.trim()}" vs original: "${command.trim()}"`, + ); + continue; + } + const subCall = { name: toolCall.name, args: { command: subCmd }, @@ -190,38 +213,43 @@ export class PolicyEngine { if (subResult.decision === PolicyDecision.DENY) { aggregateDecision = PolicyDecision.DENY; - break; // Fail fast + matchedRule = subResult.rule ?? rule; // Blame the rule that denied it + break; } else if (subResult.decision === PolicyDecision.ASK_USER) { aggregateDecision = PolicyDecision.ASK_USER; - // efficient: we can only strictly downgrade from ALLOW to ASK_USER, - // but we must continue looking for DENY. + if (!matchedRule) matchedRule = subResult.rule ?? rule; } } - decision = aggregateDecision; } else { - // Single command, rule match is valid - decision = this.applyNonInteractiveMode(rule.decision); + // Single command found. Rely on the initial rule match. + decision = rule.decision; } } else { - decision = this.applyNonInteractiveMode(rule.decision); + // No subcommands found (should have been caught by generic check above unless command was empty) + decision = rule.decision; } } else { - decision = this.applyNonInteractiveMode(rule.decision); + // Not a shell command OR decision is not ALLOW (DENY/ASK_USER apply immediately) + decision = rule.decision; } + matchedRule = rule; break; } } if (!decision) { - // No matching rule found, use default decision + // No matching rule found debugLogger.debug( `[PolicyEngine.check] NO MATCH - using default decision: ${this.defaultDecision}`, ); - decision = this.applyNonInteractiveMode(this.defaultDecision); + decision = this.defaultDecision; } + // Apply non-interactive mode constraint to the final decision + decision = this.applyNonInteractiveMode(decision); + // If decision is not DENY, run safety checkers if (decision !== PolicyDecision.DENY && this.checkerRunner) { for (const checkerRule of this.checkers) { @@ -247,7 +275,7 @@ export class PolicyEngine { debugLogger.debug( `[PolicyEngine.check] Safety checker requested ASK_USER: ${result.reason}`, ); - decision = PolicyDecision.ASK_USER; + decision = this.applyNonInteractiveMode(PolicyDecision.ASK_USER); } } catch (error) { debugLogger.debug( @@ -263,7 +291,7 @@ export class PolicyEngine { } return { - decision: this.applyNonInteractiveMode(decision), + decision, rule: matchedRule, }; } diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 89c8362f5dc..a55bbe3d884 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -8,24 +8,33 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { PolicyEngine } from './policy-engine.js'; import { PolicyDecision } from './types.js'; import type { FunctionCall } from '@google/genai'; +import { buildArgsPatterns } from './toml-loader.js'; describe('Shell Safety Policy', () => { let policyEngine: PolicyEngine; - beforeEach(() => { - policyEngine = new PolicyEngine({ + // Helper to create a policy engine with a simple command prefix rule + function createPolicyEngineWithPrefix(prefix: string) { + const argsPatterns = buildArgsPatterns(undefined, prefix, undefined); + // Since buildArgsPatterns returns array of patterns (strings), we pick the first one + // and compile it. + const argsPattern = new RegExp(argsPatterns[0]!); + + return new PolicyEngine({ rules: [ { toolName: 'run_shell_command', - // Mimic the regex generated by toml-loader for commandPrefix = ["git log"] - // Regex: "command":"git log(?:[\s"]|$) - argsPattern: /"command":"git log(?:[\s"]|$)/, + argsPattern, decision: PolicyDecision.ALLOW, - priority: 1.01, // Higher priority than default + priority: 1.01, }, ], defaultDecision: PolicyDecision.ASK_USER, }); + } + + beforeEach(() => { + policyEngine = createPolicyEngineWithPrefix('git log'); }); it('SHOULD match "git log" exactly', async () => { @@ -70,6 +79,25 @@ describe('Shell Safety Policy', () => { const result = await policyEngine.check(toolCall, undefined); expect(result.decision).toBe(PolicyDecision.ASK_USER); }); + + it('SHOULD NOT allow "git log; rm -rf /" (semicolon separator)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log; rm -rf /' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow "git log || rm -rf /" (OR separator)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log || rm -rf /' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + it('SHOULD NOT allow "git log &&& rm -rf /" when prefix is "git log" (parse failure)', async () => { const toolCall: FunctionCall = { name: 'run_shell_command', @@ -77,7 +105,164 @@ describe('Shell Safety Policy', () => { }; // Desired behavior: Should fail safe (ASK_USER or DENY) because parsing failed. - // If we let it pass as "single command" that matches prefix, it's dangerous. + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow command substitution $(rm -rf /)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'echo $(rm -rf /)' }, + }; + // `splitCommands` recursively finds nested commands (e.g., `rm` inside `echo $()`). + // The policy engine requires ALL extracted commands to be allowed. + // Since `rm` does not match the allowed prefix, this should result in ASK_USER. + + // Let's try with a rule that allows `echo` + const echoPolicy = createPolicyEngineWithPrefix('echo'); + const result = await echoPolicy.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD allow command substitution if inner command is ALSO allowed', async () => { + // Both `echo` and `git` allowed. + const argsPatternsEcho = buildArgsPatterns(undefined, 'echo', undefined); + const argsPatternsGit = buildArgsPatterns(undefined, 'git', undefined); // Allow all git + + const policyEngineWithBoth = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsEcho[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + }, + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsGit[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'echo $(git log)' }, + }; + + const result = await policyEngineWithBoth.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + it('SHOULD NOT allow command substitution with backticks `rm -rf /`', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'echo `rm -rf /`' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow process substitution <(rm -rf /)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'diff <(git log) <(rm -rf /)' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow process substitution >(rm -rf /)', async () => { + // Note: >(...) is output substitution, but syntax is similar. + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'tee >(rm -rf /)' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow piped commands "git log | rm -rf /"', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log | rm -rf /' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow argument injection via --arg=$(rm -rf /)', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log --format=$(rm -rf /)' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD NOT allow complex nested commands "git log && echo $(git log | rm -rf /)"', async () => { + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log && echo $(git log | rm -rf /)' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + }); + + it('SHOULD allow complex allowed commands "git log && echo $(git log)"', async () => { + // Both `echo` and `git` allowed. + const argsPatternsEcho = buildArgsPatterns(undefined, 'echo', undefined); + const argsPatternsGit = buildArgsPatterns(undefined, 'git', undefined); + + const policyEngineWithBoth = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsEcho[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + }, + { + toolName: 'run_shell_command', + // Matches "git" at start of *subcommand* + argsPattern: new RegExp(argsPatternsGit[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log && echo $(git log)' }, + }; + + const result = await policyEngineWithBoth.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('SHOULD NOT allow generic redirection > /dev/null with unsafe target', async () => { + // Current logic allows redirections if the main command is allowed, + // as `splitCommands` (bash parser) sees `git log > file` as just `git log`. + // This test documents current behavior: it IS allowed. + // If usage of redirection needs to be blocked, we'd need stricter policies. + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log > /tmp/test' }, + }; + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + + it('SHOULD NOT allow PowerShell @(...) usage if it implies code execution', async () => { + // Bash parser fails on PowerShell syntax @(...) (returns empty subcommands). + // The policy engine correctly identifies this as unparseable and falls back to ASK_USER. + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log @(Get-Process)' }, + }; const result = await policyEngine.check(toolCall, undefined); expect(result.decision).toBe(PolicyDecision.ASK_USER); }); diff --git a/packages/core/src/policy/toml-loader.ts b/packages/core/src/policy/toml-loader.ts index 79454c2e988..8e3775e5b4f 100644 --- a/packages/core/src/policy/toml-loader.ts +++ b/packages/core/src/policy/toml-loader.ts @@ -332,26 +332,11 @@ export async function loadPoliciesFromToml( return rule.modes.includes(approvalMode); }) .flatMap((rule) => { - // Transform commandPrefix/commandRegex to argsPattern - let effectiveArgsPattern = rule.argsPattern; - const commandPrefixes: string[] = []; - - if (rule.commandPrefix) { - const prefixes = Array.isArray(rule.commandPrefix) - ? rule.commandPrefix - : [rule.commandPrefix]; - commandPrefixes.push(...prefixes); - } else if (rule.commandRegex) { - effectiveArgsPattern = `"command":"${rule.commandRegex}`; - } - - // Expand command prefixes to multiple patterns - const argsPatterns: Array = - commandPrefixes.length > 0 - ? commandPrefixes.map( - (prefix) => `"command":"${escapeRegex(prefix)}(?:[\\s"]|$)`, - ) - : [effectiveArgsPattern]; + const argsPatterns = buildArgsPatterns( + rule.argsPattern, + rule.commandPrefix, + rule.commandRegex, + ); // For each argsPattern, expand toolName arrays return argsPatterns.flatMap((argsPattern) => { @@ -419,24 +404,11 @@ export async function loadPoliciesFromToml( return checker.modes.includes(approvalMode); }) .flatMap((checker) => { - let effectiveArgsPattern = checker.argsPattern; - const commandPrefixes: string[] = []; - - if (checker.commandPrefix) { - const prefixes = Array.isArray(checker.commandPrefix) - ? checker.commandPrefix - : [checker.commandPrefix]; - commandPrefixes.push(...prefixes); - } else if (checker.commandRegex) { - effectiveArgsPattern = `"command":"${checker.commandRegex}`; - } - - const argsPatterns: Array = - commandPrefixes.length > 0 - ? commandPrefixes.map( - (prefix) => `"command":"${escapeRegex(prefix)}(?:[\\s"]|$)`, - ) - : [effectiveArgsPattern]; + const argsPatterns = buildArgsPatterns( + checker.argsPattern, + checker.commandPrefix, + checker.commandRegex, + ); return argsPatterns.flatMap((argsPattern) => { const toolNames: Array = checker.toolName @@ -504,3 +476,32 @@ export async function loadPoliciesFromToml( return { rules, checkers, errors }; } + +/** + * Helper to build arg patterns from configuration. + * Extracted to reduce duplication and allow testing. + */ +export function buildArgsPatterns( + argsPattern?: string, + commandPrefix?: string | string[], + commandRegex?: string, +): Array { + let effectiveArgsPattern = argsPattern; + const commandPrefixes: string[] = []; + + if (commandPrefix) { + const prefixes = Array.isArray(commandPrefix) + ? commandPrefix + : [commandPrefix]; + commandPrefixes.push(...prefixes); + } else if (commandRegex) { + effectiveArgsPattern = `"command":"${commandRegex}`; + } + + // Expand command prefixes to multiple patterns. + // We append [\s"] to ensure we match whole words only (e.g., "git" but not "github"). + // Since we match against JSON stringified args, the value is always followed by a space or a closing quote. + return commandPrefixes.length > 0 + ? commandPrefixes.map((prefix) => `"command":"${escapeRegex(prefix)}[\\s"]`) + : [effectiveArgsPattern]; +} From 78cfc42e5f14823ac436de38870f78e14a718888 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Mon, 15 Dec 2025 09:06:03 -0800 Subject: [PATCH 02/10] Update packages/core/src/policy/policy-engine.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- packages/core/src/policy/policy-engine.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index da538fa1da3..f73046e0187 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -234,7 +234,7 @@ export class PolicyEngine { decision = rule.decision; } - matchedRule = rule; + if (!matchedRule) matchedRule = rule; break; } } From 0b2a5f6f9cce976210bc1cbfae177a58aeb566eb Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Mon, 15 Dec 2025 13:36:06 -0800 Subject: [PATCH 03/10] fix(policy): correct rule attribution for compound shell commands This change fixes an issue where the PolicyEngine would not correctly attribute the rule that caused an ASK_USER decision when processing compound shell commands (e.g., 'git status && unknown_cmd'). It ensures that if a subcommand triggers an ASK_USER decision (either by default or explicit rule), the 'matchedRule' reflects that specific cause, rather than being overwritten or left undefined. Also adds a 'name' property to PolicyRule for better debugging and updates tests to cover these scenarios. --- packages/core/src/policy/policy-engine.ts | 111 ++++++------- packages/core/src/policy/shell-safety.test.ts | 150 ++++++++++++++++++ packages/core/src/policy/types.ts | 5 + 3 files changed, 211 insertions(+), 55 deletions(-) diff --git a/packages/core/src/policy/policy-engine.ts b/packages/core/src/policy/policy-engine.ts index f73046e0187..54f27bfb51d 100644 --- a/packages/core/src/policy/policy-engine.ts +++ b/packages/core/src/policy/policy-engine.ts @@ -180,61 +180,8 @@ export class PolicyEngine { `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`, ); - if (isShellCommand && rule.decision === PolicyDecision.ALLOW) { - // For shell commands, if the matching rule is ALLOW, we must verify ALL subcommands allow it. - // If subsCommands is set (length > 0), we check them. - if (subCommands && subCommands.length > 0) { - if (subCommands.length > 1) { - debugLogger.debug( - `[PolicyEngine.check] Compound command detected: ${subCommands.length} parts`, - ); - let aggregateDecision = PolicyDecision.ALLOW; - // We need to check if *any* subcommand fails the check. - // Note: We are recursively checking subcommands against the FULL rule set. - // This ensures that "git log" matches the "git log" rule, - // and "rm -rf /" matches (or doesn't match) its own rules. - - for (const subCmd of subCommands) { - // Prevent infinite recursion if the subcommand is identical to the original command. - // This happens when splitCommands returns the root command along with its children. - // We use trimmed comparison to be robust against whitespace differences. - if (command && subCmd.trim() === command.trim()) { - debugLogger.debug( - `[PolicyEngine.check] Skipping recursion for self-referential command: "${subCmd.trim()}" vs original: "${command.trim()}"`, - ); - continue; - } - - const subCall = { - name: toolCall.name, - args: { command: subCmd }, - }; - const subResult = await this.check(subCall, serverName); - - if (subResult.decision === PolicyDecision.DENY) { - aggregateDecision = PolicyDecision.DENY; - matchedRule = subResult.rule ?? rule; // Blame the rule that denied it - break; - } else if (subResult.decision === PolicyDecision.ASK_USER) { - aggregateDecision = PolicyDecision.ASK_USER; - if (!matchedRule) matchedRule = subResult.rule ?? rule; - } - } - decision = aggregateDecision; - } else { - // Single command found. Rely on the initial rule match. - decision = rule.decision; - } - } else { - // No subcommands found (should have been caught by generic check above unless command was empty) - decision = rule.decision; - } - } else { - // Not a shell command OR decision is not ALLOW (DENY/ASK_USER apply immediately) - decision = rule.decision; - } - - if (!matchedRule) matchedRule = rule; + matchedRule = rule; + decision = rule.decision; break; } } @@ -247,6 +194,60 @@ export class PolicyEngine { decision = this.defaultDecision; } + if (isShellCommand && decision !== PolicyDecision.DENY) { + // For shell commands, if we are not already denying, we must verify ALL subcommands. + // We check subcommands if there's more than one (meaning decomposition happened). + // If subsCommands is set (length > 0), we check them. + if (subCommands && subCommands.length > 1) { + debugLogger.debug( + `[PolicyEngine.check] Compound command detected: ${subCommands.length} parts`, + ); + let aggregateDecision: PolicyDecision = decision; + // We need to check if *any* subcommand fails the check. + // This ensures that "git log" matches the "git log" rule, + // and "rm -rf /" matches (or doesn't match) its own rules. + + for (const subCmd of subCommands) { + // Prevent infinite recursion if the subcommand is identical to the original command. + // This happens when splitCommands returns the root command along with its children. + // We use trimmed comparison to be robust against whitespace differences. + if (command && subCmd.trim() === command.trim()) { + debugLogger.debug( + `[PolicyEngine.check] Skipping recursion for self-referential command: "${subCmd.trim()}" vs original: "${command.trim()}"`, + ); + continue; + } + + const subCall = { + name: toolCall.name, + args: { command: subCmd }, + }; + const subResult = await this.check(subCall, serverName); + + if (subResult.decision === PolicyDecision.DENY) { + aggregateDecision = PolicyDecision.DENY; + matchedRule = subResult.rule ?? matchedRule; // Blame the rule that denied it + break; + } else if (subResult.decision === PolicyDecision.ASK_USER) { + // Downgrade ALLOW to ASK_USER, but keep DENY if already denied + if (aggregateDecision === PolicyDecision.ALLOW) { + aggregateDecision = PolicyDecision.ASK_USER; + matchedRule = subResult.rule ?? undefined; // If explicit rule, use it, else undefined + } else if ( + aggregateDecision === PolicyDecision.ASK_USER && + subResult.rule !== undefined && + matchedRule === undefined + ) { + // If already ASK_USER (e.g., from default), and this subcommand has a specific ASK_USER rule, + // and no specific rule has been blamed yet, then blame this specific rule. + matchedRule = subResult.rule; + } + } + } + decision = aggregateDecision; + } + } + // Apply non-interactive mode constraint to the final decision decision = this.applyNonInteractiveMode(decision); diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index a55bbe3d884..63cfedb08a7 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -266,4 +266,154 @@ describe('Shell Safety Policy', () => { const result = await policyEngine.check(toolCall, undefined); expect(result.decision).toBe(PolicyDecision.ASK_USER); }); + + it('SHOULD match DENY rule even if nested/chained with unknown command', async () => { + // Scenario: + // git commit -m "..." (Unknown/No Rule -> ASK_USER) + // git push (DENY -> DENY) + // Overall should be DENY. + const argsPatternsPush = buildArgsPatterns( + undefined, + 'git push', + undefined, + ); + + const denyPushPolicy = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsPush[0]!), + decision: PolicyDecision.DENY, + priority: 2, + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git commit -m "msg" && git push' }, + }; + + const result = await denyPushPolicy.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.DENY); + }); + + it('SHOULD aggregate ALLOW + ASK_USER to ASK_USER and blame the ASK_USER part', async () => { + // Scenario: + // `git status` (ALLOW) && `unknown_command` (ASK_USER by default) + // Expected: ASK_USER, and the matched rule should be related to the unknown_command + const argsPatternsGitStatus = buildArgsPatterns( + undefined, + 'git status', + undefined, + ); + + const policyEngine = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsGitStatus[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + name: 'allow_git_status_rule', // Give a name to easily identify + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git status && unknown_command' }, + }; + + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + // Expect the matched rule to be null/undefined since it's the default decision for 'unknown_command' + // or the rule that led to the ASK_USER decision. In this case, it should be the rule for 'unknown_command', which is the default decision. + // The policy engine's `matchedRule` will be the rule that caused the final decision. + // If it's a default ASK_USER, then `result.rule` should be undefined. + expect(result.rule).toBeUndefined(); + }); + + it('SHOULD aggregate ASK_USER (default) + ASK_USER (rule) to ASK_USER and blame the specific ASK_USER rule', async () => { + // Scenario: + // `unknown_command_1` (ASK_USER by default) && `another_unknown_command` (ASK_USER by explicit rule) + // Expected: ASK_USER, and the matched rule should be the explicit ASK_USER rule + const argsPatternsAnotherUnknown = buildArgsPatterns( + undefined, + 'another_unknown_command', + undefined, + ); + + const policyEngine = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsAnotherUnknown[0]!), + decision: PolicyDecision.ASK_USER, + priority: 2, + name: 'ask_another_unknown_command_rule', + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'unknown_command_1 && another_unknown_command' }, + }; + + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + expect(result.rule?.name).toBe('ask_another_unknown_command_rule'); + }); + + it('SHOULD aggregate ASK_USER (rule) + ASK_USER (rule) to ASK_USER and blame the first specific ASK_USER rule in subcommands', async () => { + // Scenario: + // `known_ask_command_1` (ASK_USER by explicit rule 1) && `known_ask_command_2` (ASK_USER by explicit rule 2) + // Expected: ASK_USER, and the matched rule should be explicit ASK_USER rule 1. + // The current implementation prioritizes the rule that changes the decision to ASK_USER, if any. + // If multiple rules lead to ASK_USER, it takes the first one. + const argsPatternsAsk1 = buildArgsPatterns( + undefined, + 'known_ask_command_1', + undefined, + ); + const argsPatternsAsk2 = buildArgsPatterns( + undefined, + 'known_ask_command_2', + undefined, + ); + + const policyEngine = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsAsk1[0]!), + decision: PolicyDecision.ASK_USER, + priority: 2, + name: 'ask_rule_1', + }, + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsAsk2[0]!), + decision: PolicyDecision.ASK_USER, + priority: 2, + name: 'ask_rule_2', + }, + ], + defaultDecision: PolicyDecision.ALLOW, // Set default to ALLOW to ensure rules are hit + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'known_ask_command_1 && known_ask_command_2' }, + }; + + const result = await policyEngine.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ASK_USER); + // Expect the rule that first caused ASK_USER to be blamed + expect(result.rule?.name).toBe('ask_rule_1'); + }); }); diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 410d9ff1c95..b9aa8d6e442 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -95,6 +95,11 @@ export type SafetyCheckerConfig = | InProcessCheckerConfig; export interface PolicyRule { + /** + * A unique name for the policy rule, useful for identification and debugging. + */ + name?: string; + /** * The name of the tool this rule applies to. * If undefined, the rule applies to all tools. From 10d4cc870f5ce6bb5996f9755dfa0f2a452661c4 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Mon, 15 Dec 2025 14:36:48 -0800 Subject: [PATCH 04/10] fix(test): mock os.platform to linux in shell-safety tests This ensures that the tests, which use Bash-specific syntax (chained commands, subshells), always use the Bash parser in shell-utils. This avoids parsing failures on Windows where the logic would otherwise default to PowerShell parsing. --- packages/core/src/policy/shell-safety.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 63cfedb08a7..132c7e5fbae 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -4,7 +4,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +// Mock os.platform to return 'linux' so that shell-utils uses the bash parser +// even when running tests on Windows. The tests below use bash syntax (&&, $()) +// which may fail to parse or parse differently with the PowerShell parser. +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + platform: () => 'linux', + }; +}); + import { PolicyEngine } from './policy-engine.js'; import { PolicyDecision } from './types.js'; import type { FunctionCall } from '@google/genai'; From 7818004252fb8bb1f3ba3101bfe3f8ec28e48448 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 8 Jan 2026 13:19:48 -0800 Subject: [PATCH 05/10] test(policy): mock shell-utils in shell-safety.test.ts to fix Windows CI --- packages/core/src/policy/shell-safety.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index cb46663d88b..6164afb73f1 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -17,6 +17,76 @@ vi.mock('node:os', async (importOriginal) => { }; }); +// Mock shell-utils to avoid relying on tree-sitter WASM which is flaky in CI on Windows +vi.mock('../utils/shell-utils.js', async (importOriginal) => { + const actual = + await importOriginal(); + + // Static map of test commands to their expected subcommands + // This mirrors what the real parser would output for these specific strings + const commandMap: Record = { + 'git log': ['git log'], + 'git log --oneline': ['git log --oneline'], + 'git logout': ['git logout'], + 'git log && rm -rf /': ['git log', 'rm -rf /'], + 'git log; rm -rf /': ['git log', 'rm -rf /'], + 'git log || rm -rf /': ['git log', 'rm -rf /'], + 'git log &&& rm -rf /': [], // Simulates parse failure + 'echo $(rm -rf /)': ['echo $(rm -rf /)', 'rm -rf /'], + 'echo $(git log)': ['echo $(git log)', 'git log'], + 'echo `rm -rf /`': ['echo `rm -rf /`', 'rm -rf /'], + 'diff <(git log) <(rm -rf /)': [ + 'diff <(git log) <(rm -rf /)', + 'git log', + 'rm -rf /', + ], + 'tee >(rm -rf /)': ['tee >(rm -rf /)', 'rm -rf /'], + 'git log | rm -rf /': ['git log', 'rm -rf /'], + 'git log --format=$(rm -rf /)': [ + 'git log --format=$(rm -rf /)', + 'rm -rf /', + ], + 'git log && echo $(git log | rm -rf /)': [ + 'git log', + 'echo $(git log | rm -rf /)', + 'git log', + 'rm -rf /', + ], + 'git log && echo $(git log)': ['git log', 'echo $(git log)', 'git log'], + 'git log > /tmp/test': ['git log > /tmp/test'], + 'git log @(Get-Process)': [], // Simulates parse failure (Bash parser vs PowerShell syntax) + 'git commit -m "msg" && git push': ['git commit -m "msg"', 'git push'], + 'git status && unknown_command': ['git status', 'unknown_command'], + 'unknown_command_1 && another_unknown_command': [ + 'unknown_command_1', + 'another_unknown_command', + ], + 'known_ask_command_1 && known_ask_command_2': [ + 'known_ask_command_1', + 'known_ask_command_2', + ], + }; + + return { + ...actual, + initializeShellParsers: vi.fn(), + splitCommands: (command: string) => { + if (Object.prototype.hasOwnProperty.call(commandMap, command)) { + return commandMap[command]; + } + // Fallback for simple commands not in map, or throw if strictness preferred + // For now, assume it's atomic if not in map, but logically we should ensure map coverage + const known = commandMap[command]; + if (known) return known; + // Default fallback for unmatched simple cases in development, but explicit map is better + return [command]; + }, + hasRedirection: (command: string) => + // Simple regex check sufficient for testing the policy engine's handling of the *result* of hasRedirection + /[><]/.test(command), + }; +}); + import { PolicyEngine } from './policy-engine.js'; import { PolicyDecision, ApprovalMode } from './types.js'; import type { FunctionCall } from '@google/genai'; From 029e04df8e774e93a27dcebc8be61fc0a339ad00 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 8 Jan 2026 14:19:06 -0800 Subject: [PATCH 06/10] test: remove `os.platform` mock from `shell-safety.test.ts` --- packages/core/src/policy/shell-safety.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 6164afb73f1..ad5b59e49ab 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -6,17 +6,6 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -// Mock os.platform to return 'linux' so that shell-utils uses the bash parser -// even when running tests on Windows. The tests below use bash syntax (&&, $()) -// which may fail to parse or parse differently with the PowerShell parser. -vi.mock('node:os', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - platform: () => 'linux', - }; -}); - // Mock shell-utils to avoid relying on tree-sitter WASM which is flaky in CI on Windows vi.mock('../utils/shell-utils.js', async (importOriginal) => { const actual = From 32c63b18afb94264fdd77e80d132255784f4942e Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 8 Jan 2026 14:19:06 -0800 Subject: [PATCH 07/10] test: remove `os.platform` mock from `shell-safety.test.ts` --- packages/core/src/policy/shell-safety.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index 6164afb73f1..ad5b59e49ab 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -6,17 +6,6 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -// Mock os.platform to return 'linux' so that shell-utils uses the bash parser -// even when running tests on Windows. The tests below use bash syntax (&&, $()) -// which may fail to parse or parse differently with the PowerShell parser. -vi.mock('node:os', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - platform: () => 'linux', - }; -}); - // Mock shell-utils to avoid relying on tree-sitter WASM which is flaky in CI on Windows vi.mock('../utils/shell-utils.js', async (importOriginal) => { const actual = From 343ee91faa2bfa1041a8b6c3a8897ac234928fcc Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 8 Jan 2026 14:25:55 -0800 Subject: [PATCH 08/10] test: add and refine shell safety policy tests for command redirection and the `allowRedirection` rule. --- package-lock.json | 41 +++++++------------ packages/core/src/policy/shell-safety.test.ts | 30 +++++++++++++- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1830ea72950..47196542abe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2501,7 +2501,6 @@ "integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.2", @@ -2682,7 +2681,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -2716,7 +2714,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.0.1.tgz", "integrity": "sha512-MaZk9SJIDgo1peKevlbhP6+IwIiNPNmswNL4AF0WaQJLbHXjr9SrZMgS12+iqr9ToV4ZVosCcc0f8Rg67LXjxw==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -3085,7 +3082,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.0.1.tgz", "integrity": "sha512-dZOB3R6zvBwDKnHDTB4X1xtMArB/d324VsbiPkX/Yu0Q8T2xceRthoIVFhJdvgVM2QhGVUyX9tzwiNxGtoBJUw==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -3119,7 +3115,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.0.1.tgz", "integrity": "sha512-wf8OaJoSnujMAHWR3g+/hGvNcsC16rf9s1So4JlMiFaFHiE4HpIA3oUh+uWZQ7CNuK8gVW/pQSkgoa5HkkOl0g==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/resources": "2.0.1" @@ -3172,7 +3167,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.0.1.tgz", "integrity": "sha512-xYLlvk/xdScGx1aEqvxLwf6sXQLXCjk3/1SQT9X9AoN5rXRhkdvIFShuNNmtTEPRBqcsMbS4p/gJLNI2wXaDuQ==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/resources": "2.0.1", @@ -4410,7 +4404,6 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4688,7 +4681,6 @@ "integrity": "sha512-6sMvZePQrnZH2/cJkwRpkT7DxoAWh+g6+GFRK6bV3YQo7ogi3SX5rgF6099r5Q53Ma5qeT7LGmOmuIutF4t3lA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.35.0", "@typescript-eslint/types": "8.35.0", @@ -5700,7 +5692,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -6145,7 +6136,8 @@ "version": "1.1.1", "resolved": "https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz", "integrity": "sha512-PCVAQswWemu6UdxsDFFX/+gVeYqKAod3D3UVm91jHwynguOwAvYPhx8nNlM++NqRcK6CxxpUafjmhIdKiHibqg==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/array-includes": { "version": "3.1.9", @@ -7433,6 +7425,7 @@ "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz", "integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==", "license": "MIT", + "peer": true, "dependencies": { "safe-buffer": "5.2.1" }, @@ -8755,7 +8748,6 @@ "integrity": "sha512-GsGizj2Y1rCWDu6XoEekL3RLilp0voSePurjZIkxL3wlm5o5EC9VpgaP7lrCvjnkuLvzFBQWB3vWB3K5KQTveQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.12.1", @@ -9360,6 +9352,7 @@ "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.1.tgz", "integrity": "sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==", "license": "MIT", + "peer": true, "engines": { "node": ">= 0.6" } @@ -9369,6 +9362,7 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", + "peer": true, "dependencies": { "ms": "2.0.0" } @@ -9378,6 +9372,7 @@ "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", + "peer": true, "engines": { "node": ">= 0.8" } @@ -9631,6 +9626,7 @@ "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", "integrity": "sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ==", "license": "MIT", + "peer": true, "dependencies": { "debug": "2.6.9", "encodeurl": "~2.0.0", @@ -9649,6 +9645,7 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", + "peer": true, "dependencies": { "ms": "2.0.0" } @@ -9657,13 +9654,15 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/finalhandler/node_modules/statuses": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", + "peer": true, "engines": { "node": ">= 0.8" } @@ -10947,7 +10946,6 @@ "resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.4.6.tgz", "integrity": "sha512-QHl6l1cl3zPCaRMzt9TUbTX6Q5SzvkGEZDDad0DmSf5SPmT1/90k6pGPejEvDCJprkitwObXpPaTWGHItqsy4g==", "license": "MIT", - "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.0.0", @@ -14136,7 +14134,8 @@ "version": "0.1.12", "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.12.tgz", "integrity": "sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==", - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/path-type": { "version": "3.0.0", @@ -14716,7 +14715,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.0.tgz", "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -14727,7 +14725,6 @@ "integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -16972,7 +16969,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -17199,8 +17195,7 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD", - "peer": true + "license": "0BSD" }, "node_modules/tsx": { "version": "4.20.3", @@ -17208,7 +17203,6 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -17392,7 +17386,6 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -17555,6 +17548,7 @@ "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", "integrity": "sha512-pMZTvIkT1d+TFGvDOqodOclx0QWkkgi6Tdoa8gC8ffGAAqz9pzPTZWAybbsHHoED/ztMtkv/VoYTYyShUn81hA==", "license": "MIT", + "peer": true, "engines": { "node": ">= 0.4.0" } @@ -17610,7 +17604,6 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -17727,7 +17720,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -17741,7 +17733,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -18448,7 +18439,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -19013,7 +19003,6 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index ad5b59e49ab..eb040602790 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -315,7 +315,7 @@ describe('Shell Safety Policy', () => { expect(result.decision).toBe(PolicyDecision.ALLOW); }); - it('SHOULD NOT allow generic redirection > /dev/null with unsafe target', async () => { + it('SHOULD NOT allow generic redirection > /tmp/test', async () => { // Current logic downgrades ALLOW to ASK_USER for redirections if redirection is not explicitly allowed. const toolCall: FunctionCall = { name: 'run_shell_command', @@ -325,6 +325,34 @@ describe('Shell Safety Policy', () => { expect(result.decision).toBe(PolicyDecision.ASK_USER); }); + it('SHOULD allow generic redirection > /tmp/test if allowRedirection is true', async () => { + // If PolicyRule has allowRedirection: true, it should stay ALLOW + const argsPatternsGitLog = buildArgsPatterns( + undefined, + 'git log', + undefined, + ); + const policyWithRedirection = new PolicyEngine({ + rules: [ + { + toolName: 'run_shell_command', + argsPattern: new RegExp(argsPatternsGitLog[0]!), + decision: PolicyDecision.ALLOW, + priority: 2, + allowRedirection: true, + }, + ], + defaultDecision: PolicyDecision.ASK_USER, + }); + + const toolCall: FunctionCall = { + name: 'run_shell_command', + args: { command: 'git log > /tmp/test' }, + }; + const result = await policyWithRedirection.check(toolCall, undefined); + expect(result.decision).toBe(PolicyDecision.ALLOW); + }); + it('SHOULD NOT allow PowerShell @(...) usage if it implies code execution', async () => { // Bash parser fails on PowerShell syntax @(...) (returns empty subcommands). // The policy engine correctly identifies this as unparseable and falls back to ASK_USER. From 6e2ab2d00cef9fd4812f55160f7f8d40f7fa5828 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Thu, 8 Jan 2026 14:33:32 -0800 Subject: [PATCH 09/10] revert package-lock.json --- package-lock.json | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 47196542abe..1830ea72950 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2501,6 +2501,7 @@ "integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.2", @@ -2681,6 +2682,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", + "peer": true, "engines": { "node": ">=8.0.0" } @@ -2714,6 +2716,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.0.1.tgz", "integrity": "sha512-MaZk9SJIDgo1peKevlbhP6+IwIiNPNmswNL4AF0WaQJLbHXjr9SrZMgS12+iqr9ToV4ZVosCcc0f8Rg67LXjxw==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -3082,6 +3085,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.0.1.tgz", "integrity": "sha512-dZOB3R6zvBwDKnHDTB4X1xtMArB/d324VsbiPkX/Yu0Q8T2xceRthoIVFhJdvgVM2QhGVUyX9tzwiNxGtoBJUw==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -3115,6 +3119,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.0.1.tgz", "integrity": "sha512-wf8OaJoSnujMAHWR3g+/hGvNcsC16rf9s1So4JlMiFaFHiE4HpIA3oUh+uWZQ7CNuK8gVW/pQSkgoa5HkkOl0g==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/resources": "2.0.1" @@ -3167,6 +3172,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.0.1.tgz", "integrity": "sha512-xYLlvk/xdScGx1aEqvxLwf6sXQLXCjk3/1SQT9X9AoN5rXRhkdvIFShuNNmtTEPRBqcsMbS4p/gJLNI2wXaDuQ==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/resources": "2.0.1", @@ -4404,6 +4410,7 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4681,6 +4688,7 @@ "integrity": "sha512-6sMvZePQrnZH2/cJkwRpkT7DxoAWh+g6+GFRK6bV3YQo7ogi3SX5rgF6099r5Q53Ma5qeT7LGmOmuIutF4t3lA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.35.0", "@typescript-eslint/types": "8.35.0", @@ -5692,6 +5700,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -6136,8 +6145,7 @@ "version": "1.1.1", "resolved": "https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz", "integrity": "sha512-PCVAQswWemu6UdxsDFFX/+gVeYqKAod3D3UVm91jHwynguOwAvYPhx8nNlM++NqRcK6CxxpUafjmhIdKiHibqg==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/array-includes": { "version": "3.1.9", @@ -7425,7 +7433,6 @@ "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz", "integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==", "license": "MIT", - "peer": true, "dependencies": { "safe-buffer": "5.2.1" }, @@ -8748,6 +8755,7 @@ "integrity": "sha512-GsGizj2Y1rCWDu6XoEekL3RLilp0voSePurjZIkxL3wlm5o5EC9VpgaP7lrCvjnkuLvzFBQWB3vWB3K5KQTveQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.12.1", @@ -9352,7 +9360,6 @@ "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.1.tgz", "integrity": "sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.6" } @@ -9362,7 +9369,6 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", - "peer": true, "dependencies": { "ms": "2.0.0" } @@ -9372,7 +9378,6 @@ "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.8" } @@ -9626,7 +9631,6 @@ "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", "integrity": "sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ==", "license": "MIT", - "peer": true, "dependencies": { "debug": "2.6.9", "encodeurl": "~2.0.0", @@ -9645,7 +9649,6 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", - "peer": true, "dependencies": { "ms": "2.0.0" } @@ -9654,15 +9657,13 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/finalhandler/node_modules/statuses": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.8" } @@ -10946,6 +10947,7 @@ "resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.4.6.tgz", "integrity": "sha512-QHl6l1cl3zPCaRMzt9TUbTX6Q5SzvkGEZDDad0DmSf5SPmT1/90k6pGPejEvDCJprkitwObXpPaTWGHItqsy4g==", "license": "MIT", + "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.0.0", @@ -14134,8 +14136,7 @@ "version": "0.1.12", "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.12.tgz", "integrity": "sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/path-type": { "version": "3.0.0", @@ -14715,6 +14716,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.0.tgz", "integrity": "sha512-tmbWg6W31tQLeB5cdIBOicJDJRR2KzXsV7uSK9iNfLWQ5bIZfxuPEHp7M8wiHyHnn0DD1i7w3Zmin0FtkrwoCQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -14725,6 +14727,7 @@ "integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -16969,6 +16972,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -17195,7 +17199,8 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD" + "license": "0BSD", + "peer": true }, "node_modules/tsx": { "version": "4.20.3", @@ -17203,6 +17208,7 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -17386,6 +17392,7 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -17548,7 +17555,6 @@ "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", "integrity": "sha512-pMZTvIkT1d+TFGvDOqodOclx0QWkkgi6Tdoa8gC8ffGAAqz9pzPTZWAybbsHHoED/ztMtkv/VoYTYyShUn81hA==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.4.0" } @@ -17604,6 +17610,7 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -17720,6 +17727,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -17733,6 +17741,7 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -18439,6 +18448,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -19003,6 +19013,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, From 8a756846fb5921f6ff5f61492cf7780f7bcc7aa6 Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Mon, 12 Jan 2026 10:24:51 -0800 Subject: [PATCH 10/10] chore: remove redundant comments from shell safety policy test file. --- packages/core/src/policy/shell-safety.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/policy/shell-safety.test.ts b/packages/core/src/policy/shell-safety.test.ts index eb040602790..340264485ec 100644 --- a/packages/core/src/policy/shell-safety.test.ts +++ b/packages/core/src/policy/shell-safety.test.ts @@ -63,8 +63,6 @@ vi.mock('../utils/shell-utils.js', async (importOriginal) => { if (Object.prototype.hasOwnProperty.call(commandMap, command)) { return commandMap[command]; } - // Fallback for simple commands not in map, or throw if strictness preferred - // For now, assume it's atomic if not in map, but logically we should ensure map coverage const known = commandMap[command]; if (known) return known; // Default fallback for unmatched simple cases in development, but explicit map is better @@ -189,8 +187,6 @@ describe('Shell Safety Policy', () => { // `splitCommands` recursively finds nested commands (e.g., `rm` inside `echo $()`). // The policy engine requires ALL extracted commands to be allowed. // Since `rm` does not match the allowed prefix, this should result in ASK_USER. - - // Let's try with a rule that allows `echo` const echoPolicy = createPolicyEngineWithPrefix('echo'); const result = await echoPolicy.check(toolCall, undefined); expect(result.decision).toBe(PolicyDecision.ASK_USER);