c5e894d9b3
Suggested by the OtOpcUa session, the only real containerized deployment in this family and the one that hit the false positive. A store on a volume mounted inside the content root is durable across container replacement but is rejected anyway, because the rule cannot tell a mount from an ordinary directory. The message now says so, gives the fix, and forecloses the wrong one by stating that no override exists. A false positive's entire cost is the confusion in the moment, so an error that names its own fix costs nothing on the true-positive path and turns the false positive into a two-minute edit. Message only, no behaviour change. Their case for keeping the rule strict is recorded in the CHANGELOG because it is better than the trade-off I had written: the workaround is an improvement rather than a concession, an override flag has the lifecycle of a knob set once and never revisited, and mount detection would make the verdict depend on runtime platform state so the same path would pass on one host and fail on another.
365 lines
17 KiB
C#
365 lines
17 KiB
C#
using Microsoft.Extensions.Configuration;
|
|
using Microsoft.Extensions.FileProviders;
|
|
using Microsoft.Extensions.DependencyInjection;
|
|
using Microsoft.Extensions.Hosting;
|
|
using Microsoft.Extensions.Options;
|
|
using ZB.MOM.WW.Configuration;
|
|
using ZB.MOM.WW.Secrets.Configuration;
|
|
using ZB.MOM.WW.Secrets.DependencyInjection;
|
|
using ZB.MOM.WW.Secrets.Sqlite;
|
|
|
|
namespace ZB.MOM.WW.Secrets.Tests.Configuration;
|
|
|
|
/// <summary>
|
|
/// Covers the two store-path rules and the validator that applies them. Both rules exist because
|
|
/// of production incidents — a relative path landing in the process working directory, and an
|
|
/// absolute path landing inside the directory a deployment renames away — so the cases here are
|
|
/// written as those failures, not as abstract path arithmetic.
|
|
/// </summary>
|
|
public sealed class SecretsStorePathRulesTests
|
|
{
|
|
private static string Root(params string[] segments) =>
|
|
Path.Combine(OperatingSystem.IsWindows() ? @"C:\zbtest" : "/zbtest", Path.Combine(segments));
|
|
|
|
// ---------- AddIfNotRooted ----------
|
|
|
|
[Fact]
|
|
public void NotRooted_rejects_a_relative_path()
|
|
{
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfNotRooted("secrets.db", "relative", builder);
|
|
Assert.Contains("relative", builder.Failures);
|
|
}
|
|
|
|
[Fact]
|
|
public void NotRooted_accepts_an_absolute_path()
|
|
{
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfNotRooted(Root("data", "secrets.db"), "relative", builder);
|
|
Assert.True(builder.IsValid);
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData(null)]
|
|
[InlineData("")]
|
|
[InlineData(" ")]
|
|
public void NotRooted_leaves_blank_to_the_required_check(string? value)
|
|
{
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfNotRooted(value, "relative", builder);
|
|
Assert.True(builder.IsValid);
|
|
}
|
|
|
|
// ---------- AddIfUnderContentRoot ----------
|
|
|
|
[Fact]
|
|
public void UnderContentRoot_rejects_a_store_inside_the_application_directory()
|
|
{
|
|
// The 2026-08-09 shape: absolute, and inside the directory the deploy renames away.
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfUnderContentRoot(
|
|
Root("app", "Server", "secrets.db"), Root("app", "Server"), "inside", builder);
|
|
Assert.Contains("inside", builder.Failures);
|
|
}
|
|
|
|
[Fact]
|
|
public void UnderContentRoot_accepts_a_sibling_directory()
|
|
{
|
|
// The fix that was actually applied on the box: a sibling data\ directory.
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfUnderContentRoot(
|
|
Root("app", "data", "secrets.db"), Root("app", "Server"), "inside", builder);
|
|
Assert.True(builder.IsValid);
|
|
}
|
|
|
|
[Fact]
|
|
public void UnderContentRoot_does_not_treat_a_name_prefixed_sibling_as_inside()
|
|
{
|
|
// "/zbtest/app-data" merely STARTS WITH "/zbtest/app". Without a separator before the
|
|
// prefix test this is reported as inside the content root, and because the rule is
|
|
// fail-closed at startup that false positive is an application that will not boot.
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfUnderContentRoot(
|
|
Root("app-data", "secrets.db"), Root("app"), "inside", builder);
|
|
Assert.True(builder.IsValid);
|
|
}
|
|
|
|
[Fact]
|
|
public void UnderContentRoot_catches_a_path_that_only_escapes_syntactically()
|
|
{
|
|
// Normalisation matters: this resolves back inside the content root despite the "..".
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfUnderContentRoot(
|
|
Path.Combine(Root("app", "Server"), "..", "Server", "secrets.db"),
|
|
Root("app", "Server"),
|
|
"inside",
|
|
builder);
|
|
Assert.Contains("inside", builder.Failures);
|
|
}
|
|
|
|
[Fact]
|
|
public void UnderContentRoot_skips_when_the_content_root_is_unknown()
|
|
{
|
|
// Composed outside a host (the CLI does this). A rule about a directory the process does
|
|
// not know it has must not become a boot failure.
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfUnderContentRoot(Root("anywhere", "secrets.db"), null, "inside", builder);
|
|
Assert.True(builder.IsValid);
|
|
}
|
|
|
|
// ---------- the default ----------
|
|
|
|
[Fact]
|
|
public void The_default_store_path_satisfies_the_rules_it_ships_with()
|
|
{
|
|
// A library whose own default violates its own guard would fail every consumer's first
|
|
// boot after upgrading.
|
|
var builder = new ValidationBuilder();
|
|
SecretsStorePathRules.AddIfNotRooted(SecretsOptions.DefaultSqlitePath, "relative", builder);
|
|
Assert.True(builder.IsValid, $"default was '{SecretsOptions.DefaultSqlitePath}'");
|
|
}
|
|
|
|
[Fact]
|
|
public void The_default_store_path_is_not_machine_wide()
|
|
{
|
|
// Deliberately per-user: CommonApplicationData is /usr/share on Linux and macOS, which a
|
|
// normal user cannot write, so a machine-wide default would fail every dev boot in the
|
|
// family. Asserted rather than commented because it is the whole reason for the choice.
|
|
string commonAppData = Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData);
|
|
if (!string.IsNullOrEmpty(commonAppData))
|
|
{
|
|
Assert.DoesNotContain(commonAppData, SecretsOptions.DefaultSqlitePath, StringComparison.Ordinal);
|
|
}
|
|
}
|
|
|
|
// ---------- the validator ----------
|
|
|
|
private static SecretsOptions OptionsWith(string path) => new() { SqlitePath = path };
|
|
|
|
[Fact]
|
|
public void Validator_rejects_a_relative_path_and_names_the_key()
|
|
{
|
|
var validator = new SecretsOptionsValidator(hostEnvironment: null);
|
|
ValidateOptionsResult result = validator.Validate(name: null, OptionsWith("secrets.db"));
|
|
|
|
Assert.True(result.Failed);
|
|
Assert.Contains(result.Failures!, f => f.Contains("Secrets:SqlitePath", StringComparison.Ordinal));
|
|
}
|
|
|
|
[Fact]
|
|
public void Validator_rejects_a_store_under_the_content_root()
|
|
{
|
|
var env = new StubHostEnvironment { ContentRootPath = Root("app", "Server") };
|
|
var validator = new SecretsOptionsValidator(env);
|
|
|
|
ValidateOptionsResult result =
|
|
validator.Validate(name: null, OptionsWith(Root("app", "Server", "secrets.db")));
|
|
|
|
Assert.True(result.Failed);
|
|
Assert.Contains(result.Failures!, f => f.Contains("content root", StringComparison.Ordinal));
|
|
}
|
|
|
|
[Fact]
|
|
public void The_rejection_message_teaches_the_container_case()
|
|
{
|
|
// A false positive's entire cost is the confusion in the moment, and the known one is a
|
|
// store on a volume mounted inside the content root — durable in a container, rejected
|
|
// anyway, because the rule cannot tell a mount from an ordinary directory. A boot error
|
|
// that names its own fix costs nothing on the true-positive path and turns that false
|
|
// positive into a two-minute edit. Asserted rather than left to the doc comment, because
|
|
// the message is the only part of this a stuck operator will actually read.
|
|
var env = new StubHostEnvironment { ContentRootPath = Root("app") };
|
|
var validator = new SecretsOptionsValidator(env);
|
|
|
|
ValidateOptionsResult result =
|
|
validator.Validate(name: null, OptionsWith(Root("app", "data", "secrets.db")));
|
|
|
|
Assert.True(result.Failed);
|
|
string message = Assert.Single(result.Failures!);
|
|
Assert.Contains("mounted volume", message, StringComparison.OrdinalIgnoreCase);
|
|
Assert.Contains("OUTSIDE the content root", message, StringComparison.Ordinal);
|
|
// And that it forecloses the wrong fix, rather than leaving the reader to hunt for a flag.
|
|
Assert.Contains("not look for an override", message, StringComparison.OrdinalIgnoreCase);
|
|
}
|
|
|
|
[Fact]
|
|
public void Validator_accepts_an_absolute_path_outside_the_content_root()
|
|
{
|
|
var env = new StubHostEnvironment { ContentRootPath = Root("app", "Server") };
|
|
var validator = new SecretsOptionsValidator(env);
|
|
|
|
ValidateOptionsResult result =
|
|
validator.Validate(name: null, OptionsWith(Root("app", "data", "secrets.db")));
|
|
|
|
Assert.True(result.Succeeded);
|
|
}
|
|
|
|
// ---------- registration ----------
|
|
|
|
[Fact]
|
|
public void AddZbSecrets_is_idempotent_and_registers_exactly_one_validator()
|
|
{
|
|
// AddZbSecrets is called twice in real compositions — a replication package calls it after
|
|
// the application already has, and one consumer calls it twice directly. An earlier
|
|
// implementation used TryAddEnumerable with a factory descriptor, which THROWS on the
|
|
// second call ("indistinguishable from other services") rather than skipping.
|
|
IConfiguration config = new ConfigurationBuilder()
|
|
.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["Secrets:SqlitePath"] = Root("app", "data", "secrets.db"),
|
|
})
|
|
.Build();
|
|
|
|
var services = new ServiceCollection();
|
|
services.AddZbSecrets(config, "Secrets");
|
|
services.AddZbSecrets(config, "Secrets");
|
|
|
|
Assert.Single(services.Where(d => d.ServiceType == typeof(SecretsOptionsValidator)));
|
|
Assert.Single(services.Where(d => d.ServiceType == typeof(IValidateOptions<SecretsOptions>)));
|
|
}
|
|
|
|
[Fact]
|
|
public void A_bad_path_actually_fails_when_the_options_are_resolved()
|
|
{
|
|
// The rules and the validator can both be correct while the REGISTRATION is wrong, and a
|
|
// validator that is never invoked is precisely the "guard that does not fire" failure this
|
|
// whole change exists to prevent. So resolve the options for real and require the throw.
|
|
IConfiguration config = new ConfigurationBuilder()
|
|
.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["Secrets:SqlitePath"] = "secrets.db",
|
|
})
|
|
.Build();
|
|
|
|
var services = new ServiceCollection();
|
|
services.AddZbSecrets(config, "Secrets");
|
|
using ServiceProvider provider = services.BuildServiceProvider();
|
|
|
|
var ex = Assert.Throws<OptionsValidationException>(
|
|
() => _ = provider.GetRequiredService<IOptions<SecretsOptions>>().Value);
|
|
Assert.Contains(ex.Failures, f => f.Contains("Secrets:SqlitePath", StringComparison.Ordinal));
|
|
}
|
|
|
|
[Fact]
|
|
public void A_good_path_resolves_cleanly()
|
|
{
|
|
// Positive control: without it, the test above would still pass if EVERY path were rejected.
|
|
IConfiguration config = new ConfigurationBuilder()
|
|
.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["Secrets:SqlitePath"] = Root("app", "data", "secrets.db"),
|
|
})
|
|
.Build();
|
|
|
|
var services = new ServiceCollection();
|
|
services.AddZbSecrets(config, "Secrets");
|
|
using ServiceProvider provider = services.BuildServiceProvider();
|
|
|
|
Assert.Equal(
|
|
Root("app", "data", "secrets.db"),
|
|
provider.GetRequiredService<IOptions<SecretsOptions>>().Value.SqlitePath);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task A_pre_host_container_given_the_content_root_refuses_before_the_store_is_created()
|
|
{
|
|
// REGRESSION (found by the HistorianGateway adoption of 0.6.0). An application that expands
|
|
// ${secret:} references before building its host composes this subsystem in a throwaway
|
|
// ServiceCollection with NO IHostEnvironment, then migrates the store from that container.
|
|
// The content-root rule was skipped there — so an absolute in-content-root path passed the
|
|
// only rule still applying, the migrator CREATED the database, and the boot then failed a
|
|
// second later once the real host registered IHostEnvironment. That left an empty store at
|
|
// the rejected path: the exact artifact that made the outage this guard exists to prevent
|
|
// read as "the database is there, it's just empty".
|
|
string contentRoot = Path.Combine(Path.GetTempPath(), $"zb-prehost-{Guid.NewGuid():N}");
|
|
Directory.CreateDirectory(contentRoot);
|
|
string dbPath = Path.Combine(contentRoot, "store.db");
|
|
|
|
try
|
|
{
|
|
IConfiguration config = new ConfigurationBuilder()
|
|
.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["Secrets:SqlitePath"] = dbPath,
|
|
})
|
|
.Build();
|
|
|
|
var services = new ServiceCollection();
|
|
services.AddZbSecrets(config, "Secrets", contentRoot); // the content root, explicitly
|
|
using ServiceProvider provider = services.BuildServiceProvider();
|
|
|
|
// Mirror what a pre-host expander actually does: resolve the MIGRATOR and run it.
|
|
// Resolving the connection factory alone proves nothing here — its constructor never
|
|
// touches the filesystem, so a "no file created" assertion around it passes whether
|
|
// the guard works or not. The migration is what creates the database.
|
|
Exception? thrown = await Record.ExceptionAsync(
|
|
() => provider.GetRequiredService<SqliteSecretsStoreMigrator>().MigrateAsync(default));
|
|
|
|
// ORDER MATTERS, and the obvious order is wrong. Asserting the throw FIRST would mask
|
|
// the stronger finding: a regression that stops the throw but still creates the store
|
|
// fails with "no exception was thrown" and never mentions the database sitting at the
|
|
// rejected path — which IS the defect. "It threw" is the weaker claim, so it is
|
|
// asserted last. (The defect this test guards was itself "a boot outcome was observed
|
|
// instead of the artifact being probed", so closing it with an outcome-first assertion
|
|
// would have repeated the mistake.)
|
|
Assert.False(File.Exists(dbPath), "a rejected path must not be left holding a store file");
|
|
Assert.False(File.Exists(dbPath + "-wal"));
|
|
Assert.False(File.Exists(dbPath + "-shm"));
|
|
Assert.IsType<OptionsValidationException>(thrown);
|
|
}
|
|
finally
|
|
{
|
|
Directory.Delete(contentRoot, recursive: true);
|
|
}
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Without_a_content_root_the_pre_host_container_still_cannot_be_told_it_is_fine()
|
|
{
|
|
// Positive control for the test above: the SAME path, with the content root NOT supplied,
|
|
// is the pre-0.6.1 behaviour — the rule is skipped and nothing objects. This is what makes
|
|
// the assertion above about the explicit content root rather than about paths in general.
|
|
string contentRoot = Path.Combine(Path.GetTempPath(), $"zb-prehost-{Guid.NewGuid():N}");
|
|
Directory.CreateDirectory(contentRoot);
|
|
string dbPath = Path.Combine(contentRoot, "store.db");
|
|
|
|
try
|
|
{
|
|
IConfiguration config = new ConfigurationBuilder()
|
|
.AddInMemoryCollection(new Dictionary<string, string?>
|
|
{
|
|
["Secrets:SqlitePath"] = dbPath,
|
|
})
|
|
.Build();
|
|
|
|
var services = new ServiceCollection();
|
|
services.AddZbSecrets(config, "Secrets"); // no content root — the CLI's situation
|
|
using ServiceProvider provider = services.BuildServiceProvider();
|
|
|
|
// Absolute, so the rooted rule passes; the content-root rule cannot fire. Documented,
|
|
// and the reason pre-host composition must pass the content root explicitly.
|
|
await provider.GetRequiredService<SqliteSecretsStoreMigrator>().MigrateAsync(default);
|
|
|
|
// Assert the database IS created here, not merely that nothing threw. This is what
|
|
// keeps the REJECTING test honest: "no file was created" is indistinguishable from
|
|
// "nothing in this container ever creates files" unless something proves the write
|
|
// path is live. An Assert.NotNull on a resolved factory proves neither — and if the
|
|
// pre-host container ever stopped touching the store, it would silently void the
|
|
// rejecting test while continuing to pass itself.
|
|
Assert.True(File.Exists(dbPath), "the accepted path must actually get a store file");
|
|
}
|
|
finally
|
|
{
|
|
Directory.Delete(contentRoot, recursive: true);
|
|
}
|
|
}
|
|
|
|
private sealed class StubHostEnvironment : IHostEnvironment
|
|
{
|
|
public string EnvironmentName { get; set; } = "Production";
|
|
public string ApplicationName { get; set; } = "Test";
|
|
public string ContentRootPath { get; set; } = string.Empty;
|
|
public IFileProvider ContentRootFileProvider { get; set; } = null!;
|
|
}
|
|
}
|