Skip to content

Commit 5018fc3

Browse files
fix(plugin-audit): an object definition changed through the metadata door takes effect without a restart (#24676)
Fixes #24565 Clause-②: no ## What changed `installAuditWriters` (`packages/plugins/plugin-audit/src/audit-writers.ts`) kept two process-lifetime copies of `engine.getSchema`: `objectDefCache` behind `getObjectDef` and `fieldDefsCache` behind `getFieldDefs`. Each object's FIRST answer was held until a restart. Both copies are gone. `getObjectDef` now reads the registered definition on every call, and `getFieldDefs` reads `.fields` off that same read. That covers every reader at once: the `enable.files` / `enable.feeds` capability gates, the ledger's credential mask and `internal` omission (`ledgerView`), the activity mirror's `enable.activities` switch, `activityMilestones`, the title field, the object label and the computed-field drop. What the metadata door accepts is unchanged. ## Measured before choosing the route (on `f89d29e38`, real kernel) Probe: `ObjectKernel` + `ObjectQLPlugin` (its in-house metadata protocol registered as the `protocol` service) + `AuditPlugin` + `SqliteWasmDriver`. It saved an object twice through `protocol.saveMetaItem` (the producer behind `PUT /api/v1/meta/object/:name`) and listened on every channel. - **The defect reproduces.** First save, no `enable`, then the attachment insert answers `FILES_DISABLED 403`. Second save with `enable: { files: true }`: `success=true`, and `engine.getSchema('qa_kid').enable.files` reads `true`. The same insert still answers `FILES_DISABLED 403`. - **Which signal fires on the door's save (A2):** only `protocol.onMetadataMutation` (`object/qa_kid active`), and it fires AFTER the registry holds the new definition (`files=true` at fire time). `metadata:reloaded` does NOT fire on this save. It fires on per-item publish, package publish-drafts and dev reload. The metadata service on this composition has no `subscribe` at all. - **Cost of reading per call (A3):** `ObjectQL.getSchema` is `registry.getObject`, which is two map reads over the registry's own memoized merge (`SchemaRegistry.resolveObject` / `mergedObjectCache`). It measured 32 to 44 ns per call against 7 ns for a bare `Map.get` (1,000,000 calls, shared box). The audited writes ask more often now: one audited insert went from 16 `getSchema` calls to 23, and one audited update from 9 to 25. That is under 1 microsecond per write, against a mean audited insert of 3,301 us before and 3,077 us after on the same harness (300 inserts each, shared-box noise). ## Route: drop the copy, not invalidate it (four axes) The card's triage text says "invalidates on the metadata change event". The dispatch offered dropping the copy as the alternative, to be chosen on the four axes. This PR drops it: - **Real business need.** Both routes fix the measured door. But the door announces on a channel `metadata:reloaded` subscribers never see, and the other doors that change a definition (per-item publish, package publish, dev reload with `notify: false`, a peer replica's replay, a delete, a plugin's own `registerObject`) announce on different channels or not at all. A read per call follows all of them by construction. An invalidation follows only the channels it subscribes to. - **Long-term soundness.** The registry is the one owner of an object's current definition and already memoizes it behind its own invalidation points. Its own docblock (`resolveOwnerLayer`) refuses a second cache for this reason. This package already reads per call where the definition decides: `declaresActivityStampColumns` on every `sys_activity` insert, and `currentMilestoneWatched` on the read side. - **Keeping AI-authored metadata honest.** A declaration takes effect on the next write, whatever door changed it. There is no announce for a future door to forget. - **Startup scope.** The smallest change: two maps deleted, no subscription, no new gate. ⛔ No TTL either: an expiring copy is the same stale answer for a shorter time. ## Tests New `audit-writers.definition-follows-metadata.integration.test.ts`, on the same real composition as the probe. Every definition change goes through `protocol.saveMetaItem`, and there is no restart: 1. Object saved with no `enable`: attach refused (`FILES_DISABLED` / 403). Saved again with `enable.files: true`: the read-back shows `true` and the same attach is admitted. 2. Reverse: saved with `files: true`, attach admitted. Saved with `files: false`: the same attach is refused again. 3. Control (the card's): authored with `enable.files: true` from the start, never looked up before: admitted. 4. Second reader, the ledger mask: an object audited once, then a `password` field added through the door, then the record updated. The `sys_audit_log` row's `new_value` carries the mask (`SECRET_MASK`), never the value. **Ablation** (on committed `56218f119`, through `scripts/ablation-replace.mjs` in wrap mode with a restore trap): the held copy was put back into `getObjectDef`. The anchor hit once (1 to 0), and the blob went from `e5840a3a6b5b` to `8bfd1f9e7ff2`. Cases 1, 2 and 4 went red; control 3 stayed green: - 1: `expected { code: 'FILES_DISABLED', status: 403 } to deeply equal { admitted: true }` - 2: `expected { admitted: true } to deeply equal { code: 'FILES_DISABLED', status: 403 }` - 4: red at its masking assertion (the ledger row carried the field's value, not the mask). The restore was proven by the tool: blob after restore equals the HEAD blob, and `git diff HEAD` is empty. ## Local verification (head `30a57ba9d`) - `pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2` (whole package, on `56218f119`): 58 files, 934 tests passed. The only later commit (`30a57ba9d`) types the new test's option bags. On that head the new file reran 4/4 green, and `pnpm --filter @objectstack/plugin-audit typecheck` exited 0, with `check:test-typecheck: OK` and 0 errors. - Build: the dependency closure first (`turbo run build --filter=@objectstack/plugin-audit^...`, 15/15), then the package itself. All of it ran under `scripts/pm/os-verify-lock.sh`. - The gate union from `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (70 commands) was rerun in full on `30a57ba9d`. All 70 exited 0. `dispatch-gates --ran` reads: `70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN`. - In the first pass, on `56218f119`, `check:query-options-erasure` was red: the test surface grew from 236 to 237 sites because the new test passed its option bags `as any`. They are now typed with `satisfies`, and the surface is back at 236. - Also in that first pass, `check:dual-build-cjs-loads` and `check:i18n` exited 3 (PREREQUISITE NOT MET: no workspace `dist/`). Both measured green on the final head. - The `test:repo` project and the full CI farm are left to CI. ## Acceptance notes - **Rows written before this fix:** a security note on them is held off GitHub for the maintainer (#9387). The seat carries it. - **Same family, other package, not measured through a door:** `@objectstack/metadata-core`'s `createFieldPresenceProbe` (field-set cache) and `createResolver` (organization-column cache) also hold a per-object answer for the process lifetime. Their docblock premise reads "object schemas are static after registration". In this file they are asked about `sys_user`, `sys_audit_log` and `sys_activity`, and about the audited object's organization column. They sit outside this card's file surface and were left untouched. - `sys_attachment` in the test is a stand-in under the real name (service-storage owns the definition). The gate reads only `parent_object`. --- _Generated by [Claude Code](https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0431002 commit 5018fc3

3 files changed

Lines changed: 203 additions & 26 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
fix(plugin-audit): an object definition changed through the metadata door takes effect in the audit writers at once, with no restart
6+
7+
Clause-②: no
8+
9+
The audit writers kept each object's first definition for the life of the process. They now read the registered definition on every call. The registry already holds the merged definition behind its own invalidation points, so the read is two map lookups.
10+
11+
- **Attachments follow `enable.files`.** An object whose `enable.files` was switched on through `PUT /api/v1/meta/object/:name` kept refusing `sys_attachment` writes with 403 `FILES_DISABLED` until the server restarted, although the metadata read-back already showed `enable.files: true`. The same attachment is now admitted at once, and switching the flag off refuses it again. `enable.feeds` on comments follows the same read.
12+
- **The ledger copy follows field changes.** A `password` field added through the door to an already-audited object is masked on the ledger copy from the moment it is added, as on every other read exit. The same read decides the mask for `secret` fields and the omission of `internal` ones.
13+
- **The activity row follows the object.** `enable.activities`, `activityMilestones`, the title field and the object label are read from the current definition too.
14+
15+
What the metadata door accepts is unchanged.
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
// [#24565] The real-engine pin that the audit writers answer from an object's
4+
// CURRENT definition. It boots a kernel, the metadata door, a SQLite driver and
5+
// the audit plugin, so it lives in its own file. Static imports, so the load is
6+
// paid during collection, not by one test's timeout.
7+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
8+
import { ObjectKernel } from '@objectstack/core';
9+
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
10+
import type { DataEngineInsertOptions, EngineQueryOptions, EngineUpdateOptions } from '@objectstack/spec/data';
11+
import { SECRET_MASK } from '@objectstack/objectql/core';
12+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
13+
import { AuditPlugin } from './audit-plugin.js';
14+
import { singlePostureTenancy } from './__tests__/single-posture-tenancy.js';
15+
16+
/**
17+
* [#24565] A definition changed through the metadata door takes effect in the
18+
* audit writers at once, with no restart.
19+
*
20+
* `installAuditWriters` used to keep each object's first `getSchema` answer for
21+
* the life of the process. An object whose `enable.files` was switched on
22+
* through the door kept refusing attachments with 403 `FILES_DISABLED` until a
23+
* restart, while the door's own read-back showed the new value. The same copy
24+
* fed every other reader, the ledger's credential mask among them.
25+
*
26+
* The composition is the one the door runs in: `ObjectKernel` +
27+
* `ObjectQLPlugin` (which assembles the metadata protocol and registers it as
28+
* the `protocol` service) + `AuditPlugin` over a real SQLite driver. Every
29+
* definition change goes through `protocol.saveMetaItem`, the producer behind
30+
* `PUT /api/v1/meta/object/:name`, and nothing else touches the registry
31+
* between the two writes a case compares. Each case uses its own object, and
32+
* its first write makes the writers look the definition up BEFORE the change,
33+
* which is the lookup a held copy kept.
34+
*
35+
* `sys_attachment` is a stand-in under the real name: service-storage owns the
36+
* definition, and the gate reads only `parent_object`.
37+
*/
38+
describe('audit writers answer from the current definition after a metadata-door change (#24565)', () => {
39+
const HARNESS_PACKAGE = 'com.objectstack.audit.test.definition-follows-metadata';
40+
const SYSTEM = { context: { isSystem: true } } satisfies DataEngineInsertOptions;
41+
42+
let kernel: ObjectKernel;
43+
let engine: ObjectQL;
44+
let protocol: { saveMetaItem(request: Record<string, unknown>): Promise<{ success?: boolean }> };
45+
46+
const text = (name: string) => ({ name, label: name, type: 'text' as const });
47+
48+
/** An object body as an author saves it through the door. */
49+
const objectBody = (name: string, extra: Record<string, unknown> = {}) => ({
50+
name,
51+
label: name,
52+
// The runtime object door requires an authored OWD.
53+
sharingModel: 'private',
54+
fields: { name: text('name') },
55+
...extra,
56+
});
57+
58+
const save = async (name: string, extra: Record<string, unknown> = {}) => {
59+
const result = await protocol.saveMetaItem({ type: 'object', name, item: objectBody(name, extra) });
60+
expect(result?.success).toBe(true);
61+
};
62+
63+
/** One attachment onto `parent`: admitted, or the refusal's envelope. */
64+
const attach = async (parent: string): Promise<{ admitted: true } | { code: unknown; status: unknown }> => {
65+
try {
66+
await engine.insert(
67+
'sys_attachment',
68+
{ parent_object: parent, parent_id: 'rec-1', file_id: 'file-1' },
69+
SYSTEM,
70+
);
71+
return { admitted: true };
72+
} catch (e: any) {
73+
return { code: e?.code, status: e?.status };
74+
}
75+
};
76+
const REFUSED = { code: 'FILES_DISABLED', status: 403 };
77+
78+
beforeAll(async () => {
79+
kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false } as any);
80+
await kernel.use(new ObjectQLPlugin());
81+
await kernel.use(singlePostureTenancy());
82+
await kernel.use(new AuditPlugin());
83+
await kernel.bootstrap();
84+
85+
engine = kernel.getService<ObjectQL>('objectql');
86+
const driver = new SqliteWasmDriver({ filename: ':memory:' });
87+
await driver.connect();
88+
engine.registerDriver(driver, true);
89+
engine.registry.registerObject(
90+
{
91+
name: 'sys_attachment',
92+
label: 'Attachment',
93+
fields: { parent_object: text('parent_object'), parent_id: text('parent_id'), file_id: text('file_id') },
94+
} as any,
95+
HARNESS_PACKAGE,
96+
);
97+
await engine.syncSchemas();
98+
protocol = kernel.getService('protocol') as typeof protocol;
99+
}, 120_000);
100+
101+
afterAll(async () => {
102+
if (kernel) {
103+
await Promise.race([kernel.shutdown(), new Promise<void>((resolve) => setTimeout(resolve, 10_000))]);
104+
}
105+
}, 30_000);
106+
107+
it('enable.files switched ON through the door admits the same attachment, with no restart', async () => {
108+
await save('qa_kid');
109+
// Refused while the object declares no files, and this refusal is the
110+
// lookup a held copy would have kept.
111+
expect(await attach('qa_kid')).toEqual(REFUSED);
112+
113+
await save('qa_kid', { enable: { files: true } });
114+
// The registry already serves the new value, so the door's read-back shows it.
115+
expect(engine.getSchema('qa_kid')?.enable?.files).toBe(true);
116+
expect(await attach('qa_kid')).toEqual({ admitted: true });
117+
});
118+
119+
it('switched OFF again through the door, the same attachment is refused again', async () => {
120+
await save('qa_kid_reverse', { enable: { files: true } });
121+
expect(await attach('qa_kid_reverse')).toEqual({ admitted: true });
122+
123+
await save('qa_kid_reverse', { enable: { files: false } });
124+
expect(await attach('qa_kid_reverse')).toEqual(REFUSED);
125+
});
126+
127+
it('control: an object authored with enable.files from the start, never looked up before, admits it', async () => {
128+
await save('qa_kid_from_start', { enable: { files: true } });
129+
expect(await attach('qa_kid_from_start')).toEqual({ admitted: true });
130+
});
131+
132+
// `password`, not `secret`: both are masked on the ledger copy, and a
133+
// `password` value is stored as written (ADR-0100) where a `secret` one needs
134+
// a crypto provider this harness does not wire.
135+
it('the ledger masks a password field added through the door after the object was first audited', async () => {
136+
const OBJECT = 'qa_vault';
137+
const TYPED = 'typed-24565';
138+
await save(OBJECT);
139+
// An audited write BEFORE the change: the ledger looks the definition up here.
140+
const row = (await engine.insert(OBJECT, { name: 'first' }, SYSTEM)) as { id: unknown };
141+
const id = String(row.id);
142+
143+
await save(OBJECT, { fields: { name: text('name'), login_pw: { name: 'login_pw', label: 'Password', type: 'password' } } });
144+
await engine.update(OBJECT, { login_pw: TYPED }, { where: { id }, ...SYSTEM } satisfies EngineUpdateOptions);
145+
146+
const updates = (await engine.find(
147+
'sys_audit_log',
148+
{ where: { object_name: OBJECT, record_id: id, action: 'update' }, ...SYSTEM } satisfies EngineQueryOptions,
149+
)) as Array<{ new_value: unknown }>;
150+
expect(updates).toHaveLength(1);
151+
const raw = String(updates[0].new_value);
152+
expect(raw).not.toContain(TYPED);
153+
const recorded = JSON.parse(raw) as Record<string, unknown>;
154+
// The change IS recorded, as "a value is set", never the value.
155+
expect(recorded.login_pw).toBe(SECRET_MASK);
156+
});
157+
});

‎packages/plugins/plugin-audit/src/audit-writers.ts‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,37 +1389,42 @@ export function installAuditWriters(
13891389
// with the auth-event writer so both stamp on the same answer.
13901390
const objectHasField = createFieldPresenceProbe(engine);
13911391

1392-
// Cached full field-definition map per object (for ADR-0052 §5b trackHistory
1393-
// rendering — needs labels/options, not just field names).
1394-
const fieldDefsCache = new Map<string, Record<string, any> | null>();
1395-
const getFieldDefs = (objectName: string): Record<string, any> | null => {
1396-
if (fieldDefsCache.has(objectName)) return fieldDefsCache.get(objectName) ?? null;
1397-
let defs: Record<string, any> | null = null;
1392+
/**
1393+
* [#24565] The object's REGISTERED definition, read on every call and never
1394+
* held here. Every gate and every row this installer writes asks it: the
1395+
* `enable.files` / `enable.feeds` capability gates, the ledger's credential
1396+
* mask and `internal` omission, the activity mirror's `enable.activities`
1397+
* switch, its milestones, titles, labels and computed-field drop.
1398+
*
1399+
* ⛔ No second cache over the registry. The registry is the one owner of an
1400+
* object's current definition and already memoizes the merged body behind
1401+
* its own invalidation points (`SchemaRegistry.resolveObject`), so a read
1402+
* here costs two map lookups. A copy here would have to be retired at every
1403+
* place the definition can change: the metadata door, a per-item or package
1404+
* publish, a dev reload, a peer replica's replay, a delete, a plugin's own
1405+
* `registerObject`. They do not all announce, and they announce on different
1406+
* channels (`metadata:reloaded` does not fire for the door's save). The copy
1407+
* this replaces kept each object's first answer for the life of the process,
1408+
* so an object whose `enable.files` was switched on through the door kept
1409+
* refusing attachments with 403 `FILES_DISABLED` until a restart, while the
1410+
* door's own read-back showed the new value. ⛔ Nor a TTL: an expiring copy
1411+
* is the same stale answer for a shorter time. The same rule this package
1412+
* already keeps for the stamped columns (`declaresActivityStampColumns`) and
1413+
* for a milestone declaration on the read side (`currentMilestoneWatched`).
1414+
*/
1415+
const getObjectDef = (objectName: string): any => {
13981416
try {
1399-
const schema: any =
1400-
typeof (engine as any).getSchema === 'function' ? (engine as any).getSchema(objectName) : null;
1401-
const fields = schema?.fields;
1402-
if (fields && typeof fields === 'object' && !Array.isArray(fields)) defs = fields;
1417+
return typeof (engine as any).getSchema === 'function' ? (engine as any).getSchema(objectName) : null;
14031418
} catch {
1404-
/* ignore — best-effort; absence just means we fall back to generic text */
1419+
return null; // best-effort — no definition means each reader's own fallback
14051420
}
1406-
fieldDefsCache.set(objectName, defs);
1407-
return defs;
14081421
};
14091422

1410-
// Cached full object definition (for ADR-0052 §5b.2 activityMilestones —
1411-
// object-level metadata, not just fields).
1412-
const objectDefCache = new Map<string, any>();
1413-
const getObjectDef = (objectName: string): any => {
1414-
if (objectDefCache.has(objectName)) return objectDefCache.get(objectName);
1415-
let def: any = null;
1416-
try {
1417-
def = typeof (engine as any).getSchema === 'function' ? (engine as any).getSchema(objectName) : null;
1418-
} catch {
1419-
/* ignore — best-effort */
1420-
}
1421-
objectDefCache.set(objectName, def);
1422-
return def;
1423+
// The full field-definition map (ADR-0052 §5b trackHistory rendering needs
1424+
// labels/options, not just field names), from the same per-call read.
1425+
const getFieldDefs = (objectName: string): Record<string, any> | null => {
1426+
const fields = getObjectDef(objectName)?.fields;
1427+
return fields && typeof fields === 'object' && !Array.isArray(fields) ? fields : null;
14231428
};
14241429

14251430
/**

0 commit comments

Comments
 (0)