Skip to content

Commit a82ce52

Browse files
committed
feat(objectql): mask password fields on the generic read path (#2036)
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QJrKxe1jXGudFT3nUks1yN
1 parent beaf2de commit a82ce52

12 files changed

Lines changed: 406 additions & 39 deletions

File tree

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# ADR-0100: Mask `password` Fields on the Generic Read Path
2+
3+
- **Status**: Accepted
4+
- **Date**: 2026-07-18
5+
- **Issue**: #2036 (found via #2033 / #2025 / #2028 field-type round-trip work)
6+
- **Relates to**: ADR-0077 (authoring-surface boundary), ADR-0078 (no silently
7+
inert metadata), ADR-0069 (enterprise authentication hardening)
8+
9+
## Context
10+
11+
A `password`-typed field declared on a **non-auth** object (e.g.
12+
`showcase_field_zoo.f_password`) round-tripped **plaintext** through the generic
13+
CRUD engine: it was stored as-is and read back verbatim over the data API. This
14+
is unlike the `secret` field channel, which encrypts on write into `sys_secret`
15+
and masks to `SECRET_MASK` (`••••••••`) on read, so plaintext never leaves the
16+
engine outside a privileged `resolveSecret` call.
17+
18+
This is a low-code platform where field types are author-driven (often by an AI).
19+
Someone modeling a `password` field on a custom object reasonably expects
20+
credential-grade handling; today they silently got plaintext storage and
21+
plaintext reads, a runtime/security trap the static gates do not catch. The
22+
real one-way hashing lives entirely in the auth subsystem (better-auth endpoints,
23+
the hashed `sys_account.password` `text` column) and never touches an authored
24+
`password` field.
25+
26+
The issue framed four options: (1) mask on read like `secret`; (2) hash on write
27+
in the generic path; (3) an author-time guard; (4) document as auth-only. Option
28+
2 is the wrong fit — one-way hashing only makes sense for credential
29+
*verification*, which a non-auth object never does, and doing it in the engine
30+
would stand up a second, unmanaged credential store that violates the
31+
"auth subsystem owns credentials" boundary. Option 4 leaves the silent trap in
32+
place. This ADR adopts **1 + 3**.
33+
34+
## Decision
35+
36+
1. **Mask on read, everywhere the generic path returns rows.** A `password`
37+
field on a generic object is masked to `SECRET_MASK` in `find` / `findOne`
38+
(and therefore in `$expand`, which re-enters `find`). The read set is computed
39+
by `collectMaskedReadFields` (`packages/objectql/src/secret-fields.ts`), which
40+
returns every `secret` field plus every `password` field; `maskSecretFields`
41+
in `engine.ts` consumes it. `secret` behavior is unchanged.
42+
43+
2. **Plaintext at rest, by design.** Unlike `secret`, a `password` value is
44+
**not** encrypted and gets **no** `sys_secret` row — it is stored verbatim.
45+
Masking is a read-path transform only. This keeps the change minimal and
46+
avoids standing up a second credential store; it also means a `password`
47+
field needs no `CryptoProvider` (no fail-closed throw). Authors who need
48+
reversible encryption-at-rest should use `secret`.
49+
50+
3. **Echoed-mask write guard.** Because a read now returns `SECRET_MASK`, a
51+
client that reads a record and PATCHes it back would otherwise overwrite the
52+
stored value with the literal mask. `encryptSecretFields` drops any masked
53+
field (secret or password) whose incoming value equals `SECRET_MASK`, so an
54+
unchanged round-trip is a no-op. The one accepted cost: the literal string
55+
`••••••••` cannot itself be stored as a password via an echoing client.
56+
57+
4. **`managedBy: 'better-auth'` exemption.** The auth subsystem reads its
58+
identity rows through the engine's `find`/`findOne` (the better-auth ObjectQL
59+
adapter). Masking a credential column there would break login. Objects marked
60+
`managedBy: 'better-auth'` are therefore exempt from password masking. Today
61+
this is a safety net, not load-bearing: no shipped identity object even
62+
declares a `password`-typed field (`sys_account.password` is a hashed `text`
63+
column), pinned by a platform-objects test so retyping it becomes a
64+
deliberate decision rather than a silent login break.
65+
66+
5. **Non-fatal author-time warning (ADR-0077/0078).** `ObjectSchema.create()`
67+
emits a `console.warn` (deduped per object name) when a `password` field is
68+
declared on a non-`better-auth` object, steering authors to `Field.secret`
69+
for reversible machine credentials or to the auth subsystem for login
70+
credentials. It is a *warning*, not a build error: `password` now has a
71+
defined generic-path contract (so ADR-0078 does not compel an error), and the
72+
field-zoo example intentionally exercises every field type — a hard error
73+
would be self-inflicted breakage. Raw `.parse()` stays silent, since it also
74+
loads persisted metadata and `create()` is the authoring surface (ADR-0077).
75+
76+
## Consequences
77+
78+
- Edit forms that prefill from a read now show the mask for `password` fields —
79+
identical to the existing `secret` UX. Unchanged-value saves are protected by
80+
the echoed-mask guard (Decision 3).
81+
- The generic read path no longer leaks credential plaintext for `password`
82+
fields; the field-zoo HTTP round-trip pins this (`f_password` upgraded from
83+
`present` to `masked`).
84+
- This ADR also retroactively documents the `secret` field channel, resolving
85+
the dangling "ADR (secret field channel)" references in `field.zod.ts` and
86+
`secret-fields.ts`.
87+
88+
## Non-goals / follow-ups
89+
90+
- **`aggregate()` masking gap.** `aggregate()` masks neither `secret` nor
91+
`password` — a pre-existing gap for `secret`. Post-hoc masking of aggregate
92+
output would corrupt group keys; the correct fix is to *reject* aggregations
93+
that reference a credential field. Tracked as a separate follow-up issue, not
94+
addressed here.
95+
- **Hashing / verification for authored `password` fields** — explicitly out of
96+
scope (Decision 2). Credential verification belongs to the auth subsystem.

examples/app-showcase/src/data/objects/field-zoo.object.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export const FieldZoo = ObjectSchema.create({
3535
f_email: Field.email({ label: 'Email', searchable: true }),
3636
f_url: Field.url({ label: 'URL' }),
3737
f_phone: Field.phone({ label: 'Phone' }),
38-
f_password: Field.password({ label: 'Password (one-way hash)' }),
38+
f_password: Field.password({ label: 'Password (masked on read)' }),
3939
f_secret: Field.secret({ label: 'Secret (encrypted at rest)' }),
4040

4141
// ── Rich content ─────────────────────────────────────────────────────

packages/objectql/src/core.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export {
7575
isSecretRef,
7676
parseSecretRef,
7777
collectSecretFields,
78+
collectMaskedReadFields,
7879
} from './secret-fields.js';
7980

8081
// Utilities

packages/objectql/src/engine.ts

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { IRealtimeService, RealtimeEventPayload } from '@objectstack/spec/contra
1919
import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts';
2020
import {
2121
collectSecretFields,
22+
collectMaskedReadFields,
2223
makeSecretRef,
2324
parseSecretRef,
2425
isSecretRef,
@@ -1363,19 +1364,28 @@ export class ObjectQL implements IDataEngine {
13631364
}
13641365

13651366
/**
1366-
* Encrypt any `secret`-typed fields on `row` in place before it reaches the
1367-
* driver. Each plaintext is wrapped by the ICryptoProvider, persisted as a
1368-
* `sys_secret` row, and replaced on `row` by an opaque ref. Cleartext never
1369-
* reaches the business table.
1367+
* Normalize credential fields on `row` in place before it reaches the driver.
1368+
*
1369+
* Two channels share the read mask ({@link SECRET_MASK}) but differ on write
1370+
* (see ADR-0100):
1371+
*
1372+
* - **`secret`** — encrypted: each plaintext is wrapped by the ICryptoProvider,
1373+
* persisted as a `sys_secret` row, and replaced on `row` by an opaque ref.
1374+
* Cleartext never reaches the business table.
1375+
* - **`password`** (generic, non-`better-auth`) — plaintext at rest: stored
1376+
* verbatim, no encryption and no `sys_secret` row. Only the echoed-mask drop
1377+
* below applies to it.
13701378
*
13711379
* Rules:
1372-
* - No secret fields on the object ⇒ no-op (fast path, no crypto cost).
1373-
* - `null`/`undefined` value ⇒ left as-is (clears the secret).
1374-
* - Value already a ref (re-save of an unchanged ref) ⇒ left as-is.
1375-
* - Value equal to the read mask ⇒ dropped, so a form round-trip that
1376-
* echoes the mask does not overwrite the stored secret.
1377-
* - **Fail-closed:** any other value with no CryptoProvider registered, or
1378-
* no reachable `sys_secret` store, THROWS — never persists cleartext.
1380+
* - Any masked field (secret or password) whose value equals the read mask ⇒
1381+
* the key is dropped, so a form round-trip that echoes the mask does not
1382+
* overwrite the stored value.
1383+
* - No secret fields on the object ⇒ no further work (fast path, no crypto).
1384+
* - `null`/`undefined` secret value ⇒ left as-is (clears the secret).
1385+
* - Secret value already a ref (re-save of an unchanged ref) ⇒ left as-is.
1386+
* - **Fail-closed:** any other secret value with no CryptoProvider registered,
1387+
* or no reachable `sys_secret` store, THROWS — never persists cleartext.
1388+
* (A `password` field needs no CryptoProvider — it is stored as-is.)
13791389
*/
13801390
private async encryptSecretFields(
13811391
object: string,
@@ -1385,6 +1395,17 @@ export class ObjectQL implements IDataEngine {
13851395
): Promise<void> {
13861396
if (!row || typeof row !== 'object') return;
13871397
const schema = this._registry.getObject(object);
1398+
1399+
// Echoed-mask drop for every field the read path masks (secret + generic
1400+
// password). The read path returns SECRET_MASK; a client that PATCHes it
1401+
// back means "unchanged", so drop the key rather than persist the literal
1402+
// mask. Doing this up front keeps the better-auth exemption in one place
1403+
// (collectMaskedReadFields) and covers objects that have a password field
1404+
// but no secret field. (#2036, ADR-0100)
1405+
for (const field of collectMaskedReadFields(schema)) {
1406+
if (field in row && row[field] === SECRET_MASK) delete row[field];
1407+
}
1408+
13881409
const secretFields = collectSecretFields(schema);
13891410
if (secretFields.length === 0) return;
13901411

@@ -1394,12 +1415,6 @@ export class ObjectQL implements IDataEngine {
13941415

13951416
if (value === null || typeof value === 'undefined') continue; // clear
13961417
if (isSecretRef(value)) continue; // already encrypted ref
1397-
if (value === SECRET_MASK) {
1398-
// The read path masks secrets to SECRET_MASK; a form that echoes it
1399-
// back means "unchanged". Drop the key so the stored secret survives.
1400-
delete row[field];
1401-
continue;
1402-
}
14031418

14041419
if (!this.cryptoProvider) {
14051420
throw new Error(
@@ -1446,20 +1461,23 @@ export class ObjectQL implements IDataEngine {
14461461
}
14471462

14481463
/**
1449-
* Mask `secret`-typed fields on read so plaintext never leaves the engine
1450-
* through the normal query path. A set secret becomes {@link SECRET_MASK};
1451-
* an unset one stays `null`. Privileged callers that genuinely need the
1452-
* plaintext use {@link resolveSecret} against the stored ref.
1464+
* Mask credential fields on read so plaintext never leaves the engine through
1465+
* the normal query path. Covers `secret` fields (always) and `password` fields
1466+
* on generic, non-`better-auth` objects (see {@link collectMaskedReadFields}
1467+
* and ADR-0100). A set value becomes {@link SECRET_MASK}; an unset one stays
1468+
* `null`. Privileged callers that genuinely need a secret's plaintext use
1469+
* {@link resolveSecret} against the stored ref; a `password` field is stored
1470+
* as plaintext at rest, so its cleartext is only ever reachable off this path.
14531471
*/
14541472
private maskSecretFields(object: string, rows: any): void {
14551473
if (!rows) return;
14561474
const schema = this._registry.getObject(object);
1457-
const secretFields = collectSecretFields(schema);
1458-
if (secretFields.length === 0) return;
1475+
const maskedFields = collectMaskedReadFields(schema);
1476+
if (maskedFields.length === 0) return;
14591477
const list = Array.isArray(rows) ? rows : [rows];
14601478
for (const row of list) {
14611479
if (!row || typeof row !== 'object') continue;
1462-
for (const field of secretFields) {
1480+
for (const field of maskedFields) {
14631481
if (!(field in row)) continue;
14641482
row[field] = row[field] == null ? null : SECRET_MASK;
14651483
}

packages/objectql/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ export {
102102
isSecretRef,
103103
parseSecretRef,
104104
collectSecretFields,
105+
collectMaskedReadFields,
105106
} from './secret-fields.js';
106107

107108
// Export Utilities

packages/objectql/src/secret-fields.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,3 +232,91 @@ describe('objectql secret-field channel', () => {
232232
}
233233
});
234234
});
235+
236+
// A generic (non-better-auth) object with a `password` field. Unlike `secret`,
237+
// it is plaintext at rest but masked on read — no crypto involved (ADR-0100).
238+
const deviceObject = {
239+
name: 'device', label: 'Device',
240+
fields: {
241+
id: { name: 'id', label: 'ID', type: 'text' as const },
242+
name: { name: 'name', label: 'Name', type: 'text' as const },
243+
admin_password: { name: 'admin_password', label: 'Admin Password', type: 'password' as const },
244+
},
245+
};
246+
247+
// An identity table the auth subsystem owns: `managedBy: 'better-auth'` exempts
248+
// its `password` field from masking so better-auth's own reads see the value.
249+
const authUserObject = {
250+
name: 'authy_user', label: 'Auth User', managedBy: 'better-auth' as const,
251+
fields: {
252+
id: { name: 'id', label: 'ID', type: 'text' as const },
253+
password: { name: 'password', label: 'Password', type: 'password' as const },
254+
},
255+
};
256+
257+
async function buildPasswordEngine() {
258+
// Deliberately NO CryptoProvider — a password field must not require one.
259+
const engine = new ObjectQL();
260+
const { driver, stores } = makeMemoryDriver();
261+
engine.registerDriver(driver, true);
262+
await engine.init();
263+
engine.registry.registerObject(deviceObject as any);
264+
engine.registry.registerObject(authUserObject as any);
265+
return { engine, stores };
266+
}
267+
268+
describe('objectql password-field masking (ADR-0100)', () => {
269+
let ctx: Awaited<ReturnType<typeof buildPasswordEngine>>;
270+
beforeEach(async () => { ctx = await buildPasswordEngine(); });
271+
272+
it('stores plaintext at rest (no crypto), but masks on find / findOne', async () => {
273+
const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'hunter2' });
274+
275+
// At rest: the driver row holds the verbatim plaintext, and nothing was
276+
// written to sys_secret (no encryption channel for password).
277+
const stored = ctx.stores.get('device')!.get(created.id) as any;
278+
expect(stored.admin_password).toBe('hunter2');
279+
expect(ctx.stores.get('sys_secret')?.size ?? 0).toBe(0);
280+
281+
// On read: the generic path never echoes the plaintext.
282+
const viaFind = (await ctx.engine.find('device', { where: { id: created.id } }))[0] as any;
283+
expect(viaFind.admin_password).toBe(SECRET_MASK);
284+
const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any;
285+
expect(viaOne.admin_password).toBe(SECRET_MASK);
286+
});
287+
288+
it('an unset password reads back as null, not the mask', async () => {
289+
const created = await ctx.engine.insert('device', { name: 'router', admin_password: null });
290+
const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any;
291+
expect(viaOne.admin_password).toBeNull();
292+
});
293+
294+
it('echoing the read mask back does NOT overwrite the stored password', async () => {
295+
const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'keep-me' });
296+
297+
// Form round-trip: user renames the device, the masked field echoes back.
298+
await ctx.engine.update('device', { id: created.id, name: 'gateway', admin_password: SECRET_MASK });
299+
300+
const stored = ctx.stores.get('device')!.get(created.id) as any;
301+
expect(stored.name).toBe('gateway');
302+
expect(stored.admin_password).toBe('keep-me'); // unchanged plaintext
303+
});
304+
305+
it('updating with a real new value replaces the stored plaintext', async () => {
306+
const created = await ctx.engine.insert('device', { name: 'router', admin_password: 'old-pw' });
307+
await ctx.engine.update('device', { id: created.id, admin_password: 'new-pw' });
308+
309+
const stored = ctx.stores.get('device')!.get(created.id) as any;
310+
expect(stored.admin_password).toBe('new-pw');
311+
// And a read still masks it.
312+
const viaOne = await ctx.engine.findOne('device', { where: { id: created.id } }) as any;
313+
expect(viaOne.admin_password).toBe(SECRET_MASK);
314+
});
315+
316+
it('better-auth identity tables are exempt: their password field is NOT masked on read', async () => {
317+
const created = await ctx.engine.insert('authy_user', { password: 'hashed-by-auth' });
318+
const viaOne = await ctx.engine.findOne('authy_user', { where: { id: created.id } }) as any;
319+
// Masking here would break login — the auth subsystem must read its own value.
320+
expect(viaOne.password).toBe('hashed-by-auth');
321+
});
322+
});

packages/objectql/src/secret-fields.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* Secret-field channel — helpers for the `secret` FieldType.
4+
* Credential-field channels — helpers for the `secret` and `password` FieldTypes.
55
*
66
* A `secret` field (DB password, API key, token) is **reversible**: the engine
77
* encrypts it on write via the registered `ICryptoProvider`, persists the
@@ -10,8 +10,19 @@
1010
* the Settings subsystem (`sys_setting.value_enc → sys_secret.id`), generalized
1111
* to object fields.
1212
*
13-
* Contrast with `password` — a one-way hash owned by the auth subsystem, never
14-
* decrypted. The two never share a code path.
13+
* A `password` field on a **generic** (non-`better-auth`) object is **plaintext
14+
* at rest** but **masked on read** — the engine stores the value verbatim (no
15+
* encryption, no `sys_secret` row) yet returns {@link SECRET_MASK} through the
16+
* normal query path, so cleartext never leaves the engine. This closes #2036,
17+
* where a `password` field round-tripped plaintext. See ADR-0100. The two types
18+
* share only the read mask ({@link collectMaskedReadFields}); their write paths
19+
* differ (secret encrypts, password is left untouched).
20+
*
21+
* The auth subsystem's own credentials are a third, separate channel: better-auth
22+
* one-way hashes them into identity tables (`sys_account.password`, a hashed
23+
* `text` column) off the generic CRUD path. Objects it owns carry
24+
* `managedBy: 'better-auth'` and are exempt from password masking so login reads
25+
* still see the stored hash.
1526
*/
1627

1728
import type { ServiceObject } from '@objectstack/spec/data';
@@ -59,3 +70,30 @@ export function collectSecretFields(schema: ServiceObject | undefined | null): s
5970
}
6071
return out;
6172
}
73+
74+
/**
75+
* Collect the names of fields that must be masked to {@link SECRET_MASK} on the
76+
* generic read path: every `secret` field, plus every `password` field — the
77+
* latter only when the object is **not** `managedBy: 'better-auth'`.
78+
*
79+
* The better-auth exemption is deliberate: the auth subsystem reads its identity
80+
* rows through the engine's find/findOne, and masking a credential column there
81+
* would break login. Today no identity object even declares a `password`-typed
82+
* field (`sys_account.password` is a hashed `text` column), but the guard keeps
83+
* masking safe if that ever changes. See ADR-0100.
84+
*
85+
* Returns an empty array when the schema has no fields or no maskable fields, so
86+
* callers can fast-path on `length === 0`.
87+
*/
88+
export function collectMaskedReadFields(schema: ServiceObject | undefined | null): string[] {
89+
const fields = (schema as any)?.fields as Record<string, { type?: string }> | undefined;
90+
if (!fields) return [];
91+
const isBetterAuth = (schema as any)?.managedBy === 'better-auth';
92+
const out: string[] = [];
93+
for (const [name, def] of Object.entries(fields)) {
94+
if (!def) continue;
95+
if (def.type === 'secret') out.push(name);
96+
else if (def.type === 'password' && !isBetterAuth) out.push(name);
97+
}
98+
return out;
99+
}

0 commit comments

Comments
 (0)