Keep shared Codex auth read-only during usage refresh - #2944
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 auth-provider
|
Codex review: needs changes before merge. Reviewed August 16, 2026, 1:20 AM ET / 05:20 UTC. ClawSweeper reviewWhat this changesThe PR makes Codex usage refresh read shared OAuth files without rewriting them, adds stale-native CLI recovery, and stores managed workspace selection privately in CodexBar. Merge readinessKeep open: the PR’s managed-workspace guard is bypassed by Automatic mode, which can still fall through to the unguarded Codex CLI strategy after an OAuth failure. The shared-auth ownership work is not on current main, so the PR remains necessary after this P1 repair and an owner decision on opt-in external credential discovery. Priority: P1 Review scores
Verification
How this fits togetherCodexBar reads Codex credentials to fetch usage and reset-credit data, then presents it under the selected account or workspace. This change separates shared-file ownership from CodexBar-owned workspace metadata before a usage snapshot reaches the app. flowchart LR
A[Codex auth file] --> B[Credential reader]
C[Managed workspace metadata] --> D[OAuth usage request]
B --> D
D --> E{Credential or request fails?}
E -->|stale native| F[Codex CLI recovery]
E -->|other| G[Surface OAuth error]
F --> H[Usage snapshot]
D --> H
Decision needed
Why: Current source exposes no supported mechanism to pass CodexBar’s selected workspace identity to the CLI, so only an owner can choose between fail-closed behavior and intentional scope substitution. Before merge
Findings
Agent review detailsSecurityNeeds attention: The PR improves shared-token handling, but its remaining Automatic fallback can cross the selected workspace boundary. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Apply the managed-workspace scope rule to every Automatic-mode CLI fallback, preserve the OAuth error when scope cannot be carried, and land the optional external-source setting only with explicit approval of its default-off consent contract. Do we have a high-confidence way to reproduce the issue? Yes—source inspection establishes that an Automatic-mode OAuth fallback advances to the unguarded CLI strategy under a selected managed workspace; a focused pipeline regression can reproduce it without real credentials. Is this the best way to solve the issue? No—the explicit-OAuth guard does not cover the equivalent Automatic fallback. The narrow safe repair is to make that fallback workspace-aware or preserve the OAuth error. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2f951e278d27. LabelsLabel changes:
Label justifications:
EvidenceSecurity concerns:
Acceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (23 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
P1
Yuxin-Qiao
changed the title
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
clawsweeper
Bot
added
status: 📣 needs proof
|
@clawsweeper re-review |
clawsweeper
Bot
added
status: 📣 needs proof
|
@clawsweeper re-review Follow-up pushed at |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Added redacted real-native runtime evidence to the PR body: an isolated copy of the installed native auth file successfully served a real OAuth usage request through the CLI, while the copied auth file's SHA-256, owner-only mode, and size remained unchanged. No token or account email is included. External-source and stale CLI-recovery proof remain explicitly marked unavailable rather than fabricated. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Fixed the P1 at |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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: 91ed021fcb
ℹ️ 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".
| case .notFound, .unreadable, .missingTokens, .nativeRefreshRequired, .readOnlySource: | ||
| return true |
There was a problem hiding this comment.
When the default source mode is .auto and a managed workspace's native credential becomes stale, .nativeRefreshRequired returns true here and the pipeline advances to the ordinary CodexCLIUsageStrategy from the auto strategy list. That strategy bypasses CodexOAuthNativeRefreshCLIStrategy.isAvailable, including its codexWorkspaceID == nil guard, so the CLI can return the workspace stored in auth.json rather than the selected managed workspace (or have the result rejected as an account mismatch). Apply the workspace-scope guard to auto fallback as well, or route auto recovery through the guarded strategy.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Thanks @Yuxin-Qiao — this landed via #2970, which carried all 18 of your commits onto current main, resolved the conflicts, and fixed two review findings (managed-workspace CLI scope leakage and a stale external OAuth fallback). Closing as superseded; the changelog credits you. |
…n-zero history, Antigravity dashboard lanes, Claude OAuth revoked detection steipete#2946 Codex routed models pricing — codex_routed_provider() + strip_route_prefix() in cost_pricing.rs; deepseek/, kimi/, opencode/ routes price against matching models.dev provider; unknown provider/ prefixes return None. steipete#2944 Codex auth.json read-only during refresh — codex_external_oauth_sources_allowed setting (default OFF); is_external_oauth + last_refresh fields on CodexCredentials; enforce_external_oauth_gate (8-day staleness window). steipete#2932 Codex known-zero history — known_zero field on CostSummary; set in both cache-debounce and full-scan paths (history_coverage_established && sessions_count == 0); knownZero JSON field + CLI text update. steipete#2963 Antigravity dashboard lanes — quota-bucket dedup in parse_user_status; models sharing the same (remaining_fraction, reset_time) collapse to one lane. steipete#2516 Claude revoked vs missing OAuth — ProviderError::OAuthRevoked variant; revocation detection (401/403 with revoked/invalid_grant/token_revoked); 15-min CLI result cache (LazyLock<Mutex<Option<CachedCliResult>>>); fetch_via_auto returns cached CLI result when OAuth revoked, and stale cache when all live sources fail.
…n-zero history, Antigravity dashboard lanes, Claude OAuth revoked detection steipete#2946 Codex routed models pricing — codex_routed_provider() + strip_route_prefix() in cost_pricing.rs; deepseek/, kimi/, opencode/ routes price against matching models.dev provider; unknown provider/ prefixes return None. steipete#2944 Codex auth.json read-only during refresh — codex_external_oauth_sources_allowed setting (default OFF); is_external_oauth + last_refresh fields on CodexCredentials; enforce_external_oauth_gate (8-day staleness window). steipete#2932 Codex known-zero history — known_zero field on CostSummary; set in both cache-debounce and full-scan paths (history_coverage_established && sessions_count == 0); knownZero JSON field + CLI text update. steipete#2963 Antigravity dashboard lanes — quota-bucket dedup in parse_user_status; models sharing the same (remaining_fraction, reset_time) collapse to one lane. steipete#2516 Claude revoked vs missing OAuth — ProviderError::OAuthRevoked variant; revocation detection (401/403 with revoked/invalid_grant/token_revoked); 15-min CLI result cache (LazyLock<Mutex<Option<CachedCliResult>>>); fetch_via_auto returns cached CLI result when OAuth revoked, and stale cache when all live sources fail.
* Port upstream 0.50.1: Cursor rename + Ollama cookie stripping (steipete#2951, steipete#2949) * Port upstream 0.50.1: OpenCode Go session+weekly pace in CLI (steipete#2957) * Port upstream 0.50.1: chart axis label centering (steipete#2974) * Port upstream 0.50.1: Kiro re-authenticate via kiro-cli login (steipete#2340) * Port upstream 0.50.1: serve identity follows app redaction per-request (steipete#2960) * Port upstream 0.50.1: Codex routed pricing, auth.json read-only, known-zero history, Antigravity dashboard lanes, Claude OAuth revoked detection steipete#2946 Codex routed models pricing — codex_routed_provider() + strip_route_prefix() in cost_pricing.rs; deepseek/, kimi/, opencode/ routes price against matching models.dev provider; unknown provider/ prefixes return None. steipete#2944 Codex auth.json read-only during refresh — codex_external_oauth_sources_allowed setting (default OFF); is_external_oauth + last_refresh fields on CodexCredentials; enforce_external_oauth_gate (8-day staleness window). steipete#2932 Codex known-zero history — known_zero field on CostSummary; set in both cache-debounce and full-scan paths (history_coverage_established && sessions_count == 0); knownZero JSON field + CLI text update. steipete#2963 Antigravity dashboard lanes — quota-bucket dedup in parse_user_status; models sharing the same (remaining_fraction, reset_time) collapse to one lane. steipete#2516 Claude revoked vs missing OAuth — ProviderError::OAuthRevoked variant; revocation detection (401/403 with revoked/invalid_grant/token_revoked); 15-min CLI result cache (LazyLock<Mutex<Option<CachedCliResult>>>); fetch_via_auto returns cached CLI result when OAuth revoked, and stale cache when all live sources fail. * Port upstream 0.50.1: Mistral PAYG spend, cost-summary display style, per-provider accent color Item 1 — Mistral PAYG current-month API spend (steipete#2821, steipete#2947): - Add MonthlyPlan variant to MetricPreference enum - Add currency_symbol field to CostSnapshot with with_currency_symbol builder - Set currency_symbol from Mistral billing API response - Add MonthlyPlan to bridge label/parse, tray selected_metric_percent (None = no bar) - Show formatted cost amount in provider_status_label for MonthlyPlan - Add format_cost_amount helper in bridge.rs - Add Mistral monthly spend row in MenuCardDetails - Add monthlyPlan option to MenuBarMetricSection for Mistral Item 2 — Menu cost-summary display style per provider (steipete#2976): - Add CostSummaryDisplayStyle enum (Compact/Detailed/Hidden) in types.rs - Add cost_summary_display_style field to Settings + Default + RawSettings round-trip - Add to SettingsSnapshot bridge + SettingsUpdate + apply in Tauri commands - Add bridge label/parse functions - Apply in MenuCardDetails: hidden hides cost section, compact shows used/limit only - Update describeCard to filter hasCost for hidden style - Add Select control in UsageSpendTab Item 3 — Per-provider accent color override (steipete#2972): - Add accent_color field to ProviderConfig - Add brand_color(ProviderId) function in provider.rs (mirrors frontend registry) - Add accent_color/set_accent_color/effective_accent_color accessors in Settings - Add set/get/get_effective Tauri commands + normalize_hex_accent_color validator - Add providerAccentColors map to SettingsSnapshot bridge - Add setProviderAccentColor/getProviderAccentColor/getProviderEffectiveAccentColor to tauri.ts - Create AccentColorSection component (hex input, color picker, reset button) - Inject --provider-accent CSS variable on MenuCard article and ChartsSection - Update menu-metric__bar-fill and chart colors to use --provider-accent fallback - Update chartPalette providerCostColor/providerCreditsColor fallback chain - Pass providerAccentColors through TrayPanel/PopOutPanel/ProvidersTab/ProviderDetailPane Also: - Add 11 new locale keys to all 7 .ftl files + keys.ts - Add costSummaryDisplayStyle + providerAccentColors to all test mock SettingsSnapshot - Update chartPalette tests for --provider-accent fallback * Resolve merge conflicts and fix gate checks * Thermo-nuclear: extract ollama cookies, codex pricing, simplify accent color * fix: correct serde attribute placement on currency_symbol after rebase
2 participants
Summary
auth.jsonas Codex CLI-owned: usage refresh never redeems or publishes a shared refresh token from CodexBar.CODEX_HOMEprecedence and explicit-home isolation.Why
A generation check plus a CodexBar-only lock cannot form a compare-and-swap protocol with Codex CLI. Codex CLI can replace
auth.jsonbetween a read and a rename, and a refresh response can rotate the refresh token. The safe boundary is therefore read-only shared-file access: Codex CLI owns native refresh and persistence, while CodexBar uses the winning credentials only in memory.Validation
swift test --filter CodexOAuthCredentialReadTests— 27 passed.swift test --filter CodexOAuthResetCreditFetchTests— 6 passed.swift test --filter CodexOAuthTests— 33 passed.make check— passed; SwiftFormat 0 files and SwiftLint 0 violations.make test— 864 selections in 72 groups; 72/72 passed, 0 failures, 0 retries, and 0 timeouts.Safety evidence
Scope
This PR is intentionally limited to OAuth source ownership, refresh safety, account identity propagation, reset-credit consistency, and portability. It does not change Codex CLI's credential format or claim that CodexBar can safely persist another application's rotated token.
Documentation follow-up (2026-08-16)
c7ab5e509.auth.json.make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations) andgit diff --checkpassed. The previous final-headmake testrun remains 864/864 across 72/72 groups.Organization fallback follow-up (2026-08-16)
47d03bedc.swift test --filter CodexOAuthCredentialReadTests(28 passed),swift test --filter CodexOAuth(89 tests across 7 suites passed),make checkpassed, and fullmake testpassed (864/864 selections; 72/72 groups; 0 failures, retries, or timeouts).Account ID normalization follow-up (2026-08-16)
15acee507.tokens.account_idvalues were treated as present and could suppress valid JWT organization fallback; direct account IDs are now normalized with the samenonEmptyrule as JWT claims.swift test --filter CodexOAuth(90 tests across 7 suites passed),make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations), and fullmake testpassed (864/864 selections; 72/72 groups; 0 failures, retries, or timeouts).OAuth guide flow follow-up (2026-08-16)
273c9a40c.CODEX_HOMEprecedence, opt-in external discovery, CLI-owned stale-native recovery, fail-closed stale external sources, and snapshot reuse for reset-credit enrichment.make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations),git diff --checkpassed, and no product code changed in this follow-up.OAuth guide ownership cleanup follow-up (2026-08-16)
87b2b36d.auth.json.make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations),git diff --checkpassed, and no product code changed in this follow-up.CI classification (2026-08-16)
make checkpassed and no product code changed.Reset-credit snapshot follow-up (2026-08-16)
d58b1beb4.codexResetCreditsAttemptedmarker when reset-credit enrichment was attempted with the winning in-memory credential snapshot. GenericUsageStoreenrichment no longer rereadsauth.jsonafter that attempt fails; reset-credit-only OAuth results fail closed instead of mixing accounts.swift test --filter CodexResetCreditOutcomeTests(11 passed),swift test --filter CodexOAuth(90 tests across 7 suites passed),swift test --filter ProviderArchitectureGatekeeperTests(38 passed),make check, and fullmake test(864/864 selections; 72/72 groups; 0 failures, retries, or timeouts).Documentation and CI follow-up (2026-08-16)
7204480e7.docs/codex.md: managed workspace selection is private CodexBar metadata and never edits or republishesauth.json.make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations) andgit diff --checkpassed.macOS Swift tests are required but deferred, not a product failure.Real native runtime evidence (2026-08-16)
auth.jsonwas placed in an isolated owner-only temporaryCODEX_HOME; the original file was not used as a writable target.HOME=<isolated-home> CODEX_HOME=<isolated-codex-home> swift run --skip-build CodexBarCLI usage --provider codex --source oauth --format text --no-credits --no-color.auth.jsonSHA-256 was equal before and after the request; owner-only mode and size (0600, 4013 bytes) were unchanged. The original nativeauth.jsonwas not modified.--no-creditskept this proof scoped to the usage request; no reset-credit enrichment or refresh/persistence path was invoked. This is real fresh-native usage proof, not a claim of stale CLI recovery or external-source proof.openaiOAuth entry, and the native token contains a direct account claim, so real external-source and organization-only fallback traces remain unavailable without fabricating credentials. Those maintainer decisions stay explicit.Managed workspace CLI-recovery follow-up (2026-08-16)
91ed021fc.CodexOAuthNativeRefreshCLIStrategynow reports itself unavailable, because the Codex CLI app-server has no supported way to receive CodexBar's selected workspace account header. Recovery can no longer substitute theauth.jsonworkspace for the user-selected workspace.swift test --filter CodexOAuth(91 tests across 7 suites passed),swift test --filter ProviderArchitectureGatekeeperTests(38 passed),make checkpassed (SwiftFormat 0 files; SwiftLint 0 violations), and fullmake testpassed (865 selections; 73/73 groups; 0 failures, retries, or timeouts).