|
| 1 | +--- |
| 2 | +"@objectstack/platform-objects": patch |
| 3 | +"@objectstack/plugin-auth": patch |
| 4 | +--- |
| 5 | + |
| 6 | +fix(security): `sys_account.password` and `previous_password_hashes` stop serializing on the data API — `internal: true`, with the raw-engine readers converted to the privileged accessor (#8676) |
| 7 | + |
| 8 | +<!-- adr-0087: not-required (no-migration-prescription) Two field-level flags |
| 9 | +added to one existing declaration, plus one new export on a plugin-internal |
| 10 | +helper module (`internal-field-readback.ts`, not an exported surface of the |
| 11 | +package). Nothing authorable is renamed, retired or tombstoned, so there is no |
| 12 | +conversion to register. The behavioural change is that two columns holding |
| 13 | +one-way password hashes stop being returned on the generic data path, while the |
| 14 | +ADR-0069 D1 reuse ring and better-auth's sign-in verifier keep reading them |
| 15 | +through the engine's privileged accessor. --> |
| 16 | + |
| 17 | +`sys_account.password` (the credential hash) and `previous_password_hashes` (the |
| 18 | +ADR-0069 D1 reuse-prevention ring) serialized on `/api/v1/data/sys_account`, |
| 19 | +which declares `apiEnabled: true, apiMethods: ['get','list']` — to an **admin |
| 20 | +for every user's row**, and to a **member for their own** (the |
| 21 | +`sys_account_self` RLS policy grants `select` on `user_id == current_user.id`). |
| 22 | + |
| 23 | +These are one-way hashes, not reversible outbound credentials — which is why |
| 24 | +#7987 correctly refused to bundle them with the OAuth tokens. But a served |
| 25 | +password hash is an offline-cracking target, and `previous_password_hashes` |
| 26 | +multiplies it by the history ring while its own declaration says it is *never |
| 27 | +exposed in UI*. This is the disposition #7728 already reached for |
| 28 | +`sys_api_key.key`, which was **also** a stored hash and was still ruled unfit to |
| 29 | +serialize through the API face. |
| 30 | + |
| 31 | +Neither credential collector could reach them: `collectMaskedReadFields` keys on |
| 32 | +the field **TYPE** (`secret` / `password`) *and* exempts objects declaring |
| 33 | +`managedBy: 'better-auth'`, which this object is — while these columns are |
| 34 | +`text` / `textarea`. Two independent barriers, both missing. |
| 35 | + |
| 36 | +**The fix is two declarations plus two recovery seams**, and the second seam is |
| 37 | +the part a bare flag would have missed: |
| 38 | + |
| 39 | +- both columns are declared `internal: true` — the opt-in, type-independent flag |
| 40 | + from #7728 meaning *the declared value is never returned on the generic data |
| 41 | + path*. Storage, filtering and indexing are untouched: the strip runs on rows |
| 42 | + the driver has already produced. |
| 43 | +- **better-auth's adapter readers** are recovered by the existing per-object |
| 44 | + readback table, widened with `password`: the sign-in verifier compares against |
| 45 | + the hash on the row `internalAdapter.findCredentialAccount(userId)` returns, |
| 46 | + so the strip alone would break password sign-in for every user. |
| 47 | +- **plugin-auth's own RAW-engine readers** are recovered by a new seam in the |
| 48 | + same module, `recoverInternalFieldsForSystemRead`. This is the half that makes |
| 49 | + the flag safe: the readback table is imported by exactly one file |
| 50 | + (better-auth's storage adapter), so it cannot reach a caller that reads the |
| 51 | + engine directly — and the engine's strip has **no `isSystem` carve-out** by |
| 52 | + #7728's design. Measured against a real ObjectQL engine: the reuse ring's |
| 53 | + `findOne` returns `{"id":"a1"}` for a query that names both columns in an |
| 54 | + explicit projection under `context: { isSystem: true }`. |
| 55 | + |
| 56 | + Left unrecovered, `assertPasswordNotReused` would become a **silent no-op** — |
| 57 | + its comparison list empties, the loop never runs, `PASSWORD_REUSE` is never |
| 58 | + thrown, and its own `catch { return undefined }` means nothing announces it. |
| 59 | + The ADR-0069 D1 control would report success while accepting every reused |
| 60 | + password. Its unit tests would have stayed green throughout, because they use |
| 61 | + fake engines that never apply the strip. |
| 62 | + |
| 63 | +**No ADR-0100 guard change, and none was needed.** `Engine.resolveInternalField` |
| 64 | +has exactly one predicate — `internal === true` — so flagging the columns makes |
| 65 | +them legitimately dereferenceable through the privileged accessor. The ADR-0100 |
| 66 | +sentence in its refusal message is prose explaining why a *non-flagged* field has |
| 67 | +other channels, not a second predicate; the guard stays exactly as selective as |
| 68 | +it was, and a non-flagged column on the same object is still refused with |
| 69 | +`INVALID_FIELD` / 400. |
| 70 | + |
| 71 | +Regression proof drives both directions on a real booted stack: both columns are |
| 72 | +absent for both personas — including a caller who spells them out in `?select=` — |
| 73 | +while the values remain on disk and reachable through the privileged accessor, |
| 74 | +password sign-in still works, and the reuse ring still grows across a password |
| 75 | +change on every transport lane. |
0 commit comments