[ROSETTA] dangerous-actions hook: MCP cmd/shell_command fields can't carry override marker #173

evaluate.ts's MCP_SHELL_FIELDS includes cmd and shell_command (scanned for danger patterns), but MCP_MARKER_FIELDS omits both, so hasAIReviewedMarker never finds a marker placed there. An MCP tool using either field name gets soft-denied with no way to append # Rosetta-AI-reviewed and proceed, violating the documented always-overridable guarantee. Fix by adding cmd/shell_command to MCP_MARKER_FIELDS and add a test alongside the existing command-field marker test (dangerous-actions.test.ts:713-719).

🤖 Rosetta Plan

Size: SMALL

Objective

Fix the unreachable-override bug in evaluate.ts so MCP tool calls with dangerous shell patterns in cmd or shell_command fields can be overridden via the # Rosetta-AI-reviewed marker, same as command.

Root cause

MCP_MARKER_FIELDS is a hand-maintained list that silently diverged from MCP_SHELL_FIELDS (cmd, shell_command are shell fields but not marker fields) — anyone adding a new shell/content field must remember to update two lists, and nobody did.

Approach

• Derive MCP_MARKER_FIELDS as the set union of MCP_SHELL_FIELDS and MCP_CONTENT_FIELDS, instead of hand-listing it, so this class of bug is structurally impossible going forward.
• MCP_PATH_FIELDS stays excluded from the union — confirmed intentional (comment at evaluate.ts:19-21: the marker must never double as an operation-target override); this is a different, correct design decision, not the bug — do not touch it.
• Reorder the three MCP_*_FIELDS constants so MCP_SHELL_FIELDS/MCP_CONTENT_FIELDS are declared before the now-derived MCP_MARKER_FIELDS.
• No changes needed to hasAIReviewedMarker or evalMcpCall — both already consume these constants correctly; only the constant definition is wrong.
• Additive test coverage only; no existing test behavior changes.

Files to modify

• src/hooks/src/hooks/dangerous-actions/evaluate.ts (lines 29-33) — replace hand-maintained MCP_MARKER_FIELDS with a derived union; reorder declarations.
• src/hooks/tests/dangerous-actions.test.ts — add two tests near lines 713-728 covering cmd and shell_command marker override.

Tech Specs

Code change — in evaluate.ts, replace:

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;

with:

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;

/** Marker-eligible fields = shell fields union content fields. MCP_PATH_FIELDS is
 *  deliberately excluded (see comment above MARKER_FIELDS_BY_TOOL) — the marker
 *  must never double as an operation-target override. Derived rather than
 *  hand-maintained so a newly added shell/content field can never silently lack
 *  marker coverage — this divergence was the root cause of issue #173. */
const MCP_MARKER_FIELDS = [...new Set([...MCP_SHELL_FIELDS, ...MCP_CONTENT_FIELDS])] as const;

No other lines change.

Tests to add — in dangerous-actions.test.ts, after the existing command-field marker test (lines 713-719), same describe block, using the existing mcpCtx helper: two new tests covering the cmd field and the shell_command field, each with a dangerous shell pattern plus the # Rosetta-AI-reviewed marker in that same field, asserting evaluateDangerous(...) returns null — mirroring the exact shape of the existing command-field test at lines 713-719 (tool name e.g. mcp__some_plugin__run_cmd / mcp__some_plugin__run_shell, payload { cmd: 'rm -rf /tmp/x # Rosetta-AI-reviewed' } / { shell_command: 'rm -rf /tmp/x # Rosetta-AI-reviewed' }).

If the suite has no existing no-marker deny precedent for cmd/shell_command fields, add matching regression-guard tests (same shape, no marker, expect deny kind); skip if equivalent coverage already exists.

Acceptance Criteria

• evaluateDangerous returns null for an mcp-call context with a dangerous shell pattern in cmd or shell_command plus # Rosetta-AI-reviewed in that same field.
• evaluateDangerous still soft-denies (non-null, reconsider) for cmd/shell_command with a dangerous pattern and no marker (regression guard).
• All pre-existing tests in dangerous-actions.test.ts pass unchanged, including the command/query marker precedents at lines 713-728.
• MCP_MARKER_FIELDS contains exactly MCP_SHELL_FIELDS union MCP_CONTENT_FIELDS, verifiable by inspection.

Testing Strategy

Run src/hooks unit test suite (Vitest) covering dangerous-actions.test.ts; no manual/E2E testing needed for this pure-function fix.

Risks / Open Questions

• None blocking. Checked: MCP_CONTENT_FIELDS was already a full subset of the old MCP_MARKER_FIELDS — no equivalent latent gap there. MCP_PATH_FIELDS exclusion is intentional design, out of scope.
• Field order in the derived array is irrelevant (hasAIReviewedMarker uses .some()).
• After editing, run venv/bin/python scripts/pre_commit.py per ARCHITECTURE.md — Development, since src/* changed (builds/tests hook bundles).