fix(platform-objects): sys_session.token stops serializing on the data API — internal: true (#7823)

.changeset/session-token-internal.md

@@ -0,0 +1,94 @@
+---
+"@objectstack/platform-objects": patch
+"@objectstack/objectql": patch
+"@objectstack/metadata-protocol": patch
+"@objectstack/plugin-auth": patch
+"@objectstack/rest": patch
+---
+
+fix(security): `sys_session.token` stops serializing on the data API — `internal: true`, with the write-response strip relocated to the generic-data-path ingress (#7823)
+
+<!-- adr-0087: not-required (no-migration-prescription) One field-level flag added
+to one existing declaration, plus an internal relocation of where that flag's
+write-response half is enforced (engine write sites → the metadata-protocol
+ingress). Nothing authorable is renamed, retired or tombstoned, so there is no
+conversion to register. The behavioural changes are that a field which already
+DECLARED it was never exposed stops being exposed, and that better-auth's
+session-lifecycle routes keep working while it does. -->
+
+`sys_session.token` — the **live bearer credential** for an active session —
+declared `description: 'Opaque session token — never exposed in UI'` and then
+serialized anyway on the generic data path.
+
+**Scope the persona precisely: this is an ADMIN-CROSS-USER disclosure**, not an
+any-authenticated-caller one. Measured on a real engine (`bootStack(showcaseStack)`,
+in-process HTTP + sqlite-wasm):
+
+- **admin**, `GET /data/sys_session` (list) — 200, `token` present on every row,
+ the admin's own **and every other user's**;
+- **admin**, `GET /data/sys_session/{another user's id}` — 200, that member's
+ token verbatim;
+- **admin**, `?select=id,token` — 200, present;
+- anonymous — 401, fully denied;
+- member — self-scoped reads only, and a cross-user get-by-id still answers
+ **404**: the `sys_session_self` RLS policy was already holding that line and
+ is untouched here.
+
+**Why this is more than exposure.** The sibling column closed by #7728
+(`sys_api_key.key`) is a stored SHA-256 hash. This one is not: the disclosure was
+**replay-proven** — a member's token, taken exactly as it came back to the admin
+off the data API, authenticates as that member when sent as
+`Authorization: Bearer <token>`. So the defect was admin-to-member
+**impersonation**, and any admin-adjacent read (an integration, a leaked admin
+API response, a support tool) inherited it.
+
+**The fix is one declaration plus one relocation** (maintainer ruling
+2026-08-13, "A-prime + compose"):
+
+- `sys_session.token` is declared `internal: true` — the opt-in,
+ type-independent flag minted by #7728 meaning *the declared value is never
+ returned on the generic data path*. The engine's READ-path strip is
+ unchanged and closes the disclosure.
+- The flag's **write-response** half moves out of the engine's insert/update
+ result paths — where it conflated "never on the generic data path" with
+ "never returned to the engine-level writer" and broke `signIn`/`signUp`
+ (better-auth reads the minted session row back off the insert result) —
+ into the **generic-data-path ingress**: every `*Data` write face in
+ `@objectstack/metadata-protocol` routes its response records through the
+ single exported helper `omitInternalFieldsFromWriteResponse`, held there by
+ a tripwire test that enumerates the ingress surface and fails on any face
+ the sentinel reaches (or any new `*Data` face with no recipe). The
+ `sys_api_key.key` PATCH-body closure (#7728's fourth surface) is preserved
+ at the ingress, byte-for-byte for callers. `@objectstack/rest`'s
+ cross-object batch update — the one write mouth outside the protocol —
+ applies the same shared strip.
+- better-auth's session-lifecycle readbacks (revoke-other-sessions,
+ sliding-expiry refresh, expired-session cleanup) read `token` back off
+ adapter find results, which the read strip starves — measured:
+ `POST /auth/revoke-other-sessions` answered `200 {"status":true}` while the
+ other session kept authenticating. The adapter now re-attaches the token
+ through `Engine.resolveInternalField` (#8118's privileged batch accessor) —
+ no engine carve-out, no second accessor. Plain bearer validation never
+ needed the readback and is untouched.
+
+`hidden: true` was never the broken contract (spec defines it as "Hidden from
+default UI", never as "stripped from serialization"); the broken contract was the
+field's own description.
+
+**Not retyped, deliberately.** `Field.secret` would encrypt at rest and replace
+the column with a `sys_secret` ref, destroying the by-token session lookup
+better-auth performs on every authenticated request — it would break
+authentication in order to fix a disclosure. `Field.password` is inert here: the
+read mask skips `password` on `managedBy: 'better-auth'` objects, and it collects
+by **TYPE** regardless, which a `text` column never satisfies. Two independent
+barriers, so the column stays `text`.
+
+**Storage, filtering and indexing are untouched** — the strip runs on the rows the
+driver has already produced, after the predicate has been evaluated and the unique
+index on `token` used. The regression proof drives both directions: sessions still
+mint, the minted bearer still authenticates (`GET /auth/get-session` ⇒ 200), a
+`where: { token }` lookup still resolves the row server-side while that same row
+comes back with no `token` key, and revoke-other-sessions / expired-session
+cleanup are pinned on the ROW they act on, not the status code that lied.
+Without those, a change that simply broke authentication would satisfy every
+"absent" assertion.

packages/metadata-protocol/src/index.ts

@@ -5,6 +5,14 @@ export { ObjectStackProtocolImplementation, ConcurrentUpdateError, normalizeView
 // ObjectQL FALLBACK in `@objectstack/runtime`'s `callData` builds the SAME one
 // instead of minting a second not-found shape. See `recordNotFoundError`.
 export { recordNotFoundError } from './protocol.js';
+// [#7823] The write-response half of the `internal: true` guarantee — THE
+// single helper every generic write ingress routes its response records
+// through (A-prime ruling, 2026-08-13). Tripwire-enforced; see the module
+// header for why it lives at the ingress and not in the engine.
+export {
+ omitInternalFieldsFromWriteResponse,
+ collectInternalWriteResponseFields,
+} from './write-response-internal-fields.js';
 export { createMetadataProtocolPlugin, assembleMetadataProtocol } from './plugin.js';
 export type { MetadataProtocolPluginOptions, AssembleMetadataProtocolOptions } from './plugin.js';
 // [#6710] The declared authoring channel — the explicit expression of ADR-0005's

packages/metadata-protocol/src/protocol.ts

@@ -11,6 +11,7 @@ import { readEnvWithDeprecation, resolveTenancyPosture } from '@objectstack/type
 // the posture, is what the runtime authoring gate is told.
 import { postureEnforcesWall } from '@objectstack/spec/security';
 import type { MetadataHostEngine } from './host-engine.js';
+import { omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js';
 import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js';
 // [#7560] ADR-0070's read-only-package rule, shared with the `/packages`
 // lifecycle gate in `@objectstack/runtime` — see `./package-writability.js`.
@@ -7625,6 +7626,14 @@ export class ObjectStackProtocolImplementation implements
  const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
  if (request.context !== undefined) opts.context = request.context;
  const result = await this.engine.insert(request.object, data, opts);
+ // [#7823] The 201 body is a GENERIC-DATA-PATH surface: strip
+ // `internal: true` fields here, at the ingress, per the A-prime ruling
+ // (2026-08-13). The engine deliberately no longer strips its own write
+ // results — better-auth reads a minted `sys_session.token` back off
+ // them — so this line is what keeps a flagged credential out of the
+ // external create response. Tripwire-enforced; see
+ // `write-response-internal-fields.ts`.
+ omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
  return {
  object: request.object,
  id: result.id,
@@ -7699,6 +7708,12 @@ export class ObjectStackProtocolImplementation implements
  const insertData = stripReadonlyForInsert(schema, data, ctx);
 
  const result = await this.engine.insert(request.object, insertData, ctxOpt as any);
+ // [#7823] Same ingress strip as `createData` — a clone's 201 body is
+ // the same generic-data-path surface. (The SOURCE row was read through
+ // the engine's find path, which already omits internal fields, so the
+ // copy never carried one in; this guards the INSERT RESULT, which the
+ // engine returns whole by design.)
+ omitInternalFieldsFromWriteResponse(schema, result);
  return {
  object: request.object,
  id: result.id,
@@ -7797,6 +7812,16 @@ export class ObjectStackProtocolImplementation implements
  ? { ...(request.data as Record<string, unknown>), id: request.id }
  : request.data;
  const result = await this.engine.update(request.object, writeData, opts);
+ // [#7823] The PATCH 200 body is the surface #7728's fourth measurement
+ // caught: a client revoking a `sys_api_key` (apiMethods keeps `update`
+ // open, #7727) got the stored hash back in this response. That closure
+ // used to live in the engine's by-id update path and RELOCATED here
+ // under the A-prime ruling (2026-08-13) — the engine's write results
+ // stay whole for privileged server-side writers, and THIS line is the
+ // sole closure of that surface. Pinned by
+ // `api-key-hash-not-serialized.dogfood.test.ts` and the ingress
+ // tripwire.
+ omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
  return {
  object: request.object,
  id: request.id,
@@ -7832,6 +7857,22 @@ export class ObjectStackProtocolImplementation implements
  };
  }
 
+ /**
+ * [#7823] The write-response `internal: true` strip, exposed for generic
+ * write ingresses that live OUTSIDE this class. The one consumer today is
+ * the REST cross-object transactional batch (`POST /batch` in
+ * `@objectstack/rest`), whose UPDATE arm calls `ql.update` directly — a
+ * deliberate #3835-era choice made when the engine still stripped its own
+ * write results — and pushes the returned row into the response body.
+ * `@objectstack/rest` does not depend on this package, so it reaches the
+ * helper through the protocol instance it already holds (duck-typed, the
+ * way it probes `createManyData`). In-place, idempotent, non-objects
+ * skipped — see `write-response-internal-fields.ts`.
+ */
+ omitInternalWriteFields(object: string, records: unknown): void {
+ omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(object), records);
+ }
+
  /**
  * [#4435] Does this row EXIST? A fact about the database — deliberately
  * NOT "may this caller see it".
@@ -8432,6 +8473,7 @@ export class ObjectStackProtocolImplementation implements
  const stripped = stripReadonlyForInsert(batchSchema, record.data || record, context);
  const ev = diffDroppedFields(object, record.data || record, stripped, 'readonly');
  const created = await this.engine.insert(object, stripped, insertCtx as any);
+ omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
  results.push({ id: created.id, success: true, data: created, index, ...(ev ? { droppedFields: [ev] } : {}) });
  succeeded++;
  break;
@@ -8447,6 +8489,7 @@ export class ObjectStackProtocolImplementation implements
  // [#3455] Collect the engine's LEGAL write strips per row.
  const dropped: DroppedFieldsEvent[] = [];
  const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
+ omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
  results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
  succeeded++;
  break;
@@ -8478,13 +8521,16 @@ export class ObjectStackProtocolImplementation implements
  if (existing) {
  const dropped: DroppedFieldsEvent[] = [];
  const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
+ omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
  results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
  } else {
  const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, insertCtx as any);
+ omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
  results.push({ id: created.id, success: true, data: created, index });
  }
  } else {
  const created = await this.engine.insert(object, record.data || record, insertCtx as any);
+ omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
  results.push({ id: created.id, success: true, data: created, index });
  }
  succeeded++;
@@ -8708,6 +8754,12 @@ export class ObjectStackProtocolImplementation implements
  const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
  if (request.context !== undefined) opts.context = request.context;
  const records = await this.engine.insert(request.object, rows, opts);
+ // [#7823] Bulk create is the same generic-data-path surface as the
+ // single-record 201 — one strip over the returned rows, at the
+ // ingress. (Today's `internal`-flagged objects grant no `bulk`
+ // apiMethod, so this face cannot reach one over REST yet; the strip is
+ // here so the flag's guarantee does not depend on that staying true.)
+ omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), records);
  const merged = mergeDroppedFieldEvents(dropped);
  return {
  object: request.object,
@@ -8764,6 +8816,15 @@ export class ObjectStackProtocolImplementation implements
  rows,
  opts,
  );
+ // [#7823] Per-outcome ingress strip — the partial-success face hands
+ // each written row back as `outcomes[i].record`, so each is the same
+ // generic-data-path surface as a single-record 201 body.
+ if (Array.isArray(outcomes)) {
+ const outcomeSchema = this.engine.registry?.getObject(request.object);
+ for (const o of outcomes) {
+ if (o?.record) omitInternalFieldsFromWriteResponse(outcomeSchema, o.record);
+ }
+ }
  if (Array.isArray(outcomes)) {
  for (let i = 0; i < outcomes.length; i++) {
  if (!outcomes[i]) continue;
@@ -8824,6 +8885,10 @@ export class ObjectStackProtocolImplementation implements
  const results: BatchDataRowResult[] = [];
  let succeeded = 0;
  let failed = 0;
+ // [#7823] Ingress strip over each row's `data` payload — the bulk
+ // update face is the same generic-data-path surface as the by-id
+ // PATCH body, one row at a time. Resolved once; the loop reuses it.
+ const updateManySchema = this.engine.registry?.getObject(object);
 
  for (const [index, record] of records.entries()) {
  try {
@@ -8857,6 +8922,7 @@ export class ObjectStackProtocolImplementation implements
  const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
  if (context !== undefined) opts.context = context;
  const updated = await this.engine.update(object, record.data || {}, opts);
+ omitInternalFieldsFromWriteResponse(updateManySchema, updated); // [#7823]
  results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
  succeeded++;
  } catch (err: any) {