Skip to content

[Decision] sys_secret orphan reaping: which vehicle carries a destructive sweep over stored credentials — migration, admin command, or opt-in script? #8259

Description

@os-zhuang

Filed by the domain:services PM 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 — ⚠️ DISCHARGED. PR #8261 merged 2026-08-13 02:2xZ; the measurements this card was waiting on are in and reviewed. See "Recommendation" below, which the measurements changed.

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_secret row 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 pnpm script / 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.

⚠️ Recommendation — A (amended 2026-08-13 02:26Z by measurement; superseded the original B)

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_enc is not the only column holding a sys_secret handle. There are three producers:

  1. SettingsService → handle in sys_setting.value_enc
  2. the engine's secret-field channel (packages/objectql/src/engine.ts:4737, three write paths) → a secret: + id ref on any business row, including tenant-authored objects
  3. the datasource credential binder → a sys_secret: + id ref at external.credentialsRef

Two are invisible from service-settings, and (2) is not statically enumerable — it is every secret-typed field on every registered object, instance-specific and not precomputable. ⇒ A sweep keyed on sys_setting alone would delete live datasource credentials and live business-row secrets.

Three further facts that hardened A:

  • orphaned is a heuristic verdict, not a decision procedure — sys_secret carries no producer column and (namespace, key) means something different per producer. 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 exposure framing is inverted for this population: the value still 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, which is most of the urgency argument for doing it at all.

Four-edge card face

  • Platform long-term coherence — A costs no permanent obligation. B adds one operator-initiated maintenance action. C adds a startup-time destructive stage every future deployment inherits and nobody can opt out of.
  • Measured business pull — Real but weakened by measurement: the orphans exist, but the exposure a sweep would close is not the credential the admin believed retired. ⛔ Zero complaints is not zero pull for a silent-by-construction exposure — but the remedy no longer matches the headline risk.
  • AI-agent error-resistance — B is loudly reversible-by-omission: default is a report, destruction needs an explicit flag, so an agent that runs it without reading gets a report. C makes the destructive path the default and the silent one.
  • Startup scope discipline — A is the smallest thing that could work. C costs a migration that must stay correct forever against every future sys_secret shape, on pain of destroying credentials.

Not this card


⚠️ Authoring note: an earlier revision of this body wrote the two ref shapes above with angle brackets around the id placeholder. The sanitizer silently stripped them as HTML tags, leaving secret: and sys_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.

Activity

  1. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    ⚠️ My recommendation above is amended by measurement — the condition I attached to it has fired

    Status 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() call GREEN
    sys_setting.value_enc is the only column holding a handle RED — FALSIFIED
    Audit trail records digests, not handles GREEN

    sys_secret reportedly has three producer paths, not one — and says so in its own schema (platform-objects/src/system/sys-secret.object.ts):

    1. SettingsService → handle in sys_setting.value_enc
    2. the engine's secret-field channel → secret: id on an arbitrary business row, including tenant-authored objects
    3. the datasource credential binder → sys_secret: id at external.credentialsRef

    Two are invisible from service-settings, and (2) is not statically enumerable — it is every secret-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

  2. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    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_enc is the only column holding a handle ❌ falsified
    audit records digests, not handles ✅ re-established

    sys_secret has three producers, as its own schema states: SettingsService; the engine's secret-field channel, which stores secret: id on any business row including tenant-authored objects; and the datasource credential binder, which stores sys_secret: id at external.credentialsRef. There is no producer column on sys_secret, and namespace/key mean different things per producer. So a sweep keyed on sys_setting alone 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:

    1. 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.

    2. 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, never orphaned.


    Generated by Claude Code

  3. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Blocker discharged — this card is now decidable

    Blocked-by: #8103 in 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/main rather than taking the dev's word: the engine's secret-field channel (packages/objectql/src/engine.ts:4737, invoked from three write paths) and the datasource binder (sys_secret://… at external.credentialsRef in service-datasource) both exist and are both invisible from service-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-settings and, for the engine's channel, is instance-specific (every secret-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:

    1. orphaned is a heuristic verdict, not a decision procedure — no producer column on sys_secret, and (namespace, key) means something different per producer. So even the bucket a sweep would target is not provably safe to delete.
    2. 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:blocked against this card and will stay quiet until you rule. No nagging from this seat.


    Generated by Claude Code

  4. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    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 --delete flag 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):

    Label swap: needs-user-decision → pm:blocked (Blocked-by: #8103 already in the body) + domain:services.


    Generated by Claude Code

  5. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    ⚠️ The ruling's own condition had already fired when the ruling was written — literal execution selects A, not B

    Reading 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_enc is not the only column holding a sys_secret handle. Three producers exist — SettingsService; the engine's secret-field channel (packages/objectql/src/engine.ts:4737, three write paths, storing secret:<id> on any business row including tenant-authored objects); and the datasource binder (sys_secret://… at external.credentialsRef). Two are invisible from service-settings and the engine's holder set is not statically enumerable.

    I verified that independently on origin/main rather 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:

    1. 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.
    2. 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

  6. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    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: #8103 was 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-decision is 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 goes pm:blocked against 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

  7. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    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_enc is the only column holding a handle) was falsified and reviewer-confirmed nine hours before that ruling was recorded — three producers exist, two invisible from service-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; orphaned is 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's Blocked-by: #8103 was spent at 02:26Z and is discharged, not carried. Downstream: #8103 moves to pm: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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions