fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration by yetval · Pull Request #97881 · openclaw/openclaw

yetval · 2026-06-29T16:53:07Z

What Problem This Solves

openclaw doctor --fix migrates a user's legacy auth credentials into the canonical per-agent SQLite store and then backs up and removes the old JSON files. Both migration writers rebuild each credential from a hardcoded subset of fields and silently drop the rest, so the migration permanently loses refresh-critical and secret material:

applyLegacyAuthStore (src/agents/auth-profiles/persisted.ts), which imports the legacy auth.json file, drops keyRef and tokenRef (secret-ref indirection) and the OAuth clientId, idToken, and chatgptPlanType fields.
coerceLegacyFlatCredential (src/commands/doctor-auth-flat-profiles.ts), which imports a flat-shaped auth-profiles.json, drops the identical fields and returns null for a secret-ref-only credential, so that credential is not imported at all.

The post-migration verification only checks that profile ids exist, so the corruption passes silently. The legacy file is then removed, and the user later hits auth failures (OAuth that can no longer refresh, or an API key with no secret) with no upgrade-time warning. The two writers read different files (auth.json vs a flat auth-profiles.json), so each is independently reachable; fixing one is not enough.

This is distinct from #97522/#97541 (doctor overwrites a second OAuth account), #96544 (map-key collision), and #93814 (trajectory export). The open #93352 adds a runtime read-path importer in different functions and does not address the dropped fields.

Why This Change Was Made

The asymmetry is reader vs writer. The canonical credential reader parseCredentialEntry (used by loadLegacyAuthProfileStore and coercePersistedAuthProfileStore) already normalizes and preserves the full credential, including keyRef, tokenRef, clientId, idToken, and chatgptPlanType. The two migration writers hand-rebuilt a lossy copy of that normalization instead of reusing it.

applyLegacyAuthStore receives an already-parsed AuthProfileCredential (LegacyAuthStore = Record<string, AuthProfileCredential>), so it now preserves that credential as-is and only defaults the provider from the map key. This deletes the duplicate field-by-field rebuild.
coerceLegacyFlatCredential now delegates field extraction to the same canonical parser via the new parseLegacyCredentialEntry, keeping its existing flat-shape type inference as the acceptance gate and hasUsableAuthProfileCredential as the usability gate. This keeps every prior accept/reject decision identical while preserving all fields, and removes the duplicate parser.

Net production code drops by 46 lines, and both migration paths now share one normalization path, so they cannot drift apart again.

User Impact

Users running openclaw doctor --fix with a legacy auth.json or a flat auth-profiles.json keep their full credentials after migration: OAuth refresh material (clientId, idToken, chatgptPlanType) survives, secret-ref-backed API keys and tokens survive, and secret-ref-only credentials are no longer dropped. No config or behavior change for credentials that already migrated cleanly.

Evidence

Both migration legs were driven with their real production entry points on pristine origin/main (6de357ad47) and on this patch, with identical inputs.

The legacy auth.json leg was driven through loadLegacyAuthProfileStore + applyLegacyAuthStore (the exact functions the doctor import calls). The flat auth-profiles.json leg was driven end to end through maybeMigrateAuthProfileJsonStoresToSqlite (the real doctor migration), then read back from the real SQLite store via loadPersistedAuthProfileStore.

Verification

node scripts/run-vitest.mjs src/agents/auth-profiles/persisted-boundary.test.ts src/commands/doctor-auth-flat-profiles.test.ts passes (38 tests). The 3 new tests fail on pristine origin/main and pass with this patch.
oxlint and oxfmt --check clean on the 4 changed files.
node scripts/run-tsgo.mjs -p tsconfig.core.json and -p test/tsconfig/tsconfig.core.test.json both clean.
Full build not run (no packaging, dynamic-import, or module-boundary changes).

Real behavior proof

Behavior addressed: openclaw doctor --fix migration dropped OAuth refresh fields (clientId/idToken/chatgptPlanType) and secret refs (keyRef/tokenRef) from both the legacy auth.json writer and the flat auth-profiles.json writer, silently degrading or destroying credentials.
Real environment tested: drove the real migration functions on pristine origin/main 6de357a and on the patched tree with identical inputs. The legacy leg ran loadLegacyAuthProfileStore + applyLegacyAuthStore against an on-disk auth.json. The flat leg ran maybeMigrateAuthProfileJsonStoresToSqlite end to end and read the result back from the real per-agent SQLite store via loadPersistedAuthProfileStore. Everything stayed real; nothing stubbed.
Exact steps or command run after this patch: wrote a legacy auth.json and a flat auth-profiles.json holding OAuth extras and secret-ref credentials, ran the real migration, then printed the resulting credential fields.
Evidence after fix:

# BEFORE (pristine origin/main 6de357ad47)
# legacy auth.json leg (loadLegacyAuthProfileStore + applyLegacyAuthStore)
migrated.clientId       = undefined
migrated.idToken        = undefined
migrated.chatgptPlanType= undefined
migrated openai api_key has neither key nor keyRef -> credential is unusable (auth lost).
migrated anthropic token has neither token nor tokenRef -> credential is unusable (auth lost).
# flat auth-profiles.json leg (real migration -> SQLite -> loadPersistedAuthProfileStore)
chutes:default  clientId        = undefined
chutes:default  idToken         = undefined
chutes:default  chatgptPlanType = undefined
openai:default  keyRef          = undefined

# AFTER (this patch, identical inputs)
# legacy auth.json leg
migrated.clientId       = "chutes-client-id-123"
migrated.idToken        = "ID_TOKEN_xyz"
migrated.chatgptPlanType= "pro"
migrated api_key fields: email,keyRef,provider,type
migrated token   fields: expires,provider,tokenRef,type
# flat auth-profiles.json leg
chutes:default  clientId        = "chutes-client-id-123"
chutes:default  idToken         = "ID_TOKEN_xyz"
chutes:default  chatgptPlanType = "pro"
openai:default  keyRef          = {"source":"env","provider":"default","id":"OPENAI_API_KEY"}

Observed result after fix: on identical inputs the migrated credentials now retain clientId, idToken, chatgptPlanType, and the keyRef/tokenRef secret references on both the legacy auth.json path and the flat auth-profiles.json path, where before they were undefined or the credential was dropped entirely.
What was not tested: no live provider auth request was made against a real OAuth or API endpoint; full build was not run.

Open question for maintainers

The code-level loss is proven and unmasked on both writers. The real-world blast radius depends on whether shipped flat auth.json / auth-profiles.json files actually carried keyRef, tokenRef, clientId, idToken, or chatgptPlanType. The canonical reader contains explicit parsing for exactly these fields out of the legacy flat shape, which is strong evidence they shipped, but a maintainer who knows the shipped on-disk history can confirm severity.

clawsweeper · 2026-06-29T16:55:00Z

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 12:59 PM ET / 16:59 UTC.

Summary
The PR changes legacy auth-profile doctor migration so both auth.json and flat auth-profiles.json imports preserve SecretRef and OAuth refresh fields, with focused regression tests.

PR surface: Source -46, Tests +136. Total +90 across 4 files.

Reproducibility: yes. Source inspection shows current main drops the cited fields in both migration writers, and the PR body plus tests exercise the actual migration functions with before/after credential-field output.

Review metrics: 1 noteworthy metric.

Credential migration writers: 2 changed. Both legacy auth.json and flat auth-profiles.json doctor import writers are touched, so reviewers should evaluate the upgrade behavior as one credential-migration surface.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-auth-flat-profiles.test.ts, migration/backfill/repair: src/commands/doctor-auth-flat-profiles.ts, serialized state: src/agents/auth-profiles/persisted-boundary.test.ts, unknown-data-model-change: src/commands/doctor-auth-flat-profiles.ts. Migration or upgrade compatibility proof is recorded; maintainers should verify it before merge.

Root-cause cluster
Relationship: canonical
Canonical: #97881
Summary: This PR is the canonical current item for doctor auth migration field preservation; related auth migration items are adjacent or partially overlapping but do not replace it.

Members:

partial_overlap: fix(auth-profiles): import legacy auth-profiles.json into SQLite store on load #93352 - It covers lazy runtime import of legacy auth-profiles JSON into SQLite, while this PR covers field loss during doctor migration writers.
adjacent_distinct: doctor --fix silently overwrites a second OAuth account for the same provider, collapsing auth.profiles 2->1 and wiping its config metadata #97522 - It reported a different doctor auth data-loss bug involving OAuth account overwrite during legacy-id repair and is already closed.
adjacent_distinct: fix(doctor): merge colliding model-ref map keys instead of dropping #96544 - It is another doctor migration data-preservation fix, but for model-ref map key collisions rather than auth credential field preservation.
independent: fix(trajectory): export legacy v1 sessions without entry timestamps #93814 - It was mentioned as distinct context but concerns trajectory export, not auth-profile migration.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] This changes doctor migration for credential material, so shipped legacy auth shapes beyond the covered auth.json and flat auth-profiles.json examples remain the main upgrade-review boundary.
[P1] The PR intentionally preserves more OAuth and SecretRef material into SQLite; maintainers should confirm that this matches the credential-store privacy and migration contract for upgraded installs.

Maintainer options:

Land after auth-migration owner pass (recommended)
The branch has focused before/after proof and tests for both changed writers, so maintainers can accept the residual shipped-shape risk after owner review and required CI completion.
Add broader shipped-shape coverage first
If maintainers want a lower-risk landing, add one more regression around an observed shipped auth-profile shape before merge.
Hold for the runtime-import PR only if scope converges
Pause only if maintainers decide this doctor migration fix should be bundled with the separate runtime lazy-import work in fix(auth-profiles): import legacy auth-profiles.json into SQLite store on load #93352.

Next step before merge

No automated repair is needed; maintainer review should focus on the upgrade-sensitive credential migration risk and then decide whether to merge.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes auth migration field preservation only and adds focused tests without new dependencies, workflows, or execution hooks.

Review details

Best possible solution:

Land the focused migration fix that reuses the canonical credential parser for both legacy writers, keeping coverage on both JSON migration paths and avoiding any new runtime fallback.

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

Yes. Source inspection shows current main drops the cited fields in both migration writers, and the PR body plus tests exercise the actual migration functions with before/after credential-field output.

Is this the best way to solve the issue?

Yes. Reusing the canonical credential parser is the best local fix because it removes duplicate field-copy logic in both writers instead of adding another manual preservation list that can drift again.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6de357ad4772.

Label changes

Label justifications:

P1: The PR fixes upgrade-time auth credential loss that can break provider authentication after openclaw doctor --fix.
merge-risk: 🚨 compatibility: The diff changes doctor migration behavior for legacy on-disk auth stores during upgrade.
merge-risk: 🚨 auth-provider: The migrated data controls OAuth refresh fields, API key SecretRefs, token SecretRefs, and provider auth selection.
merge-risk: 🚨 security-boundary: The changed code handles secret references and OAuth/token material that are explicitly treated as sensitive on disk.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied before/after live output from real migration functions over on-disk JSON and SQLite readback, which is sufficient for this non-visual doctor migration change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied before/after live output from real migration functions over on-disk JSON and SQLite readback, which is sufficient for this non-visual doctor migration change.

Evidence reviewed

PR surface:

Source -46, Tests +136. Total +90 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	26	72	-46
Tests	2	138	2	+136
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	164	74	+90

What I checked:

Repository policy applied: Root AGENTS.md and the scoped src/agents/AGENTS.md were read; the auth/config migration compatibility guidance affected the merge-risk classification. (AGENTS.md:28, 6de357ad4772)
Current main drops fields in auth.json migration: applyLegacyAuthStore rebuilds each credential from a narrow field subset, so api_key/token SecretRefs and OAuth clientId, idToken, and chatgptPlanType are not copied into the SQLite-bound store. (src/agents/auth-profiles/persisted.ts:734, 6de357ad4772)
Canonical parser already preserves the requested fields: normalizeRawCredentialEntry already normalizes keyRef, tokenRef, apiKey, idToken, clientId, and chatgptPlanType, which supports reusing it instead of duplicating migration-specific field copies. (src/agents/auth-profiles/persisted.ts:118, 6de357ad4772)
Current main drops fields in flat migration: coerceLegacyFlatCredential currently returns hand-built credential objects and only keeps a small subset of fields for api_key, token, and OAuth credentials. (src/commands/doctor-auth-flat-profiles.ts:204, 6de357ad4772)
Patch routes both writers through canonical preservation: At PR head, parseLegacyCredentialEntry exposes the existing parser, applyLegacyAuthStore copies the parsed credential as-is, and the flat importer delegates field extraction to that parser after preserving its type/usability gate. (src/commands/doctor-auth-flat-profiles.ts:218, 800a873414fc)
Migration entry point reaches both changed writers: maybeMigrateAuthProfileJsonStoresToSqlite imports canonical or flat auth-profile stores and separately loads legacy auth.json, then calls applyLegacyAuthStore before merging and cleanup. (src/commands/doctor-auth-flat-profiles.ts:779, 6de357ad4772)

Likely related people:

steipete: Auth profiles were moved into SQLite in merged PR refactor(auth): store auth profiles in SQLite #89102, touching both central auth-profile persistence and doctor migration files. (role: feature owner; confidence: high; commits: 815b2384c299, 9afcbbec5e60; files: src/agents/auth-profiles/persisted.ts, src/commands/doctor-auth-flat-profiles.ts)
Pick-cat: Merged work in fix(doctor): import default-agent auth profiles into sqlite #93156 extended the doctor auth-profile SQLite import path now being fixed here. (role: recent area contributor; confidence: high; commits: 79b2987f0287, ad2cd230644e, c2a56e3c3fc0; files: src/commands/doctor-auth-flat-profiles.ts, src/commands/doctor-auth-flat-profiles.test.ts)
fuller-stack-dev: Merged PR fix(auth): verify SQLite auth migration before cleanup #91740 added verification-before-cleanup behavior in the same doctor SQLite auth migration file. (role: recent migration contributor; confidence: medium; commits: f8590ca1f8d9; files: src/commands/doctor-auth-flat-profiles.ts, src/commands/doctor-auth-flat-profiles.test.ts)
alkor2000: Merged PR fix(doctor): rewrite non-canonical api_key field to key in auth profiles #86809 worked on legacy api_key alias repair in the same doctor auth-profile migration surface. (role: adjacent repair contributor; confidence: medium; commits: b99873361765; files: src/commands/doctor-auth-flat-profiles.ts, src/commands/doctor-auth-canonical-api-key-alias.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

openclaw-barnacle · 2026-07-17T04:29:50Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

…or auth migration Both legacy auth migration writers rebuilt each credential from a hardcoded field subset, dropping keyRef/tokenRef and the OAuth clientId/idToken/ chatgptPlanType, even though the canonical reader already preserves them. applyLegacyAuthStore now keeps the already-parsed credential as-is, and coerceLegacyFlatCredential delegates field extraction to the shared parseLegacyCredentialEntry while keeping its existing type and usability gates. The flat writer also no longer drops a secret-ref-only credential. This routes both migration paths through one normalization path so doctor --fix stops silently degrading or destroying credentials before removing the legacy files.

yetval · 2026-07-24T02:53:47Z

@jalehman Seems like the checks are passing can this be merged??

jalehman · 2026-07-24T03:22:06Z

Merged via squash.

Prepared head SHA: ccb381101997d5a6ecfc3a623afd66624e154976
Landed commit: edc344834ba0f4e3b561c8919cf84bdd50ff8b8b

jalehman · 2026-07-24T04:10:48Z

@jalehman Seems like the checks are passing can this be merged??

Sorry about the delay — thought I had merged this!

yetval · 2026-07-24T04:13:16Z

@jalehman Seems like the checks are passing can this be merged??

Sorry about the delay — thought I had merged this!

All good!!

…or auth migration (openclaw#97881) Both legacy auth migration writers rebuilt each credential from a hardcoded field subset, dropping keyRef/tokenRef and the OAuth clientId/idToken/ chatgptPlanType, even though the canonical reader already preserves them. applyLegacyAuthStore now keeps the already-parsed credential as-is, and coerceLegacyFlatCredential delegates field extraction to the shared parseLegacyCredentialEntry while keeping its existing type and usability gates. The flat writer also no longer drops a secret-ref-only credential. This routes both migration paths through one normalization path so doctor --fix stops silently degrading or destroying credentials before removing the legacy files.

yetval requested a review from a team as a code owner June 29, 2026 16:53

openclaw-barnacle Bot added commands Command implementations agents Agent runtime and tooling size: M labels Jun 29, 2026

yetval mentioned this pull request Jun 30, 2026

fix(doctor): merge legacy flat auth repair into existing SQLite store #98245

Merged

clawsweeper Bot mentioned this pull request Jul 10, 2026

fix(auth-profiles): import legacy auth-profiles.json into SQLite store on load #93352

Closed

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 17, 2026

jalehman self-assigned this Jul 22, 2026

jalehman force-pushed the fix/auth-migration-field-loss branch from 800a873 to ccb3811 Compare July 22, 2026 19:23

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jul 23, 2026

jalehman merged commit edc3448 into openclaw:main Jul 24, 2026
192 checks passed

github-actions Bot mentioned this pull request Jul 24, 2026

📡 Upstream Digest — 2026-07-24 05:15 UTC curtismercier/openclaw-mods#1387

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration#97881

fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration#97881
jalehman merged 1 commit into
openclaw:mainfrom
yetval:fix/auth-migration-field-loss

yetval commented Jun 29, 2026

Uh oh!

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 17, 2026

Uh oh!

yetval commented Jul 24, 2026

Uh oh!

Uh oh!

jalehman commented Jul 24, 2026

Uh oh!

jalehman commented Jul 24, 2026

Uh oh!

yetval commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

yetval commented Jun 29, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Verification

Real behavior proof

Open question for maintainers

Uh oh!

clawsweeper Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 17, 2026

Uh oh!

yetval commented Jul 24, 2026

Uh oh!

Uh oh!

jalehman commented Jul 24, 2026

Uh oh!

jalehman commented Jul 24, 2026

Uh oh!

yetval commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 29, 2026 •

edited

Loading