diff --git a/.changeset/capability-gate-update-verb.md b/.changeset/capability-gate-update-verb.md new file mode 100644 index 0000000000..60b92f8500 --- /dev/null +++ b/.changeset/capability-gate-update-verb.md @@ -0,0 +1,19 @@ +--- +"@objectstack/plugin-audit": patch +--- + +**Behaviour change (tightening):** `enable.files` / `enable.feeds` are now enforced on the **update** verb, not only on insert (#10170). + +Both capability gates in `audit-writers.ts` registered on `beforeInsert` only. `enable.files` says whether `sys_attachment` rows may **target** an object and `enable.feeds` whether `sys_comment` rows may target it — properties of the target object, not of the verb that got a row there — so a re-point via update landed rows the declaration refuses: a caller who could not *create* an attachment on an object without `enable.files: true` could *move* an existing one onto it, and a comment could be re-threaded into a `feeds: false` object's thread. The access kits authorize the re-point (`comment-access-hooks.ts` since #4630, `attachment-access-hooks.ts` since #10091), but those are **access** checks — the capability half was never asked on update. + +What an operator will now observe: + +- An update of `sys_attachment` whose payload sets `parent_object` to an object that does not declare `enable: { files: true }` is refused with **403 `FILES_DISABLED`** — the same envelope the insert path has emitted since #2727 (ADR-0112: `code` + `status`). Fail-closed as on insert: an absent `enable` block, an absent flag, and an unknown parent object all reject. +- An update of `sys_comment` whose payload sets `thread_id` to a thread on an object declaring `enable: { feeds: false }` is refused with **403 `FEEDS_DISABLED`**. Opt-out semantics as on insert: only an explicit `false` rejects, and a missing or free-form `thread_id` is still allowed through — this is capability gating, not access control. +- Both apply on **both dispatch shapes**: a by-id update (`dispatch.mode` `record`) and a predicate `multi: true` update, which is evaluated per matched row (#5574 / ADR-0058 Addendum II). An unscoped predicate update is refused on its first matched row. + +**No existing row is newly refused, and no update that is not a re-point changes.** The gates read the payload: an update that never names `parent_object` / `thread_id` returns on the gate's first line, so renames, body edits, reaction writes and other column updates on a row whose parent object has since had the capability flipped off keep working exactly as before. Only a write that makes a row *newly target* a walled object is refused. + +**Blast radius.** A structural sweep of the 4 660 in-tree source files found **no** caller — none in `packages/` source, `examples/`, or the dogfood apps — that issues an update whose payload names `parent_object`, and none that re-points `thread_id`; in the console the only `sys_attachment` write is a create, and the only `sys_comment` update writes `reactions`. If you have your own "move this attachment" or "move this comment" flow, point it at a target object that declares the capability, or declare it on the target. + +No new error code: both codes are existing standard-catalog members already registered in `packages/spec/src/api/error-code-ledger.zod.ts` and already mapped to 403 by `packages/rest/src/error-response.ts`. diff --git a/packages/plugins/plugin-audit/src/audit-hook-object-scope.test.ts b/packages/plugins/plugin-audit/src/audit-hook-object-scope.test.ts index cbc40440c3..7cdc231b78 100644 --- a/packages/plugins/plugin-audit/src/audit-hook-object-scope.test.ts +++ b/packages/plugins/plugin-audit/src/audit-hook-object-scope.test.ts @@ -437,7 +437,7 @@ function makeRecordingEngine() { const AUDIT_WRITER_EVENTS = ['afterInsert', 'afterUpdate', 'afterDelete']; describe('[#5860] the skip list is declared on the registration face', () => { - it('plugin-audit declares NO `beforeUpdate` / `beforeDelete` hook (#6656)', () => { + it('plugin-audit declares NO GLOBAL `beforeUpdate` / `beforeDelete` hook (#6656)', () => { const { engine, registrations } = makeRecordingEngine(); installAuditWriters(engine); @@ -445,16 +445,60 @@ describe('[#5860] the skip list is declared on the registration face', () => { // from a read count — this is the face `hasHooksFor` reads, so it is what // decides whether the engine's per-row bulk dispatch runs at all. // - // Scoped to the two events `captureBefore` held. The plugin's OTHER - // before-phase registrations are unrelated capability gates on a single - // named object each (`beforeInsert` on `sys_comment` for `enable.feeds`, - // on `sys_attachment` for `enable.files`); they read no prior row, and - // asserting "no before-phase hook at all" would fail on them while - // measuring nothing about this card. - const preImageEvents = registrations - .map((r) => r.event) - .filter((e) => e === 'beforeUpdate' || e === 'beforeDelete'); - expect(preImageEvents).toEqual([]); + // [#10170] The filter is on GLOBAL registrations, not on the event names. + // It used to be on the event names, and the case above already recorded + // why that was only ever a PROXY: the plugin's capability gates are + // "unrelated … on a single named object each", they "read no prior row", + // and an assertion that caught them "would fail on them while measuring + // nothing about this card". While those gates were insert-only, filtering + // by event name expressed that carve-out exactly. #10170 registers them on + // `beforeUpdate` too — `enable.files`/`enable.feeds` are properties of the + // TARGET object, so a re-point via update is inside the declaration — and + // the proxy stopped tracking the property. + // + // What #6656 retired was `captureBefore`: an UNSCOPED pre-image reader + // that made `hasHooksFor(, 'beforeUpdate')` true system-wide + // and bought a prior-row read on every update in the stack. That is the + // invariant, and it is what this now asserts. An object-SCOPED gate costs + // the demand gate nothing beyond its own object — and on these two + // objects nothing at all: `comment-access-hooks.ts` (#4630) and + // service-storage's `attachment-access-hooks.ts` (#10091) already declare + // `beforeUpdate` scoped to `sys_comment` / `sys_attachment`, so + // `hasHooksFor` is already true for both wherever the access kits install. + const globalPreImage = registrations + .filter((r) => r.event === 'beforeUpdate' || r.event === 'beforeDelete') + .filter((r) => r.options?.object === undefined) + .map((r) => r.event); + expect(globalPreImage).toEqual([]); + }); + + it('[#10170] the two capability gates are declared on insert AND update, each scoped to one object', () => { + // The other half of the case above: the reason a `beforeUpdate` + // registration is admissible here is that it names ONE object. Assert that + // rather than leaving it to the negative filter — a future gate that + // forgot its `object` scope would otherwise only be caught by the absence + // test above, which reads as "nothing was retired", not "a gate went + // global". + const { engine, registrations } = makeRecordingEngine(); + installAuditWriters(engine); + + // BEFORE-phase only: `sys_comment` also carries the M10.8 @mention + // notification hook on `afterInsert`, which is not a capability gate. + const gateEvents = (object: string) => + registrations + .filter((r) => r.options?.object === object && r.event.startsWith('before')) + .map((r) => r.event) + .sort(); + + expect(gateEvents('sys_comment')).toEqual(['beforeInsert', 'beforeUpdate']); + expect(gateEvents('sys_attachment')).toEqual(['beforeInsert', 'beforeUpdate']); + + // …and neither of them widened into the global allow half. + for (const r of registrations) { + if (r.options?.object === 'sys_comment' || r.options?.object === 'sys_attachment') { + expect(r.options?.excludeObjects).toBeUndefined(); + } + } }); it('all writer registrations carry `excludeObjects` and stay global otherwise', () => { diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index d76789ed95..f7a8885d64 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -1567,6 +1567,21 @@ export function installAuditWriters( * unconventional thread_id is allowed through: this is capability * gating, not access control, and free-form threads have no object to * gate on. + * + * [#10170] Registered on `beforeUpdate` as well as `beforeInsert`, because + * the flag is a property of the TARGET OBJECT — "does this object allow + * comments at all" — and not of the verb that made a row target it. On + * insert only, a caller who could not *create* a comment on a + * `feeds: false` object could *re-thread* an existing one into it, and the + * row landed. `comment-access-hooks.ts` authorizes that re-point (#4630: + * the new `thread_id`'s parent must be readable) — but that is an ACCESS + * check; the capability half was never asked on the update verb. + * + * On update, an ABSENT `thread_id` means "not a re-thread", and the same + * first line returns. That is what keeps an ordinary body/reaction edit on + * an existing row working after its object's `enable.feeds` is flipped off: + * the narrowing reaches re-points, not every later write to a grandfathered + * row. */ const enforceFeedsCapability = async (ctx: HookContext) => { const data: any = (ctx.input as any)?.data; @@ -1585,6 +1600,7 @@ export function installAuditWriters( } }; engine.registerHook('beforeInsert', enforceFeedsCapability, { object: 'sys_comment', packageId }); + engine.registerHook('beforeUpdate', enforceFeedsCapability, { object: 'sys_comment', packageId }); /** * `enable.files` server-side enforcement (#2727). The generic Attachments @@ -1600,11 +1616,26 @@ export function installAuditWriters( * store the file URL in the record's own column via service-storage and * never create a sys_attachment row, so field-level attachments keep * working regardless of this flag. + * + * [#10170] Registered on `beforeUpdate` as well, for the feeds gate's + * reason one object over: `enable.files` says whether attachments may + * TARGET this object, so a re-point that makes a row target it is inside + * the declaration whether or not a creation happened. On insert only, a + * caller barred from *creating* an attachment on a `files: false` object + * could *move* an existing one onto it. `attachment-access-hooks.ts` + * authorizes the re-point (#10091: the new `parent_object`/`parent_id` + * must be editable) — access, again, not capability. */ const enforceFilesCapability = async (ctx: HookContext) => { const data: any = (ctx.input as any)?.data; const parentObject = data?.parent_object; - if (typeof parentObject !== 'string' || parentObject.length === 0) return; // schema requires it; let validation report the miss + // Two meanings, one line. On INSERT an absent `parent_object` is a + // schema violation — left to validation to report, so the gate never + // shadows the real diagnostic. On UPDATE (#10170) it means "this write is + // not a re-point", so there is no new target to ask about and the row's + // existing parent was already gated when it was created. Either way the + // gate has nothing to say. + if (typeof parentObject !== 'string' || parentObject.length === 0) return; const def = getObjectDef(parentObject); if (def?.enable?.files !== true) { const err: any = new Error(`File attachments are not enabled for object '${parentObject}' (requires enable.files: true)`); @@ -1615,6 +1646,29 @@ export function installAuditWriters( } }; engine.registerHook('beforeInsert', enforceFilesCapability, { object: 'sys_attachment', packageId }); + engine.registerHook('beforeUpdate', enforceFilesCapability, { object: 'sys_attachment', packageId }); + + /* + * [#10170] Why neither `beforeUpdate` registration above declares + * `dispatchUnscopedMultiWrite` (#9719, widened to `beforeUpdate` by #9974). + * + * That flag buys ONE extra dispatch, with the whole-operation context and + * before any matched row is resolved, for guards that refuse an operation + * SHAPE — "a `multi: true` update with no `where` at all". These two are not + * shape guards: they read the PAYLOAD, which the per-row fan-out delivers + * verbatim to every matched row (#5574 / ADR-0058 Addendum II D1–D2 builds a + * fresh context per row per phase, carrying the same payload object). So an + * unscoped multi update that re-points onto a walled parent is already + * refused on the first matched row, without the flag — pinned in + * `capability-gate-update-verb.test.ts`. + * + * Declaring it would narrow further than the declaration justifies: the one + * case it would ADD is a ZERO-MATCH unscoped write, where nothing is written + * and therefore nothing ever comes to target the walled object. Refusing + * that is an operation-shape policy — the #4757 `sys_attachment` and #4630 + * `sys_comment` guards' territory, declared on their own registrations — not + * the capability opt-in this card restores. + */ /** * M10.8: Dedicated hook on `sys_comment` afterInsert that parses the diff --git a/packages/plugins/plugin-audit/src/capability-gate-update-verb.test.ts b/packages/plugins/plugin-audit/src/capability-gate-update-verb.test.ts new file mode 100644 index 0000000000..aac0f31bed --- /dev/null +++ b/packages/plugins/plugin-audit/src/capability-gate-update-verb.test.ts @@ -0,0 +1,406 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10170] `enable.files` / `enable.feeds` are properties of the TARGET + * object, so the verb that made a row target it does not change the answer. + * + * Both capability gates in `audit-writers.ts` registered on `beforeInsert` + * only, which left a re-point via UPDATE outside the declaration: a caller who + * could not *create* a `sys_attachment` on a `files: false` object could + * *move* an existing one onto it, and a `sys_comment` could be re-threaded + * into a feeds-disabled object's thread. The access kits authorize the + * re-point (`comment-access-hooks.ts` since #4630, + * `attachment-access-hooks.ts` since #10091) — those are ACCESS checks, and + * the capability half was never asked on the update verb. + * + * This file runs against a REAL `ObjectQL` (a stub driver underneath), not the + * hand-rolled fake in `audit-writers.test.ts`, for two reasons the fake cannot + * serve: + * + * 1. the fake's `registerHook` ignores the `{ object }` scoping option, so a + * registration's OBJECT SCOPE is unobservable there — and this change is + * a registration change; + * 2. the gap has to be pinned on BOTH dispatch shapes, and "by-id" vs + * "predicate/per-row" is an engine behaviour (#5574 / ADR-0058 Addendum + * II D1–D2 builds a fresh context per matched row per phase). Only the + * real engine fans out. + * + * Every rejection asserts the ADR-0112 envelope — `code` AND `status`, never + * one alone — and every direction is pinned twice: the disabled parent must be + * REFUSED and the enabled parent must still SUCCEED. A one-directional pin + * passes just as well on a gate that refuses everything. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import type { EngineQueryOptions, EngineUpdateOptions } from '@objectstack/spec/data'; +import { installAuditWriters } from './audit-writers.js'; + +const text = (name: string, primaryKey = false) => ({ + name, + label: name, + type: 'text' as const, + ...(primaryKey ? { primaryKey: true } : {}), +}); + +const fieldMap = (...names: string[]) => + Object.fromEntries(names.map((n) => [n, text(n, n === 'id')])); + +/** A stub driver with just enough storage for the write paths under test. */ +function makeStubDriver(): any { + const stores = new Map>>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { + s = new Map(); + stores.set(o, s); + } + return s; + }; + let nextId = 0; + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (expected ?? null)) return false; + } + return true; + }; + const d: any = { + name: 'memory', + version: '0.0.0', + supports: {}, + async connect() {}, + async disconnect() {}, + async checkHealth() { + return true; + }, + async execute() { + return null; + }, + async syncSchema() {}, + async find(o: string, ast: any) { + return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + }, + async findOne(o: string, ast: any) { + for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(o: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(o).set(id, row); + return row; + }, + async update(o: string, id: string, data: Record) { + const s = storeFor(o); + const cur = s.get(id); + if (!cur) return null; + const u = { ...cur, ...data, id }; + s.set(id, u); + return u; + }, + // `driver.updateMany(object, AST, data, options)` — the second argument is + // the compiled AST, not a bare `where`. Stubbing it as `(o, where, data)` + // silently matches nothing and resolves 0, which reads exactly like "the + // predicate write was accepted and touched no row" — a vacuous pass on the + // very path this file exists to measure. + async updateMany(o: string, ast: any, data: Record) { + const s = storeFor(o); + let n = 0; + for (const [id, row] of s) { + if (!matches(row, ast?.where)) continue; + s.set(id, { ...row, ...data, id }); + n += 1; + } + return n; + }, + async delete() { + return true; + }, + async count(o: string, ast: any) { + return (await d.find(o, ast)).length; + }, + async aggregate() { + return []; + }, + }; + return d; +} + +/** + * Boot a real engine carrying the two gated objects, an attachments-enabled + * parent, an attachments-disabled parent, and the audit sinks (registered so + * the `afterUpdate` audit writer has somewhere real to land — its failures are + * swallowed by design, and a swallowed failure would make this harness lie + * about which write actually happened). + */ +async function boot() { + const engine = new ObjectQL(); + const driver = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + + const reg = engine.registry; + reg.registerObject({ + name: 'lead_open', + label: 'Lead (capabilities on)', + fields: fieldMap('id', 'name'), + enable: { files: true, feeds: true }, + } as any, 'test.fixture'); + reg.registerObject({ + name: 'lead_walled', + label: 'Lead (capabilities off)', + fields: fieldMap('id', 'name'), + // `files` is opt-IN (spec default false) and `feeds` opt-OUT (default + // true), so the walled object has to say `feeds: false` explicitly while + // merely NOT saying `files: true` already walls attachments off. Both are + // spelled out here so the fixture reads as one "capabilities off" object. + enable: { files: false, feeds: false }, + } as any, 'test.fixture'); + reg.registerObject({ + name: 'sys_attachment', + label: 'Attachment', + fields: fieldMap('id', 'parent_object', 'parent_id', 'file_id', 'file_name'), + } as any, 'test.fixture'); + reg.registerObject({ + name: 'sys_comment', + label: 'Comment', + fields: fieldMap('id', 'thread_id', 'body'), + } as any, 'test.fixture'); + reg.registerObject({ + name: 'sys_audit_log', + label: 'Audit Log', + fields: fieldMap( + 'id', 'action', 'user_id', 'actor', 'object_name', 'record_id', 'old_value', 'new_value', 'tenant_id', + ), + } as any, 'test.fixture'); + reg.registerObject({ + name: 'sys_activity', + label: 'Activity', + fields: fieldMap( + 'id', 'type', 'timestamp', 'summary', 'actor_id', 'object_name', 'record_id', 'record_label', 'metadata', + ), + } as any, 'test.fixture'); + + installAuditWriters(engine as any, 'test.audit'); + return engine; +} + +const seedAttachment = async (engine: ObjectQL, parent = 'lead_open', fileId = 'file-1') => + (await engine.insert('sys_attachment', { + parent_object: parent, + parent_id: 'rec-1', + file_id: fileId, + } as any)) as any; + +const seedComment = async (engine: ObjectQL, thread = 'lead_open:rec-1') => + (await engine.insert('sys_comment', { thread_id: thread, body: 'hello' } as any)) as any; + +/* ──────────────────────────────────────────────────────────────────────────── + * The CONTROL — proves the harness is wired before anything is read from it. + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#10170] control: the insert-verb gates are live in this harness', () => { + it('refuses the identical shape on INSERT — files and feeds alike', async () => { + const engine = await boot(); + + await expect( + engine.insert('sys_attachment', { + parent_object: 'lead_walled', + parent_id: 'rec-1', + file_id: 'file-1', + } as any), + ).rejects.toMatchObject({ code: 'FILES_DISABLED', status: 403, object: 'lead_walled' }); + + await expect( + engine.insert('sys_comment', { thread_id: 'lead_walled:rec-1', body: 'hi' } as any), + ).rejects.toMatchObject({ code: 'FEEDS_DISABLED', status: 403, object: 'lead_walled' }); + + // …and the enabled parent is accepted, so the control is two-directional + // too: `enable` really round-trips through the registry, and the gates are + // reading it rather than refusing everything. + await expect(seedAttachment(engine)).resolves.toBeTruthy(); + await expect(seedComment(engine)).resolves.toBeTruthy(); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * `enable.files` — the re-point via UPDATE, on both dispatch shapes. + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#10170] enable.files is asked on the UPDATE verb too', () => { + it('by-id (dispatch.mode "record"): re-point onto a files-disabled parent → 403 FILES_DISABLED', async () => { + const engine = await boot(); + const row = await seedAttachment(engine); + + await expect( + engine.update('sys_attachment', { parent_object: 'lead_walled' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).rejects.toMatchObject({ code: 'FILES_DISABLED', status: 403, object: 'lead_walled' }); + + // Refused BEFORE the statement: the stored row still names its old parent. + const after: any = await engine.findOne('sys_attachment', { where: { id: String(row.id) } } satisfies EngineQueryOptions); + expect(after.parent_object).toBe('lead_open'); + }); + + it('by-id: a re-point onto a files-ENABLED parent still succeeds', async () => { + const engine = await boot(); + const row = await seedAttachment(engine); + + await expect( + engine.update('sys_attachment', { parent_object: 'lead_open', parent_id: 'rec-2' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + + const after: any = await engine.findOne('sys_attachment', { where: { id: String(row.id) } } satisfies EngineQueryOptions); + expect(after.parent_id).toBe('rec-2'); + }); + + it('predicate (dispatch.mode "per-row"): re-point onto a files-disabled parent → 403 FILES_DISABLED', async () => { + const engine = await boot(); + await seedAttachment(engine, 'lead_open', 'file-1'); + await seedAttachment(engine, 'lead_open', 'file-1'); + + await expect( + engine.update('sys_attachment', { parent_object: 'lead_walled' } as any, { + multi: true, + where: { file_id: 'file-1' }, + } satisfies EngineUpdateOptions), + ).rejects.toMatchObject({ code: 'FILES_DISABLED', status: 403, object: 'lead_walled' }); + + const rows: any[] = await engine.find('sys_attachment', { where: {} } satisfies EngineQueryOptions); + expect(rows.map((r) => r.parent_object)).toEqual(['lead_open', 'lead_open']); + }); + + it('predicate: a re-point onto a files-ENABLED parent still succeeds for every matched row', async () => { + const engine = await boot(); + await seedAttachment(engine, 'lead_open', 'file-1'); + await seedAttachment(engine, 'lead_open', 'file-1'); + + await expect( + engine.update('sys_attachment', { parent_id: 'rec-9' } as any, { + multi: true, + where: { file_id: 'file-1' }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + + const rows: any[] = await engine.find('sys_attachment', { where: {} } satisfies EngineQueryOptions); + expect(rows.map((r) => r.parent_id)).toEqual(['rec-9', 'rec-9']); + }); + + it('an UNSCOPED predicate write is refused on its first matched row — no dispatchUnscopedMultiWrite needed', async () => { + // Pins the reasoning the registration block states: these gates read the + // PAYLOAD, and the per-row fan-out delivers it to every matched row, so the + // #9719/#9974 whole-operation dispatch buys nothing here. The only case it + // would add is a ZERO-MATCH unscoped write — where nothing is written, so + // nothing ever comes to target the walled object. + const engine = await boot(); + await seedAttachment(engine, 'lead_open', 'file-1'); + await seedAttachment(engine, 'lead_open', 'file-1'); + + await expect( + engine.update('sys_attachment', { parent_object: 'lead_walled' } as any, { multi: true } satisfies EngineUpdateOptions), + ).rejects.toMatchObject({ code: 'FILES_DISABLED', status: 403, object: 'lead_walled' }); + + const rows: any[] = await engine.find('sys_attachment', { where: {} } satisfies EngineQueryOptions); + expect(rows.map((r) => r.parent_object)).toEqual(['lead_open', 'lead_open']); + }); + + it('an update that does not carry parent_object is not re-checked (an unchanged parent is not a re-point)', async () => { + // The gate reads the PAYLOAD, so an update that never names `parent_object` + // asks nothing — which is what keeps a rename/metadata edit on an existing + // row working after its parent object's `enable.files` is flipped off. + // Without this, the narrowing would reach every later write to a + // grandfathered row, not just the re-points the card is about. + const engine = await boot(); + const row = await seedAttachment(engine, 'lead_open'); + + await expect( + engine.update('sys_attachment', { file_name: 'renamed.pdf' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + await expect( + engine.update('sys_attachment', { file_name: 'bulk.pdf' } as any, { + multi: true, + where: { file_id: 'file-1' }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * `enable.feeds` — the same two shapes on the comment thread re-point. + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#10170] enable.feeds is asked on the UPDATE verb too', () => { + it('by-id: re-threading into a feeds-disabled object → 403 FEEDS_DISABLED', async () => { + const engine = await boot(); + const row = await seedComment(engine); + + await expect( + engine.update('sys_comment', { thread_id: 'lead_walled:rec-1' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).rejects.toMatchObject({ code: 'FEEDS_DISABLED', status: 403, object: 'lead_walled' }); + + const after: any = await engine.findOne('sys_comment', { where: { id: String(row.id) } } satisfies EngineQueryOptions); + expect(after.thread_id).toBe('lead_open:rec-1'); + }); + + it('by-id: re-threading into a feeds-ENABLED object still succeeds', async () => { + const engine = await boot(); + const row = await seedComment(engine); + + await expect( + engine.update('sys_comment', { thread_id: 'lead_open:rec-2' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + }); + + it('predicate: re-threading into a feeds-disabled object → 403 FEEDS_DISABLED', async () => { + const engine = await boot(); + await seedComment(engine); + await seedComment(engine); + + await expect( + engine.update('sys_comment', { thread_id: 'lead_walled:rec-1' } as any, { + multi: true, + where: { body: 'hello' }, + } satisfies EngineUpdateOptions), + ).rejects.toMatchObject({ code: 'FEEDS_DISABLED', status: 403, object: 'lead_walled' }); + }); + + it('predicate: re-threading into a feeds-ENABLED object still succeeds', async () => { + const engine = await boot(); + await seedComment(engine); + await seedComment(engine); + + await expect( + engine.update('sys_comment', { thread_id: 'lead_open:rec-2' } as any, { + multi: true, + where: { body: 'hello' }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + }); + + it('a free-form or absent thread_id stays allowed — capability gating, not access control', async () => { + const engine = await boot(); + const row = await seedComment(engine); + + await expect( + engine.update('sys_comment', { body: 'edited' } as any, { where: { id: String(row.id) } } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + await expect( + engine.update('sys_comment', { thread_id: 'free-form-thread' } as any, { + where: { id: String(row.id) }, + } satisfies EngineUpdateOptions), + ).resolves.toBeTruthy(); + }); +});