Skip to content

Commit 81ad21e

Browse files
committed
fix(objectql): droppedFields on a create names only keys the caller sent, never a middleware fill (#21682)
ObjectQL.insert took its caller snapshot (suppliedPerRow) inside the middleware chain's innermost step, after a write middleware had already filled the payload. On a walled posture @objectstack/organizations fills an absent organization_id, so the static-readonly strip took the platform's own value and droppedFields reported it on every create that named no organization. The console shows every non-empty droppedFields as a warning toast. The keys each row carries are now recorded at insert's entry, before the chain runs, keyed by the row object. The snapshot keeps only those keys. The values stay as the chain handed them on, so every key the caller did send is judged as before. There is no name list. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
1 parent e955488 commit 81ad21e

3 files changed

Lines changed: 130 additions & 5 deletions

File tree

‎packages/objectql/src/engine-insert-static-readonly-strip.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,53 @@ describe('#14147 — the exemptions, each one load-bearing', () => {
228228
expect(o.created?.completed_at, 'the hook wrote it — provenance, not value equality').toBe('2026-01-01T00:00:00Z');
229229
});
230230

231+
// [#21682] A write MIDDLEWARE runs before the caller snapshot is taken, so
232+
// its fill used to read as a key the caller sent: stripped, and reported in
233+
// `droppedFields` (`@objectstack/organizations`' `organization_id` fill, on
234+
// every walled create naming no organization). The column here is an
235+
// arbitrary author-declared one: what makes a key the platform's is WHEN it
236+
// appeared, never its name.
237+
const fillAbsent = (key: string, value: unknown) => (engine: ObjectQL) => {
238+
engine.registerMiddleware(async (opCtx: any, next: () => Promise<void>) => {
239+
if (opCtx.operation === 'insert') {
240+
for (const row of Array.isArray(opCtx.data) ? opCtx.data : [opCtx.data]) {
241+
if (row && !(key in row)) row[key] = value;
242+
}
243+
}
244+
await next();
245+
});
246+
};
247+
248+
it('[#21682] a write middleware’s fill is not caller-supplied either: it lands, and nothing is reported', async () => {
249+
const o = await observeInsert({ title: 'T' }, { context: { userId: 'u1' } }, fillAbsent('completed_at', 'stamp'));
250+
expect(o.created?.completed_at, 'the platform’s value reaches the driver').toBe('stamp');
251+
expect(o.dropped, 'droppedFields names only keys the caller sent').toEqual([]);
252+
});
253+
254+
it('[#21682] …per row on the batch path, beside a key the caller DID send, which is still taken and reported', async () => {
255+
const { engine, creates } = await makeEngine();
256+
fillAbsent('locked_note', 'stamp')(engine);
257+
const dropped: DroppedFieldsEvent[] = [];
258+
await engine.insert('duly_task', [
259+
{ title: 'A' },
260+
{ title: 'B', completed_at: 'forged' },
261+
] as any, { context: { userId: 'u1' }, onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } } as any);
262+
expect(creates.map((c) => c.locked_note), 'the fill lands on every row').toEqual(['stamp', 'stamp']);
263+
expect(creates[1]).not.toHaveProperty('completed_at');
264+
expect(dropped).toEqual([{ object: 'duly_task', fields: ['completed_at'], reason: 'readonly' }]);
265+
});
266+
267+
it('[#21682] a key the caller sent is judged as before even when a middleware rewrote its value', async () => {
268+
const o = await observeInsert({ title: 'T', completed_at: 'forged' }, { context: { userId: 'u1' } }, (engine) => {
269+
engine.registerMiddleware(async (opCtx: any, next: () => Promise<void>) => {
270+
if (opCtx.operation === 'insert') opCtx.data.completed_at = 'rewritten';
271+
await next();
272+
});
273+
});
274+
expect(o.created, 'the caller named the key, so the strip still owns it').not.toHaveProperty('completed_at');
275+
expect(o.dropped).toEqual([{ object: 'duly_task', fields: ['completed_at'], reason: 'readonly' }]);
276+
});
277+
231278
it('a PLATFORM-INTERNAL object is left to its own 403 guard (ADR-0086 / #3004, carried over)', async () => {
232279
// The reserved `sys_` namespace, and a `managedBy` bucket whose columns
233280
// carry their own fail-closed refusal, have dedicated write governance —

‎packages/objectql/src/engine.ts‎

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,6 +2063,63 @@ function droppedFieldEvents(
20632063
return events;
20642064
}
20652065

2066+
/**
2067+
* [#21682] The keys the CALLER sent on each row of an insert, recorded at
2068+
* `insert`'s entry, BEFORE the middleware chain runs, and keyed by the row
2069+
* OBJECT.
2070+
*
2071+
* `insert` takes its caller snapshot (`suppliedPerRow`) inside the middleware
2072+
* chain's innermost step, so by then a write middleware may already have
2073+
* filled the payload: `@objectstack/organizations` fills an absent
2074+
* `organization_id` with the active organization, and
2075+
* `@objectstack/plugin-security` fills an absent `owner_id` with the acting
2076+
* user. Both write IN PLACE, onto the very row objects recorded here. Without
2077+
* this record the snapshot reads those fills as keys the caller sent, so the
2078+
* static-`readonly` strip took the platform's own `organization_id` and
2079+
* `droppedFields` reported it, on every walled create that named no
2080+
* organization. The console announces every non-empty `droppedFields` as a
2081+
* warning toast. This is the insert-side twin of the update path's #8093
2082+
* (ADDRESSING IS NOT PAYLOAD): a value the platform put on the payload is
2083+
* not one the caller lost.
2084+
*
2085+
* Keyed by IDENTITY, not by index, so the answer survives a middleware that
2086+
* reorders a batch. A row a middleware REPLACED wholesale has no entry, and
2087+
* {@link callerSuppliedRow} then keeps every key it carries: the verdict from
2088+
* before this record existed. That is the over-reporting direction, never the
2089+
* under-stripping one.
2090+
*
2091+
* ⛔ No name list. Which keys are the platform's is answered by WHEN they
2092+
* appeared, so a new stamping middleware is covered without being named here.
2093+
*/
2094+
function callerKeySets(data: unknown): WeakMap<object, ReadonlySet<string>> {
2095+
const sets = new WeakMap<object, ReadonlySet<string>>();
2096+
for (const row of Array.isArray(data) ? data : [data]) {
2097+
if (row !== null && typeof row === 'object' && !sets.has(row)) {
2098+
sets.set(row, new Set(Object.keys(row)));
2099+
}
2100+
}
2101+
return sets;
2102+
}
2103+
2104+
/**
2105+
* One row of `insert`'s caller snapshot: a shallow COPY of the row as the
2106+
* middleware chain handed it on, keeping only the keys the caller sent
2107+
* (`sent`, from {@link callerKeySets}).
2108+
*
2109+
* Only WHICH keys is narrowed. A kept key keeps the value the chain handed
2110+
* on, so every key the caller did send is judged exactly as it was before
2111+
* #21682. `sent` undefined means "this row has no record" and keeps every key.
2112+
*/
2113+
function callerSuppliedRow(row: unknown, sent: ReadonlySet<string> | undefined): Record<string, unknown> {
2114+
const copy: Record<string, unknown> = { ...((row ?? {}) as Record<string, unknown>) };
2115+
if (sent) {
2116+
for (const key of Object.keys(copy)) {
2117+
if (!sent.has(key)) delete copy[key];
2118+
}
2119+
}
2120+
return copy;
2121+
}
2122+
20662123
/**
20672124
* Evaluate formula virtual fields against the raw rows a driver handed back —
20682125
* the read path (`find` / `findOne`) and, since #5504, the write path's
@@ -12817,6 +12874,10 @@ export class ObjectQL implements IObjectQLEngine {
1281712874
data = normalizeBlankTypedValues(this._registry.getObject(object), data);
1281812875
data = normalizeNumericStringValues(this._registry.getObject(object), data);
1281912876

12877+
// [#21682] What the CALLER sent, recorded before any write middleware
12878+
// fills the payload. See `callerKeySets`.
12879+
const callerKeys = callerKeySets(data);
12880+
1282012881
const opCtx: OperationContext = {
1282112882
object,
1282212883
operation: 'insert',
@@ -12844,6 +12905,14 @@ export class ObjectQL implements IObjectQLEngine {
1284412905
// untouched, hooks run after and may override.
1284512906
const nowSnap = new Date();
1284612907
const isBatch = Array.isArray(opCtx.data);
12908+
// [#21682] Each row's caller key set, looked up NOW, on the row objects
12909+
// the middleware chain handed on. The computed-field door below may
12910+
// replace a row with a copy. Index-aligned from here on: every pass
12911+
// between here and the snapshot keeps the rows' order and count.
12912+
const callerKeysPerRow: Array<ReadonlySet<string> | undefined> =
12913+
(isBatch ? (opCtx.data as unknown[]) : [opCtx.data]).map(
12914+
(row) => (row !== null && typeof row === 'object' ? callerKeys.get(row) : undefined),
12915+
);
1284712916
// [#8682] The declared-field door — see `undeclaredWriteFieldErrors` for
1284812917
// what used to run below it for a request that was already refused.
1284912918
// FIRST, so nothing downstream (defaults, summary seeding, the hooks, the
@@ -12879,10 +12948,17 @@ export class ObjectQL implements IObjectQLEngine {
1287912948
}
1288012949
// [#4441] The RAW caller payload per row — before `applyFieldDefaults`
1288112950
// resolves any `defaultValue` / `current_user` token and before the
12882-
// beforeInsert hooks stamp `owner_id` / `organization_id` /
12883-
// `created_by`. The reference check consults it to decide WHAT THE
12884-
// CALLER ACTUALLY SENT, so neither a platform stamp nor a backfilled
12885-
// default is ever reported as the caller's bad reference.
12951+
// beforeInsert hooks stamp `created_by`. The reference check consults it
12952+
// to decide WHAT THE CALLER ACTUALLY SENT, so neither a platform stamp
12953+
// nor a backfilled default is ever reported as the caller's bad
12954+
// reference.
12955+
//
12956+
// [#21682] ...and without the keys a write MIDDLEWARE filled, which ran
12957+
// before this step: `organization_id` (`@objectstack/organizations`) and
12958+
// `owner_id` (`@objectstack/plugin-security`) are filled there, not by a
12959+
// hook. Each row keeps only the keys the caller sent (`callerKeySets`,
12960+
// recorded at entry), so the strips below report and take only those.
12961+
// The values stay as the chain handed them on.
1288612962
//
1288712963
// [#6339] It carries the caller's VALUES, and it is taken HERE — ahead of
1288812964
// the hooks — as an explicit shallow COPY. Both halves are load-bearing:
@@ -12904,7 +12980,7 @@ export class ObjectQL implements IObjectQLEngine {
1290412980
// (#5591).
1290512981
const suppliedPerRow: Array<Record<string, unknown>> =
1290612982
(isBatch ? (opCtx.data as any[]) : [opCtx.data]).map(
12907-
(row) => ({ ...((row ?? {}) as Record<string, unknown>) }),
12983+
(row, i) => callerSuppliedRow(row, callerKeysPerRow[i]),
1290812984
);
1290912985
// [#20082] The write's ONE permission resolution, shared by every consumer
1291012986
// below that needs the map: the CEL defaults here, the re-default after

‎packages/plugins/organizations/src/create-explicit-organization-wall.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,8 @@ describe('[#21682] droppedFields names only keys the caller sent, so the organiz
385385
});
386386

387387
it('a readonly key the caller sends beside the fill is reported alone', async () => {
388+
// `created_by` is an injected readonly column. No audit hook stamps it in
389+
// this composition, so the caller's value is the one the strip takes.
388390
const b = await boot();
389391

390392
const { outcome, dropped } = await createReporting(b, INJECTED, { id: 'r2', name: 'new', created_by: 'usr_forged' }, MEMBER_CTX);

0 commit comments

Comments
 (0)