Repository navigation
Commit 616311c
fix(service-settings): a settings change's audit row records the value before and after, with secrets as digests (#30937)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DCcyTeyTsVPTTFaZHQJekZS)_
domain:server#1 · os-elon-musk · session_016WV4cy588UNGpATRLjBi9h
Fixes #30923
Claim: 6684421676 (full re-post of 6684227943 with this branch; file
surface amended for the dogfood pin, the QA checklist entry and the
plugin-audit redaction file)
Clause-②: yes (widening)
Metadata-protocol: none
## 维护者速读
- 改了什么:修改设置时写入审计日志的 `config_change`
记录,现在会记下改前值(`old_value`)和改后值(`new_value`),非密钥类设置是可读的原值;密钥类设置(清单标
`encrypted: true`,`password`
类型自动算)两边都只记掩码摘要,从不记明文。审计读取端同时补上一道限制:字段级权限看不到设置值的读者,在这条记录里也看不到值和摘要。
- 为什么改:安全审查要求访问策略(如允许的 IP
范围)的每次修改都能看出改前改后;此前这条记录只有新值的摘要,没有任何入口能读出前后值。ADR-0007
的「审计与历史」一节本来就规定要记前后值、加密值只记占位和校验和,这次是把实现补齐到 ADR。
-
风险与回滚:只往审计行里多写内容,不改列、不改设置写入本身;密钥规则只在设置服务一处判定,测试覆盖密钥改写、重置、标记翻转三种情况都不出明文。读取端收窄只影响被字段级权限屏蔽了设置值的读者。单
PR 可整体回滚。
- 先看:`packages/services/service-settings/src/settings-service.ts` 的
`auditReadable` / `auditPrevious`(密钥规则与改前值);`config-change-audit.ts` 的
`old_value` / `new_value`
写法;`packages/plugins/plugin-audit/src/audit-log-field-redaction.ts` 新增的
`config_change` 收窄。
- 两分钟验证:`pnpm --filter @objectstack/service-settings exec vitest run
src/config-change-audit.test.ts`,预期 24 个用例全过,其中 `#30923` 开头的 6 个覆盖 IP
范围前后值、重置、首次写入和密钥。
- 需要你做的:无,席位落地。
## What changes
ADR-0007 § Audit & history decides that every settings write records its
before and after on `sys_audit_log`, and that a value declared
`encrypted: true` is recorded only as a placeholder plus a checksum. The
`config_change` row recorded a digest of the after and nothing of the
before. This PR implements the ADR. No ADR records "a digest, never the
value" (searched `docs/adr/**`); the module header that said so was the
deviation, and it is rewritten.
- **`config_change` row (`config-change-audit.ts`).** `new_value` is `{
namespace, key, scope, value, digest }`, and `null` on a reset as
before. `old_value` (an existing column, previously never written) has
the same shape for the value the same stored settings row held before
the write (the row `record_id` names). It is `null` when that row held
no value: a first write at that rung, or a row an earlier reset cleared.
A reset records its before there.
- **The secret rule, in one place (`SettingsService.auditReadable`).** A
value is written readable only when the key is not secret-valued and the
row it comes from is not stored `encrypted: true`. Secret-valued is the
manifest's `encrypted: true`, which `type: 'password'` forces
(`registerManifest` reads it into `encryptedKeys`, `settings-service.ts`
about line 1259). That is the only sensitivity flag a specifier
declares. No `sensitive` flag exists in `packages/spec`, and none is
needed, because every credential key in the shipped manifests is `type:
'password'`. A secret's sides carry only `digest`: the encrypted marker,
which is the word `encrypted`, a colon and the provider's keyed digest,
wrapped in angle brackets (the bare word in angle brackets when no keyed
digest is available). That is byte-for-byte the `new_value` these rows
carried before, so a reader that parses it keeps working. The before of
a secret is the same keyed digest the settings trail's `old_hash`
records. `oldHash` is now taken when either ledger is wired, before the
reaper destroys the previous ciphertext. A plaintext row of a key
declared secret since then, and an encrypted row of a key no longer
declared secret, are both recorded masked. `metadata` is unchanged and
carries no value.
- **What the sink is handed (`settings-service.types.ts`).**
`SettingsAuditSink.record`'s entry gains two optional fields: `value`,
the after, readable, absent for a secret and on a reset; and `previous`,
`{ value?, digest } | null`. A host sink that ignores them works as
before.
- **Every door.** `sys_setting` declares `apiMethods: ['get', 'list']`
and `managedBy: 'engine-owned'`, so the data API cannot write it. Every
settings door (Setup, `PUT /api/settings/:namespace`, the built-in
`reset` action, `resetNamespace`, in-process `set`/`setMany`) writes
through `SettingsService.setMany`, the only caller of the sink.
- **Who is served the readable value (`plugin-audit`,
`audit-log-field-redaction.ts`).** The ledger's read-side field
redaction narrowed only the CRUD mirror's create/update/delete
snapshots, and served `config_change` rows as written on the ground that
they held only a digest. A `config_change` row about a settings store
(`sys_setting`, or `sys_platform_setting` for a global value) is now
judged by that store's `value` field. A reader not served `value`
(masked or withheld by field-level security) gets neither the value nor
its digest on either side, because an unkeyed digest of a short value
answers "is it X?". `namespace`, `key` and `scope` stay. The mirror's
row about the same write is narrowed the same way. A snapshot that is
not a JSON object is dropped (fail closed). A `config_change` row naming
no settings store (the deployment notice's hide) is served as written,
as before.
## Tests
- `config-change-audit.test.ts`, new top-level describe, using the real
`SettingsServicePlugin` boot, its own sink, and the real `auth`
manifest:
- `auth.allowed_ip_ranges`: a first write records `old_value: null`, and
a change records both values readable. The before's digest equals the
previous after's digest and the settings trail's `old_hash`, and
`metadata` carries no value.
- The same through `PUT /api/settings/auth`.
- A reset records its before and `new_value: null`, and a write after a
reset records a null before.
- A secret key written, rotated and reset: no plaintext in `old_value`,
`new_value` or `metadata`. Every side is the encrypted marker around an
`hmac-sha256:` digest of 64 hex digits, each before equals the previous
after, and the `new_value` keys stay `namespace, key, scope, digest`.
- A key declared secret after a plaintext write: the replaced plaintext
is never recorded.
- A key no longer declared secret: the encrypted row it replaces is
recorded masked.
- `audit-log-field-redaction.test.ts`, new top-level describe, on
`redactAuditLogRows` over rows in the writer's shape (plugin-audit does
not depend on service-settings):
- A reader with `value` masked on `sys_setting` or on
`sys_platform_setting` gets `{ namespace, key, scope }` on both sides.
- A control reader gets the row byte-identical.
- A `config_change` row naming no settings store is served as written.
- A non-JSON snapshot is dropped.
- `settings-config-change-audit.dogfood.test.ts`: the two assertions
that pinned the old shape now assert the value. The first case now
expects `new_value.value`, and the #30209 case now expects the row to
carry the workspace name instead of asserting that it does not.
- `docs/qa/platform-checklist/areas/platform-core.json`
(`platform-core.settings-audit-secret-fingerprint`): `source[1]` now
describes the readable before and after, and the item gets revision 6
with a history row. No clause changed, because every clause there scores
the secret `mail.api_key`, whose rows still carry only the masked keyed
digest.
<details><summary>Measured on main 0e88208 before the fix (in-process
settings service, real sink, real auth and mail manifests)</summary>
- `auth.allowed_ip_ranges`, set, then changed, then reset: `old_value`
absent on all three rows. `new_value` was
`{"namespace":"auth","key":"allowed_ip_ranges","scope":"global","digest":"sha256:…"}`
on the two sets and `null` on the reset.
- `mail.api_key`, set, then rotated, then reset: `old_value` absent.
`new_value` carried namespace, key, scope and the encrypted marker
around an `hmac-sha256:` digest, then `null` on the reset. No plaintext.
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs; anchor 1 → 0,
mutation marker 0 → 1, restored blob == HEAD blob; both suites import
src, so no build between)</summary>
- A. `auditReadable` returns `true` (the secret rule dropped): 3 of the
6 `#30923` service cases fail. They are the secret write/rotate/reset
case (`new_value` contains the secret), the key declared secret later
(`old_value` contains the old plaintext), and the key no longer declared
secret (`old_value` has a `value` property). The 3 readable cases stay
green.
- B. the `config_change` branch removed from the redaction guard: 3 of
the 5 new redaction cases fail (masked on `sys_setting`, masked on
`sys_platform_setting`, non-JSON snapshot). The control case and the
no-store case stay green.
</details>
## Acceptance notes
- `old_value` / `new_value` describe the stored row at the write's own
rung, the row `record_id` names, and that row's previous value is what
`old_hash` already fingerprints. A first tenant-scope write records a
null before even when a global value or the default was in force.
- `Clause-②: yes (widening)`: the served row carries more. The
plugin-audit half removes the value and digest only for a reader whose
field-level security already withholds the settings store's `value`.
That reader was served the unkeyed digest of a non-secret value before,
so for them this is a security narrowing. The seat may judge whether
that half wants its own arm.
- A comment in `plugin-audit/src/audit-plugin.ts` ("a `config_change`
row's digest sits next to the mirror's row that carries the value")
still reads true for secrets but undersells non-secret rows. That file
is outside this PR's surface, so it is noted here and not edited.
## Verification
Local runs on the final head are quoted with that head in the report
comment on #30923: `@objectstack/service-settings` and
`@objectstack/plugin-audit` `test` and `typecheck`; the changed dogfood
file; every family `node scripts/pm/dispatch-gates.mjs --commands`
derives from this diff, reconciled with `--ran`; `pnpm lint`. CI's
required checks are the authority for everything else.
---
_Generated by [Claude
Code](https://claude.ai/code/session_016WV4cy588UNGpATRLjBi9h)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 3479c33 commit 616311c
9 files changed
Lines changed: 449 additions & 26 deletions
File tree
- .changeset
- docs/qa/platform-checklist/areas
- packages
- plugins/plugin-audit/src
- qa/dogfood/test
- services/service-settings/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3326 | 3326 | | |
3327 | 3327 | | |
3328 | 3328 | | |
3329 | | - | |
| 3329 | + | |
3330 | 3330 | | |
3331 | 3331 | | |
3332 | 3332 | | |
| |||
3408 | 3408 | | |
3409 | 3409 | | |
3410 | 3410 | | |
3411 | | - | |
| 3411 | + | |
3412 | 3412 | | |
3413 | 3413 | | |
3414 | 3414 | | |
| |||
3444 | 3444 | | |
3445 | 3445 | | |
3446 | 3446 | | |
| 3447 | + | |
| 3448 | + | |
| 3449 | + | |
| 3450 | + | |
| 3451 | + | |
| 3452 | + | |
3447 | 3453 | | |
3448 | 3454 | | |
3449 | 3455 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
350 | 350 | | |
351 | 351 | | |
352 | 352 | | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
50 | 59 | | |
51 | 60 | | |
52 | | - | |
53 | | - | |
54 | | - | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
55 | 65 | | |
56 | 66 | | |
57 | 67 | | |
| |||
87 | 97 | | |
88 | 98 | | |
89 | 99 | | |
| 100 | + | |
90 | 101 | | |
91 | 102 | | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
92 | 113 | | |
93 | 114 | | |
94 | 115 | | |
| |||
111 | 132 | | |
112 | 133 | | |
113 | 134 | | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
114 | 152 | | |
115 | 153 | | |
116 | 154 | | |
| |||
141 | 179 | | |
142 | 180 | | |
143 | 181 | | |
144 | | - | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
145 | 186 | | |
146 | 187 | | |
147 | 188 | | |
| |||
166 | 207 | | |
167 | 208 | | |
168 | 209 | | |
169 | | - | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
170 | 214 | | |
171 | 215 | | |
172 | 216 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
137 | | - | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
138 | 140 | | |
139 | 141 | | |
140 | 142 | | |
| |||
157 | 159 | | |
158 | 160 | | |
159 | 161 | | |
160 | | - | |
161 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
162 | 165 | | |
163 | 166 | | |
164 | 167 | | |
165 | 168 | | |
166 | 169 | | |
167 | 170 | | |
168 | | - | |
| 171 | + | |
169 | 172 | | |
170 | 173 | | |
171 | 174 | | |
| |||
0 commit comments