Uh oh!
There was an error while loading. Please reload this page.
fix(platform-objects): sys_session.token stops serializing on the data API — internal: true (#7823) - #7996
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 5 package(s): 25 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also reference the affected code. These are read-only:
|
huangyiirene
commented
Aug 12, 2026
⛔ CI RED — and it is the break-the-product direction, not a flake. ⛔ Auto-merge NOT armed, PR stays draft.Every job's own conclusion read.
|
os-zhuang
commented
Aug 13, 2026
⛔ Not mergeable as it stands — and the red is not a flake. Diagnosis from a neighbouring seat.Posted by the State8 jobs red, all from 2026-08-12 10:23–10:35Z, and the PR has not been touched since 10:45Z — roughly 24 hours:
|
…sion-token-internal
…sion-token-internal
…e generic-data-path ingress, and route better-auth session readbacks through resolveInternalField (#7823) A-prime + compose, per the 2026-08-13 maintainer ruling: - objectql: the engine's two omitInternalFields write-response sites are removed — engine write results stay whole, so better-auth's createWithHooks reads the minted sys_session.token back and signIn/signUp work. The generic READ-path strip is unchanged. - metadata-protocol: omitInternalFieldsFromWriteResponse (single exported helper) applied by every *Data write face — createData / cloneData / updateData / createManyData / insertManyData / updateManyData / batchData — plus a tripwire test that enumerates the *Data surface, fails on any face a flagged sentinel reaches, and fails on any new face with no recipe. - rest: the cross-object batch update mouth (direct ql.update) applies the same strip through the protocol instance (dormant today — no flagged object grants bulk — wired so the guarantee does not depend on that). - plugin-auth: session rows read back through the better-auth adapter get token re-attached via Engine.resolveInternalField (#8118's accessor), so revoke-other-sessions / sliding refresh / expired cleanup act again while the generic data API keeps returning token-less rows. Fail-closed when a stripped row meets an engine without the accessor. - dogfood: revoke-other-sessions pinned on the other session's liveness, and expired-session cleanup pinned on the row, not the status code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy
…er A-prime (#7823) They pinned exactly the limb the ruling relocates — the engine omitting internal fields from its own insert/update results, which is what broke signIn. Replaced wholesale (fixture-triage rule): they now pin the LIVENESS half — engine write results KEEP the flagged value — so re-adding an engine-level strip (the regression that broke authentication) goes red here, while the external-body guarantee is pinned at the ingress by the tripwire and the dogfood suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy
os-zhuang
commented
Aug 13, 2026
PM note — the PR BODY must be rewritten before this leaves draft. It currently states a falsified premise as its central justification. (
Three claims in the body are no longer true:
⭐ What stands and must be KEPT in the rewrite — this is not a request to discard the body:
The rewrite needs to replace the mechanism narrative — one declaration honoured at three sites → the strip relocated to the generic-data-path ingress, with a shared helper and a tripwire, plus the lifecycle readback through
Generated by Claude Code |
…sion-token-internal
…he shared dispatch predicate (#7823) check:engine-double-contract flagged makeSentinelEngine()'s delete()/update() as unpinned engine doubles — exactly the toolchain trap AGENTS.md names. Route both through assertEngineDeleteDispatch/assertEngineUpdateDispatch from @objectstack/metadata-core, the same pattern protocol.batch-verb-driver-text.test.ts already uses. No behavioural change to the tripwire itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy
Fixes#7823
size/xl). Rewritten by thedomain:engine-coreverification/finishing pass (sessionsession_01RDTnVvsgA6cUZ4xFVtPZRy) that ran the discriminating tests below; the sections marked ⭐ are carried over unchanged from the original body because they remain accurate.The defect
sys_session.token— the live bearer credential for an active session — declareddescription: 'Opaque session token — never exposed in UI'and then serialized anyway on the generic data path.⭐ The persona is precise: this is an ADMIN-CROSS-USER disclosure, not an any-authenticated-caller one. Measured on a real engine (
bootStack(showcaseStack), in-process HTTP + sqlite-wasm):?select=id,tokenGET /data/sys_session(list)tokenon every row — own and every other user'sGET /data/sys_session/{another user's id}?select=id,tokensys_session_selfRLS was already holding that line, untouched here⭐ Why this is more than exposure
#7728's sibling column (
sys_api_key.key) is a stored SHA-256 hash. This one is not — the disclosure was replay-proven: a member's token, taken exactly as it came back to the admin off the data API, authenticates as that member when sent as anAuthorization: Bearerheader carrying it. So the defect was admin-to-member impersonation, and any admin-adjacent read (an integration, a leaked admin API response, a support tool) inherited it. The distinction that matters is audited impersonation (the sanctioned path, carryingimpersonated_by) versus silent, indefinite session theft that survives the admin's own rights being revoked.What changed and why (relocated, not a one-liner)
The first shape declared
internal: trueon the field and honoured it at three places: the engine's read-path strip (unchanged, still closes the disclosure) plus two engine-level write-response sites inpackages/objectql/src/engine.ts(omitInternalFields, called after the insert and by-id-update hook dispatch). That write-response placement conflated two different guarantees:createWithHooksreads the mintedsys_sessionrow back off exactly that insert result to build its own response, so stripping there brokesignIn/signUpoutright (measured:verify signIn: no token in response).Plain removal of the two engine limbs was also measured wrong before this PR's current shape landed: the by-id-update limb was the sole closure of #7728's fourth measured surface — with it neutralised,
PATCH /data/sys_api_key/{id}answered200with the stored 64-hexkeyhash in the body. This is the measurement that falsified the original body's "no reader exists" framing: a reader did exist, just not in the UI layer that was checked — it was better-auth's own storage adapter, sitting directly on the engine's insert/update results.The maintainer's ruling (A-prime + compose, 2026-08-13) resolves both measurements by relocating rather than removing:
createData/updateData/cloneDataand every other*Datawrite face inpackages/metadata-protocol/src/protocol.ts(plus the REST cross-object batch's directql.updatemouth inpackages/rest/src/rest-server.ts) — through one exported helper,omitInternalFieldsFromWriteResponse(packages/metadata-protocol/src/write-response-internal-fields.ts).sys_api_keyPATCH body stays hash-free at the ingress ⇒ behaviour and the existing dogfood pin (api-key-hash-not-serialized.dogfood.test.ts) are byte-for-byte unchanged.The ruling's own condition — gated in this PR, not deferred: relocating to the ingress means a future
*Datawrite face that forgets the helper leaks silently.protocol.write-response-internal-fields.tripwire.test.tscloses that gap: it enumerates every*Datamethod on the protocol class at runtime (a prototype walk by the*Datanaming convention, not a hand-written list), requires each to have a registered "recipe" or fails the suite with instructions, and deep-scans each face's response for a sentinel value a fixture engine's write results always carry. A built-in negative control (leakyData, a subclass that returns an engine result verbatim) proves the enumeration and the scan both go red on a real leak.I re-verified this tripwire is not just asserted but actually load-bearing: temporarily removed the helper call from
createData, watchedcreateData: response never carries the internal sentinelgo RED (createData leaked an internal field: {...,"vault_secret":"INTERNAL-SENTINEL-7823-NEVER-SERIALIZED"}), then restored the file from the committed HEAD (byte-identical, confirmed bygit hash-object) and watched all 18 tests go green again. Because the enumeration is a runtime prototype walk rather than a hand-kept list, it also catches a new, not-yet-imagined fourth ingress — demonstrated by the same negative control, which is exercised on every run.Lifecycle readback — the second half of the ruling (Q2: compose)
The read-path strip removing
tokenfrom every find/findOne result also starves better-auth's own session-lifecycle routes, which readsession.tokenback off the adapter's read results (not just as awherefilter):revoke-other-sessionsfilterslistSessions(userId)rows bytokenand deletes by it;Measured on
main-shape code before this fix:POST /auth/revoke-other-sessionsanswered200 {"status":true}while the other session kept authenticating — a security control reporting success while doing nothing.The fix wires the adapter's session reads (
findOne/findManyinpackages/plugins/plugin-auth/src/objectql-adapter.ts) through a new seam,session-token-readback.ts, which re-attaches the token viaEngine.resolveInternalField— the privileged batch accessor already onmainfrom #8118, not a new mechanism. Bounded tosys_session, respects the caller's own projection (a read that named columns withouttokenkeeps that shape), and fails closed and loudly if a stripped session row ever meets an engine with no accessor, rather than degrading into another silent no-op.⭐ Not retyped, deliberately
Field.secretwould encrypt at rest and replace the column with asys_secretref, destroying the by-token session lookup better-auth performs on every authenticated request — it would break authentication in order to fix a disclosure.Field.passwordis inert here: the read mask skipspasswordonmanagedBy: 'better-auth'objects, and collects by type regardless, which atextcolumn never satisfies. Two independent barriers, so the column staystext.⭐ Storage, filtering and indexing are untouched
The strip runs on rows the driver has already produced, after the predicate has been evaluated and the unique index on
tokenused:GET /auth/get-session⇒ 200)where: { token }lookup still resolves the row server-side, while that same row comes back with notokenkey?select=id,tokenreturns 200 without the column — closed by construction, since the strip acts on result rows rather than the projectionWithout the liveness half, a change that simply broke authentication would satisfy every "absent" assertion.
Evidence from this verification pass
All commands run from a fresh worktree at this branch's tip (dependency closure built first per
pnpm --filteron each package's dependency set, using the^...suffix), inside the sharedflockverification lock:api-key-hash-not-serialized.dogfood.test.ts, untouched by this branch — last touched in4c5e80e, api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728): 5/5 tests green, isolated run.session-token-not-serialized.dogfood.test.ts: 11/11 tests green, isolated run — includingrevoke-other-sessions ACTUALLY revokesandthe by-token session lookup still resolves server-side.@objectstack/metadata-protocolfull suite: 82 files / 1214 tests green (includes the tripwire, 18/18).@objectstack/objectqlfull suite: 197 files / 3539 tests green.@objectstack/plugin-authfull suite: 52 files / 1182 tests green (includessession-token-readback.test.ts).@objectstack/restfull suite: 112 files / 1843 tests green.@objectstack/platform-objectsfull suite: 17 files / 351 tests green.tsc --noEmit) clean onobjectql,plugin-auth,rest,platform-objects.POST /auth/sign-in/email: response keys['redirect','token','user'], token present (32 chars); the same session'sGET /data/sys_session/{id}comes back with 13 other columns and notokenkey.git hash-object.check:engine-double-contract,check:nul-bytes,check:i18n,check:adr-anchors,check:authz-resolver,check:cross-package-test-inputs,check:docs-audit-scope,check:durability-log-level,check:filter-alias-parity,check:meta-type-normalized,check:route-envelope,check:stack-collection-maps,check:test-source-alias,check:type-source-resolution,check:query-options-erasure,check:type-check-coverage.Packages touched
@objectstack/platform-objects(sys-session.object.ts—internal: true),@objectstack/objectql(engine.ts— removed the two write-response strip sites;internal-fields.test.tsre-judged),@objectstack/metadata-protocol(protocol.ts, newwrite-response-internal-fields.ts+ its tripwire test,index.tsexport),@objectstack/plugin-auth(objectql-adapter.ts, newsession-token-readback.ts+ test),@objectstack/rest(rest-server.ts— batch update arm applies the shared strip).Generated by Claude Code