From 29049c4ef69ec168468290d766664175567a38de Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 28 Sep 2026 21:23:52 -0400 Subject: [PATCH] fix(site-secrets): contain secret-store outages per site; audit removal-only deploys Secret-store outage (final review M5): SiteSecretsCollector returns a store read failure as SiteSecretsCollection.StoreError instead of throwing. A deploy now fails only the sites that reference a secret (nothing sent, not stamped, DeploySiteSecrets row with the error), and ComputeCurrentArtifactHashesAsync omits only those sites (they read Out of date) instead of the exception aborting the whole sweep and the fleet-wide pending-updates check. Removal-only deploys: the site reply gains SecretsRemoved (proto field 7, names only) - the deployed_secrets rows the deploy soft-removed. Central carries it on SiteArtifactResult and writes the DeploySiteSecrets row with a removed list whenever it is non-empty, so dropping a site's last secret: reference is audited centrally rather than only in site logs. --- CLAUDE.md | 2 +- ...-28-site-secrets-artifact-deploy-design.md | 27 +- .../Component-DeploymentManager.md | 29 +- docs/requirements/Component-SiteRuntime.md | 1 + .../Artifacts/ArtifactDeploymentResponse.cs | 15 +- .../Grpc/SiteCommandDtoMapper.cs | 8 +- .../Protos/site_command.proto | 1 + .../SiteCommandGrpc/SiteCommand.cs | 781 +++++++++--------- .../ArtifactDeploymentService.cs | 112 ++- .../SiteSecretsCollector.cs | 37 +- .../Actors/DeploymentManagerActor.cs | 18 +- .../Grpc/SiteCommandDtoMapperSecretsTests.cs | 19 + .../ArtifactDeploymentServiceTests.cs | 178 ++++ .../SiteSecretsCollectorTests.cs | 41 +- ...loymentManagerActorArtifactSecretsTests.cs | 29 + 15 files changed, 875 insertions(+), 423 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b15e2431..25416184 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,7 +141,7 @@ spec for each is `docs/requirements/Component-.md`, and `README.md` carrie - Per-instance operation lock covers all mutating commands (deploy, disable, enable, delete). - **Bulk site deployment (`DeploymentService.DeploySiteAsync`, arch-review WP2.5)** redeploys every out-of-date instance at a site through one flatten session (a template chain shared by N instances is walked once) in three phases — serial Prepare, bounded-parallel Send (`SiteDeploymentMaxParallelism`, default 4), serial Finalize — still **not all-or-nothing**: any one instance's failure doesn't block the rest. **Disabled instances are SKIPPED by bulk deploy as of 2026-08-19 (arch-review P7-2)** — they are reported in the summary's additive `SkippedInstances` list, never flattened/sent/re-enabled (single-instance deploy remains the explicit operator path that deploys+enables a Disabled instance); **Up-to-date instances are ALSO skipped as of 2026-08-20 (arch-review P7-6)** — a redeploy is not idempotent at the site (it resets static overrides and clears native alarm state), so `DeploySiteAsync` pre-filters through `IStaleInstanceProbe` (trustworthy since P7-4 fixed its memo timing) and reports unchanged instances in `SkippedInstances` with reason `up-to-date`; the filter is conservative (every uncertainty deploys: missing snapshot, `ShouldQuerySiteBeforeRedeploy`, null/faulting probe). Additive `MgmtDeploySiteCommand.Force` / CLI `deploy site --force` restores the unconditional push for what central cannot see (restored volume, rebuilt node) — **Disabled still wins under `--force`**. Also P7-5: the three phases share one scoped DbContext and a failed `SaveChanges` leaves the entity tracked, so one permanent failure used to cascade to every later instance and strand records `InProgress`; `MarkDeploymentFailedAsync` now detaches pending changes and retries the `Failed` write once on the clean tracker; Prepare's per-instance reconcile also runs under a short `SiteDeploymentReconcileTimeout` (5s) with a once-per-batch unreachable-site circuit-breaker, so a down site no longer costs 30s × N serial before the first send (P7-3). The `PendingDeployment` staging row (5-minute TTL) is written per-instance immediately before that instance's round-trip in the Send phase, not upfront in Prepare, so a large batch's tail instances don't have their TTL clock already half-spent by the time they're reached. - **Site artifact revision + fleet-wide pending-updates indicator (2026-09-14, `docs/plans/2026-09-14-pending-updates-design.md`).** `Site.ArtifactRevisionHash`/`ArtifactsDeployedAt` (migration `AddSiteArtifactRevision`) are stamped by `ArtifactDeploymentService` only on a site whose artifact deploy succeeds, so a redeployed fleet's artifacts are trackable per site the same way instance staleness already is. `PendingUpdatesEvaluator` combines that with `DeploymentService.GetInstanceStalenessAsync` (one shared `FlattenSession` for the whole fleet) into a `PendingUpdatesSnapshot`, held behind a process-wide singleton `IPendingUpdatesCache` (`ScadaBridge:DeploymentManager:PendingUpdatesCacheTtl`, default 60s) so N Blazor circuits cost one fleet flatten per TTL, not one each. Surfaces: the Sites page's per-site **Artifacts** badge (`Current`/`Out of date`/`Never deployed`) + instance-staleness line + header **Re-check**, and a rail-footer **pending updates** indicator (Deployer/Administrator only, 30s poll, site-scoped). **Upgrade consequence:** every site reads *Never deployed* until one artifact deploy runs post-upgrade — deliberate, not a bug. Also shipped: the rail nav now sits in its own scroll pane (`NavMenu`'s `.rail-scroll` + `site.css`) so a long nav can't push the footer/indicator off-screen; the `ZB.MOM.WW.Theme` kit itself is untouched (register row 123 proposes upstreaming it). -- **Site secrets ride the artifact deploy; the gRPC secrets hub is retired in ScadaBridge (2026-09-28, `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md`).** `Deploy Artifacts` now ships each site **only the secrets it references** — `SiteSecretsCollector` scans a site's data-connection JSON for `secret:` (`SecretReferenceScanner`, protocol-agnostic, bootstrap names like `SB-GRPC-PSK-*` excluded) and resolves each against the central `ISecretStore`; a missing/tombstoned secret fails **that site's** deploy before anything is sent (fail-closed), ciphertext only ever crosses the wire, and secrets factor into `ArtifactRevisionHashService`'s staleness hash (a missing secret hashes as a distinguishable marker, so an unresolvable reference never reads *Current*; a site with no secret references hashes identically whether the field is null or empty). A pre-feature site (answers with no `SecretsApplied`) or a partial apply is also treated as a deploy failure and is never stamped; one `DeploySiteSecrets`/`SiteSecrets` audit row (`secrets` name + revision and `missing` names only, never ciphertext; no `removed` field — a removal-only deploy writes no central row) is written per site deploy that carried secrets or a missing-secret failure, on both the sweep and the per-site retry paths. Site-side: `deployed_secrets` is the 11th LocalDb-replicated table (ciphertext rows only, upsert-then-soft-remove); the per-node `DeployedSecretsMirror` (`SiteRuntime:DeployedSecretsMirrorInterval`, default 15s, also run synchronously right after an active-node apply) reconciles it into that node's local `ISecretStore`, ledgers what it mirrored in the node-local `mirrored_secrets` table so an out-of-band local secret sharing a deployed name is never silently touched (a mirror-owned name deleted or edited locally is healed back to the deployed value on the next pass, even at an unchanged revision), and trial-decrypts every pass to catch a KEK mismatch. Secrets are mirrored before the deploy's data connections reach the DCL; a rotation with unchanged connection config takes effect on the next (re)connect. **A site pair using deployed secrets must run LocalDb replication** — otherwise only the node active at deploy time holds them. Central runs one shared secret store, `Secrets:Store:Provider=SqlServer` (Central role only; a clustered central left on `Sqlite` gets a boot Warning) — the pre-host `${secret:}` expander reads the same store the running node serves. **ALL secrets replication is retired in ScadaBridge** — the pull-only gRPC hub and the SQL-Server replication mode both — `Secrets:Replication:Enabled` set to anything but empty/`false` now fails `StartupValidator` on either role with a message pointing at Deploy Artifacts / `Secrets:Store:Provider=SqlServer`. +- **Site secrets ride the artifact deploy; the gRPC secrets hub is retired in ScadaBridge (2026-09-28, `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md`).** `Deploy Artifacts` now ships each site **only the secrets it references** — `SiteSecretsCollector` scans a site's data-connection JSON for `secret:` (`SecretReferenceScanner`, protocol-agnostic, bootstrap names like `SB-GRPC-PSK-*` excluded) and resolves each against the central `ISecretStore`; a missing/tombstoned secret fails **that site's** deploy before anything is sent (fail-closed), ciphertext only ever crosses the wire, and secrets factor into `ArtifactRevisionHashService`'s staleness hash (a missing secret hashes as a distinguishable marker, so an unresolvable reference never reads *Current*; a site with no secret references hashes identically whether the field is null or empty). A pre-feature site (answers with no `SecretsApplied`) or a partial apply is also treated as a deploy failure and is never stamped; one `DeploySiteSecrets`/`SiteSecrets` audit row (`secrets` name + revision, `missing` and `removed` names, never ciphertext — `removed` comes from the site's `SecretsRemoved` reply, so a removal-only deploy is audited too) is written per site deploy that carried, lacked or retired secrets, on both the sweep and the per-site retry paths. A central secret-store read failure fails only the sites that reference a secret and leaves them *Out of date* in the pending-updates check — it no longer aborts the sweep or the fleet-wide check — and both artifact-deploy CLI commands exit 1 when any site failed. Site-side: `deployed_secrets` is the 11th LocalDb-replicated table (ciphertext rows only, upsert-then-soft-remove); the per-node `DeployedSecretsMirror` (`SiteRuntime:DeployedSecretsMirrorInterval`, default 15s, also run synchronously right after an active-node apply) reconciles it into that node's local `ISecretStore`, ledgers what it mirrored in the node-local `mirrored_secrets` table so an out-of-band local secret sharing a deployed name is never silently touched (a mirror-owned name deleted or edited locally is healed back to the deployed value on the next pass, even at an unchanged revision), and trial-decrypts every pass to catch a KEK mismatch. Secrets are mirrored before the deploy's data connections reach the DCL; a rotation with unchanged connection config takes effect on the next (re)connect. **A site pair using deployed secrets must run LocalDb replication** — otherwise only the node active at deploy time holds them. Central runs one shared secret store, `Secrets:Store:Provider=SqlServer` (Central role only; a clustered central left on `Sqlite` gets a boot Warning) — the pre-host `${secret:}` expander reads the same store the running node serves. **ALL secrets replication is retired in ScadaBridge** — the pull-only gRPC hub and the SQL-Server replication mode both — `Secrets:Replication:Enabled` set to anything but empty/`false` now fails `StartupValidator` on either role with a message pointing at Deploy Artifacts / `Secrets:Store:Provider=SqlServer`. - **Create Instance picks its template from the Templates folder tree** (2026-08-21): the flat `