[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

[baozhoutao]

Found by a retest of platform-core.settings-hub-roundtrip (QA run #7514) against origin/main @ b602d536, rebuilt — specifically while checking the #7522/PR #7554 secret-redaction fix for over-correction. That fix is verified correct and is not the cause; see the attribution section.

Impact

A secret rotation appears to succeed and does not happen. PUT /api/settings/<ns> with a new value for an encrypted key returns 200 with a correctly redacted body, sys_setting.updated_at advances, and a new sys_secret row containing the new plaintext is inserted — but sys_setting.value_enc still points at the first handle, so the effective stored secret is still the original value.

The operationally dangerous shape: rotating a leaked SMTP password or provider API key looks like it worked — 200, masked read-back, audit row — while the old credential remains the one in force. Every signal an admin can see says the rotation landed.

The first write of any secret is correct, so this is invisible until the second one.

Reproduction (4×)

1. Sign in as admin.
2. PUT /api/settings/sms {provider:'twilio', twilio_account_sid:'AC1', twilio_from:'+15550000000', twilio_auth_token:'alpha'} → 200.
3. Read sys_setting (namespace=sms, key=twilio_auth_token): value_enc = sec_A. Decrypt sys_secret[sec_A] (AES-256-GCM, dev data key, AAD sms|twilio_auth_token) → alpha. ✅ correct.
4. PUT /api/settings/sms {twilio_auth_token:'beta'} → 200, correctly redacted body.
5. Re-read: sys_setting.updated_at has advanced; a new sys_secret row exists whose ciphertext decrypts to beta; but sys_setting.value_enc is still sec_A, so the effective secret is still alpha.

Expected: value_enc repointed to the new handle; effective secret beta.
Actual: rotation lost; orphan ciphertext rows accumulate (observed 7 → 8 → 9 across three writes).

Reproduced on mail.smtp_password (×2), storage.s3_secret_access_key, and sms.twilio_auth_token (×2, including after a server restart).

Root cause (located)

packages/platform-objects/src/system/sys-setting.object.ts:174 declares:

value_enc: Field.text({ readonly: true })

The engine strips readonly fields from a non-system caller's UPDATE payload (stripReadonlyFields, packages/objectql/src/engine.ts:8504 and :8651), so SettingsService.upsertRow's
engine.update(objectName, { where, data: { ...row } }) (packages/services/service-settings/src/settings-service.ts:1857)
can never repoint the handle.

The INSERT path is unaffected — which is exactly why the first write of a secret is correct and every later one is not.

Not a regression of PR #7554

• The readonly: true flag dates to 2026-08-07 (b3c1f3cd); the redaction landed 2026-08-11 (23bc6e16).
• The redaction only drops keys whose submitted value equals the mask. In this reproduction the PUT carries a genuinely new value, and the response echoes the key rather than dropping it — so the mask-drop path is visibly not involved.

Acceptance criteria

• A second (and third) PUT of a new value for an encrypted key repoints sys_setting.value_enc so the effective secret is the new one; a read-after-write through the service resolves the new plaintext.
• The mask-echo no-op is preserved: PUT with •••••••• still leaves the stored ciphertext byte-identical (this currently works and must not regress).
• No orphan sys_secret rows are left behind by a rotation — or, if orphans are acceptable, they are reaped.
• The first-write path is unchanged.

What is verified working (scoping this card)

The #7522 redaction fix itself is sound and should not be touched:

• Both flavours read back as •••••••• — type:'password' (mail.smtp_password) and encrypted:true (mail.api_key) — with every cascadeChain entry masked rather than stripped.
• Whole-body literal scan across all 10 namespace bodies and GET /api/settings → zero plaintext hits.
• Unset secrets still serve as a present key with value: null (not omitted — QA checklist: platform-core.settings-hub-roundtrip's secret clause says what must NOT be returned but never what IS — it cannot distinguish masked from omitted #7573 shape holds).
• Non-secret keys in the same body still carry real values; a non-encrypted key whose value genuinely is •••••••• is stored and served verbatim.
• Env-locked secrets still report source:'env', locked:true, lockedReason, and still refuse PUT with 409 SETTINGS_LOCKED.
• Route pins green: settings-routes.test.ts + settings-secret-redaction.test.ts → 29/29.

Source

Retest of platform-core.settings-hub-roundtrip clause 10 at b602d536. Storage-layer facts were read by decrypting sys_secret out-of-band with the dev data key, not inferred from the API.

[hotlong]

Triage: queued pm:queue + priority:p0; stays domain:services (fix surface is packages/services/service-settings upsert path plus the sys_setting.value_enc readonly declaration in packages/platform-objects — the mechanism is stripReadonlyFields on the service's non-system engine.update).

p0 rationale (binary, not a gradient): the security-recovery action itself silently fails — a leaked credential survives a rotation that answers 200 with every visible signal (redacted echo, advanced updated_at, new sys_secret row) saying it landed. First write is correct, so it is invisible until the second; 4× repro across three namespaces; root cause located to file:line.

Scope guards, kept as acceptance: the #7522/PR #7554 redaction behaviour and the mask-echo no-op must not regress; orphan sys_secret rows must be either repointed-and-reaped or explicitly accepted; first-write path unchanged.

Adjacent, ⛔ do not fold in: #8026 (NoopCryptoAdapter fail-open) is the other end of the same settings-encryption path, held as a finding on direction rather than impact.

---

Generated by Claude Code

[hotlong]

Triage addendum: target:v17 added. Binary test: an RC whose settings hub answers 200 to a credential rotation that did not happen is a release-notes-apology defect on a published surface (classes ① and ④).

---

Generated by Claude Code

[huangyiirene]

Claim: PM loop round 8 — P0 插队,维护者直接指示
Session: session_01JVfWSQN9RDDGwdVSmMc29x
Branch: claude/issue-8030-settings-rotation-repoint-handle
Worktree: cloud session's own clone (mode:cloud)
Domain: domain:services
File surface: packages/services/service-settings/src/settings-service.ts(upsertRow 的 engine.update 调用,~:1857)+ 其测试。⚠️ packages/platform-objects/src/system/sys-setting.object.ts:174 与 packages/objectql/src/engine.ts(stripReadonlyFields,:8504/:8651)是 READ 面 —— 见下面的 STOP 分叉。
Container & model: M(根因已定位,但修法形状有跨席分叉 + 存量孤儿行处置),mode:cloud,model: opus

插队出处(代执行指令必须带出处,#6668 先例):维护者 2026-08-12 在本 PM 会话中的指令,原话逐字、未翻译:

8030 插队

据此超出本轮批次上限、打破轮次节奏立即派发。P0 插队豁免的只有排队:⛔ 不豁免同文件/同包串行、⛔ 不豁免认领协议、⛔ 不豁免门禁。三条逐一核过,见下。

本卡在本席上一枪(#6015 评论 5267012820)里是「已标记为紧急、但本席未碰,交分诊定级」的那一张 —— 当时它是裸 bug,本席明说 act-immediately 的依据不适用于它。它此后已被分诊定级:priority:p0 + security + pm:queue + target:v17(发版阻塞)。⇒ 现在它既有定级、又有维护者直派,两条路径都通。

串行约束逐条核过

• 在飞 Regression from #7799: for ~60s after every restart, a webhook holding an encrypted signing secret silently drops its subscription — no delivery, no sys_http_delivery row, while it still reads active:true #8022 占 plugin-webhooks/src/auto-enqueuer.ts 及该插件 boot 接线 —— 不同包,不相交 ✅
• 在飞 finding: a no-locale sendTemplate resolves the en-US row but renders format filters under the runtime locale — renderOpts.locale stays unset #7801 占 plugin-email/src/email-service.ts —— 不同包,不相交 ✅
• 已合 service-settings has no typecheck script — turbo silently no-ops it, and ~5 test files carry pre-existing type errors behind the unwired gate #7925(PR fix(service-settings): wire the typecheck script and clear the 14 errors the unexecuted gate hid #7959)动过 service-settings 的 package.json 与五个测试文件,未动 settings-service.ts 本体;已落地,无在飞冲突 ✅
• 队列里的 [security] webhook custom headers are still cleartext in two JSON blobs — the sibling of #7799 that PR #7901 did not close #7986 / examples/app-todo: is_completed and is_overdue are readonly flags that nothing ever maintains — permanently false, and one of them is read by a hook #7226 未派,不构成约束

⚠️ #5536 ride-along —— 本次真的触发了

本席常设「派发前必查」的三个缝文件之一就是 service-settings/src/settings-service.ts:1200。本卡的落点在同一文件(~:1857),按车道规则 ride-along 已欠,派发令必带。⛔ 仍不单点修 storage hasAny;⛔ 仍不借此晋级 #5536。

(对照:round 6 的 #7925 声明面只碰 settings-service.**test**.ts,ride-along 核过、未触发 —— 那是条件式 ride-along 的正确结局;这一张是它真正触发的样子。)

根因已由报告定位(前提,dev 须复核)

sys-setting.object.ts:174 把 value_enc 声明为 Field.text({ readonly: true });引擎对非 system 调用方的 UPDATE payload 剥掉 readonly 字段(engine.ts:8504 / :8651),而 upsertRow 走的正是 engine.update(...) —— 于是句柄永远重指不了。INSERT 路径不受影响,所以第一次写正确、之后每一次都错。

⚠️ STOP 分叉(跨席)

PM 假说(须实测,鼓励证伪):正解在 service 侧 —— SettingsService 是特权写入者,应以 system context 执行这次 update,让 readonly 剥离不适用;value_enc 保持 readonly 对外部调用方是正确且必须保留的(它防的是外部直接改句柄)。

⛔ 若实测表明必须改 sys-setting.object.ts 的字段声明(落 platform-objects ⇒ domain:metadata #6367)或改 stripReadonlyFields 的语义(落 packages/objectql ⇒ domain:engine-core #6019)—— STOP 并上报,由本席按跨席声明处理,⛔ dev 不自行跨席落地。摘掉 readonly 让任何调用方都能重指句柄,方向上就是错的,除非有更强的实测理由。

---

Generated by Claude Code

[huangyiirene]

PR #8063 (draft) · branch claude/issue-8030-settings-rotation-repoint-handle · commit db96543c

Reporting at draft-PR time as instructed; CI is still queueing. One ratchet reading is still measuring (check:type-check-debt needs the full workspace closure built) — I will post it as a follow-up.

§3 root cause — held, verified end-to-end, not assumed

sys-setting.object.ts:174 value_enc: Field.text({ readonly: true }) + stripReadonlyFields gated on if (!opCtx.context?.isSystem) (engine.ts:8502, and the updateMany twin at :8649) + upsertRow's un-elevated engine.update. INSERT deliberately exempt (#3413), which is why write 1 is correct and every later one is not.

Confirmed against a real engine rather than from reading — the engine says it itself:

Field 'value_enc' on 'sys_setting' is read-only: the caller-supplied value was DROPPED and the update is being COMMITTED WITHOUT IT — the call returns success while this column keeps its stored value (#2948). Server-side code that legitimately writes read-only columns … must declare itself trusted by passing { context: { isSystem: true } } on the write.

One addition to the filer's account: updated_by is readonly: true on the same object, so it was being stripped on every update too — after the first write, sys_setting.updated_by never changed again. Same root cause, same fix; worth knowing because it means the audit face of the row was also stale, not just the handle. (updated_at was advancing because the driver stamps its own; the service's value was stripped alongside the others.)

§5 STOP fork — did not fire

The PM hypothesis measured correct. The fix is entirely service-side; nothing in packages/platform-objects or packages/objectql is touched.

• upsertRow performs its UPDATE in a system context. SettingsService is a privileged writer — assertPermitted, the env-lock/upper-scope-lock pre-flight and validatePatch have all run before persistence, and value_enc/updated_by are columns it owns rather than ones a caller forged. Precedent cited in the code: ObjectQL.recomputeSummaries' elevation (crud-permission-matrix: an allowed member create of showcase_task returns 500 while the row is written (summary recompute runs under caller context) #7673), which is the same shape one layer up — a platform-owned read-only column silently never landing because the platform's own write looked like a caller's.
• SettingsEngine.update gains a context member; wrapEngineAsSettingsEngine forwards it on both branches. In practice the settings row write takes the multi branch (its where is the composite key, never an id), so the by-id branch is the half that would rot unnoticed — it has its own pin.
• ⛔ readonly: true is preserved. There is a dedicated case asserting that a non-system caller reaching sys_setting directly still cannot repoint a secret handle. If someone later "fixes" this by deleting the flag, that case goes red.

§7.3 orphan rows — reaped

Reasoning is security, not tidiness: an orphan is a decryptable copy of exactly the credential the admin just retired, sitting in sys_secret under the same data key, accumulating one row per rotation (your 7 → 8 → 9). Leaving them makes the exposure grow with the hygiene we ask operators to practise, which inverts the point of rotating.

Safe to delete: handle ids are minted per encrypt() call, sys_setting.value_enc is the only column that holds one, and the audit trail records digests (sha256:…) rather than handles, so it stays readable after the ciphertext is gone.

Two deliberate softenings: SettingsSecretStore.delete is optional (a store that cannot delete — and every pre-existing test double — keeps working, accepting the orphans), and the call is best-effort and ordered after the repoint. By the time it runs the new secret is already in force, which is the property that matters; a throw is logged loudly (…could not be deleted from sys_secret — the rotation itself SUCCEEDED…) and never raised. Both are pinned.

Reset-to-null reaps too — clearing a secret should not leave the cleared value decryptable.

§6 #5536 ride-along — considered, declined, with reason

My landing site is settings-service.ts, so the obligation triggered. It does not fit, on three counts:

1. Wrong path. The fix lands on the WRITE path (upsertRow ~:1857, setMany ~:1340). settings 消费缝丢弃 ResolvedSettingValue.source —— 服务无法区分「管理员写过的值」和「schema 默认值」 #5536 is about the CONSUMPTION seam. They share a file and nothing else — no line I touched reads or drops source.
2. The one consumption line in this file cannot carry it. snapshotOf (raw[k] = v.value) builds the Record<key, value> map that createClient({ schema }) hands to a consumer's Zod parse. Injecting source changes the parsed shape for every createClient consumer — that is a breaking change to a spec-declared client contract, and shipping it as a rider on a P0 security card would be exactly the wrong way to make it.
3. The measured victims are in other packages. service-storage:397, service-sms:157 and plugin-email (via createClient) read getNamespace() directly, out of this lane. Fixing them needs the platform-level ruling OS_STORAGE_ROOT 对任何非默认值不生效:CLI 与设置服务的 env 通道错位,schema 默认值覆盖宿主构造配置(原报「swap 假警告」归因已被 #4096 时间线推翻,警告本身是准确的) #4968 asks for (retain source vs. derive authored: boolean), not a local patch.

I also considered adding a bare sourcesOf(namespace, ctx) accessor here as the "enabling half". Rejected: with no consumer in this PR it is declared-but-not-delivered surface (PD #10) and it would let the real decision be deferred behind an API that looks like it was already made.

⛔ Did not single-point-fix storage's hasAny; ⛔ did not promote #5536.

§7.5 reverse-verification

Method: in one worktree, with the test and every seam change identical, I removed only the fix's two behavioural lines (the context: SETTINGS_SYSTEM_WRITE_CONTEXT on the update, and the reap call), ran, then restored. That isolates causality more tightly than a plain origin/main run, which would have failed at import time on the seam the test needs rather than on the defect.

Red (fix removed): 6 failed / 6 passed — failing exactly where you reported:

× a SECOND and THIRD write of a new value take effect
 AssertionError: expected 'sec_d261eae843e18ebc94d0dc28dfef3fd2'
 not to be 'sec_d261eae843e18ebc94d0dc28dfef3fd2'
× resetting to null clears the handle … expected 'sec_05c5f66…' to be null
× three rotations leave exactly one row … expected [ {…}, {…} ] to have a length of 1 but got 2
× a store with no delete keeps working … expected 'alpha' to be 'beta'

The 6 that stayed green with the fix removed are the ones that should: the first-write path, the readonly-for-external-callers pin, the mask-echo no-op, and the adapter shape. That asymmetry is the evidence the test is measuring the defect and not the diff.

Green (fix applied): 12 passed.

§7.2 / §7.4 — mask-echo and first write

• Mask-echo no-op: pinned by driving the route's own helper — dropEchoedSecretMasks({ twilio_auth_token: '••••••••', … }) drops the key, so setMany never writes, and the case asserts the stored value_enc and the raw ciphertext strings are identical before/after, with the non-secret key in the same body still landing. Green both before and after the fix (it is not on the changed path).
• First write: the INSERT branch is untouched — the elevation is on the UPDATE branch only. Pinned as "one write → one row, one handle, one decryptable sys_secret row", plus a non-encrypted key that still round-trips with zero sys_secret rows. Green with the fix removed, which is the proof it is not on the changed path.

Why the tests use a real engine

The defect is the engine's strip rule meeting the platform's field declaration, so a hand-written fake modelling "drop read-only keys" would only prove the fake matches the fix. The file boots a real ObjectQL over the real SysSetting/SysSecret schemas, through the real IDataEngine adapter, into the real service.

§8 ratchet readings

• check-test-source-alias: OK — "72 packages with tests scanned; 63 registered…". It went red first (NEW unaliased artifact import(s) since this entry was measured: @objectstack/core — my test is the first in this package to reach settings-service-plugin.ts, which pulls resolveAuthzContext from core). Fixed the sanctioned way: a new packages/services/service-settings/vitest.config.ts aliasing @objectstack/objectql and @objectstack/core to source, anchored-regex array form. ⛔ The registry was not widened — service-settings' entry is byte-identical (platform-objects, spec, types). Aliasing those three as well would be a legitimate shrink but is a separate card, not a rider on this one.
• check:type-check-debt: self-test green ("23 semantic + 24 observation + 15 re-measure + 12 built-closure + 9 auto-lowering cases hold"); the --re-measure half needs the whole workspace dist closure built and is running now — I will post the reading. ⛔ Nothing was added to the ledger; service-settings stays out (tsc --noEmit in the package: 0 errors, and its tsconfig include: ["src"] covers the new test file).
• check:engine-double-contract: OK — 166 pinned, 133 ledgered, 2 exempt, unchanged. The new fake in the adapter cases is not an engine stand-in (it asserts the argument shape the adapter produces, and never routes a dispatch decision), and the gate did not pick it up.
• ADR-0112: no refusal was added or changed. The 409 SETTINGS_LOCKED path is untouched — the env-lock pre-flight runs in setMany well before persistence.
• packages/spec/src/**: not touched, so no gen:schema / gen:docs.
• eslint on all touched files: clean. service-settings suite: 413 passed / 20 files (was 401 / 19).

Deliberately not done

• SettingsService falls back to a NoopCryptoAdapter whose encrypt() is base64 — fail-open where the engine's Field.secret() path fails closed #8026 (NoopCryptoAdapter fail-open) — not folded in, per the triage note.
• A backfill for existing orphans. Reaping is forward-only: rows orphaned by rotations that already happened on a deployed instance stay. Doing it properly needs a migration that walks sys_secret against live value_enc values, and a P0 hotfix is the wrong vehicle for a destructive sweep over a secrets table. Worth its own card if you want it — say the word and I will file it.
• Aliasing the remaining three deps in the new vitest.config.ts (would shrink the check-test-source-alias registry entry). Mechanically easy, but it changes module resolution for all 20 test files in the package and belongs on its own card.

---

Generated by Claude Code