Uh oh!
There was an error while loading. Please reload this page.
fix(service-settings): settings writes reach sys_audit_log as config_change (#8145) - #8288
Conversation
…change (#8145) `SettingsServicePlugin` passed `undefined` for `bindEngine`'s `SettingsAuditSink` argument — the slot documented since Phase 3 as the one that writes the generic `sys_audit_log`. So every settings write was audited into `sys_setting_audit` and nowhere else, and the declared `config_change` enum member, the shipped `config_changes` list view and the console filter over it were permanently empty (#7675 step 2: filter → total 0). Dual-write, per the 2026-08-12 maintainer ruling on #7675 which left the choice to the implementation: the generic ledger gets a `config_change` row per changed key (attributed, tenant-stamped, digest only — never a value) and `sys_setting_audit` keeps its rows unchanged. The new write is best-effort because plugin-audit is optional, and a REFUSED write emits no row on either ledger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
Measured on `origin/main`, where the case is red: vitest's 5s default cut the poll short, so the failure read as a bare timeout instead of `waitForRows`'s own message naming the object and the filter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARidKDYSCD56LaygrvDPnk
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8145
PUT /api/settings/branding, then$filter={"action":"config_change"}→ total 0. That is #7675 step 2, and it held for the whole life of that enum member: the declaredconfig_changeaction, the shippedconfig_changeslist view and the console filter that offers the value were three surfaces advertising a class of audit event the platform never wrote. Settings changes were audited — intosys_setting_audit,action: 'set'— and nowhere else, whilesettings-service.types.tspromisedsys_audit_logrows "for every successful write".The cause was one argument
SettingsAuditSinkis the slot documented since Phase 3 as the one that writes the generic ledger. It isSettingsService.bindEngine's second parameter, andSettingsServicePlugin.start()passedundefinedfor it. Nothing else was missing — the service called the sink on every write, the enum declared the value, the view filtered on it.The decision the ruling left open: dual-write
The 2026-08-12 maintainer ruling on #7675 permits 双写 or 改道 and leaves the choice to the implementation (以实现定契约).
sys_setting_auditkeeps its rows, measured rather than assumed:docs/qa/platform-checklist/areas/platform-core.jsonasserts asys_setting_auditrow per settings write as shipped platform behaviour, andmanifest.test.tspins the object's registration.objectuigap the dispatch flagged is CLOSED. Searched the sibling checkout (HEAD54d34d2, 1632.tsxfiles) forsys_setting_audit/setting_audit/settingAuditacross every tracked file: no hits. There is no settings-audit view in the console. Combined withpackages/,apps/,examples/, the only readers are the two above.sys_setting_auditholdsnamespace,key,scope,old_hash/new_hash,source,encrypted,reason— none of which has a column onsys_audit_log; the generic ledger answers "who changed platform configuration, when" across subsystems, which a per-namespace table cannot.packages/platform-objectssurface plus a stored-row question this card is not the place to decide.Duplicate-row cost is bounded and deliberately accepted: one extra row per changed key per settings write. These are admin-rate operations, nothing like the per-tick/per-chunk writers ADR-0057 D5 excluded from auditing, and
sys_audit_logcarries its own retention so growth is policy-capped.What lands on the ledger
One row per changed key:
action: 'config_change',object_name: 'sys_setting',record_id: null(a settings row is keyed on the composite(namespace, key, scope, user_id)— the same honest shapeplugin-auth's run-levelimportrow uses), attributed on bothuser_idandactor, and stamped with the caller's tenant plusorganization_idwhere the deployment declares that column — without which RLS hides every row and leaves the view exactly as empty as the defect.metadatacarries namespace/key/scope/encrypted;new_valuecarries a digest, never a value.Best-effort by construction:
sys_audit_logbelongs to the OPTIONAL@objectstack/plugin-audit, so on a deployment without it the insert throws on every settings write. The sink swallows and reports once per process, and the service's own sink call is now wrapped for the same reason — the guard is what protects a host-supplied sink from taking the write path down.A refused write emits nothing
A write refused before anything is persisted is not a successful one, so neither ledger records it. Pinned for the anonymous deny (
403 SETTINGS_FORBIDDEN) and the env-pinned key (409 SETTINGS_LOCKED) — both code and status — and separately for #8026'sSETTINGS_CRYPTO_UNAVAILABLE.start()passescryptoProvider: this.opts.cryptoProvider ?? new LocalCryptoProvider()andsecretStore: this.buildSecretStore(engine)— both always present — whileassertEncryptionAvailablereturns early on exactly that pair. So wherever the settings service has an engine, every declared-encrypted key CAN be encrypted and that refusal never fires; it is reachable only on an engine-less service or one whose host binds its own engine without a provider, and it is pinned there.Verification
Reverse-verified in two directions, both predicted RED before running:
origin/main, dogfood test kept — the parent's reproduction, red: the RESTconfig_changefilter polls to exhaustion (total 0), the shipped view's own declared filter returns 0 rows.sys_setting_audithad its row throughout, which is what makes the "unchanged" half a real measurement rather than an empty one.undefined— 9 of 11 seam cases red. The two refusal cases fail only on their non-vacuity half (expected [] to have a length of 1), which is precisely what that guard exists to catch: their zero-length assertions stay green either way.Green after:
service-settings451/451 + typecheck; the new dogfood file 4/4; the three neighbouringsys_audit_log/settings dogfood files 11/11; all 10 downstream consumers ofservice-settingstypecheck clean.Gates run locally:
check:nul-bytes(plus a control-byte self-scan of every changed file),check:cross-package-test-inputs,check:docs-audit-scope,check:route-envelope,check:test-source-alias,check:type-source-resolution,check-changeset-fixed, and — added from re-deriving against the actual diff —check:changeset-gate-self-tests,check:objectui-changeset,check:changeset-no-major,check:query-options-erasure,check:type-check-coverage,check:engine-double-contract. All pass.check:dev-prereqsreports the worktree is not fully built (8 packages this diff never touches have nodist) — a local build-state condition, not a finding.Scope note
packages/qa/dogfood/test/is outside the dispatch's declared file surface. The card's acceptance is explicitly end to end, and only a booted stack with plugin-audit installed can see the real object, its realactionenum and the shipped list view —service-settingsmust not depend on that plugin, which is the very reason the write is best-effort. The addition is one test file; production changes stay insidepackages/services/service-settings/src/**, andpackages/plugins/plugin-audit/**is untouched.Generated by Claude Code