Skip to content

Commit e474853

Browse files
huangyiireneclaude
andauthored
fix(platform-objects): sys_session.token stops serializing on the data API — internal: true (#7823) (#7996)
* wip: internal:true on sys_session.token * test+changeset for #7823 * test: add session lifecycle arm * 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 * test(objectql): re-judge the two engine-level write-response pins under A-prime (#7823) They pinned exactly the limb the ruling relocates — the engine omitting internal fields from its own insert/update results, which is what broke signIn. Replaced wholesale (fixture-triage rule): they now pin the LIVENESS half — engine write results KEEP the flagged value — so re-adding an engine-level strip (the regression that broke authentication) goes red here, while the external-body guarantee is pinned at the ingress by the tripwire and the dogfood suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy * fix(metadata-protocol): pin the tripwire fixture's delete/update to the shared dispatch predicate (#7823) check:engine-double-contract flagged makeSentinelEngine()'s delete()/update() as unpinned engine doubles — exactly the toolchain trap AGENTS.md names. Route both through assertEngineDeleteDispatch/assertEngineUpdateDispatch from @objectstack/metadata-core, the same pattern protocol.batch-verb-driver-text.test.ts already uses. No behavioural change to the tripwire itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 79021fc commit e474853

13 files changed

Lines changed: 1396 additions & 30 deletions
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
---
2+
"@objectstack/platform-objects": patch
3+
"@objectstack/objectql": patch
4+
"@objectstack/metadata-protocol": patch
5+
"@objectstack/plugin-auth": patch
6+
"@objectstack/rest": patch
7+
---
8+
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)
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) One field-level flag added
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. -->
18+
19+
`sys_session.token` — the **live bearer credential** for an active session —
20+
declared `description: 'Opaque session token — never exposed in UI'` and then
21+
serialized anyway on the generic data path.
22+
23+
**Scope the persona precisely: this is an ADMIN-CROSS-USER disclosure**, not an
24+
any-authenticated-caller one. Measured on a real engine (`bootStack(showcaseStack)`,
25+
in-process HTTP + sqlite-wasm):
26+
27+
- **admin**, `GET /data/sys_session` (list) — 200, `token` present on every row,
28+
the admin's own **and every other user's**;
29+
- **admin**, `GET /data/sys_session/{another user's id}` — 200, that member's
30+
token verbatim;
31+
- **admin**, `?select=id,token` — 200, present;
32+
- anonymous — 401, fully denied;
33+
- member — self-scoped reads only, and a cross-user get-by-id still answers
34+
**404**: the `sys_session_self` RLS policy was already holding that line and
35+
is untouched here.
36+
37+
**Why this is more than exposure.** The sibling column closed by #7728
38+
(`sys_api_key.key`) is a stored SHA-256 hash. This one is not: the disclosure was
39+
**replay-proven** — a member's token, taken exactly as it came back to the admin
40+
off the data API, authenticates as that member when sent as
41+
`Authorization: Bearer <token>`. So the defect was admin-to-member
42+
**impersonation**, and any admin-adjacent read (an integration, a leaked admin
43+
API response, a support tool) inherited it.
44+
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.
73+
74+
`hidden: true` was never the broken contract (spec defines it as "Hidden from
75+
default UI", never as "stripped from serialization"); the broken contract was the
76+
field's own description.
77+
78+
**Not retyped, deliberately.** `Field.secret` would encrypt at rest and replace
79+
the column with a `sys_secret` ref, destroying the by-token session lookup
80+
better-auth performs on every authenticated request — it would break
81+
authentication in order to fix a disclosure. `Field.password` is inert here: the
82+
read mask skips `password` on `managedBy: 'better-auth'` objects, and it collects
83+
by **TYPE** regardless, which a `text` column never satisfies. Two independent
84+
barriers, so the column stays `text`.
85+
86+
**Storage, filtering and indexing are untouched** — the strip runs on the rows the
87+
driver has already produced, after the predicate has been evaluated and the unique
88+
index on `token` used. The regression proof drives both directions: sessions still
89+
mint, the minted bearer still authenticates (`GET /auth/get-session` ⇒ 200), a
90+
`where: { token }` lookup still resolves the row server-side while that same row
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`.
@@ -7625,6 +7626,14 @@ export class ObjectStackProtocolImplementation implements
76257626
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
76267627
if (request.context !== undefined) opts.context = request.context;
76277628
const result = await this.engine.insert(request.object, data, opts);
7629+
// [#7823] The 201 body is a GENERIC-DATA-PATH surface: strip
7630+
// `internal: true` fields here, at the ingress, per the A-prime ruling
7631+
// (2026-08-13). The engine deliberately no longer strips its own write
7632+
// results — better-auth reads a minted `sys_session.token` back off
7633+
// them — so this line is what keeps a flagged credential out of the
7634+
// external create response. Tripwire-enforced; see
7635+
// `write-response-internal-fields.ts`.
7636+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
76287637
return {
76297638
object: request.object,
76307639
id: result.id,
@@ -7699,6 +7708,12 @@ export class ObjectStackProtocolImplementation implements
76997708
const insertData = stripReadonlyForInsert(schema, data, ctx);
77007709

77017710
const result = await this.engine.insert(request.object, insertData, ctxOpt as any);
7711+
// [#7823] Same ingress strip as `createData` — a clone's 201 body is
7712+
// the same generic-data-path surface. (The SOURCE row was read through
7713+
// the engine's find path, which already omits internal fields, so the
7714+
// copy never carried one in; this guards the INSERT RESULT, which the
7715+
// engine returns whole by design.)
7716+
omitInternalFieldsFromWriteResponse(schema, result);
77027717
return {
77037718
object: request.object,
77047719
id: result.id,
@@ -7797,6 +7812,16 @@ export class ObjectStackProtocolImplementation implements
77977812
? { ...(request.data as Record<string, unknown>), id: request.id }
77987813
: request.data;
77997814
const result = await this.engine.update(request.object, writeData, opts);
7815+
// [#7823] The PATCH 200 body is the surface #7728's fourth measurement
7816+
// caught: a client revoking a `sys_api_key` (apiMethods keeps `update`
7817+
// open, #7727) got the stored hash back in this response. That closure
7818+
// used to live in the engine's by-id update path and RELOCATED here
7819+
// under the A-prime ruling (2026-08-13) — the engine's write results
7820+
// stay whole for privileged server-side writers, and THIS line is the
7821+
// sole closure of that surface. Pinned by
7822+
// `api-key-hash-not-serialized.dogfood.test.ts` and the ingress
7823+
// tripwire.
7824+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), result);
78007825
return {
78017826
object: request.object,
78027827
id: request.id,
@@ -7832,6 +7857,22 @@ export class ObjectStackProtocolImplementation implements
78327857
};
78337858
}
78347859

7860+
/**
7861+
* [#7823] The write-response `internal: true` strip, exposed for generic
7862+
* write ingresses that live OUTSIDE this class. The one consumer today is
7863+
* the REST cross-object transactional batch (`POST /batch` in
7864+
* `@objectstack/rest`), whose UPDATE arm calls `ql.update` directly — a
7865+
* deliberate #3835-era choice made when the engine still stripped its own
7866+
* write results — and pushes the returned row into the response body.
7867+
* `@objectstack/rest` does not depend on this package, so it reaches the
7868+
* helper through the protocol instance it already holds (duck-typed, the
7869+
* way it probes `createManyData`). In-place, idempotent, non-objects
7870+
* skipped — see `write-response-internal-fields.ts`.
7871+
*/
7872+
omitInternalWriteFields(object: string, records: unknown): void {
7873+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(object), records);
7874+
}
7875+
78357876
/**
78367877
* [#4435] Does this row EXIST? A fact about the database — deliberately
78377878
* NOT "may this caller see it".
@@ -8432,6 +8473,7 @@ export class ObjectStackProtocolImplementation implements
84328473
const stripped = stripReadonlyForInsert(batchSchema, record.data || record, context);
84338474
const ev = diffDroppedFields(object, record.data || record, stripped, 'readonly');
84348475
const created = await this.engine.insert(object, stripped, insertCtx as any);
8476+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
84358477
results.push({ id: created.id, success: true, data: created, index, ...(ev ? { droppedFields: [ev] } : {}) });
84368478
succeeded++;
84378479
break;
@@ -8447,6 +8489,7 @@ export class ObjectStackProtocolImplementation implements
84478489
// [#3455] Collect the engine's LEGAL write strips per row.
84488490
const dropped: DroppedFieldsEvent[] = [];
84498491
const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
8492+
omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
84508493
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
84518494
succeeded++;
84528495
break;
@@ -8478,13 +8521,16 @@ export class ObjectStackProtocolImplementation implements
84788521
if (existing) {
84798522
const dropped: DroppedFieldsEvent[] = [];
84808523
const updated = await this.engine.update(object, record.data || {}, { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); }, ...ctxOpt } as any);
8524+
omitInternalFieldsFromWriteResponse(batchSchema, updated); // [#7823]
84818525
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
84828526
} else {
84838527
const created = await this.engine.insert(object, { id: record.id, ...(record.data || {}) }, insertCtx as any);
8528+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
84848529
results.push({ id: created.id, success: true, data: created, index });
84858530
}
84868531
} else {
84878532
const created = await this.engine.insert(object, record.data || record, insertCtx as any);
8533+
omitInternalFieldsFromWriteResponse(batchSchema, created); // [#7823]
84888534
results.push({ id: created.id, success: true, data: created, index });
84898535
}
84908536
succeeded++;
@@ -8708,6 +8754,12 @@ export class ObjectStackProtocolImplementation implements
87088754
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
87098755
if (request.context !== undefined) opts.context = request.context;
87108756
const records = await this.engine.insert(request.object, rows, opts);
8757+
// [#7823] Bulk create is the same generic-data-path surface as the
8758+
// single-record 201 — one strip over the returned rows, at the
8759+
// ingress. (Today's `internal`-flagged objects grant no `bulk`
8760+
// apiMethod, so this face cannot reach one over REST yet; the strip is
8761+
// here so the flag's guarantee does not depend on that staying true.)
8762+
omitInternalFieldsFromWriteResponse(this.engine.registry?.getObject(request.object), records);
87118763
const merged = mergeDroppedFieldEvents(dropped);
87128764
return {
87138765
object: request.object,
@@ -8764,6 +8816,15 @@ export class ObjectStackProtocolImplementation implements
87648816
rows,
87658817
opts,
87668818
);
8819+
// [#7823] Per-outcome ingress strip — the partial-success face hands
8820+
// each written row back as `outcomes[i].record`, so each is the same
8821+
// generic-data-path surface as a single-record 201 body.
8822+
if (Array.isArray(outcomes)) {
8823+
const outcomeSchema = this.engine.registry?.getObject(request.object);
8824+
for (const o of outcomes) {
8825+
if (o?.record) omitInternalFieldsFromWriteResponse(outcomeSchema, o.record);
8826+
}
8827+
}
87678828
if (Array.isArray(outcomes)) {
87688829
for (let i = 0; i < outcomes.length; i++) {
87698830
if (!outcomes[i]) continue;
@@ -8824,6 +8885,10 @@ export class ObjectStackProtocolImplementation implements
88248885
const results: BatchDataRowResult[] = [];
88258886
let succeeded = 0;
88268887
let failed = 0;
8888+
// [#7823] Ingress strip over each row's `data` payload — the bulk
8889+
// update face is the same generic-data-path surface as the by-id
8890+
// PATCH body, one row at a time. Resolved once; the loop reuses it.
8891+
const updateManySchema = this.engine.registry?.getObject(object);
88278892

88288893
for (const [index, record] of records.entries()) {
88298894
try {
@@ -8857,6 +8922,7 @@ export class ObjectStackProtocolImplementation implements
88578922
const opts: any = { where: { id: record.id }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
88588923
if (context !== undefined) opts.context = context;
88598924
const updated = await this.engine.update(object, record.data || {}, opts);
8925+
omitInternalFieldsFromWriteResponse(updateManySchema, updated); // [#7823]
88608926
results.push({ id: record.id, success: true, data: updated, index, ...(dropped.length > 0 ? { droppedFields: dropped } : {}) });
88618927
succeeded++;
88628928
} catch (err: any) {

0 commit comments

Comments
 (0)