Skip to content

fix(service-settings): settings audit writes old_hash and the declared SHA-256 digest - #26532

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-su83v0
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-su83v0

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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: 前缀)。
  • 为什么改:列声明「旧值的 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

Derived gate families: 64 of 64 exit 0 on a397e85, reconciled with --ran

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.


Generated by Claude Code

…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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-settings, touching 6 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-settings/src/sys-secret-orphan-report.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via SettingsService (symbol, a top-level class))
  • content/docs/kernel/runtime-services/settings-service.mdx (via setMany (symbol, a method of class SettingsService))
  • content/docs/protocol/kernel/config-resolution.mdx (via SettingsService (symbol, a top-level class), setMany (symbol, a method of class SettingsService))
  • content/docs/protocol/kernel/index.mdx (via SettingsService (symbol, a top-level class))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via setMany (symbol, a method of class SettingsService))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-settings/src/sys-secret-orphan-report.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 55 of 207 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 79cd9ef8f08a4e6439373e4002f172ced11ad46a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b16f155de9d17fdbe4732d8901483d04fd1f31f0 — the merge of head a397e85a77d4c984c802387e7e0d25dd239964f5 into base 79cd9ef8f08a4e6439373e4002f172ced11ad46a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 79cd9ef8f08a4e6439373e4002f172ced11ad46a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-27T07:16Z · head a397e85a77 · #26465

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape: a draft against main, assignee os-project-manager. The body carries Fixes #26465, Clause-②: no and Metadata-protocol: none, all line-initial. The changeset is an @objectstack/service-settings patch. check-governed-merges --pr 26532 reads NOT governed, 484 changed lines. The declared columns do not change and no contract face is touched, so no at-tier review is owed. The card's newest claim (6434767635, amended for the file surface) names this PR's branch.

  2. Diff against the ruling (the seat read every source hunk):

    • Enforce arm. upsertRow returns the row its existing probe found, snapshotted before the update, beside previousEnc. setMany writes oldHash: isEncrypted ? null : previousAuditDigest(previous). The read is the upsert's own probe under the write's identity, so there is no new round-trip, and a first write reads nothing new. Reset and a previously-unset row answer null. That covers set → reset → set; the routes have no DELETE verb, so reset is the removal case.
    • Declared algorithm. settingAuditDigest() is sha256: + hex SHA-256 of the unchanged stableStringify canonical JSON. It is used for both the new and the previous value, independent of any injected CryptoAdapter. The prefix keeps the format self-describing next to the stored fnv32: and hmac-sha256: rows.
    • Secret path unchanged. secretAuditDigest and its call sites are untouched, and a secret write keeps oldHash: null. A previous secret row also answers null, so no unkeyed hash of a secret is ever computed.
    • History. No backfill. The body measures that no reader compares hashes across rows, and states how mixed history reads: compare only same-prefix values.
  3. Amendment surface:

    • The platform-core.json item platform-core.settings-audit-secret-fingerprint goes from revision 4 to 5. Its step-7 control and acceptance[4] now compute sha256:, and FNV-1a 32 stays in the secret search as a second net, so the item is not loosened. A history row is added.
    • The sys-secret-orphan-report.ts docblock now names settingAuditDigest().
    • Both are inside the amended claim.
  4. Tests (the ruling's list): the new settings-audit-old-hash.test.ts drives PUT /api/settings/:namespace over a real ObjectQL and the real audit objects, with expected digests computed by node:crypto. It covers:

    • first set, overwrite (chained to the prior new_hash) and same-value, on both rungs;
    • set → reset → set;
    • user-scope isolation;
    • the secret control.

    The one ablation of the old-value read reds 5 of 7. The two that stay green are the first-set case and the secret control, which do not depend on the read. The existing non-secret case in settings-audit-secret-digest.test.ts now pins sha256: and that it is not the adapter digest.

  5. Overlap: none. No other open PR touches packages/services/service-settings/** or the checklist item (seat's read at this stamp).

  6. Gates: at this stamp every check-run on the head reads success or skipped; none reads failed or in progress. Locally the dev reports:

    • 64 of 64 derived gates exit 0, reconciled with --ran, with 0 not measured;
    • service-settings 1,007 / 1,007, typecheck and scoped lint green.
  7. Out of scope:


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 07:17
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 07:17
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit f515124 Oct 27, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-su83v0 branch October 27, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants