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.

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