reapRotatedSecret deletes the IN-FORCE ciphertext when a SettingsEngine adapter drops `context`, leaving a dangling value_enc and an unreadable setting

[os-zhuang]

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.

[os-zhuang]

domain:services seat (#6021) — context for grading. ⛔ Not graded by me, and no domain:* 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 context left 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 dangling value_enc and 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:

1. It is latent, not live. The shipped adapter forwards context: { isSystem: true }, so no shipped path reaches it today. Same hazard-by-construction shape as SettingsService falls back to a NoopCryptoAdapter whose encrypt() is base64 — fail-open where the engine's Field.secret() path fails closed #8026 — real, but not an incident.
2. The trigger is documented as a supported extension point. SettingsEngine's own doc comment contemplates exactly this reader ("⛔ An adapter over IDataEngine MUST 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.
3. The doc warning is now factually stale, and that is arguably the cheapest half to fix: it still describes only the old consequence ("the rotated-away credential stays in force"). An adapter author reading it today will not learn that the failure is now data loss. Correcting that sentence costs nothing and is independent of whichever direction the code fix takes.

The suggested direction on the card (verify rather than infer — re-read and delete previousEnc only when value_enc really is nextEnc) 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 }) in sys-secret-orphan-report.test.ts on PR #8261 is the faithful pre-fix instance; flipping withDelete back on reproduces the 1 → 1 → 2 table. Whoever takes this does not have to build the rig.

⚠️ Routing note, not a routing decision: the defect is in 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]

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:blocked on #8103: the repro harness lives on its PR #8261 (in flight, same package — boot({ forwardContext: false }) in sys-secret-orphan-report.test.ts). On unblock, re-verify: (a) the harness file survived to main as 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 in settings-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, delete previousEnc only when value_enc really is nextEnc) is a suggestion, not a ruling.

---

Generated by Claude Code