Skip to content

list_instance_tools reports nine mutating tools as scope: "read" — one field carries two incompatible meanings, and its sibling deliberately fails the other way #563

Description

@serge-ivo

list_instance_tools reports nine mutating tools as scope: "read" — one field carries two incompatible meanings, and its sibling deliberately fails the other way

What an auditor sees

list_instance_tools's own description says:

plus tier … Use this to verify an agent is read-only before trusting it with sensitive data

Measured against instance bd43f4de-ef35-4051-bdec-43f8571414a1 (Repo Coder, production, 2026-08-15) — GET /v1/instances/:id/tools, 104 rows. Nine of them mutate and report scope: "read":

tool what it does reported scope
start_work starts a durable autonomous run, spends BYOK tokens read
stop_work cancels a running loop read
end_coding_session kills the engine on the owner's machine read
set_behaviour rewrites the agent's stored persona/style read
set_stats_card adds/edits/removes a stats card read
run_pipeline starts a pipeline (irreversible connector work) read
create_ticket writes a board ticket, possibly needs_approval with an action read
record_feedback writes a durable feedback row read
dedupe_upsert writes records into the instance's collection read

In the same listing, built-ins that mutate report correctly: write_memory, delete_memory, create_task, update_task, configure_board, set_user_preference all carry scope: "write". So one response tells the reader that configure_board (changes kanban columns) is a write and set_behaviour (changes who the agent is) is a read.

Mechanism — two files, two opposite defaults, one merged column

Verified. The listing merges rows from two policy sources with contradictory fail directions.

workers/api/src/lib/builtin-tool-policy.ts:145 fails closed:

export function scopeOfBuiltin(name: string): "read" | "write" {
	return BUILTIN_TOOL_SCOPES[name] ?? "write";
}

under a header (:39-44) that states the rule explicitly:

over-reporting a read as a write costs an auditor a question, under-reporting a write as a read is the bug this file exists to close.

workers/api/src/lib/instance-tool-policy.ts:174 fails open:

scope: t.scope ?? "read",

and so does the connector flattener, workers/api/src/lib/connectors/registry.ts:172. FIRST_PARTY_TOOLS (lib/tool-registry.ts:144+) declares tier on all eleven entries and scope on none — grep -c 'scope:' lib/tool-registry.ts → 1, and that one is the audit-log line at :743.

The sharper finding: scope is not an audit label, and the fix cannot be "declare write"

Verified. In the registry, scope is the trigger for the write-consent gate, not a statement about mutation. lib/steps.ts:518 says so in as many words:

scope: "read", // no external connector; writes the instance's OWN collection via the DO

And runRegistryTool (lib/tool-registry.ts:631-642) makes a connector-less write tool unreachable:

if (tool.scope === "write") {
	const authority = consentInstanceOf({...});
	if (!tool.connector || !(await hasConsent(env, authority, tool.connector, "write"))) {
		return { ..., content: `Writing via the ${label} connector isn't permitted…`, success: false };
	}
}

So stamping scope: "write" on start_work would not fix the report — it would break the tool, because it has no connector to consent to. That is why the nine are where they are, and it is why this needs a second field rather than a corrected value.

Enforcement impact — none, stated plainly

Verified. No gate is bypassed by this. All nine are connector: undefined, writeConsent: "n/a", and the consent gate is per-connector, so n/a is the correct verdict for them regardless of scope. grep for consumers of resolveToolPolicy finds routes/tools.ts (the listing + the generic invoker) and store/console/src/lib/toolPolicy.ts (display); neither branches on entry.scope. The defect is misreporting to an auditor, not a hole in the gate. An issue that claimed otherwise would send the implementer to runRegistryTool, which is correct as written.

Blast radius — #561 does NOT consume this field

Verified. #561's tools/list annotations are driven by TOOL_RISK in workers/mcp/src/tool-metadata.ts:97+, a hand-written table over the ~135 MCP server tool names (list_agents, add_knowledge, …), derived back from safety.ts in the test. It never reads instance-tool-policy's scope. The two namespaces do not meet, so this stays confined to the listing and to list_instance_tools. Worth stating because the opposite would have made this urgent.

What to do — cheapest first

1. Add mutates: boolean to ToolDef, separate from scope. scope keeps its real meaning ("does this need external write consent") and stops pretending to answer the audit question. The listing reports mutates; runRegistryTool keeps gating on scope unchanged, so nothing that runs today changes behaviour.

2. Make the omission fail the build, the way the built-in side already does. builtin-tool-policy.test.ts fails when a definition exists with no BUILTIN_TOOL_SCOPES entry. Mirror it: a registry ToolDef with no mutates fails a test. That is what makes this not recur; the default direction is a distant second.

3. Correct the description. workers/mcp/src/instance-tools/base.ts:29 says `scope` (read/write) with no gloss, and the sentence "use this to verify an agent is read-only" is what turns an imprecise field into a wrong answer. It must name which field answers "does it mutate".

Alternatives considered and rejected

  • Flip the default to "write" in instance-tool-policy.ts:174. Would make the listing honest for these nine by accident, and dishonest for the ~29 genuinely-read-only step and connector tools that also omit scope (map, filter, slice, parse_json, http_reachable, …). It also does not stop a new tool from omitting it. A default is not a decision; a required field is.
  • Declare scope: "write" on the nine. Breaks them at tool-registry.ts:631 — see above. Rejected on evidence, not taste.
  • Derive mutates from the handler. builtin-tool-policy.ts:36 already explains why the equivalent cannot be done there: nothing in a ToolDef says whether the handler mutates. Same argument applies.

Acceptance criteria

  • GET /v1/instances/:id/tools reports the nine tools above as mutating, and map/filter/slice/repo_grep/github_read_issue as not.
  • Adding a ToolDef to FIRST_PARTY_TOOLS, STEP_TOOLS or a connector without the mutation field fails a test naming the tool.
  • No change to which tools run or are refused: a test asserts runRegistryTool's refusal set is byte-identical before and after.
  • list_instance_tools's description states which field answers "does it mutate".

Regression risk

The consent gate keys on scope; touching that field is how this becomes a security change instead of a reporting one. Keep scope untouched and add a field beside it. The test to have: assert every currently-consent-gated tool is still gated, enumerated from the registry rather than typed out (the pattern seed-drift.test.ts and security-invariants.test.ts already use here).


Measured 2026-08-15 against production, instance bd43f4de-ef35-4051-bdec-43f8571414a1: 104 rows; tier counts connector: 49, standard: 30, base: 21, runtime: 4; 38 rows are connector-less with scope: "read", of which the nine above mutate.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend / Worker / API workbugSomething isn't workingconnectorsConnector + tool framework

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions