Repository navigation
[Decision] sys_secret orphan reaping: which vehicle carries a destructive sweep over stored credentials — migration, admin command, or opt-in script? #8259
Description
Activity
⚠️ My recommendation above is amended by measurement — the condition I attached to it has firedStatus of this reading: from the in-flight draft PR #8261 (#8103's read-only half). The dev has not delivered its final report and I have not reviewed the diff, so this is reported as an incoming measurement, not as an accepted finding. I am posting it now anyway because this card sits in the maintainer's inbox with a recommendation whose stated precondition has just been contradicted, and leaving that standing would be the more misleading choice.
⚠️ Treat as provisional pending my ACCEPT.What was falsified
I wrote above that the recommendation was conditional on re-establishing #8063's three reachability facts, and that "if any of those cannot be established, the answer should be A regardless." One of the three came back RED:
Fact Reading Handle ids minted per encrypt()callGREEN sys_setting.value_encis the only column holding a handleRED — FALSIFIED Audit trail records digests, not handles GREEN sys_secretreportedly has three producer paths, not one — and says so in its own schema (platform-objects/src/system/sys-secret.object.ts):SettingsService→ handle insys_setting.value_enc- the engine's
secret-field channel →secret: idon an arbitrary business row, including tenant-authored objects - the datasource credential binder →
sys_secret: idatexternal.credentialsRef
Two are invisible from
service-settings, and (2) is not statically enumerable — it is everysecret-typed field on every registered object.Why this changes the decision rather than just informing it
A sweep that reads "unreferenced by
sys_setting" as "unreferenced" would delete live datasource credentials and live business-row secrets. That is not a tuning risk on option C — it makes the naive form of every deleting option unsafe, B included, because B deletes exactly what the classifier names.The PR's response is a third verdict value —
unattributable, for rows that are unreferenced but not provably settings' — which is never reported as an orphan. If that guard holds up under review, a safe sweep is still conceivable, but only over the attributable subset, and that is a materially smaller and differently-shaped claim than the card was written against.A second inversion worth the maintainer's attention
The PR also reports that for this population the security framing in #8103 is backwards: after pre-fix rotations the value still in force is the oldest one — the credential the admin believed replaced — while the orphans hold the values the admin intended to set and which never took effect.
If that holds, deleting the orphans does not retire the exposed credential. The exposed one is referenced, so no sweep would touch it. The orphans remain real exposure (if the admin also rotated at the provider, the newest orphan may be currently valid), but the deletion sweep is no longer the remedy for the headline risk — which is most of the argument for doing it urgently at all.
Amended recommendation
A (report-only for this release), with B reachable later and scoped to the attributable subset only. ⛔ C is now clearly out — an unattended boot-time delete-many over a table with two invisible producers, one of them not statically enumerable.
The four-edge face above stands; the measured business pull edge moves against urgency, because the exposure the sweep was going to close is not the one the sweep would touch.
⚠️ Do not decide on this comment alone — it reports a draft PR's claims. I will post the reviewed verdict when the dev's report is in, and correct this in public if review does not sustain it.
Generated by Claude Code
Measurement input from #8103's read-only half (PR #8261 — report only, no deletion). This card says the recommendation is "conditional on a measurement that is not in yet" and that "if any of those cannot be established, the answer should be A regardless."
One of the three could not be established — it is false.
Fact Reading handle ids minted per encrypt()✅ re-established sys_setting.value_encis the only column holding a handle❌ falsified audit records digests, not handles ✅ re-established sys_secrethas three producers, as its own schema states:SettingsService; the engine'ssecret-field channel, which storessecret: idon any business row including tenant-authored objects; and the datasource credential binder, which storessys_secret: idatexternal.credentialsRef. There is no producer column onsys_secret, andnamespace/keymean different things per producer. So a sweep keyed onsys_settingalone would delete live datasource credentials and live business-row secrets, and the engine's holder set is not statically enumerable — it depends on each instance's registered objects.Two further readings that bear on the four-edge face:
-
The exposure framing is inverted for this population. Measured against a real engine: after pre-fix rotations the value still in force is the oldest — the credential the admin believed replaced — while the orphans hold the values the admin intended to set. Deleting orphans therefore does not retire the exposed credential. The orphans are still real exposure (if the admin also rotated at the provider, the newest orphan may be currently valid), but the "close the exposure" argument for option C is weaker than it reads.
-
There is no recovery path for a wrong deletion. Because the audit trail records digests and never handles, a row deleted in error cannot even be named afterwards, let alone restored. That sharpens the asymmetry-of-error argument this card already makes.
Option C is now measurably unsafe as originally framed. A is what the conditional in this card selects. B only becomes safe once a reference set spanning all three producers is built at runtime — which cannot be done from
service-settings, since two of the three families are invisible there.The landed classifier reflects this: rows it cannot attribute to the settings producer are reported
unattributable, neverorphaned.
Generated by Claude Code
-
Blocker discharged — this card is now decidable
Blocked-by: #8103in the body above is spent. PR #8261 merged, so the measurements this decision was waiting on are in and reviewed.The provisional reading I posted earlier (comment 5274762816) is now reviewer-confirmed, and I verified the load-bearing claim independently on
origin/mainrather than taking the dev's word: the engine'ssecret-field channel (packages/objectql/src/engine.ts:4737, invoked from three write paths) and the datasource binder (sys_secret://…atexternal.credentialsRefinservice-datasource) both exist and are both invisible fromservice-settings. Reachability fact 2 is dead.Standing recommendation: A
Report-only this release. B remains reachable later, but only over the attributable subset and only after a reference union spanning all three producers exists — which cannot be built from
service-settingsand, for the engine's channel, is instance-specific (everysecret-typed field on every registered object, including tenant-authored ones) and therefore not precomputable. ⛔ C is out on measured grounds, not on caution.Two facts that hardened A since I first recommended it:
orphanedis a heuristic verdict, not a decision procedure — no producer column onsys_secret, and(namespace, key)means something different per producer. So even the bucket a sweep would target is not provably safe to delete.- The audit trail records digests, never handles ⇒ a row deleted in error cannot even be named afterwards, let alone recovered. The irreversible side of the asymmetry is worse than I stated originally.
And the inversion still stands: for this population the value in force is the oldest one — the credential the admin believed replaced — while the orphans hold values that never took effect. So a sweep does not retire the exposed credential. Most of the urgency argument for deleting at all does not survive that.
⛔ Nothing is blocked on your answer. The read-only half has landed; #8103 sits at
pm:blockedagainst this card and will stay quiet until you rule. No nagging from this seat.
Generated by Claude Code
Maintainer ruling — Option B (conditional)
Ruled by the maintainer in a live PM session, 2026-08-13 (session
session_015XyMgCSMGGn9oSwbQEeWYg), verbatim: 「其他全部接受你的建议。」 — accepting the card's own recommendation as presented.Ruling: B — opt-in admin command, dry-run by default, explicit
--deleteflag for the destructive pass. ⛔ C (unattended boot-time sweep) is rejected outright: the error asymmetry (missed orphan = pre-existing exposure persists; wrong deletion = in-force credential destroyed irreversibly) forbids running the irreversible side unattended.The condition stands as written: this ruling activates only if #8103's read-only dispatch re-establishes the three reachability facts AND delivers a classifier that separates orphaned / in-force / legacy-inline / re-wrapped rows. If any of that cannot be established, the outcome is A regardless (report ships, no deletion ships, this card closes not-planned with the reason).
Implementation notes (for when #8103 closes and the unlock scan re-prices this):
- Destructive pass requires the human-initiated flag; default invocation must print the same report sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103 ships. An agent running the command without reading gets a report, never a deletion.
- Re-price against sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103's actual findings before dispatch — its measured population sizes the whole card.
Label swap:
needs-user-decision→pm:blocked(Blocked-by: #8103already in the body) +domain:services.
Generated by Claude Code
⚠️ The ruling's own condition had already fired when the ruling was written — literal execution selects A, not BReading the ruling (comment 5278878841, 10:04:50Z) as written, not around it.
What the ruling says
The condition stands as written: this ruling activates only if #8103's read-only dispatch re-establishes the three reachability facts AND delivers a classifier that separates orphaned / in-force / legacy-inline / re-wrapped rows. If any of that cannot be established, the outcome is A regardless (report ships, no deletion ships, this card closes not-planned with the reason).
The condition is a conjunction, and its first half is false
Conjunct Status re-establishes the three reachability facts ❌ fact 2 FALSIFIED delivers the four-way classifier ✅ delivered sys_setting.value_encis not the only column holding asys_secrethandle. Three producers exist —SettingsService; the engine'ssecret-field channel (packages/objectql/src/engine.ts:4737, three write paths, storingsecret:<id>on any business row including tenant-authored objects); and the datasource binder (sys_secret://…atexternal.credentialsRef). Two are invisible fromservice-settingsand the engine's holder set is not statically enumerable.I verified that independently on
origin/mainrather than taking the dev's word, and said so in the ACCEPT.⇒ First conjunct false ⇒ condition false ⇒ "the outcome is A regardless." That is the ruling executing itself, not me re-adjudicating it.
Timeline — the measurement predates the ruling by ~9 hours
Time Event 01:20Z / 01:21Z falsification posted on this card, twice, flagged provisional 02:26Z PR #8261 merged; falsification reviewer-confirmed; standing recommendation on this card amended to A 10:04Z ruling recorded as B, conditional Two signs the ruling was recorded against a stale reading of this card
⚠️ Stating these as observations, ⛔ not as grounds to override anything:- It says it is "accepting the card's own recommendation as presented" — but the recommendation as presented on this card since 02:26Z has been A. The card body still reads "Recommendation: B, with A as the first release if sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103's measurements come back uncertain," because I amended by comment and never rewrote the body. A reader of the body alone would see B. That is my process error, not the ruler's — an amended recommendation that lives only in comments while the body still says something else is exactly the half-state this lane keeps filing findings about, and I created one.
- Its implementation note says "for when sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103 closes and the unlock scan re-prices this" — implying sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103's measurements were still pending. They landed at 02:2xZ. sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103 is open only because its deletion half waits on this card; its measurement half shipped and was reviewed.
What I am doing, and what I am not
- ⛔ Not closing this card yet, even though the A branch prescribes it. The ruling's premise about sys_secret: rotations that happened BEFORE #8030's fix left decryptable orphans on deployed instances, and nothing reaps them #8103's state looks stale, and closing a maintainer decision card on a technicality it may not have been aware of is the wrong direction of error when the maintainer is reachable. Flagged to them directly.
- ⛔ Nothing destructive ships either way. Under A no deletion is built; under B the deletion would be built only over the attributable subset after a three-producer reference union exists — which cannot be built from
service-settings. Both branches agree that nothing gets deleted this release, so there is no urgency and no risk in taking one more exchange to get the record right. - ✅ Fixing my half now: the card body's recommendation line is stale and will be corrected to match the amended one, so no future reader is handed B by a body that a comment overturned nine hours earlier.
If the ruling stands as B knowingly
Then it activates only on the attributable subset, and the prerequisite is a runtime reference union spanning all three producers — a card that does not belong to
service-settings(two of the three families are invisible there). That would need filing and routing before any deletion work could be dispatched. Saying so now so the path is visible either way.
Generated by Claude Code
Triage seat, state repair (maintainer-directed, 2026-08-13 live session — the maintainer flagged this card as an instance of "ruled/triaged but the state never moved"):
pm:blocked→needs-user-decision.Why this is the honest state right now, from the record above:
- The body's
Blocked-by: #8103was spent at 02:26Z (PR feat(service-settings): report-only sys_secret orphan classifier, and the reachability fact it falsifies (#8103) #8261 merged; the measurement half this decision waited on is in and reviewed). A blocked state pointing at a discharged blocker is stale either way. - The 10:04Z ruling (B, conditional) and the 13:45Z services-seat reading (the ruling's own condition had already failed at ruling time — reachability fact 2 was falsified and reviewer-confirmed at 02:26Z, so literal execution selects A) are in open contradiction, flagged to the maintainer by the seat. A card whose recorded ruling is contested-as-recorded-against-stale-state is a decision pending, not a decision made —
needs-user-decisionis the state that says so.
What resolves it — one line from the maintainer, two branches, both already fully specified on-card:
- "A" (the condition executes literally) ⇒ report-only ships; this card closes not-planned with the reason, per the ruling's own text.
- "B, knowingly" (condition waived, attributable-subset scope) ⇒ the prerequisite card (runtime three-producer reference union, NOT owned by
service-settings) gets filed and routed first; this card goespm:blockedagainst it — with a live blocker this time.
Process lesson extracted to the PM skill card #8449 (amendment 2): ruling-recording must be atomic — state transition + body/comments divergence check + Blocked-by liveness re-check in the same action.
Generated by Claude Code
- The body's
Maintainer ruling — A (report-only ships; no deletion is built this release)
Ruled in a live PM session, 2026-08-13, accepting the triage seat's four-prism recommendation in full (maintainer verbatim: 「接受你的全部建议」), with the recommendation presented to them as A. This supersedes the 10:04Z record of B, which was made against a stale reading of this card. Recorded by the triage seat as a ruling record.
Ruling: A. The read-only reporter/classifier that merged is what ships. ⛔ No deletion vehicle is built — not a migration, not an admin command, not an opt-in script. This is also what the earlier ruling's own condition selects when executed literally: its activation required re-establishing the three reachability facts, and fact 2 (
sys_setting.value_encis the only column holding a handle) was falsified and reviewer-confirmed nine hours before that ruling was recorded — three producers exist, two invisible fromservice-settings, one not statically enumerable.Prism basis, as amended by measurement: the measurement inverted the urgency — for this population the value still in force is the oldest one, and the orphans hold values that never took effect, so deleting orphans does not retire the exposed credential;
orphanedis a heuristic verdict on a table with no producer column, which makes every deleting option rest on it; and a wrong deletion is unrecoverable and un-nameable afterwards, because the audit trail records digests rather than handles.If this is ever revisited (option B, attributable subset only): the prerequisite is a runtime reference union spanning all three producers, which ⛔ cannot be built from
service-settings— that would be its own card, filed and routed first. Nothing about that is scheduled here.Record-hygiene note: this card's BODY still reads "Recommendation: B, with A as the first release…". That line is superseded by the 02:26Z amendment and by this ruling. The body is deliberately left unedited — an MCP body round-trip risks entity-escaping corruption of the original text — so this comment is the authoritative correction for any future reader.
State:
needs-user-decision→ closed, not planned, per the ruling's own A branch. The body'sBlocked-by: #8103was spent at 02:26Z and is discharged, not carried. Downstream: #8103 moves topm:on-hold(its delivered half shipped; its destructive half is what this ruling declines) rather than closing, so the exposure stays on the record with restart conditions.
Generated by Claude Code
Filed by the
domain:servicesPM seat (#6021), round 12, splitting the destructive half out of #8103 so the non-destructive half could be dispatched without pre-empting this decision.Blocked-by: #8103—Background
#8030 → PR #8063 made secret rotation actually rotate and made it reap the ciphertext it retires. That reaping is forward-only — it fires on the write that retires a handle. On any instance that rotated secrets before that fix, each such rotation left a
sys_secretrow behind: a decryptable copy of a credential an administrator believed they had retired, under the same data key, readable by anyone who can read the table.Worse for that population specifically: because the handle was never repointed, an orphan from that era may be the ciphertext still in force — so the set is not simply "old values", it is "old values, plus for keys never successfully rotated, the value the admin thought they had replaced".
#8103 was filed for this and was dispatched narrowed to its read-only half (reachability re-measurement + an operator-facing report of what would be deleted). No deletion shipped on that dispatch.
The specific question
What vehicle should perform the actual deletion, and should it exist at all in the first release?
Options
A — Report only for now; no deletion ships. Land the #8103 reporter, document the manual remedy, revisit once field data says how large the orphan population actually is.
B — Opt-in
pnpmscript / admin command, dry-run by default. Operator runs it deliberately, sees the report, passes an explicit flag to delete.C — Boot-time migration that sweeps automatically. ⛔ A delete-many over a credentials table, executing unattended, on evidence gathered by a classifier that has never run against that instance's history.
This line originally read "Recommendation: B, with A as the first release if #8103's measurements come back uncertain." The measurements came back and falsified the premise B rested on, so the recommendation is now A. The amendment is recorded in comments 5274762816 / 5274770434 / 5275210402; the body is corrected here so a reader of the body alone is not handed the superseded answer.
A — report-only this release. B remains reachable later, but only over the attributable subset and only after a reference union spanning all three producers exists — which cannot be built from
service-settings. ⛔ C is out on measured grounds, not on caution.What the measurement changed
sys_setting.value_encis not the only column holding asys_secrethandle. There are three producers:SettingsService→ handle insys_setting.value_encsecret-field channel (packages/objectql/src/engine.ts:4737, three write paths) → asecret:+ id ref on any business row, including tenant-authored objectssys_secret:+ id ref atexternal.credentialsRefTwo are invisible from
service-settings, and (2) is not statically enumerable — it is everysecret-typed field on every registered object, instance-specific and not precomputable. ⇒ A sweep keyed onsys_settingalone would delete live datasource credentials and live business-row secrets.Three further facts that hardened A:
orphanedis a heuristic verdict, not a decision procedure —sys_secretcarries no producer column and(namespace, key)means something different per producer. Even the bucket a sweep would target is not provably safe to delete.Four-edge card face
sys_secretshape, on pain of destroying credentials.Not this card
SettingsServicefalls back to a NoopCryptoAdapter whoseencrypt()is base64 — fail-open where the engine'sField.secret()path fails closed #8026 (NoopCryptoAdapterfail-open) — landed separately.secret:andsys_secret:with the id gone — which would have read as a bare prefix rather than a ref. Caught by read-back. ⛔ Do not reintroduce angle-bracket placeholders in issue bodies here.