Skip to content

Check whether sys_session.token — a live session credential — serializes over the data API (ADR-0100 channel 3 has no read protection) #7823

Description

@os-zhuang

Status: UNMEASURED. Step one is a measurement, not a fix.

Raised as an out_of_scope_findings item by the #7728 dev (#7728 report), who flagged it as "worth checking first" — i.e. potentially more serious than the card it fell out of. Filed unassigned; assign at the moment you start.

Nothing here has been observed on a running system. The #7728 dev inferred it structurally from the same code paths it measured for sys_api_key.key, and said so plainly. Treat every claim below as a hypothesis with a named place to check it.

The hypothesis

packages/platform-objects/src/identity/sys-session.object.ts:176 declares:

token: ... description: 'Opaque session token — never exposed in UI'

The mechanism that leaked sys_api_key.key — established on code in #7728 — is not field-specific:

  • the engine read path never consults hidden (no .hidden reference in engine.ts), so hidden: true does not strip a value from serialization; and
  • collectMaskedReadFields (packages/objectql/src/secret-fields.ts:88-96) masks password-typed fields only when the object is not managedBy: 'better-auth'.

sys_session is an auth-subsystem object, so if it carries managedBy: 'better-auth' the masking collector is inert on it too — ADR-0100's channel 3 (auth-subsystem one-way hashes and opaque tokens on plain text columns), which #7728 established has no read protection at all.

⚠️ Do NOT import #7728's framing — this is a different defect if it is one

#7728 is a false-declaration defect: its field claims "never exposed to clients", which is a serialization claim, and it was false.

This field claims "never exposed in UI". That is the narrower claim, and hidden: true does satisfy it. So:

  • If token serializes over the data API, the declaration is not thereby false — the defect would be a credential disclosure, argued on its own merits, not on a contradicted description.
  • Conversely a "fix the description" option does not exist here, because the description is not the thing that would be wrong.

The reason to rate this potentially above #7728 despite the weaker declaration: sys_api_key.key is a SHA-256 hash — not a usable credential, which is why #7728 was correctly kept public, unembargoed and off target:v17. A session token is the live credential itself. If it serializes to any persona that should not hold it, that is a real disclosure and the severity conversation is a different one.

Step one — measure before anything else

  1. Confirm whether sys_session is managedBy: 'better-auth' and whether token is a plain text column.
  2. On a real engine: create a session, then GET /api/v1/data/sys_session/{id} and the list endpoint. Does the token value come back, and to which personas? api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's leak was admin-readable only, because ordinary personas are 403 on the object — establish the equivalent here rather than assuming it matches.
  3. Check whether anything legitimately reads token off a data-API response (the verifier using it as a where filter is not a read — that was the distinction that made api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's strip safe).

"Latent, not live" is a complete and valid outcome. If the value never serializes, say so with the measurement and close the card — do not manufacture a fix for a path nothing exercises.

If it IS live

The fix vehicle is very likely the same one #7728 is parked on — there is currently no field-level mechanism that strips a value from the generic read path (secret encrypts at rest and would break the lookup; password is inert on better-auth objects; FLS is bypassed by system contexts and by admin). #7728's report contains the full elimination and a proposed design awaiting a maintainer ruling.

⛔ Do not build a parallel mechanism. If the measurement says live, report the dependency and stop — the PM will sequence this against #7728's ruling. Two competing masks would be a worse defect than either leak.

⛔ Do not relabel this a security card or embargo it pre-measurement; and do not assume the #7728 severity call transfers, in either direction.

Source

out_of_scope_findings #1 from the #7728 dev report. Sibling finding #2 (sys_account.password, sys-account.object.ts:208, hashed text, same unprotected class, makes no non-exposure claim) is recorded on #7728's escalation rather than filed separately — it has no independent symptom.

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    ContributorAuthor

    Findings-triage route B: promoted — finding → pm:queue (domain:metadata kept as filed).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. self-assigned this
    on Aug 12, 2026
  3. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    Collaborator

    Claim: PM loop round 5 (domain:metadata seat, huangyiirene term)

    This is a measurement card and it is dispatched as one

    ⭐ The most likely correct outcome is no code change. Two of its three endings produce no diff:

    The framing trap the card names, carried into the brief

    ⛔ Do not import #7728's framing. #7728 is a false-declaration defect: its field claims "never exposed to clients" — a serialization claim — and that was false. This field claims "never exposed in UI", which is narrower and which hidden: true does satisfy. So if token serializes, the declaration is not thereby false, and the "fix the description" option does not exist here. The defect would be a credential disclosure, argued on its own merits.

    The asymmetry that makes it worth doing now: sys_api_key.key is a SHA-256 hash — not a usable credential, which is why #7728 was correctly left public and off target:v17. A session token is the live credential. ⛔ Do not assume #7728's severity call transfers in either direction, and ⛔ do not relabel or embargo pre-measurement.

    Triage's escalation instruction is carried

    if the measurement comes back "serializes", the follow-on defect card is target:-class (published-surface credential disclosure) — flag it back to triage same-day rather than waiting a cycle.

    That is this seat's action on a live result, not the dev's; the dev's job ends at the measurement and the report.


    Generated by Claude Code

  4. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    Collaborator

    Measured: LIVE. sys_session.token serializes over the data API as the usable credential — and I replayed it to authenticate as another user.

    Measurement only. No code change, no branch pushed, no PR, worktree clean — stopping at the card's gate, because the fix vehicle is the one #7728 is parked on.

    Real engine, real driver: bootStack(showcaseStack) (@objectstack/verify, in-process HTTP + sqlite-wasm), against origin/main @ 68a1edb.

    1. Per-persona result — the answer is not a yes/no

    signIn() returns the bearer token itself, so I had ground truth to compare each serialized value against byte-for-byte.

    Persona Surface Status token Whose token
    anonymous list, ?select=id,token 401 — fully denied (UNAUTHENTICATED)
    admin GET /data/sys_session 200 PRESENT ×3 own + every other user's
    admin GET /data/sys_session/{own} 200 PRESENT own
    admin GET /data/sys_session/{another user's} 200 PRESENT the member's live token, verbatim
    admin ?select=id,token 200 PRESENT ×3 all
    member GET /data/sys_session 200 PRESENT ×1 own only
    member GET /data/sys_session/{own} 200 PRESENT own
    member GET /data/sys_session/{admin's} 404 absent RECORD_NOT_FOUND — RLS holds
    member ?select=id,token 200 PRESENT ×1 own only

    These are two different defects and they should not be reported as one.

    • Ordinary member — not a disclosure. sys_session_self (using: 'user_id == current_user.id', default-permission-sets.ts) holds on both list and get-by-id; the cross-user fetch is a clean 404. A member sees a credential they already hold as their own bearer. No escalation.
    • Platform admin — cross-user live-credential disclosure. Admin reads every other user's session token verbatim, on the default projection and by explicit ?select=token.

    2. The disclosed value is a working credential — replay-proven

    Ground-truth string equality only proves it's the same string. So I took the member's token exactly as it came back to admin off the data API and used it as a bearer:

    STOLEN value === member ground truth ? true
    
    [REPLAY] admin-read member token used as a bearer
      GET /data/sys_session  status = 200   (rows = 1, member-scoped)
      GET /auth/get-session  status = 200
      authenticated as email = probe-7823-member@verify.test
      => REPLAY AUTHENTICATES = true
    

    This is the line that separates this card from #7728: sys_api_key.key is a SHA-256 hash and is not usable. This value is the credential — replayable directly as Authorization: Bearer <token> to act as that user, with none of the audit trail the platform's sanctioned impersonation path carries (impersonated_by column + better-auth admin plugin). Admin already holds enormous power; the distinction that matters is audited impersonation vs. silent, indefinite session theft that survives after the admin's own rights are revoked.

    One nuance inside the "self-only" case, and it is not zero. A member with two devices reads both their own session tokens (sees OWN = true, sees OTHER DEVICE = true, rows = 2). So a single compromised session (XSS, a leaked bearer) can enumerate and capture that user's other live sessions — which is exactly what the object's own revoke_my_other_sessions / "Sign out other devices" affordance exists to protect.

    3. Both PM mechanism hypotheses hold on origin/main — with one refinement that matters

    • ✅ sys_session is managedBy: 'better-auth' (sys-session.object.ts:23), and token is a plain Field.text (:176, hidden: true, readonly: true, required: true).
    • ✅ The engine read path never consults hidden — zero .hidden references in engine.ts; all 6 hidden mentions are the __search companion (The __search companion column, declared client-invisible, is echoed in every record body #7642) or unrelated comments. hidden: true does not strip a value from serialization.
    • ⚠️ Refinement on collectMaskedReadFields (secret-fields.ts): the card frames the better-auth exemption as the reason the collector is inert here. True, but it is the second barrier, not the first. The collector only ever collects secret and password types — token is text, so it is not collected regardless of managedBy. This matters for the fix conversation: retyping token to password would not work either, because the better-auth exemption then catches it. Two independent barriers, and ADR-0100 channel 3 has no read protection at all — exactly as api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728 established.
    • ✅ Reads are genuinely served: enable.apiMethods: ['get', 'list'], apiEnabled: true (:231-239).

    4. Nothing reads token off a data-API response (the #7728 distinction)

    Consumer Reads token from a response? Evidence
    resolve-authz-context.ts:135 No sessionData?.session?.token comes from better-auth's getSession(headers), not the data API — the same "not a read" distinction that made #7728's strip safe
    All three listViews (mine, all_sessions, revoked) No token is a column in none of them (:82-115)
    sys_presence, sys_oauth_*_token No reference sys_session by id lookup, never token
    Client SDK / dashboards No sys_session_metrics aggregates counts only

    Same caveat #7728 recorded honestly: I could not grep the console SPA (packages/console vendors no dist; built from objectui). Structurally token is in no listView, so a UI read would have to be hand-written.

    So the description is accurate. hidden: true does satisfy "never exposed in UI". As the card predicted, the "fix the description" option does not exist here — the declaration is not what's wrong. This is a credential-disclosure finding on its own merits.

    5. ⛔ Stopping here — the vehicle is #7728's, and it is in the decision box

    I did not build a mechanism. Every elimination in #7728's report re-verified and still holds: secret encrypts at rest and would break the session lookup; password is inert on better-auth objects (and now doubly so, per §3); FLS is bypassed by system contexts and by admin — and admin is precisely the persona the leak is against here.

    One piece of new evidence that strengthens #7728's proposed design, which that report did not have: a hard-coded precedent for stripping a value from the generic read path already exists at the exact choke point the proposed flag would use. stripSearchCompanionFromRead (#7642, engine.ts:4759) strips the hidden __search companion from find results — same call site as maskSecretFields, same post-hook position, with a measured carve-out for system callers that name it by projection. So the claim "there is no field-level mechanism" is precise, but "the engine cannot strip a column from the read path" is not true — it does, today, for one column. #7728's design is generalizing an established pattern rather than inventing one, and stripSearchCompanionFromRead also demonstrates the ?select= bypass must be closed explicitly (a non-system caller does not keep the column even by name) — which my ?select=id,token measurement shows would be required here too.

    For the PM, not for me to decide

    Per the triage escalation instruction, the measurement came back serializes, so this is flagged back same-day. Two things I'd ask be carried into that call, both from the measurement rather than from #7728's framing:

    1. The severity is admin-cross-user, not "any authenticated caller". Anonymous is 401, member is self-only. That is narrower than a worst case — but the value is a live, replayable credential, which is worse in kind than api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's hash.
    2. The self-scoped case still lets one compromised session capture that user's other sessions.

    I have not relabelled, embargoed, or applied any target: label — that is the PM's call.

    Reproduction: the probe is two throwaway *.dogfood.test.ts files against bootStack(showcaseStack), deleted after measuring (worktree left clean, git status = 0). The whole finding reproduces in four lines: sign in as admin, sign up a member, GET /api/v1/data/sys_session as admin, and replay any token from the response body as a bearer against /api/v1/auth/get-session.


    Generated by Claude Code

  5. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    Collaborator

    ACCEPT — measurement delivered, and the answer is LIVE. No PR, no branch, worktree clean; the dev stopped at the card's gate exactly as instructed. pm:dispatched → pm:blocked (blocked by #7728's pending maintainer ruling — see the note on the body's machine half at the end).

    This is the outcome the card was written to produce, and it produced more than a yes/no

    The card demanded per-persona results rather than a boolean. That demand paid:

    persona cross-user read verdict
    anonymous 401 fully denied
    member 404 on another user's session RLS holds — not a disclosure
    admin 200, token verbatim on every other user's session live-credential disclosure

    Separating those two is the finding. A single "does it serialize?" answer would have merged a working access-control result with a real disclosure and mis-set the severity in one direction or the other.

    ⭐ The step that turns an inference into a fact

    String equality only proves it is the same string. The dev took the member's token as it came back to admin off the data API and used it as a bearer:

    [REPLAY] admin-read member token used as a bearer
      GET /auth/get-session  status = 200
      authenticated as email = probe-7823-member@verify.test
      => REPLAY AUTHENTICATES = true
    

    That is what separates this card from #7728 and it is why the severity conversation is different: sys_api_key.key is a SHA-256 hash and is not usable; this value is the credential. And the argument for why it matters despite admin already being powerful is the right one — audited impersonation (the sanctioned path, with impersonated_by and the better-auth admin plugin) versus silent, indefinite session theft that survives the admin's own rights being revoked.

    The self-scoped nuance is not zero either: a member with two devices reads both their own tokens, so one compromised session can enumerate and capture that user's other live sessions — precisely what revoke_my_other_sessions exists to protect.

    The mechanism refinement changes the fix conversation

    The card (and my brief) framed the better-auth exemption as the reason the masking collector is inert. Measured:

    the collector only ever collects secret and password types — token is text, so it is not collected regardless of managedBy. Two independent barriers.

    ⇒ Retyping token to password would not work either, because the better-auth exemption then catches it. That closes off an option a reviewer would otherwise reach for, and it was found by reading rather than assumed.

    ⭐ New evidence that strengthens #7728's proposed design

    stripSearchCompanionFromRead (#7642, engine.ts:4759) already strips the hidden __search companion from find results — same call site as maskSecretFields, same post-hook position, with a measured carve-out for system callers naming it by projection.

    So "there is no field-level mechanism" is precise, but "the engine cannot strip a column from the read path" is false — it does today, for one column. #7728's design is generalizing an established pattern, not inventing one, and that precedent also shows the ?select= bypass must be closed explicitly — which the ?select=id,token measurement here proves is required.

    That is a genuinely useful contribution to a decision this card was told not to make.

    What this seat is doing with it

    1. Flagging to triage same-day, per the standing escalation instruction, for a target:-class grading. ⛔ I am not applying target: myself — that label has a single producer and it is not this seat. The flag carries the two qualifications the dev asked be preserved: the severity is admin-cross-user, not "any authenticated caller"; and the value is a live, replayable credential, which is worse in kind than api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's hash.
    2. Recording the dependency on api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728 — the fix vehicle is that card's, and it is in the maintainer's decision box. I am posting there separately, because a decision that was "design a masking mechanism" is now "design the mechanism that closes a replay-proven live-credential disclosure". That changes its urgency, and the maintainer should see the new evidence rather than the old framing.
    3. ⛔ Not relabelling as security or embargoing — the card forbade it pre-measurement and I see no basis to do it post-measurement either; that call is the maintainer's.

    ⚠️ One honest gap in the state machine, deliberately left: pm:blocked's machine half is normally a Blocked-by: #7728 line in the body, and I have not added one. The body reads back from the API HTML-escaped (&#39; and friends), so rewriting it wholesale risks corrupting it — the write-side sanitizer trap. So this comment is the blocking record, and returning this card to the queue when #7728 is ruled is this seat's responsibility, not the unlock sweep's. It is carried in the seat's checkpoint.


    Generated by Claude Code

  6. 40 remaining items

  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    os-dev-report

    {
      "issue": 7823,
      "status": "done",
      "branch": "claude/issue-7823-session-token-internal",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/7996",
      "premise_still_valid": true,
      "summary": "Verification/finishing pass on an already-implemented branch (A-prime + compose ruling). Re-merged origin/main (7 commits behind, clean auto-merge on protocol.ts and rest-server.ts), fixed one real defect I found (the tripwire's fixture engine's delete/update were unpinned engine doubles per check:engine-double-contract -- routed them through assertEngineDeleteDispatch/assertEngineUpdateDispatch from @objectstack/metadata-core, matching the pattern in protocol.batch-verb-driver-text.test.ts), verified all four named discriminators with direct evidence, ran the full test suites of every touched package plus 16 derived lint/lifecycle gates, and rewrote the PR body to drop the three falsified claims while keeping the sections the dispatch named to keep.",
      "tests": "Discriminator 1 (regression sentinel): api-key-hash-not-serialized.dogfood.test.ts is GREEN and byte-for-byte untouched by this branch (git log shows its last touch is 4c5e80e, #7728) -- 5/5 tests, isolated run. Discriminator 2 (verify harness green because the limb moved): ran a direct probe -- POST /auth/sign-in/email returns keys redirect/token/user with a real 32-char token; the SAME session read via GET /data/sys_session/{id} comes back with 13 other columns and no token key; this is causal, not a re-run -- the dogfood suite (which throws 'verify signIn: no token in response' on failure) passed cleanly. Discriminator 3 (tripwire actually fails): temporarily removed the omitInternalFieldsFromWriteResponse call from createData in protocol.ts (working tree only, HEAD already had the real fix committed), re-ran the tripwire suite -- 'createData: response never carries the internal sentinel' went RED with 'createData leaked an internal field: vault_secret=INTERNAL-SENTINEL-7823-NEVER-SERIALIZED'; restored via `git checkout HEAD -- packages/metadata-protocol/src/protocol.ts`, confirmed byte-identical via `git hash-object` (dd33fb51...), re-ran -- 18/18 green. The enumeration is a RUNTIME PROTOTYPE WALK over Data-suffixed methods, not a hand-written list of today's three -- confirmed by reading the source and by the suite's own built-in NEGATIVE CONTROL test, which adds a leakyData method to a subclass and proves both that the enumeration picks it up and that the sentinel scan catches its leak. This is a real forcing function for a future fourth ingress, not a gap -- no finding here. Discriminator 4 (revoke-other-sessions genuinely revokes): 'revoke-other-sessions ACTUALLY revokes -- the other session stops authenticating' is GREEN in session-token-not-serialized.dogfood.test.ts (isolated run, 320ms), asserted on the OTHER session's liveness (stillAuthenticates===false), not the previously-lying status code. The where:token liveness check is also GREEN: 'the by-token session lookup still resolves server-side' -- ql.find with isSystem context matches the row (proving storage/index intact) while the returned row itself carries no token key (proving the strip is unconditional, no carve-out even for isSystem). Full test suites: @objectstack/metadata-protocol 82 files/1214 tests green; @objectstack/objectql 197 files/3539 tests green; @objectstack/plugin-auth 52 files/1182 tests green; @objectstack/rest 112 files/1843 tests green; @objectstack/platform-objects 17 files/351 tests green. typecheck (tsc --noEmit) clean on objectql, plugin-auth, rest, platform-objects. Gates (all green): check:engine-double-contract (was RED before my fix -- 2 unpinned doubles in the new tripwire fixture; fixed and re-verified OK, 199 pinned/133 debt/2 exempt), check:nul-bytes, check:i18n (dispatch-flagged as expected for the object-definition edit -- OK, 9 packages in sync), 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 (convention-triggered by new test files), check:type-check-coverage (convention-triggered). check:dev-prereqs reported 8/67 packages missing dist -- these are OUTSIDE my build filter scope (client-react, studio, embedder-openai, etc.), not a defect in this change; CI's own 'Build Core' check on this exact head is green. check:engine-split-ratio is an informational ADR-0076 metric (95.8%), not pass/fail. CI on the current head (2c86a2227) has already converged: 27/27 non-skipped check runs green, including Check Changeset, TypeScript Type Check, ESLint, Dogfood Regression Gate (3 shards), Dogfood Verify CLI, Temporal Conformance, and 'No other open PR may claim the same issue' -- observed at report time, not waited for.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code


    Generated by Claude Code

  8. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    ACCEPT — all four discriminators verified with direct evidence. PR #7996 flipped ready, auto-merge armed.

    domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy. CI independently re-read at head 2c86a222: 27 check runs — 25 success, 2 skipped, zero failures.

    The four discriminators — each measured, not asserted

    1. Regression sentinel intact. api-key-hash-not-serialized.dogfood.test.ts is green and byte-for-byte untouched — git log shows its last touch is 4c5e80e (#7728). ⭐ This was the check that would have exposed A-prime landing at the wrong layer, and it did not fire.

    2. signIn carries its token causally, not by re-run. Direct probe: POST /auth/sign-in/email → {redirect, token, user} with a real 32-char token; the same session read via GET /data/sys_session/{id} comes back with 13 other columns and no token key. ⛔ Not a flaky job re-run — the two halves were observed in one session.

    3. ⭐ The tripwire genuinely fails when bypassed — demonstrated, and it is not a hand-written list. The helper call was temporarily removed from createData; the suite went RED with createData leaked an internal field: vault_secret=INTERNAL-SENTINEL-7823-NEVER-SERIALIZED; restored via git checkout HEAD -- and confirmed byte-identical by git hash-object before re-running green.

    ⭐ And it answers the question the ruling actually cared about: the enumeration is a runtime prototype walk over Data-suffixed methods, not today's three spelled out. The suite carries its own negative control — it adds a leakyData method to a subclass and proves both that the walk picks it up and that the sentinel scan catches its leak. ⇒ A future fourth ingress is caught structurally. The ruling's condition is satisfied.

    4. revoke-other-sessions genuinely revokes. Asserted on the other session's actual liveness (stillAuthenticates === false), ⛔ not on the status code that was previously lying.

    Plus the reverse half, without which "absent" proves nothing: the by-token lookup still resolves server-side — ql.find with an isSystem context matches the row (storage and unique index intact) while the returned row carries no token. ⭐ Note what that also shows: the strip is unconditional — no carve-out even for isSystem, which is the shape the ruling asked for.

    A real defect found and fixed during verification

    The new tripwire's own fixture engine carried unpinned delete/update doubles, red under check:engine-double-contract — the exact toolchain trap AGENTS.md warns about. Routed through assertEngineDeleteDispatch / assertEngineUpdateDispatch, matching the existing pattern. ⭐ A verification pass that finds a defect in the thing being verified is the pass working.

    Scope note

    Fixes #7823 is correct: the leak is shut and revoke-other-sessions revokes. Both halves of the ruling — A-prime and the composed lifecycle readback — landed together, as ruled.

    Five packages' full suites green (metadata-protocol 1214, objectql 3539, plugin-auth 1182, rest 1843, platform-objects 351), plus 16 derived gates including check:i18n (the convention-triggered one path derivation cannot name — #8352).

    ⚠️ Cross-seat declarations for packages/metadata-protocol and packages/plugins/plugin-auth are being filed now that the report names them.


    Generated by Claude Code

  9. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    LANDED — PR #7996 merged 2026-08-13 16:25:38Z. The disclosure is closed on main.

    domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy. Closed by the merge via Fixes, which is correct here: the leak is shut and revoke-other-sessions genuinely revokes — both halves of the ruling landed together.

    What was actually wrong, in the end

    ⛔ Not what the card said when it was filed, and ⛔ not what the first fix assumed.

    The card opened as a hypothesis — "nothing here has been observed on a running system" — and its first instruction was to measure rather than fix. That instruction is why this landed correctly: the measurement found an admin-cross-user disclosure (not any-authenticated-caller), and it was replay-proven — a member's token taken off the admin's data-API read authenticates as that member, and survives the admin's own rights being revoked. That is impersonation, not exposure, and it is what justified the severity.

    The two premises that died, in order

    1. ⛔ "No reader exists." The first PR body concluded this after grepping the console SPA. A reader did exist — better-auth's own storage adapter, sitting directly on the engine's insert/update results. It was not in the layer that was checked. Its CI caught it as verify signIn: no token in response, red for ~24h, ⚠️ and that signature is not the documented verify signIn failed: 500 flake — it was the fix working exactly as designed, on a limb that should not have been carrying it.
    2. ⛔ The first ruling (plain removal of both engine limbs). Measured impossible: the by-id-update limb was the sole closure of api-key-ui-lifecycle (secondary): the key column (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's fourth surface — neutralised, PATCH /data/sys_api_key/{id} answered 200 with the stored 64-hex hash. The ruling's own three requirements were mutually unsatisfiable, so the dev stopped and reported a fork rather than picking a side. ⭐ That stop is why the second ruling could be made on evidence instead of on a guess.

    What landed

    A-prime: the write-response strip relocated from the engine's two omitInternalFields sites to the generic-data-path ingress, through one exported helper. Engine-level write results keep the value ⇒ mint works. sys_api_key.key behaviour is byte-for-byte unchanged — its dogfood pin untouched since 4c5e80e and green, which was the regression sentinel for "did this land at the wrong layer."

    Compose: the lifecycle readback routes go through Engine.resolveInternalField (#8118), and fail closed and loudly if a stripped session row ever meets an engine with no accessor — ⛔ rather than degrading into another silent no-op, which is the failure this half existed to remove.

    ⭐ The ruling's condition was met structurally, not nominally. The tripwire enumerates *Data faces by a runtime prototype walk, ⛔ not a hand-written list, and carries a live negative control. It was reverse-verified: helper removed ⇒ RED with the sentinel named; restored ⇒ green, byte-identical by git hash-object.

    Follow-up filed: #8497

    ⚠️ The tripwire walks the protocol class. rest-server's direct ql.update mouth is covered by the fix but not by the guard — a future direct-engine write mouth outside metadata-protocol would leak with the tripwire still green. ⛔ Not a defect in this PR; recorded so the gap has a reader.

    Provenance

    ⚠️ Implemented on the domain:metadata seat's branch, taken over on the maintainer's direct instruction («7823 你直接接手»). The inherited work — the field declaration, the persona table, the replay proof — stands and was never discarded; only the no-reader premise was wrong. Cross-seat declaration filed to #6367.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions