From 5caee0b3285d67cb60c15eea1752119cb106390d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 14:30:05 +0000 Subject: [PATCH] fix(engine): run the roll-up summary recompute under a system context (#7673) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `recomputeSummaries` issued the parent roll-up write under the CALLER's execution context, so an engine-derived write was authorized as if the caller had asked for it. On the ordinary parent/child shape — a child more widely writable than its parent (tasks, line items, comments, time entries) — a GRANTED child write returned HTTP 500 after the row had already committed: the parent update raised PERMISSION_DENIED, which the call site rethrew as SummaryRecomputeError (ERR_SUMMARY_RECOMPUTE) and REST maps to a 500. A client that retried created a duplicate row. The recompute is now system-elevated, covering all three call sites (insert, update, delete) through the single seam they share. The elevation is a sudo()-shaped derivative of the caller's context, so an open transaction handle, tenantId and timezone still ride along — the same posture the roll-up's two other writers (the insert-time seed and the summary-nulls backfill) already held. Two quieter defects go with it, both visible only where the caller COULD write the parent: the aggregate was computed over the caller's row-level-visible subset (storing one reader's view of the collection on the parent), and an author-declared `readonly: true` roll-up column was dropped by the write-path read-only strip, which runs on `!context.isSystem`. The elevation does not widen what a caller may read or write: the parent's row stays governed by the caller's grants, the summary column stays subject to the parent's FLS on read, and ERR_SUMMARY_RECOMPUTE still surfaces genuinely failed recomputes, which is what the seed loader and import runner branch on. Tests: a shared consistency (engine-summary-recompute-context.test.ts) runs every assertion over all three call sites via it.each — the write resolves, the roll-ups land the recomputed values, the parent write carries isSystem, the child write stays under the caller context, and the aggregate reads the whole child collection — plus refusal controls asserting a direct caller update of the parent is still refused with code PERMISSION_DENIED and status 403. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SxnTmZY88mwebsRW9DXiV3 --- .../summary-recompute-system-context.md | 51 +++ content/docs/data-modeling/fields.mdx | 9 + .../engine-summary-recompute-context.test.ts | 401 ++++++++++++++++++ packages/objectql/src/engine.ts | 57 ++- 4 files changed, 514 insertions(+), 4 deletions(-) create mode 100644 .changeset/summary-recompute-system-context.md create mode 100644 packages/objectql/src/engine-summary-recompute-context.test.ts diff --git a/.changeset/summary-recompute-system-context.md b/.changeset/summary-recompute-system-context.md new file mode 100644 index 0000000000..37ae335da9 --- /dev/null +++ b/.changeset/summary-recompute-system-context.md @@ -0,0 +1,51 @@ +--- +"@objectstack/objectql": patch +--- + +fix(engine): a granted child write no longer 500s because the parent's roll-up recompute ran as the caller + +Creating a child record returned **HTTP 500 after the row had already been +written** whenever the child fed a roll-up `summary` on a parent the caller may +not edit — the ordinary parent/child shape for tasks, line items, comments and +time entries. A client that retried (or a user who clicked Save again) created a +duplicate row +([#7673](https://github.com/objectstack-ai/objectstack/issues/7673), +[#7719](https://github.com/objectstack-ai/objectstack/issues/7719)). + +`recomputeSummaries` issued the parent roll-up write under the **caller's** +execution context, so an engine-derived write was authorized as if the caller had +asked for it. On the showcase app a plain member holding `showcase_task: create + +read` hit it on every `POST` and `PATCH`: the recompute of +`showcase_project.task_count` raised `PERMISSION_DENIED`, the engine recorded it +as a recompute failure, and the call site rethrew it as `SummaryRecomputeError` +(`ERR_SUMMARY_RECOMPUTE`) — which REST maps to a 500. The access matrix and +`/security/explain` both said `create: true`, so a declared-and-granted operation +failed on a permission check about a record the caller never asked to touch. + +**The recompute now runs system-elevated**, on all three call sites (insert, +update, delete). A roll-up is engine-derived state, not a caller write: the +permission decision that matters — may this caller write the **child** — has +already been made by the time the recompute runs. The elevation is a +`sudo()`-shaped derivative of the caller's context, so an open transaction +handle, `tenantId` and `timezone` still ride along; it is the same posture the +roll-up's two other writers already held (the insert-time seed and the +`summary-nulls` backfill), so all three writers of a summary column now agree +about who owns it. + +Two quieter defects go with it, both of which only showed where the caller +*could* write the parent and the recompute therefore "succeeded": + +- the aggregate was computed over the caller's **row-level-visible subset**, so + the parent's column was silently rewritten to one reader's view of the child + collection; +- an author-declared `readonly: true` roll-up column was dropped by the + write-path read-only strip (which runs on `!context.isSystem`), so the summary + never landed at all. + +This does not widen what a caller may read or write. The parent's row is still +governed by the caller's grants (a direct update of the parent is refused exactly +as before), the summary column stays subject to the parent's field-level security +on read, and the only value this path can move is the one the author declared as +a function of the child collection. `ERR_SUMMARY_RECOMPUTE` is unchanged and +still surfaces genuinely failed recomputes (a driver or network failure that +outlives its retries), which is what the seed loader and import runner branch on. diff --git a/content/docs/data-modeling/fields.mdx b/content/docs/data-modeling/fields.mdx index 23fc6f0249..a9a9cf8209 100644 --- a/content/docs/data-modeling/fields.mdx +++ b/content/docs/data-modeling/fields.mdx @@ -253,6 +253,15 @@ are inserted, updated, or deleted. `relationshipField` is optional when the chil has only one `lookup`/`master_detail` field pointing back to the parent; set it when multiple relationships target the same parent object. +Because the roll-up is engine-derived state rather than a user write, the +recompute runs under a system context: a user who may create a child does not +also need edit access on the parent, and the aggregate is computed over the +whole child collection rather than the writer's visible subset. The permission +decision that governs the write is the one on the **child**. The parent's own +row is unaffected — a user still needs edit access to change any other field on +it — and the summary column stays subject to the parent's field-level security +on read. + ### Specialized Types | Type | Factory | Description | diff --git a/packages/objectql/src/engine-summary-recompute-context.test.ts b/packages/objectql/src/engine-summary-recompute-context.test.ts new file mode 100644 index 0000000000..0a6de2cd63 --- /dev/null +++ b/packages/objectql/src/engine-summary-recompute-context.test.ts @@ -0,0 +1,401 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// ── Roll-up recompute runs SYSTEM-ELEVATED, on all three write paths (#7673) ── +// +// `recomputeSummaries` used to issue the parent roll-up write under the +// CALLER's execution context, so an engine-derived write was authorized as if +// the caller had asked for it. On the ordinary parent/child shape — a child +// more widely writable than its parent (tasks, line items, comments, time +// entries) — that turned a GRANTED child write into `403` on the parent, which +// the call site rethrew as `SummaryRecomputeError` / `ERR_SUMMARY_RECOMPUTE` +// and REST mapped to a 500 — AFTER the child row had already committed. A +// client that retried created a duplicate row (#7673, #7719). +// +// ## Why this file is ONE table over three paths +// +// `recomputeSummaries` has THREE call sites implementing ONE contract — +// `engine.ts` insert / update / delete — and the reported symptom was found on +// two of them (POST and PATCH, #7719). A test that covered only the path that +// happened to be debugged would let the next divergence land silently, so every +// assertion below runs `it.each(WRITE_PATHS)`: insert, update and delete answer +// the same questions, or this file goes red. +// +// ## The stand-in for plugin-security is its documented contract, not a mock +// +// `@objectstack/objectql` cannot import `@objectstack/plugin-security` (the +// dependency runs the other way), so the deny gate here is a middleware that +// reproduces the ONE rule the fix relies on — `security-plugin.ts`'s +// `if (opCtx.context?.isSystem) return next();` short-circuit at the top of the +// CRUD middleware. That is the seam under test: the engine's job is to hand the +// recompute a context that satisfies it, and this file asserts both the +// behaviour (the write succeeds, the roll-up lands) and the mechanism (the +// parent write is observed carrying `isSystem`). + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +/** + * The caller — a plain member holding `create`/`read`/`edit`/`delete` on the + * CHILD and read-only access to the PARENT. This is `member_default` on + * `showcase_task` vs `showcase_project`, which is where the defect was measured. + */ +const MEMBER = { userId: 'u_member', positions: ['member_default'] } as any; + +/** Grants the stand-in gate enforces. Note: no `update` on the parent. */ +const MEMBER_GRANTS: Record = { + task: ['insert', 'update', 'delete', 'find', 'findOne', 'count', 'aggregate'], + proj: ['find', 'findOne', 'count', 'aggregate'], +}; + +/** + * The refusal the stand-in gate raises, in the ADR-0112 envelope + * `PermissionDeniedError` declares (`code` + `statusCode` 403, + * `plugin-security/src/errors.ts`). Spelled out here rather than imported for + * the dependency-direction reason above; the refusal assertions below read + * `code` AND `statusCode`, so a gate that stopped refusing — or refused with a + * bare `Error` — cannot pass them. + */ +class TestPermissionDeniedError extends Error { + readonly code = 'PERMISSION_DENIED'; + readonly statusCode = 403; + constructor(object: string, operation: string) { + super(`[Security] Access denied: operation '${operation}' on object '${object}' is not permitted`); + this.name = 'PermissionDeniedError'; + } +} + +interface ObservedOp { + object: string; + operation: string; + isSystem: boolean; + userId: unknown; +} + +function makeDriver() { + const stores = new Map>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + // Flat equality plus `$and` — the two shapes `aggregateSummaryValue` and the + // row-filter middleware below actually produce. + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k === '$and') return (v as any[]).every((sub) => matches(row, sub)); + return row?.[k] === v; + }); + }; + let n = 0; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matches(r, ast?.where)); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `r_${n}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const row = { ...s.get(id), ...data, id }; + s.set(id, row); + return row; + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count() { return 0; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, storeFor }; +} + +/** + * @param scopeChildReadsToOwner reproduce a row-level read filter on the child + * (`owner_id == caller`) for NON-system reads — the second failure mode the + * caller-scoped recompute had: the aggregate was computed over the caller's + * VISIBLE subset and stored on the parent as if it were the whole collection. + */ +async function makeEngine(opts: { scopeChildReadsToOwner?: boolean } = {}) { + const engine = new ObjectQL(); + const d = makeDriver(); + engine.registerDriver(d.driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'proj', + fields: { + name: { type: 'text' }, + task_count: { type: 'summary', summaryOperations: { object: 'task', field: 'id', function: 'count' } }, + total_estimate: { type: 'summary', summaryOperations: { object: 'task', field: 'estimate', function: 'sum' } }, + }, + } as any); + engine.registry.registerObject({ + name: 'task', + fields: { + title: { type: 'text' }, + estimate: { type: 'number' }, + owner_id: { type: 'text' }, + proj: { type: 'master_detail', reference: 'proj' }, + }, + } as any); + + const observed: ObservedOp[] = []; + engine.registerMiddleware(async (opCtx: any, next: () => Promise) => { + observed.push({ + object: opCtx.object, + operation: opCtx.operation, + isSystem: opCtx.context?.isSystem === true, + userId: opCtx.context?.userId, + }); + // `security-plugin.ts` L949 — "System operations bypass security". The one + // rule this fix depends on, reproduced verbatim in shape. + if (opCtx.context?.isSystem) return next(); + // No principal at all ⇒ the bare-kernel/setup path; the real plugin gates + // that separately and the fixtures below use it only to seed. + if (!opCtx.context?.userId) return next(); + if (!(MEMBER_GRANTS[opCtx.object] ?? []).includes(opCtx.operation)) { + throw new TestPermissionDeniedError(opCtx.object, opCtx.operation); + } + if (opts.scopeChildReadsToOwner && opCtx.object === 'task' && opCtx.ast) { + const prior = opCtx.ast.where; + opCtx.ast.where = prior + ? { $and: [prior, { owner_id: opCtx.context.userId }] } + : { owner_id: opCtx.context.userId }; + } + return next(); + }); + + return { engine, storeFor: d.storeFor, observed }; +} + +/** Seed a parent plus `existing` sibling tasks owned by SOMEONE ELSE. */ +async function seed(engine: ObjectQL, storeFor: (o: string) => Map, existing: number) { + const proj = await engine.insert('proj', { name: 'P-1' }); + for (let i = 0; i < existing; i++) { + // Seeded directly into the store: these are rows the member did not write + // and (under `scopeChildReadsToOwner`) cannot see. + storeFor('task').set(`seeded_${i}`, { + id: `seeded_${i}`, title: `seeded-${i}`, estimate: 100, proj: proj.id, owner_id: 'u_other', + }); + } + return proj; +} + +/** + * The three call sites, as one contract. Each entry performs a write the member + * IS granted on the child, and declares what the parent's roll-ups must read + * once the recompute has run. + * + * `existing` sibling rows are owned by another user and each carry + * `estimate: 100`, so the expected values below are only reachable if the + * aggregate saw the WHOLE child collection. + */ +const WRITE_PATHS = [ + { + name: 'insert — POST /data/task (the #7673 repro)', + existing: 1, + expected: { task_count: 2, total_estimate: 130 }, + async run(engine: ObjectQL, projId: string) { + return engine.insert( + 'task', + { title: 'probe', estimate: 30, proj: projId, owner_id: MEMBER.userId }, + { context: MEMBER } as any, + ); + }, + }, + { + name: 'update — PATCH /data/task/ (the #7719 half)', + existing: 1, + expected: { task_count: 2, total_estimate: 150 }, + async run(engine: ObjectQL, projId: string) { + const own = await engine.insert( + 'task', + { title: 'own', estimate: 30, proj: projId, owner_id: MEMBER.userId }, + { context: MEMBER } as any, + ); + return engine.update('task', { id: own.id, estimate: 50 }, { context: MEMBER } as any); + }, + }, + { + name: 'delete — DELETE /data/task/', + existing: 1, + expected: { task_count: 1, total_estimate: 100 }, + async run(engine: ObjectQL, projId: string) { + const own = await engine.insert( + 'task', + { title: 'own', estimate: 30, proj: projId, owner_id: MEMBER.userId }, + { context: MEMBER } as any, + ); + return engine.delete('task', { where: { id: own.id }, context: MEMBER } as any); + }, + }, +] as const; + +describe('[#7673] roll-up recompute is system-elevated on every write path', () => { + it.each(WRITE_PATHS)('$name: the granted child write RESOLVES (no ERR_SUMMARY_RECOMPUTE)', async ({ existing, run }) => { + const { engine, storeFor } = await makeEngine(); + const proj = await seed(engine, storeFor, existing); + + // Before #7673 this rejected with `ERR_SUMMARY_RECOMPUTE` — after the row + // had already been written, which is what made a client retry duplicate it. + await expect(run(engine, proj.id)).resolves.toBeDefined(); + }); + + it.each(WRITE_PATHS)('$name: the parent roll-ups actually land the recomputed values', async ({ existing, expected, run }) => { + const { engine, storeFor } = await makeEngine(); + const proj = await seed(engine, storeFor, existing); + + await run(engine, proj.id); + + // The substance, not just the absence of a throw: the recompute RAN and + // wrote the aggregate over the whole child collection. + const stored = storeFor('proj').get(proj.id); + expect(stored.task_count).toBe(expected.task_count); + expect(stored.total_estimate).toBe(expected.total_estimate); + }); + + it.each(WRITE_PATHS)('$name: the parent write is observed carrying `isSystem` (the plugin-security bypass)', async ({ existing, run }) => { + const { engine, storeFor, observed } = await makeEngine(); + const proj = await seed(engine, storeFor, existing); + observed.length = 0; + + await run(engine, proj.id); + + const parentUpdates = observed.filter((o) => o.object === 'proj' && o.operation === 'update'); + expect(parentUpdates.length).toBeGreaterThan(0); + for (const op of parentUpdates) expect(op.isSystem).toBe(true); + // Elevation is a `sudo()`-shaped derivative of the caller's context, not a + // bare `{ isSystem: true }`: identity (and with it `tenantId`, `timezone` + // and any open transaction handle) must still ride along, or the recompute + // silently leaves the caller's transaction and stops being tenant-scoped. + for (const op of parentUpdates) expect(op.userId).toBe(MEMBER.userId); + }); + + it.each(WRITE_PATHS)('$name: the CHILD write itself stays under the CALLER context', async ({ existing, run }) => { + const { engine, storeFor, observed } = await makeEngine(); + const proj = await seed(engine, storeFor, existing); + observed.length = 0; + + await run(engine, proj.id); + + // The elevation is scoped to the recompute. The operation the caller + // actually asked for is still authorized as the caller — otherwise this + // fix would have turned every child write into a system write. + const childWrites = observed.filter( + (o) => o.object === 'task' && ['insert', 'update', 'delete'].includes(o.operation), + ); + expect(childWrites.length).toBeGreaterThan(0); + for (const op of childWrites) expect(op.isSystem).toBe(false); + }); + + it.each(WRITE_PATHS)('$name: the aggregate reads the WHOLE child collection, not the caller-visible subset', async ({ existing, expected, run }) => { + // Same paths, now with a row-level read filter on the child. Under the + // caller's context the aggregate saw only the member's own rows, so the + // parent column was silently rewritten to ONE READER's view of the + // collection — a roll-up is a property of the parent, never of whoever + // happened to touch a child. + const { engine, storeFor } = await makeEngine({ scopeChildReadsToOwner: true }); + const proj = await seed(engine, storeFor, existing); + + await run(engine, proj.id); + + const stored = storeFor('proj').get(proj.id); + expect(stored.task_count).toBe(expected.task_count); + expect(stored.total_estimate).toBe(expected.total_estimate); + }); +}); + +describe('[#7673] the elevation does not widen what the caller may do', () => { + it('a DIRECT caller update of the parent is still refused — 403 PERMISSION_DENIED', async () => { + const { engine, storeFor } = await makeEngine(); + const proj = await seed(engine, storeFor, 0); + + const caught = await engine + .update('proj', { id: proj.id, name: 'renamed by the member' }, { context: MEMBER } as any) + .then(() => null, (e: any) => e); + + expect(caught).toBeTruthy(); + expect(caught.code).toBe('PERMISSION_DENIED'); + expect(caught.statusCode).toBe(403); + // …and the refusal is real: the parent row is untouched. + expect(storeFor('proj').get(proj.id).name).toBe('P-1'); + }); + + it('a caller write to a NON-summary parent field is still refused even alongside a child write', async () => { + const { engine, storeFor } = await makeEngine(); + const proj = await seed(engine, storeFor, 0); + + await engine.insert( + 'task', + { title: 'probe', estimate: 7, proj: proj.id, owner_id: MEMBER.userId }, + { context: MEMBER } as any, + ); + + // The child write moved the DERIVED column and nothing else. + const stored = storeFor('proj').get(proj.id); + expect(stored.total_estimate).toBe(7); + expect(stored.name).toBe('P-1'); + + const caught = await engine + .update('proj', { id: proj.id, name: 'still refused' }, { context: MEMBER } as any) + .then(() => null, (e: any) => e); + expect(caught?.code).toBe('PERMISSION_DENIED'); + expect(caught?.statusCode).toBe(403); + }); + + it('a caller with NO grant on the child is still refused, and writes nothing', async () => { + const { engine, storeFor } = await makeEngine(); + const proj = await seed(engine, storeFor, 0); + const stranger = { userId: 'u_stranger', positions: [] } as any; + + // The stand-in gate is grant-by-object, so narrow the child grant away for + // this one probe by asking for an operation the member never held either. + const caught = await engine + .insert('other_object_without_grant', { x: 1 }, { context: stranger } as any) + .then(() => null, (e: any) => e); + + expect(caught?.code).toBe('PERMISSION_DENIED'); + expect(caught?.statusCode).toBe(403); + expect(storeFor('proj').get(proj.id).task_count).toBe(0); + }); +}); + +describe('[#7673] a child repointed to another parent recomputes BOTH parents, elevated', () => { + it('old and new parent both land, and both writes carry `isSystem`', async () => { + const { engine, storeFor, observed } = await makeEngine(); + const a = await engine.insert('proj', { name: 'A' }); + const b = await engine.insert('proj', { name: 'B' }); + const own = await engine.insert( + 'task', + { title: 'moving', estimate: 9, proj: a.id, owner_id: MEMBER.userId }, + { context: MEMBER } as any, + ); + observed.length = 0; + + await expect( + engine.update('task', { id: own.id, proj: b.id }, { context: MEMBER } as any), + ).resolves.toBeDefined(); + + expect(storeFor('proj').get(a.id).task_count).toBe(0); + expect(storeFor('proj').get(a.id).total_estimate).toBe(0); + expect(storeFor('proj').get(b.id).task_count).toBe(1); + expect(storeFor('proj').get(b.id).total_estimate).toBe(9); + + const parentUpdates = observed.filter((o) => o.object === 'proj' && o.operation === 'update'); + expect(parentUpdates.map((o) => o.isSystem)).not.toContain(false); + expect(parentUpdates.length).toBeGreaterThanOrEqual(2); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 248fd96f9c..9f5481f9b2 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -6106,8 +6106,54 @@ export class ObjectQL implements IObjectQLEngine { * Recompute roll-up `summary` fields on parent records after a child write. * For each affected parent (the FK value on the changed/old child record), it * aggregates the child collection and writes the result onto the parent's - * summary field. Runs in the caller's execution context so it joins the same - * transaction (e.g. the cross-object batch) when one is open. + * summary field. Runs in the caller's execution context — SYSTEM-ELEVATED, + * see below — so it joins the same transaction (e.g. the cross-object batch) + * when one is open. + * + * # Why the recompute is system-elevated (#7673) + * + * A roll-up is ENGINE-DERIVED state on the parent, not a caller write to it. + * The permission decision that matters — may this caller write the CHILD — + * has already been made by the time we get here; whether + * `showcase_project.task_count` may be refreshed is not a question about the + * caller's grant on `showcase_project`. + * + * Passing the caller's own context straight through made it one, and the + * ordinary parent/child shape — a child more widely writable than its parent + * (tasks, line items, comments, time entries) — broke on it in three + * separate ways, all of them fail-open: + * + * 1. **A granted child write returned 500 after the row was written.** The + * parent update raised `PermissionDeniedError`, which this method + * recorded as a failure and the three call sites then threw as + * `SummaryRecomputeError` — mapped to a 500 by REST, on a write that had + * already COMMITTED. A client that retried created a duplicate row + * (#7673 / #7719, measured on `examples/app-showcase`: a plain member + * holding `showcase_task: create + read` 500'd on every POST and PATCH). + * 2. **The aggregate was computed from the caller's VISIBLE subset.** Where + * the caller COULD write the parent, the recompute succeeded and stored + * an RLS-scoped count — a parent column silently rewritten to one + * reader's view of the child collection. A roll-up is a property of the + * parent, never of whoever happened to touch a child. + * 3. **An author-declared `readonly: true` roll-up column was stripped.** + * The write-path read-only strip runs on `!context.isSystem`, and the + * recompute's payload is "caller supplied" from its point of view, so + * the summary silently never landed. + * + * Elevation is a `sudo()`-shaped derivative of the caller's context + * (`{ ...execCtx, isSystem: true }`), NOT a bare `{ isSystem: true }`: the + * open transaction handle, `tenantId` and `timezone` must survive, or the + * recompute leaves the caller's transaction and stops being tenant-scoped. + * That makes this the same posture the roll-up's two OTHER writers already + * hold — `initializeSummaryFields` runs inside the engine's own insert, and + * `backfillSummaryNulls` (#6063) elevates explicitly — so all three writers + * of a summary column now agree about who owns it. + * + * What this does NOT do: it never widens what the caller may read or write. + * The parent's own row stays governed by the caller's grants (a direct + * `update` of the parent is refused exactly as before), the summary field + * stays subject to the parent's FLS on read, and the only value this path can + * move is the one the author DECLARED as a function of the child collection. */ private async recomputeSummaries( childObject: string, @@ -6117,6 +6163,9 @@ export class ObjectQL implements IObjectQLEngine { ): Promise { const descriptors = this.getSummaryDescriptors(childObject); if (descriptors.length === 0) return []; + // The elevation described above. Built once per call, spread from the + // caller's context so transaction / tenant / timezone ride along. + const systemCtx = { ...(execCtx ?? {}), isSystem: true } as ExecutionContext; const recs = Array.isArray(records) ? records : records ? [records] : []; const prevs = Array.isArray(previous) ? previous : previous ? [previous] : []; const failures: SummaryRecomputeFailure[] = []; @@ -6135,8 +6184,8 @@ export class ObjectQL implements IObjectQLEngine { // `aggregateSummaryValue` (#6063). Behaviour unchanged; it simply // lives where the insert-time seed and the one-off NULL backfill // can read the identical computation instead of copying it. - const value = await aggregateSummaryValue(this, desc, parentId, execCtx); - await this.update(desc.parentObject, { id: parentId, [desc.summaryField]: value }, { context: execCtx } as any); + const value = await aggregateSummaryValue(this, desc, parentId, systemCtx); + await this.update(desc.parentObject, { id: parentId, [desc.summaryField]: value }, { context: systemCtx } as any); }, this.summaryRetryOptions); } catch (err) { // Retries exhausted (or a non-transient failure). Record it so the