Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
isolomatov-gd wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −2
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/hooks/src/hooks/dangerous-actions/evaluate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ const MARKER_FIELDS_BY_TOOL: Readonly<Record<string, readonly string[]>> = {
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 };

Expand Down
35 changes: 35 additions & 0 deletions src/hooks/tests/dangerous-actions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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(
Expand Down