Skip to content

Commit bac5b4a

Browse files
committed
fix(security): relocate the internal-field write-response strip to the generic-data-path ingress, and route better-auth session readbacks through resolveInternalField (#7823)
A-prime + compose, per the 2026-08-13 maintainer ruling: - objectql: the engine's two omitInternalFields write-response sites are removed — engine write results stay whole, so better-auth's createWithHooks reads the minted sys_session.token back and signIn/signUp work. The generic READ-path strip is unchanged. - metadata-protocol: omitInternalFieldsFromWriteResponse (single exported helper) applied by every *Data write face — createData / cloneData / updateData / createManyData / insertManyData / updateManyData / batchData — plus a tripwire test that enumerates the *Data surface, fails on any face a flagged sentinel reaches, and fails on any new face with no recipe. - rest: the cross-object batch update mouth (direct ql.update) applies the same strip through the protocol instance (dormant today — no flagged object grants bulk — wired so the guarantee does not depend on that). - plugin-auth: session rows read back through the better-auth adapter get token re-attached via Engine.resolveInternalField (#8118's accessor), so revoke-other-sessions / sliding refresh / expired cleanup act again while the generic data API keeps returning token-less rows. Fail-closed when a stripped row meets an engine without the accessor. - dogfood: revoke-other-sessions pinned on the other session's liveness, and expired-session cleanup pinned on the row, not the status code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy
1 parent ccc72a1 commit bac5b4a

11 files changed

Lines changed: 968 additions & 32 deletions

‎.changeset/session-token-internal.md‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
---
22
"@objectstack/platform-objects": patch
3+
"@objectstack/objectql": patch
4+
"@objectstack/metadata-protocol": patch
5+
"@objectstack/plugin-auth": patch
6+
"@objectstack/rest": patch
37
---
48

5-
fix(platform-objects): `sys_session.token` stops serializing on the data API — `internal: true` (#7823)
9+
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)
610

711
<!-- adr-0087: not-required (no-migration-prescription) One field-level flag added
8-
to one existing declaration. Nothing is renamed, retired or tombstoned, so there
9-
is no conversion to register. The only behavioural change is that a field which
10-
already DECLARED it was never exposed stops being exposed. -->
12+
to one existing declaration, plus an internal relocation of where that flag's
13+
write-response half is enforced (engine write sites → the metadata-protocol
14+
ingress). Nothing authorable is renamed, retired or tombstoned, so there is no
15+
conversion to register. The behavioural changes are that a field which already
16+
DECLARED it was never exposed stops being exposed, and that better-auth's
17+
session-lifecycle routes keep working while it does. -->
1118

1219
`sys_session.token` — the **live bearer credential** for an active session —
1320
declared `description: 'Opaque session token — never exposed in UI'` and then
@@ -35,10 +42,34 @@ off the data API, authenticates as that member when sent as
3542
**impersonation**, and any admin-adjacent read (an integration, a leaked admin
3643
API response, a support tool) inherited it.
3744

38-
**The fix is one declaration.** `internal: true` — the opt-in, type-independent
39-
flag minted by #7728 meaning *the declared value is never returned on the generic
40-
data path* — is honoured at `Engine.maskSecretFields`' collector branch and at the
41-
two write-response sites. No spec or engine change was needed.
45+
**The fix is one declaration plus one relocation** (maintainer ruling
46+
2026-08-13, "A-prime + compose"):
47+
48+
- `sys_session.token` is declared `internal: true` — the opt-in,
49+
type-independent flag minted by #7728 meaning *the declared value is never
50+
returned on the generic data path*. The engine's READ-path strip is
51+
unchanged and closes the disclosure.
52+
- The flag's **write-response** half moves out of the engine's insert/update
53+
result paths — where it conflated "never on the generic data path" with
54+
"never returned to the engine-level writer" and broke `signIn`/`signUp`
55+
(better-auth reads the minted session row back off the insert result) —
56+
into the **generic-data-path ingress**: every `*Data` write face in
57+
`@objectstack/metadata-protocol` routes its response records through the
58+
single exported helper `omitInternalFieldsFromWriteResponse`, held there by
59+
a tripwire test that enumerates the ingress surface and fails on any face
60+
the sentinel reaches (or any new `*Data` face with no recipe). The
61+
`sys_api_key.key` PATCH-body closure (#7728's fourth surface) is preserved
62+
at the ingress, byte-for-byte for callers. `@objectstack/rest`'s
63+
cross-object batch update — the one write mouth outside the protocol —
64+
applies the same shared strip.
65+
- better-auth's session-lifecycle readbacks (revoke-other-sessions,
66+
sliding-expiry refresh, expired-session cleanup) read `token` back off
67+
adapter find results, which the read strip starves — measured:
68+
`POST /auth/revoke-other-sessions` answered `200 {"status":true}` while the
69+
other session kept authenticating. The adapter now re-attaches the token
70+
through `Engine.resolveInternalField` (#8118's privileged batch accessor) —
71+
no engine carve-out, no second accessor. Plain bearer validation never
72+
needed the readback and is untouched.
4273

4374
`hidden: true` was never the broken contract (spec defines it as "Hidden from
4475
default UI", never as "stripped from serialization"); the broken contract was the
@@ -55,7 +86,9 @@ barriers, so the column stays `text`.
5586
**Storage, filtering and indexing are untouched** — the strip runs on the rows the
5687
driver has already produced, after the predicate has been evaluated and the unique
5788
index on `token` used. The regression proof drives both directions: sessions still
58-
mint, the minted bearer still authenticates (`GET /auth/get-session` ⇒ 200), and a
89+
mint, the minted bearer still authenticates (`GET /auth/get-session` ⇒ 200), a
5990
`where: { token }` lookup still resolves the row server-side while that same row
60-
comes back with no `token` key. Without those, a change that simply broke
61-
authentication would satisfy every "absent" assertion.
91+
comes back with no `token` key, and revoke-other-sessions / expired-session
92+
cleanup are pinned on the ROW they act on, not the status code that lied.
93+
Without those, a change that simply broke authentication would satisfy every
94+
"absent" assertion.

‎packages/metadata-protocol/src/index.ts‎

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

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { readEnvWithDeprecation, resolveTenancyPosture } from '@objectstack/type
1111
// the posture, is what the runtime authoring gate is told.
1212
import { postureEnforcesWall } from '@objectstack/spec/security';
1313
import type { MetadataHostEngine } from './host-engine.js';
14+
import { omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js';
1415
import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js';
1516
// [#7560] ADR-0070's read-only-package rule, shared with the `/packages`
1617
// lifecycle gate in `@objectstack/runtime` — see `./package-writability.js`.
@@ -7578,6 +7579,14 @@ export class ObjectStackProtocolImplementation implements
75787579
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
75797580
if (request.context !== undefined) opts.context = request.context;
75807581
const result = await this.engine.insert(request.object, data, opts);
7582+
// [#7823] The 201 body is a GENERIC-DATA-PATH surface: strip
7583+
// `internal: true` fields here, at the ingress, per the A-prime ruling
7584+
// (2026-08-13). The engine deliberately no longer strips its own write
7585+
// results — better-auth reads a minted `sys_session.token` back off
7586+
// them — so this line is what keeps a flagged credential out of the
7587+
// external create response. Tripwire-enforced; see
7588+
// `write-response-internal-fields.ts`.
7589+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
75817590
return {
75827591
object: request.object,
75837592
id: result.id,
@@ -7652,6 +7661,12 @@ export class ObjectStackProtocolImplementation implements
76527661
const insertData = stripReadonlyForInsert(schema, data, ctx);
76537662

76547663
const result = await this.engine.insert(request.object, insertData, ctxOpt as any);
7664+
// [#7823] Same ingress strip as `createData` — a clone's 201 body is
7665+
// the same generic-data-path surface. (The SOURCE row was read through
7666+
// the engine's find path, which already omits internal fields, so the
7667+
// copy never carried one in; this guards the INSERT RESULT, which the
7668+
// engine returns whole by design.)
7669+
omitInternalFieldsFromWriteResponse(schema, result);
76557670
return {
76567671
object: request.object,
76577672
id: result.id,
@@ -7750,6 +7765,16 @@ export class ObjectStackProtocolImplementation implements
77507765
? { ...(request.data as Record<string, unknown>), id: request.id }
77517766
: request.data;
77527767
const result = await this.engine.update(request.object, writeData, opts);
7768+
// [#7823] The PATCH 200 body is the surface #7728's fourth measurement
7769+
// caught: a client revoking a `sys_api_key` (apiMethods keeps `update`
7770+
// open, #7727) got the stored hash back in this response. That closure
7771+
// used to live in the engine's by-id update path and RELOCATED here
7772+
// under the A-prime ruling (2026-08-13) — the engine's write results
7773+
// stay whole for privileged server-side writers, and THIS line is the
7774+
// sole closure of that surface. Pinned by
7775+
// `api-key-hash-not-serialized.dogfood.test.ts` and the ingress
7776+
// tripwire.
7777+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
77537778
return {
77547779
object: request.object,
77557780
id: request.id,
@@ -7785,6 +7810,22 @@ export class ObjectStackProtocolImplementation implements
77857810
};
77867811
}
77877812

7813+
/**
7814+
* [#7823] The write-response `internal: true` strip, exposed for generic
7815+
* write ingresses that live OUTSIDE this class. The one consumer today is
7816+
* the REST cross-object transactional batch (`POST /batch` in
7817+
* `@objectstack/rest`), whose UPDATE arm calls `ql.update` directly — a
7818+
* deliberate #3835-era choice made when the engine still stripped its own
7819+
* write results — and pushes the returned row into the response body.
7820+
* `@objectstack/rest` does not depend on this package, so it reaches the
7821+
* helper through the protocol instance it already holds (duck-typed, the
7822+
* way it probes `createManyData`). In-place, idempotent, non-objects
7823+
* skipped — see `write-response-internal-fields.ts`.
7824+
*/
7825+
omitInternalWriteFields(object: string, records: unknown): void {
7826+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(object), records);
7827+
}
7828+
77887829
/**
77897830
* [#4435] Does this row EXIST? A fact about the database — deliberately
77907831
* NOT "may this caller see it".
@@ -8385,6 +8426,7 @@ export class ObjectStackProtocolImplementation implements
83858426
const stripped = stripReadonlyForInsert(batchSchema, record.data || record, context);
83868427
const ev = diffDroppedFields(object, record.data || record, stripped, 'readonly');
83878428
const created = await this.engine.insert(object, stripped, insertCtx as any);
8429+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
83888430
results.push({ id: created.id, success: true, data: created, index, ...(ev ? { droppedFields: [ev] } : {}) });
83898431
succeeded++;
83908432
break;
@@ -8400,6 +8442,7 @@ export class ObjectStackProtocolImplementation implements
84008442
// [#3455] Collect the engine's LEGAL write strips per row.
84018443
const dropped: DroppedFieldsEvent[] = [];
84028444
const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
8445+
omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
84038446
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
84048447
succeeded++;
84058448
break;
@@ -8431,13 +8474,16 @@ export class ObjectStackProtocolImplementation implements
84318474
if (existing) {
84328475
const dropped: DroppedFieldsEvent[] = [];
84338476
const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
8477+
omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
84348478
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
84358479
} else {
84368480
const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, insertCtx as any);
8481+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
84378482
results.push({ id: created.id, success: true, data: created, index });
84388483
}
84398484
} else {
84408485
const created = await this.engine.insert(object, record.data || record, insertCtx as any);
8486+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
84418487
results.push({ id: created.id, success: true, data: created, index });
84428488
}
84438489
succeeded++;
@@ -8661,6 +8707,12 @@ export class ObjectStackProtocolImplementation implements
86618707
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
86628708
if (request.context !== undefined) opts.context = request.context;
86638709
const records = await this.engine.insert(request.object, rows, opts);
8710+
// [#7823] Bulk create is the same generic-data-path surface as the
8711+
// single-record 201 — one strip over the returned rows, at the
8712+
// ingress. (Today's `internal`-flagged objects grant no `bulk`
8713+
// apiMethod, so this face cannot reach one over REST yet; the strip is
8714+
// here so the flag's guarantee does not depend on that staying true.)
8715+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), records);
86648716
const merged = mergeDroppedFieldEvents(dropped);
86658717
return {
86668718
object: request.object,
@@ -8717,6 +8769,15 @@ export class ObjectStackProtocolImplementation implements
87178769
rows,
87188770
opts,
87198771
);
8772+
// [#7823] Per-outcome ingress strip — the partial-success face hands
8773+
// each written row back as `outcomes[i].record`, so each is the same
8774+
// generic-data-path surface as a single-record 201 body.
8775+
if (Array.isArray(outcomes)) {
8776+
const outcomeSchema = this.engine.registry?.getObject(request.object);
8777+
for (const o of outcomes) {
8778+
if (o?.record) omitInternalFieldsFromWriteResponse(outcomeSchema, o.record);
8779+
}
8780+
}
87208781
if (Array.isArray(outcomes)) {
87218782
for (let i = 0; i < outcomes.length; i++) {
87228783
if (!outcomes[i]) continue;
@@ -8777,6 +8838,10 @@ export class ObjectStackProtocolImplementation implements
87778838
const results: BatchDataRowResult[] = [];
87788839
let succeeded = 0;
87798840
let failed = 0;
8841+
// [#7823] Ingress strip over each row's `data` payload — the bulk
8842+
// update face is the same generic-data-path surface as the by-id
8843+
// PATCH body, one row at a time. Resolved once; the loop reuses it.
8844+
const updateManySchema = this.engine.registry?.getObject(object);
87808845

87818846
for (const [index, record] of records.entries()) {
87828847
try {
@@ -8810,6 +8875,7 @@ export class ObjectStackProtocolImplementation implements
88108875
const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
88118876
if (context !== undefined) opts.context = context;
88128877
const updated = await this.engine.update(object, record.data || {}, opts);
8878+
omitInternalFieldsFromWriteResponse(updateManySchema, updated); // [#7823]
88138879
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
88148880
succeeded++;
88158881
} catch (err: any) {

0 commit comments

Comments
 (0)