From 68ae2c489b9b5ad420d77265761a02e9edf94cf5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 02:58:11 +0000 Subject: [PATCH 1/2] feat(objectql): report stored lookup references that resolve to nothing (#4551) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4441 made the write path refuse an unresolvable reference id but deliberately exempted `isSystem` writes: seed replay, package install and boot provisioning legitimately write in an order that only resolves once the batch completes, and failing them closed turns an ordering detail into a boot failure. That exemption is correct and is untouched here. What it left behind is a residual the PR recorded rather than accepted — the platform itself can still write a reference into the void, and nothing says so. This is the "something says so": a read-only audit that walks stored rows and reports every non-`readonly` reference value naming no row of its declared target. It rides the existing LifecycleService sweep clock so the finding surfaces without an operator knowing to go looking (the same argument that put #4469's `inspectStrandedRequests` on the approvals SLA clock). Judgments are #4441's own, not new ones: `readonly` fields are skipped (their values are platform-minted — `sys_metadata_history.recorded_by` holds the sentinel string `actor ?? 'system'`), empty values are not references, and which fields are references is `referenceTargetOf` — the single arbiter the write-path guard and the expand gate already share. The existence oracle is the engine's own `referenceExists`, passed in as a port, so the audit and the enforcement answer "does this id exist" with one predicate and cannot drift apart. Three properties the report is built around: - It NEVER rewrites. The rows were genuinely written; auto-nulling would make stored data disagree with what happened, and the remedy is an operator's call. - Unknown ≠ absent. A probe that cannot run counts as `undetermined`, an unlistable object lands in `unreadableObjects`, and a budget-bounded scan names the object in `truncatedObjects` — so `dangling: []` can never be misread as "everything is fine". - RBAC link tables are scanned first, derived from PLATFORM_OBJECTS_BY_PACKAGE rather than hand-listed: a dangling row there is a security-surface record resolving to nothing, and the audience-anchor gate must resolve exactly that permission set to evaluate the grant. No spec key, no authorable surface, and no change to #4441's enforcement. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- .changeset/tidy-eyes-shine.md | 38 ++ .../engine-dangling-reference-audit.test.ts | 241 +++++++++++ packages/objectql/src/engine.ts | 39 ++ packages/objectql/src/index.ts | 16 + .../dangling-reference-audit.test.ts | 374 ++++++++++++++++++ .../src/integrity/dangling-reference-audit.ts | 298 ++++++++++++++ .../src/lifecycle/lifecycle-service.test.ts | 77 ++++ .../src/lifecycle/lifecycle-service.ts | 63 +++ packages/objectql/src/plugin.ts | 5 + 9 files changed, 1151 insertions(+) create mode 100644 .changeset/tidy-eyes-shine.md create mode 100644 packages/objectql/src/engine-dangling-reference-audit.test.ts create mode 100644 packages/objectql/src/integrity/dangling-reference-audit.test.ts create mode 100644 packages/objectql/src/integrity/dangling-reference-audit.ts diff --git a/.changeset/tidy-eyes-shine.md b/.changeset/tidy-eyes-shine.md new file mode 100644 index 0000000000..c0ec079440 --- /dev/null +++ b/.changeset/tidy-eyes-shine.md @@ -0,0 +1,38 @@ +--- +'@objectstack/objectql': minor +--- + +Report stored `lookup` references that resolve to nothing (#4551) + +#4441 made the write path refuse an unresolvable reference id, but deliberately +exempted `isSystem` writes so seed replay, package install and boot-time +provisioning keep their ordering freedom. That exemption is unchanged — and it +left a residual: the platform itself could still write a reference into the void +with nothing saying so. + +New: `ObjectQL.inspectDanglingReferences()` — a **read-only** audit that walks +stored rows and reports every non-`readonly` `lookup` / `master_detail` / +`user` / `tree` value that names no row of its declared target. It runs as a leg +of the existing `LifecycleService` sweep, so the finding surfaces without an +operator knowing to go looking for it. + +- **It never rewrites.** The rows were genuinely written; auto-nulling a + dangling id would make the stored data disagree with what happened, and the + remedy (re-seed the target vs. clear the link) is an operator's call. +- **Unknown is not absent.** A probe that cannot run (target unregistered, no + driver, probe throws) counts as `undetermined`; an object whose rows cannot be + listed lands in `unreadableObjects`; a run that hits its row budget names the + object in `truncatedObjects`. So `dangling: []` can never be misread as + "everything is fine". +- **RBAC link tables are scanned first** (`sys_position_permission_set` and the + rest of `plugin-security`'s tables, derived from `PLATFORM_OBJECTS_BY_PACKAGE`): + a dangling row there is a security-surface record resolving to nothing, and + the audience-anchor gate must resolve exactly that permission set to evaluate + the grant. + +The existence oracle is the engine's own — the same predicate #4441's write-path +guard uses — so the report can never be stricter or looser than the rule it +reports on. + +Tuning: `ObjectQLPlugin`'s `lifecycle.referenceAudit` (`enabled`, `rowsPerObject`, +`maxRows`, `objects`). Nothing is authorable in metadata; no spec key was added. diff --git a/packages/objectql/src/engine-dangling-reference-audit.test.ts b/packages/objectql/src/engine-dangling-reference-audit.test.ts new file mode 100644 index 0000000000..f125349641 --- /dev/null +++ b/packages/objectql/src/engine-dangling-reference-audit.test.ts @@ -0,0 +1,241 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#4551] The audit against the REAL engine — end to end on the exact residual + * #4441 documented. + * + * #4441's write-path guard exempts `isSystem` writes on purpose: seed replay, + * package install and boot provisioning write in an order that only resolves + * once the batch completes, and failing them closed turns an ordering detail + * into a boot failure. That exemption stays. What this pins is the other half — + * the row a system write leaves behind is now SAID OUT LOUD. + * + * The two halves must also stay in agreement, which is what makes this file + * worth having on top of the unit suite (#4550: a stand-in must never be looser + * than the real implementation). Here the audit runs on the real `ObjectQL` + * with the real driver and the real `referenceExists`, so if the enforcement's + * probe and the audit's probe ever diverge, these tests are where it shows. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const permissionSet = { + name: 'aud_permission_set', + label: 'Permission Set', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + }, +}; + +/** The RBAC link-table shape: the binding an audience gate must resolve. */ +const binding = { + name: 'aud_position_permission_set', + label: 'Binding', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + permission_set_id: { + name: 'permission_set_id', label: 'Permission Set', + type: 'lookup' as const, reference: 'aud_permission_set', + required: true, deleteBehavior: 'set_null' as const, + }, + }, +}; + +const history = { + name: 'aud_history', + label: 'History', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + note: { name: 'note', label: 'Note', type: 'text' as const }, + // `sys_metadata_history.recorded_by` in miniature: a readonly lookup the + // platform fills with the sentinel string `actor ?? 'system'`. + recorded_by: { + name: 'recorded_by', label: 'Recorded By', + type: 'lookup' as const, reference: 'aud_permission_set', readonly: true, + }, + }, +}; + +function makeMemoryDriver() { + const stores = new Map>>(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + /** Read path: must NOT materialise a store, or a pure read would show up in + * the "nothing was written" snapshot as a change. */ + const peek = (obj: string) => stores.get(obj) ?? new Map>(); + 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 driver: any = { + name: 'memory', version: '0.0.0', supports: {} as any, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, + async find(object: string, ast: any) { + const rows = Array.from(peek(object).values()).filter((r) => matches(r, ast?.where)); + // A real driver honours `limit`; a double that ignored it would make the + // audit's bounded-scan reporting untestable AND looser than production. + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + findStream() { throw new Error('not implemented'); }, + async findOne(object: string, ast: any) { + for (const r of peek(object).values()) if (matches(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) return null; + const next = { ...cur, ...data, id }; + s.set(id, next); + return next; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { return storeFor(object).delete(id); }, + async count(object: string, ast: any) { return (await this.find(object, ast)).length; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { return []; }, + async bulkDelete() {}, + async updateMany(object: string, ast: any, data: Record) { + const rows = await this.find(object, ast); + for (const r of rows) storeFor(object).set(r.id as string, { ...r, ...data, id: r.id }); + return rows.length; + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, stores }; +} + +describe('[#4551] the engine reports the dangling rows its own `isSystem` exemption allows', () => { + let engine: ObjectQL; + let stores: Map>>; + const userCtx = { userId: 'u1' }; + + beforeEach(async () => { + engine = new ObjectQL(); + const mem = makeMemoryDriver(); + stores = mem.stores; + engine.registerDriver(mem.driver, true); + await engine.init(); + engine.registry.registerObject(permissionSet as any); + engine.registry.registerObject(binding as any); + engine.registry.registerObject(history as any); + await engine.insert('aud_permission_set', { id: 'ps_real', name: 'Real' }, { context: { isSystem: true } } as any); + }); + + it('the residual, stated: a system write lands a dangling binding and the audit names it', async () => { + // This is the write #4441 deliberately lets through. + await engine.insert( + 'aud_position_permission_set', + { id: 'ppr_1', permission_set_id: 'ps_never_seeded' }, + { context: { isSystem: true } } as any, + ); + + const out = await engine.inspectDanglingReferences({ objects: ['aud_position_permission_set'] }); + + expect(out.undetermined).toBe(0); + expect(out.dangling).toHaveLength(1); + expect(out.dangling[0]).toEqual({ + objectName: 'aud_position_permission_set', + recordId: 'ppr_1', + field: 'permission_set_id', + target: 'aud_permission_set', + value: 'ps_never_seeded', + }); + }); + + it('…and the enforcement it reports on is untouched — a caller write is still refused', async () => { + // #4551 is a REPORT. If this ever passes, the audit was smuggled into the + // write path, which the issue explicitly forbids. + await expect( + engine.insert( + 'aud_position_permission_set', + { permission_set_id: 'ps_never_seeded' }, + { context: userCtx } as any, + ), + ).rejects.toMatchObject({ name: 'ValidationError' }); + }); + + it('a resolvable binding is not reported', async () => { + await engine.insert( + 'aud_position_permission_set', + { id: 'ppr_ok', permission_set_id: 'ps_real' }, + { context: { isSystem: true } } as any, + ); + const out = await engine.inspectDanglingReferences({ objects: ['aud_position_permission_set'] }); + expect(out.dangling).toEqual([]); + expect(out.scanned).toBe(1); + }); + + it('the audit issues NO writes — the stored rows are byte-identical afterwards', async () => { + await engine.insert( + 'aud_position_permission_set', + { id: 'ppr_1', permission_set_id: 'ps_never_seeded' }, + { context: { isSystem: true } } as any, + ); + const snapshot = (): string => + JSON.stringify([...stores].map(([k, v]) => [k, [...v.entries()]])); + const before = snapshot(); + + const out = await engine.inspectDanglingReferences(); + + expect(out.dangling.length).toBeGreaterThan(0); + expect(snapshot()).toBe(before); + }); + + it('a readonly lookup holding a SENTINEL string is not reported', async () => { + // `recorded_by: 'system'` is not a user id and never was. #4441 skips it on + // the write path; the audit must not undo that by reporting the same value + // from the other side. + await engine.insert( + 'aud_history', { id: 'h1', note: 'n', recorded_by: 'system' }, { context: { isSystem: true } } as any, + ); + const out = await engine.inspectDanglingReferences({ objects: ['aud_history'] }); + expect(out.dangling).toEqual([]); + expect(out.undetermined).toBe(0); + }); + + it('an unregistered TARGET is `undetermined`, not a finding', async () => { + // Exactly the case `referenceExists` answers `null` for — and the audit and + // the write-path guard read that `null` the same way: the write is allowed + // through, and the audit declines to condemn the row it produced. + engine.registry.registerObject({ + name: 'aud_orphan', + label: 'Orphan', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + other: { name: 'other', label: 'Other', type: 'lookup' as const, reference: 'not_registered_anywhere' }, + }, + } as any); + await engine.insert('aud_orphan', { id: 'o1', other: 'whatever' }, { context: userCtx } as any); + + const out = await engine.inspectDanglingReferences({ objects: ['aud_orphan'] }); + expect(out.dangling).toEqual([]); + expect(out.undetermined).toBe(1); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index ceb64f6041..694cb6171e 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -80,6 +80,12 @@ import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, Validat import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; import { applyHaving } from './having-filter.js'; +import { + auditDanglingReferences, + type AuditableObject, + type DanglingReferenceAuditOptions, + type DanglingReferenceReport, +} from './integrity/dangling-reference-audit.js'; /** * The lifecycle events the engine actually dispatches via `triggerHooks`. This @@ -2106,6 +2112,39 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * [#4551] Report stored references that resolve to nothing. **Read-only** — + * this issues no writes at all. + * + * The follow-up to {@link assertReferencesResolve}'s deliberate `isSystem` + * exemption. That exemption stays exactly as #4441 wrote it (seed replay and + * boot provisioning must keep their ordering freedom); what it left behind is + * a residual — the platform itself can still write a reference into the void + * and nothing says so. This is the "something says so". + * + * The existence oracle passed to the audit is **this engine's own** + * {@link referenceExists}, not a second copy: the audit and the write-path + * guard therefore answer "does this id exist" — and "could I even tell?" — + * with one predicate, so the report can never be more or less strict than the + * rule it reports on. + * + * See {@link auditDanglingReferences} for the judgments (readonly skip, empty + * values, unknown ≠ absent) and the bounded-scan honesty of the report. + */ + async inspectDanglingReferences( + options?: DanglingReferenceAuditOptions, + ): Promise { + return auditDanglingReferences( + { + objects: () => this._registry.getAllObjects() as unknown as AuditableObject[], + find: (object, opts) => this.find(object, opts as any) as Promise>>, + probe: (target, id) => this.referenceExists(target, id), + warn: (msg, meta) => this.logger?.warn?.(msg, meta as any), + }, + options, + ); + } + /** * Register the crypto provider that backs `secret`-typed fields. * diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index df9e76eef7..ba0862ebea 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -124,6 +124,22 @@ export type { export { parseLifecycleDuration } from './lifecycle/duration.js'; export { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js'; +// [#4551] Read-only referential-integrity audit — the reporting half of the +// `isSystem` exemption #4441 deliberately left in the write-path guard. +export { + auditDanglingReferences, + SECURITY_SURFACE_OBJECTS, + DEFAULT_ROWS_PER_OBJECT, + DEFAULT_MAX_ROWS, +} from './integrity/dangling-reference-audit.js'; +export type { + DanglingReference, + DanglingReferenceReport, + DanglingReferenceAuditOptions, + DanglingReferenceAuditPort, + AuditableObject, +} from './integrity/dangling-reference-audit.js'; + // Export MetadataFacade export { MetadataFacade } from './metadata-facade.js'; diff --git a/packages/objectql/src/integrity/dangling-reference-audit.test.ts b/packages/objectql/src/integrity/dangling-reference-audit.test.ts new file mode 100644 index 0000000000..7e8ec1ef90 --- /dev/null +++ b/packages/objectql/src/integrity/dangling-reference-audit.test.ts @@ -0,0 +1,374 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#4551] Dangling stored references are FOUND, and nothing is rewritten. + * + * #4441 made the write path refuse an unresolvable lookup id, but exempted + * `isSystem` writes so seed replay / package install / boot provisioning keep + * their ordering freedom. Correct — and it leaves the platform itself able to + * write a reference into the void with nothing saying so. This suite pins the + * "something says so". + * + * Every assertion here is written to FAIL if the corresponding judgment is + * removed from the audit: + * + * - drop the probe verdict → "finds a dangling reference" fails + * - condemn on a failed probe → "an unprobeable target is undetermined" fails + * - report on an existing row → "does NOT report a reference that resolves" fails + * - add any write → "NEVER rewrites" fails (data JSON compared) + * - drop the readonly skip → "a readonly reference is not audited" fails + * - drop the empty-value skip → "empty is not a reference" fails + */ + +import { describe, it, expect } from 'vitest'; +import { + auditDanglingReferences, + SECURITY_SURFACE_OBJECTS, + type AuditableObject, + type DanglingReferenceAuditPort, +} from './dangling-reference-audit.js'; + +/** The RBAC link-table shape #4551 calls out as the priority case. */ +const binding: AuditableObject = { + name: 'sys_position_permission_set', + fields: { + id: { type: 'text', primaryKey: true }, + position_id: { type: 'lookup', reference: 'sys_position' }, + permission_set_id: { type: 'lookup', reference: 'sys_permission_set' }, + note: { type: 'text' }, + }, +}; + +/** An ordinary business object with an optional lookup and a multi-value one. */ +const task: AuditableObject = { + name: 'showcase_task', + fields: { + id: { type: 'text', primaryKey: true }, + title: { type: 'text' }, + project: { type: 'lookup', reference: 'showcase_project' }, + tags: { type: 'lookup', reference: 'showcase_tag', multiple: true }, + // Audit-provenance shape: readonly, platform-minted (`applySystemFields` + // stamps `created_by` exactly like this). #4441 skips it on the write path + // because the value there is never the caller's; the audit skips it for the + // same reason — `sys_metadata_history.recorded_by` legitimately holds the + // SENTINEL STRING 'system'. + created_by: { type: 'lookup', reference: 'sys_user', readonly: true }, + }, +}; + +/** + * A test double whose probe answers with EXACTLY the three-valued contract the + * real `ObjectQL.referenceExists` answers with — `true` / `false` / `null` for + * "could not run". #4550's failure mode is a double looser than the real + * implementation; a two-valued probe here would quietly delete the entire + * `undetermined` axis from the suite. + */ +function makePort(opts: { + objects: AuditableObject[]; + rows: Record>>; + /** Ids that exist, as `${target} ${id}`. Anything else probes `false`. */ + existing?: Set; + /** Targets whose probe cannot run at all → `null`. */ + unprobeable?: Set; + /** Targets whose probe THROWS (must be read as unknown, never as absent). */ + throwingTargets?: Set; + /** Objects whose row listing throws. */ + unreadable?: Set; +}): DanglingReferenceAuditPort & { probes: string[]; warnings: Array<[string, unknown]> } { + const probes: string[] = []; + const warnings: Array<[string, unknown]> = []; + return { + probes, + warnings, + objects: () => opts.objects, + async find(object) { + if (opts.unreadable?.has(object)) throw new Error(`no driver for ${object}`); + // Returned by reference on purpose: a mutating audit would be visible in + // the caller's own `rows` snapshot. + return opts.rows[object] ?? []; + }, + async probe(target, id) { + probes.push(`${target} ${String(id)}`); + if (opts.throwingTargets?.has(target)) throw new Error(`probe blew up on ${target}`); + if (opts.unprobeable?.has(target)) return null; + return opts.existing?.has(`${target} ${String(id)}`) ?? false; + }, + warn: (m, meta) => { warnings.push([m, meta]); }, + }; +} + +describe('[#4551] dangling stored references are reported, never rewritten', () => { + it('finds a dangling reference and reports its FULL location', async () => { + // The residual #4441 left: a system write put a permission-set id here that + // names no row. On an RBAC link table that is a security-surface record + // resolving to nothing — the audience-anchor gate has to resolve exactly + // that set to evaluate the grant. + const port = makePort({ + objects: [binding], + rows: { + sys_position_permission_set: [ + { id: 'ppr_1', position_id: 'pos_real', permission_set_id: 'ps_does_not_exist_at_all' }, + ], + }, + existing: new Set(['sys_position pos_real']), + }); + + const out = await auditDanglingReferences(port); + + expect(out.scanned).toBe(1); + expect(out.undetermined).toBe(0); + expect(out.dangling).toHaveLength(1); + // Which object, which record, which field, which id, which target object — + // the report is addressed to a human who has to go fix it. + expect(out.dangling[0]).toEqual({ + objectName: 'sys_position_permission_set', + recordId: 'ppr_1', + field: 'permission_set_id', + target: 'sys_permission_set', + value: 'ps_does_not_exist_at_all', + }); + }); + + it('does NOT report a reference that resolves', async () => { + const port = makePort({ + objects: [binding], + rows: { + sys_position_permission_set: [ + { id: 'ppr_1', position_id: 'pos_real', permission_set_id: 'ps_real' }, + ], + }, + existing: new Set(['sys_position pos_real', 'sys_permission_set ps_real']), + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toEqual([]); + expect(out.undetermined).toBe(0); + expect(out.scanned).toBe(1); + }); + + it('an unprobeable TARGET is `undetermined`, never a verdict of dangling', async () => { + // Target object not registered / on an unreachable datasource. An integrity + // report that cannot run must not invent a finding — otherwise a + // connectivity problem publishes every reference through it as broken. + const port = makePort({ + objects: [binding], + rows: { + sys_position_permission_set: [ + { id: 'ppr_1', position_id: 'pos_1', permission_set_id: 'ps_1' }, + ], + }, + unprobeable: new Set(['sys_permission_set']), + existing: new Set(['sys_position pos_1']), + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toEqual([]); + // …and it is COUNTED, so "0 dangling" can never be read as "all clear" + // when nothing could actually be checked. + expect(out.undetermined).toBe(1); + }); + + it('a probe that THROWS is `undetermined` too — same reasoning, second failure mode', async () => { + const port = makePort({ + objects: [binding], + rows: { + sys_position_permission_set: [ + { id: 'ppr_1', position_id: 'pos_1', permission_set_id: 'ps_1' }, + ], + }, + throwingTargets: new Set(['sys_permission_set', 'sys_position']), + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toEqual([]); + expect(out.undetermined).toBe(2); + }); + + it('an object whose rows cannot be listed is named, not silently counted as clean', async () => { + const port = makePort({ + objects: [binding, task], + rows: { showcase_task: [{ id: 't1', title: 'T', project: 'proj_real' }] }, + unreadable: new Set(['sys_position_permission_set']), + existing: new Set(['showcase_project proj_real']), + }); + + const out = await auditDanglingReferences(port); + expect(out.unreadableObjects).toEqual(['sys_position_permission_set']); + expect(out.dangling).toEqual([]); + }); + + it('NEVER rewrites — the stored data is byte-identical before and after', async () => { + // The rows were genuinely written. Auto-nulling a dangling id would make + // the stored data disagree with what actually happened, and the remedy + // (re-seed the target vs clear the link) is an operator judgement call. + const rows: Record>> = { + sys_position_permission_set: [ + { id: 'ppr_1', position_id: 'pos_gone', permission_set_id: 'ps_gone' }, + ], + showcase_task: [ + { id: 't1', title: 'T', project: 'proj_gone', tags: ['tag_gone'], created_by: 'system' }, + ], + }; + const before = JSON.stringify(rows); + const port = makePort({ objects: [binding, task], rows }); + + const out = await auditDanglingReferences(port); + + expect(out.dangling.length).toBeGreaterThan(0); // it really did find things + expect(JSON.stringify(rows)).toBe(before); // …and changed none of them + }); + + it('a READONLY reference field is not audited — its value was minted by the platform', async () => { + // Same judgment #4441 makes on the write path, and for the same reason: + // `stripReadonlyFields` removes a caller's value first, so what remains is + // the platform's. `sys_metadata_history.recorded_by` is the real case — a + // `lookup('sys_user')` filled with the SENTINEL STRING `actor ?? 'system'`. + const port = makePort({ + objects: [task], + rows: { showcase_task: [{ id: 't1', title: 'T', created_by: 'system' }] }, + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toEqual([]); + // Not merely unreported — never even probed. + expect(port.probes).not.toContain('sys_user system'); + }); + + it('empty values are not references — null / "" / [] are skipped', async () => { + // `deleteBehavior: 'set_null'` writes exactly these. Matching #4441's + // `isEmptyReferenceValue` is the point: one predicate, two consumers. + const port = makePort({ + objects: [task], + rows: { + showcase_task: [ + { id: 't1', title: 'A', project: null, tags: [] }, + { id: 't2', title: 'B', project: '', tags: [null, ''] }, + { id: 't3', title: 'C' }, + ], + }, + }); + + const out = await auditDanglingReferences(port); + expect(out.scanned).toBe(3); + expect(out.dangling).toEqual([]); + expect(port.probes).toEqual([]); + }); + + it('every element of a multi-value reference is checked', async () => { + const port = makePort({ + objects: [task], + rows: { showcase_task: [{ id: 't1', title: 'T', tags: ['tag_real', 'tag_gone'] }] }, + existing: new Set(['showcase_tag tag_real']), + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toHaveLength(1); + expect(out.dangling[0]).toMatchObject({ field: 'tags', value: 'tag_gone' }); + }); + + it('an already-expanded record in the slot is a read shape, not an id', async () => { + const port = makePort({ + objects: [task], + rows: { showcase_task: [{ id: 't1', title: 'T', project: { id: 'proj_1', name: 'P' } }] }, + }); + + const out = await auditDanglingReferences(port); + expect(out.dangling).toEqual([]); + expect(port.probes).toEqual([]); + }); + + it('an object with no reference fields is never read at all', async () => { + const reads: string[] = []; + const plain: AuditableObject = { name: 'plain', fields: { id: { type: 'text' }, n: { type: 'number' } } }; + const port = makePort({ objects: [plain], rows: { plain: [{ id: 'p1', n: 1 }] } }); + const findSpy = port.find.bind(port); + port.find = async (o, opts) => { reads.push(o); return findSpy(o, opts); }; + + const out = await auditDanglingReferences(port); + expect(reads).toEqual([]); + expect(out.scanned).toBe(0); + }); + + it('RBAC link tables are scanned FIRST when the budget is finite', async () => { + // A dangling row on the security surface is an unevaluable gate input, so + // it must not be the thing a bounded scan runs out of budget before seeing. + const reads: string[] = []; + const port = makePort({ + objects: [task, binding], // registration order puts the business object first + rows: { + showcase_task: [{ id: 't1', title: 'T', project: 'p' }], + sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_1' }], + }, + }); + const findSpy = port.find.bind(port); + port.find = async (o, opts) => { reads.push(o); return findSpy(o, opts); }; + + await auditDanglingReferences(port); + expect(reads[0]).toBe('sys_position_permission_set'); + // …and the priority set is DERIVED from the platform-object registry, not + // hand-listed here, so a new plugin-security table is covered for free. + expect(SECURITY_SURFACE_OBJECTS.has('sys_position_permission_set')).toBe(true); + expect(SECURITY_SURFACE_OBJECTS.has('sys_user_permission_set')).toBe(true); + }); + + it('a bounded scan says so — `truncatedObjects` stops a SAMPLE reading as a proof', async () => { + const port = makePort({ + objects: [task], + rows: { + showcase_task: [ + { id: 't1', title: 'A', project: 'proj_real' }, + { id: 't2', title: 'B', project: 'proj_real' }, + ], + }, + existing: new Set(['showcase_project proj_real']), + }); + + // The port ignores `limit` (a real driver would not), so a budget of 2 with + // 2 rows returned is exactly the "budget reached" signal. + const out = await auditDanglingReferences(port, { rowsPerObject: 2 }); + expect(out.dangling).toEqual([]); + expect(out.truncatedObjects).toEqual(['showcase_task']); + }); + + it('the same (target, id) is probed once per run', async () => { + // A link table is by definition many rows pointing at few ids; re-probing + // would also multiply a storage outage by the row count. + const port = makePort({ + objects: [binding], + rows: { + sys_position_permission_set: [ + { id: 'a', permission_set_id: 'ps_gone' }, + { id: 'b', permission_set_id: 'ps_gone' }, + { id: 'c', permission_set_id: 'ps_gone' }, + ], + }, + }); + + const out = await auditDanglingReferences(port); + expect(port.probes).toEqual(['sys_permission_set ps_gone']); + // Memoisation is an optimisation, never a loss of findings: all three rows + // are still reported individually. + expect(out.dangling.map((d) => d.recordId)).toEqual(['a', 'b', 'c']); + }); + + it('the report is logged when there is anything to say, and silent otherwise', async () => { + const clean = makePort({ + objects: [binding], + rows: { sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_real' }] }, + existing: new Set(['sys_permission_set ps_real']), + }); + await auditDanglingReferences(clean); + expect(clean.warnings).toEqual([]); + + const dirty = makePort({ + objects: [binding], + rows: { sys_position_permission_set: [{ id: 'ppr_1', permission_set_id: 'ps_gone' }] }, + }); + await auditDanglingReferences(dirty); + expect(dirty.warnings).toHaveLength(1); + expect(dirty.warnings[0][0]).toContain('#4551'); + expect((dirty.warnings[0][1] as any).references).toEqual([ + 'sys_position_permission_set#ppr_1.permission_set_id → sys_permission_set#ps_gone', + ]); + }); +}); diff --git a/packages/objectql/src/integrity/dangling-reference-audit.ts b/packages/objectql/src/integrity/dangling-reference-audit.ts new file mode 100644 index 0000000000..7c4f55c7bf --- /dev/null +++ b/packages/objectql/src/integrity/dangling-reference-audit.ts @@ -0,0 +1,298 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { referenceTargetOf } from '@objectstack/spec/data'; +import { PLATFORM_OBJECTS_BY_PACKAGE } from '@objectstack/spec/system'; + +/** + * [#4551] Read-only inspection for the residual #4441 left open: a stored + * reference that points at no row. + * + * #4441 made the WRITE path refuse an unresolvable `lookup` id — but + * deliberately exempted `isSystem` writes, because seed replay, package + * install and boot-time provisioning legitimately write rows in an order that + * only resolves once the batch completes, and failing them closed would turn + * an ordering detail into a boot failure. That exemption is correct and this + * change does not touch it. What it leaves behind is a gap of a different + * kind: **the platform itself can still write a reference into the void, and + * nothing says so.** + * + * Removing the exemption is not the fix. Beyond the boot-ordering problem, the + * platform has legitimate non-id writes of its own — `sys_metadata_history. + * recorded_by` is a `lookup('sys_user')` the metadata repository fills with the + * SENTINEL STRING `actor ?? 'system'` (that one is already out of scope via + * #4441's `readonly` narrowing). Rejecting the platform's own write is not the + * right way to report the problem. Making it VISIBLE is. + * + * ## Reports; never rewrites + * + * Same posture as {@link https://github.com/objectstack-ai/objectstack/issues/4469}'s + * `inspectStrandedRequests`, and for the same reason: the rows were genuinely + * written. Auto-nulling a dangling id would make the stored data disagree with + * what actually happened, and the remedy — re-seed the missing target, or clear + * the reference — is a judgement call an audit cannot make. So this issues + * exactly zero writes and its report is addressed to a human: which object, + * which record, which field, which id, and which object that id was supposed to + * name. + * + * ## Unknown and absent are DIFFERENT answers + * + * The single most important property here. An existence probe that cannot run + * (target object not registered, no driver, probe throws) must never be + * recorded as "the target does not exist" — otherwise a datasource outage + * publishes every reference through it as broken. Those land in + * {@link DanglingReferenceReport.undetermined}. Likewise an object whose rows + * could not be listed lands in `unreadableObjects`, and an object whose row + * budget ran out lands in `truncatedObjects` — because a bounded scan proves + * nothing about the rows it never read. Together they are what stops + * "0 dangling" from ever being read as "everything is fine". + * + * ## Scope — the same judgments #4441 already made, not new ones + * + * - **`readonly` reference fields are skipped**, exactly as the write-path + * check skips them: a non-system caller's value is stripped before the write + * (`stripReadonlyFields` / `stripReadonlyForInsert`), so what remains was + * minted by the platform — including the audit-provenance family + * (`created_by` / `updated_by` / `organization_id`, all `readonly: true` from + * `applySystemFields`) and the `recorded_by` sentinel above. + * - **Which fields are references** is `referenceTargetOf` — the single + * arbiter the write-path check and the expand gate already share, covering + * `lookup` / `master_detail` / `user` / `tree`. A hand-written type list here + * would be a second, drifting answer to a question that already has one. + * - **Empty is not a reference.** `null` / `undefined` / `''` and the empty + * array mean "no link" — what `deleteBehavior: 'set_null'` writes. + * - **An already-expanded object in the slot is not an id.** + */ + +/** One stored reference that resolves to nothing. */ +export interface DanglingReference { + /** Object holding the broken reference. */ + objectName: string; + /** Primary key of the row holding it. */ + recordId: string; + /** Field on that row. */ + field: string; + /** The object the field declares as its target. */ + target: string; + /** The id that names no row of `target`. */ + value: string; +} + +export interface DanglingReferenceReport { + /** Rows actually read and examined. */ + scanned: number; + /** References proven to point at nothing. */ + dangling: DanglingReference[]; + /** + * Reference values whose target could NOT be probed (unregistered object, no + * driver, probe threw). NOT healthy — merely unknown. + */ + undetermined: number; + /** Objects whose rows could not be listed at all. Unknown, not clean. */ + unreadableObjects: string[]; + /** + * Objects where the per-object row budget was reached before the table + * ended, so this run inspected a SAMPLE. `dangling: []` says nothing about + * the rows beyond the budget. + */ + truncatedObjects: string[]; +} + +/** Minimal object shape the audit reads — duck-typed so tests need no registry. */ +export interface AuditableObject { + name: string; + fields?: Record; +} + +/** + * The engine surface the audit needs. `probe` is deliberately a PORT rather + * than a re-implementation: the caller passes the engine's own existence check, + * so the audit and the #4441 write-path guard answer "does this id exist" with + * one predicate. A second copy here would be free to disagree — which is the + * #4550 failure mode (a stand-in looser than the real implementation) planted + * in production code rather than in a test. + */ +export interface DanglingReferenceAuditPort { + /** Every registered object, in registration order. */ + objects(): AuditableObject[]; + /** Unscoped row read. Existence is a fact about the database, not the caller. */ + find(object: string, options: Record): Promise>>; + /** + * `true` = the row exists, `false` = the probe RAN and found nothing, + * `null` = it could not run at all (→ `undetermined`). + */ + probe(target: string, id: unknown): Promise; + warn?(message: string, meta?: unknown): void; +} + +export interface DanglingReferenceAuditOptions { + /** Rows read per object. Default {@link DEFAULT_ROWS_PER_OBJECT}. */ + rowsPerObject?: number; + /** Total rows read across all objects. Default {@link DEFAULT_MAX_ROWS}. */ + maxRows?: number; + /** Restrict the scan to these objects (diagnostics / tests). */ + objects?: string[]; +} + +/** Bounded per object so one enormous table cannot starve every other. */ +export const DEFAULT_ROWS_PER_OBJECT = 500; +/** Bounded overall so the audit stays invisible next to the sweep it rides. */ +export const DEFAULT_MAX_ROWS = 5_000; + +/** + * Objects scanned FIRST when the budget is finite (#4551): the ADR-0090 + * permission model's own tables. A dangling row there is not untidy, it is a + * **security-surface record that resolves to nothing** — and the + * audience-anchor gate has to resolve exactly that permission set to evaluate + * the grant, so the binding is an unevaluable gate input. + * + * Derived from `PLATFORM_OBJECTS_BY_PACKAGE` rather than hand-listed, so a + * table added to plugin-security is prioritised without anyone remembering to + * come back here. + */ +export const SECURITY_SURFACE_OBJECTS: ReadonlySet = new Set( + PLATFORM_OBJECTS_BY_PACKAGE['plugin-security'] ?? [], +); + +/** "The stored slot names no record." Mirrors the write path's own predicate. */ +function isEmptyStoredReference(v: unknown): boolean { + return v === null || v === undefined || v === ''; +} + +/** + * Reference fields worth auditing on one object: declared target, not + * `readonly`. Returns `[]` for an object with none, which is how the audit + * avoids reading a single row of the vast majority of tables. + */ +function auditableReferenceFields(obj: AuditableObject): Array<{ name: string; target: string }> { + const fields = obj?.fields; + if (!fields || typeof fields !== 'object') return []; + const out: Array<{ name: string; target: string }> = []; + for (const [name, def] of Object.entries(fields)) { + if ((def as { readonly?: unknown })?.readonly === true) continue; + const target = referenceTargetOf(def); + if (!target) continue; + out.push({ name, target }); + } + return out; +} + +/** + * Security-surface objects first, everything else after, each group keeping + * registration order so a run is deterministic. + */ +function prioritise(objects: AuditableObject[]): AuditableObject[] { + const security: AuditableObject[] = []; + const rest: AuditableObject[] = []; + for (const o of objects) (SECURITY_SURFACE_OBJECTS.has(o?.name) ? security : rest).push(o); + return [...security, ...rest]; +} + +/** + * Walk stored rows and report every reference that resolves to nothing. + * + * Issues **no writes of any kind**. Every failure to determine an answer is + * reported as such rather than resolved into a verdict. + */ +export async function auditDanglingReferences( + port: DanglingReferenceAuditPort, + options?: DanglingReferenceAuditOptions, +): Promise { + const report: DanglingReferenceReport = { + scanned: 0, dangling: [], undetermined: 0, unreadableObjects: [], truncatedObjects: [], + }; + + let all: AuditableObject[]; + try { + all = port.objects() ?? []; + } catch { + return report; + } + const only = options?.objects ? new Set(options.objects) : undefined; + const rowsPerObject = options?.rowsPerObject ?? DEFAULT_ROWS_PER_OBJECT; + const maxRows = options?.maxRows ?? DEFAULT_MAX_ROWS; + + // One id is typically referenced by many rows (that is what a link table IS), + // so the probe is memoised for the run. `null` — could not determine — is + // cached too: a target that cannot be probed does not become probeable + // halfway through one sweep, and re-asking would multiply an outage by the + // row count. + const probed = new Map(); + const exists = async (target: string, value: string): Promise => { + const key = `${target}${value}`; + if (probed.has(key)) return probed.get(key)!; + let answer: boolean | null; + try { + answer = await port.probe(target, value); + } catch { + // A throwing probe is "could not determine", never "does not exist". + answer = null; + } + probed.set(key, answer); + return answer; + }; + + for (const obj of prioritise(all)) { + if (report.scanned >= maxRows) break; + const name = obj?.name; + if (!name || (only && !only.has(name))) continue; + const refFields = auditableReferenceFields(obj); + if (refFields.length === 0) continue; // nothing referential here — read nothing + + const budget = Math.min(rowsPerObject, maxRows - report.scanned); + let rows: Array>; + try { + rows = (await port.find(name, { + fields: ['id', ...refFields.map((f) => f.name)], + limit: budget, + context: { isSystem: true }, + })) ?? []; + } catch (err) { + // Unreadable ⇒ unknown. Recorded so the report cannot be mistaken for a + // clean bill of health on an object nothing could look at. + report.unreadableObjects.push(name); + port.warn?.('[integrity] dangling-reference audit could not list an object', { + object: name, error: (err as Error)?.message ?? String(err), + }); + continue; + } + report.scanned += rows.length; + if (rows.length >= budget) report.truncatedObjects.push(name); + + for (const row of rows) { + for (const { name: field, target } of refFields) { + const raw = row?.[field]; + if (isEmptyStoredReference(raw)) continue; + const values = Array.isArray(raw) ? raw : [raw]; + for (const v of values) { + if (isEmptyStoredReference(v)) continue; + // An expanded record in the slot is a read shape, not an id write. + if (typeof v === 'object') continue; + const answer = await exists(target, v); + if (answer === null) { report.undetermined++; continue; } + if (answer) continue; + report.dangling.push({ + objectName: name, + recordId: String(row?.id ?? ''), + field, + target, + value: String(v), + }); + } + } + } + } + + if (report.dangling.length || report.undetermined || report.unreadableObjects.length) { + port.warn?.('[integrity] stored references that resolve to nothing (#4551)', { + scanned: report.scanned, + dangling: report.dangling.length, + undetermined: report.undetermined, + unreadableObjects: report.unreadableObjects, + truncatedObjects: report.truncatedObjects, + references: report.dangling.map( + (d) => `${d.objectName}#${d.recordId}.${d.field} → ${d.target}#${d.value}`, + ), + }); + } + return report; +} diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index 4dc8771660..588307c819 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -721,3 +721,80 @@ describe('LifecycleService timers', () => { } }); }); + +/** + * [#4551] The read-only referential-integrity audit rides this sweep's clock. + * + * It is here — rather than on a clock of its own — so an operator does not have + * to know the finding exists in order to go looking for it (the same argument + * that put #4469's stranded-request inspection on the approvals SLA clock). + * Its subject is unrelated to retention; a clock is scheduling, not scope. + */ +describe('LifecycleService reference audit leg (#4551)', () => { + it('runs the audit on every sweep and carries the finding in the report', async () => { + const { engine } = captureEngine([]); + const finding = { + scanned: 2, + dangling: [{ + objectName: 'sys_position_permission_set', recordId: 'ppr_1', + field: 'permission_set_id', target: 'sys_permission_set', value: 'ps_gone', + }], + undetermined: 0, unreadableObjects: [], truncatedObjects: [], + }; + engine.inspectDanglingReferences = async () => finding; + + const report = await service(engine).sweep(); + expect(report.danglingReferences).toEqual(finding); + }); + + it('an engine without the audit reports NOTHING rather than an empty finding', async () => { + // Absence of a report is honest ("nobody looked"); a zeroed report would + // read as "looked and found nothing", which is the misreading #4551 is + // specifically trying to prevent. + const { engine } = captureEngine([]); + const report = await service(engine).sweep(); + expect('danglingReferences' in report).toBe(false); + }); + + it('an audit that throws never costs the sweep its reaping', async () => { + const { engine, deletes } = captureEngine([ + { name: 'sys_job_run', lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } }, + ]); + engine.inspectDanglingReferences = async () => { throw new Error('probe store down'); }; + + const report = await service(engine).sweep(); + expect(deletes).toHaveLength(1); + // …and a failed AUDIT is not a failed POLICY: `errors` means "a lifecycle + // policy did not get applied", which is not what happened here. + expect(report.errors).toEqual([]); + expect(report.danglingReferences).toBeUndefined(); + }); + + it('`referenceAudit.enabled: false` drops the leg and leaves lifecycle alone', async () => { + const { engine, deletes } = captureEngine([ + { name: 'sys_job_run', lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } }, + ]); + let called = 0; + engine.inspectDanglingReferences = async () => { + called += 1; + return { scanned: 0, dangling: [], undetermined: 0, unreadableObjects: [], truncatedObjects: [] }; + }; + + const report = await service(engine, { referenceAudit: { enabled: false } }).sweep(); + expect(called).toBe(0); + expect(report.danglingReferences).toBeUndefined(); + expect(deletes).toHaveLength(1); + }); + + it('forwards the audit budget and never leaks `enabled` into it', async () => { + const { engine } = captureEngine([]); + let seen: unknown; + engine.inspectDanglingReferences = async (opts: unknown) => { + seen = opts; + return { scanned: 0, dangling: [], undetermined: 0, unreadableObjects: [], truncatedObjects: [] }; + }; + + await service(engine, { referenceAudit: { enabled: true, rowsPerObject: 7, maxRows: 21 } }).sweep(); + expect(seen).toEqual({ rowsPerObject: 7, maxRows: 21 }); + }); +}); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index f51c045108..554107867b 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -2,6 +2,10 @@ import type { Lifecycle } from '@objectstack/spec/data'; import { parseLifecycleDuration } from './duration.js'; +import type { + DanglingReferenceAuditOptions, + DanglingReferenceReport, +} from '../integrity/dangling-reference-audit.js'; /** * LifecycleService — the single platform-owned enforcer of ADR-0057 @@ -68,6 +72,14 @@ export interface LifecycleEngineLike { datasource?(name: string): unknown; /** Row reads for governance (tenant enumeration); optional. */ find?(object: string, options: Record): Promise>>; + /** + * [#4551] Read-only referential-integrity audit. Optional — an engine + * without it simply contributes no finding (and the report says so by + * omitting the key, rather than by reporting zero). + */ + inspectDanglingReferences?( + options?: DanglingReferenceAuditOptions, + ): Promise; } export interface LifecycleObjectLike { @@ -118,6 +130,12 @@ export interface LifecycleServiceOptions { getSettings?(): LifecycleSettingsLike | undefined; /** Governance alert sink. Defaults to a logger warning. */ onAlert?(alert: LifecycleGovernanceAlert): void; + /** + * [#4551] Referential-integrity audit tuning. The audit rides this sweep's + * clock deliberately (see {@link LifecycleService.sweep}); `enabled: false` + * drops that leg while leaving lifecycle enforcement alone. + */ + referenceAudit?: DanglingReferenceAuditOptions & { enabled?: boolean }; } /** Per-sweep governance snapshot resolved from the `lifecycle` namespace. */ @@ -170,6 +188,12 @@ export interface LifecycleSweepReport { reclaimed: string[]; /** Governance alerts raised this sweep (quota breaches, growth spikes). */ alerts: LifecycleGovernanceAlert[]; + /** + * [#4551] Read-only referential-integrity finding for this sweep, when the + * engine offers the audit. Absent on an engine that does not (older engine, + * a test double) — which is itself honest: no report is not "clean". + */ + danglingReferences?: DanglingReferenceReport; } interface ReclaimCapableDriver { @@ -345,6 +369,19 @@ export class LifecycleService { // never a delete beyond the declared policy. await this.checkGovernance(engine, declared, report); + // [#4551] Referential-integrity audit — READ-ONLY, and the only leg of + // this sweep that writes nothing at all. + // + // It rides this clock for one reason: an operator must not have to know + // the finding exists in order to go looking for it (the same argument + // that put #4469's stranded-request inspection on the approvals SLA + // clock). Its subject is unrelated to retention, and that is fine — a + // clock is scheduling, not scope. + // + // Failure is isolated like every other leg: an audit that cannot run must + // never cost a sweep its reaping. + await this.auditReferences(engine, report); + if (report.swept.length > 0 || report.errors.length > 0 || report.alerts.length > 0) { // ADR-0057 §3.3: cleanup must not re-feed the tables it drains — one // aggregate log line per sweep is the entire trace it leaves. @@ -361,6 +398,32 @@ export class LifecycleService { } } + /** + * [#4551] Run the read-only dangling-reference audit as one leg of the sweep. + * + * Deliberately contributes NOTHING to `report.errors` on failure: those + * entries mean "a lifecycle policy did not get applied", and an audit that + * could not run has applied no policy either way. It logs instead, and its + * own report already carries `unreadableObjects` / `undetermined` so a + * partial run is never mistaken for a clean one. + */ + private async auditReferences( + engine: LifecycleEngineLike, + report: LifecycleSweepReport, + ): Promise { + const cfg = this.opts.referenceAudit; + if (cfg?.enabled === false) return; + if (typeof engine.inspectDanglingReferences !== 'function') return; + try { + const { enabled: _enabled, ...auditOptions } = cfg ?? {}; + report.danglingReferences = await engine.inspectDanglingReferences(auditOptions); + } catch (err) { + this.opts.logger.warn( + `[lifecycle] reference audit failed (${(err as Error)?.message ?? err})`, + ); + } + } + /** Resolve the `lifecycle` settings namespace into a per-sweep snapshot. * Every read is best-effort: no settings service / unregistered namespace * ⇒ declared policies apply unmodified. */ diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index 17f989552b..d61d362d80 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -7,6 +7,7 @@ import { applyConversionsToStoredItem } from '@objectstack/spec'; import { StorageNameMapping } from '@objectstack/spec/system'; import { LifecycleService } from './lifecycle/lifecycle-service.js'; import { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js'; +import type { DanglingReferenceAuditOptions } from './integrity/dangling-reference-audit.js'; import { runActionGovernanceInventory } from './action-governance.js'; import type { IMetadataService } from '@objectstack/spec/contracts'; @@ -113,11 +114,15 @@ export interface ObjectQLPluginOptions { * `enabled: false` (or env `OS_LIFECYCLE_DISABLED=1`) to disable the * periodic sweep entirely; the `lifecycle` service stays registered so * tooling can still run `sweep()` explicitly. + * + * `referenceAudit` tunes the #4551 read-only dangling-reference audit that + * rides this same clock. It writes nothing; `enabled: false` drops the leg. */ lifecycle?: { enabled?: boolean; sweepIntervalMs?: number; initialDelayMs?: number; + referenceAudit?: DanglingReferenceAuditOptions & { enabled?: boolean }; }; } From 0621320b6bfe0606310775f659243b4d08137313 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 03:11:01 +0000 Subject: [PATCH 2/2] fix(objectql): spell the audit NUL key separator as \u0000, not a raw byte MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check:nul-bytes failed on the #4551 branch: the probe-memoisation key in dangling-reference-audit.ts carried a RAW NUL byte as its separator. The separator itself is right — NUL cannot occur in an object name or a record id, so no (target, value) pair can collide with another. The spelling was wrong. A raw NUL makes ripgrep treat the whole file as binary and return zero matches, so the file silently drops out of code search and every grep-based lint; git does not warn, because it only inspects the first 8000 bytes to judge binary-ness and this one sat at offset 9908. Rewritten as the \u0000 escape, matching the existing convention at packages/rest/src/rest-server.ts:1065. The escape evaluates to the same single NUL character, so the key is byte-identical at runtime and no behaviour changes — verified by the unchanged "probed once per run" memoisation test. Comment added at the site so the escape is not "helpfully" turned back into the raw byte later. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --- .../objectql/src/integrity/dangling-reference-audit.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/integrity/dangling-reference-audit.ts b/packages/objectql/src/integrity/dangling-reference-audit.ts index 7c4f55c7bf..37a666b0e5 100644 --- a/packages/objectql/src/integrity/dangling-reference-audit.ts +++ b/packages/objectql/src/integrity/dangling-reference-audit.ts @@ -216,9 +216,16 @@ export async function auditDanglingReferences( // cached too: a target that cannot be probed does not become probeable // halfway through one sweep, and re-asking would multiply an outage by the // row count. + // + // The key separator is NUL because it cannot occur in an object name or a + // record id, so no (target, value) pair can collide with another. It is + // spelled as the \u0000 ESCAPE, never the raw byte: a raw NUL makes + // ripgrep treat the whole file as binary and return ZERO matches, dropping + // it out of code search and every grep-based lint (`pnpm check:nul-bytes` + // enforces this). The escape is byte-identical at runtime. const probed = new Map(); const exists = async (target: string, value: string): Promise => { - const key = `${target}${value}`; + const key = `${target}\u0000${value}`; if (probed.has(key)) return probed.get(key)!; let answer: boolean | null; try {