Skip to content

fix(auth): serialize auth login writes#82569

Merged
vincentkoc merged 3 commits into
mainfrom
fix/auth-login-lock
May 16, 2026
Merged

fix(auth): serialize auth login writes#82569
vincentkoc merged 3 commits into
mainfrom
fix/auth-login-lock

Conversation

@vincentkoc

@vincentkoc vincentkoc commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Serialize CLI/configure/onboarding provider auth writes through the auth-profile file lock, matching the live gateway writer contract.
  • Preserve old upsert semantics while doing that: token/API-key copy-paste normalization, local save options, SecretRef cleanup, and explicit failure when the lock/write fails.
  • Widen the fix beyond the OpenAI Codex OAuth symptom to adjacent API-key/token setup writers that can run while a gateway is live.

RCA

Live incident evidence, redacted:

  • On OpenClaw 2026.5.16-beta.2, openclaw models auth login --provider openai-codex completed successfully and updated config metadata.
  • The config file mtime changed during login, but the agent auth-profile store and credential secret file stayed at their old mtimes.
  • The persisted agent profile still had an expired OpenAI Codex OAuth credential (expires: 0) after login.
  • Gateway logs kept reporting refresh_token_reused, and Telegram /status still reported the model login as expired.

What broke:

  • PR fix(auth): stop live auth store writes from reverting fresh tokens #53211, merged 2026-03-23 by @vincentkoc as merge commit 03231c0633874527ece3fda449643edfb3775fd3, fixed the live gateway side of the stale-auth race by reloading under the auth-profile lock before runtime writes.
  • CLI/configure/onboarding auth writers still used non-locking upsert paths. A running gateway could therefore keep writing from a stale in-memory auth snapshot while the CLI reported re-login success.
  • Recent OpenAI/Codex routing changes made this old missing-lock path hit default users:

Who/how/when:

  • This was an older missing lock, not a single beta-only bad line.
  • @steipete authored the original non-locking CLI persist helper in src/commands/models/auth.ts on 2026-03-15 (a33caab280f3) and the OAuth helper write path in src/plugins/provider-auth-helpers.ts on 2026-03-16 (6d6825ea182a).
  • @BunsDev touched the Codex relogin/order path on 2026-05-04 (626e07886383992a2b8edcde96306a53671a9972).
  • @pashpashpash's May 7 and May 11 OpenAI/Codex default-routing PRs made the old missing-lock path hot for default OpenAI users.
  • @vincentkoc's March 23 PR fix(auth): stop live auth store writes from reverting fresh tokens #53211 was related remediation, but incomplete for this incident: it protected live gateway writers, not CLI/configure/onboarding login writers.

Fix

  • src/commands/models/auth.ts: CLI provider login and paste-token now use locked upsert and fail loudly if persistence fails.
  • src/plugins/provider-auth-choice.ts: configure/onboard provider auth now uses locked upsert.
  • src/plugins/provider-auth-helpers.ts: OAuth credential writes and sibling-agent sync now use locked upsert.
  • src/plugins/provider-api-key-auth.ts: generic non-interactive API-key setup now uses locked upsert.
  • extensions/github-copilot/login.ts, extensions/anthropic/register.runtime.ts, extensions/cloudflare-ai-gateway/index.ts, extensions/zai/index.ts: bundled provider setup writes now use locked upsert where they write auth profiles directly.
  • src/agents/auth-profiles/profiles.ts and src/agents/auth-profiles/upsert-with-lock.ts: locked upsert now normalizes token/API-key credentials like the old path and omits empty inline secrets when a SecretRef is used.
  • src/agents/auth-profiles/store.ts: locked updates can pass save options through to persistence.
  • src/agents/auth-profiles/persisted.ts: Vitest workers no longer trigger macOS Keychain prompts for OAuth profile master-key fallback.
  • src/plugin-sdk/provider-auth-api-key.ts: exports the locked upsert for API-key provider plugin setup.
  • CHANGELOG.md: adds the user-visible auth fix note.

Verification

  • node scripts/run-vitest.mjs src/agents/auth-profiles/profiles.test.ts src/commands/models/auth.test.ts src/commands/onboard-auth.test.ts src/plugins/provider-auth-choice.test.ts extensions/github-copilot/index.test.ts extensions/cloudflare-ai-gateway/index.test.ts extensions/anthropic/index.test.ts extensions/zai/index.test.ts passed: 9 files, 105 tests.
  • node scripts/crabbox-wrapper.mjs run --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref main --idle-timeout 90m --ttl 240m --timing-json -- CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=900000 OPENCLAW_TESTBOX=1 OPENCLAW_TESTBOX_REMOTE_RUN=1 pnpm check:changed passed on Testbox-through-Crabbox tbx_01krrcy2gx3bk4smmdjjx6gcsv (provider=blacksmith-testbox, sync=delegated, exit 0).
  • git diff --check origin/main...HEAD passed.
  • codex review --base origin/main found valid issues in earlier patch revisions; accepted findings are fixed here. The final review runner was stopped because it recursively spawned nested review commands after the accepted fixes; final closeout used manual diff review plus the targeted and Testbox proof above.

Real behavior proof

Behavior addressed: OpenAI Codex OAuth re-login can no longer report CLI success while a live gateway races the auth-profile store and keeps an expired/reused refresh-token snapshot.

Real environment tested: Live managed gateway on a remote Linux VPS reproduced the pre-fix failure with secrets and identity redacted. The post-fix code was validated with focused local tests and a Testbox-through-Crabbox changed gate that included core, extension, and plugin-SDK contract lanes.

Exact steps or command run after this patch: focused Vitest command and Testbox pnpm check:changed command listed in Verification.

Evidence after fix: CLI/configure/onboard OAuth/API-key/token auth writers now use locked auth-profile updates, preserve secret normalization, and throw on failed persistence instead of updating config/logging success.

Observed result after fix: Focused tests passed; Testbox-through-Crabbox changed gate passed with lanes=core, coreTests, extensions, extensionTests, docs.

What was not tested: I have not re-run live OpenAI Codex OAuth on the production VPS with real user credentials after this code change.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels May 16, 2026
@vincentkoc
vincentkoc force-pushed the fix/auth-login-lock branch from da31cf6 to c49c61d Compare May 16, 2026 12:14
@clawsweeper

clawsweeper Bot commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR routes CLI/provider auth login credential writes through locked auth-profile updates, preserves normalization/save behavior, adds regression coverage, and records a changelog fix.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main writes CLI/provider OAuth login credentials through non-locking upsertAuthProfile while live gateway writers save the same store under the lock. I did not run a live OAuth credential flow in this read-only review.

Real behavior proof
Not applicable: This is a maintainer/MEMBER PR, so the external contributor real-behavior-proof gate does not apply; the PR body includes redacted incident context and test/Testbox proof but no post-fix live OAuth rerun.

Next step before merge
Draft protected maintainer PR with no blocking code finding; next action is maintainer validation and undraft/merge judgment, not ClawSweeper repair.

Security
Cleared: The diff touches credential persistence but uses existing auth-store locking and secret normalization without adding dependencies, workflow execution, or broader secret exposure.

Review details

Best possible solution:

Land a maintainer-reviewed version after refreshed focused tests and changed-gate proof verify the latest head, keeping serialization at the shared auth-profile write boundary.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection gives a high-confidence reproduction path: current main writes CLI/provider OAuth login credentials through non-locking upsertAuthProfile while live gateway writers save the same store under the lock. I did not run a live OAuth credential flow in this read-only review.

Is this the best way to solve the issue?

Yes, the patch targets the implicated auth-profile persistence boundary and preserves the old normalization/save options while making lock failure fail the login. The safest merge path is to refresh tests and Testbox proof on the latest draft head before landing.

Acceptance criteria:

  • node scripts/run-vitest.mjs src/commands/models/auth.test.ts src/commands/onboard-auth.test.ts src/plugins/provider-auth-choice.test.ts
  • node scripts/crabbox-wrapper.mjs run --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref --idle-timeout 90m --ttl 240m --timing-json -- CI=1 NODE_OPTIONS=--max-old-space-size=4096 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=900000 OPENCLAW_TESTBOX=1 OPENCLAW_TESTBOX_REMOTE_RUN=1 pnpm check:changed
  • If feasible before landing, rerun the live OpenAI Codex OAuth re-login race scenario with secrets redacted.

What I checked:

  • Protected PR metadata: GitHub API data shows this PR is open, draft, authored by a MEMBER, labeled maintainer, and currently at head b3f35e5. (b3f35e5435a5)
  • Current main non-locking CLI write: Current main persists provider auth results with upsertAuthProfile before promoting the profile, so the CLI login path can write the auth-profile store outside the file-lock path described in the PR body. (src/commands/models/auth.ts:314, 1426112f9561)
  • Current main non-locking OAuth helper write: Current main writeOAuthCredentials also calls upsertAuthProfile directly for the primary OAuth credential and sibling-agent sync, matching the stale-auth race surface described by the PR. (src/plugins/provider-auth-helpers.ts:294, 1426112f9561)
  • Existing lock contract: Current main already has updateAuthProfileStoreWithLock acquiring the auth-store file lock and reloading from disk before saving, which is the right shared primitive for serializing these login writes. (src/agents/auth-profiles/store.ts:465, 1426112f9561)
  • PR routes changed auth writers through the lock: The PR diff changes CLI auth, provider auth choice, and OAuth credential helper writes to await locked upsert helpers that throw on lock/write failure. (src/commands/models/auth.ts:313, b3f35e5435a5)
  • PR preserves locked upsert semantics: The PR diff normalizes token/API-key credentials in locked upsert and lets updateAuthProfileStoreWithLock pass save options through to saveAuthProfileStore, preserving the old local upsert behavior. (src/agents/auth-profiles/profiles.ts:106, b3f35e5435a5)

Likely related people:

  • vincentkoc: Authored the current PR and the earlier merged live-writer stale-auth fix that introduced the lock/reload remediation this PR extends to login writers. (role: recent auth-store remediation contributor; confidence: high; commits: 03231c063387, b3f35e5435a5; files: src/agents/auth-profiles/store.ts, src/commands/models/auth.ts, src/plugins/provider-auth-choice.ts)
  • steipete: Commit history and PR RCA tie the original provider auth module and non-locking CLI/OAuth helper persistence paths to March refactor work. (role: introduced provider auth persistence path; confidence: medium; commits: a33caab280f3, 6d6825ea182a; files: src/commands/models/auth.ts, src/plugins/provider-auth-helpers.ts)
  • pashpashpash: Recent merged OpenAI/Codex default-routing changes made this older auth persistence race hot for normal OpenAI users, per the PR body and related PR context. (role: recent OpenAI/Codex routing contributor; confidence: medium; commits: 1c3399010815, 3b44dfc367af; files: src/commands/models/auth.ts, src/plugins/provider-auth-choice.ts)
  • BunsDev: The PR body links the stale Codex auth profile routing/order repair to the same incident area, though not to the missing write serialization itself. (role: adjacent Codex auth routing contributor; confidence: medium; commits: 626e07886383; files: src/agents/auth-profiles/profiles.ts, src/commands/models/auth.ts)

Remaining risk / open question:

  • The latest head is still draft and the PR body says focused tests/Testbox changed-gate proof needed refresh after the latest force-push.
  • No post-fix live OpenAI Codex OAuth re-login was rerun; the current proof is source-level plus focused/Testbox coverage from the pre-follow-up revision.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 1426112f9561.

@vincentkoc
vincentkoc force-pushed the fix/auth-login-lock branch 4 times, most recently from b3f35e5 to f90007d Compare May 16, 2026 12:42
@vincentkoc
vincentkoc marked this pull request as ready for review May 16, 2026 12:46
@vincentkoc
vincentkoc requested a review from a team as a code owner May 16, 2026 12:46
@vincentkoc
vincentkoc force-pushed the fix/auth-login-lock branch from f90007d to f6c20fc Compare May 16, 2026 12:51
@vincentkoc
vincentkoc merged commit f410a95 into main May 16, 2026
101 of 103 checks passed
@vincentkoc
vincentkoc deleted the fix/auth-login-lock branch May 16, 2026 12:52
@clawsweeper clawsweeper Bot mentioned this pull request Jun 28, 2026
Closed
@dagmarjeeves-lab dagmarjeeves-lab mentioned this pull request Jul 14, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling commands Command implementations extensions: anthropic extensions: cloudflare-ai-gateway extensions: github-copilot extensions: zai maintainer Maintainer-authored PR size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vincentkoc