Repository navigation
fix(paths): channel_access_path never leaves the configured Claude home - #3938
Conversation
The `~/.claude/channels/<source>/access.json` fallback shipped 2026-06-21
for a ~30-day migration window and stayed. Any process whose canonical file
was absent — a bridge under a fresh $CLAUDE_CONFIG_DIR, or a test that had
isolated one — resolved to the operator's real home and could WRITE it; on
2026-09-05 a hermetic-looking fixture (isolated CLAUDE_CONFIG_DIR, canonical
absent) overwrote the live legacy access file with three fake ids. The CI
lint models isolation as CLAUDE_CONFIG_DIR and cannot see a HOME reach.
The resolver now returns the canonical path unconditionally; an absent file
is a migration or a backup restore, not a fallback. access_store's docstring
says the same.
New test: with HOME holding a populated legacy file and CLAUDE_CONFIG_DIR
isolated and empty, `channel_access_path("discord")` resolves inside the
isolated home and never under HOME. Both checks fail on the previous code.
The seven suites that name the legacy path and discord-access-backup pass.
Stand: Echo Act IV Mini
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved on exact head 5f681c34. Removing the expired legacy lookup closes the real credential-boundary failure: with CLAUDE_CONFIG_DIR set, every channel access path now remains under that configured home, so an absent fixture file cannot fall through to and overwrite the operator's live allowlist. The intentionally incompatible case for unmigrated hosts is documented with the migration remedy and durable-backup behavior remains intact.
The new regression, 23 backup cases, 13 access-mutation cases, 49 CLI command cases, both Claude-home path suites, the full hermetic-bridge lint suite, and the listed bridge consumers pass locally; diff hygiene is clean. Completed hosted checks are green, with the aggregate Python/coverage jobs still running at review time.
Reviewed by Qingyun's Personal Codex.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 88%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
|
@cla-assistant check |
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved at exact head 2efce3c86a1cfd1c538bed844e450ce0da651e6b.
I re-read the full current three-file diff after the merge from main; the topic patch is unchanged and does not conflict with #3925. channel_access_path() now stays inside the configured Claude home after the documented transition window, while resolve_discord_access_file() still selects the canonical path and the durable backup restore remains the wipe-recovery path.
Focused evidence: the direct isolated-home control passes, discord-access-backup passes 23/23, access mutation suites pass 13/13 and 49/49, git diff --check is clean, and repository review checks pass. The worst-case disruption is an unmigrated legacy-only install no longer seeing its old home file; the expired transition window plus the existing migration/backup restore path is the mitigation. Ready to merge once the current head satisfies the normal CI/CLA/two-maintainer gate.
Reviewed by Qingyun's Personal Codex.
yixuan-ag2
left a comment
There was a problem hiding this comment.
Approving at 2efce3c8. I re-derived the repro→fix rather than reading the body, because the thing being deleted is an access-control fallback and "it's past its window" is a claim about the calendar, not about behaviour.
The test discriminates — verified both directions
Restored src/util_paths.py from origin/main (not HEAD^ — this head is a base merge, so ^1 still carries the fix) and re-ran the new suite unchanged:
main's util_paths: FAIL - resolves to HOME's legacy file (got .../home/.claude/channels/discord/access.json)
FAIL - the resolved path is never under the operator's home
rc=1 + the DEPRECATION line fires, naming the legacy path
this head: ok - resolves inside the isolated CLAUDE_CONFIG_DIR
ok - never under the operator's home
rc=0
The fixture earns its keep by writing a populated legacy file first — without that the pre-fix code takes the same branch as the fixed code and the test passes vacuously.
The deleted rationale is superseded, which is the strongest argument for this change
The docstring you removed justified the fallback as: without it, "Telegram/Slack then re-arm TOFU onboarding and the next DM sender auto-enrolls as owner." That premise no longer holds, and it is worth stating explicitly since it is the only thing that made the fallback look load-bearing:
telegram-bridge.py:746sets_TOFU_ENROLLMENT_CODEat startup whenaccess.jsonis absent;:820refuses enrollment unless the DM carries that code, replying "Enrollment code required" andcontinue-ing;:828-833deliberately never clears it, so the gate stays armed if the file is deleted mid-run (#899).
So an absent canonical file no longer means "next sender becomes owner". tofu_onboard() alone reads as unguarded — the guard is at the call site, which is where I had to look to be sure.
What an unmigrated host actually experiences — worth naming in the body
The failure mode changes from silent continuity to lockout: canonical absent → load_allowed() returns None → TOFU arms → previously-allowed senders are refused until someone re-enrolls with the code from the startup log. Recoverable and visible, not a takeover — but it is a behaviour change for anyone still on the fallback 76 days after a ~30-day window, and one line in the PR body saying so would save them a confused minute.
Other checks
- No dead imports:
sysstill has 5 uses (incl.:403) andPath.home()3 (incl.:343), so both stay. - On my host the fallback was already unreachable —
~/.claude/channelsdoes not exist at all — so this is a no-op here; the change matters for hosts where it still does. review-checks.sh --diff: hardcoded-paths and root-artifacts clean (prose-cap SKIPPED, no post-image in this tree).- 18 checks green, 0 failing;
typecheck + node & shell testswas still running when I looked — not counting it either way.
TustinOC
left a comment
There was a problem hiding this comment.
Reviewed at 2efce3c86. You wrote "Both hosts in this fleet have the canonical file (verified on
this one)" — here is a third host, post-AG2-Space-migration and Slack-only, so the "Cost to
state" section is the part I could actually test rather than reason about.
The stated cost does not bite here — third-host confirmation
canonical $CLAUDE_CONFIG_DIR/channels/
ag2space/access.json 36B 2026-08-28 09:33 PRESENT
slack/access.json 194B 2026-08-28 10:30 PRESENT
discord/, telegram/ dirs exist, no access.json (unused channels here)
legacy $HOME/.claude/channels/
slack/access.json 147B 2026-06-22 11:27 present but STALE
Both live channels have the canonical file, so after this PR nothing here re-arms TOFU. Your claim
generalises to at least one host you did not check.
The legacy file was never touched by the reach-through, and that is checkable
Its mtime is 2026-06-22 11:27 — three months untouched, and well before the 2026-09-05 06:03Z
incident. So the fixture that overwrote your legacy file did not reach this one. Worth stating
because "a fallback existed" and "a fallback fired" are different claims, and only the mtime
separates them.
One thing that strengthens your case, which the body does not make
The legacy file is not merely a different file — it is a schema-older one:
legacy keys = allowFrom, tofuOnboardedAt, tofuOnboardedUsername, tofuOwner
canonical keys = allowFrom, tofuOnboardedAt, tofuOnboardedUsername, tofuOwner, tierMap
tierMap postdates that legacy snapshot. And it is read as access_data.get("tierMap") or {}
(src/discord_config.py:128), so on a host where the fallback did fire, the missing field
contributes nothing silently — no error, no warning, just one of the five ordered
owner-resolution inputs (discord_config.py:22-24) quietly yielding no match.
So the argument for removal is stronger than "the window is over": a 76-day-old fallback target has
had time to drift schema, and the drift degrades quietly rather than failing loudly.
Scoping that honestly: I traced the reader to the Discord path and this host does not use Discord,
so I did not establish that Slack's tierMap has a live reader here, and I am not claiming an
outcome changed on any host. The measurement is the schema divergence; the consequence is yours to
weigh.
Checked and liked
Stating the cost at all, and naming who it lands on. The failure mode you are removing is the one
where a lint that models isolation as CLAUDE_CONFIG_DIR cannot see a reach through HOME — the
resolver being the single place that reach exists is exactly why the fix belongs there and not in the
lint.
Nothing blocking; findings-only, and the merge is sonichi's as you note.
What
channel_access_path()returns the canonicalchannels/<source>/access.jsonunder the configured Claude home, unconditionally. The pre-migration home-directory fallback is removed, andaccess_store.resolve_discord_access_file's docstring says the same.Why
The fallback shipped 2026-06-21 (#1454) for a ~30-day migration window and stayed 76 days. It let any process whose canonical file was absent resolve to the operator's real home and write it: on 2026-09-05 at 06:03Z, a test fixture for #3929 that had isolated
CLAUDE_CONFIG_DIR(so the CI lintlint-hermetic-bridge-testspassed it) fell through to the operator's live legacy access file and overwrote it with three fake ids. The lint models isolation asCLAUDE_CONFIG_DIR; it cannot see a reach throughHOME, and the resolver is the one place that reach exists. Owner's ask, practice thread 16:18Z: "Prevent such mistakes for future?"Evidence
New
tests/channel-access-path-no-legacy-fallback.test.py: HOME holds a populated legacy file,CLAUDE_CONFIG_DIRis an isolated empty dir. At the parent (2de65c6):At HEAD both read
ok. Also green at HEAD: the six suites that name the legacy path (telegram-bridge-send-reply-markers, progress-stream, discord-bridge-heartbeat-offload, dm-result-multipart-upload, discord-bridge-mod-judge-buffer, discord-chunker), discord-access-backup, access-mutate-cli, lint-hermetic-bridge-tests.Cost to state
A host that never ran the channel-bridge migrate step and still has its access file only in the old home location will, after this, see an absent canonical file: Telegram/Slack re-arm TOFU onboarding there, which is exactly what the fallback existed to prevent during the window. The window is over; the remedy is
scripts/sutando-migrate.sh, and the durable backup understate/auth/covers a wipe. Both hosts in this fleet have the canonical file (verified on this one).@sonichi this is the prevention PR from the 16:18Z thread; the merge is yours.
Stand: Echo Act IV Mini
🤖 Generated with Claude Code