From a82ce52980176ee4a7fd67e5e2063d653011d3cc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 06:11:55 +0000 Subject: [PATCH 1/3] feat(objectql): mask `password` fields on the generic read path (#2036) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `password`-typed field on a non-auth object round-tripped plaintext through the generic CRUD engine — neither hashed nor masked the way `secret` is. Someone modeling a `password` field on a custom object reasonably expects credential-grade handling; they silently got plaintext storage and plaintext reads. Mask `password` to SECRET_MASK on the generic read path (find/findOne, and $expand which re-enters find), mirroring `secret`. Unlike `secret`, a password value stays plaintext at rest — no encryption, no sys_secret row, no CryptoProvider required. An echoed mask is dropped on write so a form round-trip does not overwrite the stored value. Objects marked `managedBy: 'better-auth'` are exempt so the auth subsystem's own reads (which go through the engine) still see the stored value; a platform-objects pin asserts no shipped identity object even declares a `password` field today. `ObjectSchema.create()` now warns (non-fatally, deduped per object) when a `password` field is declared on a non-auth object, steering authors to `secret` or the auth subsystem. Decision and rationale recorded in ADR-0100; the aggregate() masking gap (pre-existing for `secret` too) is noted there as a follow-up. - objectql: add collectMaskedReadFields; wire mask/echoed-mask paths - spec: author-time warning in ObjectSchema.create(); correct field.zod docs - tests: password-masking units, warning units, platform-objects pin, dogfood field-zoo f_password upgraded present -> masked - examples: field-zoo label 'Password (one-way hash)' -> '(masked on read)' Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QJrKxe1jXGudFT3nUks1yN --- ...word-field-masking-in-generic-read-path.md | 96 +++++++++++++++++++ .../src/data/objects/field-zoo.object.ts | 2 +- packages/objectql/src/core.ts | 1 + packages/objectql/src/engine.ts | 66 ++++++++----- packages/objectql/src/index.ts | 1 + packages/objectql/src/secret-fields.test.ts | 88 +++++++++++++++++ packages/objectql/src/secret-fields.ts | 44 ++++++++- .../src/platform-objects.test.ts | 21 ++++ .../test/field-zoo-roundtrip.dogfood.test.ts | 9 +- packages/spec/src/data/field.zod.ts | 17 ++-- packages/spec/src/data/object.test.ts | 55 ++++++++++- packages/spec/src/data/object.zod.ts | 45 +++++++++ 12 files changed, 406 insertions(+), 39 deletions(-) create mode 100644 docs/adr/0100-password-field-masking-in-generic-read-path.md diff --git a/docs/adr/0100-password-field-masking-in-generic-read-path.md b/docs/adr/0100-password-field-masking-in-generic-read-path.md new file mode 100644 index 0000000000..c1e1738d18 --- /dev/null +++ b/docs/adr/0100-password-field-masking-in-generic-read-path.md @@ -0,0 +1,96 @@ +# ADR-0100: Mask `password` Fields on the Generic Read Path + +- **Status**: Accepted +- **Date**: 2026-07-18 +- **Issue**: #2036 (found via #2033 / #2025 / #2028 field-type round-trip work) +- **Relates to**: ADR-0077 (authoring-surface boundary), ADR-0078 (no silently + inert metadata), ADR-0069 (enterprise authentication hardening) + +## Context + +A `password`-typed field declared on a **non-auth** object (e.g. +`showcase_field_zoo.f_password`) round-tripped **plaintext** through the generic +CRUD engine: it was stored as-is and read back verbatim over the data API. This +is unlike the `secret` field channel, which encrypts on write into `sys_secret` +and masks to `SECRET_MASK` (`••••••••`) on read, so plaintext never leaves the +engine outside a privileged `resolveSecret` call. + +This is a low-code platform where field types are author-driven (often by an AI). +Someone modeling a `password` field on a custom object reasonably expects +credential-grade handling; today they silently got plaintext storage and +plaintext reads, a runtime/security trap the static gates do not catch. The +real one-way hashing lives entirely in the auth subsystem (better-auth endpoints, +the hashed `sys_account.password` `text` column) and never touches an authored +`password` field. + +The issue framed four options: (1) mask on read like `secret`; (2) hash on write +in the generic path; (3) an author-time guard; (4) document as auth-only. Option +2 is the wrong fit — one-way hashing only makes sense for credential +*verification*, which a non-auth object never does, and doing it in the engine +would stand up a second, unmanaged credential store that violates the +"auth subsystem owns credentials" boundary. Option 4 leaves the silent trap in +place. This ADR adopts **1 + 3**. + +## Decision + +1. **Mask on read, everywhere the generic path returns rows.** A `password` + field on a generic object is masked to `SECRET_MASK` in `find` / `findOne` + (and therefore in `$expand`, which re-enters `find`). The read set is computed + by `collectMaskedReadFields` (`packages/objectql/src/secret-fields.ts`), which + returns every `secret` field plus every `password` field; `maskSecretFields` + in `engine.ts` consumes it. `secret` behavior is unchanged. + +2. **Plaintext at rest, by design.** Unlike `secret`, a `password` value is + **not** encrypted and gets **no** `sys_secret` row — it is stored verbatim. + Masking is a read-path transform only. This keeps the change minimal and + avoids standing up a second credential store; it also means a `password` + field needs no `CryptoProvider` (no fail-closed throw). Authors who need + reversible encryption-at-rest should use `secret`. + +3. **Echoed-mask write guard.** Because a read now returns `SECRET_MASK`, a + client that reads a record and PATCHes it back would otherwise overwrite the + stored value with the literal mask. `encryptSecretFields` drops any masked + field (secret or password) whose incoming value equals `SECRET_MASK`, so an + unchanged round-trip is a no-op. The one accepted cost: the literal string + `••••••••` cannot itself be stored as a password via an echoing client. + +4. **`managedBy: 'better-auth'` exemption.** The auth subsystem reads its + identity rows through the engine's `find`/`findOne` (the better-auth ObjectQL + adapter). Masking a credential column there would break login. Objects marked + `managedBy: 'better-auth'` are therefore exempt from password masking. Today + this is a safety net, not load-bearing: no shipped identity object even + declares a `password`-typed field (`sys_account.password` is a hashed `text` + column), pinned by a platform-objects test so retyping it becomes a + deliberate decision rather than a silent login break. + +5. **Non-fatal author-time warning (ADR-0077/0078).** `ObjectSchema.create()` + emits a `console.warn` (deduped per object name) when a `password` field is + declared on a non-`better-auth` object, steering authors to `Field.secret` + for reversible machine credentials or to the auth subsystem for login + credentials. It is a *warning*, not a build error: `password` now has a + defined generic-path contract (so ADR-0078 does not compel an error), and the + field-zoo example intentionally exercises every field type — a hard error + would be self-inflicted breakage. Raw `.parse()` stays silent, since it also + loads persisted metadata and `create()` is the authoring surface (ADR-0077). + +## Consequences + +- Edit forms that prefill from a read now show the mask for `password` fields — + identical to the existing `secret` UX. Unchanged-value saves are protected by + the echoed-mask guard (Decision 3). +- The generic read path no longer leaks credential plaintext for `password` + fields; the field-zoo HTTP round-trip pins this (`f_password` upgraded from + `present` to `masked`). +- This ADR also retroactively documents the `secret` field channel, resolving + the dangling "ADR (secret field channel)" references in `field.zod.ts` and + `secret-fields.ts`. + +## Non-goals / follow-ups + +- **`aggregate()` masking gap.** `aggregate()` masks neither `secret` nor + `password` — a pre-existing gap for `secret`. Post-hoc masking of aggregate + output would corrupt group keys; the correct fix is to *reject* aggregations + that reference a credential field. Tracked as a separate follow-up issue, not + addressed here. +- **Hashing / verification for authored `password` fields** — explicitly out of + scope (Decision 2). Credential verification belongs to the auth subsystem. diff --git a/examples/app-showcase/src/data/objects/field-zoo.object.ts b/examples/app-showcase/src/data/objects/field-zoo.object.ts index 0461628472..c311521c06 100644 --- a/examples/app-showcase/src/data/objects/field-zoo.object.ts +++ b/examples/app-showcase/src/data/objects/field-zoo.object.ts @@ -35,7 +35,7 @@ export const FieldZoo = ObjectSchema.create({ f_email: Field.email({ label: 'Email', searchable: true }), f_url: Field.url({ label: 'URL' }), f_phone: Field.phone({ label: 'Phone' }), - f_password: Field.password({ label: 'Password (one-way hash)' }), + f_password: Field.password({ label: 'Password (masked on read)' }), f_secret: Field.secret({ label: 'Secret (encrypted at rest)' }), // ── Rich content ───────────────────────────────────────────────────── diff --git a/packages/objectql/src/core.ts b/packages/objectql/src/core.ts index ce8397c5d7..2a0381bd37 100644 --- a/packages/objectql/src/core.ts +++ b/packages/objectql/src/core.ts @@ -75,6 +75,7 @@ export { isSecretRef, parseSecretRef, collectSecretFields, + collectMaskedReadFields, } from './secret-fields.js'; // Utilities diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 229074b808..6a281fe44a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -19,6 +19,7 @@ import { IRealtimeService, RealtimeEventPayload } from '@objectstack/spec/contra import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'; import { collectSecretFields, + collectMaskedReadFields, makeSecretRef, parseSecretRef, isSecretRef, @@ -1363,19 +1364,28 @@ export class ObjectQL implements IDataEngine { } /** - * Encrypt any `secret`-typed fields on `row` in place before it reaches the - * driver. Each plaintext is wrapped by the ICryptoProvider, persisted as a - * `sys_secret` row, and replaced on `row` by an opaque ref. Cleartext never - * reaches the business table. + * Normalize credential fields on `row` in place before it reaches the driver. + * + * Two channels share the read mask ({@link SECRET_MASK}) but differ on write + * (see ADR-0100): + * + * - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider, + * persisted as a `sys_secret` row, and replaced on `row` by an opaque ref. + * Cleartext never reaches the business table. + * - **`password`** (generic, non-`better-auth`) — plaintext at rest: stored + * verbatim, no encryption and no `sys_secret` row. Only the echoed-mask drop + * below applies to it. * * Rules: - * - No secret fields on the object ⇒ no-op (fast path, no crypto cost). - * - `null`/`undefined` value ⇒ left as-is (clears the secret). - * - Value already a ref (re-save of an unchanged ref) ⇒ left as-is. - * - Value equal to the read mask ⇒ dropped, so a form round-trip that - * echoes the mask does not overwrite the stored secret. - * - **Fail-closed:** any other value with no CryptoProvider registered, or - * no reachable `sys_secret` store, THROWS — never persists cleartext. + * - Any masked field (secret or password) whose value equals the read mask ⇒ + * the key is dropped, so a form round-trip that echoes the mask does not + * overwrite the stored value. + * - No secret fields on the object ⇒ no further work (fast path, no crypto). + * - `null`/`undefined` secret value ⇒ left as-is (clears the secret). + * - Secret value already a ref (re-save of an unchanged ref) ⇒ left as-is. + * - **Fail-closed:** any other secret value with no CryptoProvider registered, + * or no reachable `sys_secret` store, THROWS — never persists cleartext. + * (A `password` field needs no CryptoProvider — it is stored as-is.) */ private async encryptSecretFields( object: string, @@ -1385,6 +1395,17 @@ export class ObjectQL implements IDataEngine { ): Promise { if (!row || typeof row !== 'object') return; const schema = this._registry.getObject(object); + + // Echoed-mask drop for every field the read path masks (secret + generic + // password). The read path returns SECRET_MASK; a client that PATCHes it + // back means "unchanged", so drop the key rather than persist the literal + // mask. Doing this up front keeps the better-auth exemption in one place + // (collectMaskedReadFields) and covers objects that have a password field + // but no secret field. (#2036, ADR-0100) + for (const field of collectMaskedReadFields(schema)) { + if (field in row && row[field] === SECRET_MASK) delete row[field]; + } + const secretFields = collectSecretFields(schema); if (secretFields.length === 0) return; @@ -1394,12 +1415,6 @@ export class ObjectQL implements IDataEngine { if (value === null || typeof value === 'undefined') continue; // clear if (isSecretRef(value)) continue; // already encrypted ref - if (value === SECRET_MASK) { - // The read path masks secrets to SECRET_MASK; a form that echoes it - // back means "unchanged". Drop the key so the stored secret survives. - delete row[field]; - continue; - } if (!this.cryptoProvider) { throw new Error( @@ -1446,20 +1461,23 @@ export class ObjectQL implements IDataEngine { } /** - * Mask `secret`-typed fields on read so plaintext never leaves the engine - * through the normal query path. A set secret becomes {@link SECRET_MASK}; - * an unset one stays `null`. Privileged callers that genuinely need the - * plaintext use {@link resolveSecret} against the stored ref. + * Mask credential fields on read so plaintext never leaves the engine through + * the normal query path. Covers `secret` fields (always) and `password` fields + * on generic, non-`better-auth` objects (see {@link collectMaskedReadFields} + * and ADR-0100). A set value becomes {@link SECRET_MASK}; an unset one stays + * `null`. Privileged callers that genuinely need a secret's plaintext use + * {@link resolveSecret} against the stored ref; a `password` field is stored + * as plaintext at rest, so its cleartext is only ever reachable off this path. */ private maskSecretFields(object: string, rows: any): void { if (!rows) return; const schema = this._registry.getObject(object); - const secretFields = collectSecretFields(schema); - if (secretFields.length === 0) return; + const maskedFields = collectMaskedReadFields(schema); + if (maskedFields.length === 0) return; const list = Array.isArray(rows) ? rows : [rows]; for (const row of list) { if (!row || typeof row !== 'object') continue; - for (const field of secretFields) { + for (const field of maskedFields) { if (!(field in row)) continue; row[field] = row[field] == null ? null : SECRET_MASK; } diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 3b75923ea0..5df6fbd5d3 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -102,6 +102,7 @@ export { isSecretRef, parseSecretRef, collectSecretFields, + collectMaskedReadFields, } from './secret-fields.js'; // Export Utilities diff --git a/packages/objectql/src/secret-fields.test.ts b/packages/objectql/src/secret-fields.test.ts index 5519f07502..0609f13030 100644 --- a/packages/objectql/src/secret-fields.test.ts +++ b/packages/objectql/src/secret-fields.test.ts @@ -232,3 +232,91 @@ describe('objectql secret-field channel', () => { } }); }); + +// A generic (non-better-auth) object with a `password` field. Unlike `secret`, +// it is plaintext at rest but masked on read — no crypto involved (ADR-0100). +const deviceObject = { + name: 'device', label: 'Device', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + admin_password: { name: 'admin_password', label: 'Admin Password', type: 'password' as const }, + }, +}; + +// An identity table the auth subsystem owns: `managedBy: 'better-auth'` exempts +// its `password` field from masking so better-auth's own reads see the value. +const authUserObject = { + name: 'authy_user', label: 'Auth User', managedBy: 'better-auth' as const, + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const }, + password: { name: 'password', label: 'Password', type: 'password' as const }, + }, +}; + +async function buildPasswordEngine() { + // Deliberately NO CryptoProvider — a password field must not require one. + const engine = new ObjectQL(); + const { driver, stores } = makeMemoryDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(deviceObject as any); + engine.registry.registerObject(authUserObject as any); + return { engine, stores }; +} + +describe('objectql password-field masking (ADR-0100)', () => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await buildPasswordEngine(); }); + + it('stores plaintext at rest (no crypto), but masks on find / findOne', async () => { + const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'hunter2' }); + + // At rest: the driver row holds the verbatim plaintext, and nothing was + // written to sys_secret (no encryption channel for password). + const stored = ctx.stores.get('device')!.get(created.id) as any; + expect(stored.admin_password).toBe('hunter2'); + expect(ctx.stores.get('sys_secret')?.size ?? 0).toBe(0); + + // On read: the generic path never echoes the plaintext. + const viaFind = (await ctx.engine.find('device', { where: { id: created.id } }))[0] as any; + expect(viaFind.admin_password).toBe(SECRET_MASK); + const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any; + expect(viaOne.admin_password).toBe(SECRET_MASK); + }); + + it('an unset password reads back as null, not the mask', async () => { + const created = await ctx.engine.insert('device', { name: 'router', admin_password: null }); + const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any; + expect(viaOne.admin_password).toBeNull(); + }); + + it('echoing the read mask back does NOT overwrite the stored password', async () => { + const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'keep-me' }); + + // Form round-trip: user renames the device, the masked field echoes back. + await ctx.engine.update('device', { id: created.id, name: 'gateway', admin_password: SECRET_MASK }); + + const stored = ctx.stores.get('device')!.get(created.id) as any; + expect(stored.name).toBe('gateway'); + expect(stored.admin_password).toBe('keep-me'); // unchanged plaintext + }); + + it('updating with a real new value replaces the stored plaintext', async () => { + const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'old-pw' }); + await ctx.engine.update('device', { id: created.id, admin_password: 'new-pw' }); + + const stored = ctx.stores.get('device')!.get(created.id) as any; + expect(stored.admin_password).toBe('new-pw'); + // And a read still masks it. + const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any; + expect(viaOne.admin_password).toBe(SECRET_MASK); + }); + + it('better-auth identity tables are exempt: their password field is NOT masked on read', async () => { + const created = await ctx.engine.insert('authy_user', { password: 'hashed-by-auth' }); + const viaOne = await ctx.engine.findOne('authy_user', { where: { id: created.id } }) as any; + // Masking here would break login — the auth subsystem must read its own value. + expect(viaOne.password).toBe('hashed-by-auth'); + }); +}); diff --git a/packages/objectql/src/secret-fields.ts b/packages/objectql/src/secret-fields.ts index eea68e24ef..4a97461dee 100644 --- a/packages/objectql/src/secret-fields.ts +++ b/packages/objectql/src/secret-fields.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * Secret-field channel — helpers for the `secret` FieldType. + * Credential-field channels — helpers for the `secret` and `password` FieldTypes. * * A `secret` field (DB password, API key, token) is **reversible**: the engine * encrypts it on write via the registered `ICryptoProvider`, persists the @@ -10,8 +10,19 @@ * the Settings subsystem (`sys_setting.value_enc → sys_secret.id`), generalized * to object fields. * - * Contrast with `password` — a one-way hash owned by the auth subsystem, never - * decrypted. The two never share a code path. + * A `password` field on a **generic** (non-`better-auth`) object is **plaintext + * at rest** but **masked on read** — the engine stores the value verbatim (no + * encryption, no `sys_secret` row) yet returns {@link SECRET_MASK} through the + * normal query path, so cleartext never leaves the engine. This closes #2036, + * where a `password` field round-tripped plaintext. See ADR-0100. The two types + * share only the read mask ({@link collectMaskedReadFields}); their write paths + * differ (secret encrypts, password is left untouched). + * + * The auth subsystem's own credentials are a third, separate channel: better-auth + * one-way hashes them into identity tables (`sys_account.password`, a hashed + * `text` column) off the generic CRUD path. Objects it owns carry + * `managedBy: 'better-auth'` and are exempt from password masking so login reads + * still see the stored hash. */ import type { ServiceObject } from '@objectstack/spec/data'; @@ -59,3 +70,30 @@ export function collectSecretFields(schema: ServiceObject | undefined | null): s } return out; } + +/** + * Collect the names of fields that must be masked to {@link SECRET_MASK} on the + * generic read path: every `secret` field, plus every `password` field — the + * latter only when the object is **not** `managedBy: 'better-auth'`. + * + * The better-auth exemption is deliberate: the auth subsystem reads its identity + * rows through the engine's find/findOne, and masking a credential column there + * would break login. Today no identity object even declares a `password`-typed + * field (`sys_account.password` is a hashed `text` column), but the guard keeps + * masking safe if that ever changes. See ADR-0100. + * + * Returns an empty array when the schema has no fields or no maskable fields, so + * callers can fast-path on `length === 0`. + */ +export function collectMaskedReadFields(schema: ServiceObject | undefined | null): string[] { + const fields = (schema as any)?.fields as Record | undefined; + if (!fields) return []; + const isBetterAuth = (schema as any)?.managedBy === 'better-auth'; + const out: string[] = []; + for (const [name, def] of Object.entries(fields)) { + if (!def) continue; + if (def.type === 'secret') out.push(name); + else if (def.type === 'password' && !isBetterAuth) out.push(name); + } + return out; +} diff --git a/packages/platform-objects/src/platform-objects.test.ts b/packages/platform-objects/src/platform-objects.test.ts index a9642e96a7..7af69ef9b6 100644 --- a/packages/platform-objects/src/platform-objects.test.ts +++ b/packages/platform-objects/src/platform-objects.test.ts @@ -69,6 +69,27 @@ describe('@objectstack/platform-objects', () => { expect((object as any).tableName).toBeUndefined(); }); + describe('no shipped object declares a `password`-typed field (ADR-0100)', () => { + // The generic read path masks `password` fields to SECRET_MASK; better-auth + // objects are exempted so login reads see the stored value + // (collectMaskedReadFields). That exemption is a safety net, not a crutch: + // no identity object today even declares a `password`-typed field — + // sys_account.password is a hashed `text` column. If anyone retypes it to + // `password`, masking would apply through the better-auth adapter and silently + // break login; this pin fails first so that change is a deliberate decision. + it.each(systemObjects)('%s has no field of type `password`', (_exportName, object) => { + const fields = ((object as any).fields ?? {}) as Record; + const passwordFields = Object.entries(fields) + .filter(([, def]) => def?.type === 'password') + .map(([name]) => name); + expect(passwordFields).toEqual([]); + }); + + it('sys_account.password is a hashed `text` column, not a `password` field', () => { + expect((SysAccount as any).fields?.password?.type).toBe('text'); + }); + }); + describe('secure-by-default posture (ADR-0066 ④)', () => { // Raw secret / live-credential stores are opted OUT of the wildcard `'*'` // grant: only an explicit per-object grant or the posture-gated superuser diff --git a/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts b/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts index 9059c6d85a..2c4a072afb 100644 --- a/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts +++ b/packages/qa/dogfood/test/field-zoo-roundtrip.dogfood.test.ts @@ -103,11 +103,12 @@ const MATRIX: FieldCase[] = [ { field: 'f_lookup', type: 'lookup', check: { kind: 'equal', write: 'acc_synthetic_0001' } }, { field: 'f_master_detail', type: 'master_detail', check: { kind: 'equal', write: 'proj_synthetic_0001' } }, { field: 'f_tree', type: 'tree', check: { kind: 'equal', write: 'cat_synthetic_0001' } }, - // security — `secret` encrypts on write and masks on read; `password` is a - // credential type the generic CRUD path stores opaquely (auth owns hashing), - // so assert it merely persists rather than asserting a plaintext contract. + // security — both credential types mask on read (plaintext never echoes back + // over HTTP). `secret` is encrypted at rest; `password` on a generic object is + // plaintext at rest but masked to SECRET_MASK on read (ADR-0100 / #2036 — the + // generic path does NOT hash it; auth owns hashing for its identity tables). { field: 'f_secret', type: 'secret', check: { kind: 'masked', write: 'topsecret-value' } }, - { field: 'f_password', type: 'password', check: { kind: 'present', write: 'p@ssw0rd!' } }, + { field: 'f_password', type: 'password', check: { kind: 'masked', write: 'p@ssw0rd!' } }, // NB: f_summary (roll-up) is intentionally absent — it's a computed // aggregate over related records, null on a childless fixture row; its // semantics are covered by dedicated roll-up tests, not value fidelity. diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 8433bdd312..ebe7c5fbf8 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -9,14 +9,19 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; */ import { lazySchema } from '../shared/lazy-schema'; export const FieldType = z.enum([ - // Core Text + // Core Text. 'password' on a generic (non-better-auth) object is plaintext at + // rest but masked to SECRET_MASK on read — the auth subsystem's one-way + // hashing applies only to its own identity tables, never to an authored + // 'password' field. Prefer 'secret' for reversible machine credentials. See + // ADR-0100. 'text', 'textarea', 'email', 'url', 'phone', 'password', // Secret — reversible, encrypted-at-rest value (DB password, API key, token). - // UNLIKE 'password' (a one-way hash owned by the auth subsystem), a 'secret' - // is round-tripped: the engine encrypts it on write via the registered - // ICryptoProvider, stores the ciphertext handle in `sys_secret`, persists only - // an opaque ref on the row, and masks it on read. Fail-closed: no provider ⇒ - // writes throw rather than persist cleartext. See ADR (secret field channel). + // UNLIKE 'password' (masked-on-read but plaintext at rest, or one-way hashed + // inside the auth subsystem), a 'secret' is round-tripped: the engine encrypts + // it on write via the registered ICryptoProvider, stores the ciphertext handle + // in `sys_secret`, persists only an opaque ref on the row, and masks it on + // read. Fail-closed: no provider ⇒ writes throw rather than persist cleartext. + // See ADR-0100. 'secret', // Rich Content 'markdown', 'html', 'richtext', diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 6d1f39806e..a99abb297f 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { ObjectSchema, ObjectCapabilities, IndexSchema, ObjectFieldGroupSchema, ObjectExternalBindingSchema, ObjectAccessConfigSchema, LifecycleSchema, TenancyConfigSchema, resolveCrudAffordances, type ServiceObject } from './object.zod'; describe('ObjectCapabilities', () => { @@ -1408,3 +1408,56 @@ describe('userActions row predicates + resolveCrudAffordances (objectui#2614)', expect(result.success).toBe(false); }); }); + +// ADR-0100: a `password` field on a generic (non-better-auth) object is masked +// on read but plaintext at rest — not hashed. create() warns (non-fatally) to +// steer authors toward `secret` or the auth subsystem. The warning is deduped +// per object name via a module-level Set, so each test uses a unique name. +describe('ObjectSchema.create() password-field author warning (ADR-0100)', () => { + afterEach(() => vi.restoreAllMocks()); + + it('warns once when a password field is declared on a generic object', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + ObjectSchema.create({ + name: 'adr0100_generic_pw', + fields: { admin_password: { type: 'password' } }, + }); + expect(warn).toHaveBeenCalledTimes(1); + const msg = warn.mock.calls[0]?.[0] as string; + expect(msg).toContain('adr0100_generic_pw'); + expect(msg).toContain('admin_password'); + expect(msg).toContain('ADR-0100'); + expect(msg).toContain('secret'); + }); + + it('is deduped: re-creating the same object name warns only once more session-wide', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const make = () => ObjectSchema.create({ + name: 'adr0100_dedup_pw', + fields: { pw: { type: 'password' } }, + }); + make(); + make(); + make(); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('does NOT warn for a password field on a better-auth object', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + ObjectSchema.create({ + name: 'adr0100_auth_pw', + managedBy: 'better-auth', + fields: { password: { type: 'password' } }, + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('does NOT warn for a secret field (its channel is already defined)', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + ObjectSchema.create({ + name: 'adr0100_secret_only', + fields: { api_key: { type: 'secret' } }, + }); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index e68320ea2e..faa329e779 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1179,6 +1179,46 @@ function unknownKeyError(objectName: unknown, unknownKeys: string[], knownKeys: type NoExcessObjectKeys = T & Record>, never>; +/** Object names already warned about a generic `password` field (dedup per name). */ +const warnedPasswordObjects = new Set(); + +/** + * Non-fatal author-time diagnostic (ADR-0100): a `password`-typed field on a + * generic (non-`better-auth`) object gets defined-but-surprising semantics — + * masked-on-read but **plaintext at rest**, with no one-way hashing (that lives + * only in the auth subsystem). Steer authors to `secret` for reversible machine + * credentials, or to the auth subsystem for real login credentials. + * + * A warning, not an error: `password` now has a defined generic-path contract, + * and the field-zoo example intentionally exercises every field type — a hard + * error would be self-inflicted breakage. Deduped per object name so a schema + * imported many times warns once. `managedBy: 'better-auth'` objects are exempt. + */ +function warnGenericPasswordFields( + objectName: unknown, + fields: unknown, + managedBy: unknown, +): void { + if (managedBy === 'better-auth') return; + if (!fields || typeof fields !== 'object') return; + const passwordFields = Object.entries(fields as Record) + .filter(([, def]) => def && def.type === 'password') + .map(([fieldName]) => fieldName); + if (passwordFields.length === 0) return; + const name = typeof objectName === 'string' && objectName.length > 0 ? objectName : ''; + if (warnedPasswordObjects.has(name)) return; + warnedPasswordObjects.add(name); + console.warn( + `ObjectSchema.create('${name}'): field(s) ${passwordFields.map((f) => `\`${f}\``).join(', ')} ` + + "use type 'password' on a non-auth object. The generic CRUD path stores a " + + 'password field as plaintext at rest and masks it to •••••••• on read (ADR-0100) — ' + + 'it is NOT one-way hashed (that is owned by the auth subsystem, for its identity ' + + "tables only). Use `Field.secret(...)` for reversible machine credentials, or model " + + 'login credentials on the auth user object. If this is intended, the masking contract ' + + 'now applies and this warning is safe to ignore.', + ); +} + /** * [ADR-0079] Back-compat alias normalization: an object authored with the * deprecated `displayNameField` key still parses by mapping it onto the @@ -1289,6 +1329,11 @@ export const ObjectSchema = lazySchema(() => { if (unknownKeys.length > 0) { throw unknownKeyError(cfg.name, unknownKeys, knownKeys); } + // ADR-0100: warn (non-fatally) when a `password` field is declared on a + // generic, non-better-auth object — it is masked-on-read but plaintext at + // rest, not hashed. `create()` is the authoring surface (ADR-0077), so the + // steer lives here rather than in raw `.parse()`. + warnGenericPasswordFields(cfg.name, cfg.fields, cfg.managedBy); const withDefaults = { ...cfg, label: cfg.label ?? snakeCaseToLabel(cfg.name as string), From e3767137045f2d65c73a495672686769d5ad7a6d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 06:13:58 +0000 Subject: [PATCH 2/3] docs(field-types): correct `password` description to masked-on-read, not hashed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The field-type gallery said a `password` field is "stored as a one-way hash" — inaccurate for a generic object, where (per ADR-0100 / this PR) the value is plaintext at rest but masked to •••••••• on read. One-way hashing is owned by the auth subsystem and applies only to its identity tables. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QJrKxe1jXGudFT3nUks1yN --- content/docs/data-modeling/field-types.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content/docs/data-modeling/field-types.mdx b/content/docs/data-modeling/field-types.mdx index 41cbc47dfd..5a2164cf10 100644 --- a/content/docs/data-modeling/field-types.mdx +++ b/content/docs/data-modeling/field-types.mdx @@ -76,14 +76,14 @@ Phone number field. ``` ### `password` -Masked password input (stored as a one-way hash, owned by the auth subsystem). For reversible encrypted-at-rest secrets (API keys, tokens), use the `secret` type instead. +Masked password input. On a generic object the value is **plaintext at rest** but **masked to `••••••••` on read** (ADR-0100) — it is *not* one-way hashed. One-way hashing is owned by the auth subsystem and applies only to its identity tables, never to an authored `password` field. For reversible encrypted-at-rest secrets (API keys, tokens, DB passwords), use the `secret` type instead; for login credentials, model them on the auth user object. ```typescript { name: 'password', label: 'Password', type: 'password', required: true } ``` ### `secret` -Reversible, encrypted-at-rest value (DB password, API key, token). Unlike `password` (a one-way hash), a `secret` is encrypted on write via the registered crypto provider, stored as an opaque ref on the row, and masked on read. Fail-closed: with no provider configured, writes throw rather than persist cleartext. +Reversible, encrypted-at-rest value (DB password, API key, token). Unlike `password` (masked on read but plaintext at rest, or one-way hashed inside the auth subsystem), a `secret` is encrypted on write via the registered crypto provider, stored as an opaque ref on the row, and masked on read. Fail-closed: with no provider configured, writes throw rather than persist cleartext. ```typescript { name: 'api_key', label: 'API Key', type: 'secret' } From a6d21c42ce7c0e2ccd1570e1e2bbe941d0a3a8eb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 06:23:26 +0000 Subject: [PATCH 3/3] docs(adr): unify ADR-0100 to cover both credential channels (secret + password) Reframe ADR-0100 as the single record for both credential-bearing field types: it now documents the pre-existing `secret` channel (encrypted at rest, masked on read, fail-closed) alongside the new `password` masking decision (#2036), and gives the long-dangling "secret field channel" comment references a real home. Renamed the file to reflect the unified scope. All code references use the stable ADR-0100 number, unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QJrKxe1jXGudFT3nUks1yN --- docs/adr/0100-credential-field-channels.md | 125 ++++++++++++++++++ ...word-field-masking-in-generic-read-path.md | 96 -------------- 2 files changed, 125 insertions(+), 96 deletions(-) create mode 100644 docs/adr/0100-credential-field-channels.md delete mode 100644 docs/adr/0100-password-field-masking-in-generic-read-path.md diff --git a/docs/adr/0100-credential-field-channels.md b/docs/adr/0100-credential-field-channels.md new file mode 100644 index 0000000000..186c61b9f8 --- /dev/null +++ b/docs/adr/0100-credential-field-channels.md @@ -0,0 +1,125 @@ +# ADR-0100: Credential Field Channels — `secret` (encrypted) and `password` (masked) + +- **Status**: Accepted +- **Date**: 2026-07-18 +- **Issue**: #2036 (found via #2033 / #2025 / #2028 field-type round-trip work) +- **Relates to**: ADR-0077 (authoring-surface boundary), ADR-0078 (no silently + inert metadata), ADR-0069 (enterprise authentication hardening) + +This ADR unifies the two credential-bearing field types under one record. The +`secret` channel had shipped without its own ADR — code comments referenced a +"secret field channel" ADR that never existed; this document is that home. The +`password` masking decision (#2036) is the new material and is documented +alongside it, because the two types share the read mask and only make sense when +contrasted. + +## Context + +ObjectStack has three places a credential can live, and they must not be +confused: + +1. **`secret`** — a reversible machine credential (DB password, API key, token) + authored on any object. Already implemented: encrypted at rest, masked on + read, decryptable only through a privileged path. +2. **`password`** — a field type authors reach for on custom objects. Its + *intended* association (one-way hashing) belongs to the auth subsystem, but a + `password` field on a **non-auth** object never touched that subsystem. +3. **Auth-subsystem credentials** — better-auth's identity tables + (`sys_account.password`, a hashed `text` column), one-way hashed off the + generic CRUD path entirely. + +The bug (#2036): a `password`-typed field on a non-auth object (e.g. +`showcase_field_zoo.f_password`) round-tripped **plaintext** through the generic +CRUD engine — neither hashed nor masked. This is a low-code platform where field +types are author-driven (often by an AI); someone modeling a `password` field +reasonably expects credential-grade handling and silently got plaintext storage +and plaintext reads, a runtime/security trap the static gates do not catch. + +The issue framed four options: (1) mask on read like `secret`; (2) hash on write +in the generic path; (3) an author-time guard; (4) document as auth-only. Option +2 is the wrong fit — one-way hashing only makes sense for credential +*verification*, which a non-auth object never does, and doing it in the engine +would stand up a second, unmanaged credential store that violates the +"auth subsystem owns credentials" boundary. Option 4 leaves the silent trap in +place. This ADR adopts **1 + 3** for `password`, and records the pre-existing +`secret` channel it now sits beside. + +## Decision + +### A. The `secret` channel (records existing behavior) + +A `secret` field is **reversible and encrypted at rest**: + +- **Write** — the plaintext is wrapped by the registered `ICryptoProvider`, + persisted as a `sys_secret` row, and replaced on the business row by an opaque + `secret:` ref. Cleartext never reaches the business table. +- **Read** — the ref is masked to `SECRET_MASK` (`••••••••`) on the generic path + (`find`/`findOne`/`$expand`); an unset secret reads back `null`. +- **Fail-closed** — writing a secret value with no `CryptoProvider` registered, + or no reachable `sys_secret` store, THROWS rather than persist cleartext. +- **Privileged read** — `resolveSecret(ref)` is the only sanctioned way back to + plaintext (e.g. a datasource connection binder); it is never on the generic + read path. + +### B. The `password` channel (new, #2036) + +A `password` field on a generic (non-`better-auth`) object is **plaintext at +rest but masked on read**: + +1. **Masked on read** — masked to `SECRET_MASK` in `find`/`findOne` (and + `$expand`, which re-enters `find`), exactly like `secret`. +2. **Plaintext at rest, by design** — **not** encrypted, **no** `sys_secret` + row, **no** `CryptoProvider` required. Masking is a read-path transform only. + This keeps the change minimal and avoids a second credential store. Authors + who need reversible encryption-at-rest should use `secret`. +3. **Echoed-mask write guard** — because a read now returns `SECRET_MASK`, a + client that reads a record and PATCHes it back would otherwise overwrite the + stored value with the literal mask. The write path drops any masked field + (secret or password) whose incoming value equals `SECRET_MASK`, so an + unchanged round-trip is a no-op. Accepted cost: the literal string + `••••••••` cannot itself be stored as a password via an echoing client. +4. **`managedBy: 'better-auth'` exemption** — the auth subsystem reads its + identity rows *through* the engine's `find`/`findOne`, so masking a credential + column there would break login. Objects marked `managedBy: 'better-auth'` are + exempt from password masking. Today this is a safety net, not load-bearing: + no shipped identity object even declares a `password`-typed field + (`sys_account.password` is a hashed `text` column), pinned by a + platform-objects test so retyping it becomes a deliberate decision. +5. **Non-fatal author-time warning (ADR-0077/0078)** — `ObjectSchema.create()` + emits a `console.warn` (deduped per object name) when a `password` field is + declared on a non-`better-auth` object, steering authors to `Field.secret` + for reversible machine credentials or to the auth subsystem for login + credentials. It is a *warning*, not a build error: `password` now has a + defined generic-path contract (so ADR-0078 does not compel an error), and the + field-zoo example intentionally exercises every field type — a hard error + would be self-inflicted breakage. Raw `.parse()` stays silent, since it also + loads persisted metadata and `create()` is the authoring surface (ADR-0077). + +### C. Shared mechanism + +Both channels share one read-mask collector — `collectMaskedReadFields` +(`packages/objectql/src/secret-fields.ts`): every `secret` field, plus every +`password` field on a non-`better-auth` object. `maskSecretFields` (read) and the +echoed-mask drop (write) in `engine.ts` both consume it, so the better-auth +exemption lives in exactly one place. `SECRET_MASK` is the single mask constant +for both. + +## Consequences + +- Edit forms that prefill from a read now show the mask for `password` fields — + identical to the existing `secret` UX. Unchanged-value saves are protected by + the echoed-mask guard (B3). +- The generic read path no longer leaks credential plaintext for `password` + fields; the field-zoo HTTP round-trip pins this (`f_password` upgraded from + `present` to `masked`). +- The dangling "secret field channel" references in `field.zod.ts` and + `secret-fields.ts` now resolve to this ADR. + +## Non-goals / follow-ups + +- **`aggregate()` masking gap.** `aggregate()` masks neither `secret` nor + `password` — a pre-existing gap for `secret`. Post-hoc masking of aggregate + output would corrupt group keys; the correct fix is to *reject* aggregations + that reference a credential field. Tracked in #3171, not addressed here. +- **Hashing / verification for authored `password` fields** — explicitly out of + scope (B2). Credential verification belongs to the auth subsystem. diff --git a/docs/adr/0100-password-field-masking-in-generic-read-path.md b/docs/adr/0100-password-field-masking-in-generic-read-path.md deleted file mode 100644 index c1e1738d18..0000000000 --- a/docs/adr/0100-password-field-masking-in-generic-read-path.md +++ /dev/null @@ -1,96 +0,0 @@ -# ADR-0100: Mask `password` Fields on the Generic Read Path - -- **Status**: Accepted -- **Date**: 2026-07-18 -- **Issue**: #2036 (found via #2033 / #2025 / #2028 field-type round-trip work) -- **Relates to**: ADR-0077 (authoring-surface boundary), ADR-0078 (no silently - inert metadata), ADR-0069 (enterprise authentication hardening) - -## Context - -A `password`-typed field declared on a **non-auth** object (e.g. -`showcase_field_zoo.f_password`) round-tripped **plaintext** through the generic -CRUD engine: it was stored as-is and read back verbatim over the data API. This -is unlike the `secret` field channel, which encrypts on write into `sys_secret` -and masks to `SECRET_MASK` (`••••••••`) on read, so plaintext never leaves the -engine outside a privileged `resolveSecret` call. - -This is a low-code platform where field types are author-driven (often by an AI). -Someone modeling a `password` field on a custom object reasonably expects -credential-grade handling; today they silently got plaintext storage and -plaintext reads, a runtime/security trap the static gates do not catch. The -real one-way hashing lives entirely in the auth subsystem (better-auth endpoints, -the hashed `sys_account.password` `text` column) and never touches an authored -`password` field. - -The issue framed four options: (1) mask on read like `secret`; (2) hash on write -in the generic path; (3) an author-time guard; (4) document as auth-only. Option -2 is the wrong fit — one-way hashing only makes sense for credential -*verification*, which a non-auth object never does, and doing it in the engine -would stand up a second, unmanaged credential store that violates the -"auth subsystem owns credentials" boundary. Option 4 leaves the silent trap in -place. This ADR adopts **1 + 3**. - -## Decision - -1. **Mask on read, everywhere the generic path returns rows.** A `password` - field on a generic object is masked to `SECRET_MASK` in `find` / `findOne` - (and therefore in `$expand`, which re-enters `find`). The read set is computed - by `collectMaskedReadFields` (`packages/objectql/src/secret-fields.ts`), which - returns every `secret` field plus every `password` field; `maskSecretFields` - in `engine.ts` consumes it. `secret` behavior is unchanged. - -2. **Plaintext at rest, by design.** Unlike `secret`, a `password` value is - **not** encrypted and gets **no** `sys_secret` row — it is stored verbatim. - Masking is a read-path transform only. This keeps the change minimal and - avoids standing up a second credential store; it also means a `password` - field needs no `CryptoProvider` (no fail-closed throw). Authors who need - reversible encryption-at-rest should use `secret`. - -3. **Echoed-mask write guard.** Because a read now returns `SECRET_MASK`, a - client that reads a record and PATCHes it back would otherwise overwrite the - stored value with the literal mask. `encryptSecretFields` drops any masked - field (secret or password) whose incoming value equals `SECRET_MASK`, so an - unchanged round-trip is a no-op. The one accepted cost: the literal string - `••••••••` cannot itself be stored as a password via an echoing client. - -4. **`managedBy: 'better-auth'` exemption.** The auth subsystem reads its - identity rows through the engine's `find`/`findOne` (the better-auth ObjectQL - adapter). Masking a credential column there would break login. Objects marked - `managedBy: 'better-auth'` are therefore exempt from password masking. Today - this is a safety net, not load-bearing: no shipped identity object even - declares a `password`-typed field (`sys_account.password` is a hashed `text` - column), pinned by a platform-objects test so retyping it becomes a - deliberate decision rather than a silent login break. - -5. **Non-fatal author-time warning (ADR-0077/0078).** `ObjectSchema.create()` - emits a `console.warn` (deduped per object name) when a `password` field is - declared on a non-`better-auth` object, steering authors to `Field.secret` - for reversible machine credentials or to the auth subsystem for login - credentials. It is a *warning*, not a build error: `password` now has a - defined generic-path contract (so ADR-0078 does not compel an error), and the - field-zoo example intentionally exercises every field type — a hard error - would be self-inflicted breakage. Raw `.parse()` stays silent, since it also - loads persisted metadata and `create()` is the authoring surface (ADR-0077). - -## Consequences - -- Edit forms that prefill from a read now show the mask for `password` fields — - identical to the existing `secret` UX. Unchanged-value saves are protected by - the echoed-mask guard (Decision 3). -- The generic read path no longer leaks credential plaintext for `password` - fields; the field-zoo HTTP round-trip pins this (`f_password` upgraded from - `present` to `masked`). -- This ADR also retroactively documents the `secret` field channel, resolving - the dangling "ADR (secret field channel)" references in `field.zod.ts` and - `secret-fields.ts`. - -## Non-goals / follow-ups - -- **`aggregate()` masking gap.** `aggregate()` masks neither `secret` nor - `password` — a pre-existing gap for `secret`. Post-hoc masking of aggregate - output would corrupt group keys; the correct fix is to *reject* aggregations - that reference a credential field. Tracked as a separate follow-up issue, not - addressed here. -- **Hashing / verification for authored `password` fields** — explicitly out of - scope (Decision 2). Credential verification belongs to the auth subsystem.