diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/Configuration/SecretsOptionsValidator.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/Configuration/SecretsOptionsValidator.cs index 2b37da4..8aefa1e 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/Configuration/SecretsOptionsValidator.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/Configuration/SecretsOptionsValidator.cs @@ -65,7 +65,8 @@ public sealed class SecretsOptionsValidator : OptionsValidatorBase +/// +/// Both rules compare locations, not spellings (Secrets-005). "Rooted" means fully qualified: +/// Windows' drive-relative C:secrets.db and root-relative \secrets.db are rooted but +/// still resolve against a current directory, so they are rejected. "Under the content root" +/// compares both the spelled and the resolved location of each side, and fails if either is inside: +/// symbolic links and junctions on the existing part of each path are followed to their final +/// target (a store spelled through the content root is still refused even if a link inside it +/// leads elsewhere — the next deploy's tree has no such link), and on Windows the \\?\ / \\.\ device +/// prefixes are removed, a loopback administrative share (\\localhost\C$\…, +/// \\127.0.0.1\C$\…, \\<this machine>\C$\…) is mapped back to its drive, and 8.3 +/// short names (PROGRA~1) are expanded. A UNC path through any other share is compared as +/// written — what a share points at is server configuration this rule cannot see. +/// /// public static class SecretsStorePathRules { /// - /// Records a failure when is non-blank and not an absolute (rooted) - /// path on the current OS. Blank is the caller's required-field concern and passes here. + /// Records a failure when is non-blank and not a fully qualified path on + /// the current OS (: on Windows a drive-relative + /// C:secrets.db or root-relative \secrets.db is rejected, because each still resolves + /// against a current directory). Blank is the caller's required-field concern and passes here. /// /// The configured path value. /// The failure message to record. @@ -54,7 +70,7 @@ public static class SecretsStorePathRules return; } - if (!Path.IsPathRooted(value)) + if (!Path.IsPathFullyQualified(value)) { builder.Add(message); } @@ -83,21 +99,39 @@ public static class SecretsStorePathRules return; } - string fullValue; - string fullRoot; + // Two readings, and the store is inside if EITHER says so: + // - as SPELLED (full path, device prefix / loopback share normalised): a store configured + // through the content root moves with it on a rename even when a link inside carries it + // elsewhere today — the next deploy's tree has no such link, and the store is recreated + // empty at the same spelling; + // - as LOCATED (symlinks, junctions and 8.3 names resolved): a store spelled through an + // alias of the content root, or a content root reached through a "current" link, sits in + // the directory the deploy replaces. + string spelledValue, spelledRoot, locatedValue, locatedRoot; try { - fullValue = Path.GetFullPath(value); - fullRoot = Path.GetFullPath(contentRoot); + spelledValue = Spelled(value); + spelledRoot = Spelled(contentRoot); + locatedValue = Canonicalize(value); + locatedRoot = Canonicalize(contentRoot); } catch (Exception ex) when (ex is ArgumentException or NotSupportedException - or PathTooLongException or IOException) + or PathTooLongException or IOException + or UnauthorizedAccessException) { // A syntactically invalid path is a different failure with its own message; refusing // to guess here keeps one rule reporting one thing. return; } + if (IsUnder(spelledValue, spelledRoot) || IsUnder(locatedValue, locatedRoot)) + { + builder.Add(message); + } + } + + private static bool IsUnder(string fullValue, string fullRoot) + { // The separator is load-bearing, not cosmetic. Without it a bare prefix test treats // "/srv/app-data" as inside "/srv/app" and refuses a legitimate sibling directory — and // because this rule is fail-closed at startup, that false positive is an application that @@ -112,9 +146,125 @@ public static class SecretsStorePathRules ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; - if (fullValue.StartsWith(rootWithSeparator, comparison)) - { - builder.Add(message); - } + return fullValue.StartsWith(rootWithSeparator, comparison); } + + private static string Spelled(string path) => OperatingSystem.IsWindows() + ? NormalizeWindowsSpelling(Path.GetFullPath(NormalizeWindowsSpelling(path))) + : Path.GetFullPath(path); + + private const int MaxLinkDepth = 40; + + /// + /// Returns the location names, independent of how it is spelled: full + /// path, Windows device prefix stripped and loopback admin share mapped to its drive, and every + /// existing component resolved through symbolic links / junctions (and, on Windows, 8.3 short + /// names expanded). Components that do not exist yet are appended as written. + /// + /// The path to canonicalise. + /// The canonical full path. + internal static string Canonicalize(string path) => Canonicalize(path, depth: 0); + + private static string Canonicalize(string path, int depth) + { + if (depth > MaxLinkDepth) + { + throw new IOException("Too many levels of symbolic links."); + } + + string full = Spelled(path); + + string root = Path.GetPathRoot(full) ?? string.Empty; + string[] parts = full[root.Length..].Split( + new[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, + StringSplitOptions.RemoveEmptyEntries); + + string current = root; + for (int i = 0; i < parts.Length; i++) + { + string next = Path.Combine(current, parts[i]); + FileSystemInfo? info = Directory.Exists(next) ? new DirectoryInfo(next) + : File.Exists(next) ? new FileInfo(next) + : null; + + if (info is null) + { + // Not created yet (the store usually is not, at validation time): nothing on disk + // can redirect the rest, so append it as written. + return Path.Combine(new[] { current }.Concat(parts[i..]).ToArray()); + } + + if (info.LinkTarget is not null) + { + FileSystemInfo? target = info.ResolveLinkTarget(returnFinalTarget: true); + if (target is not null) + { + // The target's own parent directories may be links too: canonicalise it whole. + next = Canonicalize(target.FullName, depth + 1); + } + } + else if (OperatingSystem.IsWindows()) + { + next = ExpandShortName(next); + } + + current = next; + } + + return current; + } + + /// + /// Strips the Win32 device prefixes (\\?\C:\…, \\.\C:\…, \\?\UNC\…) and maps + /// a loopback administrative share (\\localhost\C$\…) back to its drive. + /// + private static string NormalizeWindowsSpelling(string path) + { + string p = path.Replace('/', '\\'); + if (p.StartsWith(@"\\?\UNC\", StringComparison.OrdinalIgnoreCase)) + { + p = @"\\" + p[8..]; + } + else if (p.StartsWith(@"\\?\", StringComparison.Ordinal) || p.StartsWith(@"\\.\", StringComparison.Ordinal)) + { + p = p[4..]; + } + + if (p.StartsWith(@"\\", StringComparison.Ordinal)) + { + string[] segments = p[2..].Split('\\'); + if (segments.Length >= 2 + && IsLoopbackHost(segments[0]) + && segments[1].Length == 2 + && char.IsAsciiLetter(segments[1][0]) + && segments[1][1] == '$') + { + p = segments[1][0] + @":\" + string.Join('\\', segments[2..]); + } + } + + return p; + } + + private static bool IsLoopbackHost(string host) => + host.Equals("localhost", StringComparison.OrdinalIgnoreCase) + || host.Equals("127.0.0.1", StringComparison.Ordinal) + || host.Equals("::1", StringComparison.Ordinal) + || host.Equals("[::1]", StringComparison.Ordinal) + || host.Equals(Environment.MachineName, StringComparison.OrdinalIgnoreCase); + + private static string ExpandShortName(string existingPath) + { + if (!existingPath.Contains('~')) + { + return existingPath; + } + + char[] buffer = new char[1024]; + uint length = GetLongPathNameW(existingPath, buffer, (uint)buffer.Length); + return length > 0 && length < buffer.Length ? new string(buffer, 0, (int)length) : existingPath; + } + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern uint GetLongPathNameW(string shortPath, [Out] char[] longPath, uint bufferLength); } diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathSpellingTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathSpellingTests.cs new file mode 100644 index 0000000..25f1c99 --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathSpellingTests.cs @@ -0,0 +1,151 @@ +using ZB.MOM.WW.Configuration; +using ZB.MOM.WW.Secrets.Configuration; + +namespace ZB.MOM.WW.Secrets.Tests.Configuration; + +/// +/// Secrets-005: the store-path rules must compare locations, not spellings. A path that reaches +/// the content root through a symbolic link (or a content root that is itself reached through one) +/// is still inside it; on Windows a drive-relative path is not "absolute", and device-prefixed, +/// loopback-admin-share and 8.3 spellings of a path inside the content root are still inside it. +/// +public sealed class SecretsStorePathSpellingTests : IDisposable +{ + private readonly string _base; + + public SecretsStorePathSpellingTests() + { + _base = Path.Combine(Path.GetTempPath(), $"zb-secrets-spelling-{Guid.NewGuid():N}"); + Directory.CreateDirectory(Path.Combine(_base, "app", "data")); + Directory.CreateDirectory(Path.Combine(_base, "outside")); + } + + public void Dispose() + { + try { Directory.Delete(_base, recursive: true); } catch (IOException) { } + } + + private static bool Inside(string value, string root) + { + var builder = new ValidationBuilder(); + SecretsStorePathRules.AddIfUnderContentRoot(value, root, "inside", builder); + return !builder.IsValid; + } + + private static bool NotRooted(string value) + { + var builder = new ValidationBuilder(); + SecretsStorePathRules.AddIfNotRooted(value, "relative", builder); + return !builder.IsValid; + } + + private bool TryLink(string link, string target) + { + try + { + Directory.CreateSymbolicLink(link, target); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + return false; // Windows without the symlink privilege + } + } + + [SkippableFact] + public void A_store_reached_through_a_symlink_into_the_content_root_is_inside_it() + { + string root = Path.Combine(_base, "app"); + string link = Path.Combine(_base, "alias"); + Skip.IfNot(TryLink(link, root), "cannot create a symbolic link here"); + + // Positive control: the same file spelled directly is caught today. + Assert.True(Inside(Path.Combine(root, "data", "secrets.db"), root)); + Assert.True(Inside(Path.Combine(link, "data", "secrets.db"), root)); + } + + [SkippableFact] + public void A_content_root_reached_through_a_current_symlink_still_contains_its_store() + { + // The "current -> releases/42" deployment shape: the host reports the link, the store is + // configured with the real directory (or the other way round). + string real = Path.Combine(_base, "app"); + string current = Path.Combine(_base, "current"); + Skip.IfNot(TryLink(current, real), "cannot create a symbolic link here"); + + Assert.True(Inside(Path.Combine(real, "data", "secrets.db"), current)); + Assert.True(Inside(Path.Combine(current, "data", "secrets.db"), real)); + } + + [SkippableFact] + public void A_store_spelled_through_the_content_root_stays_refused_even_if_a_link_leads_out() + { + // A "durable" link inside the app directory pointing at storage elsewhere: the bytes survive + // a rename, but the next deploy's tree has no such link, so the configured spelling then + // names a fresh empty store inside the new tree — the 2026-08-09 shape. Resolving links + // must ADD cases, never remove the spelled one. + string root = Path.Combine(_base, "app"); + string link = Path.Combine(root, "durable"); + Skip.IfNot(TryLink(link, Path.Combine(_base, "outside")), "cannot create a symbolic link here"); + + Assert.True(Inside(Path.Combine(link, "secrets.db"), root)); + Assert.False(Inside(Path.Combine(_base, "outside", "secrets.db"), root)); // control + } + + [Fact] + public void A_not_yet_created_store_is_compared_as_written() + { + string root = Path.Combine(_base, "app"); + Assert.True(Inside(Path.Combine(root, "new", "deeper", "secrets.db"), root)); + Assert.False(Inside(Path.Combine(_base, "outside", "new", "secrets.db"), root)); + } + + [SkippableFact] + public void Drive_and_root_relative_paths_are_not_absolute_on_windows() + { + Skip.IfNot(OperatingSystem.IsWindows(), "Windows path semantics"); + Assert.True(NotRooted(@"C:secrets.db")); + Assert.True(NotRooted(@"\secrets.db")); + Assert.False(NotRooted(@"C:\data\secrets.db")); + Assert.False(NotRooted(@"\\server\share\secrets.db")); + } + + [SkippableFact] + public void Device_prefixed_and_loopback_share_spellings_are_inside_on_windows() + { + Skip.IfNot(OperatingSystem.IsWindows(), "Windows path semantics"); + string root = Path.Combine(_base, "app"); + string full = Path.GetFullPath(Path.Combine(root, "data", "secrets.db")); + string drive = full[..1]; + string rest = full[3..]; + + Assert.True(Inside(@"\\?\" + full, root)); + Assert.True(Inside(@"\\.\" + full, root)); + Assert.True(Inside($@"\\localhost\{drive}$\{rest}", root)); + Assert.True(Inside($@"\\127.0.0.1\{drive}$\{rest}", root)); + Assert.True(Inside($@"\\?\UNC\localhost\{drive}$\{rest}", root)); + } + + [SkippableFact] + public void An_8_3_short_name_spelling_is_inside_on_windows() + { + Skip.IfNot(OperatingSystem.IsWindows(), "Windows path semantics"); + string longDir = Path.Combine(_base, "Application Directory"); + Directory.CreateDirectory(longDir); + string? shortDir = ShortName(longDir); + Skip.If(shortDir is null || !shortDir.Contains('~'), "8.3 names are disabled on this volume"); + + Assert.True(Inside(Path.Combine(shortDir!, "secrets.db"), longDir)); + Assert.True(Inside(Path.Combine(longDir, "secrets.db"), shortDir!)); + } + + private static string? ShortName(string path) + { + char[] buffer = new char[1024]; + uint n = GetShortPathNameW(path, buffer, (uint)buffer.Length); + return n == 0 ? null : new string(buffer, 0, (int)n); + } + + [System.Runtime.InteropServices.DllImport("kernel32.dll", CharSet = System.Runtime.InteropServices.CharSet.Unicode)] + private static extern uint GetShortPathNameW(string longPath, [System.Runtime.InteropServices.Out] char[] shortPath, uint size); +}