diff --git a/Directory.Packages.props b/Directory.Packages.props index 6badbfd5..67c3c9e0 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -104,11 +104,11 @@ - - - - - + + + + + diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index ac54161e..6d30f80b 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -21,11 +21,13 @@ x-secrets-hub-env: &secrets-hub-env # ciphertext replicates, so a node with a different KEK fails closed on resolve with a # kek_id mismatch that reads like corruption but is a deployment error. ZB_SECRETS_MASTER_KEY: "zZiBWuoaVMbJmGXToLk9Lakw0iJozXoL/7Gxac3GwJ4=" - # The appsettings default is the relative "scadabridge-secrets.db", which resolves to - # /app — inside the image's writable layer, so it is destroyed by any container - # recreate and unreachable from the host. /app/data is the node's own mounted volume - # (the one LocalDb already uses on sites; added to the central pair for this). - Secrets__SqlitePath: "/app/data/scadabridge-secrets.db" + # Must be absolute AND outside the content root (/app) — Secrets 0.6.2 validates both + # at startup, because a store inside the deployment directory is destroyed by an + # ordinary upgrade (this wiped the MxGateway key store on 2026-08-09). /data is the + # node's own bind mount, the SAME host directory as /app/data, so the existing store + # is found unmoved; only the path used to reach it changes. Do NOT point this back at + # /app/data — absolute is not sufficient, and the boot will fail. + Secrets__SqlitePath: "/data/scadabridge-secrets.db" Secrets__Replication__Enabled: "true" Secrets__Replication__Mode: "Grpc" # DEV-ONLY shared bearer token — NOT a real secret. Presented by every follower and @@ -116,6 +118,10 @@ services: # scadaproj#4) the scadabridge-secrets.db here is a pre-0.5.0 residue, but the # volume is still needed (inbound-api-keys.sqlite lives on it). - ./central-node-a/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./central-node-a/data:/data - ./central-node-a/logs:/app/logs networks: - scadabridge-net @@ -173,6 +179,10 @@ services: # scadaproj#4) the scadabridge-secrets.db here is a pre-0.5.0 residue, but the # volume is still needed (inbound-api-keys.sqlite lives on it). - ./central-node-b/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./central-node-b/data:/data - ./central-node-b/logs:/app/logs networks: - scadabridge-net @@ -197,6 +207,10 @@ services: volumes: - ./site-a-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-a-node-a/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-a-node-a/data:/data - ./site-a-node-a/logs:/app/logs networks: - scadabridge-net @@ -221,6 +235,10 @@ services: volumes: - ./site-a-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-a-node-b/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-a-node-b/data:/data - ./site-a-node-b/logs:/app/logs networks: - scadabridge-net @@ -242,6 +260,10 @@ services: volumes: - ./site-b-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-b-node-a/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-b-node-a/data:/data - ./site-b-node-a/logs:/app/logs networks: - scadabridge-net @@ -263,6 +285,10 @@ services: volumes: - ./site-b-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-b-node-b/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-b-node-b/data:/data - ./site-b-node-b/logs:/app/logs networks: - scadabridge-net @@ -284,6 +310,10 @@ services: volumes: - ./site-c-node-a/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-c-node-a/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-c-node-a/data:/data - ./site-c-node-a/logs:/app/logs networks: - scadabridge-net @@ -305,6 +335,10 @@ services: volumes: - ./site-c-node-b/appsettings.Site.json:/app/appsettings.Site.json:ro - ./site-c-node-b/data:/app/data + # Same host directory, mounted a second time OUTSIDE the content root (/app). + # Secrets 0.6.2 refuses a store path under the content root; this bind mount is + # not touched by an image rebuild, but the guard is lexical and cannot know that. + - ./site-c-node-b/data:/data - ./site-c-node-b/logs:/app/logs networks: - scadabridge-net diff --git a/docs/operations/2026-07-16-secrets-clustered-master-key.md b/docs/operations/2026-07-16-secrets-clustered-master-key.md index 3fac4b4a..1ac9350b 100644 --- a/docs/operations/2026-07-16-secrets-clustered-master-key.md +++ b/docs/operations/2026-07-16-secrets-clustered-master-key.md @@ -148,16 +148,29 @@ The committed default in `appsettings.json` is: ```json "Secrets": { - "SqlitePath": "scadabridge-secrets.db", "MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" }, "RunMigrationsOnStartup": true, "ResolveCacheTtl": "00:00:30" } ``` -This is dev-safe: `Source=Environment` needs no filesystem key, and the SQLite -path is relative to the working directory, so local dev and the -`WebApplicationFactory` `Host.Tests` boot cleanly with no external mount. +**`SqlitePath` is deliberately absent** (changed with `ZB.MOM.WW.Secrets` 0.6.2, 2026-08-11). +That version validates the setting at startup and rejects it unless it is both **absolute** and +**outside the application content root** — the two rules are independent, and the path that +destroyed the MxGateway key store on 2026-08-09 was absolute. The former committed value +(`"scadabridge-secrets.db"`) was relative and now fails the boot, so it was removed rather than +replaced: an unset path takes the library's absolute per-user `LocalApplicationData` default, +which satisfies both rules everywhere without a mount. + +This is dev-safe: `Source=Environment` needs no filesystem key, and the per-user default is +writable, so local dev and the `WebApplicationFactory` `Host.Tests` boot cleanly with no +external mount. + +**Containerized deployments need care.** The rule is lexical, so a path under the content root is +rejected even when it is a *bind mount* that an image rebuild never touches. The docker rig hit +exactly this: `/app/data/scadabridge-secrets.db` with content root `/app`. The fix was to mount +each node's existing host directory a **second** time at `/data` and point `Secrets__SqlitePath` +there — same underlying directory, so the store is found unmoved and nothing needs migrating. Local dev and docker-compose environments supply concrete secret values via the **whole-key environment override** (e.g. `ScadaBridge__Database__ConfigurationDb`), which bypasses `${secret:...}` resolution entirely, so the expander is effectively diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs index 1f997614..38d5fa01 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/Program.cs @@ -101,6 +101,12 @@ var expanderUsesSharedSqlStore = configuration["ScadaBridge:Node:Role"], "Central", StringComparison.OrdinalIgnoreCase) && SecretsRegistration.UsesGrpcHub(configuration); +// Secrets 0.6.2 validates Secrets:SqlitePath as absolute AND outside the content root, finding the +// content root from IHostEnvironment — which this throwaway ServiceCollection does not have, and +// which does not exist yet anyway (the host is not built until further down). Both the resolution +// and the registration therefore live in SecretsRegistration, where a test can enforce them; see +// AddPreHostSqliteExpander for why the content-root argument is load-bearing and silent when +// dropped, and PreHostSecretsContentRootTests for the regression guard. var expanderServices = new ServiceCollection(); if (expanderUsesSharedSqlStore) { @@ -109,7 +115,8 @@ if (expanderUsesSharedSqlStore) } else { - expanderServices.AddZbSecrets(configuration, "Secrets"); + expanderServices.AddPreHostSqliteExpander( + configuration, SecretsRegistration.ResolveHostContentRoot()); } #pragma warning disable ASP0000 // deliberate throwaway container diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs index cb18cec3..f0adcad8 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Host/SecretsRegistration.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Hosting.WindowsServices; using ZB.MOM.WW.Secrets.DependencyInjection; using ZB.MOM.WW.Secrets.Replicator.Grpc.DependencyInjection; using ZB.MOM.WW.Secrets.Replicator.SqlServer.DependencyInjection; @@ -185,6 +186,65 @@ public static class SecretsRegistration && ResolveReplicationMode(config) == SecretsReplicationMode.Grpc; } + /// + /// Resolves the content root the HOST will use, from outside the host — the Layer-A + /// ${secret:} expander in Program.cs runs before any builder exists, so + /// builder.Environment.ContentRootPath is not available to it. + /// + /// + /// Mirrors the framework's own resolution order. The Windows-Service branch is the one that + /// is easy to get wrong: UseWindowsService sets both the current directory and the + /// content root to , but only once the host is built — + /// at expander time the process working directory is still C:\Windows\system32, so + /// taking the current directory there would compare the store path against the wrong root. + /// + /// The absolute content-root path the host will resolve. + public static string ResolveHostContentRoot() => + Environment.GetEnvironmentVariable("ASPNETCORE_CONTENTROOT") + ?? Environment.GetEnvironmentVariable("DOTNET_CONTENTROOT") + ?? (WindowsServiceHelpers.IsWindowsService() + ? AppContext.BaseDirectory + : Directory.GetCurrentDirectory()); + + /// + /// Composes the SQLite secrets stack for the Layer-A pre-host ${secret:} expander. + /// + /// + /// + /// This exists as a named method rather than an inline AddZbSecrets call so that the + /// content-root argument is covered by a test (PreHostSecretsContentRootTests). The + /// argument is load-bearing and its absence is SILENT: the expander composes into a throwaway + /// that has no IHostEnvironment, so without it the + /// under-content-root rule does not run, the migrator creates the store at the rejected path, + /// and the boot then fails once the real host registers IHostEnvironment — leaving an + /// empty database behind. That "the file is there, it's just empty" artifact is the confusing + /// half of the 2026-08-09 MxGateway outage the guard exists to prevent. + /// + /// + /// A correctly configured path shows NO symptom either way, so this cannot be verified by + /// observing a clean boot — the exposure is that the guard is not running when the store is + /// created, not that any particular path is wrong. + /// + /// + /// The throwaway expander service collection. + /// Application configuration. + /// The host's content root, from . + /// The same collection, for chaining. + public static IServiceCollection AddPreHostSqliteExpander( + this IServiceCollection services, + IConfiguration config, + string contentRoot) + { + ArgumentNullException.ThrowIfNull(services); + ArgumentNullException.ThrowIfNull(config); + ArgumentException.ThrowIfNullOrWhiteSpace(contentRoot); + + // Do NOT drop the third argument. The 3-argument overload still compiles and silently + // reopens the gap described above; the regression would arrive disguised as removing a + // redundant parameter. + return services.AddZbSecrets(config, SecretsSectionPath, contentRoot); + } + /// /// The one definition of "a Central node in Grpc mode has a usable shared-store connection /// string", shared by and the Layer-A expander in diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.Central.json b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.Central.json index b5bc7e2c..b5c25a7d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.Central.json +++ b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.Central.json @@ -1,6 +1,6 @@ { "ScadaBridge": { - "_secretsClusterPosture": "Central pair KEK/store posture (production): see docs/operations/2026-07-16-secrets-clustered-master-key.md. Committed default is Source=Environment + relative SqlitePath (dev-safe); production uses a File KEK + shared store volume delivered out-of-band. Do NOT hardcode /shared paths here — the pre-host expander migrates the store at every Central boot and would break dev/test.", + "_secretsClusterPosture": "Central pair KEK/store posture (production): see docs/operations/2026-07-16-secrets-clustered-master-key.md. Committed default is Source=Environment with SqlitePath UNSET (Secrets 0.6.2 rejects a relative path, and a path inside the content root, at startup — so the library's absolute per-user default is taken in dev); production uses a File KEK + shared store volume delivered out-of-band. Do NOT hardcode /shared paths here — the pre-host expander migrates the store at every Central boot and would break dev/test.", "_nodeName": "Host-018: NodeName stamps SourceNode on AuditLog/Notifications/SiteCalls rows (CLAUDE.md 'Centralized Audit Log' decision) and backs IX_AuditLog_Node_Occurred. Convention: 'central-a'/'central-b' for central nodes, 'node-a'/'node-b' for site nodes. Override per-node in multi-node deployments (the docker per-node configs do this). When left at the default below, single-node dev rows are stamped with 'central-a'; an empty value normalises to a NULL SourceNode.", "Node": { "Role": "Central", diff --git a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json index dd0c4a47..7fcb8ace 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json +++ b/src/ZB.MOM.WW.ScadaBridge.Host/appsettings.json @@ -10,7 +10,7 @@ } }, "Secrets": { - "SqlitePath": "scadabridge-secrets.db", + "_sqlitePath": "Deliberately unset. ZB.MOM.WW.Secrets 0.6.2 validates Secrets:SqlitePath at startup — it must be ABSOLUTE and must NOT sit inside the content root, because a path inside the deployment directory is destroyed by an ordinary upgrade (this is what wiped the MxGateway API-key store on 2026-08-09). The former value here ('scadabridge-secrets.db') was relative and would now fail the boot. Leaving it unset gives dev the absolute per-user LocalApplicationData default; every real deployment supplies its own absolute path via its environment overlay.", "MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" }, "RunMigrationsOnStartup": true, "ResolveCacheTtl": "00:00:30", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/PreHostSecretsContentRootTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/PreHostSecretsContentRootTests.cs new file mode 100644 index 00000000..1f58fb2c --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/PreHostSecretsContentRootTests.cs @@ -0,0 +1,120 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Host; +using ZB.MOM.WW.Secrets.Sqlite; + +namespace ZB.MOM.WW.ScadaBridge.Host.Tests; + +/// +/// Guards the content-root argument on the Layer-A pre-host ${secret:} expander +/// (, called from Program.cs). +/// +/// +/// The argument is load-bearing and its absence is SILENT. The expander composes secrets into a +/// throwaway with no IHostEnvironment; without an explicit +/// content root the under-content-root rule does not run, the migrator creates the store at the +/// rejected path, and the boot then fails once the real host registers IHostEnvironment. +/// The leftover empty database is the "the file is there, it's just empty" artifact that made the +/// 2026-08-09 MxGateway outage read as corruption rather than as a deployment error. +/// +/// +/// +/// These tests must resolve and run . Resolving +/// only the connection factory would make every file assertion here vacuous — that constructor +/// never touches the filesystem, so no store is created whether the guard runs or not, and the +/// tests would pass for the wrong reason. +/// exists to prove the harness really does create files. +/// +/// +/// +/// Assertion order is deliberate: artifact first, outcome second. Asserting "it threw" +/// first masks the real finding — a regression that stops the throw but still creates the store +/// would report "no exception was thrown" and never mention the database sitting at the rejected +/// path, which IS the defect. Verified by simulating the regression (dropping to the 3-argument +/// overload) and confirming these fail on the FILE assertion, not the exception one. +/// +/// +public class PreHostSecretsContentRootTests +{ + private static string NewDir() => + Path.Combine(Path.GetTempPath(), "prehost-secrets-" + Guid.NewGuid().ToString("N")); + + private static IConfiguration Config(string sqlitePath) => + new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["Secrets:SqlitePath"] = sqlitePath, + ["Secrets:MasterKey:Source"] = "Environment", + ["Secrets:MasterKey:EnvVarName"] = "ZB_SECRETS_MASTER_KEY", + }).Build(); + + /// + /// Mimics the pre-host expander exactly: throwaway collection, no IHostEnvironment, + /// register, then run the migrator — which is what creates the database. + /// + private static async Task ComposeAndMigrate(string contentRoot, string sqlitePath) + { + Directory.CreateDirectory(contentRoot); + var services = new ServiceCollection(); + services.AddLogging(); + + return await Record.ExceptionAsync(async () => + { + services.AddPreHostSqliteExpander(Config(sqlitePath), contentRoot); + await using var provider = services.BuildServiceProvider(); + await provider.GetRequiredService().MigrateAsync(default); + }); + } + + private static void AssertNoStoreAt(string path) + { + string[] leftovers = new[] { path, path + "-wal", path + "-shm" } + .Where(File.Exists) + .ToArray(); + + Assert.True( + leftovers.Length == 0, + "A rejected path must not be left holding a store file — an empty database at the " + + "rejected path is exactly the artifact this guard exists to prevent. Found: " + + string.Join(", ", leftovers)); + } + + [Fact] + public async Task Control_ValidPath_ActuallyCreatesAStore() + { + // Proves the harness is capable of creating a store, so that the "no file" assertions + // in the other two tests are meaningful rather than vacuous. + var storeDir = NewDir(); + Directory.CreateDirectory(storeDir); + var path = Path.Combine(storeDir, "secrets.db"); + + var thrown = await ComposeAndMigrate(NewDir(), path); + + Assert.Null(thrown); + Assert.True(File.Exists(path), "the control must create a store, or the guards prove nothing"); + } + + [Fact] + public async Task PathInsideContentRoot_IsRejected_AndLeavesNoStore() + { + var contentRoot = NewDir(); + var path = Path.Combine(contentRoot, "data", "scadabridge-secrets.db"); + + var thrown = await ComposeAndMigrate(contentRoot, path); + + AssertNoStoreAt(path); + Assert.NotNull(thrown); + } + + [Fact] + public async Task RelativePath_IsRejected_AndLeavesNoStore() + { + // Unique name: a stale scadabridge-secrets.db from an earlier run under the old relative + // default sits in the test output directory and would make this assertion lie. + var path = "prehost-probe-" + Guid.NewGuid().ToString("N") + ".db"; + + var thrown = await ComposeAndMigrate(NewDir(), path); + + AssertNoStoreAt(path); + Assert.NotNull(thrown); + } +}