chore(secrets): adopt ZB.MOM.WW.Secrets 0.6.2 and close the pre-host guard gap

0.6.x refuses a secret store whose path is relative or inside the content root,
because a store in the deployment directory is destroyed by an ordinary upgrade —
the failure that wiped the MxGateway API-key store on 2026-08-09 and read as an
auth outage rather than a deployment error.

The pin alone would not have protected this repo. Program.cs expands ${secret:}
before the host exists, composing secrets into a throwaway ServiceCollection with
no IHostEnvironment, so the guard would not run at the moment the migrator creates
the store. That composition now lives in SecretsRegistration with an explicit
content root — resolved to match what the host resolves later, including the
Windows-Service case where the pre-host CWD is still system32 — and is covered by
PreHostSecretsContentRootTests, verified by simulating the regression and
confirming it fails on the leftover file rather than on the exception.

The docker rig needed a fix too: /app/data is absolute but inside the container's
content root, so all 8 nodes would have failed to boot. Each node's data directory
is now mounted a second time at /data; same host directory, so existing stores
carry over untouched.

Verified: build clean, 29 test assemblies green (Playwright's 159 failures are the
pre-existing SEC-36 login baseline). Not yet deployed — the rig runs the old
config until someone redeploys.
This commit is contained in:
Joseph Doherty
2026-08-11 09:16:31 -04:00
parent 3eb7df74eb
commit fdc6b0c2bb
8 changed files with 251 additions and 17 deletions
+5 -5
View File
@@ -104,11 +104,11 @@
<PackageVersion Include="ZB.MOM.WW.Auth.AspNetCore" Version="0.1.5" />
<PackageVersion Include="ZB.MOM.WW.Audit" Version="0.1.0" />
<PackageVersion Include="ZB.MOM.WW.Theme" Version="0.4.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.5.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.5.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.5.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.SqlServer" Version="0.5.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.Grpc" Version="0.5.1" />
<PackageVersion Include="ZB.MOM.WW.Secrets" Version="0.6.2" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.6.2" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Ui" Version="0.6.2" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.SqlServer" Version="0.6.2" />
<PackageVersion Include="ZB.MOM.WW.Secrets.Replicator.Grpc" Version="0.6.2" />
<PackageVersion Include="ZB.MOM.WW.LocalDb" Version="0.1.3" />
<PackageVersion Include="ZB.MOM.WW.LocalDb.Replication" Version="0.1.3" />
<PackageVersion Include="ZB.MOM.WW.LocalDb.Contracts" Version="0.1.3" />
+39 -5
View File
@@ -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
@@ -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<Program>` `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<Program>` `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
+8 -1
View File
@@ -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
@@ -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;
}
/// <summary>
/// Resolves the content root the HOST will use, from outside the host — the Layer-A
/// <c>${secret:}</c> expander in <c>Program.cs</c> runs before any builder exists, so
/// <c>builder.Environment.ContentRootPath</c> is not available to it.
/// </summary>
/// <remarks>
/// Mirrors the framework's own resolution order. The Windows-Service branch is the one that
/// is easy to get wrong: <c>UseWindowsService</c> sets both the current directory and the
/// content root to <see cref="AppContext.BaseDirectory"/>, but only once the host is built —
/// at expander time the process working directory is still <c>C:\Windows\system32</c>, so
/// taking the current directory there would compare the store path against the wrong root.
/// </remarks>
/// <returns>The absolute content-root path the host will resolve.</returns>
public static string ResolveHostContentRoot() =>
Environment.GetEnvironmentVariable("ASPNETCORE_CONTENTROOT")
?? Environment.GetEnvironmentVariable("DOTNET_CONTENTROOT")
?? (WindowsServiceHelpers.IsWindowsService()
? AppContext.BaseDirectory
: Directory.GetCurrentDirectory());
/// <summary>
/// Composes the SQLite secrets stack for the Layer-A pre-host <c>${secret:}</c> expander.
/// </summary>
/// <remarks>
/// <para>
/// This exists as a named method rather than an inline <c>AddZbSecrets</c> call so that the
/// content-root argument is covered by a test (<c>PreHostSecretsContentRootTests</c>). The
/// argument is load-bearing and its absence is SILENT: the expander composes into a throwaway
/// <see cref="IServiceCollection"/> that has no <c>IHostEnvironment</c>, 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 <c>IHostEnvironment</c> — 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.
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
/// <param name="services">The throwaway expander service collection.</param>
/// <param name="config">Application configuration.</param>
/// <param name="contentRoot">The host's content root, from <see cref="ResolveHostContentRoot"/>.</param>
/// <returns>The same collection, for chaining.</returns>
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);
}
/// <summary>
/// The one definition of "a Central node in Grpc mode has a usable shared-store connection
/// string", shared by <see cref="AddScadaBridgeSecrets"/> and the Layer-A expander in
@@ -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",
@@ -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",
@@ -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;
/// <summary>
/// Guards the content-root argument on the Layer-A pre-host <c>${secret:}</c> expander
/// (<see cref="SecretsRegistration.AddPreHostSqliteExpander"/>, called from <c>Program.cs</c>).
///
/// <para>
/// The argument is load-bearing and its absence is SILENT. The expander composes secrets into a
/// throwaway <see cref="ServiceCollection"/> with no <c>IHostEnvironment</c>; 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 <c>IHostEnvironment</c>.
/// 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.
/// </para>
///
/// <para>
/// <b>These tests must resolve and run <see cref="SqliteSecretsStoreMigrator"/>.</b> 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. <see cref="Control_ValidPath_ActuallyCreatesAStore"/>
/// exists to prove the harness really does create files.
/// </para>
///
/// <para>
/// <b>Assertion order is deliberate: artifact first, outcome second.</b> 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.
/// </para>
/// </summary>
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<string, string?>
{
["Secrets:SqlitePath"] = sqlitePath,
["Secrets:MasterKey:Source"] = "Environment",
["Secrets:MasterKey:EnvVarName"] = "ZB_SECRETS_MASTER_KEY",
}).Build();
/// <summary>
/// Mimics the pre-host expander exactly: throwaway collection, no IHostEnvironment,
/// register, then run the migrator — which is what creates the database.
/// </summary>
private static async Task<Exception?> 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<SqliteSecretsStoreMigrator>().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);
}
}