Skip to content

Commit d6bd5a1

Browse files
os-zhuangclaude
andauthored
feat(objectql): 只报告不拦截的悬空 lookup 引用巡检 (#4551) (#4555)
* feat(objectql): report stored lookup references that resolve to nothing (#4551) #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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 * fix(objectql): spell the audit NUL key separator as \u0000, not a raw byte 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a7163ea commit d6bd5a1

9 files changed

Lines changed: 1158 additions & 0 deletions

File tree

.changeset/tidy-eyes-shine.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
Report stored `lookup` references that resolve to nothing (#4551)
6+
7+
#4441 made the write path refuse an unresolvable reference id, but deliberately
8+
exempted `isSystem` writes so seed replay, package install and boot-time
9+
provisioning keep their ordering freedom. That exemption is unchanged — and it
10+
left a residual: the platform itself could still write a reference into the void
11+
with nothing saying so.
12+
13+
New: `ObjectQL.inspectDanglingReferences()` — a **read-only** audit that walks
14+
stored rows and reports every non-`readonly` `lookup` / `master_detail` /
15+
`user` / `tree` value that names no row of its declared target. It runs as a leg
16+
of the existing `LifecycleService` sweep, so the finding surfaces without an
17+
operator knowing to go looking for it.
18+
19+
- **It never rewrites.** The rows were genuinely written; auto-nulling a
20+
dangling id would make the stored data disagree with what happened, and the
21+
remedy (re-seed the target vs. clear the link) is an operator's call.
22+
- **Unknown is not absent.** A probe that cannot run (target unregistered, no
23+
driver, probe throws) counts as `undetermined`; an object whose rows cannot be
24+
listed lands in `unreadableObjects`; a run that hits its row budget names the
25+
object in `truncatedObjects`. So `dangling: []` can never be misread as
26+
"everything is fine".
27+
- **RBAC link tables are scanned first** (`sys_position_permission_set` and the
28+
rest of `plugin-security`'s tables, derived from `PLATFORM_OBJECTS_BY_PACKAGE`):
29+
a dangling row there is a security-surface record resolving to nothing, and
30+
the audience-anchor gate must resolve exactly that permission set to evaluate
31+
the grant.
32+
33+
The existence oracle is the engine's own — the same predicate #4441's write-path
34+
guard uses — so the report can never be stricter or looser than the rule it
35+
reports on.
36+
37+
Tuning: `ObjectQLPlugin`'s `lifecycle.referenceAudit` (`enabled`, `rowsPerObject`,
38+
`maxRows`, `objects`). Nothing is authorable in metadata; no spec key was added.
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#4551] The audit against the REAL engine — end to end on the exact residual
5+
* #4441 documented.
6+
*
7+
* #4441's write-path guard exempts `isSystem` writes on purpose: seed replay,
8+
* package install and boot provisioning write in an order that only resolves
9+
* once the batch completes, and failing them closed turns an ordering detail
10+
* into a boot failure. That exemption stays. What this pins is the other half —
11+
* the row a system write leaves behind is now SAID OUT LOUD.
12+
*
13+
* The two halves must also stay in agreement, which is what makes this file
14+
* worth having on top of the unit suite (#4550: a stand-in must never be looser
15+
* than the real implementation). Here the audit runs on the real `ObjectQL`
16+
* with the real driver and the real `referenceExists`, so if the enforcement's
17+
* probe and the audit's probe ever diverge, these tests are where it shows.
18+
*/
19+
20+
import { describe, it, expect, beforeEach } from 'vitest';
21+
import { ObjectQL } from './engine.js';
22+
23+
const permissionSet = {
24+
name: 'aud_permission_set',
25+
label: 'Permission Set',
26+
fields: {
27+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
28+
name: { name: 'name', label: 'Name', type: 'text' as const },
29+
},
30+
};
31+
32+
/** The RBAC link-table shape: the binding an audience gate must resolve. */
33+
const binding = {
34+
name: 'aud_position_permission_set',
35+
label: 'Binding',
36+
fields: {
37+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
38+
permission_set_id: {
39+
name: 'permission_set_id', label: 'Permission Set',
40+
type: 'lookup' as const, reference: 'aud_permission_set',
41+
required: true, deleteBehavior: 'set_null' as const,
42+
},
43+
},
44+
};
45+
46+
const history = {
47+
name: 'aud_history',
48+
label: 'History',
49+
fields: {
50+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
51+
note: { name: 'note', label: 'Note', type: 'text' as const },
52+
// `sys_metadata_history.recorded_by` in miniature: a readonly lookup the
53+
// platform fills with the sentinel string `actor ?? 'system'`.
54+
recorded_by: {
55+
name: 'recorded_by', label: 'Recorded By',
56+
type: 'lookup' as const, reference: 'aud_permission_set', readonly: true,
57+
},
58+
},
59+
};
60+
61+
function makeMemoryDriver() {
62+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
63+
const storeFor = (obj: string) => {
64+
let s = stores.get(obj);
65+
if (!s) { s = new Map(); stores.set(obj, s); }
66+
return s;
67+
};
68+
/** Read path: must NOT materialise a store, or a pure read would show up in
69+
* the "nothing was written" snapshot as a change. */
70+
const peek = (obj: string) => stores.get(obj) ?? new Map<string, Record<string, unknown>>();
71+
let nextId = 0;
72+
const matches = (row: Record<string, unknown>, where: any): boolean => {
73+
if (!where || typeof where !== 'object') return true;
74+
for (const [k, v] of Object.entries(where)) {
75+
if (k.startsWith('$')) continue;
76+
const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
77+
if ((row[k] ?? null) !== (expected ?? null)) return false;
78+
}
79+
return true;
80+
};
81+
const driver: any = {
82+
name: 'memory', version: '0.0.0', supports: {} as any,
83+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
84+
async execute() { return null; },
85+
async find(object: string, ast: any) {
86+
const rows = Array.from(peek(object).values()).filter((r) => matches(r, ast?.where));
87+
// A real driver honours `limit`; a double that ignored it would make the
88+
// audit's bounded-scan reporting untestable AND looser than production.
89+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
90+
},
91+
findStream() { throw new Error('not implemented'); },
92+
async findOne(object: string, ast: any) {
93+
for (const r of peek(object).values()) if (matches(r, ast?.where)) return r;
94+
return null;
95+
},
96+
async create(object: string, data: Record<string, unknown>) {
97+
nextId += 1;
98+
const id = (data.id as string) ?? `r_${nextId}`;
99+
const row = { ...data, id };
100+
storeFor(object).set(id, row);
101+
return row;
102+
},
103+
async update(object: string, id: string, data: Record<string, unknown>) {
104+
const s = storeFor(object);
105+
const cur = s.get(id);
106+
if (!cur) return null;
107+
const next = { ...cur, ...data, id };
108+
s.set(id, next);
109+
return next;
110+
},
111+
async upsert(object: string, data: Record<string, unknown>) {
112+
const id = data.id as string | undefined;
113+
if (id && storeFor(object).has(id)) return this.update(object, id, data);
114+
return this.create(object, data);
115+
},
116+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
117+
async count(object: string, ast: any) { return (await this.find(object, ast)).length; },
118+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
119+
return Promise.all(rows.map((r) => this.create(object, r)));
120+
},
121+
async bulkUpdate() { return []; },
122+
async bulkDelete() {},
123+
async updateMany(object: string, ast: any, data: Record<string, unknown>) {
124+
const rows = await this.find(object, ast);
125+
for (const r of rows) storeFor(object).set(r.id as string, { ...r, ...data, id: r.id });
126+
return rows.length;
127+
},
128+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
129+
async commit() {}, async rollback() {},
130+
};
131+
return { driver, stores };
132+
}
133+
134+
describe('[#4551] the engine reports the dangling rows its own `isSystem` exemption allows', () => {
135+
let engine: ObjectQL;
136+
let stores: Map<string, Map<string, Record<string, unknown>>>;
137+
const userCtx = { userId: 'u1' };
138+
139+
beforeEach(async () => {
140+
engine = new ObjectQL();
141+
const mem = makeMemoryDriver();
142+
stores = mem.stores;
143+
engine.registerDriver(mem.driver, true);
144+
await engine.init();
145+
engine.registry.registerObject(permissionSet as any);
146+
engine.registry.registerObject(binding as any);
147+
engine.registry.registerObject(history as any);
148+
await engine.insert('aud_permission_set', { id: 'ps_real', name: 'Real' }, { context: { isSystem: true } } as any);
149+
});
150+
151+
it('the residual, stated: a system write lands a dangling binding and the audit names it', async () => {
152+
// This is the write #4441 deliberately lets through.
153+
await engine.insert(
154+
'aud_position_permission_set',
155+
{ id: 'ppr_1', permission_set_id: 'ps_never_seeded' },
156+
{ context: { isSystem: true } } as any,
157+
);
158+
159+
const out = await engine.inspectDanglingReferences({ objects: ['aud_position_permission_set'] });
160+
161+
expect(out.undetermined).toBe(0);
162+
expect(out.dangling).toHaveLength(1);
163+
expect(out.dangling[0]).toEqual({
164+
objectName: 'aud_position_permission_set',
165+
recordId: 'ppr_1',
166+
field: 'permission_set_id',
167+
target: 'aud_permission_set',
168+
value: 'ps_never_seeded',
169+
});
170+
});
171+
172+
it('…and the enforcement it reports on is untouched — a caller write is still refused', async () => {
173+
// #4551 is a REPORT. If this ever passes, the audit was smuggled into the
174+
// write path, which the issue explicitly forbids.
175+
await expect(
176+
engine.insert(
177+
'aud_position_permission_set',
178+
{ permission_set_id: 'ps_never_seeded' },
179+
{ context: userCtx } as any,
180+
),
181+
).rejects.toMatchObject({ name: 'ValidationError' });
182+
});
183+
184+
it('a resolvable binding is not reported', async () => {
185+
await engine.insert(
186+
'aud_position_permission_set',
187+
{ id: 'ppr_ok', permission_set_id: 'ps_real' },
188+
{ context: { isSystem: true } } as any,
189+
);
190+
const out = await engine.inspectDanglingReferences({ objects: ['aud_position_permission_set'] });
191+
expect(out.dangling).toEqual([]);
192+
expect(out.scanned).toBe(1);
193+
});
194+
195+
it('the audit issues NO writes — the stored rows are byte-identical afterwards', async () => {
196+
await engine.insert(
197+
'aud_position_permission_set',
198+
{ id: 'ppr_1', permission_set_id: 'ps_never_seeded' },
199+
{ context: { isSystem: true } } as any,
200+
);
201+
const snapshot = (): string =>
202+
JSON.stringify([...stores].map(([k, v]) => [k, [...v.entries()]]));
203+
const before = snapshot();
204+
205+
const out = await engine.inspectDanglingReferences();
206+
207+
expect(out.dangling.length).toBeGreaterThan(0);
208+
expect(snapshot()).toBe(before);
209+
});
210+
211+
it('a readonly lookup holding a SENTINEL string is not reported', async () => {
212+
// `recorded_by: 'system'` is not a user id and never was. #4441 skips it on
213+
// the write path; the audit must not undo that by reporting the same value
214+
// from the other side.
215+
await engine.insert(
216+
'aud_history', { id: 'h1', note: 'n', recorded_by: 'system' }, { context: { isSystem: true } } as any,
217+
);
218+
const out = await engine.inspectDanglingReferences({ objects: ['aud_history'] });
219+
expect(out.dangling).toEqual([]);
220+
expect(out.undetermined).toBe(0);
221+
});
222+
223+
it('an unregistered TARGET is `undetermined`, not a finding', async () => {
224+
// Exactly the case `referenceExists` answers `null` for — and the audit and
225+
// the write-path guard read that `null` the same way: the write is allowed
226+
// through, and the audit declines to condemn the row it produced.
227+
engine.registry.registerObject({
228+
name: 'aud_orphan',
229+
label: 'Orphan',
230+
fields: {
231+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
232+
other: { name: 'other', label: 'Other', type: 'lookup' as const, reference: 'not_registered_anywhere' },
233+
},
234+
} as any);
235+
await engine.insert('aud_orphan', { id: 'o1', other: 'whatever' }, { context: userCtx } as any);
236+
237+
const out = await engine.inspectDanglingReferences({ objects: ['aud_orphan'] });
238+
expect(out.dangling).toEqual([]);
239+
expect(out.undetermined).toBe(1);
240+
});
241+
});

packages/objectql/src/engine.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, Validat
8080
import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js';
8181
import { applyInMemoryAggregation } from './in-memory-aggregation.js';
8282
import { applyHaving } from './having-filter.js';
83+
import {
84+
auditDanglingReferences,
85+
type AuditableObject,
86+
type DanglingReferenceAuditOptions,
87+
type DanglingReferenceReport,
88+
} from './integrity/dangling-reference-audit.js';
8389

8490
/**
8591
* The lifecycle events the engine actually dispatches via `triggerHooks`. This
@@ -2106,6 +2112,39 @@ export class ObjectQL implements IObjectQLEngine {
21062112
}
21072113
}
21082114

2115+
/**
2116+
* [#4551] Report stored references that resolve to nothing. **Read-only** —
2117+
* this issues no writes at all.
2118+
*
2119+
* The follow-up to {@link assertReferencesResolve}'s deliberate `isSystem`
2120+
* exemption. That exemption stays exactly as #4441 wrote it (seed replay and
2121+
* boot provisioning must keep their ordering freedom); what it left behind is
2122+
* a residual — the platform itself can still write a reference into the void
2123+
* and nothing says so. This is the "something says so".
2124+
*
2125+
* The existence oracle passed to the audit is **this engine's own**
2126+
* {@link referenceExists}, not a second copy: the audit and the write-path
2127+
* guard therefore answer "does this id exist" — and "could I even tell?" —
2128+
* with one predicate, so the report can never be more or less strict than the
2129+
* rule it reports on.
2130+
*
2131+
* See {@link auditDanglingReferences} for the judgments (readonly skip, empty
2132+
* values, unknown ≠ absent) and the bounded-scan honesty of the report.
2133+
*/
2134+
async inspectDanglingReferences(
2135+
options?: DanglingReferenceAuditOptions,
2136+
): Promise<DanglingReferenceReport> {
2137+
return auditDanglingReferences(
2138+
{
2139+
objects: () => this._registry.getAllObjects() as unknown as AuditableObject[],
2140+
find: (object, opts) => this.find(object, opts as any) as Promise<Array<Record<string, unknown>>>,
2141+
probe: (target, id) => this.referenceExists(target, id),
2142+
warn: (msg, meta) => this.logger?.warn?.(msg, meta as any),
2143+
},
2144+
options,
2145+
);
2146+
}
2147+
21092148
/**
21102149
* Register the crypto provider that backs `secret`-typed fields.
21112150
*

packages/objectql/src/index.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,22 @@ export type {
124124
export { parseLifecycleDuration } from './lifecycle/duration.js';
125125
export { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js';
126126

127+
// [#4551] Read-only referential-integrity audit — the reporting half of the
128+
// `isSystem` exemption #4441 deliberately left in the write-path guard.
129+
export {
130+
auditDanglingReferences,
131+
SECURITY_SURFACE_OBJECTS,
132+
DEFAULT_ROWS_PER_OBJECT,
133+
DEFAULT_MAX_ROWS,
134+
} from './integrity/dangling-reference-audit.js';
135+
export type {
136+
DanglingReference,
137+
DanglingReferenceReport,
138+
DanglingReferenceAuditOptions,
139+
DanglingReferenceAuditPort,
140+
AuditableObject,
141+
} from './integrity/dangling-reference-audit.js';
142+
127143
// Export MetadataFacade
128144
export { MetadataFacade } from './metadata-facade.js';
129145

0 commit comments

Comments
 (0)