docs(site-secrets): final-review doc fixes — replication prerequisite, upgrade impact, audit shape

I2: topology guide + Component-SiteRuntime + register row 127 state that a site pair using
deployed secrets must run LocalDb replication. I3: row 127 + topology guide state the upgrade
impact (whole artifact deploy fails and reads Out of date until referenced secrets exist
centrally). M6: audit row carries secrets/missing, no removed field; removal-only deploys write
no central row. M2/M3/I1 as-built notes; CLAUDE.md secrets bullet corrected.
This commit is contained in:
Joseph Doherty
2026-09-28 21:08:34 -04:00
parent 67a3950f74
commit d641186397
6 changed files with 63 additions and 10 deletions
@@ -390,7 +390,22 @@ out below.
deploymentId, secrets: [{name, revision}], missing: [name], success }`, which
structurally excludes ciphertext. Written on both `DeployCoreAsync` and
`RetryForSiteAsync`, including the fail-closed path (audited even though
nothing was sent).
nothing was sent). The row carries `secrets` and `missing` only — there is
**no `removed` field** (design §3.7 named one), and a deploy that only
removes secrets (the site's last reference dropped, an empty list shipped)
writes **no central row**; removals are logged on the site by
`DeployedSecretsMirror`.
- **Secret rotation and live connections.** The site writes and mirrors the
shipped secrets before it pushes data connections to the DCL, so a
connection created by the same deploy resolves a new secret on first
connect; a rotation with unchanged connection config takes effect on that
connection's next (re)connect — see Component-SiteRuntime.md → *Deployed
Secrets*.
- **One `secret:` parser.** `SecretReferenceScanner.TryParseSecretReference`
(trimmed, case-insensitive prefix, trimmed name; `${secret:…}` never
matches) is shared by the scanner and every adapter that resolves the
reference (today `MxGatewayDataConnection`'s `ApiKey`), so the name central
ships is exactly the name the site looks up.
- **Central store.** Central resolves secrets from `ISecretStore`, composed by
the Host per `Secrets:Store:Provider` — see Component-Host.md → *Secrets*.
- **Site apply and the per-node mirror** — see Component-SiteRuntime.md →
+4 -2
View File
@@ -166,6 +166,7 @@ flowchart TD
- `HandleDeployArtifacts` additionally accepts the command's `Secrets` list (`SecretArtifact`, ciphertext only) and writes the full shipped set to the `deployed_secrets` table via `SiteStorageService.UpsertDeployedSecretsAsync` — upsert every shipped name, soft-remove (`is_removed = 1`) any stored name absent from the set. A `null` `Secrets` field (pre-feature central) touches nothing and answers with no `SecretsApplied`. A current central always ships a list — **empty** when the site references no secrets — so dropping a site's last `secret:` reference removes the secret from both nodes. Storage detail: *Deployed Secrets (Site SQLite / LocalDb)* below.
- After writing the table, the active node runs `DeployedSecretsMirror.ReconcileAsync` **synchronously**, before replying — so the deploy response's `SecretsApplied` count and any apply/verify error reflect this deploy's own outcome, not a value left over from the mirror's background schedule. `SecretsApplied` counts only the names *this deploy shipped* that applied and verified cleanly; a mirror failure for an unrelated, already-broken name (a different deploy's leftover, or a stale removal) is logged at Warning and excluded — it neither fails this deploy nor reduces its `SecretsApplied`.
- No `DeployedSecretsMirror` registered (secrets deployment not composed on this node) with a non-empty `Secrets` list fails the deploy with `Secret deployment is not enabled on this site node`; with an empty `Secrets` list it still succeeds (`SecretsApplied = 0`).
- **Ordering against the DCL.** The shipped secrets are written and mirrored **before** the deploy's data-connection definitions are pushed to the DCL (the definitions are still stored in SQLite at the same point, and the push still goes out if the secrets step fails), so a connection created or reconfigured by the same deploy resolves a newly shipped secret on its first connect. A rotation with **unchanged** connection config does not recreate the connection (the DCL push is hash-guarded) — the new value takes effect on that connection's next (re)connect. No forced reconnect is performed.
- See *Deployed Secrets (Site SQLite / LocalDb)* below for the mirror itself.
### Instance Lifecycle Commands
@@ -439,9 +440,10 @@ Site secrets ride the artifact deploy (design `docs/plans/2026-09-28-site-secret
**`DeployedSecretsMirror`** (`SiteRuntime/Secrets/DeployedSecretsMirror.cs`) is a per-node `BackgroundService` — registered on **every** site node, not a singleton — that reconciles `deployed_secrets` into the node's own local `ISecretStore` (site nodes never run a replication-decorated store; see Component-Host.md → *Secrets*):
- **Schedule**: runs once immediately at start, then every `SiteRuntime:DeployedSecretsMirrorInterval` (default 15s, validated `> 0`), and synchronously on demand right after an active-node artifact apply (see *Deployed Secrets* under Deployment Manager Singleton above). This 15s interval is the **standby node's own convergence path** — it never receives the active node's synchronous call, only the replicated `deployed_secrets` rows via LocalDb, so it picks them up on its own next tick.
- **Schedule**: runs once immediately at start, then every `SiteRuntime:DeployedSecretsMirrorInterval` (default 15s, validated `> 0`), and synchronously on demand right after an active-node artifact apply (see *Deployed Secrets* under Deployment Manager Singleton above). This 15s interval is the **standby node's own convergence path** — it never receives the active node's synchronous call, only the replicated `deployed_secrets` rows via LocalDb, so it picks them up on its own next tick. **A pair that uses deployed secrets must therefore run LocalDb replication:** on an unreplicated pair only the node that was active at deploy time holds the secrets, and after a failover the new active node has none until the next *Deploy Artifacts* (topology guide → *Secrets: shared store, per-site deploy, and the KEK*).
- **Apply is confirm-then-ledger, not blind.** `ApplyReplicatedAsync` is strict last-writer-wins on `(UpdatedUtc, Revision)` (a tie is a no-op) and `DeleteAsync` tombstones rather than hard-deletes — so the mirror always reads the row back after applying and only ledgers it in `mirrored_secrets` when the local ciphertext matches what was shipped. A tombstoned or clock-skewed local row that the ledger already owns is safely re-stamped (`UpdatedUtc`/`Revision` bumped past the local value) and re-applied; a live row with **different** ciphertext that the ledger does **not** already own is left completely untouched — not overwritten, not ledgered, not verified — and reported as an out-of-band conflict (`"exists on this node with a newer local value; the deployed revision was not applied"`), so an operator's genuinely local secret of the same name is never silently destroyed by a later central deploy or removal.
- **Ownership, not tombstone-state, decides re-stamping.** Once a name is ledgered in `mirrored_secrets` at any revision, this mirror treats that name as its own going forward — including re-stamping a mirror-owned name that drifted (e.g. an operator edited it locally) back to the deployed value on the next pass. Central is authoritative for every name it deploys.
- **Ownership, not tombstone-state, decides re-stamping.** Once a name is ledgered in `mirrored_secrets` at any revision, this mirror treats that name as its own going forward — including re-stamping a mirror-owned name that drifted back to the deployed value on the next pass. This holds even when the name is already ledgered at the shipped revision: the mirror reads the local row back every pass, and one that is missing, tombstoned (an operator deleted it locally) or live with different ciphertext (an operator edited it) is healed through the same bumped-stamp re-apply, read-back and cache invalidation before the verify — a local delete is repaired, never misreported as a master-key mismatch. The drift is logged at Warning once per occurrence. Central is authoritative for every name it deploys.
- **Unreadable rows are isolated.** A `deployed_secrets` row whose `row_json` cannot be parsed is reported as a failure for that name (in `Failed`/`FailedNames`) instead of aborting the pass; an unreadable **removed** row is still honoured by name.
- **Removal** deletes from the local store **only if the name was previously ledgered** (present in `mirrored_secrets`) — an out-of-band local secret sharing a deployed name that was never mirrored is never touched by a central-side removal.
- **Verify.** Every pass trial-resolves every live row through `ISecretResolver` (even one not re-applied this pass, so a later KEK change is still caught) — a decrypt failure names the secret and its `KekId` and is reported as a distinct error (de-spammed: Error the first time a given `(name, revision, KekId)` fails, Debug on identical repeats, Error again after an intervening success).
- **Isolation.** One row failing to apply/verify does not block any other row in the same pass.