diff --git a/.changeset/dangling-reference-sweep.md b/.changeset/dangling-reference-sweep.md new file mode 100644 index 0000000000..a4c81a453e --- /dev/null +++ b/.changeset/dangling-reference-sweep.md @@ -0,0 +1,51 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql): report dangling lookup references left behind by `isSystem` writes (#4551) + +#4441 made the write path refuse a `lookup` id that exists in no row of the +object it references, but deliberately exempted `isSystem` writes: seed replay, +package install and boot-time provisioning legitimately write rows in an order +that is self-consistent only once the batch completes, and failing them closed +would turn an ordering detail into a boot failure. The exemption is correct, and +it left a real residual — **the platform itself could still store a reference +pointing at nothing, and nothing said so.** PR #4511 recorded that residual +rather than silently accepting it; this closes it, by reporting. + +`ObjectQL.inspectDanglingReferences()` walks the non-`readonly` reference fields +of every registered object and names each stored id that resolves to no row: +object, record id, field, target object, the id itself, and the holding row's +tenant. It **reports and never rewrites** — the rows genuinely exist and were +genuinely written, so nulling a link would make the stored data disagree with +what happened, and whether the repair is to create the missing target, re-run +the seed or delete the holder is a judgement the platform cannot make. + +Three properties are worth knowing before reading a report: + +- **Unknown is not healthy.** The probe is the same one the write path enforces + with. It answers "missing" only when it RAN and found nothing; an unregistered + target, an absent driver or a probe that throws is counted into + `undetermined`, and an unreadable object is named in `skipped`. A datasource + outage can therefore never read as a clean bill of health. +- **`readonly` references are skipped**, as on the write path: the value there + was minted by the platform, and at least one is a sentinel by design + (`sys_metadata_history.recorded_by` stores `actor ?? 'system'` in a + `lookup('sys_user')`). +- **Bounded, RBAC-first.** The RBAC link tables are visited first — a dangling + row there is a security-surface record that resolves to nothing, and the + audience-anchor gate has to resolve that very target to evaluate the grant. + Per-object and total row caps bound one pass; hitting one sets `truncated`, so + a report that stopped early cannot read as "everything was checked". + +It rides the existing ADR-0057 lifecycle clock (hourly, first run delayed past +boot — exactly when seed-written references become checkable) rather than arming +a second one, so the finding surfaces without an operator knowing to go looking. +It runs *after* the sweep and is isolated from it in both directions. +`LifecycleService.sweep()` is unchanged: tooling that calls it directly still +gets policy enforcement and nothing else. + +New exports from `@objectstack/objectql`: `DanglingReference`, +`DanglingReferenceReport`, `REFERENCE_SCAN_PRIORITY_OBJECTS`, +`DANGLING_SCAN_ROWS_PER_OBJECT`, `DANGLING_SCAN_MAX_ROWS`. Nothing was removed +or renamed, and #4441's enforcement is untouched. diff --git a/packages/objectql/src/engine-dangling-reference-sweep.test.ts b/packages/objectql/src/engine-dangling-reference-sweep.test.ts new file mode 100644 index 0000000000..cd5d00839f --- /dev/null +++ b/packages/objectql/src/engine-dangling-reference-sweep.test.ts @@ -0,0 +1,459 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#4551] The dangling-reference inspection — the residual #4441 left open. + * + * #4441 enforces referential integrity on the write path but deliberately + * exempts `isSystem` writes: seed replay, package install and boot-time + * provisioning legitimately write rows in an order that is self-consistent + * only once the batch completes, so failing them closed would turn an ordering + * detail into a boot failure. The exemption is right; what it leaves is a + * platform that can still store a reference pointing at nothing while nothing + * says so. + * + * So every case below plants its bad data through the SAME door the platform + * uses — `context: { isSystem: true }` — which is what makes the suite about + * #4551 rather than a re-test of #4441. A non-system write of the same row is + * refused before it lands (pinned at the bottom), and that is the point: the + * only writer that can produce this data is the platform. + * + * ## Why the test double is not looser than a real driver (#4550) + * + * The existence probe under test is `ObjectQL.referenceExists` — the very same + * private method the write-path enforcement calls, so the sweep and the guard + * can never disagree about what "exists" means. Nothing about it is faked here. + * The memory driver below honors `limit` and the `fields` projection exactly as + * a real driver does, because the scan's bounds are asserted (`truncated`) and + * a driver that ignored `limit` would make that assertion vacuous. + * + * Both claims were measured, not asserted. Loosening the double so `findOne` + * answers a row for everything turns FIVE cases red — including the #4441 + * write-path pin at the bottom, which is the guard that a double looser than + * the real driver cannot slip past. Removing the probe's throw turns + * "counts an unprobeable target as undetermined" red on its own. And removing + * the inspection itself turns ELEVEN of the fourteen red; the three that stay + * green are the ones that assert SILENCE (empty values, the readonly sentinel, + * and the #4441 pin), which is exactly the set that should survive. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL, REFERENCE_SCAN_PRIORITY_OBJECTS } from './engine.js'; +import { LifecycleService } from './lifecycle/lifecycle-service.js'; + +const permissionSet = { + name: 'sys_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 the issue names: a dangling row here is a + * security-surface record that resolves to nothing. */ +const binding = { + name: 'sys_position_permission_set', + label: 'Position ↔ Permission Set', + 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: 'sys_permission_set', + required: true, deleteBehavior: 'set_null' as const, + }, + organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + }, +}; + +/** An ordinary application object — the rule is about resolvability, not about + * living in `sys_*`. `tags` is the multi-value spelling. */ +const task = { + name: 'ref_task', + label: 'Task', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + project: { + name: 'project', label: 'Project', + type: 'lookup' as const, reference: 'sys_permission_set', + }, + tags: { + name: 'tags', label: 'Tags', + type: 'lookup' as const, reference: 'sys_permission_set', multiple: true, + }, + }, +}; + +/** + * The `sys_metadata_history.recorded_by` shape: a READONLY `lookup('sys_user')` + * the platform fills with `actor ?? 'system'` — a SENTINEL STRING, never a user + * id (found by the #4441 dogfood run, recorded on PR #4511). Reporting it every + * hour would be a permanent false positive, so the scan skips readonly + * references for the same reason the write path does: the value is the + * platform's, not a caller's. + */ +const history = { + name: 'ref_history', + label: 'History', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + recorded_by: { + name: 'recorded_by', label: 'Recorded By', + type: 'lookup' as const, reference: 'sys_permission_set', readonly: true, + }, + }, +}; + +/** Declares a lookup at an object that is never registered — unprobeable. */ +const orphanTarget = { + name: 'ref_orphan', + label: 'Orphan', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + ghost: { + name: 'ghost', label: 'Ghost', + type: 'lookup' as const, reference: 'ref_never_registered', + }, + }, +}; + +interface MemoryDriver { + driver: any; + stores: Map>>; + /** Object names whose `findOne` should throw — an unreadable target. */ + probeFailures: Set; + /** Object names whose `find` should throw — an unreadable holder. */ + readFailures: Set; +} + +function makeMemoryDriver(): MemoryDriver { + const stores = new Map>>(); + const probeFailures = new Set(); + const readFailures = new Set(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { s = new Map(); stores.set(obj, s); } + return s; + }; + /** Reads never materialize a table — a real driver does not create one by + * being queried, and the "nothing was rewritten" snapshot must not be + * perturbed by the scan merely having looked. */ + const readStoreFor = (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; + }; + // A real driver projects to the selected columns and stops at `limit`. The + // double does both: the scan's bounds are asserted, and a driver that ignored + // `limit` would make `truncated` untestable (#4550). + const project = (rows: Record[], ast: any) => { + let out = rows; + if (Array.isArray(ast?.fields) && ast.fields.length > 0) { + const keep: string[] = ast.fields; + out = out.map((r) => { + const picked: Record = {}; + for (const f of keep) if (f in r) picked[f] = r[f]; + return picked; + }); + } + if (typeof ast?.limit === 'number' && ast.limit >= 0) out = out.slice(0, ast.limit); + return out; + }; + 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) { + if (readFailures.has(object)) throw new Error(`datasource unreachable: ${object}`); + const rows = Array.from(readStoreFor(object).values()).filter((r) => matches(r, ast?.where)); + return project(rows, ast); + }, + findStream() { throw new Error('not implemented'); }, + async findOne(object: string, ast: any) { + if (probeFailures.has(object)) throw new Error(`datasource unreachable: ${object}`); + for (const r of readStoreFor(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() { return 0; }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, stores, probeFailures, readFailures }; +} + +/** Every stored row, as JSON — the "nothing was rewritten" oracle. */ +function snapshot(stores: Map>>): string { + const out: Record = {}; + for (const [object, rows] of [...stores.entries()].sort(([a], [b]) => a.localeCompare(b))) { + out[object] = [...rows.values()].sort((a, b) => String(a.id).localeCompare(String(b.id))); + } + return JSON.stringify(out); +} + +const SYSTEM = { context: { isSystem: true } } as any; + +describe('[#4551] dangling references produced by `isSystem` writes are reported', () => { + let engine: ObjectQL; + let mem: MemoryDriver; + + beforeEach(async () => { + engine = new ObjectQL(); + mem = makeMemoryDriver(); + engine.registerDriver(mem.driver, true); + await engine.init(); + engine.registry.registerObject(permissionSet as any); + engine.registry.registerObject(binding as any); + engine.registry.registerObject(task as any); + engine.registry.registerObject(history as any); + await engine.insert('sys_permission_set', { id: 'ps_real', name: 'Real' }, SYSTEM); + }); + + it('reports the dangling reference with a full locator', async () => { + // Exactly what seed replay does: the row lands, the target never arrives. + await engine.insert( + 'sys_position_permission_set', + { id: 'bind_1', permission_set_id: 'ps_never_seeded', organization_id: 'org_a' }, + SYSTEM, + ); + + const report = await engine.inspectDanglingReferences(); + + expect(report.dangling).toHaveLength(1); + // Object, record, field, target, id — an operator must be able to go + // straight to the row without reconstructing the query. + expect(report.dangling[0]).toEqual({ + object: 'sys_position_permission_set', + recordId: 'bind_1', + field: 'permission_set_id', + target: 'sys_permission_set', + value: 'ps_never_seeded', + organizationId: 'org_a', + }); + expect(report.undetermined).toBe(0); + expect(report.skipped).toEqual([]); + }); + + it('says nothing when the target exists', async () => { + await engine.insert( + 'sys_position_permission_set', + { id: 'bind_ok', permission_set_id: 'ps_real' }, + SYSTEM, + ); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling).toEqual([]); + expect(report.undetermined).toBe(0); + expect(report.scannedReferences).toBe(1); + }); + + it('covers ordinary application objects, including the multi-value spelling', async () => { + await engine.insert( + 'ref_task', + { id: 't1', title: 'T', project: 'proj_gone', tags: ['ps_real', 'tag_gone'] }, + SYSTEM, + ); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling.map((d) => `${d.field}:${d.value}`).sort()) + .toEqual(['project:proj_gone', 'tags:tag_gone']); + }); + + it('does not treat an empty value as a reference', async () => { + await engine.insert( + 'ref_task', + { id: 't_empty', title: 'T', project: null, tags: [] }, + SYSTEM, + ); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling).toEqual([]); + // Nothing was probed at all — `null` / `[]` mean "no link", which is what + // `deleteBehavior: 'set_null'` writes. + expect(report.scannedReferences).toBe(0); + }); + + it('skips a readonly reference — the platform-minted sentinel is not a finding', async () => { + // `sys_metadata_history.recorded_by` stores `actor ?? 'system'`: a sentinel + // string in a lookup column, by design. Reporting it hourly would be a + // permanent false positive. + await engine.insert('ref_history', { id: 'h1', recorded_by: 'system' }, SYSTEM); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling).toEqual([]); + expect(report.scannedReferences).toBe(0); + }); + + it('counts an unprobeable target as undetermined, never as dangling', async () => { + await engine.insert( + 'sys_position_permission_set', + { id: 'bind_probe', permission_set_id: 'ps_who_knows' }, + SYSTEM, + ); + // The datasource behind the TARGET goes away mid-scan. + mem.probeFailures.add('sys_permission_set'); + + const report = await engine.inspectDanglingReferences(); + + // A storage outage must never be published as a lost record… + expect(report.dangling).toEqual([]); + // …and it must be COUNTED, so `dangling: []` can never be read as + // "all clear" when nothing could actually be checked. + expect(report.undetermined).toBe(1); + }); + + it('counts a reference to an unregistered object as undetermined', async () => { + engine.registry.registerObject(orphanTarget as any); + await engine.insert('ref_orphan', { id: 'o1', ghost: 'anything' }, SYSTEM); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling).toEqual([]); + expect(report.undetermined).toBe(1); + }); + + it('names an unreadable object in `skipped` rather than dropping it silently', async () => { + await engine.insert( + 'sys_position_permission_set', + { id: 'bind_x', permission_set_id: 'ps_gone' }, + SYSTEM, + ); + mem.readFailures.add('sys_position_permission_set'); + const report = await engine.inspectDanglingReferences(); + expect(report.dangling).toEqual([]); + expect(report.skipped.map((s) => s.object)).toContain('sys_position_permission_set'); + }); + + it('NEVER rewrites: the stored data is byte-identical across a scan', async () => { + await engine.insert( + 'sys_position_permission_set', + { id: 'bind_d', permission_set_id: 'ps_never_seeded' }, + SYSTEM, + ); + await engine.insert( + 'ref_task', + { id: 't_d', title: 'T', project: 'proj_gone', tags: ['tag_gone'] }, + SYSTEM, + ); + + const before = snapshot(mem.stores); + const report = await engine.inspectDanglingReferences(); + const after = snapshot(mem.stores); + + // The rows genuinely exist and were genuinely written; nulling a dangling + // link would make the stored data disagree with what happened. + expect(report.dangling.length).toBeGreaterThan(0); + expect(after).toEqual(before); + }); + + it('visits the RBAC link tables before anything else', async () => { + // Priority is an ORDERING, never a filter — both objects are reported. + await engine.insert('ref_task', { id: 't_p', title: 'T', project: 'gone_a' }, SYSTEM); + await engine.insert( + 'sys_position_permission_set', + { id: 'b_p', permission_set_id: 'gone_b' }, + SYSTEM, + ); + const report = await engine.inspectDanglingReferences(); + expect(REFERENCE_SCAN_PRIORITY_OBJECTS).toContain('sys_position_permission_set'); + expect(report.dangling.map((d) => d.object)) + .toEqual(['sys_position_permission_set', 'ref_task']); + }); + + it('marks the report truncated when a cap stops it short', async () => { + for (let i = 0; i < 4; i++) { + await engine.insert( + 'sys_position_permission_set', + { id: `bind_${i}`, permission_set_id: 'ps_never_seeded' }, + SYSTEM, + ); + } + const report = await engine.inspectDanglingReferences({ rowsPerObject: 2 }); + // A report that stopped early must not read as "everything was checked". + expect(report.truncated).toBe(true); + expect(report.scannedRows).toBe(2); + }); + + it('the write path still refuses the same row from a non-system caller (#4441 intact)', async () => { + // The sweep exists BECAUSE this door is the only one left open. + await expect( + engine.insert( + 'sys_position_permission_set', + { permission_set_id: 'ps_never_seeded' }, + { context: { userId: 'u1' } } as any, + ), + ).rejects.toMatchObject({ code: 'VALIDATION_FAILED' }); + }); +}); + +describe('[#4551] the inspection rides the existing lifecycle clock', () => { + it('runs after the sweep, and neither leg can stop the other', async () => { + const calls: string[] = []; + const logger = { info() {}, warn() { calls.push('warn'); }, debug() {} }; + const service = new LifecycleService({ + // No engine ⇒ `sweep()` is a no-op; the inspection must still run. + getEngine: () => undefined, + logger, + sweepIntervalMs: 10_000, + initialDelayMs: 0, + inspectDanglingReferences: async () => { calls.push('inspect'); }, + }); + + service.start(); + // One macrotask for the 0ms initial timer, one for the async tick. + await new Promise((r) => setTimeout(r, 5)); + service.stop(); + + expect(calls).toContain('inspect'); + }); + + it('a failing inspection does not take the lifecycle sweep down with it', async () => { + const warnings: string[] = []; + const logger = { info() {}, warn(msg: string) { warnings.push(msg); }, debug() {} }; + let swept = 0; + const service = new LifecycleService({ + getEngine: () => undefined, + logger, + sweepIntervalMs: 10_000, + initialDelayMs: 0, + inspectDanglingReferences: async () => { throw new Error('probe exploded'); }, + }); + // Count sweeps through the public method the clock calls. + const originalSweep = service.sweep.bind(service); + (service as any).sweep = async () => { swept += 1; return originalSweep(); }; + + service.start(); + await new Promise((r) => setTimeout(r, 5)); + service.stop(); + + expect(swept).toBe(1); + expect(warnings.some((w) => w.includes('probe exploded'))).toBe(true); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index ceb64f6041..cda1285dd9 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -598,6 +598,78 @@ function isEmptyReferenceValue(v: unknown): boolean { return false; } +/** + * [#4551] One stored reference whose target row does not exist. Every field is + * a locator: an operator has to be able to go straight to the row without + * reconstructing the query. + */ +export interface DanglingReference { + /** Object whose row holds the reference. */ + object: string; + /** Id of the holding row. */ + recordId: string; + /** Field whose value points at nothing. */ + field: string; + /** Object the field declares as its target. */ + target: string; + /** The id that resolves to no row in `target`. */ + value: string; + /** Tenant of the holding row, when the object is tenant-scoped. */ + organizationId?: string | null; +} + +/** [#4551] Result of one {@link ObjectQL.inspectDanglingReferences} pass. */ +export interface DanglingReferenceReport { + /** Objects actually read (excludes ones with no reference field, and skips). */ + scannedObjects: number; + /** Rows read across every scanned object. */ + scannedRows: number; + /** Reference values probed (after empty/expanded values are skipped). */ + scannedReferences: number; + /** References whose target row does not exist — the finding. */ + dangling: DanglingReference[]; + /** + * References whose target could NOT be probed (unregistered object, no + * driver, probe threw). NOT healthy — unknown. Counted so `dangling: []` + * can never be read as "all clear". + */ + undetermined: number; + /** Objects that could not be read at all, with the reason. */ + skipped: Array<{ object: string; reason: string }>; + /** A cap stopped the scan before every row was seen. */ + truncated: boolean; +} + +/** + * [#4551] Objects the dangling-reference scan visits FIRST. Ordering only — + * nothing is excluded by being absent. + * + * These are the RBAC link tables. A dangling row in one of them is a + * security-surface record that resolves to nothing: an administrator auditing + * permissions sees a binding whose target cannot be inspected, and the + * audience-anchor gate has to resolve that very permission set to evaluate the + * grant — so it is an unevaluable gate input, not merely an untidy row. + */ +export const REFERENCE_SCAN_PRIORITY_OBJECTS: readonly string[] = [ + 'sys_position_permission_set', + 'sys_user_permission_set', + 'sys_user_position', + 'sys_team_member', +]; + +function priorityRankOf(name: unknown): number { + if (typeof name !== 'string') return 0; + const idx = REFERENCE_SCAN_PRIORITY_OBJECTS.indexOf(name); + return idx === -1 ? 0 : REFERENCE_SCAN_PRIORITY_OBJECTS.length - idx; +} + +/** Rows the dangling-reference scan reads per object in one pass. */ +export const DANGLING_SCAN_ROWS_PER_OBJECT = 1_000; +/** Rows the dangling-reference scan reads across one whole pass. */ +export const DANGLING_SCAN_MAX_ROWS = 5_000; +/** Findings named individually in the scan's log line (the rest are counted). */ +const DANGLING_SCAN_LOG_SAMPLE = 20; + export class ObjectQL implements IObjectQLEngine { /** * Ambient transaction store (ADR-0034). While a `transaction()` callback @@ -2106,6 +2178,195 @@ export class ObjectQL implements IObjectQLEngine { } } + /** + * [#4551] Report stored references that resolve to nothing — the residual + * #4441 deliberately left open. + * + * {@link assertReferencesResolve} exempts `isSystem` writes, and that + * exemption is correct: seed replay, package install and boot-time + * provisioning legitimately write rows in an order that is self-consistent + * only once the batch completes, so failing them closed would turn an + * ordering detail into a boot failure. What it leaves behind is a gap the + * platform can widen by itself — **the platform can still write a reference + * pointing at nothing, and nothing says so.** + * + * **Reports; never rewrites.** Same posture as the stranded-request + * inspection (#4469), for the same reason: the rows genuinely exist and were + * genuinely written. Nulling a dangling link would make the stored data + * disagree with what happened, and whether the right repair is to create the + * missing target, re-run the seed, or delete the holder is a judgement this + * cannot make. What an operator needs first is to know: which object, which + * row, which field, which id, in which target. + * + * **Unknown is not healthy.** The probe is {@link referenceExists} — the + * SAME one the write path enforces with, so the two can never disagree about + * what "exists" means. It answers `false` only when it RAN and found + * nothing; `null` (target unregistered, no driver, probe threw) is counted + * into `undetermined` and never reported as dangling. Without that split a + * datasource outage would read as a clean bill of health. + * + * ## Scope + * + * - **Non-`readonly` reference fields**, matching the write-path check's own + * domain (`referenceTargetOf` + the `readonly` skip). A `readonly` + * reference holds a value the platform minted, not a caller's id, and at + * least one of them stores a SENTINEL by design — + * `sys_metadata_history.recorded_by` is a `lookup('sys_user')` filled with + * `actor ?? 'system'`. Reporting that every hour would be a permanent + * false positive, and it is filed on its own terms rather than drowned + * here. + * - **Empty values are not references** (`isEmptyReferenceValue`), and an + * already-expanded `{id, …}` in the slot is not an id write — both skipped + * exactly as the write path skips them. + * - **RBAC link tables first** ({@link REFERENCE_SCAN_PRIORITY_OBJECTS}). A + * dangling row there is a security-surface record that resolves to + * nothing, and the audience-anchor gate has to resolve that very target to + * evaluate the grant. Ordering only — every other object is still scanned, + * but if a budget runs out the rows that matter most are already covered. + * + * ## Bounds + * + * A full referential scan of every table is not something to run hourly + * unbounded, so both the per-object read and the total are capped. Hitting a + * cap sets `truncated`, which is part of the same honesty rule as + * `undetermined`: a report that stopped early must not read as "everything + * was checked". Probe results are memoized per scan, so N rows pointing at + * one missing id cost one probe. + */ + async inspectDanglingReferences(options?: { + /** Rows read per object. Default {@link DANGLING_SCAN_ROWS_PER_OBJECT}. */ + rowsPerObject?: number; + /** Rows read across the whole scan. Default {@link DANGLING_SCAN_MAX_ROWS}. */ + maxRows?: number; + /** Restrict the scan to these objects (tooling / tests); default = all. */ + objects?: string[]; + }): Promise { + const report: DanglingReferenceReport = { + scannedObjects: 0, + scannedRows: 0, + scannedReferences: 0, + dangling: [], + undetermined: 0, + skipped: [], + truncated: false, + }; + + const rowsPerObject = Math.max(1, options?.rowsPerObject ?? DANGLING_SCAN_ROWS_PER_OBJECT); + const maxRows = Math.max(1, options?.maxRows ?? DANGLING_SCAN_MAX_ROWS); + + let objects: any[]; + try { + objects = (this._registry.getAllObjects?.() as any[]) ?? []; + } catch (err: any) { + this.logger.warn?.('[dangling-refs] could not enumerate objects; scan skipped', { + error: err?.message ?? String(err), + }); + return report; + } + if (options?.objects) { + const wanted = new Set(options.objects); + objects = objects.filter((o) => wanted.has(o?.name)); + } + // Priority is an ordering, never a filter — see the scope note above. + objects = [...objects].sort( + (a, b) => priorityRankOf(b?.name) - priorityRankOf(a?.name), + ); + + // `${target}\u0000${id}` → probe result, so N holders of one missing id + // cost one probe. Scoped to this scan: a later sweep must re-ask. + const probed = new Map(); + + for (const obj of objects) { + const name = obj?.name; + if (typeof name !== 'string' || !name) continue; + const fieldDefs = obj?.fields; + if (!fieldDefs || typeof fieldDefs !== 'object') continue; + + const candidates: Array<{ field: string; target: string }> = []; + for (const field of Object.keys(fieldDefs)) { + const def = (fieldDefs as any)[field]; + if (def?.readonly === true) continue; + const target = referenceTargetOf(def); + if (target) candidates.push({ field, target }); + } + if (candidates.length === 0) continue; + + if (report.scannedRows >= maxRows) { + report.truncated = true; + break; + } + + const select = ['id', ...candidates.map((c) => c.field)]; + if ((fieldDefs as any).organization_id) select.push('organization_id'); + const limit = Math.min(rowsPerObject, maxRows - report.scannedRows); + + let rows: any[]; + try { + rows = (await this.find(name, { + fields: select, + limit, + context: { isSystem: true }, + } as any)) ?? []; + } catch (err: any) { + // Unreadable object ⇒ unknown, not clean. Named in `skipped` rather + // than silently dropped, for the same reason `undetermined` exists. + report.skipped.push({ object: name, reason: err?.message ?? String(err) }); + continue; + } + + report.scannedObjects += 1; + report.scannedRows += rows.length; + if (rows.length >= limit) report.truncated = true; + + for (const row of rows) { + for (const { field, target } of candidates) { + const raw = row?.[field]; + if (isEmptyReferenceValue(raw)) continue; + const values = Array.isArray(raw) ? raw : [raw]; + for (const v of values) { + if (v === null || v === undefined || v === '') continue; + if (typeof v === 'object') continue; // already expanded — not an id write + report.scannedReferences += 1; + const key = `${target}\u0000${String(v)}`; + let resolved = probed.get(key); + if (resolved === undefined) { + resolved = await this.referenceExists(target, v); + probed.set(key, resolved); + } + if (resolved === null) { report.undetermined += 1; continue; } + if (resolved === true) continue; + report.dangling.push({ + object: name, + recordId: row?.id === undefined || row?.id === null ? '' : String(row.id), + field, + target, + value: String(v), + organizationId: row?.organization_id ?? null, + }); + } + } + } + } + + if (report.dangling.length || report.undetermined || report.truncated) { + this.logger.warn?.( + '[dangling-refs] stored references that resolve to nothing (reported, never rewritten)', + { + scannedObjects: report.scannedObjects, + scannedRows: report.scannedRows, + scannedReferences: report.scannedReferences, + dangling: report.dangling.length, + undetermined: report.undetermined, + truncated: report.truncated, + references: report.dangling + .slice(0, DANGLING_SCAN_LOG_SAMPLE) + .map((d) => `${d.object}.${d.field}[${d.recordId}] → ${d.target}:${d.value}`), + }, + ); + } + return report; + } + /** * 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..fa827d9125 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -61,6 +61,14 @@ export type { DatasourceUnavailableKind, } from './driver-connect-errors.js'; export type { InsertManyRowOutcome } from './engine.js'; +// #4551 — the read-only dangling-reference inspection's report shape, and the +// bounds/priority the scan runs under. Reports; never rewrites. +export type { DanglingReference, DanglingReferenceReport } from './engine.js'; +export { + REFERENCE_SCAN_PRIORITY_OBJECTS, + DANGLING_SCAN_ROWS_PER_OBJECT, + DANGLING_SCAN_MAX_ROWS, +} from './engine.js'; // Export in-memory aggregation fallback (used by engine.aggregate when the // driver lacks native groupBy/aggregations support; also useful for tests). diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index f51c045108..4cc900ef4b 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -118,6 +118,26 @@ export interface LifecycleServiceOptions { getSettings?(): LifecycleSettingsLike | undefined; /** Governance alert sink. Defaults to a logger warning. */ onAlert?(alert: LifecycleGovernanceAlert): void; + /** + * [#4551] A read-only inspection that rides THIS clock instead of arming a + * second one of its own — the dangling-reference scan (`ObjectQL. + * inspectDanglingReferences`). + * + * It is deliberately not part of {@link LifecycleService.sweep}: `sweep()` + * is called directly by tooling (`db:clean`, the dogfood growth gate) that + * asked for policy enforcement and nothing else, and it must keep answering + * exactly that. What the two share is the *clock* — hourly, first run + * delayed past boot so seeding and migrations have finished, which is + * precisely the window a scan of seed-written references needs. + * + * Runs AFTER the sweep, not alongside it. Concurrent would race the reaper's + * deletes (a target reaped mid-scan reads as a fresh dangling reference) and + * would put two full table walks in flight at once. + * + * Independent: a failure in either leg is logged and never affects the + * other. Absent ⇒ the clock runs the sweep alone, exactly as before. + */ + inspectDanglingReferences?(): Promise; } /** Per-sweep governance snapshot resolved from the `lifecycle` namespace. */ @@ -251,13 +271,37 @@ export class LifecycleService { const initial = this.opts.initialDelayMs ?? DEFAULT_LIFECYCLE_INITIAL_DELAY_MS; this.initialTimer = setTimeout(() => { this.initialTimer = undefined; - void this.sweep(); - this.timer = setInterval(() => void this.sweep(), interval); + void this.tick(); + this.timer = setInterval(() => void this.tick(), interval); this.timer.unref?.(); }, initial); this.initialTimer.unref?.(); } + /** + * One turn of the clock: the lifecycle sweep, then the read-only + * inspections that ride the same schedule ({@link + * LifecycleServiceOptions.inspectDanglingReferences}). + * + * Each leg is isolated — an inspection failure must never stop retention + * from being enforced, and a sweep failure must never hide a finding. + */ + private async tick(): Promise { + try { + await this.sweep(); + } catch (err) { + this.opts.logger.warn(`[lifecycle] sweep failed (${(err as Error)?.message ?? String(err)})`); + } + if (!this.opts.inspectDanglingReferences) return; + try { + await this.opts.inspectDanglingReferences(); + } catch (err) { + this.opts.logger.warn( + `[dangling-refs] inspection failed (${(err as Error)?.message ?? String(err)})`, + ); + } + } + stop(): void { if (this.initialTimer) clearTimeout(this.initialTimer); if (this.timer) clearInterval(this.timer); diff --git a/packages/objectql/src/plugin.ts b/packages/objectql/src/plugin.ts index 17f989552b..91b2318038 100644 --- a/packages/objectql/src/plugin.ts +++ b/packages/objectql/src/plugin.ts @@ -293,6 +293,15 @@ export class ObjectQLPlugin implements Plugin { return undefined; } }, + // #4551 — the residual #4441 left open: an `isSystem` write (seed + // replay, package install, boot provisioning) can still store a + // reference that points at nothing, and until now nothing said so. The + // scan is READ-ONLY; it rides this clock rather than arming its own so + // the finding surfaces without an operator knowing to go looking, and + // because this clock's first run is already delayed past boot — the + // exact moment seed-written references become checkable. + inspectDanglingReferences: () => + this.ql?.inspectDanglingReferences() ?? Promise.resolve(), ...this.lifecycleOptions, }); ctx.registerService('lifecycle', this.lifecycleService);