fix(spec,objectql): internal field flag — stop sys_api_key.key serializing on the data API (#7728)

.changeset/internal-field-flag-api-key-hash.md

@@ -0,0 +1,70 @@
+---
+"@objectstack/spec": minor
+"@objectstack/objectql": minor
+"@objectstack/platform-objects": patch
+---
+
+feat(spec): `internal: true` — a field whose value is never returned on the generic data path, applied to `sys_api_key.key` (#7728)
+
+<!-- adr-0087: not-required (no-migration-prescription) Purely additive: one new
+optional field-level key. Nothing is renamed, retired or tombstoned, so there is
+no conversion to register and no consumer action to prescribe. The only
+behavioural change is that a field which already DECLARED it was never exposed
+stops being exposed. -->
+
+`sys_api_key.key` — the stored **SHA-256 hash** of an API key — declared
+`description: 'Hashed API key value — never exposed to clients'` and then
+serialized anyway. Measured on a real engine at `origin/main`, the hash came back
+on **four** surfaces: get-by-id, list, an explicit `?select=id,key` projection,
+and the `PATCH` 200 body.
+
+`hidden: true` was not the broken contract — spec defines `hidden` as "Hidden
+from default UI", never as "stripped from serialization". The broken contract was
+the field's own description, and there was no mechanism to honour it.
+
+**Why no existing mechanism fit.** ADR-0100 names three credential channels, and
+the third — the auth subsystem's one-way hashes, which live in ordinary `text`
+columns — had no read protection at all. The engine's credential mask collects by
+field **TYPE** (`collectMaskedReadFields` walks for `secret` / `password`), so a
+`text` column is collected by nothing, *regardless* of `managedBy`; the
+better-auth exemption is the second barrier, not the first. Retyping is not
+available either: `Field.secret` encrypts at rest and replaces the column with a
+`sys_secret` ref, which destroys the `where: { key: hashApiKey(raw) }` lookup the
+API-key verifier depends on — it would break authentication in order to fix a
+disclosure — and `Field.password` is defined as *plaintext at rest*, which a
+one-way hash is not, so adopting it would swap one false declaration for another.
+
+**The new flag.** `internal: true` is an opt-in, type-independent field
+declaration meaning *the declared value is never returned on the generic data
+path*. The engine omits the key from the rows it hands back at the four post-hook
+positions the `__search` companion strip (#7642) already occupies: `find`,
+`findOne`, the 201 create body and the by-id update body.
+
+**Omission, not masking.** The credential mask signals "a value is set" without
+leaking it. `key` is `required: true`, so it is always set — the signal carries
+zero bits here, while still shipping a value under a field whose declaration
+promises none. Omitting also leaves the description string untouched, so the four
+generated translation bundles that mirror it do not churn.
+
+**`?select=` is closed by construction, and that half is load-bearing.** The strip
+acts on the result rows rather than on the projection, so a client that spells the
+column out gets a 200 without it. `select` only gates on whether a field is
+*known*, and a flagged column is known — a projection-aware strip would have
+shipped looking complete while leaking to anyone who named the column.
+
+**What is deliberately untouched**, because the flag would be unusable otherwise:
+storage and encryption; filtering and indexing, so the verifier's hash lookup
+still resolves a principal (the strip runs *after* the driver has evaluated the
+predicate); and the show-once mint path — `POST /api/v1/keys` still returns the
+raw secret exactly once at creation.
+
+Unlike its sibling `stripSearchCompanionFromRead`, this strip has **no
+system-caller carve-out**. That one keeps the `__search` column for a system
+reader that names it by projection, because it has such a reader whose backfill
+would otherwise rewrite every row on every run. This flag has none: the verifier
+uses the column as a filter and never reads it off the result, and the mint path
+returns the plaintext it generated rather than the row it inserted. An escape
+hatch nobody needs is a hole in a non-exposure guarantee.
+
+Scope is one declaration site. `sys_session.token` is tracked separately as #7823
+and `sys_account.password` is a later card; neither is adopted here.

content/docs/references/data/field.mdx

@@ -105,6 +105,7 @@ const result = CurrencyConfigSchema.parse(data);
 | **conditionalRequired** | `never` | optional | [REMOVED] `conditionalRequired` was removed in @objectstack/spec 17 (#3855) — use `requiredWhen`. Rename the key; the value (a CEL predicate) is unchanged. Run `os migrate meta --from 16` to rewrite existing sources automatically. |
 | **widget** | `string` | optional | Form widget override — names a registered field component (resolved as `field:<widget>`) to render this field instead of the `type` default. Degrades to the `type` renderer when unregistered. e.g. "object-ref", "filter-condition", "recipient-picker". |
 | **hidden** | `boolean` | optional | Hidden from default UI |
+| **internal** | `boolean` | optional | [#7728] Never return this field's value on the generic data path — the engine OMITS the key from `find`/`findOne` results, the 201 create body and the by-id update body, on the default projection AND when a client names the field in `?select=`. Storage, filtering and indexing are untouched, so a server-side verifier can still match on the column and a purpose-built mint route can still return the value once at creation. The read protection for ADR-0100's third credential channel (auth-subsystem one-way hashes on `text` columns). Omission, not masking: a mask signals 'a value is set', which carries no information on a `required` column. |
 | **readonly** | `boolean` | optional | Read-only — never editable in forms, AND server-enforced on BOTH write paths: a non-system write to this field is silently dropped from the payload on UPDATE (#2948/#3003) and on INSERT (#3043; a create can no longer directly seed e.g. `approval_status: "approved"`), symmetric with `readonlyWhen`. A stripped INSERT field still falls back to its `defaultValue`. Exempt from the strip on BOTH paths: `isSystem` writes (seed replay, migration). Exempt on the UPDATE path ONLY: an opt-in "historical" import (`preserveAudit`, #3493) — which admits a whitelist (the audit/timestamp family plus author-declared business `readonly` fields). On INSERT the exemption does NOT apply (#6640): a non-system create that requests `preserveAudit` still has its readonly fields stripped, and is warned loudly that the exemption is UPDATE-only — replaying archival readonly facts on create requires a system context. A normal (non-system) import is NOT system-context and still strips. |
 | **requiredPermissions** | `string[]` | optional | [ADR-0066 D3] Capabilities required to read/edit this field (mask on read, deny on write; AND-gate). |
 | **ackPlaintextMasking** | `boolean` | optional | [ADR-0100] Affirm a generic `password` field's plaintext-at-rest / masked-on-read contract is intended, silencing the author-time warning (#3420). No effect on non-password fields. |

packages/objectql/src/engine.ts

@@ -122,6 +122,7 @@ import type { ICryptoProvider, CryptoHandle } from '@objectstack/spec/contracts'
 import {
  collectSecretFields,
  collectMaskedReadFields,
+ collectInternalReadFields,
  collectCredentialFields,
  makeSecretRef,
  parseSecretRef,
@@ -4740,19 +4741,82 @@ export class ObjectQL implements IObjectQLEngine {
  * `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.
+ *
+ * [#7728] Second collector branch, same choke point: a field declared
+ * `internal: true` is OMITTED rather than masked — see
+ * {@link omitInternalFields} for why the two dispositions differ.
  */
  private maskSecretFields(object: string, rows: any): void {
  if (!rows) return;
  const schema = this._registry.getObject(object);
  const maskedFields = collectMaskedReadFields(schema);
- if (maskedFields.length === 0) return;
+ if (maskedFields.length > 0) {
+ const list = Array.isArray(rows) ? rows : [rows];
+ for (const row of list) {
+ if (!row || typeof row !== 'object') continue;
+ for (const field of maskedFields) {
+ if (!(field in row)) continue;
+ row[field] = row[field] == null ? null : SECRET_MASK;
+ }
+ }
+ }
+ // Runs AFTER the mask, so a field that is somehow both `secret`-typed and
+ // `internal` ends up omitted rather than masked — the stricter disposition
+ // wins, which is the only safe way for the two to compose.
+ this.omitInternalFields(object, rows);
+ }
+
+ /**
+ * [#7728] Drop every field declared `internal: true` from the rows the engine
+ * hands back — "the declared value is never returned on the generic data
+ * path". This is the read protection for ADR-0100's third credential channel:
+ * auth-subsystem one-way hashes stored in `text` columns, which the two
+ * type-keyed credential collectors structurally cannot reach.
+ *
+ * **OMIT, not mask** (maintainer ruling 2026-08-12 on #7728). The credential
+ * mask exists to signal "a value is set" without leaking it. The column this
+ * was minted for — `sys_api_key.key` — is `required: true`, so it is ALWAYS
+ * set: the signal carries zero bits, while still shipping a value under a
+ * field whose own description says it is "never exposed to clients". Omission
+ * also leaves that description string untouched, so the four generated
+ * translation bundles that mirror it do not churn.
+ *
+ * **`?select=` is covered by construction, and that is load-bearing.** The
+ * strip acts on the RESULT ROWS, not on the projection, so a client that
+ * spells the column out (`?select=id,key`) gets a 200 without it rather than
+ * a bypass. `select` only gates on whether a field is KNOWN
+ * (`assertProjectionFieldsExist`) and a flagged column is known, so a
+ * projection-aware strip would have shipped looking complete and still leaked
+ * to any caller who named the column — measured on the sibling column in
+ * #7823, and reproduced here on `sys_api_key.key` before the fix.
+ *
+ * **No system carve-out**, and this is where the shape deliberately diverges
+ * from its sibling {@link stripSearchCompanionFromRead}. That one keeps the
+ * `__search` companion for a system caller who names it by projection,
+ * because it has exactly one such reader whose backfill comparison would
+ * otherwise rewrite every row on every run. This flag has no such reader: the
+ * API-key verifier uses the column as a `where` FILTER and never reads it off
+ * the result (`resolveApiKeyPrincipal` takes `expires_at` / `user_id` /
+ * `organization_id` / `scopes`), and the mint path returns the plaintext it
+ * generated, not the row it inserted. An escape hatch nobody needs is a hole
+ * in a non-exposure guarantee, so there isn't one — if a legitimate system
+ * reader ever appears, it reads the column through a purpose-built privileged
+ * accessor, the way {@link resolveSecret} does for `secret`.
+ *
+ * Nothing below storage is touched. The strip runs on rows the driver has
+ * already produced, so the predicate has been evaluated and the index used
+ * before this method sees anything — which is precisely why authentication
+ * keeps working.
+ */
+ private omitInternalFields(object: string, rows: any): void {
+ if (!rows) return;
+ const schema = this._registry.getObject(object);
+ const internalFields = collectInternalReadFields(schema);
+ if (internalFields.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 maskedFields) {
- if (!(field in row)) continue;
- row[field] = row[field] == null ? null : SECRET_MASK;
- }
+ for (const field of internalFields) delete row[field];
  }
  }
 
@@ -7779,6 +7843,12 @@ export class ObjectQL implements IObjectQLEngine {
  // AFTER the hook dispatch, matching the read path: `afterInsert`
  // handlers still observe the whole stored row.
  stripSearchCompanion(rowCtx.result);
+ // [#7728] Same position, same reason, for `internal` fields. A write
+ // has no projection to consult here either, so the omit is
+ // unconditional. This does NOT touch the show-once mint path: that
+ // route reads only `id` off the insert result and returns the
+ // plaintext it generated itself.
+ this.omitInternalFields(object, rowCtx.result);
  }
 
  // Roll-up: recompute parent summary fields that aggregate this object.
@@ -8716,6 +8786,16 @@ export class ObjectQL implements IObjectQLEngine {
  // an affected-row COUNT (#4639), which the strip skips as a
  // non-object.
  stripSearchCompanion(hookContext.result);
+ // [#7728] …and the same for `internal` fields, on the identical
+ // argument. This is not a hypothetical symmetry: `sys_api_key` is
+ // one of the few identity objects with a write verb open
+ // (`apiMethods: ['get','list','update']`, #7727) and its declared
+ // revoke/restore row actions PATCH it, so before this line a client
+ // revoking a key got the stored hash back in the 200 body — measured,
+ // and the fourth leaking surface on the object #7728 was filed
+ // against. A predicate update resolves to an affected-row COUNT
+ // (#4639), which the omit skips as a non-object.
+ this.omitInternalFields(object, hookContext.result);
  // The record IS updated; a summary that could not recompute after
  // retries must surface, not stay silent (framework#3147).
  if (summaryFailures.length > 0) throw new SummaryRecomputeError(summaryFailures, hookContext.result);