diff --git a/docs/GatewayConfiguration.md b/docs/GatewayConfiguration.md index 7529d09..1dc80a9 100644 --- a/docs/GatewayConfiguration.md +++ b/docs/GatewayConfiguration.md @@ -91,7 +91,7 @@ Environment variables use the normal .NET double-underscore form. For example, | Option | Default | Description | |--------|---------|-------------| | `MxGateway:Authentication:Mode` | `ApiKey` | Selects public gRPC authentication. Supported values are `ApiKey` and `Disabled`. `Disabled` bypasses API-key verification and is for local development only. | -| `MxGateway:Authentication:SqlitePath` | derived from `CommonApplicationData` (`C:\ProgramData\MxGateway\gateway-auth.db` on Windows, `/usr/share/MxGateway/gateway-auth.db` or the container equivalent elsewhere) | SQLite database path for API-key records and audit rows when API-key authentication is enabled. The code default is built from `Environment.GetFolderPath(SpecialFolder.CommonApplicationData)` so the credential store never lands in the launch working directory on a non-Windows host. `appsettings.json` no longer ships an explicit value (SEC-33): the removed Windows literal was byte-identical to the Windows code default, and a Windows-absolute literal is **not** rooted on a Unix host, so it would have resolved against the CWD there. Deployed hosts still override the path through the NSSM environment (`MxGateway__Authentication__SqlitePath`). | +| `MxGateway:Authentication:SqlitePath` | derived from `CommonApplicationData` (`C:\ProgramData\MxGateway\gateway-auth.db` on Windows, `/usr/share/MxGateway/gateway-auth.db` or the container equivalent elsewhere) | SQLite database path for API-key records and audit rows when API-key authentication is enabled. The code default is built from `Environment.GetFolderPath(SpecialFolder.CommonApplicationData)` so the credential store never lands in the launch working directory on a non-Windows host. `appsettings.json` no longer ships an explicit value (SEC-33): the removed Windows literal was byte-identical to the Windows code default, and a Windows-absolute literal is **not** rooted on a Unix host, so it would have resolved against the CWD there. Deployed hosts still override the path through the NSSM environment (`MxGateway__Authentication__SqlitePath`). The validator additionally rejects a path **inside the application content root**, even an absolute one: the upgrade procedure renames that directory to `Server.bak.*`, which takes the credential store with it and silently starts an empty one. That is not hypothetical — it happened on a production host on 2026-08-09 and no gRPC client could authenticate for two days. | | `MxGateway:Authentication:PepperSecretName` | `MxGateway:ApiKeyPepper` | Configuration key used to read the HMAC pepper for API-key secret hashing. The dashboard effective configuration redacts this value. | | `MxGateway:Authentication:RunMigrationsOnStartup` | `true` | Runs SQLite auth schema migrations at gateway startup when API-key authentication is enabled. | @@ -291,7 +291,7 @@ section (a sibling of `MxGateway`, not nested under it): | Option | Default | Description | |--------|---------|-------------| -| `Secrets:SqlitePath` | `mxgateway-secrets.db` | Path to the encrypted secrets store, resolved relative to the app content root when not rooted. | +| `Secrets:SqlitePath` | `/MxGateway/mxgateway-secrets.db` | Path to the encrypted secrets store. The default is supplied in code when the key is unset (`C:\ProgramData\MxGateway\...` on Windows), not from `appsettings.json` — a store inside the application directory is renamed away by the upgrade procedure, taking the secrets with it. On non-Windows hosts the default location is usually not writable by a normal user, so a local run must set `Secrets__SqlitePath` explicitly. | | `Secrets:MasterKey:Source` | `Environment` | Key-encryption-key (KEK) provider. `Environment` reads a base64-encoded 32-byte key from an env var; `Dpapi` uses a machine-bound key file instead (see below). | | `Secrets:MasterKey:EnvVarName` | `ZB_SECRETS_MASTER_KEY` | Env var name the `Environment` provider reads the KEK from. | @@ -402,7 +402,7 @@ model requires otherwise. | `MxGateway:Galaxy:CommandTimeoutSeconds` | `60` | Per-command SQL timeout for all Galaxy browse RPCs. | | `MxGateway:Galaxy:DashboardRefreshIntervalSeconds` | `30` | Interval between background refreshes of the dashboard Galaxy summary cache. SQL is hit at most once per interval regardless of dashboard render rate. | | `MxGateway:Galaxy:PersistSnapshot` | `true` | Persists the latest successful Galaxy browse dataset to disk. When `true`, the cache reloads that snapshot at startup so clients can still browse last-known data while the Galaxy database is unreachable. The restored data is served with `Stale` status until a live query confirms it. | -| `MxGateway:Galaxy:SnapshotCachePath` | derived from `CommonApplicationData` (`C:\ProgramData\MxGateway\galaxy-snapshot.json` on Windows, `/usr/share/MxGateway/galaxy-snapshot.json` or the container equivalent elsewhere) | File path for the persisted Galaxy browse snapshot. Ignored when `PersistSnapshot` is `false`. The snapshot is written atomically (temp file plus rename). `appsettings.json` no longer ships an explicit value (SEC-33): the option is bound by the shared `ZB.MOM.WW.GalaxyRepository` package, so the gateway supplies the `CommonApplicationData`-derived default when the bound value is blank and registers `GalaxyRepositoryOptionsValidator` to enforce that — when `PersistSnapshot` is `true` — the path is non-blank, valid, and **rooted on the host running the gateway** (`Path.IsPathRooted`, current OS). A bare filename or a foreign-platform literal fails startup instead of resolving against the launch working directory (SEC-01, SEC-33). | +| `MxGateway:Galaxy:SnapshotCachePath` | derived from `CommonApplicationData` (`C:\ProgramData\MxGateway\galaxy-snapshot.json` on Windows, `/usr/share/MxGateway/galaxy-snapshot.json` or the container equivalent elsewhere) | File path for the persisted Galaxy browse snapshot. Ignored when `PersistSnapshot` is `false`. The snapshot is written atomically (temp file plus rename). `appsettings.json` no longer ships an explicit value (SEC-33): the option is bound by the shared `ZB.MOM.WW.GalaxyRepository` package, so the gateway supplies the `CommonApplicationData`-derived default when the bound value is blank and registers `GalaxyRepositoryOptionsValidator` to enforce that — when `PersistSnapshot` is `true` — the path is non-blank, valid, and **rooted on the host running the gateway** (`Path.IsPathRooted`, current OS). A bare filename or a foreign-platform literal fails startup instead of resolving against the launch working directory (SEC-01, SEC-33). The same validator also rejects a path **inside the application content root**, because the upgrade procedure renames that directory away and the cached snapshot would be discarded on every deploy. | See [Galaxy Repository Browse](./GalaxyRepository.md) for the RPC surface and behavior. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GalaxyRepositoryOptionsValidator.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GalaxyRepositoryOptionsValidator.cs index 15a035e..feb632a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GalaxyRepositoryOptionsValidator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GalaxyRepositoryOptionsValidator.cs @@ -13,6 +13,33 @@ namespace ZB.MOM.WW.MxGateway.Server.Configuration; /// public sealed class GalaxyRepositoryOptionsValidator : OptionsValidatorBase { + // See GatewayOptionsValidator for why this is nullable and what null means. + private readonly string? _contentRootPath; + + /// + /// Initializes a new instance of the class for + /// the dependency-injection path, taking the content root from the host environment. + /// + /// The host environment. + public GalaxyRepositoryOptionsValidator(IHostEnvironment environment) + { + ArgumentNullException.ThrowIfNull(environment); + _contentRootPath = environment.ContentRootPath; + } + + /// + /// Initializes a new instance of the class for + /// unit tests and non-DI callers. + /// + /// + /// Content root to test the snapshot path against; leaves the + /// content-root rule inactive. + /// + internal GalaxyRepositoryOptionsValidator(string? contentRootPath = null) + { + _contentRootPath = contentRootPath; + } + /// protected override void Validate(ValidationBuilder builder, GalaxyRepositoryOptions options) { @@ -37,5 +64,10 @@ public sealed class GalaxyRepositoryOptionsValidator : OptionsValidatorBase + /// Fails validation when resolves to a location inside + /// — the directory the application runs from. + /// + /// + /// + /// Rooted is not the same as safe, and this is the rule that closes the gap. + /// stops a store drifting with the working directory, but an + /// absolute path inside the app directory passes it cleanly — and that is what failed + /// in production on 2026-08-09. The upgrade procedure renames the app directory to + /// Server.bak.* and unpacks a new one; a store living there is renamed away with it, the + /// process then creates a fresh empty one at the same path, and nothing reports an error. All + /// API keys were lost and no gRPC client could authenticate for two days. The deploy itself was + /// executed correctly — the binaries were the point of the rename, and the store was collateral. + /// + /// + /// The same shape catches the dev-side symptom: a store under the content root lands in the + /// source tree, which is how mxgateway-secrets.db once tripped the repository's + /// tree-hygiene test. + /// + /// + /// Comparison is case-insensitive only on Windows. On a case-insensitive macOS volume this can + /// miss a violation that differs only in case, which is a missed warning in dev; assuming + /// case-insensitivity on Linux would instead reject a legitimate path, and a false startup + /// abort is the worse failure. + /// + /// + /// The configured path value. + /// The application content root to test against. + /// The failure message to record when the value is under the content root. + /// The validation builder accumulating failures. + public static void AddIfUnderContentRoot( + string? value, + string? contentRoot, + string message, + ValidationBuilder builder) + { + if (string.IsNullOrWhiteSpace(value) || string.IsNullOrWhiteSpace(contentRoot)) + { + return; + } + + // A malformed path is AddIfInvalidPath's message to report; staying silent here keeps one + // bad value from producing two failures that say different things about the same mistake. + if (!TryGetFullPath(value, out string fullValue) || !TryGetFullPath(contentRoot, out string fullRoot)) + { + return; + } + + fullRoot = fullRoot.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + + StringComparison comparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + + // The separator is load-bearing: a bare prefix test would also match a sibling directory + // whose name merely starts with the root's ("/srv/app" against "/srv/app-data"). + if (string.Equals(fullValue, fullRoot, comparison) + || fullValue.StartsWith(fullRoot + Path.DirectorySeparatorChar, comparison)) + { + builder.Add(message); + } + } + + private static bool TryGetFullPath(string value, out string fullPath) + { + try + { + fullPath = Path.GetFullPath(value); + return true; + } + catch (ArgumentException) + { + fullPath = string.Empty; + return false; + } + catch (NotSupportedException) + { + fullPath = string.Empty; + return false; + } + catch (PathTooLongException) + { + fullPath = string.Empty; + return false; + } + catch (IOException) + { + fullPath = string.Empty; + return false; + } + } } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs index 2d18295..3732209 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs @@ -15,15 +15,22 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase /// Initializes a new instance of the class for the - /// dependency-injection path, deriving the production posture from the host environment. + /// dependency-injection path, deriving the production posture and content root from the host + /// environment. /// /// The host environment. public GatewayOptionsValidator(IHostEnvironment environment) { ArgumentNullException.ThrowIfNull(environment); _isProduction = environment.IsProduction(); + _contentRootPath = environment.ContentRootPath; } /// @@ -32,15 +39,20 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase to exercise them. /// /// Whether to treat the host as running in Production. - internal GatewayOptionsValidator(bool isProduction = false) + /// + /// Content root to test store paths against; leaves the content-root + /// rule inactive, which is what a caller with no real host wants. + /// + internal GatewayOptionsValidator(bool isProduction = false, string? contentRootPath = null) { _isProduction = isProduction; + _contentRootPath = contentRootPath; } /// protected override void Validate(ValidationBuilder builder, GatewayOptions options) { - ValidateAuthentication(options.Authentication, builder); + ValidateAuthentication(options.Authentication, _contentRootPath, builder); ValidateLdap(options.Ldap, builder, _isProduction); ValidateWorker(options.Worker, builder); ValidateSessions(options.Sessions, builder); @@ -101,7 +113,10 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase GatewayConfigPathRules.AddIfInvalidPath(value, message, builder); + + // Rooted is not the same as safe: an absolute path inside the app directory passes + // AddIfNotRooted and is still renamed away by the upgrade procedure. See + // GatewayConfigPathRules.AddIfUnderContentRoot. + private static void AddIfUnderContentRoot( + string? value, + string? contentRoot, + string message, + ValidationBuilder builder) + => GatewayConfigPathRules.AddIfUnderContentRoot(value, contentRoot, message, builder); } diff --git a/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs b/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs index 49144ee..c9d2cdd 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/GatewayApplication.cs @@ -70,6 +70,8 @@ public static class GatewayApplication }); StaticWebAssetsLoader.UseStaticWebAssets(builder.Environment, builder.Configuration); + ApplyDefaultSecretsStorePath(builder.Configuration); + // Resolve ${secret:...} references in configuration BEFORE any config consumer (TLS, Kestrel, // GatewayOptions/Ldap/Galaxy validators) reads a value, using a standalone secrets provider // (envelope-decrypted via the master key). A token referencing a missing secret fails fast @@ -186,6 +188,60 @@ public static class GatewayApplication }); } + /// + /// Supplies the default location of the encrypted secrets store when nothing configured one. + /// + /// + /// + /// The store used to default to a bare relative mxgateway-secrets.db, which resolves + /// against the working directory and therefore normally lands inside the application directory. + /// That is the shape that lost every API key on a production host: the upgrade procedure renames + /// the application directory away, the store goes with it, and a fresh empty one appears in its + /// place with no error. In development the same default writes a database into the source tree. + /// + /// + /// This sets a default for an unset key; it never relocates a value someone configured. + /// That distinction matters — deliberately + /// rejects bad configured paths rather than quietly moving them, because silently relocating a + /// credential store is worse than a boot error. Choosing where to put a value nobody specified + /// is a different act from overriding one they did. + /// + /// + /// The location mirrors AuthenticationOptions.SqlitePath so both gateway stores sit + /// together, and the mechanism is the one SEC-33 already used for + /// MxGateway:Galaxy:SnapshotCachePath below — same problem, same fix, same file. It also + /// matches what docs/GatewayConfiguration.md already tells operators to + /// pass to the secret CLI — an absolute default also removes the CLI/gateway divergence + /// that a working-directory-relative path can cause. On non-Windows hosts + /// is typically not writable by a + /// normal user, so a local run there must set Secrets__SqlitePath explicitly, exactly as + /// it already must for the auth store. + /// + /// + /// This deliberately differs from the ZB.MOM.WW.Secrets library default, which is + /// -derived so the family's + /// cross-platform apps still boot locally without an override. The gateway keeps + /// CommonApplicationData because it runs as a machine-wide Windows service and its other + /// two stores — the auth database and the Galaxy snapshot — already live there; splitting them + /// would be the greater inconsistency. The value set here always wins, so the library default is + /// unreachable in this app. Do not "fix" the difference by deleting this method: that would + /// silently move the store, which is the failure this whole rule exists to prevent. + /// + /// + /// The configuration to supply the default into. + private static void ApplyDefaultSecretsStorePath(IConfiguration configuration) + { + if (!string.IsNullOrWhiteSpace(configuration["Secrets:SqlitePath"])) + { + return; + } + + configuration["Secrets:SqlitePath"] = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + "MxGateway", + "mxgateway-secrets.db"); + } + private static void ConfigureSelfSignedTls(WebApplicationBuilder builder) { if (!Security.Tls.KestrelTlsInspector.RequiresGeneratedCertificate(builder.Configuration)) diff --git a/src/ZB.MOM.WW.MxGateway.Server/appsettings.json b/src/ZB.MOM.WW.MxGateway.Server/appsettings.json index d650c53..e99f5e0 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/appsettings.json +++ b/src/ZB.MOM.WW.MxGateway.Server/appsettings.json @@ -12,7 +12,6 @@ }, "AllowedHosts": "*", "Secrets": { - "SqlitePath": "mxgateway-secrets.db", "MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" }, "RunMigrationsOnStartup": true, "ResolveCacheTtl": "00:00:30" diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GalaxyRepositoryOptionsValidatorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GalaxyRepositoryOptionsValidatorTests.cs index 204d3bc..d1d4619 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GalaxyRepositoryOptionsValidatorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GalaxyRepositoryOptionsValidatorTests.cs @@ -63,6 +63,49 @@ public sealed class GalaxyRepositoryOptionsValidatorTests Assert.True(result.Succeeded); } + /// + /// Verifies an absolute snapshot path inside the application directory fails. Rooted is not the + /// same as safe: the upgrade procedure renames that directory, so a snapshot cached there is + /// discarded on every deploy and the gateway starts cold each time. + /// + [Fact] + public void Validate_Fails_WhenSnapshotPathIsUnderContentRoot() + { + string contentRoot = Path.Combine(Path.GetTempPath(), $"mxgw-root-{Guid.NewGuid():N}"); + GalaxyRepositoryOptions options = new() + { + PersistSnapshot = true, + SnapshotCachePath = Path.Combine(contentRoot, "galaxy-snapshot.json"), + }; + + ValidateOptionsResult result = + new GalaxyRepositoryOptionsValidator(contentRoot).Validate(null, options); + + Assert.True(result.Failed); + Assert.Contains( + result.Failures!, + f => f.Contains("MxGateway:Galaxy:SnapshotCachePath") + && f.Contains("must not be inside the application directory")); + } + + /// Verifies a snapshot path outside the application directory still passes. + [Fact] + public void Validate_Succeeds_WhenSnapshotPathIsOutsideContentRoot() + { + string contentRoot = Path.Combine(Path.GetTempPath(), $"mxgw-root-{Guid.NewGuid():N}"); + GalaxyRepositoryOptions options = new() + { + PersistSnapshot = true, + SnapshotCachePath = + Path.Combine(Path.GetTempPath(), $"mxgw-data-{Guid.NewGuid():N}", "galaxy-snapshot.json"), + }; + + ValidateOptionsResult result = + new GalaxyRepositoryOptionsValidator(contentRoot).Validate(null, options); + + Assert.True(result.Succeeded); + } + /// /// Verifies the gateway supplies a rooted per-OS default when the shipped config leaves /// SnapshotCachePath blank, so the removed appsettings literal is not needed and validation diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs index c84aa3b..f5f2120 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayOptionsValidatorTests.cs @@ -598,6 +598,69 @@ public sealed class GatewayOptionsValidatorTests f => f.Contains("MxGateway:Authentication:SqlitePath") && f.Contains("rooted")); } + /// + /// Verifies an absolute auth DB path inside the application directory fails. This is + /// the gap the rooted check does not close: the path that lost every API key on a production + /// host on 2026-08-09 was absolute and passed rooting cleanly — it simply lived in the directory + /// the upgrade procedure renames away. + /// + [Fact] + public void Validate_Fails_WhenSqlitePathIsUnderContentRoot() + { + string contentRoot = Path.Combine(Path.GetTempPath(), $"mxgw-root-{Guid.NewGuid():N}"); + GatewayOptions options = CloneWithAuthentication( + ValidOptions(), + new AuthenticationOptions { SqlitePath = Path.Combine(contentRoot, "gateway-auth.db") }); + + ValidateOptionsResult result = + new GatewayOptionsValidator(contentRootPath: contentRoot).Validate(null, options); + + Assert.True(result.Failed); + Assert.Contains( + result.Failures!, + f => f.Contains("MxGateway:Authentication:SqlitePath") + && f.Contains("must not be inside the application directory")); + } + + /// + /// Verifies the content-root rule is not a bare string prefix test: a sibling directory whose + /// name merely begins with the content root's must still pass. + /// + [Fact] + public void Validate_Succeeds_WhenSqlitePathIsSiblingOfContentRoot() + { + string contentRoot = Path.Combine(Path.GetTempPath(), $"mxgw-root-{Guid.NewGuid():N}"); + GatewayOptions options = CloneWithAuthentication( + ValidOptions(), + new AuthenticationOptions { SqlitePath = contentRoot + "-data" + Path.DirectorySeparatorChar + "gateway-auth.db" }); + + ValidateOptionsResult result = + new GatewayOptionsValidator(contentRootPath: contentRoot).Validate(null, options); + + Assert.True(result.Succeeded); + } + + /// + /// Verifies a store path outside the application directory passes — the rule must reject only + /// the genuinely unsafe location, not every absolute path. + /// + [Fact] + public void Validate_Succeeds_WhenSqlitePathIsOutsideContentRoot() + { + string contentRoot = Path.Combine(Path.GetTempPath(), $"mxgw-root-{Guid.NewGuid():N}"); + GatewayOptions options = CloneWithAuthentication( + ValidOptions(), + new AuthenticationOptions + { + SqlitePath = Path.Combine(Path.GetTempPath(), $"mxgw-data-{Guid.NewGuid():N}", "gateway-auth.db"), + }); + + ValidateOptionsResult result = + new GatewayOptionsValidator(contentRootPath: contentRoot).Validate(null, options); + + Assert.True(result.Succeeded); + } + /// /// Verifies rooting is host-meaningful (SEC-33): a Windows drive-qualified literal fails on a /// Unix host (where it is not rooted) rather than being blessed and written as a junk-named