From 7ec7a0f909ca9a1b3ad03c744cbfefa29005eeee Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:51:45 +0000 Subject: [PATCH] ClientRuntimeLoader: pin the resolved root, load the resolved path, check dependencies Review (Heimdall): the root is resolved once at construction and a volume root is refused and the effective root logged; the loader loads and records the resolved final path (so a pointer naming a link does not reload every poll); dependency and native-library resolution go through the same check; the AQUARIUM_CLIENT_TRUSTED_ROOT variable is cleared after reading. Tests: UNC in any separator mix, volume root, pinned root, resolved-path storage, junction and symlink tests that report an explicit skip instead of passing silently. Co-Authored-By: Claude Sonnet 5.5 --- src/Aquarium.Engine/AquariumHost.cs | 5 +- .../Runtime/ClientRuntimeLoader.cs | 88 ++++++++---- .../ClientRuntimeLoaderTrustTests.cs | 133 +++++++++++++++--- 3 files changed, 180 insertions(+), 46 deletions(-) diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index 4f8af38..8404576 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -269,7 +269,10 @@ public static class AquariumHost } } - return Environment.GetEnvironmentVariable("AQUARIUM_CLIENT_TRUSTED_ROOT"); + // Read once and removed, so processes this one starts do not inherit a root meant for it. + var fromEnvironment = Environment.GetEnvironmentVariable("AQUARIUM_CLIENT_TRUSTED_ROOT"); + Environment.SetEnvironmentVariable("AQUARIUM_CLIENT_TRUSTED_ROOT", null); + return fromEnvironment; } private static string? ParseClientReloadPointerPath(IReadOnlyCollection args) diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index 0874af0..60f64fc 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -16,9 +16,10 @@ public sealed class ClientRuntimeLoader : IDisposable private string? currentAssemblyPath; /// - /// Every assembly this loader runs must resolve, links followed, to a path under - /// (default: the directory of the initial assembly). A relative initial path is taken against the working directory; - /// a relative or UNC reload target is refused. + /// Every assembly this loader runs, and every dependency or native library it resolves, must lie, links followed, + /// under (default: the directory of the initial assembly). The root is resolved once + /// here and pinned; a volume root is refused. A relative initial path is taken against the working directory; + /// a relative or UNC reload target is refused. The boundary this enforces is the root directory's own ACLs. /// public ClientRuntimeLoader( AquariumRuntimeOptions options, @@ -28,10 +29,26 @@ public sealed class ClientRuntimeLoader : IDisposable { this.clientAssemblyPath = string.IsNullOrWhiteSpace(clientAssemblyPath) ? null : Path.GetFullPath(clientAssemblyPath); this.clientReloadPointerPath = clientReloadPointerPath; - this.trustedRoot = string.IsNullOrWhiteSpace(trustedRoot) + this.options = options; + + var root = string.IsNullOrWhiteSpace(trustedRoot) ? this.clientAssemblyPath is null ? null : Path.GetDirectoryName(this.clientAssemblyPath) : Path.GetFullPath(trustedRoot); - this.options = options; + if (root is not null) + { + root = ResolveLinks(root); + if (string.Equals( + Path.TrimEndingDirectorySeparator(root), + Path.TrimEndingDirectorySeparator(Path.GetPathRoot(root) ?? root), + PathComparison)) + { + throw new ArgumentException($"A drive or volume root cannot be the client runtime trusted root: {root}", nameof(trustedRoot)); + } + + Console.WriteLine($"Client runtime trusted root: {root}"); + } + + this.trustedRoot = root; } public IAquariumRuntime Runtime @@ -53,11 +70,13 @@ public sealed class ClientRuntimeLoader : IDisposable return Runtime; } - public void Reload(string assemblyPath, AquariumRuntimeOptions options) + public void Reload(string assemblyPath, AquariumRuntimeOptions options) => + LoadResolved(ResolveTrusted(assemblyPath), options); + + private void LoadResolved(string assemblyPath, AquariumRuntimeOptions options) { - RequireTrusted(assemblyPath); var previous = current; - var loadContext = new ClientAssemblyLoadContext(assemblyPath); + var loadContext = new ClientAssemblyLoadContext(assemblyPath, trustedRoot!); IAquariumRuntime? nextRuntime = null; try @@ -117,18 +136,22 @@ public sealed class ClientRuntimeLoader : IDisposable return; } - if (string.IsNullOrWhiteSpace(candidatePath) - || string.Equals(candidatePath, currentAssemblyPath, StringComparison.OrdinalIgnoreCase) - || !File.Exists(candidatePath)) + if (string.IsNullOrWhiteSpace(candidatePath) || !File.Exists(candidatePath)) { return; } try { + var resolved = ResolveTrusted(candidatePath); + if (string.Equals(resolved, currentAssemblyPath, PathComparison)) + { + return; + } + current?.Runtime.FlushState(); - Reload(candidatePath, options); - Console.WriteLine($"Client runtime reload applied: {candidatePath}"); + LoadResolved(resolved, options); + Console.WriteLine($"Client runtime reload applied: {resolved}"); } catch (Exception error) { @@ -136,29 +159,36 @@ public sealed class ClientRuntimeLoader : IDisposable } } - private void RequireTrusted(string assemblyPath) + private static StringComparison PathComparison => + OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + + private string ResolveTrusted(string assemblyPath) => + trustedRoot is null + ? throw new InvalidOperationException("No trusted root for client runtime assemblies is set.") + : ResolveTrusted(trustedRoot, assemblyPath); + + // The final path of a local, fully qualified file under the already-resolved root; throws otherwise. + internal static string ResolveTrusted(string resolvedRoot, string path) { - if (trustedRoot is null) + if (!Path.IsPathFullyQualified(path) || (path.Length >= 2 && IsSeparator(path[0]) && IsSeparator(path[1]))) { - throw new InvalidOperationException("No trusted root for client runtime assemblies is set."); + throw new InvalidOperationException($"Client runtime path must be local and fully qualified: {path}"); } - if (!Path.IsPathFullyQualified(assemblyPath) - || assemblyPath.StartsWith(@"\\", StringComparison.Ordinal) - || assemblyPath.StartsWith("//", StringComparison.Ordinal)) - { - throw new InvalidOperationException($"Client runtime assembly path must be local and fully qualified: {assemblyPath}"); - } - - var relative = Path.GetRelativePath(ResolveLinks(trustedRoot), ResolveLinks(Path.GetFullPath(assemblyPath))); + var resolved = ResolveLinks(Path.GetFullPath(path)); + var relative = Path.GetRelativePath(resolvedRoot, resolved); if (relative == ".." || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || Path.IsPathRooted(relative)) { - throw new InvalidOperationException($"Client runtime assembly is outside the trusted root '{trustedRoot}': {assemblyPath}"); + throw new InvalidOperationException($"Client runtime path is outside the trusted root '{resolvedRoot}': {path}"); } + + return resolved; } + private static bool IsSeparator(char c) => c is '/' or '\\'; + // The path with every symlink or junction on it replaced by its final target. - private static string ResolveLinks(string path, int depth = 0) + internal static string ResolveLinks(string path, int depth = 0) { if (depth > 32) { @@ -210,7 +240,7 @@ public sealed class ClientRuntimeLoader : IDisposable } } - private sealed class ClientAssemblyLoadContext(string assemblyPath) : AssemblyLoadContext(isCollectible: true) + private sealed class ClientAssemblyLoadContext(string assemblyPath, string trustedRoot) : AssemblyLoadContext(isCollectible: true) { private readonly AssemblyDependencyResolver resolver = new(assemblyPath); @@ -222,13 +252,13 @@ public sealed class ClientRuntimeLoader : IDisposable } var path = resolver.ResolveAssemblyToPath(assemblyName); - return path == null ? null : LoadFromAssemblyPath(path); + return path == null ? null : LoadFromAssemblyPath(ResolveTrusted(trustedRoot, path)); } protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) { var path = resolver.ResolveUnmanagedDllToPath(unmanagedDllName); - return path == null ? IntPtr.Zero : LoadUnmanagedDllFromPath(path); + return path == null ? IntPtr.Zero : LoadUnmanagedDllFromPath(ResolveTrusted(trustedRoot, path)); } } } diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index afe9c03..57a6b9f 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -1,9 +1,40 @@ +using System.Diagnostics; using Aquarium.Engine; using Aquarium.Engine.Input; using Aquarium.Engine.Runtime; namespace Aquarium.Engine.Tests; +/// Runs only where this process can create symbolic links; otherwise reported as skipped with the reason. +public sealed class SymlinkFactAttribute : FactAttribute +{ + public SymlinkFactAttribute() + { + var probe = Path.Combine(Path.GetTempPath(), "aquarium-symlink-probe-" + Guid.NewGuid().ToString("N")); + try + { + File.CreateSymbolicLink(probe, Path.GetTempPath()); + File.Delete(probe); + } + catch (Exception error) when (error is IOException or UnauthorizedAccessException) + { + Skip = "This account cannot create symbolic links: " + error.Message; + } + } +} + +/// Junctions exist only on Windows. +public sealed class JunctionFactAttribute : FactAttribute +{ + public JunctionFactAttribute() + { + if (!OperatingSystem.IsWindows()) + { + Skip = "NTFS junctions exist only on Windows; run this test there."; + } + } +} + public sealed class ClientRuntimeLoaderTrustTests : IDisposable { private static readonly AquariumRuntimeOptions Options = new(true, null); @@ -34,8 +65,9 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable var second = Copy(Directory.CreateDirectory(Path.Combine(trusted, "slot2")).FullName); using var loader = new ClientRuntimeLoader(Options, first); loader.Load(); + var before = loader.Runtime; loader.Reload(second, Options); - Assert.NotNull(loader.Runtime); + Assert.NotSame(before, loader.Runtime); } [Fact] @@ -48,7 +80,6 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable using var loader = new ClientRuntimeLoader(Options, Copy(slot), trustedRoot: trusted); loader.Load(); loader.Reload(sibling, Options); - Assert.Throws(() => loader.Reload(Copy(outside), Options)); using var narrow = new ClientRuntimeLoader(Options, Path.Combine(slot, "client.dll")); narrow.Load(); @@ -63,7 +94,7 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable } [Fact] - public void ReloadOutsideTheRootDotDotRelativeAndUncPathsAreRefused() + public void ReloadOutsideTheRootViaDotDotOrRelativePathIsRefused() { using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); loader.Load(); @@ -71,25 +102,43 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Throws(() => loader.Reload(escaped, Options)); Assert.Throws(() => loader.Reload(Path.Combine(trusted, "..", "outside", "client.dll"), Options)); Assert.Throws(() => loader.Reload("a.dll", Options)); - Assert.Throws(() => loader.Reload("//host/share/client.dll", Options)); - Assert.Throws(() => loader.Reload(@"\\host\share\client.dll", Options)); + } + + [Theory] + [InlineData("//host/share/client.dll")] + [InlineData(@"\\host\share\client.dll")] + [InlineData(@"\/host/share\client.dll")] + [InlineData(@"/\host\share/client.dll")] + public void UncPathsInAnySeparatorMixAreRefused(string path) + { + Assert.Throws(() => ClientRuntimeLoader.ResolveTrusted(trusted, path)); } [Fact] + public void AVolumeRootCannotBeTheTrustedRoot() + { + Assert.Throws(() => new ClientRuntimeLoader(Options, Copy(trusted), trustedRoot: Path.GetPathRoot(trusted))); + } + + [Fact] + public void ADependencyOrNativeLibraryOutsideTheRootIsRefused() + { + // The load context resolves dependencies through the same check; assert the check it calls. + var dependency = Path.Combine(outside, "dep.dll"); + File.WriteAllText(dependency, ""); + Assert.Throws(() => ClientRuntimeLoader.ResolveTrusted(ClientRuntimeLoader.ResolveLinks(trusted), dependency)); + var inside = Path.Combine(trusted, "dep.so"); + File.WriteAllText(inside, ""); + Assert.Equal(inside, ClientRuntimeLoader.ResolveTrusted(ClientRuntimeLoader.ResolveLinks(trusted), inside)); + } + + [SymlinkFact] public void SymlinkedFileOrDirectoryPointingOutsideTheRootIsRefused() { - var target = Copy(outside); var fileLink = Path.Combine(trusted, "link.dll"); var dirLink = Path.Combine(trusted, "dirlink"); - try - { - File.CreateSymbolicLink(fileLink, target); - Directory.CreateSymbolicLink(dirLink, outside); - } - catch (Exception error) when (error is IOException or UnauthorizedAccessException) - { - return; // this host cannot create links - } + File.CreateSymbolicLink(fileLink, Copy(outside)); + Directory.CreateSymbolicLink(dirLink, outside); using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); loader.Load(); @@ -97,6 +146,58 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Throws(() => loader.Reload(Path.Combine(dirLink, "client.dll"), Options)); } + [JunctionFact] + public void JunctionPointingOutsideTheRootIsRefused() + { + var junction = Path.Combine(trusted, "junction"); + Copy(outside); + var mklink = Process.Start(new ProcessStartInfo("cmd.exe", $"/c mklink /J \"{junction}\" \"{outside}\"") { UseShellExecute = false, CreateNoWindow = true })!; + mklink.WaitForExit(); + Assert.Equal(0, mklink.ExitCode); + + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); + loader.Load(); + Assert.Throws(() => loader.Reload(Path.Combine(junction, "client.dll"), Options)); + } + + [SymlinkFact] + public void ALoadedLinkIsStoredByItsResolvedPathSoThePointerDoesNotReloadItEveryPoll() + { + var real = Copy(Directory.CreateDirectory(Path.Combine(trusted, "real")).FullName); + var link = Path.Combine(trusted, "link.dll"); + File.CreateSymbolicLink(link, real); + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, link); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + + Thread.Sleep(600); // the loader polls the pointer at most every 500 ms + loader.Update(0.01f, new InputState()); + var afterFirst = loader.Runtime; // the link was loaded once, by its resolved path + Thread.Sleep(600); + loader.Update(0.01f, new InputState()); + + Assert.Same(afterFirst, loader.Runtime); + } + + [SymlinkFact] + public void TheTrustedRootIsPinnedAtConstructionAndNotReResolved() + { + var first = Directory.CreateDirectory(Path.Combine(work, "first")).FullName; + var second = Directory.CreateDirectory(Path.Combine(work, "second")).FullName; + var rootLink = Path.Combine(work, "root"); + Directory.CreateSymbolicLink(rootLink, first); + using var loader = new ClientRuntimeLoader(Options, Copy(first), trustedRoot: rootLink); + loader.Load(); + + Directory.Delete(rootLink); + Directory.CreateSymbolicLink(rootLink, second); // the root now names another directory + var elsewhere = Copy(second, "other.dll"); + + Assert.Throws(() => loader.Reload(Path.Combine(rootLink, "other.dll"), Options)); + Assert.Throws(() => loader.Reload(elsewhere, Options)); + } + [Fact] public void PointerNamingAnAssemblyOutsideTheRootKeepsThePreviousRuntime() { @@ -106,7 +207,7 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable loader.Load(); var before = loader.Runtime; - Thread.Sleep(600); // the loader polls the pointer at most every 500 ms + Thread.Sleep(600); loader.Update(0.01f, new InputState()); Assert.Same(before, loader.Runtime);