feat(#1452): add exact per-action tool allowlist

[jejeje-dot]

Refs #1452

Maintainer note — this is a FEATURE, and the repo is under a feature freeze (bug/stability only).
It is parked here in a reviewable state rather than queued for merge. Nothing below is a request to land it.

What changed

• Add COMFYUI_MCP_TOOL_ACTION_ALLOW as an exact comma-separated tool:action allowlist.
• Reject malformed, empty, or wildcard rules at startup.
• Enforce the policy before handlers run across:
  • direct MCP tool registration
  • the compact call_tool catalog
  • the Claude in-process panel server
  • the Codex HTTP panel server
• Forward the policy to spawned ComfyUI MCP children, including an explicitly empty value so the child fails closed.
• Document how tool-level and action-level policy compose.

Why

#873 / #1383 made tool names enforceable, but consolidated tools mix actions with very different blast radii. An operator currently cannot allow queue:list and targeted queue:cancel without also exposing queue:edit and global queue:clear.

The allowlist is exact and intentionally has no wildcard support. New actions introduced by a future release therefore remain denied until the operator names them explicitly.

Calls without an action field are unchanged and remain governed by the existing tool-level preset/allow/deny policy.

Reachability

An allowlist that is implemented but never consulted is the failure mode worth checking before any other, so each route was pinned:

Route	How it is reached	Evidence
direct MCP registration	registerAllTools → withToolSurfaceFilter wraps server.tool, gating the handler	source assertion on the install line + handler-boundary test
compact call_tool	collectToolCatalog wraps catalog.asRegistrar(), so the catalog stores the wrapped handler that call_tool dispatches	real collectToolCatalog() dispatch, no fetch mock — reaching the handler would mean reaching ComfyUI
Codex HTTP panel	registerPanelTools calls toolActionPolicyError before the handler	real McpServer + Client over InMemoryTransport
Claude in-process panel	createPanelMcpServer, same	source assertion

Default-deny holds: once COMFYUI_MCP_TOOL_ACTION_ALLOW is set, a call carrying a string action is refused unless a rule names that exact tool:action. A tool with no rule at all can dispatch no action.

Over-permissive cases now covered

The direction that ships a hole is "permits more than intended", and a test that only proves the allowed action is allowed cannot see any of it. Added:

• Prefix / extra segment — queue:list must not admit list_all, listall, lis, list:extra, nor tools queue_admin, queuex, my_queue, enqueue.
• Prefix near-miss on the real dispatch route — queue:cancel must not admit cancel_queued. Both are genuine members of queue's action enum, so unlike a synthetic name this one survives schema validation and actually reaches the gate — and cancel_queued drops the entire pending queue, someone else's work included.
• Case — the rule grammar already rejects an uppercase rule, but nothing constrained the incoming call, which is the side a caller controls: QUEUE:list, queue:LIST, Queue:List are refused.
• Cross-product — queue:list,download_model:download must not admit queue:download or download_model:list. This is the classic allowlist bug (some(tool) && some(action)); it reads correct and passes every single-rule test.
• Whitespace — " list" / "list " are not list.
• Policy activation — an action list alone must make the policy active, or withToolSurfaceFilter returns the registrar unwrapped and the entire gate is dead code.

One behaviour was tightened rather than left defaulted: an action that is present but not a string was returning allowed, resolving the one shape the gate cannot classify in the permissive direction. It now refuses. An absent or null action is still allowed, since that is not an action call. All 34 live action fields are required z.enum([...]) and both dispatch routes validate before the handler runs, so nothing real reaches that branch today — which is exactly why it needed deciding instead of defaulting.

Validation

Re-run on Windows against current main:

• origin/main merged into the branch; no conflicts.
• npm run lint (tsc --noEmit) clean.
• npm run check:vocabulary and npm run check:unknown-collapse clean.
• Focused suites green: tool-surface-filter, panel-tools-strict-schema, panel-secrets — 200/200.
• Mutation-tested, because these tests are worth exactly what they can kill. Six widening mutations, six killed, zero survivors: prefix compare, case-insensitive compare, cross-product compare, non-string waved through, the action list no longer activating the policy, and the gate never installed on the registrar.
• Doc example audited against the live enums — every tool:action pair in it is real (get_system_stats:stats|logs|health, get_history:list|diagnose, create_workflow:create|modify|validate|node_info, enqueue_workflow:enqueue, queue:list|status|cancel). A wrong pair there would hand an operator a config that silently withholds something they meant to allow.

Full suite: 9,444 passed / 21 failed across 12 files. Every one of the 21 is Test timed out in 30000ms, on a host that was running ~280 concurrent node processes (the run reported 4,019s transform and 9,710s import against 691s wall). All of them pass in isolation:

• tool-surface-filter, vocabulary, vocabulary-handshake — 320/320.
• ui-bridge > names the versioned panel-sync remedy (#706) — 91ms isolated, and that file contains no reference to tool-surface-filter, toolActionAllowed, or COMFYUI_MCP_TOOL_ACTION_ALLOW.

No failure is attributable to this branch. Stating the limit of that claim plainly: the reason they are unrelated is machine load, so this is evidence about this host, not a green CI run. No CI checks are currently reported on this branch.

Reviewer notes

• A refused action returns an error naming COMFYUI_MCP_TOOL_ACTION_ALLOW, so the model learns the mechanism exists. That is a deliberate departure from the No way to restrict the tool surface — blocks production/multi-user deployments #873 tool-level design, where a denied tool is absent so the model never learns of it — and it is unavoidable here, since the tool must stay registered for its allowed actions. Worth a conscious ruling rather than inheriting by accident.
• Setting only COMFYUI_MCP_TOOL_ACTION_ALLOW leaves every action-free tool fully reachable. The docs say to pair it with COMFYUI_MCP_TOOL_ALLOW, which is correct, but an operator who reads the variable name as a lockdown would be wrong.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.