Skip to content

Commit d7f6302

Browse files
committed
fix(service-settings): an unmounted audit ledger is a configuration, not a fault
`buildConfigChangeAuditSink` attempted the `sys_audit_log` `config_change` insert unconditionally and reported the throw it got back as a degradation. A host that never mounted the OPTIONAL `@objectstack/plugin-audit` has no ledger to write to, so that report described a deployment behaving exactly as composed — and the insert it never should have attempted reached `ObjectQL.insert`, whose own terminal catch logs `Insert operation failed` once per settings write. The sink now probes the engine registry for `sys_audit_log` before the write. Absent, it skips at `debug` and attempts nothing. The probe records nothing and is re-taken per call, so a ledger registered later in the same boot starts recording; an engine that cannot be asked (no `getSchema`) still gets the write attempted, which is the pre-change behaviour. What is left in the catch is a mounted ledger whose insert genuinely failed — a write that claims to be audited and is not — so it reports on the `error` channel, with `warn` kept as the receiver-safe fallback for a sink with no `error`. Both arms are pinned, plus the probe's non-memoization, the unanswerable-engine path and the `warn` fallback. Claude-Session: https://claude.ai/code/session_01QGMBhvUoyD8t5zY8xHQhnP Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1bc22b3 commit d7f6302

3 files changed

Lines changed: 372 additions & 16 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@objectstack/service-settings': patch
3+
---
4+
5+
Stop reporting an unmounted `sys_audit_log` as a failed audit write.
6+
7+
`buildConfigChangeAuditSink` wrote the `config_change` compliance row blind and reported
8+
the throw it got back. On a deployment that never mounted the OPTIONAL
9+
`@objectstack/plugin-audit` — `objectstack serve --preset minimal`, an EE host that mounts
10+
no `audit`, a hosted tenant kernel — there is no ledger to write to, so every tenant
11+
settings write produced a durability complaint about a deployment behaving exactly as
12+
composed, plus an `Insert operation failed` line per write from the engine one frame down.
13+
14+
The sink now probes the engine registry for `sys_audit_log` before the write and skips at
15+
`debug` when it is absent, so no insert is attempted and neither channel says anything. The
16+
probe records nothing and is re-taken per write, so a ledger mounted later in the same boot
17+
starts recording. An engine that cannot answer the probe still gets the write attempted.
18+
19+
No API change: the exported signature, the row shape, `CONFIG_CHANGE_ACTION` and
20+
`CONFIG_CHANGE_OBJECT_NAME` are all unchanged. The remaining fault arm — a ledger that IS
21+
mounted whose insert genuinely fails — now reports on the `error` channel rather than
22+
`warn`, which is AGENTS.md's durability-degradation level for a write that claims to be
23+
audited and is not.
24+
25+
Clause-②: no

‎packages/services/service-settings/src/config-change-audit.test.ts‎

Lines changed: 209 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,33 @@ const manifest: SettingsManifest = {
8080
],
8181
};
8282

83+
/**
84+
* [#18368] The `sys_audit_log` MOUNT POINT — a stand-in declaration, not the
85+
* object.
86+
*
87+
* `sys_audit_log` belongs to `@objectstack/plugin-audit` and must not be
88+
* imported here (see the file header). That was free while the sink wrote
89+
* blind; it is not free now that the sink ASKS the registry whether the ledger
90+
* is mounted before writing, because a fixture registering nothing is a fixture
91+
* of the UNMOUNTED deployment — every case below would go green on a sink that
92+
* never writes at all.
93+
*
94+
* So the registry gets a declaration under that name and nothing more. The ROW
95+
* is still asserted at the engine seam, where the real object's absence does not
96+
* matter; what this buys is that `getSchema('sys_audit_log')` answers, which is
97+
* the only thing the mount probe reads. It deliberately declares NO
98+
* `organization_id`, so `makeFieldProbe`'s answer — and therefore every row
99+
* asserted in this file — is byte-for-byte what it was before this existed.
100+
*/
101+
const LEDGER_MOUNT_STANDIN = {
102+
name: 'sys_audit_log',
103+
label: 'Audit Log (mount stand-in)',
104+
fields: {
105+
id: { type: 'text', label: 'Id' },
106+
action: { type: 'text', label: 'Action' },
107+
},
108+
};
109+
83110
type Store = Map<string, Map<string, Record<string, unknown>>>;
84111

85112
/** A driver over plain Maps — enough of `IDataDriver` for the settings write path. */
@@ -174,6 +201,12 @@ class MockHttp implements IHttpServer {
174201
interface BootOptions {
175202
/** Make every `sys_audit_log` insert fail, to exercise the best-effort path. */
176203
ledgerThrows?: boolean;
204+
/**
205+
* [#18368] Whether this deployment MOUNTS the platform audit ledger.
206+
* Defaults to `true` — the shape every case written before #18368 assumed.
207+
* `false` is the `--preset minimal` / no-`plugin-audit` host.
208+
*/
209+
ledgerMounted?: boolean;
177210
/** Identity the write runs under. */
178211
userId?: string;
179212
tenantId?: string;
@@ -195,6 +228,10 @@ async function bootPlugin(opts: BootOptions = {}) {
195228
await engine.init();
196229
engine.registry.registerObject(SysSetting as any, OWNER_PACKAGE);
197230
engine.registry.registerObject(SysSettingAudit as any, OWNER_PACKAGE);
231+
// [#18368] The mount point — see `LEDGER_MOUNT_STANDIN`.
232+
if (opts.ledgerMounted !== false) {
233+
engine.registry.registerObject(LEDGER_MOUNT_STANDIN as any, OWNER_PACKAGE);
234+
}
198235

199236
// `sys_audit_log` is plugin-audit's object and is deliberately not resolvable
200237
// from this package (see the file header), so its insert is answered at the
@@ -211,11 +248,20 @@ async function bootPlugin(opts: BootOptions = {}) {
211248
};
212249

213250
const logged: string[] = [];
251+
// [#18368] The same lines, carrying the CHANNEL they arrived on. `logged`
252+
// stays level-blind so every pre-#18368 assertion keeps its meaning; the
253+
// level is what the two arms of #18368 are about, and a level-blind capture
254+
// cannot tell a suppressed expectation from a suppressed fault.
255+
const loggedAt: Array<{ level: string; msg: string }> = [];
256+
const at = (level: string) => (m: string) => {
257+
loggedAt.push({ level, msg: m });
258+
if (level !== 'debug' && level !== 'info') logged.push(m);
259+
};
214260
const logger = {
215-
info: () => {},
216-
warn: (m: string) => { logged.push(m); },
217-
error: (m: string) => { logged.push(m); },
218-
debug: () => {},
261+
info: at('info'),
262+
warn: at('warn'),
263+
error: at('error'),
264+
debug: at('debug'),
219265
};
220266

221267
const http = new MockHttp();
@@ -259,6 +305,10 @@ async function bootPlugin(opts: BootOptions = {}) {
259305
/** REAL `sys_setting` rows. */
260306
settingRows: () => [...rowsOf('sys_setting').values()],
261307
logged,
308+
/** [#18368] Every captured line with the channel it arrived on. */
309+
loggedAt,
310+
/** [#18368] So a case can mount the ledger mid-process and write again. */
311+
engine,
262312
http,
263313
};
264314
}
@@ -528,6 +578,9 @@ describe('#8145 — a refused write emits NO config_change row', () => {
528578
await engine.init();
529579
engine.registry.registerObject(SysSetting as any, OWNER_PACKAGE);
530580
engine.registry.registerObject(SysSettingAudit as any, OWNER_PACKAGE);
581+
// [#18368] This case asserts the sink writes on its NON-VACUITY leg, so the
582+
// ledger has to be mounted — see `LEDGER_MOUNT_STANDIN`.
583+
engine.registry.registerObject(LEDGER_MOUNT_STANDIN as any, OWNER_PACKAGE);
531584

532585
const ledgerRows: Array<Record<string, unknown>> = [];
533586
const realInsert = engine.insert.bind(engine);
@@ -580,7 +633,10 @@ describe('#8145 — a refused write emits NO config_change row', () => {
580633

581634
describe('#8145 — the config_change write is best-effort', () => {
582635
it('a failing sys_audit_log insert leaves the settings write landed and reported', async () => {
583-
// The shape of a deployment WITHOUT plugin-audit: the table does not exist.
636+
// [#18368] The ledger IS mounted (the `bootPlugin` default) and its insert
637+
// fails anyway — a provisioned-but-unreachable table. ⛔ This is no longer
638+
// "the shape of a deployment WITHOUT plugin-audit": that shape is skipped
639+
// before the insert now and is pinned in its own section below.
584640
const boot = await bootPlugin({ ledgerThrows: true, userId: 'usr_admin' });
585641

586642
const out = await boot.service.setMany(
@@ -596,7 +652,7 @@ describe('#8145 — the config_change write is best-effort', () => {
596652
// …and the operator is told, once, with the consequence and the cause.
597653
const reported = boot.logged.filter((l) => l.includes('config_change audit row NOT written'));
598654
expect(reported).toHaveLength(1);
599-
expect(reported[0]).toContain('plugin-audit');
655+
expect(reported[0]).toContain('schema sync');
600656
});
601657

602658
it('reports ONCE per process, not once per write', async () => {
@@ -609,3 +665,150 @@ describe('#8145 — the config_change write is best-effort', () => {
609665
expect(boot.settingAuditRows()).toHaveLength(3);
610666
});
611667
});
668+
669+
670+
// ---------------------------------------------------------------------------
671+
// 6. [#18368] An UNMOUNTED ledger is a CONFIGURATION, not a fault
672+
// ---------------------------------------------------------------------------
673+
674+
/**
675+
* Both arms, and NEITHER is optional.
676+
*
677+
* The quiet arm alone would pass just as well on a sink whose log line had
678+
* simply been deleted — or on one that stopped writing the ledger entirely —
679+
* so every case here carries its opposite: the control arm proves a MOUNTED
680+
* ledger whose insert genuinely fails is still reported, and on the durability
681+
* channel. What #18368 suppresses is an EXPECTATION; suppressing a fault would
682+
* be the same defect one layer down.
683+
*/
684+
describe('#18368 — an unmounted `sys_audit_log` is a configuration, not a fault', () => {
685+
const NOT_WRITTEN = 'config_change audit row NOT written';
686+
687+
it('QUIET ARM: no ledger mounted — the write lands, no insert is attempted, no ERROR', async () => {
688+
const boot = await bootPlugin({ ledgerMounted: false, userId: 'usr_admin', tenantId: 'org_1' });
689+
690+
const out = await boot.service.setMany(
691+
'branding_test',
692+
{ workspace_name: 'ObjectStack' },
693+
boot.writeCtx,
694+
);
695+
696+
// The settings write is untouched — the same two facts the cloud seat
697+
// measured on the hosted plane before filing.
698+
expect(out.workspace_name.value).toBe('ObjectStack');
699+
expect(boot.settingRows()).toHaveLength(1);
700+
expect(boot.settingAuditRows()).toHaveLength(1);
701+
702+
// ⭐ The insert is never ATTEMPTED. Asserting only on the log would leave
703+
// `ObjectQL.insert`'s own per-write `Insert operation failed` line — the
704+
// half that actually fires once per settings write — completely unpinned,
705+
// because that one is logged a frame below this fixture.
706+
expect(boot.ledgerRows()).toHaveLength(0);
707+
708+
// Nothing on `error`, nothing on `warn`, from this sink or any other.
709+
expect(boot.loggedAt.filter((l) => l.level === 'error')).toEqual([]);
710+
expect(boot.logged.filter((l) => l.includes(NOT_WRITTEN))).toEqual([]);
711+
712+
// …and it is not silent-by-accident: the configuration reading went to
713+
// `debug`, naming the remedy once.
714+
const debugLines = boot.loggedAt.filter((l) => l.level === 'debug' && l.msg.includes('sys_audit_log'));
715+
expect(debugLines).toHaveLength(1);
716+
expect(debugLines[0].msg).toContain('SKIPPED, not failed');
717+
expect(debugLines[0].msg).toContain('@objectstack/plugin-audit');
718+
});
719+
720+
it('CONTROL ARM: ledger MOUNTED and the insert genuinely fails — the ERROR still fires', async () => {
721+
// Same fixture, same failing insert, ONE difference: the ledger is mounted.
722+
const boot = await bootPlugin({ ledgerMounted: true, ledgerThrows: true, userId: 'usr_admin' });
723+
724+
await boot.service.setMany('branding_test', { workspace_name: 'ObjectStack' }, boot.writeCtx);
725+
726+
// The insert WAS attempted here — which is what makes this a fault and not
727+
// an expectation.
728+
expect(boot.ledgerRows()).toHaveLength(1);
729+
730+
const errors = boot.loggedAt.filter((l) => l.level === 'error' && l.msg.includes(NOT_WRITTEN));
731+
expect(errors).toHaveLength(1);
732+
// AGENTS.md → Degradation log levels: an `error` here owes the CONSEQUENCE
733+
// and the FIX, both in the line it prints.
734+
expect(errors[0].msg).toContain('SUCCEEDED and is on disk');
735+
expect(errors[0].msg).toContain('schema sync');
736+
// …and it did NOT land on the configuration channel.
737+
expect(boot.loggedAt.filter((l) => l.level === 'debug' && l.msg.includes(NOT_WRITTEN))).toEqual([]);
738+
// The settings write still landed, on both the row and the settings trail.
739+
expect(boot.settingRows()).toHaveLength(1);
740+
expect(boot.settingAuditRows()).toHaveLength(1);
741+
});
742+
743+
it('the probe records NOTHING: a ledger mounted mid-process starts recording', async () => {
744+
// ⛔ The memoized spelling would pass every case above and fail only this
745+
// one — a verdict the same boot can still contradict (AGENTS.md → Startup
746+
// registry reads). `plugin-audit` registers its objects from its own
747+
// lifecycle, which may run after the settings service binds its engine.
748+
const boot = await bootPlugin({ ledgerMounted: false, userId: 'usr_admin' });
749+
750+
await boot.service.setMany('branding_test', { workspace_name: 'before' }, boot.writeCtx);
751+
expect(boot.ledgerRows()).toHaveLength(0);
752+
753+
boot.engine.registry.registerObject(LEDGER_MOUNT_STANDIN as any, OWNER_PACKAGE);
754+
755+
await boot.service.setMany('branding_test', { workspace_name: 'after' }, boot.writeCtx);
756+
const rows = boot.ledgerRows();
757+
expect(rows).toHaveLength(1);
758+
expect(rows[0].action).toBe(CONFIG_CHANGE_ACTION);
759+
expect(metaOf(rows[0]).key).toBe('workspace_name');
760+
});
761+
762+
it('an engine that cannot be ASKED still attempts the write, and still reports its failure', async () => {
763+
// `getSchema` is an ObjectQL member, not an `IDataEngine` one. ⛔ An
764+
// unanswerable probe is not an answer of "absent": read that way, this
765+
// sink would go permanently silent on every lean host engine, which is the
766+
// #18368 defect inverted.
767+
const attempted: Array<Record<string, unknown>> = [];
768+
const engineWithoutGetSchema: any = {
769+
insert: async (_object: string, row: Record<string, unknown>) => {
770+
attempted.push(row);
771+
throw new Error('ledger unreachable');
772+
},
773+
};
774+
const lines: Array<{ level: string; msg: string }> = [];
775+
const sink = buildConfigChangeAuditSink(engineWithoutGetSchema, {
776+
debug: (m: string) => lines.push({ level: 'debug', msg: m }),
777+
warn: (m: string) => lines.push({ level: 'warn', msg: m }),
778+
error: (m: string) => lines.push({ level: 'error', msg: m }),
779+
} as any);
780+
781+
await sink.record({
782+
namespace: 'branding_test',
783+
key: 'workspace_name',
784+
scope: 'global',
785+
action: 'set',
786+
valueDigest: 'sha256:abc',
787+
encrypted: false,
788+
});
789+
790+
expect(attempted).toHaveLength(1);
791+
expect(attempted[0].action).toBe(CONFIG_CHANGE_ACTION);
792+
expect(lines.filter((l) => l.level === 'error' && l.msg.includes(NOT_WRITTEN))).toHaveLength(1);
793+
expect(lines.filter((l) => l.level === 'debug')).toEqual([]);
794+
});
795+
796+
it('falls back to `warn` for a sink that declares no `error` channel', async () => {
797+
// The receiver-safe fallback, pinned so the level flip cannot silently
798+
// become a DROP on a host logger that only carries `warn`.
799+
const lines: string[] = [];
800+
const sink = buildConfigChangeAuditSink(
801+
{ insert: async () => { throw new Error('ledger unreachable'); } } as any,
802+
{ warn: (m: string) => lines.push(m) } as any,
803+
);
804+
await sink.record({
805+
namespace: 'branding_test',
806+
key: 'workspace_name',
807+
scope: 'global',
808+
action: 'set',
809+
valueDigest: 'sha256:abc',
810+
encrypted: false,
811+
});
812+
expect(lines.filter((l) => l.includes(NOT_WRITTEN))).toHaveLength(1);
813+
});
814+
});

0 commit comments

Comments
 (0)