Repository navigation
Commit f515124
fix(service-settings): settings audit writes old_hash and the declared SHA-256 digest (#26532)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XFdELkVjeF5csUMkbrNabaW)_
Fixes #26465
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:设置审计表 `sys_setting_audit` / `sys_platform_setting_audit` 的
`old_hash` 现在记录写入前那个值的摘要;非密钥值的摘要从 FNV-32 改为列声明的 SHA-256(`sha256:` 前缀)。
- **为什么改**:列声明「旧值的 SHA-256,未设置时为空」,实际 `old_hash` 永远为空、新值是
FNV-32,运维无法从审计轨迹判断一次写入是否真的改了值(重复写入检测失效)。
- **风险与回滚**:不新增任何数据库读取(旧值取自 upsert 已有的存在性探测);密钥值的带密钥摘要路径不变;历史 `fnv32:`
行不回填。回滚即还原本 PR。
-
**先看**:`packages/services/service-settings/src/settings-service.ts`(`upsertRow`
返回旧行、`previousAuditDigest`),`src/crypto-adapter.ts`(`settingAuditDigest`),`src/settings-audit-old-hash.test.ts`。
- **两分钟验证**:`pnpm --filter @objectstack/service-settings exec vitest run
src/settings-audit-old-hash.test.ts`,预期 7 个用例全绿。
- **需要你做的**:无,席位落地。
## What changes
- **`old_hash` is written.** `upsertRow` already probes the row at its
identity before it updates; it now returns that previous row
(snapshotted before the update) beside the `previousEnc` handle it
already returned. `setMany` passes `oldHash:
previousAuditDigest(previous)` instead of a hard-coded `null`. The read
happens under the same identity the write targets: the global rung's
`(namespace, key)` in `sys_platform_setting`, and `(organization_id,
namespace, key, scope, user_id)` in `sys_setting`. A write with no
previous row costs no extra round-trip, because the probe was already
there. `old_hash` is null when there was no row, when the row was reset
(`value` null), and when the previous row held a secret.
- **The non-secret digest is the declared SHA-256.** A new
`settingAuditDigest(canonicalJson)` in `crypto-adapter.ts` returns
`sha256:` + hex SHA-256 of the canonical JSON (`stableStringify`,
unchanged). The write path uses it for both the new and the previous
value instead of `this.crypto.digest(...)`, so an injected
`CryptoAdapter` cannot change the column's format. The same `digest`
variable feeds the `config_change` row in `sys_audit_log`, so both
ledgers keep recording one fingerprint per write.
- **Secret path untouched.** `secretAuditDigest` (the provider's keyed
`hmac-sha256:` digest, or null) and its two call sites are
byte-identical; a secret write keeps `oldHash: null`.
- **`CryptoAdapter.digest`**: the only non-test caller in the repository
was `settings-service.ts:2428`. The interface member stays, so existing
adapters still satisfy it. Its docblock and the `NoopCryptoAdapter` FNV
comment now say the settings audit no longer reads it.
- **Claim amendment, granted by the seat:**
`docs/qa/platform-checklist/areas/platform-core.json`, item
`platform-core.settings-audit-secret-fingerprint`, revision 4 → 5. Its
step-7 positive control computed `fnv32:` for the non-secret
`mail.from_email`. After this change that control could never match, and
it could no longer prove the offline search works. The control and
acceptance[4] now compute `sha256:`. The secret search keeps FNV-1a 32
as a second net, so the item is not loosened. The `history` row says the
same.
- **In-package docblock (same claim):** `sys-secret-orphan-report.ts`
said the service passes the adapter's `digest()` for non-secret keys. It
now names `settingAuditDigest()`.
- Changeset: `@objectstack/service-settings` patch.
`packages/platform-objects` is not touched; the declared columns stand.
## Tests
New `src/settings-audit-old-hash.test.ts` drives `PUT
/api/settings/:namespace` through `registerSettingsRoutes` over a real
`ObjectQL` with the real `SysSetting` / `SysPlatformSetting` /
`SysSecret` / `SysSettingAudit` / `SysPlatformSettingAudit` objects and
the plugin's own `buildSettingAuditWriter`. It reads the audit rows back
out of the driver store, and computes the expected digests with
`node:crypto`, not the implementation:
- tenant rung (`sys_setting_audit`): first set (old null, new
`sha256:`), overwrite (old = digest of previous, chained to the prior
row's `new_hash`), same-value write (old == new), set → reset → set
(reset: old = previous digest, new null; the next set: old null). A null
value is the family's removal; the settings routes have no DELETE verb,
so reset is the delete case.
- scope: a user-scope write reads its own previous row, not another
user's.
- global rung (`sys_platform_setting_audit`): first set, overwrite,
same-value write.
- CONTROL, secret value: keyed `hmac-sha256:` `new_hash`, null
`old_hash` on both writes, no unkeyed SHA-256 of the secret anywhere in
the row.
Updated: `settings-audit-secret-digest.test.ts`, non-secret case: it
pinned the adapter's FNV digest; it now pins the `sha256:` digest on
both ledgers and that it is not the adapter's.
Ablation, once, on the old-value read: `node
scripts/ablation-replace.mjs` replaced `oldHash: isEncrypted ? null :
SettingsService.previousAuditDigest(previous),` with `oldHash: null,`.
The tool's own check confirmed it landed (anchor 1 → 0, blob `7425ed93`
→ `3b7fd19c`). Result: **5 of 7 red** (overwrite, same-value,
set→reset→set, user scope, global rung). The two green are the first-set
case and the secret control, which do not depend on the read. Restore
was verified: blob == HEAD and `git diff HEAD` is empty. The test
imports `./settings-service.js` from `src/` (no dist hop), so no rebuild
was needed between the legs.
Package run on the final head `a397e85a77` (after merging `origin/main`
`b7f2b0c290`), under the verify lock: `pnpm --filter
@objectstack/service-settings build && … typecheck && … exec vitest run
--maxWorkers=2` → `Test Files 61 passed (61)`, `Tests 1007 passed
(1007)`, `VERDICT command-exit 0`. `tsc --listFiles` includes both
edited test files. `eslint --no-inline-config
packages/services/service-settings/src` exits 0. `test:repo` and dogfood
are left to CI: the title has no `!` and there is no `(narrowing)` arm.
## Acceptance notes
- **Mixed history.** Rows written before this change carry `fnv32:`
(non-secret) and `hmac-sha256:` (secret). New non-secret rows carry
`sha256:`. No reader in the repository compares hashes across rows: `git
grep` for `old_hash` / `new_hash` / `newHash` outside service-settings
finds only fixtures, docs and the platform-object declarations. A reader
that does compare them must compare only same-prefix values. A `fnv32:`
row next to a `sha256:` row of the same value reads as "different
algorithm", not "different value", because the prefix says so. The first
write after upgrading over an `fnv32:` row records `old_hash` as the
`sha256:` digest of the stored previous value. It is computed from the
value, not copied from the old row, so it does not equal that row's
`new_hash` string. Stored rows are not rewritten (ruling).
- **Column docblock vs prefix.** `sys-setting-audit.object.ts:137-142`
says "Format: hex SHA-256 of the canonicalised JSON". The stored form is
`sha256:` + that hex (71 chars, under `maxLength: 128`). The prefix
follows the convention already in the column: `hmac-sha256:` for
secrets, `fnv32:` before. The `description` strings ("SHA-256 of the
previous value (canonicalised)") hold as written. The TSDoc line does
not mention the prefix. It is left as is, because platform-objects is
outside the claim.
- **Secret rows keep `old_hash` null.** Writing the previous secret's
keyed digest would need a `sys_secret` read plus a decrypt of the
previous value on every secret write. The ruling keeps the secret path
out of scope, so the "null only when previously unset" declaration is
still not met for `encrypted: true` rows.
- **`CryptoAdapter.digest` now has zero non-test readers in this
repository.** It is a published interface member, so retiring it is a
separate API decision and is not done here.
- Open PRs touching `packages/services/service-settings/**`: none of the
18 open non-bot PRs at start (first page of each PR's files).
## Verification
<details><summary>Derived gate families: 64 of 64 exit 0 on a397e85,
reconciled with --ran</summary>
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 64 commands from the tree at
`a397e85a77`. Each one was run with its exit code captured before any
pipe, and all 64 answered `exit 0`. `dispatch-gates --ran`: "64 derived
famil(ies) accounted for — 64 run, 0 NOT-MEASURED". The changed-lines
reading is 484 (+458 / -26), under the 3000 human-merge threshold.
On the first pass, before the merge, two runs were prerequisite
refusals. `check-plugin-teardown-shape --self-test` exited 3 on the
shallow clone, and after `git fetch --unshallow origin main` it passed.
`check:dual-build-cjs-loads` exited 3 because other packages had no
`dist/`. Both answered exit 0 in the final-head pass above, after the
merge rebuild.
Every family outside the derivation (artifact rosters, wide-population,
the type-check lanes) is left to this PR's CI check-runs.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent f486e9c commit f515124
7 files changed
Lines changed: 458 additions & 26 deletions
File tree
- .changeset
- docs/qa/platform-checklist/areas
- packages/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 | |
|---|---|---|---|
| |||
3136 | 3136 | | |
3137 | 3137 | | |
3138 | 3138 | | |
3139 | | - | |
| 3139 | + | |
3140 | 3140 | | |
3141 | 3141 | | |
3142 | 3142 | | |
| |||
3159 | 3159 | | |
3160 | 3160 | | |
3161 | 3161 | | |
3162 | | - | |
| 3162 | + | |
3163 | 3163 | | |
3164 | 3164 | | |
3165 | | - | |
| 3165 | + | |
3166 | 3166 | | |
3167 | 3167 | | |
3168 | 3168 | | |
| |||
3172 | 3172 | | |
3173 | 3173 | | |
3174 | 3174 | | |
3175 | | - | |
| 3175 | + | |
3176 | 3176 | | |
3177 | 3177 | | |
3178 | 3178 | | |
| |||
3190 | 3190 | | |
3191 | 3191 | | |
3192 | 3192 | | |
3193 | | - | |
| 3193 | + | |
3194 | 3194 | | |
3195 | | - | |
| 3195 | + | |
3196 | 3196 | | |
3197 | 3197 | | |
3198 | 3198 | | |
| |||
3203 | 3203 | | |
3204 | 3204 | | |
3205 | 3205 | | |
3206 | | - | |
| 3206 | + | |
3207 | 3207 | | |
3208 | 3208 | | |
3209 | 3209 | | |
| |||
3214 | 3214 | | |
3215 | 3215 | | |
3216 | 3216 | | |
3217 | | - | |
| 3217 | + | |
3218 | 3218 | | |
3219 | 3219 | | |
3220 | 3220 | | |
| |||
3248 | 3248 | | |
3249 | 3249 | | |
3250 | 3250 | | |
| 3251 | + | |
| 3252 | + | |
| 3253 | + | |
| 3254 | + | |
| 3255 | + | |
| 3256 | + | |
3251 | 3257 | | |
3252 | 3258 | | |
3253 | 3259 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
3 | 5 | | |
4 | 6 | | |
5 | 7 | | |
| |||
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
17 | | - | |
18 | | - | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
19 | 27 | | |
20 | 28 | | |
21 | 29 | | |
| |||
79 | 87 | | |
80 | 88 | | |
81 | 89 | | |
82 | | - | |
| 90 | + | |
| 91 | + | |
83 | 92 | | |
84 | 93 | | |
85 | 94 | | |
| |||
89 | 98 | | |
90 | 99 | | |
91 | 100 | | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
92 | 119 | | |
93 | 120 | | |
94 | 121 | | |
| |||
0 commit comments