From b35397608d0271d14e0957c94825432c8683a9a7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 2 Sep 2026 21:16:29 +0000 Subject: [PATCH 1/3] #173: fix MCP marker fields --- .../src/hooks/dangerous-actions/evaluate.ts | 5 +-- src/hooks/tests/dangerous-actions.test.ts | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) 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( From c4ddad5ff1c53b657347586e5730539717d8230a Mon Sep 17 00:00:00 2001 From: isolomatov-gd Date: Mon, 19 Oct 2026 13:07:43 -0400 Subject: [PATCH 2/3] #173: clarify marker-eligible MCP fields comment Co-Authored-By: Claude Opus 5.5 --- src/hooks/src/hooks/dangerous-actions/evaluate.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/src/hooks/dangerous-actions/evaluate.ts b/src/hooks/src/hooks/dangerous-actions/evaluate.ts index f64e1ba87..c1d40cac9 100644 --- a/src/hooks/src/hooks/dangerous-actions/evaluate.ts +++ b/src/hooks/src/hooks/dangerous-actions/evaluate.ts @@ -29,8 +29,8 @@ const MARKER_FIELDS_BY_TOOL: Readonly> = { 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. +// 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 }; From fbdc26e50000819e9c33d1d70e75423c80003d86 Mon Sep 17 00:00:00 2001 From: isolomatov-gd Date: Mon, 19 Oct 2026 13:15:58 -0400 Subject: [PATCH 3/3] #173: document that danger patterns must not contain the marker token Co-Authored-By: Claude Opus 5.5 --- src/hooks/src/hooks/dangerous-actions/evaluate.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hooks/src/hooks/dangerous-actions/evaluate.ts b/src/hooks/src/hooks/dangerous-actions/evaluate.ts index c1d40cac9..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/;