Repository navigation
fix(cli): narrow the public seat tool list on every adapter - #2994
Conversation
Public Claude, Codex, OpenCode and Qwen seats are no longer offered commonly_attach_file, matching Goose's public list. One shared list, applied in each adapter's own vocabulary. A workspace (own-repo) Claude seat keeps it. TASK-287 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4VWLC9MiWprFWXFNjJSnr
7c480b5 to
9c8bf28
Compare
TASK-287 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4VWLC9MiWprFWXFNjJSnr
lilyshen0722
left a comment
There was a problem hiding this comment.
Tier 2 gate at 2e548cd95773343150a2db7982d95947bd196c8 (cli, TASK-287). This head is 9c8bf28 plus "fix(cli): workspace seats follow the same tool list as public seats". My earlier reading was at 9c8bf28 and the verdict below covers both commits.
What I ran: a scratch copy of the head with cli/node_modules from the #2987 tree and @commonlyai/seats linked to the head's packages/seats.
- Adapter tests (claude.environment, claude, claude.context-tokens, codex, opencode, qwen) plus environment, default-environment and public-pod-sandbox-gate: 9 suites, 317 passed, 2 skipped (skips already in the suite).
- One mutant per adapter, each removing that adapter's withheld deny, plus an OpenCode mutant that moves the deny above the
_*allow. Each mutant fails the test that owns the exclusion. At 2e548cd, Claude mutants: withheld denies not appended (2 tests fail, including the workspace-seat test); workspace exemption put back (the workspace-seat test fails). Each source file was restored byte-identical after its run.
(a) Does each exclusion remove the tool on a public spawn?
- Claude: a confined spawn carries
mcp__<server>__commonly_attach_filein--disallowedTools. The delta removes the workspace exemption, so public and workspace spawns now share the list. Deny-over-allow for themcp__commonly__*wildcard is documented Claude Code behaviour and was not run live. - Codex: a public spawn emits
-c mcp_servers.<name>.disabled_tools=["commonly_attach_file"]. The key is in the config schema of the codex-cli 0.162.1 binary on this machine, next tomcp_serversandenabled_tools. Not run against a live session. - OpenCode: the deny key is
commonly_commonly_attach_file, placed aftercommonly_*. The binary is not installed here, so I read OpenCode v1.19.4 source. Config string values become rules in object order (fromConfig).evaluatetakes the last matching rule (findLast).disabled()hides a tool from the model when its last matching rule is a*deny. So the deny wins over the wildcard allow, and the tool is removed from the list the model sees, not only refused when called. - Qwen:
commonly_attach_fileandmcp__commonly__commonly_attach_fileboth go into--exclude-tools, on public spawns only. Not run; qwen is not installed here.
(b) Private seats: unchanged. The Claude list applies only on the confined policy path, and the private Claude spawn never calls it. Codex's disabled_tools is gated on ctx.publicSeat, which is sandbox.trust === 'public' after legacy normalization (internal becomes public). Qwen's list is gated on isPublic. OpenCode's deny is only in publicPermissions. The exact -c list on the private codex spawn still passes unchanged.
(c) Claude workspace seat (own-repo trust): it should not keep attach_file, and the new head is right to remove the exemption. The reason is that trust is an environment declaration, so an exemption is only as correct as the label on each seat. One list for every confined seat avoids depending on that label being right, and an owner who wants a file in a pod can still upload it through the app. This is a recommendation, and I did not find a defect in the uniform version.
Not verified by a live run: the codex key's effect on the live tool list; qwen's matching of both spellings in --exclude-tools; Claude's deny-over-allow.
Non-blocking:
- OpenCode name mismatch for server names containing a hyphen.
permissionMcpNamemaps every character outside[A-Za-z0-9_]to_, hyphens included. OpenCode's tool key (toolName→sanitizein packages/opencode/src/mcp/catalog.ts at v1.19.4) keeps-. For a server namedcommonly-mcp, the deny keycommonly_mcp_commonly_attach_filedoes not match the real keycommonly-mcp_commonly_attach_file, so the tool stays in the model's list. Current declarations usecommonly, so no present seat is affected. Suggest/[^A-Za-z0-9_-]/gto match OpenCode's sanitize, with a test that uses a hyphenated name. The existing_*allow has the same mismatch. - On a public Codex seat,
disabled_toolsis written for every stdio server in the environment spec, not only the one namedcommonly. Harmless for this tool name; noting it so a reader does not assume otherwise.
Written by Rhea, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015
GATE: PASS
lilyshen0722
left a comment
There was a problem hiding this comment.
Otto gate (Opus, Tier 2) at 2e548cd. That is 9c8bf28 plus 2e548cd, which withholds the tool on Claude workspace seats too.
Each adapter's public spawn drops commonly_attach_file:
- Claude: every confined spawn (Seatbelt and bwrap, public trust) goes through
buildPublicClaudePolicyArgs. It addsmcp__<server>__commonly_attach_fileto--disallowedToolsfor every declared server, and in Claude Code a disallow beats themcp__<server>__*allow. Public bwrap passes notrust, so it gets the deny as well. - Codex:
publicSeatistrust === 'public'afternormalizeSandboxTrust, so legacyinternalis covered too, and workspace trust is refused for codex.mcp_servers.<name>.disabled_toolsis part of the MCP server schema in the codex-cli 0.162.1 binary on this host. - OpenCode: the
<server>_commonly_attach_file: denykey comes after<server>_*: allow. Under last-match-wins the deny wins. Under longest-pattern-wins the deny key is also longer, so it wins either way. The test pins the key order. - Qwen: both spellings go into
--exclude-tools, on public spawns only.
Private seats: unchanged. Codex keeps its exact -c list. OpenCode still sets permission: 'allow'. Qwen's exclusions stay behind isPublic. Private Claude never calls the public policy builder.
Tests: the four suites pass, 194 passed and 1 skipped. I tried five mutants and each one failed its own test:
- Claude withheld denies off fails the public Seatbelt test.
- The workspace exemption restored fails the workspace test (at 2e548cd).
- The codex
disabled_toolspush removed fails the public warm MCP test. - The OpenCode deny moved before the allow fails the public permissions test.
- The Qwen spread removed fails the public launch test.
I couldn't run real OpenCode or Qwen binaries on this host. The OpenCode and Qwen results come from the config semantics and the tests.
The workspace question. Withholding it there is right, and 2e548cd does that. A workspace-trust turn only runs when every one of the seat's pods is confirmed private. Otherwise workspaceTurnPlacement runs the turn as public. But commonly_attach_file reads filePath with readFileSync in the MCP server process, so the workspace tier's Read(...) deny rules don't govern it, including ./.git/** and the home-directory denies. The jail is the only bound, and it keeps .git readable for the git tool. The tool can come back for workspace seats once attach_file confines its path to what the seat's Read policy allows. At this head the workspace test asserts the deny, and putting the trust === 'workspace' exemption back fails it.
Non-blocking:
docs/agents/COMMONLY_MCP.mdstill says "public seats"; Claude workspace seats are now included too.- Qwen's prefixed spelling is hardcoded to
mcp__commonly__. The other three adapters loop over every declared server name. A public Qwen seat with a Commonly server declared under another name keeps the tool under that name. Building the prefixed list fromObject.keys(mcp.mcpServers)would match the others.
CI at filing: CLI, MCP and hosted runtime tests is green. The backend and service shards were still running; this PR touches no backend code.
Written by Otto, a Commonly agent — https://commonly.me/v2/pods/6a920d3eb08dee6c42a81bfc
GATE: PASS
lilyshen0722
left a comment
There was a problem hiding this comment.
Connector lane review at 2e548cd (cli/).
I read 9c8bf28 earlier. The one commit since then, 2e548cd, carries out the lane ruling that a workspace seat loses commonly_attach_file too. That seat's sandbox can still read the repo's .git, and the attach tool reads outside the agent's usual read rules, so it could post .git/config (which can hold a token in a remote URL) into a pod.
buildPublicClaudePolicyArgsnow adds the withheld denies on every confined trust. Thetrust === 'workspace'exception is gone, and private seats never reach this builder.- The workspace-seat test now expects the deny (adapters.claude.environment.test.mjs:1278). Both gates' mutants show it is the case that owns this rule.
- CI at this head: 21 pass, 4 skipped, 0 failing.
The live public Codex spawn proof is still owed before TASK-287 closes. It doesn't block this merge.
LANE: PASS
@coderabbitai ignore
Public seats on Claude, Codex, OpenCode and Qwen are no longer offered
commonly_attach_file, which brings them in line with Goose's public list. All four adapters read one shared list incli/src/lib/public-withheld-tools.js, and each removes the tool its own way:mcp__<server>__commonly_attach_filein--disallowedTools(public and workspace trust)mcp_servers.<name>.disabled_tools=["commonly_attach_file"]on public seats<server>_commonly_attach_file: deny, after the<server>_*allow, so the deny wins whichever matching order applies--exclude-toolson public seatsClaude workspace seats (own-repo trust) follow the same list. Private seats are unchanged; the exact
-clist in the Codex private test confirms this.Tests: one assertion per adapter in the existing public-spawn tests, plus the Claude workspace test now expecting the same deny.
Written for TASK-287.
🤖 Generated with Claude Code
https://claude.ai/code/session_01N4VWLC9MiWprFWXFNjJSnr