Uh oh!
There was an error while loading. Please reload this page.
fix(security): sys_account OAuth access/refresh/id tokens stop serializing on the data API (#7987) - #8675
Conversation
…API (#7987) `sys_account.access_token`, `.refresh_token` and `.id_token` hold each user's live third-party OAuth credentials in cleartext, on an object declaring `apiEnabled: true, apiMethods: ['get','list']`. Measured leaking on a real booted stack in BOTH personas: an admin read another member's refresh token verbatim by id, and a member read their own off the self-scoped list — the arm `sys_session` does not have, and the sharper one, since it converts a revocable session bearer into a long-lived token this platform cannot revoke. Declares the three columns `internal: true` (#7728's flag, #7823's shape) and widens plugin-auth's readback seam to cover them: better-auth reads these back off adapter result rows (`findAccounts` feeds /get-access-token, /account-info and /refresh-token), so the read strip alone would answer REFRESH_TOKEN_NOT_FOUND and hand back an empty access token. Recovered through `Engine.resolveInternalField` (#8118) — no engine carve-out, no second accessor. `password` / `previous_password_hashes` stay out of scope per the card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#7987
sys_account.access_token,.refresh_tokenand.id_tokenhold each user's live third-party OAuth credentials — the tokens ObjectStack received from Google, GitHub or an OIDC IdP — in cleartext, on an object declaringapiEnabled: true, apiMethods: ['get','list']. They are declaredinternal: truehere, and better-auth's readback seam is widened so its own token routes keep working.The card's load-bearing question, measured
The card and both PM comments parked this on one question: does any better-auth login/refresh path read these values off a result row? It does — traced in
better-auth/dist/api/routes/account.mjsagainst the pinnedbetter-auth@1.7.0-rc.2:internalAdapter.findAccounts(userId)issuesfindManywith no projection, so the row set is exactly what the read strip empties;resolveUserAccount()picks a row out of it, andgetValidAccessToken()— behind/get-access-tokenand/account-info— then readsaccount.refreshTokento decide whether to refresh,account.accessTokento answer with, andaccount.idTokento carry forward;POST /refresh-tokenreadsaccount.refreshTokenand answersREFRESH_TOKEN_NOT_FOUND(400) when it is absent.This is not a fork, because it is the shape PR #7996 already landed a mechanism for: the adapter-level readback through
Engine.resolveInternalField(#8118). The card was parked when that mechanism did not exist yet; it does now, andinternal-field-readback.tssays in its own header that widening it tosys_accountwas #7987's call.Both personas measured leaking, on a real booted stack
bootStack(showcaseStack), in-process HTTP + sqlite-wasm, with a token planted on a member's account row, run against a build differing from this PR only in the three flags:GET /data/sys_account/{another user's account id}— 200, that member's refresh token verbatim;GET /data/sys_account— 200, their own refresh token verbatim, granted by thesys_account_selfRLS policy (selectonuser_id == current_user.id).The member arm has no analogue in #7823 and is the sharper of the two: it converts a short-lived, revocable ObjectStack session bearer into a long-lived third-party refresh token that this platform cannot revoke at all. That also answers triage's severity question (comment 5265158443, verification 2): these rows are reachable for a plain non-admin persona — for their own row.
Neither collector reached these columns:
maskSecretFieldscollects by field TYPE (textareais neithersecretnorpassword) and exempts objects withmanagedBy: 'better-auth', which this object is. Two independent reasons, as the card's survey said.What changed
packages/platform-objects/src/identity/sys-account.object.ts— the three columns getinternal: trueplus a description recording the contract. Not retyped toField.secret()(better-auth owns the writes; the engine would sit between it and its own adapter) andField.password()is inert here — both per the card's accepted argument.packages/plugins/plugin-auth/src/session-token-readback.ts→internal-field-readback.ts— the existing seam, widened from one column to a per-object table and renamed to match.packages/objectqlaccessor is CONSUMED, not restructured — no signature change, so the held-out sibling [security]sys_email.headers_jsonstores custom headers cleartext — same shape assys_http_delivery; adopt whatever remedy #8118 lands, do not decide it twice #8149 is unaffected. The accessor resolves one field per call by contract, so three columns cost three id-batched driver reads per page; widening it to a field set would restructure a surface other consumers share, to save two indexed point-reads on a path dominated by the provider round trip and the password KDF.The one non-obvious decision: fail-closed posture is per column
#7823's seam treats "the key is missing" as proof the strip ran, and throws when the engine offers no accessor. That is sound for
sys_session.token, which isrequired: true— a session row without a token does not exist.It is false for these three, which are
required: falseand are genuinely empty on every credential (password) account. Carrying the rule over broke ordinary sign-in against the in-memory fake engines — measured: 16 red tests acrosssession-of-record,session-tombstoneandimpersonation-bearer-rotation, becausefindCredentialAccountreads exactly such a row on the sign-in path. So each column now declares whether its absence is diagnostic (absenceProvesStrip), and the session column's fail-closed contract is pinned by its own test so a future edit cannot quietly take it down with the account columns.The residual risk that buys is a version-skewed engine that strips but predates #8118 — which for these columns degrades loudly (a 400 from the token routes), where the session column would have degraded into a security control silently reporting success.
Verification
Run at
011d9cd4, the head of this branch, with a clean tree.pnpm --filter @objectstack/plugin-auth --filter @objectstack/platform-objects test— 53 files / 1204 tests and 20 files / 369 tests, all passing. Baseline on a cleanorigin/mainworktree was 1195, so the +9 are this PR's.typecheckonplugin-auth,platform-objects,dogfood— clean.session-token-not-serialized,api-key-hash-not-serialized) — 3 files / 24 tests, all passing, so the seam rename does not disturb Check whethersys_session.token— a live session credential — serializes over the data API (ADR-0100 channel 3 has no read protection) #7823's or api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's own proofs.scripts/pm/dispatch-gates.mjs:check:nul-bytes,check:i18n,check:test-source-alias,check:type-source-resolution,check:changeset-gate-self-tests,check:objectui-changeset,check:query-options-erasure,check:type-check-coverage,check:type-check-debt(with the full workspace closure built, as lint.yml does),check-adr-0087-registration,check-changeset-no-major,check-empty-changeset— all green.The fixture is proven discriminating, in both directions
A credential-account row has all three columns empty, so "the response has no
refresh_tokenkey" would be true whether or not the fix exists. The fixture therefore plants real values and re-reads them out of storage through the privileged accessor underassertArmed(#8074) before asserting anything.Ablation (
internal: falseon the three columns,platform-objectsrebuilt,scripts/ablation-dist-preflight.mjsconfirming the marker reacheddist/— and confirming with--absentthat it left again on restore):beforeAllas DISARMED, becauseresolveInternalFieldrefuses a column that is not flagged;Restored and rebuilt, the fixture is green again (8/8).
Notes
4bfe1a539, which carries247c55ad3— the fix for the post-GA.changesetself-test failures that are red on the older base. Both of those gates are green at this head; nothing in this diff addresses them.packages/platform-objectsplus a seam inpackages/objectql/src. The landed precedent's seam is inpackages/plugins/plugin-auth/src, not objectql — that is the identity authority's own storage seam, and where Check whethersys_session.token— a live session credential — serializes over the data API (ADR-0100 channel 3 has no read protection) #7823 deliberately put the privileged dereference to keep the engine's read path carve-out-free. Reported rather than silently taken.passwordandprevious_password_hashesserializing on the same response. They are one-way hashes (ADR-0100's third channel), explicitly carved out by this card, and are not touched here.Generated by Claude Code