Compare commits

..

2 Commits

Author SHA1 Message Date
Joseph Doherty 5fdd8a570a fix(secrets): run the store-path guard in the pre-host container (0.6.1)
ci / windows-x86 (push) Successful in 1m19s
ci / nightly-windev (push) Has been skipped
ci / java (push) Successful in 2m59s
ci / portable (push) Successful in 10m48s
0.6.0 put the store-path rules in the shared library, but the guard was not
running at the moment that matters here.

CreateBuilder resolves ${secret:} references before the host exists, using a
throwaway ServiceCollection that contains no IHostEnvironment — and it runs the
store migrator, which creates the database. The library resolved the content
root from IHostEnvironment alone, so it could not distinguish "no content root"
from "no host registered" and skipped the under-content-root rule entirely. The
store was created at the rejected path; the boot then failed a moment later when
the real host validated. The leftover empty database with its -wal/-shm siblings
is exactly the artifact that made the 2026-08-09 credential loss read as "the
database is there, it's just empty".

The pin alone does not close this. An app with a correctly configured path shows
no symptom and is still unprotected, because the guard simply is not running when
the store is created. 0.6.1 adds a 4-argument AddZbSecrets overload taking the
content root explicitly, and the call site has to use it. The in-host
registration below needs nothing.

Verified by removing the fix rather than by observing a clean boot — which is how
this survived its first release. With the 3-argument overload the new test fails
by finding a created database at
src/ZB.MOM.WW.MxGateway.Server/probe-secrets-*.db: inside the source tree, since
that is what the content root resolves to under test.

Two things about the test itself, both of which it would have been easy to get
subtly wrong:

It asserts no-file-created before asserting that startup threw. "It threw" is the
weaker claim, and asserting it first masks the stronger one — the run that proved
this defect would have reported "no exception was thrown" and said nothing about
the database sitting in the source tree.

The accepting case asserts the database *is* created, not merely that nothing
threw. A not-null builder is close to a tautology once no exception escaped, and
it would still pass if the pre-host container stopped opening the store at all —
which would also quietly void the rejecting case, since that one can only observe
a file the migration would otherwise have written. The two assertions hold each
other up.

Found by HistorianGateway's adoption, which probed the rejected paths instead of
observing a successful boot.
2026-08-11 08:54:06 -04:00
Joseph Doherty 9bc70d1af3 Merge feat/session-health-check-and-store-path-guard: session health probe + store-path guard + Secrets 0.6.0
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m13s
ci / java (push) Successful in 2m23s
ci / portable (push) Successful in 17m53s
Three separable changes, each building on its own: the mxaccess-sessions health
check on the active tier, the content-root rule that closes the gap the 2026-08-09
credential-store loss went through, and the Secrets re-pin (0.2.3 -> 0.6.0) that
moves the same rules into the shared library.
2026-08-11 08:43:43 -04:00
3 changed files with 125 additions and 4 deletions
@@ -78,9 +78,19 @@ public static class GatewayApplication
// here (SecretNotFoundException); config with no tokens is untouched (no-op), so this is safe
// to always run. CreateBuilder is synchronous and single-shot at bootstrap, so the two awaits
// are driven via GetAwaiter().GetResult() (no sync-context deadlock risk during host startup).
// The content root is passed explicitly because this container is a throwaway
// ServiceCollection with no IHostEnvironment in it. Without it the library cannot tell
// "no content root exists" from "no host is registered", so it skips the
// under-content-root rule — and the migrator below CREATES the store before the real host
// ever validates. The boot then fails a moment later, having already left an empty
// database with its -wal/-shm siblings at the very path the rule rejects. That artifact is
// what made the 2026-08-09 outage read as "the database is there, it's just empty".
// DO NOT simplify this to the 3-argument overload: it still compiles, the app still boots
// when the path is correct, and the guard silently stops running at the one moment that
// matters.
#pragma warning disable ASP0000 // deliberate throwaway container, disposed here, shares no singletons
using (var secretsProvider = new ServiceCollection()
.AddZbSecrets(builder.Configuration, "Secrets")
.AddZbSecrets(builder.Configuration, "Secrets", builder.Environment.ContentRootPath)
.BuildServiceProvider())
#pragma warning restore ASP0000
{
@@ -21,9 +21,9 @@
<PackageReference Include="ZB.MOM.WW.Telemetry" Version="0.1.0" />
<PackageReference Include="ZB.MOM.WW.Telemetry.Serilog" Version="0.1.0" />
<PackageReference Include="ZB.MOM.WW.GalaxyRepository" Version="0.2.0" />
<PackageReference Include="ZB.MOM.WW.Secrets" Version="0.6.0" />
<PackageReference Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.6.0" />
<PackageReference Include="ZB.MOM.WW.Secrets.Ui" Version="0.6.0" />
<PackageReference Include="ZB.MOM.WW.Secrets" Version="0.6.1" />
<PackageReference Include="ZB.MOM.WW.Secrets.Abstractions" Version="0.6.1" />
<PackageReference Include="ZB.MOM.WW.Secrets.Ui" Version="0.6.1" />
<PackageReference Include="Serilog.AspNetCore" Version="10.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="6.1.1" />
<PackageReference Include="Serilog.Sinks.File" Version="7.0.0" />
@@ -0,0 +1,111 @@
using Microsoft.AspNetCore.Builder;
using ZB.MOM.WW.MxGateway.Server;
namespace ZB.MOM.WW.MxGateway.Tests.Configuration;
// Mutates the process-global Secrets__SqlitePath that GatewayApplication.CreateBuilder reads;
// serialized against every other collection so a parallel host-building test cannot inherit the
// deliberately-rejected path. See GlobalEnvironmentCollection.
[Collection(TestSupport.GlobalEnvironmentCollection.Name)]
public sealed class SecretsStorePathGuardTests
{
private const string SqlitePathVariable = "Secrets__SqlitePath";
/// <summary>
/// Verifies the store-path guard runs in the <em>pre-host</em> secrets container, which is the
/// only place it matters.
/// </summary>
/// <remarks>
/// <para>
/// <c>CreateBuilder</c> resolves <c>${secret:}</c> references before the host exists, using a
/// throwaway <see cref="Microsoft.Extensions.DependencyInjection.ServiceCollection"/> that
/// contains no <c>IHostEnvironment</c> — and it runs the store migrator, which <b>creates the
/// database</b>. A library that infers the content root from <c>IHostEnvironment</c> alone
/// cannot distinguish "no content root" from "no host registered" and skips the rule here, so
/// the store is created at the rejected path and only then does the real host refuse to start.
/// The leftover empty database with its <c>-wal</c>/<c>-shm</c> siblings is precisely the
/// artifact that made the 2026-08-09 credential loss read as "the database is there, it's just
/// empty". The gateway therefore passes the content root explicitly.
/// </para>
/// <para>
/// The assertion that no file was created is the load-bearing one. A test that merely observed
/// a failed boot would pass even while the store was being written, because the failure arrives
/// afterwards either way — which is exactly how this defect survived its first release.
/// </para>
/// </remarks>
[Fact]
public void CreateBuilder_RejectsSecretsStoreUnderContentRoot_WithoutCreatingIt()
{
string? original = Environment.GetEnvironmentVariable(SqlitePathVariable);
string contentRoot = ResolveContentRoot();
string rejected = Path.Combine(contentRoot, $"probe-secrets-{Guid.NewGuid():N}.db");
try
{
Environment.SetEnvironmentVariable(SqlitePathVariable, rejected);
// Capture rather than Assert.ThrowsAny, so the store-creation assertions below are
// reported first. Ordering matters here: "it threw" is the weaker claim, and asserting
// it first would mask the stronger one — that nothing was written before it threw.
Exception? thrown = Record.Exception(() => GatewayApplication.CreateBuilder([]));
Assert.False(File.Exists(rejected), $"the rejected store was created at {rejected}");
Assert.False(File.Exists(rejected + "-wal"), "a write-ahead log was created for the rejected store");
Assert.False(File.Exists(rejected + "-shm"), "a shared-memory file was created for the rejected store");
Assert.NotNull(thrown);
}
finally
{
Environment.SetEnvironmentVariable(SqlitePathVariable, original);
// Delete defensively: if the guard ever regresses this test writes a database into the
// content root, which on a dev machine is the source tree.
foreach (string leftover in new[] { rejected, rejected + "-wal", rejected + "-shm" })
{
if (File.Exists(leftover))
{
File.Delete(leftover);
}
}
}
}
/// <summary>
/// Verifies a store path outside the content root is accepted <em>and actually used</em>.
/// </summary>
/// <remarks>
/// The assertion is that the database exists afterwards, not merely that nothing threw. A
/// not-null builder is very close to a tautology once no exception escaped, so it would pass
/// even if the pre-host container had stopped opening the store altogether — which would also
/// silently void the negative test above, since that one can only observe a file the migration
/// would otherwise have written. Proving the accepted path gets a real database is what keeps
/// the rejected-path assertion meaningful.
/// </remarks>
[Fact]
public void CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt()
{
string? original = Environment.GetEnvironmentVariable(SqlitePathVariable);
string directory = Directory.CreateTempSubdirectory("mxgw-secrets-ok").FullName;
string accepted = Path.Combine(directory, "secrets.db");
try
{
Environment.SetEnvironmentVariable(SqlitePathVariable, accepted);
WebApplicationBuilder builder = GatewayApplication.CreateBuilder([]);
Assert.NotNull(builder);
Assert.True(File.Exists(accepted), $"the accepted store was not created at {accepted}");
}
finally
{
Environment.SetEnvironmentVariable(SqlitePathVariable, original);
Directory.Delete(directory, recursive: true);
}
}
// The content root CreateBuilder will use, taken from a builder created with the suite's normal
// (valid) store path rather than assumed from the test's working directory.
private static string ResolveContentRoot() =>
GatewayApplication.CreateBuilder([]).Environment.ContentRootPath;
}