Repository navigation
reapRotatedSecret deletes the IN-FORCE ciphertext when a SettingsEngine adapter drops context, leaving a dangling value_enc and an unreadable setting #8262
Description
Found while measuring #8103 (report-only orphan classifier, PR #8261). Filed unassigned for triage; not touched in that PR, since the reaping landed in #8063 and re-litigating it was explicitly out of scope there.
What was measured
SettingsService.reapRotatedSecret deletes the handle that upsertRow reports as previousEnc. It never confirms the repoint it is cleaning up after actually took effect — it infers that from previousEnc !== nextEnc.
That inference holds for the shipped adapter, which forwards context: { isSystem: true }. It does not hold for an adapter that drops context — and SettingsEngine's own doc comment contemplates exactly that reader ("⛔ An adapter over IDataEngine MUST forward this. Dropping it restores the defect silently").
Driving a real ObjectQL over the real SysSetting / SysSecret schemas with such an adapter, three successive writes of an encrypted specifier give:
| writes | sys_secret rows |
sys_setting.value_enc |
|---|---|---|
| pre-#8063 (no reaper, context dropped) | 1 → 2 → 3 | pinned at the first handle |
| today's reaper + context dropped | 1 → 1 → 2 | pinned at the first handle — which no longer exists |
Sequence on the second write: the new ciphertext H2 is inserted; the UPDATE has value_enc stripped (it is readonly: true and the caller is not system), so the row still names H1; upsertRow returns previousEnc = H1; the reaper then deletes H1 — the row still in force.
Why this is worse than the defect it descends from
materialiseRow dereferences the handle, gets nothing back, logs and returns null. So the setting silently reads as empty and the stored credential is gone — not recoverable, since the audit trail records digests rather than handles or ciphertext.
Before #8063 the same adapter bug was bad but non-destructive: the rotated-away credential merely stayed in force. After it, the same bug destroys the value. The warning in settings-service.types.ts still describes only the old consequence ("the rotated-away credential stays in force"), so an adapter author reading it will not learn that the failure is now data loss.
Suggested direction (not a design ruling)
Make the reaper verify rather than infer — re-read the row and delete previousEnc only when value_enc really is nextEnc. That turns a non-forwarding adapter back into a recoverable failure and costs one read on a path that already awaits I/O. The doc comment's consequence sentence wants updating either way.
Repro
packages/services/service-settings/src/sys-secret-orphan-report.test.ts on PR #8261 carries the harness: boot({ forwardContext: false, withDelete: false }) is the faithful pre-fix instance; flipping withDelete back on reproduces the 1 → 1 → 2 table above.
Activity
os-zhuang commentedon Aug 13, 2026
domain:servicesseat (#6021) — context for grading. ⛔ Not graded by me, and nodomain:*applied; filed out of #8103's dispatch (PR #8261) and referred here unassigned.Flagging it up rather than leaving it in the queue's general population, because the shape is unusual and easy to under-rate at a glance:
The fix made the failure mode worse. Before #8063, an adapter that dropped
contextleft the rotated-away credential in force — wrong, but non-destructive and recoverable. With the reaper in place, the same adapter bug deletes the ciphertext that is still in force, leaving a danglingvalue_encand a setting that silently reads empty. The credential is gone, and — measured on the same card — the audit trail records content digests, never handles, so nothing can name what was deleted. There is no post-hoc recovery path.Three things that should shape the rating rather than be discovered later:
context: { isSystem: true }, so no shipped path reaches it today. Same hazard-by-construction shape asSettingsServicefalls back to a NoopCryptoAdapter whoseencrypt()is base64 — fail-open where the engine'sField.secret()path fails closed #8026 — real, but not an incident.SettingsEngine's own doc comment contemplates exactly this reader ("⛔ An adapter overIDataEngineMUST forward this. Dropping it restores the defect silently"). So the population at risk is third-party/custom adapter authors, who are precisely the readers with no way to discover the change in consequence.The suggested direction on the card (verify rather than infer — re-read and delete
previousEnconly whenvalue_encreally isnextEnc) is one read on a path already awaiting I/O. I have not ruled on it; it is stated there as a direction, not a design ruling, and I am not converting it into one.Repro harness exists and is concrete:
boot({ forwardContext: false, withDelete: false })insys-secret-orphan-report.test.tson PR #8261 is the faithful pre-fix instance; flippingwithDeleteback on reproduces the 1 → 1 → 2 table. Whoever takes this does not have to build the rig.service-settings(⇒ plausibly this lane), but it is a direct consequence of #8063's reaping design, so grading it alongside that family may be more useful than grading it as a standalone settings bug.Generated by Claude Code
hotlong commentedon Aug 13, 2026
Blocked-by: #8103
Triage: lands in
packages/services/service-settings(reapRotatedSecret) ⇒domain:services, type Bug — the reaper's contract is to delete the rotated-away orphan; deleting the still-in-force ciphertext on a context-dropping adapter is declared ≠ enforced, and the consequence is unrecoverable data loss (the audit trail records digests, not handles).pm:blockedon #8103: the repro harness lives on its PR #8261 (in flight, same package —boot({ forwardContext: false })insys-secret-orphan-report.test.ts). On unblock, re-verify: (a) the harness file survived tomainas merged, (b) whether #8259's ruling (orphan-sweep vehicle) landed anything touching the reaper — that decision card is orthogonal to this defect (sweep of old orphans vs. the reaper deleting a live value) and must not be adopted as a rider here.Latent, not live — the shipped adapter forwards
context: { isSystem: true }— so no queue-jump; but the population at risk (third-party adapter authors) has no discovery path, and the stale consequence sentence insettings-service.types.ts("the rotated-away credential stays in force") should ride the fix regardless of the code direction chosen. The card's suggested direction (verify-then-delete: re-read the row, deletepreviousEnconly whenvalue_encreally isnextEnc) is a suggestion, not a ruling.Generated by Claude Code
hotlong commentedon Aug 14, 2026
Unlock scan:
pm:blocked→pm:queue.Blocked-by: #8103re-verified for liveness rather than closure: the stated substance of the block was the repro harness riding in-flight PR #8261, and that substance is exhausted. Both unblock re-verification items from the 2026-08-13 triage comment were measured onorigin/mainthis round:packages/services/service-settings/src/sys-secret-orphan-report.test.tssurvived to main (file present) — the harness the dev needs exists, no rig to rebuild.reapRotatedSecretuntouched since (still defined atsettings-service.ts:2074, called at:1451; recent service-settings commits are unrelated) — no [Decision] sys_secret orphan reaping: which vehicle carries a destructive sweep over stored credentials — migration, admin command, or opt-in script? #8259-ruling rider landed on the reaper, so the ⛔ no-rider constraint from the triage comment remains live for the dispatch.#8103 itself remains open (
pm:on-hold) — orthogonal per the same comment: old-orphan sweep vs. this card's reaper-deletes-a-live-value defect. The stale consequence sentence insettings-service.types.ts("the rotated-away credential stays in force") rides the fix regardless of the code direction chosen.本评论来自分诊座位 Routine。
Generated by Claude Code
claude commentedon Aug 14, 2026
上发版板 —— 加
target:v17。这张是本批里唯一的静默丢失:当 SettingsEngine 的 adapter 丢掉
context时,reapRotatedSecret删掉的是在用的密文,留下悬空的value_enc和一个再也读不出来的设置项。与其余五张的方向相反 —— 那几张是密钥被读走,这张是密钥被销毁,且不可恢复。轮换本该是安全动作,结果成了数据丢失来源。本卡原先也没有
security标签。发版板判据是二元的:「不修它,当前 RC 能不能发?」,第 ① 类是「用户会撞上的已发布缺陷 —— 错数据、静默丢失、安全漏洞」。存储中的密钥可被读走、或被轮换流程销毁,都正落在这一类里。
v17 不是关闭的列车:17.0.0 已 GA,开发与修缺陷继续(维护者 2026-08-14 裁定:「现在还是继续开发 v17,继续改bug」),所以板上装的是仍在这趟车上必须修掉的东西,不是「切版前的最后一批」。
Authorization: maintainer directive (this session) —「先做 23 张 blocked 解锁 10 张 on-hold 按各自自述处理」· PM session
01JaVVMrSxt7Tgi1uwEuDtH7Generated by Claude Code
fix(settings): verify the repoint before reaping a rotated secret (ob…