Repository navigation
fix(service-settings): settings audit writes old_hash and the declared SHA-256 digest - #26532
Conversation
…gest and the declared SHA-256 old_hash was hard-coded null and a non-secret new_hash was the adapter's FNV-1a 32 while both audit columns declare SHA-256 of the canonical value. The previous value comes off the identity probe upsertRow already makes. Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude <noreply@anthropic.com>
…ST door Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude <noreply@anthropic.com>
…ites; changeset Claude-Session: https://claude.ai/code/session_01FEkQ5ojTc5Fbj8xxXSfo1K Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b16f155de9d17fdbe4732d8901483d04fd1f31f0 && git checkout b16f155de9d17fdbe4732d8901483d04fd1f31f0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 79cd9ef8f08a4e6439373e4002f172ced11ad46a a397e85a77d4c984c802387e7e0d25dd239964f5 && git checkout -B drift-repro 79cd9ef8f08a4e6439373e4002f172ced11ad46a && git merge --no-ff a397e85a77d4c984c802387e7e0d25dd239964f5
node scripts/docs-audit/affected-docs.mjs --json 79cd9ef8f08a4e6439373e4002f172ced11ad46a
|
席位核对ACCEPT · The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.
Generated by Claude Code |
Requested by Leehom · project thread
Fixes #26465
Clause-②: no
Metadata-protocol: none
维护者速读
sys_setting_audit/sys_platform_setting_audit的old_hash现在记录写入前那个值的摘要;非密钥值的摘要从 FNV-32 改为列声明的 SHA-256(sha256:前缀)。old_hash永远为空、新值是 FNV-32,运维无法从审计轨迹判断一次写入是否真的改了值(重复写入检测失效)。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_hashis written.upsertRowalready probes the row at its identity before it updates; it now returns that previous row (snapshotted before the update) beside thepreviousEnchandle it already returned.setManypassesoldHash: previousAuditDigest(previous)instead of a hard-codednull. The read happens under the same identity the write targets: the global rung's(namespace, key)insys_platform_setting, and(organization_id, namespace, key, scope, user_id)insys_setting. A write with no previous row costs no extra round-trip, because the probe was already there.old_hashis null when there was no row, when the row was reset (valuenull), and when the previous row held a secret.settingAuditDigest(canonicalJson)incrypto-adapter.tsreturnssha256:+ hex SHA-256 of the canonical JSON (stableStringify, unchanged). The write path uses it for both the new and the previous value instead ofthis.crypto.digest(...), so an injectedCryptoAdaptercannot change the column's format. The samedigestvariable feeds theconfig_changerow insys_audit_log, so both ledgers keep recording one fingerprint per write.secretAuditDigest(the provider's keyedhmac-sha256:digest, or null) and its two call sites are byte-identical; a secret write keepsoldHash: null.CryptoAdapter.digest: the only non-test caller in the repository wassettings-service.ts:2428. The interface member stays, so existing adapters still satisfy it. Its docblock and theNoopCryptoAdapterFNV comment now say the settings audit no longer reads it.docs/qa/platform-checklist/areas/platform-core.json, itemplatform-core.settings-audit-secret-fingerprint, revision 4 → 5. Its step-7 positive control computedfnv32:for the non-secretmail.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 computesha256:. The secret search keeps FNV-1a 32 as a second net, so the item is not loosened. Thehistoryrow says the same.sys-secret-orphan-report.tssaid the service passes the adapter'sdigest()for non-secret keys. It now namessettingAuditDigest().@objectstack/service-settingspatch.packages/platform-objectsis not touched; the declared columns stand.Tests
New
src/settings-audit-old-hash.test.tsdrivesPUT /api/settings/:namespacethroughregisterSettingsRoutesover a realObjectQLwith the realSysSetting/SysPlatformSetting/SysSecret/SysSettingAudit/SysPlatformSettingAuditobjects and the plugin's ownbuildSettingAuditWriter. It reads the audit rows back out of the driver store, and computes the expected digests withnode:crypto, not the implementation:sys_setting_audit): first set (old null, newsha256:), overwrite (old = digest of previous, chained to the prior row'snew_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.sys_platform_setting_audit): first set, overwrite, same-value write.hmac-sha256:new_hash, nullold_hashon 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 thesha256:digest on both ledgers and that it is not the adapter's.Ablation, once, on the old-value read:
node scripts/ablation-replace.mjsreplacedoldHash: isEncrypted ? null : SettingsService.previousAuditDigest(previous),witholdHash: null,. The tool's own check confirmed it landed (anchor 1 → 0, blob7425ed93→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 andgit diff HEADis empty. The test imports./settings-service.jsfromsrc/(no dist hop), so no rebuild was needed between the legs.Package run on the final head
a397e85a77(after mergingorigin/mainb7f2b0c290), 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 --listFilesincludes both edited test files.eslint --no-inline-config packages/services/service-settings/srcexits 0.test:repoand dogfood are left to CI: the title has no!and there is no(narrowing)arm.Acceptance notes
fnv32:(non-secret) andhmac-sha256:(secret). New non-secret rows carrysha256:. No reader in the repository compares hashes across rows:git grepforold_hash/new_hash/newHashoutside service-settings finds only fixtures, docs and the platform-object declarations. A reader that does compare them must compare only same-prefix values. Afnv32:row next to asha256:row of the same value reads as "different algorithm", not "different value", because the prefix says so. The first write after upgrading over anfnv32:row recordsold_hashas thesha256: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'snew_hashstring. Stored rows are not rewritten (ruling).sys-setting-audit.object.ts:137-142says "Format: hex SHA-256 of the canonicalised JSON". The stored form issha256:+ that hex (71 chars, undermaxLength: 128). The prefix follows the convention already in the column:hmac-sha256:for secrets,fnv32:before. Thedescriptionstrings ("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.old_hashnull. Writing the previous secret's keyed digest would need asys_secretread 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 forencrypted: truerows.CryptoAdapter.digestnow 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.packages/services/service-settings/**: none of the 18 open non-bot PRs at start (first page of each PR's files).Verification
Derived gate families: 64 of 64 exit 0 on a397e85, reconciled with --ran
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 64 commands from the tree ata397e85a77. Each one was run with its exit code captured before any pipe, and all 64 answeredexit 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-testexited 3 on the shallow clone, and aftergit fetch --unshallow origin mainit passed.check:dual-build-cjs-loadsexited 3 because other packages had nodist/. 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.
Generated by Claude Code