diff --git a/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md b/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md index 0c9a60d20c..bd44f61fec 100644 --- a/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md +++ b/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md @@ -96,3 +96,20 @@ feature: > `tool_result`), replace via `hookSpecificOutput.updatedToolOutput`, and > enforce unicode → canary → suppress → redact in `posttool_chain.py`. See > [runtimes.md](../runtimes.md#sandbox-hook-contract). + +> **Done ([#608](https://github.com/fullsend-ai/fullsend/issues/608)):** +> The canonical Claude tool-name vocabulary is recorded once in +> `security.CanonicalClaudeTools` (with `security.LegacyClaudeTools` for +> names agents and adapters still use), mirrored into +> `tool_allowlist_pretool.py` and kept identical by a Go test; `HookPlan` +> tools and the pi adapter's maps are tested against it. The allowlist hook +> stays exact-match and fail-closed — no case-insensitive allowing — but a +> blocked name that is a case variant of an allowlisted entry is reported as +> a normalization gap (`tool_name_unnormalized` when the adapter did not +> translate, `allowlist_entry_unnormalized` when the allowlist is the +> non-canonical side, `tool_name_case_collision` when neither spelling is +> a Claude tool; all `ALLOWLIST_HOOK_ERROR`, severity `high`) rather than +> as a forbidden tool (`tool_blocked`, `critical`). MCP names are matched +> verbatim. The pi adapter's maps are held to canonical-or-legacy names +> (`ls` → `LS`), a deliberate relaxation of "canonical only". See +> [runtimes.md](../runtimes.md#sandbox-hook-contract). diff --git a/docs/runtimes.md b/docs/runtimes.md index e97377f3df..ab79fa9ec0 100644 --- a/docs/runtimes.md +++ b/docs/runtimes.md @@ -121,6 +121,7 @@ The hook scripts in `internal/security/hooks/*.py` are plain programs with no Cl - **Files:** `security.HookFiles(cfg)` returns `filename → script bytes` for the enabled hooks; `runtime.installHookScripts(sandbox, dir, cfg)` creates `dir` in the sandbox and uploads them there (executable) — any directory works. Claude uses `/sandbox/claude-config/hooks/` (`security.SandboxHooksDir`), with the wiring at `/sandbox/claude-config/hooks.json` (`security.SandboxHooksSettings`) loaded via `--settings`. - **Wiring:** `security.HookPlan(cfg)` returns ordered `HookGroup{Phase, Tools, Scripts}` entries. `Phase` is `PreToolUse`, `PostToolUse` or `PostToolUseFailure` (the last carries Claude Code's failed-call payload — `hook_event_name`, `tool_name`, `tool_input`, a string `error` — and allows no output rewrite, so the chain halts there on a canary and otherwise only detects (logging credential-shaped and control content and returning an `additionalContext` warning); adapters whose post-tool event already fires for failed calls, like pi, map it onto nothing); `Tools` are Claude Code tool names (`Bash`, `Read`, `WebFetch`, `*` = all) — runtimes with other names translate before matching (see #608). PostToolUse is a **single** `posttool_chain.py` script on `*` that applies unicode → canary → suppress → redact in-process. Unicode normalization runs first because every later content decision is made on its output: an attacker who splits a canary or a secret with zero-width or fullwidth characters must not evade detection and then have the chain reassemble the clean value (Claude Code runs matching hooks in parallel and does not merge two `updatedToolOutput` rewrites). Individual sanitizer files and `canary_posttool.py` are shipped as libraries the driver imports; adapters should invoke the chain, not the stages. `GenerateHooksConfig` is rendered from `HookPlan`, so the two cannot diverge. +- **Canonical tool-name vocabulary (#608):** `security.CanonicalClaudeTools` (`internal/security/canonical_tools.go`) lists the tool names Claude Code exposes (verified 2026-08-23 against the live tools reference, i.e. the latest release; the CHANGELOG records no tool changes since the 2.1.234 pinned in the sandbox image — re-check on every pin bump); `security.LegacyClaudeTools` lists names Claude Code no longer has but that agent `tools:` frontmatter and adapters still use (`LS`, `MultiEdit`, `Task` → `Agent`, …). `FULLSEND_TOOL_ALLOWLIST` and `security.HookGroup.Tools` are written in this vocabulary; it is a **reference** checked by tests, not validated at run time — `TestHookPlan_ToolsAreCanonical` pins every `HookPlan` tool, `TestPiToolNameMapsUseClaudeVocabulary` pins the pi adapter's maps to canonical *or legacy* names (pi's `ls` maps to `LS`, which Claude Code no longer sends — an agent allowlisted in canonical-only vocabulary sees pi's `ls` as a plain `tool_blocked`) (`piToolForClaude`/`claudeToolForPi` in `internal/runtime/pi_agent.go` → `fullsend-manifest.json` `hooks.toolNames` → `fullsend-hooks.js` `claudeToolName()`), and `TestToolAllowlistHook_VocabularyMatchesGo` keeps the copy inside `tool_allowlist_pretool.py` identical to the Go set. Adapters must translate to this vocabulary before invoking any hook script. An un-translated name is still **blocked** (the allowlist is exact-match, fail-closed), but `tool_allowlist_pretool.py` distinguishes a normalization gap from a forbidden tool when the blocked name equals an allowlisted entry case-insensitively: if the allowlisted entry is a Claude name the reason is `ALLOWLIST_HOOK_ERROR: tool name '' is not canonical Claude vocabulary (expected ''); the runtime adapter must translate it` (for a legacy entry: `… is not the legacy Claude name the allowlist uses (expected 'LS') …`) with a `tool_name_unnormalized` finding (severity `high`, action `block`); if instead the *tool name* is the Claude one (e.g. `Bash` against an allowlist written as `bash`) the reason names the `FULLSEND_TOOL_ALLOWLIST` entry (`… is not Claude vocabulary (expected canonical name 'Bash'); fix the allowlist`) and logs `allowlist_entry_unnormalized`; if neither side is a Claude tool name the reason says so, blames neither, and logs `tool_name_case_collision`. These three findings are `high`, not `critical`, so an adapter gap does not trip `critical`-keyed escalation the way a forbidden tool does. Names with no case-insensitive match keep the `tool_blocked` finding (severity `critical`); a non-string `tool_name` blocks with the JSON contract rather than a traceback. MCP tools (`mcp____`) are not canonical — they are matched verbatim and a case variant is treated as a different tool (`tool_blocked`). The diagnostic only sees *case* variants: a renaming gap such as pi reporting every edit as `Edit` while an agent is allowlisted only for `MultiEdit` surfaces as a plain `tool_blocked`. No case-insensitive *allow* is performed. - **Wire protocol (per script):** JSON on stdin — `{"tool_name": ..., "tool_input": {...}}` for PreToolUse. PostToolUse payloads include the tool output as `tool_response` (Claude Code; string or structured object such as Bash `{stdout, stderr, interrupted, isImage}`) with `tool_result` accepted as a fallback. Exit `0` = allow. *Blocking* scripts (all PreToolUse scripts, standalone `canary_posttool.py`, and `posttool_chain.py` when its canary stage fires) exit `1` and print `{"decision":"block","reason":"..."}` on stdout; the adapter must stop the tool call (or, post-tool, drop the result) and surface the reason. *Sanitizing* stages (suppress/unicode/redact) always exit `0` and, when they changed something, print `{"hookSpecificOutput":{"hookEventName":"PostToolUse","updatedToolOutput": }, "tool_result": }`. Empty stdout = unchanged. `updatedToolOutput` must match the tool's output shape — a bare string is ignored for built-in Claude Code tools. `scan_text` flattens every string field (including `stderr`), newline-joined so a needle cannot match across a field boundary (such a match would be unredactable, since the redactors rewrite each field independently); `apply_text` writes a replacement into the first text slot and blanks the rest, or leaves unrecognized structured shapes unchanged. Unicode normalization skips identifier fields (`hook_io.IDENTIFIER_KEYS`: paths, URLs, commands, exact-match edit strings) — NFKC would hand Claude a path that does not exist on disk; secret redaction still walks them, since it only replaces matched patterns. - **Sanitizer scope (what is rewritten, and what is not):** the PostToolUse stages exist to remove *controls-relevant* content and nothing else, because an agent edits against what it reads — a rewritten `Read` result means `Edit.old_string` no longer matches the file, and a `Write` of what it saw persists the rewrite. *Secret redaction* masks credential-shaped values only: the prefix patterns (`ghp_…`, `sk-…`, `AKIA…`, bearer headers, private-key blocks, database URLs) plus env/JSON shapes that need both a secret-bearing name (`…_TOKEN`, `api_key`, `accessToken`, not `TOKEN_URL`/`KEY_ID`/`publicKey`) and a value that is not an identifier, member path (`request.headers.authorization`), URL, path, placeholder or word phrase (`test-secret`, `ghs_policy_token`); a source-style `name = expr` counts only when the value is a quoted literal. A sweep of 900 fullsend files through the chain rewrites only test files holding token-shaped fakes. *Context suppression* condenses the output of exactly one verification command (`go test`, `pytest`, `npm test`, `make test`, `pre-commit run`, `gitleaks detect`, `scan-secrets`) with optional setup prefixes (`cd`, `export`, `source`), and only from positive evidence the tool printed (`ok `, `N passed`, `…Passed`, `no leaks`) — silence is never condensed into "passed", because a hook whose interpreter is missing is silent too and Claude Code's Bash result carries no exit code (so linters and `go vet`/`go build`, whose clean run prints nothing, are never condensed); the command must *start* with the tool (after wrappers that run it: `VAR=…`, `sudo`, `nice`, `timeout `, `env VAR=…`, `uvx`, `npx`, `uv run`, `mise exec --`, stacked; `python3.12 -m pytest` counts) — a command that merely mentions it, such as `grep -n scan-secrets hooks.py`, keeps its output; pipelines (`| tail` can cut the `FAIL` line; a `|` inside quotes such as `-run 'A|B'` is not a pipeline), `$(…)`, chains of two tools (`pytest; go test`, and deliberately also `go test && go vet` — one summary cannot speak for two), a trailing `echo $?`, and any output carrying a failure marker (`FAIL`, `panic:`, `Traceback`, `3 failed`) pass through untouched; comment lines and backslash continuations are tolerated. *Unicode* strips invisible, bidi, tag, NUL and ANSI/OSC characters and runs of variation selectors, but keeps compatibility characters (fullwidth, ligatures, CJK punctuation) and single emoji/CJK selectors — NFKC is applied to a *detection copy* (canary, secret patterns); a field is emitted normalized only when the normalized copy reveals an escape sequence or a secret the original hid. Every rewrite attaches `hookSpecificOutput.additionalContext` so the agent knows the output was changed and why, and every hook entry carries `timeout: 30` (Claude Code's 600 s default fails open — so does the 30 s one, for PreToolUse blockers included; the scripts finish in milliseconds and `tirith_check.py` bounds its own scan at 5 s, so the budget is headroom, not a ceiling the scripts approach). - **Fail modes:** blocking scripts fail **closed** on malformed JSON or oversized input (> 10 × 1024 × 1024 characters, read from text-mode stdin) — they block. Empty/whitespace-only stdin is treated as "no tool call" and allowed by every script; a payload without `tool_name` blocks only in the allowlist hook. `tirith_check.py` fails **open** when the `tirith` binary is missing, times out or errors, unless `TIRITH_REQUIRED=1` (which `appendHookEnv` writes when Tirith is enabled — adapters must make sure it reaches the script). Sanitizing scripts and each `posttool_chain.py` sanitizer stage fail **open** — malformed input or a stage exception is passed through unchanged (exit 0; the unicode hook logs an `input_truncated` finding), and a stage failure is recorded in `findings.jsonl` as `_stage_error`. Adapters must not treat a sanitizer's empty stdout as an error. The **canary stage fails closed**: a scan that raises is treated as a hit, a hit whose redaction cannot be verified clean withholds the output entirely rather than emitting it, and `exit 1` is unconditional. Because `posttool_chain.py` is the only PostToolUse entry point Claude Code schedules, input the driver cannot read — malformed JSON, or more than the 10 MB limit — also blocks (`exit 1`, `continue: false`) whenever `FULLSEND_CANARY_TOKEN` is set, instead of skipping detection; with no canary token configured it stays fail-open. Detection and redaction share one case-insensitive matcher (`hook_io.canary_pattern`), so a token that is detected is always one that can be redacted. diff --git a/internal/runtime/pi_agent_test.go b/internal/runtime/pi_agent_test.go index ccf53219f9..30a9c05944 100644 --- a/internal/runtime/pi_agent_test.go +++ b/internal/runtime/pi_agent_test.go @@ -3,6 +3,7 @@ package runtime import ( "testing" + "github.com/fullsend-ai/fullsend/internal/security" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -158,8 +159,31 @@ func TestPiToolNameMapsAreInverse(t *testing.T) { t.Parallel() for claude, pi := range piToolForClaude { if claude == "MultiEdit" { + // MultiEdit and Edit both map onto pi's single edit tool, so the + // adapter reports every pi edit as "Edit". An agent allowlisted + // only for MultiEdit is therefore blocked under pi as a plain + // tool_blocked — a renaming gap the hook's case-variant diagnostic + // (#608) cannot see. Documented in docs/runtimes.md; not a bug in + // the maps. continue } assert.Equal(t, claude, claudeToolForPi[pi], "pi tool %q must map back to %q", pi, claude) } + for pi, claude := range claudeToolForPi { + assert.Equal(t, pi, piToolForClaude[claude], "Claude tool %q must map back to %q", claude, pi) + } +} + +// The hook scripts only recognise canonical (or legacy) Claude names, so the +// adapter's translation table must stay inside that vocabulary (#608). +func TestPiToolNameMapsUseClaudeVocabulary(t *testing.T) { + t.Parallel() + for pi, claude := range claudeToolForPi { + assert.True(t, security.KnownClaudeTool(claude), + "claudeToolForPi[%q] = %q is not a canonical or legacy Claude tool name", pi, claude) + } + for claude := range piToolForClaude { + assert.True(t, security.KnownClaudeTool(claude), + "piToolForClaude key %q is not a canonical or legacy Claude tool name", claude) + } } diff --git a/internal/security/canonical_tools.go b/internal/security/canonical_tools.go new file mode 100644 index 0000000000..acd7733412 --- /dev/null +++ b/internal/security/canonical_tools.go @@ -0,0 +1,94 @@ +package security + +// CanonicalClaudeTools is the set of tool names Claude Code exposes today. +// It is the canonical hook vocabulary (ADR 0090, #608): FULLSEND_TOOL_ALLOWLIST +// entries and HookGroup.Tools are written in these names, and a runtime adapter +// (pi: claudeToolForPi → fullsend-manifest.json hooks.toolNames → +// fullsend-hooks.js claudeToolName) translates its own names to them before +// any hook script runs. A name the adapter did not translate is still blocked +// (the allowlist is exact-match, fail-closed) but tool_allowlist_pretool.py +// reports it as an adapter gap rather than a forbidden tool. +// +// Verified 2026-08-23 against the live tools reference +// (https://code.claude.com/docs/en/tools-reference, which documents the +// latest release — 2.1.241 at the time); the CHANGELOG for 2.1.235–2.1.241 +// records no tool additions or removals relative to the 2.1.234 pinned in +// images/sandbox/Containerfile. Re-check on every pin bump. +// +// MCP tool names (mcp____) are not canonical: they pass through +// verbatim and the allowlist matches them exactly, so the hook never treats +// their case variants as a translation gap. +// +// The hook script carries its own copy (CANONICAL_TOOLS / LEGACY_TOOLS in +// hooks/tool_allowlist_pretool.py) because the scripts have no access to Go; +// TestToolAllowlistHook_VocabularyMatchesGo keeps the two identical. +var CanonicalClaudeTools = map[string]bool{ + "Agent": true, + "Artifact": true, + "AskUserQuestion": true, + "Bash": true, + "CronCreate": true, + "CronDelete": true, + "CronList": true, + "Edit": true, + "EndConversation": true, + "EnterPlanMode": true, + "EnterWorktree": true, + "ExitPlanMode": true, + "ExitWorktree": true, + "Glob": true, + "Grep": true, + "ListAgents": true, + "ListMcpResourcesTool": true, + "LSP": true, + "Monitor": true, + "NotebookEdit": true, + "PowerShell": true, + "PushNotification": true, + "Read": true, + "ReadMcpResourceTool": true, + "RemoteTrigger": true, + "ReportFindings": true, + "ScheduleWakeup": true, + "SendMessage": true, + "SendUserFile": true, + "ShareOnboardingGuide": true, + "Skill": true, + "TaskCreate": true, + "TaskGet": true, + "TaskList": true, + "TaskOutput": true, + "TaskStop": true, + "TaskUpdate": true, + "TodoWrite": true, + "ToolSearch": true, + "WaitForMcpServers": true, + "WebFetch": true, + "WebSearch": true, + "Workflow": true, + "Write": true, +} + +// LegacyClaudeTools are tool names Claude Code no longer exposes but which +// still appear in agent `tools:` frontmatter and in runtime adapter maps (the +// pi adapter reports pi's `ls` as LS and accepts MultiEdit for its edit tool). +// Claude Code itself never sends these to a hook; an adapter may. The value is +// the current replacement, "" when there is none. +var LegacyClaudeTools = map[string]string{ + "LS": "", + "MultiEdit": "Edit", + "NotebookRead": "Read", + "Task": "Agent", + "TodoRead": "", +} + +// KnownClaudeTool reports whether name is a canonical or legacy Claude tool +// name — i.e. a name the hook scripts can be expected to see from a runtime +// adapter. +func KnownClaudeTool(name string) bool { + if CanonicalClaudeTools[name] { + return true + } + _, legacy := LegacyClaudeTools[name] + return legacy +} diff --git a/internal/security/canonical_tools_test.go b/internal/security/canonical_tools_test.go new file mode 100644 index 0000000000..f03b4d0b99 --- /dev/null +++ b/internal/security/canonical_tools_test.go @@ -0,0 +1,115 @@ +package security + +import ( + "regexp" + "sort" + "strings" + "testing" + + "github.com/fullsend-ai/fullsend/internal/harness" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// pythonNameSet extracts the quoted names of a `NAME = frozenset({...})` +// literal from the embedded hook script, so the Go and Python copies of the +// vocabulary can be compared without executing Python. +func pythonNameSet(t *testing.T, script []byte, name string) []string { + t.Helper() + re := regexp.MustCompile(`(?s)\n` + name + ` = frozenset\(\s*\{(.*?)\}\s*\)`) + m := re.FindSubmatch(script) + require.NotNil(t, m, "%s frozenset literal not found in tool_allowlist_pretool.py", name) + // A commented-out entry is absent from the runtime set; do not count it. + // Only whole-line comments are stripped — keep the literals one name per + // line with no trailing comments. + body := regexp.MustCompile(`(?m)^\s*#.*$`).ReplaceAll(m[1], nil) + var names []string + for _, q := range regexp.MustCompile(`"([^"]+)"`).FindAllSubmatch(body, -1) { + names = append(names, string(q[1])) + } + sort.Strings(names) + return names +} + +func sortedKeys[V any](m map[string]V) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +func TestToolAllowlistHook_VocabularyMatchesGo(t *testing.T) { + t.Parallel() + assert.Equal(t, sortedKeys(CanonicalClaudeTools), + pythonNameSet(t, ToolAllowlistPreToolHook, "CANONICAL_TOOLS"), + "CANONICAL_TOOLS in tool_allowlist_pretool.py must mirror security.CanonicalClaudeTools") + assert.Equal(t, sortedKeys(LegacyClaudeTools), + pythonNameSet(t, ToolAllowlistPreToolHook, "LEGACY_TOOLS"), + "LEGACY_TOOLS in tool_allowlist_pretool.py must mirror security.LegacyClaudeTools") +} + +func TestPythonNameSet_IgnoresCommentedEntries(t *testing.T) { + t.Parallel() + script := []byte("x = 1\nNAMES = frozenset(\n {\n \"Bash\",\n # \"LS\", # removed\n \"Read\",\n }\n)\n") + assert.Equal(t, []string{"Bash", "Read"}, pythonNameSet(t, script, "NAMES")) +} + +func TestCanonicalClaudeTools_WellFormed(t *testing.T) { + t.Parallel() + for name := range CanonicalClaudeTools { + assert.Equal(t, strings.TrimSpace(name), name) + assert.NotEmpty(t, name) + // A `Bash(gh,jq)` permission spec or a list must never be mistaken + // for a tool name. + assert.NotContains(t, name, "(") + assert.NotContains(t, name, ",") + assert.False(t, strings.HasPrefix(name, "mcp__"), "MCP names are not canonical: %s", name) + _, legacy := LegacyClaudeTools[name] + assert.False(t, legacy, "%s cannot be both canonical and legacy", name) + } + for name, replacement := range LegacyClaudeTools { + assert.NotEmpty(t, name) + if replacement != "" { + assert.True(t, CanonicalClaudeTools[replacement], + "legacy %s points at %s, which is not canonical", name, replacement) + } + assert.True(t, KnownClaudeTool(name)) + } + assert.True(t, KnownClaudeTool("Bash")) + assert.False(t, KnownClaudeTool("bash")) + assert.False(t, KnownClaudeTool("mcp__github__issue_read")) +} + +// Every tool HookPlan binds a script to must be canonical vocabulary — the +// plan is what every runtime adapter matches against, so a non-canonical name +// here would silently never fire on any runtime. +func TestHookPlan_ToolsAreCanonical(t *testing.T) { + t.Parallel() + on := true + configs := map[string]*harness.Harness{ + "defaults": nil, + "all-enabled": {Security: &harness.SecurityConfig{SandboxHooks: &harness.SandboxHooks{ + Tirith: &harness.TirithConfig{Enabled: &on}, + SSRFPreTool: &on, + SecretRedactPostTool: &on, + UnicodePostTool: &on, + ContextSuppressPostTool: &on, + CanaryPreTool: &on, + CanaryPostTool: &on, + ToolAllowlistPreTool: &harness.ToolAllowlistConfig{Enabled: &on}, + }}}, + } + for label, h := range configs { + for _, g := range HookPlan(SandboxHookConfigFromHarness(h)) { + for _, tool := range g.Tools { + if tool == AllTools { + continue + } + assert.True(t, CanonicalClaudeTools[tool], + "%s: HookPlan %s group binds non-canonical tool %q", label, g.Phase, tool) + } + } + } +} diff --git a/internal/security/hooks/tool_allowlist_pretool.py b/internal/security/hooks/tool_allowlist_pretool.py index 2f7dc023f3..2453b994fb 100644 --- a/internal/security/hooks/tool_allowlist_pretool.py +++ b/internal/security/hooks/tool_allowlist_pretool.py @@ -52,6 +52,138 @@ def log_finding(name: str, severity: str, detail: str, action: str) -> None: pass +# Canonical Claude Code tool names — the hook vocabulary (ADR 0090, #608). +# Mirror of security.CanonicalClaudeTools (internal/security/canonical_tools.go); +# TestToolAllowlistHook_VocabularyMatchesGo fails if the two drift. Verified +# 2026-08-23 against the live tools reference (latest release, 2.1.241 then; +# no tool changes in the CHANGELOG since the pinned 2.1.234). +CANONICAL_TOOLS = frozenset( + { + "Agent", + "Artifact", + "AskUserQuestion", + "Bash", + "CronCreate", + "CronDelete", + "CronList", + "Edit", + "EndConversation", + "EnterPlanMode", + "EnterWorktree", + "ExitPlanMode", + "ExitWorktree", + "Glob", + "Grep", + "ListAgents", + "ListMcpResourcesTool", + "LSP", + "Monitor", + "NotebookEdit", + "PowerShell", + "PushNotification", + "Read", + "ReadMcpResourceTool", + "RemoteTrigger", + "ReportFindings", + "ScheduleWakeup", + "SendMessage", + "SendUserFile", + "ShareOnboardingGuide", + "Skill", + "TaskCreate", + "TaskGet", + "TaskList", + "TaskOutput", + "TaskStop", + "TaskUpdate", + "TodoWrite", + "ToolSearch", + "WaitForMcpServers", + "WebFetch", + "WebSearch", + "Workflow", + "Write", + } +) + +# Names Claude Code no longer exposes but which agent `tools:` frontmatter and +# runtime adapters (pi: ls -> LS, MultiEdit -> edit) still use. Mirror of +# security.LegacyClaudeTools. +LEGACY_TOOLS = frozenset( + { + "LS", + "MultiEdit", + "NotebookRead", + "Task", + "TodoRead", + } +) + +MCP_PREFIX = "mcp__" + + +def _is_known_tool(name: str) -> bool: + return name in CANONICAL_TOOLS or name in LEGACY_TOOLS + + +def _find_case_match(tool_name: str, allowed: frozenset[str]) -> str | None: + """Return the allowlisted name that equals tool_name case-insensitively. + + None when there is no such entry, or when the name is an MCP tool + (``mcp____``): MCP names are not canonical vocabulary and are + matched verbatim, so a case variant is a different tool, not an + un-normalized one. When several entries fold to the same value, a known + Claude tool name wins; otherwise the first in sorted order. + """ + folded = tool_name.casefold() + if folded.startswith(MCP_PREFIX): + return None + candidates = sorted(name for name in allowed if name.casefold() == folded) + if not candidates: + return None + for name in candidates: + if _is_known_tool(name): + return name + return candidates[0] + + +def _unnormalized_diagnosis(tool_name: str, match: str) -> tuple[str, str, str]: + """Classify a case-variant collision: (finding name, detail, block reason). + + The allowlist is exact-match, so the call is blocked either way; the + diagnosis only says *which side* is not Claude vocabulary: + tool_name_unnormalized (the runtime adapter did not translate), + allowlist_entry_unnormalized (the allowlist is written in the wrong + case) or tool_name_case_collision (neither spelling is a Claude tool). + """ + if _is_known_tool(match): + if match in CANONICAL_TOOLS: + what = f"is not canonical Claude vocabulary (expected '{match}')" + else: + what = f"is not the legacy Claude name the allowlist uses (expected '{match}')" + return ( + "tool_name_unnormalized", + f"Tool '{tool_name}' is a case variant of Claude tool '{match}'", + f"ALLOWLIST_HOOK_ERROR: tool name '{tool_name}' {what}; " + "the runtime adapter must translate it", + ) + if _is_known_tool(tool_name): + kind = "canonical" if tool_name in CANONICAL_TOOLS else "legacy" + return ( + "allowlist_entry_unnormalized", + f"Allowlist entry '{match}' is a case variant of Claude tool '{tool_name}'", + f"ALLOWLIST_HOOK_ERROR: FULLSEND_TOOL_ALLOWLIST entry '{match}' is not " + f"Claude vocabulary (expected {kind} name '{tool_name}'); fix the allowlist", + ) + return ( + "tool_name_case_collision", + f"Tool '{tool_name}' is a case variant of allowlisted '{match}'; neither is a Claude tool", + f"ALLOWLIST_HOOK_ERROR: tool name '{tool_name}' is a case variant of " + f"allowlisted '{match}'; neither is a Claude tool name and tool names " + "are case-sensitive", + ) + + def _parse_allowlist(env_value: str | None) -> frozenset[str]: if env_value is None: return frozenset() @@ -75,9 +207,18 @@ def main() -> None: sys.stdout.write(_ERR_UNEXPECTED) sys.exit(1) + if not isinstance(hook_input, dict): + # Valid JSON that is not an object ([] / null / 123) is malformed for + # this protocol; keep the JSON block contract rather than tracebacking. + sys.stdout.write(_ERR_MALFORMED) + sys.exit(1) + tool_name = hook_input.get("tool_name", "") - if not tool_name: - json.dump({"decision": "block", "reason": "Tool name is empty or missing"}, sys.stdout) + if not isinstance(tool_name, str) or not tool_name: + json.dump( + {"decision": "block", "reason": "Tool name is empty, missing or not a string"}, + sys.stdout, + ) sys.exit(1) env_value = os.environ.get("FULLSEND_TOOL_ALLOWLIST") @@ -86,6 +227,17 @@ def main() -> None: if tool_name in allowed_tools: sys.exit(0) + # A case variant of an allowlisted name is still blocked (exact match, + # fail-closed), but reported as a normalization gap — an adapter that did + # not translate to Claude vocabulary, or an allowlist entry that is not + # Claude vocabulary — instead of as a forbidden tool (ADR 0090, #608). + case_match = _find_case_match(tool_name, allowed_tools) + if case_match is not None: + finding, detail, reason = _unnormalized_diagnosis(tool_name, case_match) + log_finding(finding, "high", detail, "block") + json.dump({"decision": "block", "reason": reason}, sys.stdout) + sys.exit(1) + log_finding("tool_blocked", "critical", f"Tool '{tool_name}' blocked by allowlist", "block") reason = f"Tool '{tool_name}' is NOT in the allowlist" json.dump({"decision": "block", "reason": reason}, sys.stdout) diff --git a/internal/security/hooks/tool_allowlist_pretool_test.py b/internal/security/hooks/tool_allowlist_pretool_test.py index 94488566d6..d88fce3928 100644 --- a/internal/security/hooks/tool_allowlist_pretool_test.py +++ b/internal/security/hooks/tool_allowlist_pretool_test.py @@ -2,11 +2,15 @@ from __future__ import annotations +import importlib.util +import io import json import os import subprocess import sys +import pytest + HOOK_PATH = os.path.join(os.path.dirname(__file__), "tool_allowlist_pretool.py") @@ -89,3 +93,230 @@ def test_missing_tool_name_blocks(): assert code == 1 response = json.loads(stdout) assert response["decision"] == "block" + + +def test_unnormalized_name_blocked_with_diagnostic(): + """A case variant of a canonical allowlisted name is blocked with ALLOWLIST_HOOK_ERROR.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "bash"}), + {"FULLSEND_TOOL_ALLOWLIST": "Bash,Read,Write"}, + ) + assert code == 1 + response = json.loads(stdout) + assert response["decision"] == "block" + assert "ALLOWLIST_HOOK_ERROR" in response["reason"] + assert "'bash'" in response["reason"] + assert "'Bash'" in response["reason"] + assert "runtime adapter must translate" in response["reason"] + + +def test_uppercase_variant_blocked_with_diagnostic(): + """ALL-CAPS variant also gets the diagnostic.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "BASH"}), + {"FULLSEND_TOOL_ALLOWLIST": "Bash,Read,Write"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "ALLOWLIST_HOOK_ERROR" in response["reason"] + assert "'Bash'" in response["reason"] + + +def test_legacy_name_variant_names_adapter(): + """pi's `ls` reaching the hook untranslated is an adapter gap against legacy `LS`.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "ls"}), + {"FULLSEND_TOOL_ALLOWLIST": "Read,LS"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "ALLOWLIST_HOOK_ERROR" in response["reason"] + assert "is not the legacy Claude name the allowlist uses (expected 'LS')" in response["reason"] + assert "canonical" not in response["reason"] + assert "runtime adapter must translate" in response["reason"] + + +def test_unnormalized_allowlist_entry_names_allowlist(): + """Canonical tool vs lowercase allowlist: the allowlist is the un-normalized side.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "Bash"}), + {"FULLSEND_TOOL_ALLOWLIST": "bash,read"}, + ) + assert code == 1 + response = json.loads(stdout) + assert response["decision"] == "block" + assert "ALLOWLIST_HOOK_ERROR" in response["reason"] + assert "FULLSEND_TOOL_ALLOWLIST entry 'bash'" in response["reason"] + assert "expected canonical name 'Bash'" in response["reason"] + assert "adapter" not in response["reason"] + + +def test_unnormalized_allowlist_entry_for_legacy_name(): + """A legacy Claude name is still the known side, but not called canonical.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "Task"}), + {"FULLSEND_TOOL_ALLOWLIST": "TASK"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "FULLSEND_TOOL_ALLOWLIST entry 'TASK'" in response["reason"] + assert "expected legacy name 'Task'" in response["reason"] + + +def test_unknown_case_variant_blames_neither_side(): + """Neither spelling is Claude vocabulary: no adapter or allowlist is blamed.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "FOO"}), + {"FULLSEND_TOOL_ALLOWLIST": "foo"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "ALLOWLIST_HOOK_ERROR" in response["reason"] + assert "neither is a Claude tool name" in response["reason"] + + +def test_mcp_case_variant_is_plain_block(): + """MCP names are matched verbatim; a case variant is a different tool, not a gap.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "mcp__github__Issue_Read"}), + {"FULLSEND_TOOL_ALLOWLIST": "mcp__github__issue_read"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "ALLOWLIST_HOOK_ERROR" not in response["reason"] + assert "NOT in the allowlist" in response["reason"] + + +def test_no_case_match_keeps_tool_blocked(): + """A tool with no case-insensitive match uses the standard tool_blocked path.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "WebFetch"}), + {"FULLSEND_TOOL_ALLOWLIST": "Bash,Read,Write"}, + ) + assert code == 1 + response = json.loads(stdout) + assert response["decision"] == "block" + assert "ALLOWLIST_HOOK_ERROR" not in response["reason"] + assert "NOT in the allowlist" in response["reason"] + + +def test_canonical_name_still_allowed(): + """Exact match is still allowed — no false positive from case matching.""" + code, _stdout = _run_hook( + json.dumps({"tool_name": "Bash"}), + {"FULLSEND_TOOL_ALLOWLIST": "Bash,Read,Write"}, + ) + assert code == 0 + + +def test_non_string_tool_name_blocks_with_json(): + """A non-string tool_name must not escape as a traceback: block with the JSON contract.""" + for value in (123, True, ["Bash"], {"name": "Bash"}): + code, stdout = _run_hook( + json.dumps({"tool_name": value}), + {"FULLSEND_TOOL_ALLOWLIST": "Bash,Read,Write"}, + ) + assert code == 1, value + response = json.loads(stdout) + assert response["decision"] == "block" + assert "not a string" in response["reason"] + + +def test_non_object_payload_blocks_with_json(): + """Valid JSON that is not an object must block with the contract, not traceback.""" + for raw in ("[]", "null", "true", "false", "123", '"Bash"'): + code, stdout = _run_hook(raw, {"FULLSEND_TOOL_ALLOWLIST": "Bash"}) + assert code == 1, raw + response = json.loads(stdout) + assert response["decision"] == "block" + assert "malformed" in response["reason"] + + +def test_case_fold_collision_prefers_known_name(): + """With `bash` and `Bash` both allowlisted, the diagnostic names the Claude one.""" + code, stdout = _run_hook( + json.dumps({"tool_name": "BASH"}), + {"FULLSEND_TOOL_ALLOWLIST": "bash,Bash"}, + ) + assert code == 1 + response = json.loads(stdout) + assert "expected 'Bash'" in response["reason"] + assert "runtime adapter must translate" in response["reason"] + + +def _load_hook_module(): + spec = importlib.util.spec_from_file_location("tool_allowlist_pretool", HOOK_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _run_in_process( + monkeypatch, tmp_path, tool_name: str, allowlist: str +) -> tuple[int, dict, list]: + module = _load_hook_module() + findings = tmp_path / "findings.jsonl" + monkeypatch.setattr(module, "FINDINGS_PATH", str(findings)) + monkeypatch.setenv("FULLSEND_TOOL_ALLOWLIST", allowlist) + monkeypatch.setattr(sys, "stdin", io.StringIO(json.dumps({"tool_name": tool_name}))) + out = io.StringIO() + monkeypatch.setattr(sys, "stdout", out) + with pytest.raises(SystemExit) as exc: + module.main() + logged = [] + if findings.exists(): + logged = [json.loads(line) for line in findings.read_text().splitlines() if line] + return int(exc.value.code or 0), json.loads(out.getvalue() or "{}"), logged + + +def test_unnormalized_finding_is_logged(monkeypatch, tmp_path): + """The adapter-gap path logs tool_name_unnormalized (high, block), not tool_blocked.""" + code, response, logged = _run_in_process(monkeypatch, tmp_path, "bash", "Bash,Read") + assert code == 1 + assert response["decision"] == "block" + assert [f["name"] for f in logged] == ["tool_name_unnormalized"] + assert logged[0]["severity"] == "high" + assert logged[0]["action"] == "block" + assert logged[0]["scanner"] == "tool_allowlist_pretool" + assert "'Bash'" in logged[0]["detail"] + + +def test_unnormalized_allowlist_entry_finding_is_logged(monkeypatch, tmp_path): + code, _response, logged = _run_in_process(monkeypatch, tmp_path, "Bash", "bash,read") + assert code == 1 + assert [f["name"] for f in logged] == ["allowlist_entry_unnormalized"] + assert logged[0]["severity"] == "high" + assert logged[0]["action"] == "block" + + +def test_case_collision_finding_is_logged(monkeypatch, tmp_path): + """Neither spelling is Claude vocabulary: a distinct finding, not an adapter gap.""" + code, _response, logged = _run_in_process(monkeypatch, tmp_path, "FOO", "foo") + assert code == 1 + assert [f["name"] for f in logged] == ["tool_name_case_collision"] + assert logged[0]["severity"] == "high" + + +def test_tool_blocked_finding_unchanged(monkeypatch, tmp_path): + """A genuinely forbidden tool keeps the critical tool_blocked finding.""" + code, _response, logged = _run_in_process(monkeypatch, tmp_path, "WebFetch", "Bash,Read") + assert code == 1 + assert [f["name"] for f in logged] == ["tool_blocked"] + assert logged[0]["severity"] == "critical" + + +def test_allowed_tool_logs_nothing(monkeypatch, tmp_path): + code, _response, logged = _run_in_process(monkeypatch, tmp_path, "Bash", "Bash,Read") + assert code == 0 + assert logged == [] + + +def test_vocabulary_sets_are_disjoint_and_well_formed(): + module = _load_hook_module() + assert module.CANONICAL_TOOLS + assert not (module.CANONICAL_TOOLS & module.LEGACY_TOOLS) + for name in module.CANONICAL_TOOLS | module.LEGACY_TOOLS: + assert name.strip() == name and name + assert "(" not in name and "," not in name, name + assert not name.startswith(module.MCP_PREFIX), name