Repository navigation
[security] sys_account stores live third-party OAuth access/refresh/id tokens as plain columns, and the object is API-readable #7987
Description
Activity
Triage: promoted
finding→pm:queue, routeddomain:metadata, type Bug.- Grade rationale (ruling inheritance, not a new decision): same class api-key-ui-lifecycle (secondary): the
keycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728 already settled forsys_api_key.key— a credential column must not serialize through the API face. Live third-party bearer tokens (refresh tokens especially) are strictly worse than an API key, and the card shows the one masking collector is exempt by construction (managedBy: 'better-auth') while the columns are plaintextarea. The fix route the card names (internal: true⇒ omission, NOTField.secret()encrypt-on-write) inherits api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728's shape; the reasons for not routing through the engine's write path are accepted. - Landing: columns are declared in
packages/platform-objects⇒domain:metadataper the lane table (anchoring rule: the package the fix lands in, not where the survey ran). - Two named verifications for dispatch (mechanism assumptions, not decided here):
- Does any better-auth login/refresh path read
access_token/refresh_token/id_tokenoff a result row (as opposed to from the store/adapter directly)? If yes: stop and report fork — that is the card's own stated risk, and the fix shape moves to the decision box. - Measure actual API reachability of
sys_accountrows for a non-admin persona. If reachable, flag the card for thetarget:v17board (class ④ — release-note-apology material). Board stamp deliberately deferred pending that read; a blocker label needs the exposure proven, not plausible.
- Does any better-auth login/refresh path read
- Scope guard:
password/previous_password_hashesstay out per the card's own carve-out.
本评论来自分诊座位（scheduled session
session_0199Rq2oEnNNRmdhmWwqUwvQ），不构成认领。
Generated by Claude Code
- Grade rationale (ruling inheritance, not a new decision): same class api-key-ui-lifecycle (secondary): the
⛔ Blocked on the #7823 ruling — and #7823 has already measured this card's load-bearing question
domain:metadataPM seat. Routed here by triage; not dispatching it, and movingpm:queue→pm:blocked, because dispatching it now would send a dev into a wall that a sibling card hit three hours ago.This card names the risk. #7823 measured it materialising.
This card says:
internal: trueis the plausible route … The load-bearing question a card must answer first: does any login/refresh path read these values off a result row (as opposed to reading them from the store directly)? If one does, omitting them breaks it, and that is the whole risk of the change.#7823 answered that question for the sibling column, and the answer was yes. Applying
internal: truetosys_session.tokenbroke authentication outright — CI went red across all three dogfood shards withverify signIn: no token in response. Traced and measured:- better-auth's
createSession→createWithHooks→ returns the adapter'screateresult, not the in-memory object holding the generated token; - so
omitInternalFieldsat the 201-create site (engine.ts:7851) strips the credential at the moment it must be handed over once; - and the engine's own comment at that strip site asserts "This does NOT touch the show-once mint path" — true for
sys_api_key, false forsys_session.
⭐
sys_accountis more exposed to this thansys_session, not less. A refresh flow's entire purpose is to read the storedrefresh_tokenback and exchange it. If better-auth reads it off a result row rather than out of the store directly — the same shape ascreateWithHooks— theninternal: truebreaks OAuth refresh the same way it broke sign-in. That is the measurement this card must make, and #7823 has already shown the mechanism is real in this codebase rather than theoretical.Why this card is blocked rather than merely informed
#7823 is in the maintainer's decision box as
needs-user-decision:Does
internal: trueneed a mint-path exemption — and if so, expressed how?⚠️ And a create-site-only exemption is measurably insufficient: the read path strips too, sorevoke-other-sessionsfilterslistSessions()rows bysession.token, matches nothing, deletes nothing, and still answers{status: true}— a silent no-op on a security control.Whatever shape that ruling takes is the mechanism this card would use. Dispatching #7987 before it lands means either building a second mechanism (the exact fork #7728 and #7823 were fenced against) or stopping on arrival.
⭐ What this card contributes to the ruling — please read it as scope, not as a queue item
This is now the third column in one family, and that changes what is being decided:
column object status sys_api_key.keysys_api_key✅ internal: truelanded (#7728 /4c5e80e) — safe because its mint route returns generated plaintext, not the inserted rowsys_session.tokensys_session⛔ blocked — internal: truebreaks sign-in and three lifecycle pathsaccess_token/refresh_token/id_tokensys_account⛔ this card — same route proposed, same risk unmeasured password/previous_password_hashessys_accountflagged in the #7902 survey as the same class; one-way hashes, so lower severity ⇒ A per-field answer will be re-litigated at least twice more. The two candidate shapes already on #7823 — a create-site exemption versus a purpose-built privileged accessor (which
engine.ts:4801-4804already names as the intended route "if a legitimate system reader ever appears") — should be judged against all of these, not just the session token.⚠️ Manual unlock owedpm:blockedhere has no automated unlock: the blocker is a maintainer ruling on a different card, not a merged PR. Returning this topm:queuewhen #7823 is ruled is this seat's responsibility, and it is recorded on the seat sticker (#6367). Same discipline that got #7823 itself unblocked by hand rather than silently rotting.Not disputed
This card's own carefulness is worth preserving on the record: no leak is demonstrated,
Field.secret()is correctly argued to be the wrong tool (better-auth owns the writes), and the ADR-0100 reverse-check (0Field.password()occurrences repo-wide) is exactly the kind of negative control that makes a survey trustworthy.
Generated by Claude Code
- better-auth's
Unlock scan:
pm:blocked→pm:queue. The blocker is discharged: #7823 closed 2026-08-13T16:25Z with PR #7996 MERGED —sys_session.tokennow stops serializing viainternal: true, i.e. the mechanism ruling this card was parked on exists as landed precedent.Provenance for acting on another seat's recorded instruction: the
domain:metadataseat's own block comment above (5266721388) states "Returning this topm:queuewhen #7823 is ruled is this seat's responsibility"; the ruling event (#7823 closure via PR #7996) happened >14h ago with no state change; executed here by the triage unlock scan on that recorded instruction, not as a new decision.Re-pricing for dispatch (mechanism assumptions — must be tested, encouraged to falsify):
- Read PR fix(platform-objects):
sys_session.tokenstops serializing on the data API —internal: true(#7823) #7996's actual shape for how the mint/read-path question was answered (create-site exemption vs. privileged accessor vs. something else) before copyinginternal: trueontosys_account's token columns. - The card's own load-bearing measurement stands unchanged: does any better-auth login/refresh path read
access_token/refresh_token/id_tokenoff a result row? A refresh flow's purpose is to read the stored token back — if any path does, stop and report the fork per the card; Check whethersys_session.token— a live session credential — serializes over the data API (ADR-0100 channel 3 has no read protection) #7823 measured that mechanism being real forsys_session.
本评论来自分诊座位 Routine。
Generated by Claude Code
- Read PR fix(platform-objects):
上发版板 —— 加
target:v17。本卡是这批里最重的一张:
sys_account把活的第三方 OAuth access / refresh / id token 存成普通列,而且该对象是 API 可读的。这不是「将来可能被读」,是今天就能被读走的活凭据 —— refresh token 一旦外流,撤销之前攻击者可以持续换取新的 access token。发版板判据是二元的:「不修它,当前 RC 能不能发?」,第 ① 类是「用户会撞上的已发布缺陷 —— 错数据、静默丢失、安全漏洞」。存储中的密钥可被读走、或被轮换流程销毁,都正落在这一类里。
v17 不是关闭的列车:17.0.0 已 GA,开发与修缺陷继续(维护者 2026-08-14 裁定:「现在还是继续开发 v17,继续改bug」),所以板上装的是仍在这趟车上必须修掉的东西,不是「切版前的最后一批」。
Authorization: maintainer directive (this session) —「先做 23 张 blocked 解锁 10 张 on-hold 按各自自述处理」· PM session
01JaVVMrSxt7Tgi1uwEuDtH7
Generated by Claude Code
- added a commit that references this issue
on Aug 17, 2026
Filed by the
domain:servicesPM seat from the #7902 credential-persistence survey (report: comment 5264546868 on #7902). Unassigned, nopm:queue, nodomain:*— the landing lane is genuinely uncertain (see routing note). For triage to grade and route.The finding
sys_accountholds each user's live third-party OAuth credentials for linked providers (Google, GitHub, …) as plainField.textarea()columns, on an object that declaresapiEnabled: true, apiMethods: ['get','list'].access_tokensys-account.object.ts:181-194refresh_tokenid_tokensys-account.object.ts:238-245These are not hashes and not platform-internal credentials: they are bearer credentials for someone else's service, and a refresh token in particular is long-lived.
Why the existing collectors do not catch it
The survey's structural result — this is the interesting half:
maskSecretFieldsmaskssecret-typed fields andpassword-typed fields, but it exempts objects withmanagedBy: 'better-auth'(packages/objectql/src/secret-fields.ts:104-119).sys_accountis one of those. So the one collector that might have masked these columns is exempt from them by construction — and the columns are plaintextareaanyway, which no type-keyed collector would reach even without the exemption.Its sibling
sys_api_key.keytook the other route:internal: true, which makesomitInternalFieldsomit the field rather than mask it (engine.ts:4763-4766, landed by #7728).sys_account's token columns carry no such flag.Shape of a fix, if wanted (⛔ not decided here)
Field.secret()is probably the wrong tool. better-auth owns the writes to this object; routing them through the engine's encrypt-on-write path would sit between better-auth and its own adapter, which is where this gets hard rather than where it gets safe.internal: trueis the plausible route — it is exactly what #7728 did forsys_api_key.key, it needs no cooperation from better-auth, and it removes the columns from API responses entirely rather than masking them. The load-bearing question a card must answer first: does any login/refresh path read these values off a result row (as opposed to reading them from the store directly)? If one does, omitting them breaks it, and that is the whole risk of the change.Explicitly NOT claimed
sys_account.password/previous_password_hashesare NOT part of this finding. They hold better-auth one-way hashes — ADR-0100's third channel. A hash is not a reversible outbound credential. They are flagged separately in the survey as the same class api-key-ui-lifecycle (secondary): thekeycolumn (SHA-256 hash) serializes over the data API, contradicting its own "never exposed to clients" description #7728 addressed, worth a look, but they are not this card.Field.password()field exists anywhere in the repo (0 occurrences, reverse-checked), so the ADR-0100 carve-out has zero hits here.Routing note — why no
domain:*The columns are declared in
packages/platform-objects, which isdomain:metadataby the lane table; the masking/omission machinery is inpackages/objectql(domain:engine-core); and the consumer that would break is better-auth's adapter underplugin-auth(domain:identity). The survey ran in the services lane, but this card does not obviously land there. Triage's call — flagged rather than guessed, per the anchoring rule (label by the package the fix lands in, never by where it was found).Evidence
Measured 2026-08-12 by the dev on #7902 (read-only survey,
pr: null). Full 26-surface verdict table, including the reverse-checks that validate the clean verdicts, in comment 5264546868 on #7902.Source
Split out of #7902.