fix(switch): identity-guard account write-back to stop profile cross-contamination#50
Conversation
…contamination
Profile switch and the launch-time bootstrap (`sync_root_state_to_current_profile`)
copied the live `~/.codex` state back into whatever profile the `.current_profile`
marker named, with no check that the account actually in `~/.codex/auth.json` is the
one that profile holds. When the live account drifted away from the marker — a manual
`codex login` outside the app, the official Codex app re-authing, or hand-edits to
`~/.codex` — the next switch (or merely relaunching the app, since bootstrap runs the
same write-back) silently overwrote an unrelated profile's stored credentials with the
wrong account ("串号" / cross-contamination), and could lose the overwritten account's
auth entirely.
Write-back is now gated by `resolve_backup_target` (shared/runtime/profiles.rs), which
fingerprints the live account from the stable `tokens.account_id` (falling back to the
id_token `email`) via the new `load_account_identity_from_path`, and resolves the real
target in five cases:
1. live identity unknown (apikey / placeholder / unreadable) -> marker (legacy)
2. marker slot owns the live account -> marker (happy path)
3. live account owned by a *different* managed profile -> that profile
4. marker slot is an empty placeholder, live account is new -> marker (seat it)
5. live account is unmanaged -> None (refuse the write-back; no contamination)
switch_core uses it instead of the blind marker backup. The bootstrap write-back is
hoisted into a shared `switch_core::sync_root_state_to_current_profile_with_home` (the
macOS / Windows copies were mirror-duplicated; they now delegate) which additionally
heals a stale marker when the live account drifted to another managed profile.
Tests: +10 (5 resolve_backup_target cases, 3 bootstrap sync cases, 1 full-switch
regression proven to fail on the pre-fix baseline, 1 account-identity extraction).
113 -> 114 shared/mac lib tests pass.
状态
手测建议(可选 —— 自动化已覆盖,以下是想亲眼确认时的步骤)A. 正常切换冒烟:多张卡片来回切,确认每张卡片显示的账号/额度正确、切换后 B. 复现并验证修复(需要 2 个真实账号 X、Y):
合并清单(你的活)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af650613b7
ℹ️ 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".
Follow-up to the identity guard: refine case 5 (live ~/.codex account owned by no managed profile). Previously the launch-time sync just skipped the write-back and left the stale `.current_profile` marker in place, so the dashboard kept showing a wrong card as "current" with no signal to the user. Now: - `detect_unmanaged_live_account` (shared/runtime/profiles.rs) reports the live account's label when it has a resolvable identity that no profile owns (reusing the extracted `find_profile_owning_identity`). - The bootstrap sync clears every active marker via the new `fs_ops::clear_active_markers` when that condition holds, so no card is falsely flagged current. - `load_profiles_snapshot` recomputes the condition each call and carries it as `ProfilesSnapshotResponse.unmanaged_live_account`; the dashboard shows a one-time, account-named toast (`unmanagedAccountToast`, EN + zh) prompting the user to switch to or create the matching card. The prompt de-dupes per distinct account and resets when the live account is managed again. Tests: +3 (detect flags unmanaged / returns None when managed / returns None when unidentifiable); the existing bootstrap-unmanaged test now asserts the marker is cleared. 117 lib tests pass; tsc --noEmit clean.
跟进:case 5 已按「清空标记 + 弹提示」实现(commit
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e9a3db2cd
ℹ️ 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".
…slot Addresses a codex review P2 on resolve_backup_target case 4. The placeholder- seating path used `slot_identity(marked).is_none()`, but a `None` identity means "no OAuth identity" — which is also true for an API-key card (and for a malformed / unreadable auth.json), not just an empty placeholder. So when the marker pointed at an API-key card and the live root had drifted to an OAuth account owned by no card, the write-back seated the OAuth state on top of the API-key card and destroyed its credentials — the very contamination this PR fixes, just shifted onto API-key users. Case 4 now gates on the new `metadata::auth_is_empty_placeholder`, which returns true only when auth.json parses and carries no usable credentials of any kind (no OAuth tokens beyond the `replace-me` seed, no `auth_mode = "apikey"`, no non-empty `OPENAI_API_KEY`). API-key, malformed, unreadable, and real-OAuth slots are all rejected, so the live account falls through to case 5 (refuse + prompt) instead of overwriting real credentials. Tests: +2 (apikey marker slot is never seated and keeps its key; direct auth_is_empty_placeholder coverage across placeholder / empty / apikey / raw key / real-oauth / malformed / missing). 119 lib tests pass.
Addresses a second codex review P2 on load_profiles_snapshot. The unmanaged detection was independent of current_card, so a single snapshot could both flag `unmanaged_live_account` AND return the old marked card as `current` (plus its quota) — contradictory UI state. Bootstrap only clears the marker on launch, so mid-session drift (an external `codex login` while the app is open) hit exactly this case. The snapshot now suppresses the current card when `unmanaged_live_account` is `Some`: `current_profile` is forced to `None` before building the response, so no card is shown as current (in the list or the header) while the prompt says the live account is unmanaged. `load_current_live_quota` gets the same guard so its quota response stays consistent. Tests: +2 (snapshot shows current card on identity match; snapshot suppresses current card + quota + list "current" status on unmanaged drift). 121 lib tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2dcc396c9
ℹ️ 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".
…erprint
Addresses a third codex review P2. Identity was a single prefixed fingerprint
(`acct:<id>` preferred, else `email:<addr>`) compared by exact equality. But one
account can present an email-only auth.json before a refresh writes
`account_id`: the slot's stored identity stays `email:…` while the live root
becomes `acct:…`, and the exact match then treats the same account as
unmanaged — refusing/rerouting the write-back and clearing/suppressing the
current card, so legacy email-only slots stop receiving refreshed root state.
Identity is now an `AccountIdentity { account_id, email }` and two identities are
the same account when they share a non-empty account_id OR a non-empty email.
Each field is globally unique to one account, so OR-matching never merges two
distinct accounts, but it keeps an email-only slot matching the same account
once an id appears. resolve_backup_target, find_profile_owning_identity, and
detect_unmanaged_live_account all match via `same_account`.
Tests: +2 (AccountIdentity::same_account matrix; an email-only slot still
matches an account_id-bearing live root and is neither refused nor flagged
unmanaged); existing identity test updated to assert both fields are captured.
123 lib tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3ba82bbe9
ℹ️ 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".
| let Some(root_identity) = load_account_identity_from_path(&codex_home.join("auth.json")) else { | ||
| return marked; |
There was a problem hiding this comment.
Refuse API-key write-back to identified stale markers
When the live root has an unidentifiable but real auth state such as {"auth_mode":"apikey"} and .current_profile still points at an OAuth card, this early return sends the caller to backup_root_state_to_profile, overwriting that identified OAuth profile with the API-key root. That is the same stale-marker contamination path this guard is meant to prevent; even if the root has no OAuth identity, the marked slot can still be checked and should not be treated as safe unless it is also unidentifiable/placeholder/API-key-compatible.
Useful? React with 👍 / 👎.
The bug (user-reported "串号")
Switching profiles sometimes crossed accounts — card A would end up holding account B's credentials, and the overwritten account's auth could be lost.
Root cause: both the switch flow (
switch_core.rs) and the launch-time bootstrap (sync_root_state_to_current_profile) copied the live~/.codexstate back into whatever profile the.current_profilemarker named, with no check that the account actually sitting in~/.codex/auth.jsonis the one that profile holds. The whole model trusted a marker file as the source of truth for "who is live".When the live account drifted away from the marker, the next write-back blind-copied the wrong account into a stale-marked slot. Drift sources, all realistic for multi-account users:
codex loginin a terminal,~/.codex.Because the bootstrap runs the same write-back on every launch, contamination could happen silently just by relaunching the app — no Switch click required.
This is current-version behavior (HEAD / 1.5.11), not a fixed legacy artifact. PR #44 ("relax tokens constraint…") touched the metadata-display derivation, not this write-back path.
The fix
A new
resolve_backup_target(shared/runtime/profiles.rs) gates the write-back by account identity, fingerprinted from the stabletokens.account_id(falling back to the id_tokenemail) via the newload_account_identity_from_path:switch_coreuses it instead of the blind marker backup. The bootstrap write-back is hoisted into a sharedswitch_core::sync_root_state_to_current_profile_with_home— the macOS/Windows copies were mirror-duplicated and now delegate (share-don't-duplicate) — which additionally heals a stale marker when the live account drifted to another managed profile.Known, intentional trade-offs
auth.jsoncan't be parsed (apikey, placeholder, or a transient unreadable read) the guard falls back to the marker — i.e. exactly the old behavior, never worse. This is required so apikey / placeholder cards keep refreshing.Tests (+10, all green: 114 lib tests)
resolve_backup_target: 5 cases (happy / unmanaged-drift refuse / reassign-to-owner / placeholder-seat / apikey-fallback)sync_root_state_to_current_profile_with_home: marker-heal-on-drift, skip-and-preserve-on-unmanaged, refresh-marked-on-matchswitch_does_not_contaminate_stale_marker_profile_with_drifted_account: full switch path — verified to FAIL on the pre-fix baseline (blind copy wroteacct_Zinto carda), passes with the guardload_account_identity_from_path: account_id-preferred / email-fallback / apikey→None / placeholder→NoneImplemented with assistance from Claude Code.
🤖 Generated with Claude Code