diff --git a/src/hooks/src/hooks/dangerous-actions/evaluate.ts b/src/hooks/src/hooks/dangerous-actions/evaluate.ts index 097de8b9c..4c46dde8a 100644 --- a/src/hooks/src/hooks/dangerous-actions/evaluate.ts +++ b/src/hooks/src/hooks/dangerous-actions/evaluate.ts @@ -13,6 +13,7 @@ import { * Matches the `Rosetta-AI-reviewed` brand token with word boundaries on both sides. * Accepts any surrounding context: `# Rosetta-AI-reviewed`, `-- Rosetta-AI-reviewed`, * plain `Rosetta-AI-reviewed`. Rejects merged words like `XRosetta-AI-reviewedY`. + * Danger patterns in `patterns.ts` must never contain this token, or a match overrides itself. */ const MARKER_RE = /\bRosetta-AI-reviewed\b/; @@ -26,11 +27,12 @@ const MARKER_FIELDS_BY_TOOL: Readonly> = { MultiEdit: ['edits'], }; -const MCP_MARKER_FIELDS = ['command', 'sql', 'query', 'new_string', 'content'] as const; - const MCP_SHELL_FIELDS = ['command', 'cmd', 'shell_command'] as const; const MCP_PATH_FIELDS = ['path', 'file_path', 'filePath', 'target', 'target_path'] as const; const MCP_CONTENT_FIELDS = ['content', 'new_string', 'query', 'sql'] as const; +// Markers are accepted in the fields the agent can append them to (shell and content). +// Paths remain excluded so an override cannot be carried solely by changing an operation target. +const MCP_MARKER_FIELDS = [...new Set([...MCP_SHELL_FIELDS, ...MCP_CONTENT_FIELDS])] as const; type PatternHit = { result: HookResult; pattern: DangerPattern | null }; diff --git a/src/hooks/tests/dangerous-actions.test.ts b/src/hooks/tests/dangerous-actions.test.ts index 5f5f598d1..ab3965b83 100644 --- a/src/hooks/tests/dangerous-actions.test.ts +++ b/src/hooks/tests/dangerous-actions.test.ts @@ -650,6 +650,13 @@ describe('evaluateDangerous — MCP tool calls (mcp-call kind)', () => { expect((r as {kind:'deny';reason:string}).reason).toContain('rm-rf-root'); }); + test('mcp shell command with marker only in path → deny (paths cannot override)', () => { + expect(evaluateDangerous(mcpCtx( + 'mcp__plugin_serena_serena__execute_shell_command', + { command: 'rm -rf /tmp/x', path: '/tmp/Rosetta-AI-reviewed' } + ))?.kind).toBe('deny'); + }); + test('mcp filesystem write_file to .aws/credentials → advise aws-credentials (non-blocking)', () => { const r = evaluateDangerous(mcpCtx( 'mcp__filesystem__write_file', @@ -718,6 +725,34 @@ describe('evaluateDangerous — MCP tool calls (mcp-call kind)', () => { expect(r).toBeNull(); }); + test('mcp shell cmd with `# Rosetta-AI-reviewed` → null (marker applies to every MCP shell field)', () => { + expect(evaluateDangerous(mcpCtx( + 'mcp__shell__run', + { cmd: 'rm -rf /tmp/x # Rosetta-AI-reviewed' } + ))).toBeNull(); + }); + + test('mcp shell cmd without marker → deny', () => { + expect(evaluateDangerous(mcpCtx( + 'mcp__shell__run', + { cmd: 'rm -rf /tmp/x' } + ))?.kind).toBe('deny'); + }); + + test('mcp shell shell_command with `# Rosetta-AI-reviewed` → null (marker applies to every MCP shell field)', () => { + expect(evaluateDangerous(mcpCtx( + 'mcp__shell__run', + { shell_command: 'rm -rf /tmp/x # Rosetta-AI-reviewed' } + ))).toBeNull(); + }); + + test('mcp shell shell_command without marker → deny', () => { + expect(evaluateDangerous(mcpCtx( + 'mcp__shell__run', + { shell_command: 'rm -rf /tmp/x' } + ))?.kind).toBe('deny'); + }); + // Obj9: MCP marker in query field (not just command) test('mcp postgres query with TRUNCATE + marker in query → null (query field in MCP_MARKER_FIELDS)', () => { const r = evaluateDangerous(mcpCtx(