diff --git a/src/hooks/src/hooks/dangerous-actions/evaluate.ts b/src/hooks/src/hooks/dangerous-actions/evaluate.ts index 097de8b9c..f64e1ba87 100644 --- a/src/hooks/src/hooks/dangerous-actions/evaluate.ts +++ b/src/hooks/src/hooks/dangerous-actions/evaluate.ts @@ -26,11 +26,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 only in writable MCP payload fields. 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(