Skip to content

Commit 7bd6447

Browse files
os-warrenclaude
andauthored
fix(objectql): mirror data's own descriptor in the flat-input Proxy (#12397) (#12581)
`installFlatInput`'s `getOwnPropertyDescriptor` trap answered every key `data` carries with one fixed literal and never read `data`'s real descriptor. Since #12277 routed `defineProperty` into `data`, a hook can put a key on the record payload with non-default attributes, and the synthesis reported the defaults back regardless — `enumerable: false` read back as `enumerable: true` while `Object.keys` correctly omitted the key. The trap now mirrors `data`'s own descriptor, forcing `configurable: true` because the proxy target is the wrapper, which does not carry the key: a verbatim mirror is a proxy-invariant violation and throws `TypeError` on any key held non-configurable, taking `Object.keys` and spread with it. Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o Co-authored-by: Claude <noreply@anthropic.com>
1 parent b307bfd commit 7bd6447

3 files changed

Lines changed: 337 additions & 2 deletions

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): the flat-input Proxy mirrors `data`'s own descriptor instead of synthesising one (#12397)
6+
7+
`installFlatInput` hands a declarative hook a flat-record Proxy over the
8+
engine's `{ data, options, id? }` wrapper. Its `getOwnPropertyDescriptor` trap
9+
answered every key `data` carries with one fixed literal —
10+
`{ configurable: true, enumerable: true, writable: true, value: data[prop] }`
11+
and never read `data`'s real descriptor. For a key created by ordinary
12+
assignment that synthesis is the truth, which is why it cost nothing for as
13+
long as assignment was the only way a key could arrive.
14+
15+
#12277 routed `defineProperty` into `data`, so a hook can now put a key on the
16+
record payload with non-default attributes for the first time, and the
17+
synthesis reported the defaults back regardless:
18+
19+
```js
20+
Object.defineProperty(ctx.input, 'k', { value: 1, enumerable: false, configurable: true });
21+
Object.getOwnPropertyDescriptor(ctx.input, 'k'); // reported enumerable: true — it is not
22+
Object.keys(ctx.input); // …while this correctly omitted 'k'
23+
```
24+
25+
Two instruments over one payload, contradicting each other. The trap now
26+
mirrors `data`'s own descriptor.
27+
28+
`configurable` is the one attribute that cannot be mirrored: the proxy target
29+
is the wrapper, which does not carry the record key, and a proxy may not report
30+
a property its target lacks as non-configurable — a verbatim mirror throws
31+
`TypeError` on any key `data` holds as `configurable: false`, and takes
32+
`Object.keys` and spread down with it, since both reach every listed key
33+
through this trap. It is forced `true`; `enumerable` / `writable` are mirrored.
34+
35+
Two further observable consequences, both pinned:
36+
37+
- Reading a descriptor no longer runs author code. The synthesis evaluated
38+
`data[prop]` to fill `value`, so asking a payload that holds an accessor for
39+
its descriptor invoked the getter; a mirror copies `get`/`set` across
40+
untouched.
41+
- `prop in data` is true for the whole prototype chain, so the synthesis
42+
answered for inherited keys too — `Object.getOwnPropertyDescriptor(input,
43+
'toString')` returned an own, enumerable, writable data property no payload
44+
has ever held, and `Object.hasOwn(input, 'toString')` was `true`. Only an own
45+
key has a descriptor to mirror; inherited keys now report `undefined`, while
46+
`'toString' in input` and the read itself are unchanged.
47+
48+
Enumeration is untouched: `ownKeys` still lists exactly `data`'s own enumerable
49+
keys and the mirror reports those as enumerable, so `Object.keys`, spread,
50+
`Object.entries` and the sandbox's `unwrapProxyToPlain` see byte-identical
51+
results. What a record payload may hold, how `defineProperty` routes into
52+
`data`, and how the engine persists it are all untouched.
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#12397] `Object.getOwnPropertyDescriptor(ctx.input, k)` reports what `data`
5+
* actually holds — it does not synthesise an answer.
6+
*
7+
* `installFlatInput` (`hook-wrappers.ts`) answered the descriptor trap for any
8+
* key `data` carries with a fixed literal:
9+
*
10+
* ```
11+
* { configurable: true, enumerable: true, writable: true, value: data[prop] }
12+
* ```
13+
*
14+
* For a key created by ordinary assignment that synthesis is the truth, which
15+
* is why it cost nothing for as long as assignment was the only way a key could
16+
* arrive. #12277 routed `defineProperty` into `data`, so a hook can now put a
17+
* key on the record payload with NON-DEFAULT attributes — and the synthesis
18+
* reports the defaults back regardless.
19+
*
20+
* ## The constraint that shapes the fix
21+
*
22+
* The proxy target is the `{ data, options, id? }` WRAPPER, which does not
23+
* carry the record key at all. A proxy may not report a property the target
24+
* does not have as non-configurable, so a naive mirror throws `TypeError` on
25+
* any key `data` holds as `configurable: false` — and, because `Object.keys`
26+
* walks `ownKeys` through this trap, it throws on plain enumeration too. The
27+
* mirror therefore FORCES `configurable: true` and mirrors the rest. Both legs
28+
* are pinned below: the forced one (`a data key held non-configurable`) and the
29+
* mirrored ones (`enumerable`, `writable`).
30+
*
31+
* ## What these cases deliberately do NOT pin
32+
*
33+
* Whether a record payload may carry an ACCESSOR at all — and what the engine
34+
* should do persisting one, since it persists a payload by evaluating it — is a
35+
* contract question about the payload, not about this trap. Nothing here widens
36+
* or narrows it: the accessor case below asserts only the two facts that hold
37+
* under every answer to it (the trap does not throw, and reading a descriptor
38+
* does not RUN the getter — the synthesis did, via `data[prop]`). Routing
39+
* (`defineProperty` → `data`) and persistence are untouched by this card.
40+
*
41+
* `wrapDeclarativeHook` is driven directly rather than through `ObjectQL`, for
42+
* the reason the sibling trap-set file (`hook-input-mutation-traps.test.ts`)
43+
* gives: the subject is the wrapper's Proxy, and a full engine dispatch would
44+
* put a driver's own copy semantics between the hook and the assertion.
45+
*/
46+
47+
import { describe, it, expect } from 'vitest';
48+
import { wrapDeclarativeHook } from './hook-wrappers.js';
49+
50+
const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} };
51+
52+
/** Run `handler` as a declarative hook over a caller payload; return the row the engine keeps. */
53+
async function runHook(
54+
data: Record<string, unknown>,
55+
handler: (input: any) => void,
56+
): Promise<Record<string, unknown>> {
57+
const meta: any = { name: 'descriptor_probe', object: 'case', event: 'beforeInsert' };
58+
const wrapped = wrapDeclarativeHook(meta, (async (ctx: any) => handler(ctx.input)) as any, {
59+
logger: silentLogger,
60+
});
61+
const raw: any = { data, options: {} };
62+
await wrapped({ object: 'case', event: 'beforeInsert', input: raw } as any);
63+
return raw.data as Record<string, unknown>;
64+
}
65+
66+
describe('[#12397] the flat-input descriptor trap mirrors `data` instead of synthesising', () => {
67+
it('REPRODUCTION — a key defined non-enumerable reports non-enumerable', async () => {
68+
// The card's repro, verbatim. Pre-fix this reported `enumerable: true`
69+
// for a key that is not enumerable — and `Object.keys` agreed with the
70+
// truth, not with the descriptor, so the two instruments an author has
71+
// contradicted each other.
72+
const seen: Record<string, unknown> = {};
73+
const persisted = await runHook({ subject: 'help' }, (input) => {
74+
Object.defineProperty(input, 'k', { value: 1, enumerable: false, configurable: true });
75+
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'k');
76+
seen.objectKeys = Object.keys(input);
77+
seen.read = input.k;
78+
});
79+
80+
expect(seen.descriptor).toEqual({
81+
value: 1,
82+
writable: false,
83+
enumerable: false,
84+
configurable: true,
85+
});
86+
// The conjunction: the descriptor the author reads agrees with the row the
87+
// engine is left holding. Asserting either alone would pass on a proxy
88+
// whose two halves disagree, which is the defect itself.
89+
expect(Object.getOwnPropertyDescriptor(persisted, 'k')).toEqual({
90+
value: 1,
91+
writable: false,
92+
enumerable: false,
93+
configurable: true,
94+
});
95+
expect(seen.objectKeys).toEqual(['subject']);
96+
expect(seen.read).toBe(1);
97+
});
98+
99+
it('`writable: false` is reported as such — the descriptor agrees with what assignment does', async () => {
100+
// The instrument and the operation used to disagree in the loudest possible
101+
// way: the descriptor advertised `writable: true` while the very next
102+
// assignment threw, because the `set` trap writes into `data` from strict
103+
// module code.
104+
const seen: Record<string, unknown> = {};
105+
await runHook({ subject: 'help' }, (input) => {
106+
Object.defineProperty(input, 'frozen_key', {
107+
value: 'V',
108+
enumerable: true,
109+
writable: false,
110+
configurable: true,
111+
});
112+
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'frozen_key');
113+
try {
114+
input.frozen_key = 'REASSIGNED';
115+
seen.assignmentThrew = false;
116+
} catch (err) {
117+
seen.assignmentThrew = true;
118+
seen.assignmentError = (err as Error).constructor.name;
119+
}
120+
seen.afterAssign = input.frozen_key;
121+
});
122+
123+
expect(seen.descriptor).toEqual({
124+
value: 'V',
125+
writable: false,
126+
enumerable: true,
127+
configurable: true,
128+
});
129+
expect(seen.assignmentThrew).toBe(true);
130+
expect(seen.assignmentError).toBe('TypeError');
131+
expect(seen.afterAssign).toBe('V');
132+
});
133+
134+
it('INVARIANT — a data key held non-configurable is reported configurable, and does not throw', async () => {
135+
// The reason the mirror cannot be naive. The proxy target is the wrapper,
136+
// which does not carry `locked`; reporting a target-absent property as
137+
// non-configurable is a proxy invariant violation, so mirroring
138+
// `configurable` verbatim throws `TypeError` here — and takes
139+
// `Object.keys`/spread down with it, since those reach every listed key
140+
// through this same trap.
141+
const data: Record<string, unknown> = { subject: 'help' };
142+
Object.defineProperty(data, 'locked', {
143+
value: 'L',
144+
enumerable: true,
145+
writable: false,
146+
configurable: false,
147+
});
148+
149+
const seen: Record<string, unknown> = {};
150+
await runHook(data, (input) => {
151+
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'locked');
152+
seen.objectKeys = Object.keys(input);
153+
seen.spread = { ...input };
154+
// `Object.prototype.hasOwnProperty.call`, not `Object.hasOwn`: this
155+
// package's programs run against `lib: ES2020` (see the workspace
156+
// tsconfig), where the ES2022 spelling is a type error — and this file is
157+
// read by the TEST_DEBT re-measure program, whose count is a ratchet.
158+
seen.hasOwn = Object.prototype.hasOwnProperty.call(input, 'locked');
159+
});
160+
161+
// `configurable` is FORCED, `enumerable`/`writable` are MIRRORED. That is
162+
// the whole contract of this trap in one assertion.
163+
expect(seen.descriptor).toEqual({
164+
value: 'L',
165+
writable: false,
166+
enumerable: true,
167+
configurable: true,
168+
});
169+
expect(seen.objectKeys).toEqual(['subject', 'locked']);
170+
expect(seen.spread).toEqual({ subject: 'help', locked: 'L' });
171+
expect(seen.hasOwn).toBe(true);
172+
});
173+
174+
it('POSITIVE CONTROL — an ordinary assigned key still reads back as a plain data descriptor', async () => {
175+
// Every existing consumer sees this shape and must keep seeing it: the
176+
// mirror is only visible on keys that were not created by assignment.
177+
const seen: Record<string, unknown> = {};
178+
await runHook({ subject: 'help' }, (input) => {
179+
input.owner_id = 'U1';
180+
seen.assigned = Object.getOwnPropertyDescriptor(input, 'owner_id');
181+
seen.caller = Object.getOwnPropertyDescriptor(input, 'subject');
182+
});
183+
expect(seen.assigned).toEqual({
184+
value: 'U1',
185+
writable: true,
186+
enumerable: true,
187+
configurable: true,
188+
});
189+
expect(seen.caller).toEqual({
190+
value: 'help',
191+
writable: true,
192+
enumerable: true,
193+
configurable: true,
194+
});
195+
});
196+
197+
it('an INHERITED key has no own descriptor to mirror, and no longer gets a fabricated one', async () => {
198+
// `prop in data` is true for the whole prototype chain, so the synthesis
199+
// answered `Object.getOwnPropertyDescriptor(input, 'toString')` with an own,
200+
// enumerable, writable data property that no payload has ever held. `in`
201+
// stays true (inherited keys ARE `in` the object) and the read still
202+
// resolves up the chain — only the own-ness claim changes.
203+
const seen: Record<string, unknown> = {};
204+
await runHook({ subject: 'help' }, (input) => {
205+
seen.descriptor = Object.getOwnPropertyDescriptor(input, 'toString');
206+
seen.hasOwn = Object.prototype.hasOwnProperty.call(input, 'toString');
207+
seen.inOperator = 'toString' in input;
208+
seen.readable = typeof input.toString;
209+
});
210+
expect(seen.descriptor).toBeUndefined();
211+
expect(seen.hasOwn).toBe(false);
212+
expect(seen.inOperator).toBe(true);
213+
expect(seen.readable).toBe('function');
214+
});
215+
216+
it('an accessor on the payload: the trap neither throws nor RUNS the getter', async () => {
217+
// Deliberately narrow — see the file header. Whether a record payload may
218+
// carry an accessor at all is a contract question this card does not
219+
// answer, so this pins only what is true under either answer. The second
220+
// half is a property the synthesis did NOT have: it read `data[prop]` to
221+
// fill `value`, so merely asking for a descriptor invoked author code.
222+
let getterCalls = 0;
223+
const data: Record<string, unknown> = { subject: 'help' };
224+
Object.defineProperty(data, 'derived', {
225+
get() {
226+
getterCalls += 1;
227+
return 'COMPUTED';
228+
},
229+
enumerable: true,
230+
configurable: true,
231+
});
232+
233+
const seen: Record<string, unknown> = {};
234+
await runHook(data, (input) => {
235+
seen.descriptorRead = () => Object.getOwnPropertyDescriptor(input, 'derived');
236+
seen.callsAfterDescriptor = ((): number => {
237+
Object.getOwnPropertyDescriptor(input, 'derived');
238+
return getterCalls;
239+
})();
240+
});
241+
242+
expect(seen.callsAfterDescriptor).toBe(0);
243+
});
244+
});

packages/objectql/src/hook-wrappers.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,10 +606,49 @@ function installFlatInput(ctx: HookContext): () => void {
606606
: [];
607607
return Array.from(new Set(dataKeys));
608608
},
609+
// [#12397] MIRRORS `data`'s own descriptor; it does not synthesise one.
610+
// The literal that stood here — `{ configurable: true, enumerable: true,
611+
// writable: true, value: data[prop] }` — happens to be the truth for every
612+
// key created by ordinary assignment, which is why it cost nothing while
613+
// assignment was the only way a key could arrive. #12277 routed
614+
// `defineProperty` into `data`, so a hook can now put a key on the record
615+
// payload with NON-DEFAULT attributes, and the synthesis kept reporting the
616+
// defaults: `Object.defineProperty(input, 'k', { enumerable: false, … })`
617+
// read back `enumerable: true` while `Object.keys(input)` — which reaches
618+
// the same key through `ownKeys`/`data` — correctly omitted it. Two
619+
// instruments, one payload, contradicting answers.
620+
//
621+
// `configurable` is the one attribute that CANNOT be mirrored. The proxy
622+
// target is the `{ data, options, id? }` wrapper, which does not carry the
623+
// record key at all, and a proxy may not report a property its target lacks
624+
// as non-configurable — so mirroring it verbatim throws `TypeError` on any
625+
// key `data` holds as `configurable: false`, and takes `Object.keys` and
626+
// spread down with it, since both reach every listed key through this trap.
627+
// It is therefore FORCED true and the rest mirrored. That forcing is the
628+
// proxy's own constraint, not a claim about the payload.
629+
//
630+
// Two consequences worth naming, both pinned in
631+
// `hook-input-descriptor-mirror.test.ts`:
632+
//
633+
// - Reading a descriptor no longer RUNS author code. The synthesis
634+
// evaluated `data[prop]` to fill `value`, so asking a payload holding
635+
// an accessor for its descriptor invoked the getter; a mirror copies
636+
// `get`/`set` across untouched.
637+
// - `prop in data` is true for the whole prototype chain, so the
638+
// synthesis answered for INHERITED keys too — `toString` reported as an
639+
// own, enumerable, writable data property no payload has ever held.
640+
// Only an own key has a descriptor to mirror; the rest fall through.
641+
//
642+
// What this trap deliberately does NOT decide: whether a record payload may
643+
// carry an accessor at all, and what the engine should do persisting one
644+
// (it persists a payload by evaluating it). That is a contract question
645+
// about the payload, and neither routing nor persistence is touched here —
646+
// the trap reports what is there, under every answer to it.
609647
getOwnPropertyDescriptor(target, prop) {
610648
const data = target.data;
611-
if (data && typeof data === 'object' && prop in data) {
612-
return { configurable: true, enumerable: true, writable: true, value: (data as any)[prop] };
649+
if (data && typeof data === 'object') {
650+
const own = Object.getOwnPropertyDescriptor(data, prop);
651+
if (own) return { ...own, configurable: true };
613652
}
614653
// Wrapper keys: still descriptors so `prop in input` works, but
615654
// marked non-enumerable so they don't appear in Object.keys().

0 commit comments

Comments
 (0)