Skip to content

Commit 6f56c12

Browse files
committed
fix(objectql): refuse a symbol key at the flat-input set/defineProperty traps (#12603)
Maintainer ruling, 2026-08-27, Option C refusal arm: a record payload is a declarable, string-keyed field set -- no metadata schema can declare a symbol field, so a symbol key on ctx.input is a JS-runtime artifact leaking toward storage. installFlatInput's `set` and `defineProperty` traps now throw a TypeError naming the key kind and the surface, before the write ever reaches `data`, instead of routing it through silently the way #12277 routes every other mutation. Measured pre-fix, on origin/main: a symbol-keyed `set` succeeded silently, the value reached `data` and persisted to the row the engine stores, and only `Reflect.ownKeys`/`getOwnPropertySymbols` omitted it from enumeration -- two instruments said "own", enumeration said "no", while the persisted row held it regardless. That is exactly the "hiding what you persist" shape #12277/#12397/ #12578 exist to abolish; this closes it from the write side instead of the enumeration side (Option B, publishing symbols via Reflect.ownKeys, was declined -- it would have made an undeclarable key kind a published contract). `ownKeys` itself is untouched, per the ruling: ownKeys can never observe a symbol key that set/defineProperty never let onto `data`. Inverts the pin `hook-input-ownkeys-agreement.test.ts` carried OPEN since #12578 into a REFUSAL pin, in place -- the same case, turned around, not a second assertion stacked beside the first. Part of #12603 Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
1 parent ead7317 commit 6f56c12

4 files changed

Lines changed: 225 additions & 37 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
fix(objectql): the flat-input proxy REFUSES a symbol key at `set`/`defineProperty` instead of silently persisting it (#12603)
6+
7+
**Bump level, argued**: `minor`, not `patch`. Every sibling in this family (#12277,
8+
#12397, #12578, #12601) shipped `patch` because each closed an instrument
9+
DISAGREEMENT — the accepted set of writes never changed, only which read-back
10+
told the truth about them. This card is different in kind: `ctx.input[sym] =
11+
value` and `Object.defineProperty(ctx.input, sym, …)` **used to succeed**, and
12+
now **throw**. That is a narrowing of the accept set on `ctx.input` — a surface
13+
every hook body touches — which is the exact shape `8cc8401`
14+
(`@objectstack/objectql` 17.2.0, "BREAKING (accept-set tightening)") argued
15+
`minor` for under this repo's launch-window convention (pre-1.0 semantics: a
16+
breaking change does not burn a major version while the stack versions in
17+
lockstep — see `scripts/check-changeset-no-major.mjs`). `patch` here would
18+
under-declare a change that can turn a passing hook into a throwing one.
19+
20+
**What changed.** `installFlatInput`'s `set` and `defineProperty` traps
21+
(`packages/objectql/src/hook-wrappers.ts`) now refuse a symbol-keyed write with
22+
a `TypeError` naming the key kind and the surface, instead of routing it into
23+
the record payload (`data`) the way every string-keyed write is routed.
24+
Measured on the pre-fix tree: a symbol-keyed `set` succeeded silently, the
25+
value reached `data` and persisted to the row the engine stores, and only
26+
`Object.getOwnPropertySymbols` / `Reflect.ownKeys` omitted it from enumeration
27+
— two instruments said "own", enumeration said "no", while the persisted row
28+
held it regardless.
29+
30+
**Why a refusal, not a fourth instrument fix.** Maintainer ruling, 2026-08-27
31+
(Option C, refusal arm), on the payload-contract question #12578 measured and
32+
deliberately left open rather than decided: a record payload is a declarable,
33+
**string-keyed** field set — no metadata schema can declare a symbol field, so
34+
a symbol key on this surface is a JS-runtime artifact leaking toward storage,
35+
not a legal payload field. Option B (publish symbols too, via
36+
`Reflect.ownKeys`) was declined — it would have made an undeclarable key kind a
37+
published contract instead of closing the question. Hiding a key the engine
38+
nonetheless persists is precisely the shape #12277/#12397/#12578 exist to
39+
abolish; refusing the write at the boundary closes that gap from the other
40+
side, before persistence rather than after enumeration.
41+
42+
`ownKeys` itself is **untouched** — still `Object.getOwnPropertyNames(data)`,
43+
exactly as #12578 landed it. With the write refused, `data` can never carry a
44+
symbol key for that trap (or `Reflect.ownKeys`) to disagree about, so there is
45+
nothing left for this card to change there.
46+
47+
**Migration.** Code that wrote a symbol key onto `ctx.input` — almost always by
48+
accident, e.g. spreading an object that carried a symbol-keyed cache entry onto
49+
the payload — now throws instead of silently losing the write to enumeration.
50+
Use a string key, or keep the value off the payload entirely (a local
51+
variable, or a WeakMap keyed by the record) if it was never meant to be
52+
stored. No other hook-input read/write path changes: reads, `has`, `delete`,
53+
and every string-keyed write behave exactly as before.
54+
55+
Inverts the pin `hook-input-ownkeys-agreement.test.ts` carried OPEN since
56+
#12578 (the disagreement, deliberately left standing) into a REFUSAL pin
57+
(the write throws, nothing persists) — the same case, turned around in place,
58+
not a second assertion stacked beside the first.

‎content/docs/automation/hooks.mdx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,25 @@ envelope's value for a reserved name (`packages/objectql/src/hook-wrappers.ts`,
161161
#12601).
162162
</Callout>
163163

164+
<Callout type="warn" title="A symbol key is refused, not stored">
165+
A record payload is a declarable, string-keyed field set — no metadata schema
166+
can declare a symbol field. Writing one onto `ctx.input`, whether by
167+
assignment or `Object.defineProperty`, throws a `TypeError` naming the key and
168+
the surface instead of silently accepting it:
169+
170+
```ts
171+
const cacheKey = Symbol('cache-entry');
172+
handler: async (ctx) => {
173+
ctx.input[cacheKey] = value; // throws: symbol keys are not a valid record-payload field
174+
};
175+
```
176+
177+
This most often happens by accident — spreading an object that carries a
178+
symbol-keyed cache entry onto the payload. Use a string key, or keep the value
179+
off the payload entirely if it is not meant to be stored (`packages/objectql/src/hook-wrappers.ts`,
180+
#12603).
181+
</Callout>
182+
164183
```typescript
165184
import { Hook } from '@objectstack/spec/data';
166185

‎packages/objectql/src/hook-input-ownkeys-agreement.test.ts‎

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,17 @@
4949
* That disagreement is the trap's whole purpose (the payload-diff idiom must
5050
* see record fields only) and is pinned as DECLARED so it cannot be mistaken
5151
* for a residue of the defect above.
52-
* - SYMBOL KEYS carry the identical disagreement and are deliberately left
53-
* carrying it. Publishing them is a one-word change here
54-
* (`Object.getOwnPropertyNames` -> `Reflect.ownKeys`), but whether a record
55-
* payload may hold a symbol key at all is a question about the PAYLOAD
56-
* contract — the boundary #12397 drew and this card does not cross. It is
57-
* reported open on #12578 and pinned below in its open state, so answering
58-
* it changes a recorded fact instead of an unnoticed one.
52+
* - SYMBOL KEYS used to carry the identical disagreement, pinned open below:
53+
* the write succeeded silently, persisted into `data`, and only the
54+
* enumeration face omitted it. [#12603] The maintainer ruling (2026-08-27,
55+
* Option C refusal arm) answered the payload-contract question this file
56+
* left open: a record payload is a declarable, string-keyed field set, and
57+
* no metadata schema can declare a symbol field. `set` and `defineProperty`
58+
* now REFUSE a symbol-keyed write, loudly, before it ever reaches `data` —
59+
* Option B (publish via `Reflect.ownKeys`) was declined, because it would
60+
* have made the undeclarable key kind a published contract instead of
61+
* closing it. The case below is INVERTED accordingly, in place: it used to
62+
* pin the disagreement open, and now pins the refusal.
5963
*
6064
* `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, for
6165
* the reason the sibling trap-set files give: the subject is the wrapper's
@@ -195,31 +199,52 @@ describe('[#12578] the flat-input `ownKeys` reports the payload own-key set, and
195199
expect(seen.readMulti).toBe(false);
196200
});
197201

198-
it('OPEN QUESTION, pinned in its open state — a symbol key carries the same disagreement', async () => {
199-
// Reported on #12578 rather than decided here: publishing symbol keys
200-
// through `ownKeys` is `Reflect.ownKeys` in one line, but whether the
201-
// record payload may CARRY a symbol key is a payload-contract question and
202-
// a maintainer floor (#12397's boundary).
202+
it('[#12603] REFUSAL, not agreement — a symbol key is rejected before it can ever reach data', async () => {
203+
// INVERTS the OPEN QUESTION pin this case used to carry (verbatim, before
204+
// this card): `input[sym] = value` succeeded silently, persisted into
205+
// `data`, and only `Reflect.ownKeys`/`getOwnPropertySymbols` omitted it —
206+
// two instruments said own, enumeration said no, and the payload the
207+
// engine persisted held it regardless.
203208
//
204-
// What the measurement establishes, and what this case records: symbol keys
205-
// already reach `data` through the `set` trap and already persist. So the
206-
// open question is about what the enumeration face should PUBLISH, not
207-
// about what a hook can already put on the row.
209+
// Maintainer ruling, 2026-08-27, Option C refusal arm: a record payload is
210+
// a declarable, string-keyed field set — no metadata schema can declare a
211+
// symbol field, so the write is refused at the boundary instead of hidden
212+
// after it lands. There is no longer a persisted symbol key for the three
213+
// instruments to disagree about, so this is a refusal pin, not an
214+
// agreement pin — asserted for both traps the ruling names.
208215
const raw: any = { data: { subject: 'help' }, options: {} };
209-
const sym = Symbol.for('objectstack.test.12578');
210-
const seen: Record<string, unknown> = {};
216+
const sym = Symbol.for('objectstack.test.12603');
217+
218+
let setThrew: unknown;
219+
await runHook(raw, (input) => {
220+
try {
221+
input[sym] = 'symvalue';
222+
} catch (e) {
223+
setThrew = e;
224+
}
225+
});
226+
expect(setThrew).toBeInstanceOf(TypeError);
227+
const setMessage = (setThrew as TypeError).message;
228+
expect(setMessage).toMatch(/symbol/i); // names the key kind
229+
expect(setMessage).toMatch(/hook input/i); // names the surface
230+
// Refused BEFORE `data` is touched — nothing persisted, sibling field intact.
231+
expect(Object.getOwnPropertySymbols(raw.data)).toEqual([]);
232+
expect(raw.data.subject).toBe('help');
233+
234+
let definePropertyThrew: unknown;
211235
await runHook(raw, (input) => {
212-
input[sym] = 'symvalue';
213-
seen.ownness = ownness(input, sym);
214-
seen.symbols = Object.getOwnPropertySymbols(input);
236+
try {
237+
Object.defineProperty(input, sym, { value: 'dp-value', enumerable: true, configurable: true });
238+
} catch (e) {
239+
definePropertyThrew = e;
240+
}
215241
});
242+
expect(definePropertyThrew).toBeInstanceOf(TypeError);
243+
expect((definePropertyThrew as TypeError).message).toMatch(/symbol/i);
244+
expect(Object.getOwnPropertySymbols(raw.data)).toEqual([]);
216245

217-
// Today: two instruments say own, enumeration says no — the defect's shape,
218-
// deliberately left standing on this half.
219-
expect(seen.ownness).toEqual({ enumeration: false, hasOwnProperty: true, descriptor: true });
220-
expect(seen.symbols).toEqual([]);
221-
// …while the payload the engine persists holds it.
222-
expect(Object.getOwnPropertySymbols(raw.data)).toEqual([sym]);
223-
expect((raw.data as any)[sym]).toBe('symvalue');
246+
// The three instruments now agree there is no such key at all — the
247+
// refusal closes the disagreement this file otherwise exists to police.
248+
expect(ownness(raw.data, sym)).toEqual(NOT_OWN);
224249
});
225250
});

‎packages/objectql/src/hook-wrappers.ts‎

Lines changed: 95 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,60 @@ export function wrapDeclarativeHook(
511511
* `get` and `getOwnPropertyDescriptor` traps below for the instrument-by-
512512
* instrument account, and `content/docs/automation/hooks.mdx` (Hook Context)
513513
* for the author-facing statement of the same rule.
514+
*
515+
* [#12603] ⛔ SYMBOL keys are REFUSED, not routed. `set` and `defineProperty`
516+
* throw a `TypeError` for a symbol-keyed write instead of letting it reach
517+
* `data` — see those two traps below for the refusal and
518+
* `refuseSymbolPayloadKey` for the error text. See "Symbol keys" below the
519+
* reserved-name callout in `content/docs/automation/hooks.mdx`.
520+
*/
521+
522+
/**
523+
* [#12603] Maintainer ruling, 2026-08-27, Option C refusal arm: a record
524+
* payload is a declarable, string-keyed field set — no metadata schema can
525+
* declare a symbol field, so a symbol key on this flat `ctx.input` face is a
526+
* JS-runtime artifact leaking toward storage, not a legal payload field.
527+
*
528+
* Thrown from the `set` and `defineProperty` traps, BEFORE either touches
529+
* `data` — so a refused write never persists and never needs hiding from
530+
* enumeration. That is the shape change from the pre-#12603 state: a
531+
* symbol-keyed write used to succeed silently, reach `data`, and persist,
532+
* while only `Object.getOwnPropertySymbols` / `Reflect.ownKeys` omitted it
533+
* (pinned open in `hook-input-ownkeys-agreement.test.ts` under #12578). Hiding
534+
* a key the engine nonetheless persisted is exactly the shape #12277, #12397
535+
* and #12578 exist to abolish; refusing the write outright is the other way
536+
* to close that gap, and the one this ruling chose (Option B — publish
537+
* symbols via `Reflect.ownKeys` — was declined for minting an undeclarable
538+
* key kind as contract).
539+
*
540+
* A plain `TypeError`, not a new subclass: this fires from INSIDE the hook
541+
* BODY (an author wrote `input[sym] = …`), the same category as the native
542+
* `TypeError` the `defineProperty` trap already lets through for a
543+
* non-configurable descriptor the target does not carry (see that trap's own
544+
* comment) — an author-code defect, not a declarative-layer diagnostic like
545+
* `HookConditionError`. It is therefore subject to the ordinary handler error
546+
* path: `onError: 'log'` can swallow it, `retryPolicy` can retry it, exactly
547+
* as any other throw from the handler body (unlike `HookConditionError`,
548+
* which is deliberately raised OUTSIDE that path — see the comment on that
549+
* class for why the two are not the same shape).
550+
*
551+
* The message names the key kind (a symbol, not "a bad key"), the surface
552+
* (hook input), and the fix (a string key, or keep the value off the payload)
553+
* — written for the accidental case the ruling calls out: an author spreading
554+
* an object that happens to carry a symbol-keyed cache entry onto `ctx.input`.
514555
*/
556+
function refuseSymbolPayloadKey(prop: symbol, trap: 'set' | 'defineProperty'): never {
557+
const verb = trap === 'set' ? 'Cannot set' : 'Cannot define';
558+
throw new TypeError(
559+
`${verb} ${String(prop)} on hook input: a symbol key is not a valid record-payload field. ` +
560+
'A record payload is a declarable, string-keyed field set — no metadata schema can declare ' +
561+
'a symbol field, so a symbol key here would be a JS-runtime artifact leaking toward storage. ' +
562+
'Use a string key, or keep the value off the payload entirely (e.g. a local variable) if it ' +
563+
'is not meant to be stored. This often happens by accident, such as spreading an object that ' +
564+
'carries a symbol-keyed cache entry onto ctx.input.'
565+
);
566+
}
567+
515568
function installFlatInput(ctx: HookContext): () => void {
516569
const raw: any = ctx.input ?? {};
517570
const looksWrapped =
@@ -545,7 +598,14 @@ function installFlatInput(ctx: HookContext): () => void {
545598
}
546599
return Reflect.get(target, prop, receiver);
547600
},
601+
// [#12603] Symbol keys are REFUSED here, before anything else runs — a
602+
// symbol can never equal one of the four reserved (string) names, so the
603+
// check can sit first without disturbing that branch below. See
604+
// `refuseSymbolPayloadKey` for why this throws instead of routing.
548605
set(target, prop, value) {
606+
if (typeof prop === 'symbol') {
607+
refuseSymbolPayloadKey(prop, 'set');
608+
}
549609
if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
550610
(target as any)[prop] = value;
551611
return true;
@@ -600,12 +660,23 @@ function installFlatInput(ctx: HookContext): () => void {
600660
// A throw is a diagnosis; the silence was not. Omitting `configurable`
601661
// entirely (the common spelling, and every spelling `Object.assign` and
602662
// spread produce) is unaffected.
663+
//
664+
// [#12603] Symbol keys are refused here too, identically to `set` and for
665+
// the same reason — see `refuseSymbolPayloadKey`.
603666
defineProperty(target, prop, desc) {
667+
if (typeof prop === 'symbol') {
668+
refuseSymbolPayloadKey(prop, 'defineProperty');
669+
}
604670
if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
605671
return Reflect.defineProperty(target, prop, desc);
606672
}
607673
return Reflect.defineProperty(ensureData(), prop, desc);
608674
},
675+
// [#12603] `deleteProperty` is deliberately NOT guarded: since `set` and
676+
// `defineProperty` now refuse every symbol-keyed write before it reaches
677+
// `data`, a symbol key can never be there to delete. `delete input[sym]`
678+
// falls through exactly as it always has for any key `data` does not
679+
// own — a harmless no-op reporting success, not a persistence lie.
609680
has(target, prop) {
610681
if (prop === 'id' || prop === 'options' || prop === 'ast' || prop === 'data') {
611682
return prop in target;
@@ -670,15 +741,30 @@ function installFlatInput(ctx: HookContext): () => void {
670741
// SAME value everywhere it is read. See the descriptor trap's own comment
671742
// for the full account.
672743
//
673-
// SYMBOL KEYS are deliberately still absent, and this is NOT a finding
674-
// that they do not belong on a payload: `Reflect.ownKeys(data)` here would
675-
// additionally publish them, and whether the record payload may carry a
676-
// symbol key at all is a question about the PAYLOAD contract (they already
677-
// reach `data` through the `set` trap and already persist — measured), not
678-
// about this trap. It is open, reported on #12578, and the day it is
679-
// answered "yes" this line becomes `Reflect.ownKeys`. Until then the
680-
// symbol half of the disagreement is pinned AS open in the sibling test,
681-
// so an answer changes a recorded fact rather than an unnoticed one.
744+
// [#12603] SYMBOL KEYS are absent here for a settled reason now, not an
745+
// open one: the maintainer ruling (2026-08-27, Option C refusal arm)
746+
// answered the payload-contract question this trap's comment used to
747+
// leave open ("may a record payload carry a symbol key at all?") with
748+
// NO — a record payload is a declarable, string-keyed field set, and no
749+
// metadata schema can declare a symbol field. `set` and `defineProperty`
750+
// now REFUSE a symbol-keyed write before it ever reaches `data` (see
751+
// `refuseSymbolPayloadKey`), so a symbol can no longer BE an own key of
752+
// `data` for this trap to omit or report.
753+
//
754+
// This trap itself is deliberately UNCHANGED by that ruling —
755+
// `Object.getOwnPropertyNames(target.data)` stays exactly what #12578
756+
// landed. `Reflect.ownKeys(data)` (Option B) was the alternative the
757+
// ruling declined: publishing symbols through enumeration would mint an
758+
// undeclarable key kind as contract, which is the opposite of what was
759+
// ruled. With the write refused at the boundary, the two spellings would
760+
// agree anyway — `data` can never carry a symbol key for them to differ
761+
// on — so there is no remaining reason to touch this line, and #12578's
762+
// own ruling (this card must not re-litigate `ownKeys`) forbids it.
763+
//
764+
// What used to be pinned OPEN in `hook-input-ownkeys-agreement.test.ts`
765+
// (the instrument disagreement, deliberately left standing) is now
766+
// pinned as a REFUSAL in the same file: the write throws, so there is no
767+
// persisted symbol key left for the three instruments to disagree about.
682768
ownKeys(target) {
683769
return target.data && typeof target.data === 'object'
684770
? Object.getOwnPropertyNames(target.data)

0 commit comments

Comments
 (0)