Files
mxaccessgw/src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayConfigPathRules.cs
T
Joseph Doherty 882c7ca3cd 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.
2026-08-11 08:42:16 -04:00

155 lines
6.7 KiB
C#

using ZB.MOM.WW.Configuration;
namespace ZB.MOM.WW.MxGateway.Server.Configuration;
/// <summary>
/// Shared filesystem-path validation primitives used by more than one options validator
/// (<see cref="GatewayOptionsValidator"/> and <see cref="GalaxyRepositoryOptionsValidator"/>).
/// Both the auth credential store and the Galaxy snapshot are written by the running gateway
/// process, so both must reject paths the host cannot use — the rules live here once so the two
/// validators cannot drift.
/// </summary>
internal static class GatewayConfigPathRules
{
/// <summary>
/// Fails validation when <paramref name="value"/> is not an absolute (rooted) path <em>on the
/// host running the validator</em>. Security-sensitive paths (the auth DB, the self-signed
/// private key, the Galaxy snapshot) must be absolute: a non-rooted value silently resolves
/// against the launch working directory, so the store moves with the CWD and can leak into the
/// source tree. Rooting is checked with <see cref="Path.IsPathRooted(string)"/> — the current
/// OS — so a Windows drive/UNC literal on a Unix host fails fast at startup rather than being
/// blessed and then written as a junk-named relative file (the SEC-01/SEC-33 mechanism). Reject
/// rather than auto-root; silent relocation of a credential store is worse than a boot error.
/// Blank is handled by the caller's required-field check and is not treated as non-rooted here.
/// </summary>
/// <param name="value">The configured path value.</param>
/// <param name="message">The failure message to record when the value is not rooted.</param>
/// <param name="builder">The validation builder accumulating failures.</param>
public static void AddIfNotRooted(string? value, string message, ValidationBuilder builder)
{
if (string.IsNullOrWhiteSpace(value))
{
return;
}
if (!Path.IsPathRooted(value))
{
builder.Add(message);
}
}
/// <summary>
/// Fails validation when <paramref name="value"/> is non-blank but not a syntactically valid
/// filesystem path (as judged by <see cref="Path.GetFullPath(string)"/>). Blank values are the
/// caller's required-field concern and pass here.
/// </summary>
/// <param name="value">The configured path value.</param>
/// <param name="message">The failure message to record when the value is not a valid path.</param>
/// <param name="builder">The validation builder accumulating failures.</param>
public static void AddIfInvalidPath(string? value, string message, ValidationBuilder builder)
{
if (string.IsNullOrWhiteSpace(value))
{
return;
}
if (!TryGetFullPath(value, out _))
{
builder.Add(message);
}
}
/// <summary>
/// Fails validation when <paramref name="value"/> resolves to a location inside
/// <paramref name="contentRoot"/> — the directory the application runs from.
/// </summary>
/// <remarks>
/// <para>
/// <b>Rooted is not the same as safe, and this is the rule that closes the gap.</b>
/// <see cref="AddIfNotRooted"/> stops a store drifting with the working directory, but an
/// absolute path <em>inside the app directory</em> passes it cleanly — and that is what failed
/// in production on 2026-08-09. The upgrade procedure renames the app directory to
/// <c>Server.bak.*</c> 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.
/// </para>
/// <para>
/// The same shape catches the dev-side symptom: a store under the content root lands in the
/// source tree, which is how <c>mxgateway-secrets.db</c> once tripped the repository's
/// tree-hygiene test.
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
/// <param name="value">The configured path value.</param>
/// <param name="contentRoot">The application content root to test against.</param>
/// <param name="message">The failure message to record when the value is under the content root.</param>
/// <param name="builder">The validation builder accumulating failures.</param>
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;
}
}
}