Skip to content

Commit e744244

Browse files
fix(plugin-audit): the compliance ledger keeps no copy of secret-store key material (#26683)
Part of #26640 — the writer half; the at-rest rewrite's run entry point is #26687. Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:审计台账(`sys_audit_log` 与 `sys_activity`)写入时不再复制密钥存储与两张设置表中的加密密钥材料列;另附一个一次性改写模块,可清除已写入副本中的这些列(热表与归档库)。 - 为什么改:台账是第二份长期留存的副本,此前把应仅限平台访问的加密材料一并复制进去(详情按维护者披露裁决不公开)。 - 风险与回滚:只删副本中的列,不改源表、不改读侧规则;台账仍记录每次创建/更新/删除(哪条记录、标签、密钥 id、算法、版本、操作人与时间)。回滚即 revert 本 PR。 - 先看:`packages/plugins/plugin-audit/src/key-material-copy.ts`(规则与名单)、`audit-writers.ts` 中 `ledgerView` 新增的一行调用、`key-material-copy-migration.ts` 的改写器。 - 两分钟验证:`pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2 src/audit-internal-field-omission.test.ts src/key-material-copy-migration.test.ts`,预期 2 个文件全部通过。 - 需要你做的:无。已写入旧副本的执行入口(`packages/cli` 的 CLI 命令)由 #26687 承接。 ## What changes - **Writer.** `ledgerView` (the one view every ledger and activity copy is taken through) now omits a closed, writer-held list of key-material columns, by the same disposition as a field declared `internal`: omitted, not masked. The list (`KEY_MATERIAL_COPY_COLUMNS`, `key-material-copy.ts`) names the secret store and the two settings holders, keyed by the platform-object definitions' own names. Neither existing limb reached these columns: they are plain text columns, not credential fields, and not `internal`, because their producers read them back through the generic engine read. `titleFieldOf` refuses a listed column as a record title, as it refuses masked and `internal` columns. - **What the ledger still records.** Every create, update and delete of those rows keeps its ledger and activity row, with the record id, producer-scoped label, key id, algorithm, version, actor and time. A write that changes only the material still records its row, with the material on neither side. I chose this over excluding the store from the CRUD mirror: the store has three producers, and only the settings producer writes the settings audit trail, so an exclusion would leave the other two with no ledger record at all. - **The settings holders are in the same class.** Their encrypted-value column holds a handle when a crypto provider is wired and the material inline on the legacy adapter path, and a copy cannot tell which. The holder's own header already says the audit log holds only a placeholder and a digest of an encrypted value. So the copy carries neither (fail closed). - **Copies already written.** `key-material-copy-migration.ts` adds pure planners (`planKeyMaterialAuditRowPatch`, `planKeyMaterialActivityRowPatch`) and two runners: `migrateKeyMaterialCopies` (engine, hot tables) and `migrateArchivedKeyMaterialCopies` (the archive datasource's driver). They drive the mail-log body rewrite's runner and two stores, which `mail-log-body-migration.ts` now exports as `runLedgerCopyRewrite`, `ledgerCopyStoreOverEngine` and `ledgerCopyStoreOverArchiveDriver`. The mail-log rewrite keeps its behaviour and its log lines. So there is one ledger-copy rewrite, and the write call sites stay the two that already existed: the tenant-audit write-site census is unchanged at 244. Both runs are dry by default and idempotent. They change only the copy column, keep each row's stored `updated_at`, and report counts only. Both runners are exported from the package index. - **Test config.** `vitest.config.ts` gains one anchored alias for `@objectstack/platform-objects/system`, so the new census resolves the definitions from source (`check:test-source-alias`). - **Comment.** `sys-secret.object.ts`: the `trackHistory: false` comment now says what the ledger records for the store. Comment only. ## Tests - `audit-internal-field-omission.test.ts` gains a census block over the REAL definitions. It pins that the writer-held list equals the census and that every listed column is declared by its object, so a rename reds. On create, update and delete, the control column is recorded on every side and no listed column appears on any side or anywhere at rest (ledger or activity). A write of only the material still records its row. A same-named column on an unlisted object is still recorded. - `key-material-copy-migration.test.ts`: the planners keep every other key, leave unlisted objects and material-free rows alone, and are idempotent. The engine runner covers dry run, apply, re-run as a no-op, writes as the platform keeping `updated_at`, and an unreadable table counted as a failure with no value in any log line. The archive runner runs against a real SQLite driver, including a never-created archive table. ## Acceptance notes - **Remaining half: the run entry point.** The precedents ship their at-rest rewrite behind an `os migrate …` command in `packages/cli`, which is outside this card's file surface, so this PR ships the exported runners without a command. Copies written before this release keep the material until an entry point runs them: #26687. - **The mail-log rewrite was restructured, not changed.** Its runner and stores were lifted into exported functions so the key-material rewrite drives the same ones. Its existing test file (`mail-log-body-migration.test.ts`) passes unchanged. The `os migrate audit-mail-bodies` integration tests in `packages/cli` exercise it through the built package and are left to CI. - **Read side unchanged by design.** The ledger's read rules are not touched. ## Verification <details><summary>Writer-layer pin, and its ablation</summary> On the fix (HEAD `21dca99112`): `vitest run src/audit-internal-field-omission.test.ts src/key-material-copy-migration.test.ts` gives `Test Files 2 passed (2)`, `Tests 54 passed (54)`. Ablation (`scripts/ablation-replace.mjs`, the writer's one new call replaced by a no-op, anchor 1 to 0, blob `ad8f76be13d4` to `6ee8dfd95a08`): `Tests 6 failed | 37 passed (43)`. The 6 red are exactly the new census cases (3 objects × 2). Restored: blob equals HEAD (`ad8f76be13d4`), `git diff HEAD` empty. The test imports the writer from source, so no build stood between the mutation and the run. </details> <details><summary>Composition check (one-off, not committed)</summary> A temporary test exercised the real settings producer, its crypto provider and a real `ObjectQL` engine over the real definitions, with the writer installed, through two writes of an encrypted value. On the fix, none of the encrypted values the producer minted was at rest in the ledger, and every create and delete row of the store was still written. With the writer call ablated, every minted value was at rest. No plaintext was at rest in either run. The scratch file was deleted, and nothing of it is committed. </details> <details><summary>Package suites and gates</summary> All readings on head `c38dcc7257` (the final commit), exit codes captured before any pipe. - `pnpm --filter @objectstack/plugin-audit test`: `Test Files 65 passed (65)`, `Tests 1047 passed (1047)`, lock `VERDICT command-exit 0`. - `pnpm --filter @objectstack/plugin-audit typecheck`: exit 0. The test layer is included (`check:test-typecheck: OK`, 0 errors). - `pnpm --filter @objectstack/platform-objects typecheck` and `test` (comment-only edit, read at `cd0e7a5d7d`): exit 0, `Test Files 96 passed (96)`, `Tests 1361 passed (1361)`. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`: 64 commands, every one run. `--ran` gives `64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN`, a DERIVED zero because all 64 carry an exit code. - An earlier pass, before the last commit, was red on `check-tenant-audit-census` (my first runner added two write call sites) and on `check:test-source-alias` (the `/system` alias was missing). Both are fixed by the last commit, and both are green above. - The derivation flagged the tree as 2 commits behind `origin/main`, because 2 gate scripts changed there. Neither script is in this card's 64 commands, and the two commits touch neither package, so I did not merge. ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-adoption.mjs :: exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-dts-emitted.mjs --self-test :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-issue-citations.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0 node scripts/check-tenant-audit-census.mjs :: exit 0 node scripts/check-tenant-audit-census.mjs --self-test :: exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0 node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0 node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0 node scripts/docs-audit/check-affected-docs.mjs :: exit 0 node scripts/docs-audit/check-drift-comment.mjs :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:ci-filter-parity :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:doc-authoring :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:dts-closure :: exit 0 pnpm check:dual-build-cjs-loads :: exit 0 pnpm check:durability-log-level :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:error-status-conformance :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:i18n :: exit 0 pnpm check:i18n-stale-fill :: exit 0 pnpm check:issue-citations :: exit 0 pnpm check:keyed-text-bounds :: exit 0 pnpm check:lean-entry-closure :: exit 0 pnpm check:logger-receiver-detach :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:objectql-double-limit :: exit 0 pnpm check:org-identifier :: exit 0 pnpm check:page-declaration-shape :: exit 0 pnpm check:platform-object-tenancy-census :: exit 0 pnpm check:pm-changeset-deadline-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:registry-log-declared :: exit 0 pnpm check:rest-log-spy-declared :: exit 0 pnpm check:slot-lookup :: exit 0 pnpm check:sourcemap-no-sources-content :: exit 0 pnpm check:system-context-census :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 pnpm --filter @objectstack/plugin-audit test :: exit 0 pnpm --filter @objectstack/plugin-audit typecheck :: exit 0 ``` </details> --- _Generated by [Claude Code](https://claude.ai/code/session_012YjcDon6BvR9MN93ZsFq3U)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bb91ae8 commit e744244

10 files changed

Lines changed: 816 additions & 61 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
fix(plugin-audit): the compliance ledger keeps no copy of secret-store key material
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The audit writer copies each audited row into `sys_audit_log` (`new_value` / `old_value`) and into `sys_activity.metadata`. For the secret store and the two settings holders, that copy carried the encrypted key-material column. Every new copy now omits those columns, the way a field declared `internal` is omitted:
11+
12+
- **What is still recorded.** Each create, update and delete of those rows keeps its ledger and activity row: which record, its producer-scoped label, key id, algorithm and version, who changed it and when. A write that changes only the key material still records its row, with the material on neither side.
13+
- **Which columns.** The list is closed, held by the writer (`KEY_MATERIAL_COPY_COLUMNS`), and keyed by the platform-object definitions' own names. A column of the same name on any other object is recorded as before.
14+
- **Copies already written.** `migrateKeyMaterialCopies(engine, logger, { apply })` and `migrateArchivedKeyMaterialCopies(driver, logger, { apply })` remove the columns from copies stored before this release, in the hot `sys_audit_log` / `sys_activity` tables and in an archive datasource. Both runs are dry by default, idempotent, change only the copy column, and keep each row's stored `updated_at`. Their reports hold counts only. The pure planners `planKeyMaterialAuditRowPatch` and `planKeyMaterialActivityRowPatch` are exported with them.

‎packages/platform-objects/src/system/sys-secret.object.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,12 @@ export const SysSecret = ObjectSchema.create({
168168
],
169169

170170
enable: {
171-
trackHistory: false, // rotation events are recorded by sys_setting_audit
171+
// Settings rotation events are recorded by sys_setting_audit. `trackHistory`
172+
// governs field-change summaries only: the compliance ledger's CRUD mirror
173+
// still records each create / update / delete of a row here — which row,
174+
// its producer-scoped label, key id, algorithm and version — and keeps no
175+
// copy of the key material (plugin-audit `key-material-copy.ts`).
176+
trackHistory: false,
172177
// [ADR-0103] Engine-owned: secrets are minted/rotated only by the settings /
173178
// secret service (SYSTEM_CTX), never via the generic data API. Locked to
174179
// reads (ciphertext only; decryption is a separate privileged path). Since

‎packages/plugins/plugin-audit/src/audit-internal-field-omission.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ import {
4040
SysVerification,
4141
} from '@objectstack/platform-objects/identity';
4242
import { SysEmail, SysIdentityEmail } from '@objectstack/platform-objects/audit';
43+
import { SysPlatformSetting, SysSecret, SysSetting } from '@objectstack/platform-objects/system';
4344
import { SECRET_MASK } from '@objectstack/objectql/core';
4445
import { installAuditWriters } from './audit-writers.js';
46+
import { KEY_MATERIAL_COPY_COLUMNS } from './key-material-copy.js';
4547
import { singlePostureDefaultOrganization } from './__tests__/single-posture-tenancy.js';
4648

4749
type Row = Record<string, unknown>;
@@ -353,3 +355,97 @@ describe('[#24625] the ledger CRUD mirror masks a mail log\'s body columns', ()
353355
expect(created.body_text).toBe('control-body');
354356
});
355357
});
358+
359+
/**
360+
* The key-material columns `key-material-copy.ts` lists are omitted from every
361+
* ledger copy, as an `internal` field is. Same in-memory boot as above, over
362+
* the REAL definitions; a control column proves the mirror still records the
363+
* row, and a write of only the material still records that a change happened.
364+
*/
365+
describe('the ledger CRUD mirror omits encrypted key-material columns', () => {
366+
const MATERIAL_CENSUS: Array<{ def: ObjectDef; material: string[]; control: string }> = [
367+
{ def: SysSecret as unknown as ObjectDef, material: ['ciphertext'], control: 'kms_key_id' },
368+
{ def: SysSetting as unknown as ObjectDef, material: ['value_enc'], control: 'locked_reason' },
369+
{ def: SysPlatformSetting as unknown as ObjectDef, material: ['value_enc'], control: 'locked_reason' },
370+
];
371+
372+
it('the writer-owned list is exactly the census, and each listed column is declared by its object', () => {
373+
expect([...KEY_MATERIAL_COPY_COLUMNS.entries()].map(([o, c]) => [o, [...c]])).toEqual(
374+
MATERIAL_CENSUS.map(({ def, material }) => [def.name, material]),
375+
);
376+
for (const { def, material } of MATERIAL_CENSUS) {
377+
for (const column of material) expect(def.fields, `${def.name}.${column} is declared`).toHaveProperty(column);
378+
}
379+
});
380+
381+
describe.each(MATERIAL_CENSUS.map((c) => [c.def.name, c] as const))('%s', (_name, { def, material, control }) => {
382+
it('create, update and delete record the control column and none of the material', async () => {
383+
const { engine, storeFor } = await boot(def);
384+
const created = await engine.insert(def.name, seedRow(def, material, control, 'create'), SYS);
385+
const id = String(created.id);
386+
const changed = seedRow(def, material, control, 'update');
387+
const patch: Row = { [control]: changed[control] };
388+
for (const column of material) patch[column] = changed[column];
389+
await engine.update(def.name, patch, { where: { id }, ...SYS } as any);
390+
await engine.delete(def.name, { where: { id }, ...SYS } as any);
391+
392+
const rows = ledgerRows(storeFor, def.name);
393+
const one = (a: string) => {
394+
const hit = rows.filter((r) => r.action === a);
395+
expect(hit, `one ${a} row`).toHaveLength(1);
396+
return hit[0];
397+
};
398+
const sides = {
399+
createNew: parse(one('create').new_value),
400+
updateOld: parse(one('update').old_value),
401+
updateNew: parse(one('update').new_value),
402+
deleteOld: parse(one('delete').old_value),
403+
};
404+
expect(sides.createNew[control]).toBe('control-create');
405+
expect(sides.updateOld[control]).toBe('control-create');
406+
expect(sides.updateNew[control]).toBe('control-update');
407+
expect(sides.deleteOld[control]).toBe('control-update');
408+
for (const column of material) {
409+
for (const [side, snapshot] of Object.entries(sides)) {
410+
expect(snapshot, `${def.name}.${column} on ${side}`).not.toHaveProperty(column);
411+
}
412+
}
413+
414+
const atRest = JSON.stringify([...rows, ...activityRows(storeFor, def.name)]);
415+
for (const column of material) {
416+
for (const phase of ['create', 'update']) {
417+
expect(atRest, `${def.name}.${column} (${phase}) at rest`).not.toContain(marker(def.name, column, phase));
418+
}
419+
}
420+
expect(activityRows(storeFor, def.name).length, 'the activity mirror ran').toBeGreaterThan(0);
421+
});
422+
423+
it('a write of ONLY the material still records its row, with neither value', async () => {
424+
const { engine, storeFor } = await boot(def);
425+
const created = await engine.insert(def.name, seedRow(def, material, control, 'create'), SYS);
426+
const column = material[0];
427+
await engine.update(def.name, { [column]: marker(def.name, column, 'rotate') }, { where: { id: String(created.id) }, ...SYS } as any);
428+
429+
const updates = ledgerRows(storeFor, def.name).filter((r) => r.action === 'update');
430+
expect(updates, 'the change is recorded').toHaveLength(1);
431+
expect(parse(updates[0].old_value)).not.toHaveProperty(column);
432+
expect(parse(updates[0].new_value)).not.toHaveProperty(column);
433+
const atRest = JSON.stringify([...ledgerRows(storeFor, def.name), ...activityRows(storeFor, def.name)]);
434+
expect(atRest).not.toContain(marker(def.name, column, 'create'));
435+
expect(atRest).not.toContain(marker(def.name, column, 'rotate'));
436+
});
437+
});
438+
439+
it('a column of the same name on an object the list does not name is still recorded (control)', async () => {
440+
const { engine, storeFor } = await boot(SysSecret as unknown as ObjectDef);
441+
const other = {
442+
name: 'key_material_control', label: 'Control',
443+
fields: { id: f('id', 'text', { primaryKey: true }), ciphertext: f('ciphertext', 'text'), value_enc: f('value_enc', 'text') },
444+
};
445+
engine.registry.registerObject(other as any, 'com.objectstack.test.key-material-control');
446+
await engine.insert('key_material_control', { ciphertext: 'control-a', value_enc: 'control-b' }, SYS);
447+
const created = parse(ledgerRows(storeFor, 'key_material_control').filter((r) => r.action === 'create')[0].new_value);
448+
expect(created.ciphertext).toBe('control-a');
449+
expect(created.value_enc).toBe('control-b');
450+
});
451+
});

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import type { IDataEngine } from '@objectstack/spec/contracts';
2020
import { SECRET_MASK, collectInternalReadFields, collectMaskedReadFields } from '@objectstack/objectql/core';
2121
// [#24625] The mask of a mail-log row's body columns in a copy of it.
2222
import { maskMailLogBodyCopy } from './mail-log-body-mask.js';
23+
// The key-material columns no copy of a row carries (`key-material-copy.ts`).
24+
import { keyMaterialColumnsOf, omitKeyMaterialFromCopy } from './key-material-copy.js';
2325
// [#7230] ADR-0079's record display-name contract, imported rather than
2426
// re-derived. `resolveDisplayField` IS the definition of "which field is this
2527
// object's human title" (`nameField` → deprecated `displayNameField` alias →
@@ -1558,6 +1560,8 @@ export function installAuditWriters(
15581560
if (!titleField || titleField === 'id') return undefined;
15591561
if (collectMaskedReadFields(def).includes(titleField)) return undefined;
15601562
if (collectInternalReadFields(def).includes(titleField)) return undefined;
1563+
// …and a key-material column, which `ledgerView` omits by the same list.
1564+
if (keyMaterialColumnsOf(objectName).includes(titleField)) return undefined;
15611565
return titleField;
15621566
};
15631567

@@ -1854,6 +1858,12 @@ export function installAuditWriters(
18541858
// keeps the body behind its field gate; the copy never holds it in clear.
18551859
// [#24693] One helper, the one the at-rest rewrite of older copies applies.
18561860
maskMailLogBodyCopy(objectName, out);
1861+
// A column that stores encrypted key material as plain text
1862+
// (`key-material-copy.ts` lists them and says why neither limb above
1863+
// reaches one) is OMITTED on every side, as an `internal` field is: the
1864+
// copy records which row changed and how, never the material. One helper,
1865+
// the one the at-rest rewrite of older copies applies.
1866+
omitKeyMaterialFromCopy(objectName, out);
18571867
// [#21120] A stored metadata BODY (`sys_metadata` / `sys_metadata_history`)
18581868
// is not a top-level secret field, so the mask above never touches it. The
18591869
// audit copy is a credential read exit exactly like `/meta`, so the body is

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,29 @@ export type {
8888
MailLogBodyMigrationOptions,
8989
MailLogBodyMigrationReport,
9090
} from './mail-log-body-migration.js';
91+
// The key-material columns no ledger copy carries, the one helper the writer
92+
// and the at-rest rewrite of older copies both apply, and that rewrite's pure
93+
// planners and runners (hot tables and the archive datasource).
94+
export {
95+
KEY_MATERIAL_COPY_COLUMNS,
96+
KEY_MATERIAL_COPY_OBJECTS,
97+
keyMaterialColumnsOf,
98+
omitKeyMaterialFromCopy,
99+
} from './key-material-copy.js';
100+
export {
101+
migrateKeyMaterialCopies,
102+
migrateArchivedKeyMaterialCopies,
103+
planKeyMaterialAuditRowPatch,
104+
planKeyMaterialActivityRowPatch,
105+
KEY_MATERIAL_COPY_TABLES,
106+
} from './key-material-copy-migration.js';
107+
export type {
108+
KeyMaterialArchiveDriver,
109+
KeyMaterialCopyTable,
110+
KeyMaterialMigrationEngine,
111+
KeyMaterialMigrationOptions,
112+
KeyMaterialMigrationReport,
113+
} from './key-material-copy-migration.js';
91114
// [ADR-0139 E6] The stored-row backfill of `sys_activity`'s two writer-stamped
92115
// read-rule columns, and the one planner it, the CRUD mirror and the
93116
// before-insert hook stamp by. The CLI command

0 commit comments

Comments
 (0)