Files
ScadaBridge/docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md
T
Joseph Doherty 4e6409184c Merge branch 'fix/standby-secret-mirror-status' (M1: standby mirror failures on the health report)
# Conflicts:
#	CLAUDE.md
#	docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md
2026-09-28 21:37:03 -04:00

279 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Site secrets ride the artifact deploy — design
**Status:** design, 2026-09-28. Plan: `2026-09-28-site-secrets-artifact-deploy.md`.
## 1. Problem
A site resolves secrets at runtime (today: an MxGateway data connection's `ApiKey: "secret:<name>"`,
`DataConnectionLayer/Adapters/MxGatewayDataConnection.cs:101`). The only way a secret reaches a site
is the pull-only gRPC secrets hub (`Secrets:Replication:Mode=Grpc`), which:
- copies **every** central secret to **every** participating site node every 30 s — no per-site scope;
- is invisible to the deployment model: the Sites page *Artifacts current / out of date* badge
(`Site.ArtifactRevisionHash`, 2026-09-14) knows nothing about secrets, so a site can read *Current*
while its data connection cannot resolve its key;
- logs nothing on a successful in-sync sweep (library `GrpcSecretSyncService` logs only when rows were
pulled or on failure) and exposes no status;
- is enabled only on the docker rig's site-a; vd03 and QA run with replication OFF, so each site node's
`secrets.db` is filled by hand.
## 2. Decisions (user, 2026-09-28)
| # | Decision |
|---|---|
| U1 | A site receives **only the secrets it references** — never other sites' keys. |
| U2 | The artifact deploy **replaces** the gRPC hub as the way secrets reach sites (hub retired for ScadaBridge sites). |
| U3 | Logging = **deploy log lines** (central + site), **audit rows**, and **hub sweep logging** (for the transition until the hub is retired, and for other hub consumers). |
| U4 | Central deploys from **one shared SQL-Server secret store** used by both central nodes. |
## 3. Design
### 3.1 Which secrets a site needs — derived, not configured
`SecretReferenceScanner.Scan(IEnumerable<DataConnectionArtifact>)` walks each connection's
`PrimaryConfigurationJson` and `BackupConfigurationJson` and collects every JSON **string value**
of the form `secret:<name>` (prefix case-insensitive, `<name>` trimmed, must be a valid `SecretName`).
It is protocol-agnostic on purpose: a new adapter that adopts `secret:` references is covered with
no scanner change. Result: a distinct, ordinal-sorted name set per site, each name carrying the
data connections that reference it (for error messages and logs).
**Deliberately NOT included:** bootstrap secrets — the site's gRPC PSK (`SB-GRPC-PSK-{siteId}`), the
LocalDb replication key, the secrets master key. The site needs them *before* it can receive an
artifact deploy, and pushing a rotated PSK through the channel it authenticates would sever that
channel. They stay out-of-band (service environment / hand-seeded store), exactly as today on vd03/QA.
`${secret:X}` references inside a node's own `appsettings` are node-local and invisible to central —
also out of scope.
### 3.2 Central source of truth — the shared SQL store (U4)
New option `Secrets:Store:Provider` (`Sqlite` default | `SqlServer`), read **only on the Central
role**. `SqlServer` registers the library's `AddZbSecretsSqlServerStore` against
`Secrets:SqlServer:ConnectionString` (literal/env value — never `${secret:}`, same bootstrap rule as
today) so both central nodes read and write one store; the `/admin/secrets` page edits it. This is
the store the hub's Grpc-mode central already uses — decoupled from the hub. A clustered Central left
on `Sqlite` boots but `StartupValidator` logs a Warning (same shape as the inbound API key-store
warning, row 108), because the two nodes would deploy different secret sets depending on which is
active.
### 3.3 The wire: ciphertext rows, never plaintext
`DeployArtifactsCommand` gains an additive `Secrets` list of `SecretArtifact` — the library's
`StoredSecret` fields verbatim (name, content type, ciphertext/nonce/tag, wrapped DEK/nonce/tag,
`KekId`, `Revision`, created/updated UTC). Central never decrypts; the site applies the row with
`ISecretStore.ApplyReplicatedAsync` (last-writer-wins on `UpdatedUtc`/`Revision`, the same path the
hub sweep uses). **Every node must therefore share the KEK** (`ZB_SECRETS_MASTER_KEY` / master-key
file) — exactly the hub's existing requirement. A mismatch is detected at deploy time (§3.5), not at
the next reconnect. Proto: `DeployArtifactsCommandDto.secrets = 9` (`SecretArtifactListDto`, absent ⇒
null) and `ArtifactDeploymentResponseDto` gains `secrets_applied` (wrapper `Int32Value`, absent ⇒ null).
**Fail-closed on a missing secret.** If a referenced name is absent or tombstoned in the central
store, that site's deploy fails before anything is sent: `Secret 'X' referenced by data connection(s)
A, B is not in the central secret store`. Shipping the rest would stamp the site *Current* while a
connection cannot resolve its key.
### 3.4 Staleness — secrets are part of the artifact hash
`ArtifactRevisionHashService` adds `Secrets` → `HashableSecret { ContentDigest, KekId, Missing,
Name, Revision }` (alphabetical — the existing ordering test covers it). `ContentDigest` = SHA-256 of
ciphertext‖nonce‖tag‖wrappedDek, so a KEK rewrap (new wrapped DEK, same revision) correctly marks
sites out of date. `ComputeCurrentArtifactHashesAsync` hashes missing names with `Missing = true`, so
a site referencing a not-yet-created secret reads *Out of date*, never *Current*. The list is
null-when-empty and the serializer omits nulls, so **sites with no secret references keep their
current hash** — only sites that reference secrets flip to *Out of date* once after rollout.
### 3.5 Site apply — both nodes, via a replicated table
The artifact apply runs on the active node's `DeploymentManagerActor` singleton, but `secrets.db` is
a per-node file outside LocalDb. So:
1. **`deployed_secrets`** — a new LocalDb table in `SiteStorageSchema` (`name` PK, `row_json` = the
`SecretArtifact` ciphertext row as JSON, `revision`, `updated_utc`, `is_removed` 0/1,
`deployment_id`, `applied_at`), added to `SiteLocalDbSetup.ReplicatedTables` (becomes the 11th;
LocalDb 0.3.x snapshots just that table on an already-replicating pair). No FKs (row 105 guard).
Ciphertext only — the no-plaintext rule that kept `smtp_configurations` unregistered holds.
`HandleDeployArtifacts` applies it full-set: upsert every shipped row, mark rows absent from the
set `is_removed = 1` (kept, so the standby learns about the removal).
2. **`DeployedSecretsMirror`** — a per-node service (every site node, not a singleton) that reconciles
`deployed_secrets` into the node's own `ISecretStore`: `ApplyReplicatedAsync` for live rows,
`DeleteAsync` for removed rows **only if the name was previously mirrored** (a local, unreplicated
`mirrored_secrets` table records names this mirror wrote — an out-of-band secret of the same name
that was never deployed is never deleted). After applying, it **trial-resolves** each deployed
secret through `ISecretResolver`; a decrypt failure (KEK mismatch) is an error naming the secret
and the row's `KekId`. It runs at startup, every `SiteRuntime:DeployedSecretsMirrorInterval`
(default 15 s — the standby's convergence path), and on demand.
3. The active node's apply calls the mirror **synchronously** after writing the table and fails the
deploy response if any secret did not apply or verify; on success the response carries
`SecretsApplied = n`. The standby converges through LocalDb replication + its own mirror within one
interval.
### 3.6 Mixed versions
A pre-feature site ignores proto field 9 and answers `Success` with `secrets_applied` absent. Central
treats *command carried ≥ 1 secret AND `SecretsApplied` is null* as a **failure** for that site
(`site build predates secret deployment — upgrade the site`) and does not stamp the hash. A command
with no secrets is unaffected, so an old site with no secret references deploys exactly as before.
Upgrade central first, as always.
### 3.7 Logging and audit (U3)
- **Central, per site:** Information `Deploying {Count} secret(s) to site {SiteId}: {Name}@r{Revision},…`;
Error on a missing secret (names + referencing connections). Never values or ciphertext.
- **Site, active apply and every mirror pass that changes something:** Information per secret
`Applied deployed secret {Name} r{Revision} (kek {KekId})` / `Removed deployed secret {Name}`;
Error on apply/verify failure; one Information summary per pass. An in-sync mirror pass logs at
Debug only.
- **Audit:** one `IAuditService` row per site deploy that carried secrets — action `DeploySiteSecrets`,
entity type `SiteSecrets`, entity id = site identifier, after-state `{ deploymentId, secrets:
[{name, revision}], removed: [name], success }`. Same config-change audit as the existing
`DeployArtifacts` row.
- **Hub sweep (library `ZB.MOM.WW.Secrets` 0.6.3, scadaproj):** every sweep outcome is observable —
in-sync sweeps at Debug plus an Information heartbeat at most once per `InSyncLogInterval`
(default 1 h), a `Recovered after {Failures} failed sweep(s)` Information line, and a public
`ISecretsHubSyncStatus` (`LastSweepUtc`, `LastSuccessUtc`, `ConsecutiveFailures`, `LastError`).
Useful for the rig until the hub is retired and for any other hub consumer.
### 3.8 Hub retirement (U2)
Once the artifact path is live on the rig, ScadaBridge stops composing the hub: the Site role no
longer registers `AddZbSecretsGrpcHubClient`, the Central role no longer maps the hub endpoint, and
`StartupValidator` refuses `Secrets:Replication:Enabled=true` with `Mode=Grpc` with a message pointing
at the artifact deploy. The library keeps the hub (other consumers); only ScadaBridge stops using it.
The rig's site-a moves from the hub to the artifact path; central keeps the shared SQL store through
`Secrets:Store:Provider=SqlServer`.
## 4. Rollout (deployment records, not code)
vd03 and QA need, per environment: the `zbsecrets` store in the configuration SQL Server + both
centrals on `Secrets:Store:Provider=SqlServer`; **one KEK across every central and site node**
(verify before enabling — the mirror's verify step reports a mismatch by name); existing hand-seeded
site secrets recreated at central; then *Deploy Artifacts*. Tracked as a deferred-work register row.
## 5. Out of scope
Per-site explicit secret lists; secrets for external systems / database connections (they ship
literal values today); plaintext transport; moving bootstrap secrets through the deploy.
## 6. Implementation notes (as built)
Execution (tasks 1–11, ledger `.superpowers/sdd/2026-09-28-site-secrets-artifact-deploy/progress.md`)
matched this design in shape but landed several deviations, called out here so the doc stays
accurate as the spec:
- **Central collection.** The scanner and per-site resolve described in §3.1/§3.3 are implemented
as `SiteSecretsCollector` (`ZB.MOM.WW.ScadaBridge.DeploymentManager`), not inline in
`ArtifactDeploymentService`. Fail-closed (§3.3) is per site, not per batch: a site with an
unresolved reference is skipped (`_communicationService` is never called for it) while every
other site in the same sweep proceeds normally.
- **Audit (§3.7).** The `DeploySiteSecrets` / `SiteSecrets` audit row is written on **both**
`DeployCoreAsync` (the sweep) and `RetryForSiteAsync` (the per-site retry), including the
fail-closed path on each — the design's §3.7 wording ("one row per site deploy that carried
secrets") did not call out the retry path explicitly; it is in scope and covered.
- **Stamp gating (§3.6).** Implemented as designed, plus one addition: a **partial apply**
(`SecretsApplied` less than the shipped count, not just the mixed-version `null` case) is also
treated as a failure and is not stamped, with its own message (`Site applied {n} of {m}
secrets`) distinct from the "predates secret deployment" wording.
- **Hash (§3.4).** Implemented as designed. Confirmed explicitly (not just implied by
"null-when-empty"): an **empty** `Secrets`/`MissingSecrets` list and a **null** one hash
identically, and the pre-existing golden hash for a site with no secret references is
unchanged by this feature (pinned by a dedicated regression test).
- **Site apply (§3.5).** `HandleDeployArtifacts` writes the full `deployed_secrets` set and then
runs `DeployedSecretsMirror.ReconcileAsync` **synchronously** before replying, exactly as
designed. `SecretsApplied` counts only the names *this deploy shipped* that applied and
verified without error — a mirror failure for a name this deploy did not ship (e.g. another,
already-broken deployed secret, or an unrelated removal) is logged at Warning, excluded from
the count, and does not fail the deploy. Removal failures are likewise Warnings, never deploy
failures.
- **`DeployedSecretsMirror` — apply is confirm-then-ledger, not the blind `ApplyReplicatedAsync`
§3.5 originally described.** The real library store is strict last-writer-wins on
`(UpdatedUtc, Revision)` (a tie is a no-op) and tombstones rather than hard-deletes, which the
naive "apply then ledger" sequence gets wrong in two ways: a routine remove-then-re-add can
no-op against a still-live tombstone while the ledger is written anyway (silently broken), and
an out-of-band local secret with a genuinely newer stamp can be ledgered and later deleted by a
central tombstone (destroying the operator's own secret). The mirror now always reads the row
back after applying and only ledgers when the local ciphertext matches what was shipped; a
tombstoned or ledger-owned row is safely re-stamped (`UpdatedUtc`/`Revision` bumped past the
local value, including across clock skew) and re-applied, while a live row with different
ciphertext that the ledger does **not** already own is left completely untouched and reported
as a distinct out-of-band-conflict failure. Ownership is decided from `mirrored_secrets`
membership at any revision, not from tombstone state alone — so a mirror-owned name that drifts
locally (an operator's edit over an already-deployed name) is re-stamped back to the deployed
value on the next pass: **central is authoritative for every name it deploys.** Every apply/
delete also invalidates the shared `ISecretResolver` cache before the verify step, and one
poisoned row never blocks the rest of the pass.
- **Hub retirement (§3.8) — scope widened to ALL secrets replication, not just the gRPC hub.**
ScadaBridge also retires the historical SQL-Server *replication* mode
(`Secrets:Replication:Mode=SqlServer`, distinct from the still-supported
`Secrets:Store:Provider=SqlServer` shared-store feature) for the same reason the gRPC hub goes:
a site's `ISecretStore` must be the plain, undecorated local SQLite store, because
`DeployedSecretsMirror` calls `DeleteAsync` directly and a replication decorator would publish
that delete fleet-wide. `Secrets:Replication:Enabled` set to anything other than
absent/empty/`false` (a deliberately strict, non-boolean-coerced check — `"0"`/`"1"`/`"yes"`
all count as "set") now fails `StartupValidator.Validate` on **either** role. A leftover
`Secrets:Replication:Mode` or any `Secrets:GrpcHub:*` key logs a startup Warning naming both
sections. The pre-host `${secret:}` expander was fixed in the same task set to read the shared
SQL store on a `Store:Provider=SqlServer` central (it previously only did this for the retired
Grpc mode) — without that fix a central's boot-time `${secret:}` tokens and its running
`ISecretResolver` could resolve a token differently.
- **`ZB.MOM.WW.Secrets` 0.6.3 (hub sweep logging, §3.7's third bullet — `EventId`s 4/5/6,
`ISecretsHubSyncStatus`) is published to the Gitea feed (2026-09-28, after merge), but
ScadaBridge does not consume it** — hub retirement (above) removes ScadaBridge's only dependency
on the hub client, so it stays on 0.6.2. It is there for the library's other hub consumers
(OtOpcUa).
- **Empty, not null, when a site references no secrets (Task 13 live-gate fix).** The command
builder first shipped `Secrets = null` for a site with no references ("null-when-empty", borrowed
from the hash convention). But the site treats `null` as "central shipped no secrets" and touches
nothing, so dropping a site's LAST `secret:` reference never soft-removed the row and the secret
stayed live on both nodes. Central now always ships a list (empty when there are no references);
the hash is unchanged because an empty and a null list hash identically.
- **Final-review fixes.**
- *Same-revision drift is healed.* A name already ledgered at the shipped revision is still read
back every pass; a missing, tombstoned or edited local row is re-stamped back to the shipped
ciphertext before verify (previously a local delete was misreported as a KEK mismatch and
blocked the site's deploys).
- *Unreadable `row_json`* is a per-name failure, not a pass-wide exception.
- *Site ordering:* secrets are written and mirrored before the deploy's data connections are
pushed to the DCL; a rotation with unchanged connection config takes effect on the connection's
next (re)connect (no forced reconnect).
- *One `secret:` parser* (`SecretReferenceScanner.TryParseSecretReference`) shared by the scanner
and `MxGatewayDataConnection`, so a padded/case-variant reference resolves the shipped name.
- *Audit shape (§3.7).* The `DeploySiteSecrets` row carries `secrets`, `missing` and `removed`.
(Shipped first without `removed`, so a removal-only deploy wrote no central row; closed by the
follow-ups below.)
- *Deployment prerequisite:* a site pair using deployed secrets must run LocalDb replication
(topology guide), since on an unreplicated pair only the node active at deploy time gets them.
- **Follow-ups closed after merge (2026-09-28).**
- *Secret-store outage is per site (review M5).* `SiteSecretsCollector` returns a store read
failure as `SiteSecretsCollection.StoreError` instead of throwing. A deploy fails only the
sites that reference a secret (nothing is sent to them; they get a `DeploySiteSecrets` row
with the error), and `ComputeCurrentArtifactHashesAsync` leaves those sites out of its map,
which the pending-updates evaluator reads as *Out of date*. Sites with no secret references
never touch the store and are unaffected. Before this, one `GetAsync` exception aborted the
whole sweep and the fleet-wide pending-updates check.
- *Removal-only deploys are audited.* The site's reply gained `SecretsRemoved` (proto
`ArtifactDeploymentResponseDto.secrets_removed = 7`, names only; absent/empty reads as null):
the `deployed_secrets` rows that deploy soft-removed. Central carries it on
`SiteArtifactResult.SecretsRemoved` and writes the `DeploySiteSecrets` row whenever it is
non-empty, with a `removed` list. An old site never sends it, so its removals stay unaudited
centrally, as before.
- *CLI exit code.* `site deploy-artifacts` and `deploy artifacts` exit `1` when the summary's
`failureCount` is non-zero (same rule `deploy site` already had).
- *Hub-era site-a secrets (review M8).* The retired hub had copied six `hub-gate-*`/`shared-gate-*`
test secrets (2026-08-07) into both rig site-a nodes' local stores. The mirror deliberately
never deletes a name it did not ledger, so they were removed once by hand on the rig (both
nodes stopped together; see the live-gate doc). QA and vd03 never ran the hub
(`Secrets:Replication:Enabled=false`), so nothing like this exists there.
- **Standby mirror failures reach central (follow-up, review finding M1).** A `DeployedSecretsMirror`
failure on the standby (e.g. a KEK mismatch) was only logged on that node — site health reports
are sent by the active node only. Each node now records its current failure set (names +
messages, never values) in a new, **12th LocalDb-replicated table `secret_mirror_status`** (one
row per node keyed by `node_name`), written only when the set changes plus once per start-up; a
per-node `SecretMirrorStatusReporter` puts every node's row on the active node's report as
`SiteHealthReport.SecretMirrorStatuses` (proto `SiteHealthReportDto` field 33, presence-wrapped),
and the Central UI health card shows a *Secret mirror failing* badge + per-node list. Requires
LocalDb replication for the standby's row to be visible, the same prerequisite as the secrets.
- **Rig posture.** `docker/docker-compose.yml`'s central nodes now carry
`Secrets__Store__Provider=SqlServer` plus a shared KEK; site-a shares that KEK (it is the
secrets-via-artifact-deploy site); site-b/site-c deliberately carry a **mismatched** KEK so the
`DeployedSecretsMirror` verify-failure path has a real, provable negative on the rig rather than
an assumed one.