fix(config): reject credential and cache paths inside the application directory
Rooted is not the same as safe, and the gap between the two cost a production
host every one of its API keys on 2026-08-09.
MxGateway:Authentication:SqlitePath was set to an absolute path inside the
directory the upgrade procedure renames to Server.bak.*. That passes the
existing rooted check cleanly. The deploy renamed the directory away, the store
went with it, and the gateway created a fresh empty one at the same path — no
error, no log line. No gRPC consumer could authenticate for two days. The deploy
itself was correct: the binaries were the point of the rename and the store was
collateral.
GatewayConfigPathRules gains AddIfUnderContentRoot, applied to the auth store
and the Galaxy snapshot. Both are written by the running process and both are
lost the same way. The rule compares resolved full paths and requires a
directory-separator boundary, so a sibling directory whose name merely starts
with the content root's ("/srv/app-data" against "/srv/app") is not treated as
inside it — on a fail-closed startup rule, that false positive would be a
gateway that refuses to boot on a legitimate path. Case sensitivity follows the
running OS rather than assuming case-insensitivity everywhere, which would
reject /srv/App as under /srv/app on Linux where they are different directories.
The rule is not exempted in Development. An environment-conditional guard is
never exercised where the mistake is made, and what failed in production was a
config that looked fine.
Secrets:SqlitePath is the same defect one layer down: it shipped as a bare
relative "mxgateway-secrets.db", which is how a stray database landed in
src/…Server/ and tripped the repository's tree-hygiene test. It is bound by the
shared ZB.MOM.WW.Secrets package, so appsettings.json now ships no value and the
default is computed from CommonApplicationData in code — the same mechanism
SEC-33 already used for the Galaxy snapshot, ten lines away, for the same reason.
Setting a default for an unset key is deliberately not the same act as
relocating a value someone configured, which these rules still refuse to do.
Note the migration edge this creates: a host relying on the old repo default now
looks somewhere new, finds nothing, and creates an empty store — this bug
re-introduced by its own fix. Deployed hosts are safe because they set the path
explicitly, in appsettings copied forward or in the service environment. The
latter is the more robust of the two, since it cannot be lost by a missed
preserve step.
This commit is contained in:
@@ -15,15 +15,22 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
// rather than merely warn. Non-production hosts keep the permissive dev posture.
|
||||
private readonly bool _isProduction;
|
||||
|
||||
// The application content root. Store paths must not live under it — see
|
||||
// GatewayConfigPathRules.AddIfUnderContentRoot. Null for non-DI callers that supply no
|
||||
// environment, which skips the rule rather than inventing a root to test against.
|
||||
private readonly string? _contentRootPath;
|
||||
|
||||
/// <summary>
|
||||
/// Initializes a new instance of the <see cref="GatewayOptionsValidator"/> 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.
|
||||
/// </summary>
|
||||
/// <param name="environment">The host environment.</param>
|
||||
public GatewayOptionsValidator(IHostEnvironment environment)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(environment);
|
||||
_isProduction = environment.IsProduction();
|
||||
_contentRootPath = environment.ContentRootPath;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -32,15 +39,20 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
/// hard-stops do not fire; pass <see langword="true"/> to exercise them.
|
||||
/// </summary>
|
||||
/// <param name="isProduction">Whether to treat the host as running in Production.</param>
|
||||
internal GatewayOptionsValidator(bool isProduction = false)
|
||||
/// <param name="contentRootPath">
|
||||
/// Content root to test store paths against; <see langword="null"/> leaves the content-root
|
||||
/// rule inactive, which is what a caller with no real host wants.
|
||||
/// </param>
|
||||
internal GatewayOptionsValidator(bool isProduction = false, string? contentRootPath = null)
|
||||
{
|
||||
_isProduction = isProduction;
|
||||
_contentRootPath = contentRootPath;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
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<GatewayOption
|
||||
builder);
|
||||
}
|
||||
|
||||
private static void ValidateAuthentication(AuthenticationOptions options, ValidationBuilder builder)
|
||||
private static void ValidateAuthentication(
|
||||
AuthenticationOptions options,
|
||||
string? contentRootPath,
|
||||
ValidationBuilder builder)
|
||||
{
|
||||
if (!Enum.IsDefined(options.Mode))
|
||||
{
|
||||
@@ -123,6 +138,11 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
options.SqlitePath,
|
||||
"MxGateway:Authentication:SqlitePath must be an absolute (rooted) path so the credential store never lands in the launch working directory.",
|
||||
builder);
|
||||
AddIfUnderContentRoot(
|
||||
options.SqlitePath,
|
||||
contentRootPath,
|
||||
$"MxGateway:Authentication:SqlitePath must not be inside the application directory ({contentRootPath}). The upgrade procedure renames that directory, which abandons the credential store and silently starts an empty one — every API key is lost and no client can authenticate.",
|
||||
builder);
|
||||
AddIfBlank(
|
||||
options.PepperSecretName,
|
||||
"MxGateway:Authentication:PepperSecretName is required when API-key authentication is enabled.",
|
||||
@@ -555,4 +575,14 @@ public sealed class GatewayOptionsValidator : OptionsValidatorBase<GatewayOption
|
||||
|
||||
private static void AddIfInvalidPath(string? value, string message, ValidationBuilder builder)
|
||||
=> 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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user