Skip to content

fix(service-settings): a secret overwrite's audit row records the previous value's keyed digest as old_hash - #26600

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-s4tyh8
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-s4tyh8

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #26534

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:密钥类设置(encrypted: true)被覆盖时,审计行 old_hash 不再恒为 null,而是记上一次值的带密钥摘要(hmac-sha256:…),与 new_hash 同一算法;所以覆盖时 old_hash 等于上一行的 new_hash,同值重写时 old_hash == new_hash。
  • 为什么改:列声明“仅在之前未设置时为 null”,但密钥行一直写 null,运维无法从审计判断一次轮换是真换了密钥还是写回了同一个值,而这恰是最需要判断的行。
  • 风险与回滚:低。只有“密钥覆盖”这一种写多一次 sys_secret 读和一次解密,且在旧密文被回收之前;读不到或解不开时记 null,写入照常成功;明文只在函数内用于算摘要,不记日志、不返回、不存储;从不写无密钥哈希。回滚即 revert 本 PR。
  • 先看:packages/services/service-settings/src/settings-service.ts(setMany 中 upsertRow 与 reapRotatedSecret 之间新增的 oldHash,以及 previousSecretAuditDigest);src/settings-audit-old-hash.test.ts(#26534 一组用例)。
  • 两分钟验证:pnpm --filter @objectstack/service-settings exec vitest run src/settings-audit-old-hash.test.ts,预期 14/14 通过,其中 #26534 八条。
  • 需要你做的:无,席位落地。

What changes

SettingsService.setMany now computes the audit's oldHash after upsertRow (whose existence probe names the previous row) and before reapRotatedSecret (which destroys the previous secret's sys_secret row):

  • Secret write (encrypted: true key): old_hash is previousSecretAuditDigest(...), the same keyed digest new_hash records (secretAuditDigest, the provider's keyedDigest), taken over the previous value. The previous plaintext is read through a new private decryptStored(row), factored out of materialiseRow with no behaviour change there, and is digested and dropped inside that frame. null when the previous row held no secret (no row, a reset row, or a row written while the key was not secret-valued), when no keyed digest is available (checked before the read, so that write pays nothing), or when the previous value cannot be read back (handle gone, decrypt refused; the latter warns once on the logger with the key and the decrypt error, never the value). It never fails the write.
  • Non-secret write: unchanged, previousAuditDigest(previous) (service-settings: sys_setting_audit.old_hash is never written (hard-coded null) and new_hash for a non-secret value is an FNV-32 digest, while both columns declare "SHA-256 of the … value" #26465).
  • With no auditWriter wired, nothing is computed.

Never a digest of ciphertext (it changes on every re-encryption), never an unkeyed hash of the plaintext. The secret reaper (#8030 previousEnc) is unchanged. Changeset: @objectstack/service-settings patch.

Why

sys_setting_audit.old_hash and sys_platform_setting_audit.old_hash declare "Null when previously unset", but the secret arm passed oldHash: isEncrypted ? null : ..., so every secret overwrite recorded null. The encrypted column exists to flag rotation events, and without old_hash an operator cannot tell a rotation that changed the secret from one that rewrote the same value. Rulings: triage 6436617436 and the server seat 2 ruling, arm (a).

Tests

All through PUT /api/settings/:namespace on the #26465 harness (real ObjectQL, real platform objects, the plugin's own sys_secret store and audit writer). The old secret CONTROL that pinned old_hash: null is replaced by a #26534 block:

Case Asserted
first secret write old_hash null, keyed new_hash
secret overwrite old_hash keyed and equal to the previous row's new_hash; differs from the new new_hash
same-value secret rewrite old_hash == new_hash == previous row's new_hash
secret → reset → set reset: old_hash = previous keyed digest, new_hash null; next set: old_hash null
cost a non-secret write reads no sys_secret; a first write reads only its own handle; an overwrite reads the previous handle exactly one more time than a boot with no keyed digest, and that handle is still reaped afterwards
no keyed digest both columns null, write lands
previous handle gone old_hash null, keyed new_hash, write lands and reads back
manifest flips the key non-secret → secret → non-secret both crossings record old_hash null; no unkeyed digest of the secret

Every secret case keeps the "no plaintext and no unkeyed SHA-256 of the secret anywhere in the row" assertion.

Ablation (committed fix at d66cfb39, scripts/ablation-replace.mjs): await this.previousSecretAuditDigest(namespace, key, previous) replaced by null, anchor 1 → 0, blob 148247002eaf → 0994b6cb2558. Result: 4 failed, 10 passed (overwrite, same-value, reset, cost). Restored: blob == HEAD 148247002eaf, git diff HEAD empty.

Package at d66cfb39: pnpm --filter @objectstack/service-settings test Test Files 61 passed (61), Tests 1014 passed (1014); typecheck (tsc --noEmit, include: ["src"], tests included) exit 0. eslint --no-inline-config on packages/services/service-settings: exit 0.

Acceptance notes

  • Cost measured. The write path already decrypts the key's current value once in validatePatch (it resolves get() for every touched key), so a secret overwrite now makes two reads of the previous handle where it made one. That earlier read is the cascade's effective value, which can come from another rung or the environment, so it is not the row's previous value and is not reused.
  • Crossing between non-secret and secret is not reachable through PUT. encrypted comes from the manifest, and only a re-registration (registerManifest replaces the namespace's entry; in practice a deploy that edits the manifest while stored rows persist) changes it. Both crossings record old_hash null: non-secret → secret has no previous secret to key-digest, and secret → non-secret must not write an unkeyed digest of the previous secret. Pinned by the last case.
  • QA checklist. platform-core.settings-audit-secret-fingerprint says nothing about a secret row's old_hash, and no digest algorithm changed, so its revision is not bumped. Its "no unkeyed digest in any row" clause still holds: the new old_hash is keyed.

Verification

Derived gate families (`dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, run at d66cfb3)

60 derived, 59 run with exit 0, 1 NOT MEASURED: pnpm check:dual-build-cjs-loads exit 3 (prerequisite not met: needs a full build; CI runs it). node scripts/check-plugin-teardown-shape.mjs --self-test first exited 3 on the shallow clone; after git fetch origin 621a4876 (its pinned fixture) it ran: exit 0, 48 cases. dispatch-gates.mjs --ran: "60 derived famil(ies) accounted for — 59 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)". test:repo was not run locally (the diff touches no repo-project test); CI Test Core runs it.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (No packages/spec change.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes. (Scoped to the touched package.)
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (Not touched.)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FEkQ5nYPKQq7oxF4u1xFKB

…vious value's keyed digest as old_hash

The secret arm of the settings audit write passed old_hash null on every
write, though the column declares null only when the key was previously
unset. old_hash on a secret write is now the same keyed digest new_hash
uses, taken over the previous value: the upsert's probe names the
previous row, its handle is read and decrypted before the reaper
destroys it, and the plaintext never leaves that frame. No keyed digest,
an unset or reset previous value, or a handle that no longer resolves
records null and never fails the write.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5nYPKQq7oxF4u1xFKB
@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 5 documentable anchor(s).

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
  • 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 fd97351593a3d7c76d0def9da093cdbf3558222f → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 002de79f81109b41e4fbbe150c795b8bd42e039d — the merge of head d66cfb3989510da0161333bf93bfeef650ca7626 into base fd97351593a3d7c76d0def9da093cdbf3558222f, 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 002de79f81109b41e4fbbe150c795b8bd42e039d && git checkout 002de79f81109b41e4fbbe150c795b8bd42e039d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fd97351593a3d7c76d0def9da093cdbf3558222f d66cfb3989510da0161333bf93bfeef650ca7626 && git checkout -B drift-repro fd97351593a3d7c76d0def9da093cdbf3558222f && git merge --no-ff d66cfb3989510da0161333bf93bfeef650ca7626

node scripts/docs-audit/affected-docs.mjs --json fd97351593a3d7c76d0def9da093cdbf3558222f

⚠️ 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 fd97351593a3d7c76d0def9da093cdbf3558222f → 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-27T10:11Z · head d66cfb3989 · #26534

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 #26534, Clause-②: no and Metadata-protocol: none, all line-initial. The changeset is an @objectstack/service-settings patch. check-governed-merges --pr 26600 reads NOT governed, 342 changed lines. No declaration moves, so no at-tier review is owed. The card's newest claim (6437559421) names this PR's branch.

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

    • Same digest as new_hash. old_hash on a secret write comes from previousSecretAuditDigest. That calls the same secretAuditDigest(namespace, key, plain) new_hash uses, over the previous row's decrypted plaintext, which is the same input form the new value is digested in. So an overwrite's old_hash equals the prior row's new_hash.
    • Null cases. It is null when the previous row held no secret, when its handle is gone, or when no keyed digest is available. The availability check comes before the read, so that write pays nothing. A failed decrypt also gives null: it logs the key and the error, never the value, and does not fail the write.
    • No ciphertext digest and no unkeyed hash. The plaintext lives only in that frame.
    • Order and cost. The digest is computed after the upsert's probe and before the [security] Rotating an encrypted setting silently does nothing — the new ciphertext is written but sys_setting.value_enc still points at the old handle, so a leaked credential stays live after a 200 #8030 reap, so the previous handle still resolves. With no auditWriter, nothing is computed.
    • Refactor. The decrypt is decryptStored(row), factored out of materialiseRow with the same branches and the same null-on-missing-handle. materialiseRow's catch-and-null behaviour is unchanged.
  3. Tests (the ruling's list), through PUT /api/settings/:namespace:

    • first secret write;
    • overwrite, with old_hash equal to the prior keyed new_hash;
    • same-value rewrite, with old_hash == new_hash;
    • secret → reset → set;
    • the cost case;
    • both manifest crossings (non-secret ↔ secret, reachable only by re-registering the manifest), which record old_hash null.

    The "no unkeyed SHA-256 of the secret anywhere in the row" assertion stays. One ablation of the previous-value read reds 4 of 14 and was restored clean.

  4. Overlap: chore(deps)(deps-dev): bump the development-dependencies group across 1 directory with 22 updates #26556 (dependabot dev-dependency bump) touches packages/services/service-settings/package.json only, and git merge-tree with its head is clean. No other open PR touches the package (seat's read at this stamp).

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

    • service-settings 1,014 / 1,014 and typecheck 0;
    • 60 derived gates: 59 exit 0, with check:dual-build-cjs-loads not measured (prerequisite; CI covers it).
  6. Out of scope:


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 10:11
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 10:12
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 179598e Oct 27, 2026
48 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-s4tyh8 branch October 27, 2026 10:29
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