From d44666ec69f3ff103197654bf7259dbee659e4bd Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:45:23 +0000 Subject: [PATCH 1/8] ClientRuntimeLoader: client assemblies must resolve under a trusted root The reload pointer, and the initial --client-assembly, named any path and ran whatever it held in the engine process. Reload now refuses relative and UNC paths and anything that, links followed, lies outside the trusted root (--client-trusted-root or AQUARIUM_CLIENT_TRUSTED_ROOT; default: the directory of the initial assembly). dev-reload passes its slot directory as the root. Co-Authored-By: Claude Sonnet 5.5 --- scripts/dev-reload.ps1 | 3 +- src/Aquarium.Engine/AquariumHost.cs | 16 ++- .../Runtime/ClientRuntimeLoader.cs | 64 +++++++++++- .../Aquarium.Engine.Tests.csproj | 1 + .../ClientRuntimeLoaderTrustTests.cs | 97 +++++++++++++++++++ 5 files changed, 177 insertions(+), 4 deletions(-) create mode 100644 tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs diff --git a/scripts/dev-reload.ps1 b/scripts/dev-reload.ps1 index 26a9520..391b40f 100644 --- a/scripts/dev-reload.ps1 +++ b/scripts/dev-reload.ps1 @@ -195,7 +195,8 @@ function Start-AquariumProcess { "--cache", $cultCachePath, "--shader-source", (Join-Path $SlotPath "Render\Shaders\D3D12HeightField.hlsl"), "--client-assembly", $LiveAssemblyPath, - "--client-reload-pointer", $liveReloadPointerPath + "--client-reload-pointer", $liveReloadPointerPath, + "--client-trusted-root", $slotRoot ) if ($RunHeadless) { $arguments += "--headless" diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index 7ba0c72..4f8af38 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -18,7 +18,7 @@ public static class AquariumHost ParseFieldReservoirMode(args), ParseFieldReservoirScale(args), ParseFieldReservoirSpatialReuseBudget(args)); - using var runtimeLoader = new ClientRuntimeLoader(runtimeOptions, ParseClientAssemblyPath(args), ParseClientReloadPointerPath(args)); + using var runtimeLoader = new ClientRuntimeLoader(runtimeOptions, ParseClientAssemblyPath(args), ParseClientReloadPointerPath(args), ParseClientTrustedRoot(args)); var runtime = runtimeLoader.Load(); if (runtimeOptions.RenderDebugModeOverride is { } renderDebugModeOverride) { @@ -258,6 +258,20 @@ public static class AquariumHost ?? Environment.GetEnvironmentVariable("AQUARIUM_LIVE_ASSEMBLY"); } + private static string? ParseClientTrustedRoot(IReadOnlyCollection args) + { + var values = args.ToArray(); + for (var index = 0; index < values.Length - 1; index++) + { + if (string.Equals(values[index], "--client-trusted-root", StringComparison.OrdinalIgnoreCase)) + { + return values[index + 1]; + } + } + + return Environment.GetEnvironmentVariable("AQUARIUM_CLIENT_TRUSTED_ROOT"); + } + private static string? ParseClientReloadPointerPath(IReadOnlyCollection args) { var values = args.ToArray(); diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index 950e974..0874af0 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -8,16 +8,29 @@ public sealed class ClientRuntimeLoader : IDisposable { private readonly string? clientAssemblyPath; private readonly string? clientReloadPointerPath; + private readonly string? trustedRoot; private readonly Stopwatch reloadClock = Stopwatch.StartNew(); private readonly AquariumRuntimeOptions options; private ReloadedRuntime? current; private TimeSpan lastReloadCheck; private string? currentAssemblyPath; - public ClientRuntimeLoader(AquariumRuntimeOptions options, string? clientAssemblyPath = null, string? clientReloadPointerPath = null) + /// + /// 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. + /// + public ClientRuntimeLoader( + AquariumRuntimeOptions options, + string? clientAssemblyPath = null, + string? clientReloadPointerPath = null, + string? trustedRoot = null) { - this.clientAssemblyPath = clientAssemblyPath; + this.clientAssemblyPath = string.IsNullOrWhiteSpace(clientAssemblyPath) ? null : Path.GetFullPath(clientAssemblyPath); this.clientReloadPointerPath = clientReloadPointerPath; + this.trustedRoot = string.IsNullOrWhiteSpace(trustedRoot) + ? this.clientAssemblyPath is null ? null : Path.GetDirectoryName(this.clientAssemblyPath) + : Path.GetFullPath(trustedRoot); this.options = options; } @@ -42,6 +55,7 @@ public sealed class ClientRuntimeLoader : IDisposable public void Reload(string assemblyPath, AquariumRuntimeOptions options) { + RequireTrusted(assemblyPath); var previous = current; var loadContext = new ClientAssemblyLoadContext(assemblyPath); IAquariumRuntime? nextRuntime = null; @@ -122,6 +136,52 @@ public sealed class ClientRuntimeLoader : IDisposable } } + private void RequireTrusted(string assemblyPath) + { + if (trustedRoot is null) + { + throw new InvalidOperationException("No trusted root for client runtime assemblies is set."); + } + + 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))); + if (relative == ".." || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || Path.IsPathRooted(relative)) + { + throw new InvalidOperationException($"Client runtime assembly is outside the trusted root '{trustedRoot}': {assemblyPath}"); + } + } + + // The path with every symlink or junction on it replaced by its final target. + private static string ResolveLinks(string path, int depth = 0) + { + if (depth > 32) + { + throw new InvalidOperationException($"Too many links resolving {path}."); + } + + for (var probe = path; probe is not null; probe = Path.GetDirectoryName(probe)) + { + FileSystemInfo info = Directory.Exists(probe) ? new DirectoryInfo(probe) : new FileInfo(probe); + if (info.LinkTarget is null) + { + continue; + } + + var target = info.ResolveLinkTarget(returnFinalTarget: true)?.FullName + ?? throw new InvalidOperationException($"Link does not resolve: {probe}"); + var rest = Path.GetRelativePath(probe, path); + return ResolveLinks(rest == "." ? target : Path.GetFullPath(Path.Combine(target, rest)), depth + 1); + } + + return path; + } + private string RequireClientAssemblyPath() { if (!string.IsNullOrWhiteSpace(clientAssemblyPath)) diff --git a/tests/Aquarium.Engine.Tests/Aquarium.Engine.Tests.csproj b/tests/Aquarium.Engine.Tests/Aquarium.Engine.Tests.csproj index 63d3d04..7937bdd 100644 --- a/tests/Aquarium.Engine.Tests/Aquarium.Engine.Tests.csproj +++ b/tests/Aquarium.Engine.Tests/Aquarium.Engine.Tests.csproj @@ -20,6 +20,7 @@ + diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs new file mode 100644 index 0000000..0ef0a7b --- /dev/null +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -0,0 +1,97 @@ +using Aquarium.Engine; +using Aquarium.Engine.Input; +using Aquarium.Engine.Runtime; + +namespace Aquarium.Engine.Tests; + +public sealed class ClientRuntimeLoaderTrustTests : IDisposable +{ + private static readonly AquariumRuntimeOptions Options = new(true, null); + private readonly string work = Path.Combine(Path.GetTempPath(), "aquarium-trust-" + Guid.NewGuid().ToString("N")); + private readonly string trusted; + private readonly string outside; + + public ClientRuntimeLoaderTrustTests() + { + trusted = Directory.CreateDirectory(Path.Combine(work, "trusted")).FullName; + outside = Directory.CreateDirectory(Path.Combine(work, "outside")).FullName; + } + + public void Dispose() => Directory.Delete(work, recursive: true); + + // A real client assembly: the minimal sample, copied so its location is ours to choose. + private static string Copy(string directory, string name = "client.dll") + { + var path = Path.Combine(directory, name); + File.Copy(typeof(Aquarium.Sample.Minimal.MinimalRuntime).Assembly.Location, path); + return path; + } + + [Fact] + public void AssemblyInsideTheTrustedRootLoadsAndReloads() + { + var first = Copy(trusted, "a.dll"); + var second = Copy(Directory.CreateDirectory(Path.Combine(trusted, "slot2")).FullName); + using var loader = new ClientRuntimeLoader(Options, first); + loader.Load(); + loader.Reload(second, Options); + Assert.NotNull(loader.Runtime); + } + + [Fact] + public void InitialAssemblyOutsideTheExplicitRootIsRefused() + { + using var loader = new ClientRuntimeLoader(Options, Copy(outside), trustedRoot: trusted); + Assert.Throws(() => loader.Load()); + } + + [Fact] + public void ReloadOutsideTheRootDotDotRelativeAndUncPathsAreRefused() + { + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); + loader.Load(); + var escaped = Copy(outside); + 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)); + } + + [Fact] + 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 + } + + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); + loader.Load(); + Assert.Throws(() => loader.Reload(fileLink, Options)); + Assert.Throws(() => loader.Reload(Path.Combine(dirLink, "client.dll"), Options)); + } + + [Fact] + public void PointerNamingAnAssemblyOutsideTheRootKeepsThePreviousRuntime() + { + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, Copy(outside)); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + var before = loader.Runtime; + + Thread.Sleep(600); // the loader polls the pointer at most every 500 ms + loader.Update(0.01f, new InputState()); + + Assert.Same(before, loader.Runtime); + } +} -- 2.47.3 From e0fc49b7201ecb68668cdae436b7c36128f409c8 Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:47:56 +0000 Subject: [PATCH 2/8] dev-reload: trusted root is the dev root, so dev-watch live-slots reloads load dev-watch writes the reload pointer under artifacts/dev-reload/live-slots, a sibling of slots; a slot-directory root refused every watch reload and the reopen path. Test: a sibling directory loads inside an explicit root, not under the default root, and an outside assembly is still refused. Co-Authored-By: Claude Sonnet 5.5 --- scripts/dev-reload.ps1 | 2 +- .../ClientRuntimeLoaderTrustTests.cs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/scripts/dev-reload.ps1 b/scripts/dev-reload.ps1 index 391b40f..7ab8ad3 100644 --- a/scripts/dev-reload.ps1 +++ b/scripts/dev-reload.ps1 @@ -196,7 +196,7 @@ function Start-AquariumProcess { "--shader-source", (Join-Path $SlotPath "Render\Shaders\D3D12HeightField.hlsl"), "--client-assembly", $LiveAssemblyPath, "--client-reload-pointer", $liveReloadPointerPath, - "--client-trusted-root", $slotRoot + "--client-trusted-root", $devRoot ) if ($RunHeadless) { $arguments += "--headless" diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index 0ef0a7b..afe9c03 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -38,6 +38,23 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.NotNull(loader.Runtime); } + [Fact] + public void SiblingDirectoryOfTheInitialAssemblyLoadsOnlyInsideTheExplicitRoot() + { + // dev-watch reloads from live-slots, a sibling of the slot the host started in. + var slot = Directory.CreateDirectory(Path.Combine(trusted, "slots", "s1")).FullName; + var live = Directory.CreateDirectory(Path.Combine(trusted, "live-slots", "s2")).FullName; + var sibling = Copy(live); + 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(); + Assert.Throws(() => narrow.Reload(sibling, Options)); // default root is the initial directory + } + [Fact] public void InitialAssemblyOutsideTheExplicitRootIsRefused() { -- 2.47.3 From 7ec7a0f909ca9a1b3ad03c744cbfefa29005eeee Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:51:45 +0000 Subject: [PATCH 3/8] 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); -- 2.47.3 From 45b0b38a2e11e55a41d656ce6f47e5d23cb73eb5 Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:57:09 +0000 Subject: [PATCH 4/8] Trusted root: assemblies-only dir, env cleared on every path, string checks first, escaped logs Review (Heimdall): dev scripts keep slots and live-slots under artifacts/dev-reload/assemblies and pass that as the root, not the whole dev root; --client-trusted-root refuses an empty or missing value and the environment variable is cleared on every path; the UNC/fully-qualified string rule runs before File.Exists on a pointer target; control characters are escaped in every logged path. Co-Authored-By: Claude Sonnet 5.5 --- scripts/dev-reload.ps1 | 7 +- scripts/dev-watch.ps1 | 2 +- src/Aquarium.Engine/AquariumHost.cs | 30 ++++-- .../Runtime/ClientRuntimeLoader.cs | 55 +++++++--- .../ClientRuntimeLoaderTrustTests.cs | 102 ++++++++++++++++++ 5 files changed, 169 insertions(+), 27 deletions(-) diff --git a/scripts/dev-reload.ps1 b/scripts/dev-reload.ps1 index 7ab8ad3..aa86abb 100644 --- a/scripts/dev-reload.ps1 +++ b/scripts/dev-reload.ps1 @@ -18,7 +18,10 @@ $liveProjectPath = if ([System.IO.Path]::IsPathRooted($ClientProject)) { $Client $liveProjectPath = (Resolve-Path $liveProjectPath).Path $liveAssemblyName = [System.IO.Path]::GetFileNameWithoutExtension($liveProjectPath) $devRoot = Join-Path $repoRoot "artifacts\dev-reload" -$slotRoot = Join-Path $devRoot "slots" +# Everything the host may load as client code lives under $assemblyRoot (slots here, live-slots from dev-watch); +# it is the host's trusted root, so state, cache and logs in $devRoot are not part of it. +$assemblyRoot = Join-Path $devRoot "assemblies" +$slotRoot = Join-Path $assemblyRoot "slots" $visibleStatePath = Join-Path $devRoot "state.clixml" $headlessStatePath = Join-Path $devRoot "headless-state.clixml" $statePath = if ($Headless) { $headlessStatePath } else { $visibleStatePath } @@ -196,7 +199,7 @@ function Start-AquariumProcess { "--shader-source", (Join-Path $SlotPath "Render\Shaders\D3D12HeightField.hlsl"), "--client-assembly", $LiveAssemblyPath, "--client-reload-pointer", $liveReloadPointerPath, - "--client-trusted-root", $devRoot + "--client-trusted-root", $assemblyRoot ) if ($RunHeadless) { $arguments += "--headless" diff --git a/scripts/dev-watch.ps1 b/scripts/dev-watch.ps1 index cc9ef3c..10bd9ca 100644 --- a/scripts/dev-watch.ps1 +++ b/scripts/dev-watch.ps1 @@ -20,7 +20,7 @@ $liveProjectPath = (Resolve-Path $liveProjectPath).Path $liveAssemblyName = [System.IO.Path]::GetFileNameWithoutExtension($liveProjectPath) $clientSourceRoot = Split-Path $liveProjectPath $watchLogPath = Join-Path $repoRoot "artifacts\dev-reload\watch.log" -$liveSlotRoot = Join-Path $repoRoot "artifacts\dev-reload\live-slots" +$liveSlotRoot = Join-Path $repoRoot "artifacts\dev-reload\assemblies\live-slots" $liveReloadPointerPath = Join-Path $repoRoot "artifacts\dev-reload\live-current.txt" $shaderPath = Join-Path $repoRoot "src\Aquarium.Engine\Render\Shaders\Aquarium.hlsl" $engineShaderRoot = Join-Path $repoRoot "src\Aquarium.Engine\Render\Shaders" diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index 8404576..edf5d42 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -258,20 +258,28 @@ public static class AquariumHost ?? Environment.GetEnvironmentVariable("AQUARIUM_LIVE_ASSEMBLY"); } - private static string? ParseClientTrustedRoot(IReadOnlyCollection args) + internal static string? ParseClientTrustedRoot(IReadOnlyCollection args) { - var values = args.ToArray(); - for (var index = 0; index < values.Length - 1; index++) - { - if (string.Equals(values[index], "--client-trusted-root", StringComparison.OrdinalIgnoreCase)) - { - return values[index + 1]; - } - } - - // Read once and removed, so processes this one starts do not inherit a root meant for it. + // Read once and removed on every path, 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); + + var values = args.ToArray(); + for (var index = 0; index < values.Length; index++) + { + if (!string.Equals(values[index], "--client-trusted-root", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (index + 1 >= values.Length || string.IsNullOrWhiteSpace(values[index + 1])) + { + throw new ArgumentException("--client-trusted-root needs a directory; an empty value would fall back to the default root."); + } + + return values[index + 1]; + } + return fromEnvironment; } diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index 60f64fc..a032d96 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -42,10 +42,10 @@ public sealed class ClientRuntimeLoader : IDisposable 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)); + throw new ArgumentException($"A drive or volume root cannot be the client runtime trusted root: {Printable(root)}", nameof(trustedRoot)); } - Console.WriteLine($"Client runtime trusted root: {root}"); + Console.WriteLine($"Client runtime trusted root: {Printable(root)}"); } this.trustedRoot = root; @@ -136,13 +136,19 @@ public sealed class ClientRuntimeLoader : IDisposable return; } - if (string.IsNullOrWhiteSpace(candidatePath) || !File.Exists(candidatePath)) + if (string.IsNullOrWhiteSpace(candidatePath)) { return; } try { + RequireLocalFullyQualified(candidatePath); // string rules first: no filesystem or network probe of a UNC path + if (!File.Exists(candidatePath)) + { + return; + } + var resolved = ResolveTrusted(candidatePath); if (string.Equals(resolved, currentAssemblyPath, PathComparison)) { @@ -151,11 +157,11 @@ public sealed class ClientRuntimeLoader : IDisposable current?.Runtime.FlushState(); LoadResolved(resolved, options); - Console.WriteLine($"Client runtime reload applied: {resolved}"); + Console.WriteLine($"Client runtime reload applied: {Printable(resolved)}"); } catch (Exception error) { - Console.Error.WriteLine($"Client runtime reload failed; keeping previous runtime. {error.Message}"); + Console.Error.WriteLine($"Client runtime reload failed; keeping previous runtime. {Printable(error.Message)}"); } } @@ -170,21 +176,44 @@ public sealed class ClientRuntimeLoader : IDisposable // 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 (!Path.IsPathFullyQualified(path) || (path.Length >= 2 && IsSeparator(path[0]) && IsSeparator(path[1]))) - { - throw new InvalidOperationException($"Client runtime path must be local and fully qualified: {path}"); - } - + RequireLocalFullyQualified(path); 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 path is outside the trusted root '{resolvedRoot}': {path}"); + throw new InvalidOperationException($"Client runtime path is outside the trusted root '{Printable(resolvedRoot)}': {Printable(path)}"); } return resolved; } + internal static void RequireLocalFullyQualified(string path) + { + if (!Path.IsPathFullyQualified(path) || (path.Length >= 2 && IsSeparator(path[0]) && IsSeparator(path[1]))) + { + throw new InvalidOperationException($"Client runtime path must be local and fully qualified: {Printable(path)}"); + } + } + + // Paths come from files other people can write: show control characters escaped, never as raw log lines. + internal static string Printable(string text) + { + var builder = new System.Text.StringBuilder(text.Length); + foreach (var c in text) + { + if (char.IsControl(c)) + { + builder.Append(c switch { '\n' => "\\n", '\r' => "\\r", '\t' => "\\t", _ => $"\\u{(int)c:x4}" }); + } + else + { + builder.Append(c); + } + } + + return builder.ToString(); + } + private static bool IsSeparator(char c) => c is '/' or '\\'; // The path with every symlink or junction on it replaced by its final target. @@ -192,7 +221,7 @@ public sealed class ClientRuntimeLoader : IDisposable { if (depth > 32) { - throw new InvalidOperationException($"Too many links resolving {path}."); + throw new InvalidOperationException($"Too many links resolving {Printable(path)}."); } for (var probe = path; probe is not null; probe = Path.GetDirectoryName(probe)) @@ -204,7 +233,7 @@ public sealed class ClientRuntimeLoader : IDisposable } var target = info.ResolveLinkTarget(returnFinalTarget: true)?.FullName - ?? throw new InvalidOperationException($"Link does not resolve: {probe}"); + ?? throw new InvalidOperationException($"Link does not resolve: {Printable(probe)}"); var rest = Path.GetRelativePath(probe, path); return ResolveLinks(rest == "." ? target : Path.GetFullPath(Path.Combine(target, rest)), depth + 1); } diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index 57a6b9f..bebacc8 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -212,4 +212,106 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Same(before, loader.Runtime); } + + [Fact] + public void AnUncPointerIsRefusedAndReportedBeforeAnyFilesystemProbe() + { + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, "//no-such-host.invalid/share/client.dll"); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + + var errors = CaptureError(() => + { + Thread.Sleep(600); + loader.Update(0.01f, new InputState()); + }); + + // File.Exists on this path would return false silently; the string rule must speak first. + Assert.Contains("local and fully qualified", errors); + } + + [Fact] + public void ControlCharactersInAPointerTargetAreEscapedInTheLog() + { + var hostile = Path.Combine(outside, "a\nClient runtime reload applied: forged.dll"); + File.Copy(typeof(Aquarium.Sample.Minimal.MinimalRuntime).Assembly.Location, hostile); + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, hostile); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + + var errors = CaptureError(() => + { + Thread.Sleep(600); + loader.Update(0.01f, new InputState()); + }); + + Assert.Contains("outside the trusted root", errors); + Assert.DoesNotContain("\nClient runtime reload applied", errors); + Assert.Contains("\\nClient runtime reload applied", errors); + Assert.Equal("a\\u0007b", ClientRuntimeLoader.Printable("a\u0007b")); + } + + [Fact] + public void TheTrustedRootEnvironmentVariableIsClearedOnEveryPathAndAnEmptyFlagIsRefused() + { + const string Name = "AQUARIUM_CLIENT_TRUSTED_ROOT"; + Environment.SetEnvironmentVariable(Name, "/from/env"); + Assert.Equal("/from/flag", AquariumHost.ParseClientTrustedRoot(["--client-trusted-root", "/from/flag"])); + Assert.Null(Environment.GetEnvironmentVariable(Name)); + + Environment.SetEnvironmentVariable(Name, "/from/env"); + Assert.Equal("/from/env", AquariumHost.ParseClientTrustedRoot([])); + Assert.Null(Environment.GetEnvironmentVariable(Name)); + + Environment.SetEnvironmentVariable(Name, "/from/env"); + Assert.Throws(() => AquariumHost.ParseClientTrustedRoot(["--client-trusted-root", ""])); + Assert.Null(Environment.GetEnvironmentVariable(Name)); + + Environment.SetEnvironmentVariable(Name, "/from/env"); + Assert.Throws(() => AquariumHost.ParseClientTrustedRoot(["--client-trusted-root"])); + Assert.Null(Environment.GetEnvironmentVariable(Name)); + } + + [Fact] + public void DevScriptsKeepEverythingLoadableUnderOneAssembliesRootThatIsNotTheWholeDevRoot() + { + var scripts = Path.Combine(RepositoryRoot(), "scripts"); + var reload = File.ReadAllText(Path.Combine(scripts, "dev-reload.ps1")); + var watch = File.ReadAllText(Path.Combine(scripts, "dev-watch.ps1")); + + Assert.Contains("\"--client-trusted-root\", $assemblyRoot", reload); + Assert.DoesNotContain("\"--client-trusted-root\", $devRoot", reload); + Assert.Contains("$slotRoot = Join-Path $assemblyRoot \"slots\"", reload); + Assert.Contains("artifacts\\dev-reload\\assemblies\\live-slots", watch); + } + + private static string CaptureError(Action action) + { + var original = Console.Error; + using var captured = new StringWriter(); + Console.SetError(captured); + try + { + action(); + } + finally + { + Console.SetError(original); + } + + return captured.ToString(); + } + + private static string RepositoryRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "Fensalir.sln"))) + { + directory = directory.Parent; + } + + return directory?.FullName ?? throw new InvalidOperationException("Fensalir.sln not found above the test binaries."); + } } -- 2.47.3 From 467a97f5404b7cb14026783154ea6a23f6f740ff Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 11:04:00 +0000 Subject: [PATCH 5/8] Trusted root: lexical check before any filesystem access, cached env setting, stricter log escaping Review (Heimdall): a path outside the pinned root (as resolved or as given) is refused by string comparison before File.Exists or any link is followed; the trusted-root setting is its own small type that takes the environment variable once, caches it and refuses blank values; Printable also escapes backslash, format characters (bidi, zero-width) and the line and paragraph separators; dev-watch matches the host ack by the unique slot leaf with doubled backslashes; dev-reload prunes live-slots and rejects stale reopen state up front. Co-Authored-By: Claude Sonnet 5.5 --- scripts/dev-reload.ps1 | 21 +++- scripts/dev-watch.ps1 | 7 +- src/Aquarium.Engine/AquariumHost.cs | 25 +---- .../Runtime/ClientRuntimeLoader.cs | 40 +++++-- .../Runtime/TrustedRootSetting.cs | 48 +++++++++ .../ClientRuntimeLoaderTrustTests.cs | 101 +++++++++++++++--- 6 files changed, 196 insertions(+), 46 deletions(-) create mode 100644 src/Aquarium.Engine/Runtime/TrustedRootSetting.cs diff --git a/scripts/dev-reload.ps1 b/scripts/dev-reload.ps1 index aa86abb..92e7bc0 100644 --- a/scripts/dev-reload.ps1 +++ b/scripts/dev-reload.ps1 @@ -22,6 +22,7 @@ $devRoot = Join-Path $repoRoot "artifacts\dev-reload" # it is the host's trusted root, so state, cache and logs in $devRoot are not part of it. $assemblyRoot = Join-Path $devRoot "assemblies" $slotRoot = Join-Path $assemblyRoot "slots" +$liveSlotRoot = Join-Path $assemblyRoot "live-slots" $visibleStatePath = Join-Path $devRoot "state.clixml" $headlessStatePath = Join-Path $devRoot "headless-state.clixml" $statePath = if ($Headless) { $headlessStatePath } else { $visibleStatePath } @@ -82,9 +83,12 @@ function Remove-OldSlots { return } - $slots = Get-ChildItem -Path $slotRoot -Directory -ErrorAction SilentlyContinue | - Sort-Object LastWriteTimeUtc -Descending | - Select-Object -Skip $RetainSlots + # Both roots hold client code the host may load; dev-watch adds a live slot per change and never prunes. + $slots = foreach ($root in @($slotRoot, $liveSlotRoot)) { + Get-ChildItem -Path $root -Directory -ErrorAction SilentlyContinue | + Sort-Object LastWriteTimeUtc -Descending | + Select-Object -Skip $RetainSlots + } foreach ($slot in $slots) { try { @@ -245,6 +249,17 @@ function Reopen-VisibleProcess { $liveAssemblyPath = [string]$state.liveAssembly } + # Stale state fails here, in words, instead of as a refused load inside the host. + $assemblyRootPrefix = [System.IO.Path]::GetFullPath($assemblyRoot).TrimEnd('\') + '\' + foreach ($recorded in @($slotPath, $liveAssemblyPath)) { + if (-not [System.IO.Path]::GetFullPath($recorded).StartsWith($assemblyRootPrefix, [System.StringComparison]::OrdinalIgnoreCase)) { + throw "Visible dev-reload state names '$recorded', outside the assemblies root '$assemblyRoot' (state from before the trusted-root layout, or the root moved). Run dev-reload without -Reopen to rebuild." + } + } + if (-not (Test-Path -LiteralPath $liveAssemblyPath)) { + throw "Visible dev-reload state names '$liveAssemblyPath', which no longer exists (pruned slot). Run dev-reload without -Reopen to rebuild." + } + $process = Start-AquariumProcess -SlotPath $slotPath -LiveAssemblyPath $liveAssemblyPath -RunHeadless:$false Wait-ForStartedAquarium -Process $process -ExpectWindow:$true diff --git a/scripts/dev-watch.ps1 b/scripts/dev-watch.ps1 index 10bd9ca..0b9c9cb 100644 --- a/scripts/dev-watch.ps1 +++ b/scripts/dev-watch.ps1 @@ -455,12 +455,15 @@ function Wait-LiveReloadAcknowledged { $stderrLog = if ($state.stderrLog) { [string]$state.stderrLog } else { $null } $timeoutSeconds = [Math]::Max(1, $LiveReloadAckTimeoutSeconds) $deadline = (Get-Date).AddSeconds($timeoutSeconds) - $appliedLine = "Client runtime reload applied: $liveAssemblyPath" + # The host logs the resolved path with backslashes doubled, which can differ from the path built here when the + # repo sits under a link. The slot directory name is unique, so match its leaf and the assembly name. + $slotLeaf = Split-Path -Leaf (Split-Path -Parent $liveAssemblyPath) + $appliedSuffix = ("$slotLeaf\" + (Split-Path -Leaf $liveAssemblyPath)).Replace("\", "\\") while ((Get-Date) -lt $deadline) { if (Test-Path $stdoutLog) { $stdoutTail = (Get-Content -Path $stdoutLog -Tail 120 -ErrorAction SilentlyContinue) -join "`n" - if ($stdoutTail.Contains($appliedLine)) { + if ($stdoutTail -match ("(?m)^Client runtime reload applied: .*" + [regex]::Escape($appliedSuffix) + "\s*$")) { Write-WatchLog "Live reload acknowledged by host: $liveAssemblyPath" return } diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index edf5d42..1b0d430 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -258,30 +258,9 @@ public static class AquariumHost ?? Environment.GetEnvironmentVariable("AQUARIUM_LIVE_ASSEMBLY"); } - internal static string? ParseClientTrustedRoot(IReadOnlyCollection args) - { - // Read once and removed on every path, 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); + private static readonly TrustedRootSetting TrustedRoot = TrustedRootSetting.FromProcessEnvironment(); - var values = args.ToArray(); - for (var index = 0; index < values.Length; index++) - { - if (!string.Equals(values[index], "--client-trusted-root", StringComparison.OrdinalIgnoreCase)) - { - continue; - } - - if (index + 1 >= values.Length || string.IsNullOrWhiteSpace(values[index + 1])) - { - throw new ArgumentException("--client-trusted-root needs a directory; an empty value would fall back to the default root."); - } - - return values[index + 1]; - } - - return fromEnvironment; - } + private static string? ParseClientTrustedRoot(IReadOnlyCollection args) => TrustedRoot.Resolve(args); private static string? ParseClientReloadPointerPath(IReadOnlyCollection args) { diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index a032d96..a9c227a 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -9,6 +9,7 @@ public sealed class ClientRuntimeLoader : IDisposable private readonly string? clientAssemblyPath; private readonly string? clientReloadPointerPath; private readonly string? trustedRoot; + private readonly string? trustedRootAsGiven; private readonly Stopwatch reloadClock = Stopwatch.StartNew(); private readonly AquariumRuntimeOptions options; private ReloadedRuntime? current; @@ -34,6 +35,7 @@ public sealed class ClientRuntimeLoader : IDisposable var root = string.IsNullOrWhiteSpace(trustedRoot) ? this.clientAssemblyPath is null ? null : Path.GetDirectoryName(this.clientAssemblyPath) : Path.GetFullPath(trustedRoot); + this.trustedRootAsGiven = root; if (root is not null) { root = ResolveLinks(root); @@ -144,6 +146,7 @@ public sealed class ClientRuntimeLoader : IDisposable try { RequireLocalFullyQualified(candidatePath); // string rules first: no filesystem or network probe of a UNC path + RequireLexicallyUnderRoot(candidatePath); // and no probe of anything outside the root, whatever a link there points at if (!File.Exists(candidatePath)) { return; @@ -171,15 +174,34 @@ public sealed class ClientRuntimeLoader : IDisposable private string ResolveTrusted(string assemblyPath) => trustedRoot is null ? throw new InvalidOperationException("No trusted root for client runtime assemblies is set.") - : ResolveTrusted(trustedRoot, assemblyPath); + : ResolveTrusted(trustedRoot, assemblyPath, trustedRootAsGiven); + + private void RequireLexicallyUnderRoot(string path) => + RequireLexicallyUnder(Path.GetFullPath(path), trustedRoot, trustedRootAsGiven); + + // Pure string comparison against the root as resolved and as given: a path outside both is refused before any + // filesystem access, so a link there (to a UNC share, a mapped drive) is never followed or probed. + private static void RequireLexicallyUnder(string fullPath, params string?[] roots) + { + if (!roots.Any(root => root is not null && IsUnder(root, fullPath))) + { + throw new InvalidOperationException($"Client runtime path is outside the trusted root '{Printable(roots[0] ?? "")}': {Printable(fullPath)}"); + } + } + + private static bool IsUnder(string root, string path) + { + var relative = Path.GetRelativePath(root, path); + return !(relative == ".." || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || Path.IsPathRooted(relative)); + } // The final path of a local, fully qualified file under the already-resolved root; throws otherwise. - internal static string ResolveTrusted(string resolvedRoot, string path) + internal static string ResolveTrusted(string resolvedRoot, string path, string? rootAsGiven = null) { RequireLocalFullyQualified(path); + RequireLexicallyUnder(Path.GetFullPath(path), resolvedRoot, rootAsGiven); var resolved = ResolveLinks(Path.GetFullPath(path)); - var relative = Path.GetRelativePath(resolvedRoot, resolved); - if (relative == ".." || relative.StartsWith(".." + Path.DirectorySeparatorChar, StringComparison.Ordinal) || Path.IsPathRooted(relative)) + if (!IsUnder(resolvedRoot, resolved)) { throw new InvalidOperationException($"Client runtime path is outside the trusted root '{Printable(resolvedRoot)}': {Printable(path)}"); } @@ -195,13 +217,19 @@ public sealed class ClientRuntimeLoader : IDisposable } } - // Paths come from files other people can write: show control characters escaped, never as raw log lines. + // Paths come from files other people can write: backslashes, control, format (bidi, zero-width) and line/paragraph + // separator characters are escaped, so a name cannot forge or hide a log line. internal static string Printable(string text) { var builder = new System.Text.StringBuilder(text.Length); foreach (var c in text) { - if (char.IsControl(c)) + if (c == '\\') + { + builder.Append("\\\\"); + } + else if (char.IsControl(c) || char.GetUnicodeCategory(c) is System.Globalization.UnicodeCategory.Format + or System.Globalization.UnicodeCategory.LineSeparator or System.Globalization.UnicodeCategory.ParagraphSeparator) { builder.Append(c switch { '\n' => "\\n", '\r' => "\\r", '\t' => "\\t", _ => $"\\u{(int)c:x4}" }); } diff --git a/src/Aquarium.Engine/Runtime/TrustedRootSetting.cs b/src/Aquarium.Engine/Runtime/TrustedRootSetting.cs new file mode 100644 index 0000000..f99bd2e --- /dev/null +++ b/src/Aquarium.Engine/Runtime/TrustedRootSetting.cs @@ -0,0 +1,48 @@ +namespace Aquarium.Engine.Runtime; + +/// +/// The client runtime trusted root as given on the command line or in AQUARIUM_CLIENT_TRUSTED_ROOT. The environment +/// value is taken (read and cleared) the first time anything is resolved, on every path, and cached: children do not +/// inherit it and a second resolution does not silently fall back to the default root. A blank flag value, a flag with +/// no value or a blank environment value is refused. +/// +internal sealed class TrustedRootSetting(Func takeEnvironment) +{ + private const string Flag = "--client-trusted-root"; + private const string Variable = "AQUARIUM_CLIENT_TRUSTED_ROOT"; + private readonly Lazy fromEnvironment = new(takeEnvironment); + + public static TrustedRootSetting FromProcessEnvironment() => new(() => + { + var value = Environment.GetEnvironmentVariable(Variable); + Environment.SetEnvironmentVariable(Variable, null); + return value; + }); + + public string? Resolve(IReadOnlyCollection args) + { + var environment = fromEnvironment.Value; + if (environment is not null && string.IsNullOrWhiteSpace(environment)) + { + throw new ArgumentException($"{Variable} is set but empty; an empty value would fall back to the default root."); + } + + var values = args.ToArray(); + for (var index = 0; index < values.Length; index++) + { + if (!string.Equals(values[index], Flag, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (index + 1 >= values.Length || string.IsNullOrWhiteSpace(values[index + 1])) + { + throw new ArgumentException($"{Flag} needs a directory; an empty value would fall back to the default root."); + } + + return values[index + 1]; + } + + return environment; + } +} diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index bebacc8..2dde1e6 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -254,24 +254,87 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable } [Fact] - public void TheTrustedRootEnvironmentVariableIsClearedOnEveryPathAndAnEmptyFlagIsRefused() + public void TheTrustedRootEnvironmentVariableIsTakenOnceOnEveryPathAndCached() { const string Name = "AQUARIUM_CLIENT_TRUSTED_ROOT"; Environment.SetEnvironmentVariable(Name, "/from/env"); - Assert.Equal("/from/flag", AquariumHost.ParseClientTrustedRoot(["--client-trusted-root", "/from/flag"])); - Assert.Null(Environment.GetEnvironmentVariable(Name)); + var setting = TrustedRootSetting.FromProcessEnvironment(); + Assert.Equal("/from/flag", setting.Resolve(["--client-trusted-root", "/from/flag"])); + Assert.Null(Environment.GetEnvironmentVariable(Name)); // cleared although the flag won + Assert.Equal("/from/env", setting.Resolve([])); // cached: a second resolution does not fall to the default root + } - Environment.SetEnvironmentVariable(Name, "/from/env"); - Assert.Equal("/from/env", AquariumHost.ParseClientTrustedRoot([])); - Assert.Null(Environment.GetEnvironmentVariable(Name)); + [Theory] + [InlineData("")] + [InlineData(" ")] + public void ABlankEnvironmentValueIsRefusedLikeABlankFlag(string value) + { + Assert.Throws(() => new TrustedRootSetting(() => value).Resolve([])); + Assert.Throws(() => new TrustedRootSetting(() => null).Resolve(["--client-trusted-root", value])); + Assert.Throws(() => new TrustedRootSetting(() => null).Resolve(["--client-trusted-root"])); + } - Environment.SetEnvironmentVariable(Name, "/from/env"); - Assert.Throws(() => AquariumHost.ParseClientTrustedRoot(["--client-trusted-root", ""])); - Assert.Null(Environment.GetEnvironmentVariable(Name)); + [Fact] + public void APathOutsideTheRootIsRefusedBeforeAnyLinkOnItIsFollowed() + { + // A link loop outside the root: following it would throw "Too many links"; the string check must refuse first. + var a = Path.Combine(outside, "a"); + var b = Path.Combine(outside, "b"); + try + { + Directory.CreateSymbolicLink(a, b); + Directory.CreateSymbolicLink(b, a); + } + catch (Exception error) when (error is IOException or UnauthorizedAccessException) + { + // No links here: the check is still exercised by the nonexistent outside path below. + } - Environment.SetEnvironmentVariable(Name, "/from/env"); - Assert.Throws(() => AquariumHost.ParseClientTrustedRoot(["--client-trusted-root"])); - Assert.Null(Environment.GetEnvironmentVariable(Name)); + var error2 = Assert.Throws( + () => ClientRuntimeLoader.ResolveTrusted(ClientRuntimeLoader.ResolveLinks(trusted), Path.Combine(a, "client.dll"))); + Assert.Contains("outside the trusted root", error2.Message); + } + + [Fact] + public void ANonexistentPointerTargetOutsideTheRootIsRefusedAndReportedNotSilentlyIgnored() + { + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, Path.Combine(outside, "not-there.dll")); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + + var errors = CaptureError(() => + { + Thread.Sleep(600); + loader.Update(0.01f, new InputState()); + }); + + Assert.Contains("outside the trusted root", errors); + } + + [SymlinkFact] + public void ARootReachedThroughALinkAsGivenStillAcceptsPathsUnderIt() + { + var real = Directory.CreateDirectory(Path.Combine(work, "real")).FullName; + var rootLink = Path.Combine(work, "rootlink"); + Directory.CreateSymbolicLink(rootLink, real); + Copy(real, "a.dll"); + var viaLink = Path.Combine(rootLink, "b.dll"); + File.Copy(typeof(Aquarium.Sample.Minimal.MinimalRuntime).Assembly.Location, viaLink); + using var loader = new ClientRuntimeLoader(Options, Path.Combine(rootLink, "a.dll"), trustedRoot: rootLink); + loader.Load(); + loader.Reload(viaLink, Options); + } + + [Fact] + public void LoggedPathsEscapeBackslashesLineSeparatorsAndBidiAndZeroWidthControls() + { + Assert.Equal("a\\\\b", ClientRuntimeLoader.Printable("a\\b")); + Assert.Equal("a\\u2028b", ClientRuntimeLoader.Printable("a\u2028b")); + Assert.Equal("a\\u2029b", ClientRuntimeLoader.Printable("a\u2029b")); + Assert.Equal("a\\u202eb", ClientRuntimeLoader.Printable("a\u202eb")); + Assert.Equal("a\\u200bb", ClientRuntimeLoader.Printable("a\u200bb")); + Assert.Equal("plain-name.dll", ClientRuntimeLoader.Printable("plain-name.dll")); } [Fact] @@ -287,6 +350,20 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Contains("artifacts\\dev-reload\\assemblies\\live-slots", watch); } + [Fact] + public void DevScriptsMatchTheHostAckByItsUniqueSlotLeafPruneLiveSlotsAndRejectStaleReopenState() + { + var scripts = Path.Combine(RepositoryRoot(), "scripts"); + var reload = File.ReadAllText(Path.Combine(scripts, "dev-reload.ps1")); + var watch = File.ReadAllText(Path.Combine(scripts, "dev-watch.ps1")); + + Assert.Contains(".Replace(\"\\\", \"\\\\\")", watch); // the host logs backslashes doubled + Assert.DoesNotContain("Contains($appliedLine)", watch); + Assert.Contains("@($slotRoot, $liveSlotRoot)", reload); + Assert.Contains("outside the assemblies root", reload); + Assert.Contains("no longer exists (pruned slot)", reload); + } + private static string CaptureError(Action action) { var original = Console.Error; -- 2.47.3 From d2f280d9f21807f919235f6b7385629837e0bee4 Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 11:30:47 +0000 Subject: [PATCH 6/8] Client runtime: escape logged paths once by code point, report untrusted mount points as refusals, load assemblies without file locks Exception messages carry raw paths and every console write escapes them once (Printable iterates runes: astral format characters and lone surrogates). AquariumHost reports startup refusals escaped. IOException from link resolution becomes InvalidOperationException. Managed assemblies load from memory so the temp dir can be deleted on Windows. The newline-in-file-name test is skipped on Windows. Co-Authored-By: Claude Sonnet 5.5 --- src/Aquarium.Engine/AquariumHost.cs | 18 ++++- .../Runtime/ClientRuntimeLoader.cs | 75 +++++++++++++++---- .../ClientRuntimeLoaderTrustTests.cs | 66 +++++++++++++++- 3 files changed, 139 insertions(+), 20 deletions(-) diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index 1b0d430..104af9e 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -18,8 +18,22 @@ public static class AquariumHost ParseFieldReservoirMode(args), ParseFieldReservoirScale(args), ParseFieldReservoirSpatialReuseBudget(args)); - using var runtimeLoader = new ClientRuntimeLoader(runtimeOptions, ParseClientAssemblyPath(args), ParseClientReloadPointerPath(args), ParseClientTrustedRoot(args)); - var runtime = runtimeLoader.Load(); + ClientRuntimeLoader? loader = null; + IAquariumRuntime runtime; + try + { + loader = new ClientRuntimeLoader(runtimeOptions, ParseClientAssemblyPath(args), ParseClientReloadPointerPath(args), ParseClientTrustedRoot(args)); + runtime = loader.Load(); + } + catch (Exception error) + { + // Startup refusals carry raw, attacker-writable paths; the console gets them escaped, once, here. + Console.Error.WriteLine($"Client runtime failed to start. {ClientRuntimeLoader.Printable(error.Message)}"); + loader?.Dispose(); + return 1; + } + + using var runtimeLoader = loader; if (runtimeOptions.RenderDebugModeOverride is { } renderDebugModeOverride) { runtime.GraphicsSettings = (runtime.GraphicsSettings with diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index a9c227a..2c69d41 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -44,7 +44,7 @@ public sealed class ClientRuntimeLoader : IDisposable Path.TrimEndingDirectorySeparator(Path.GetPathRoot(root) ?? root), PathComparison)) { - throw new ArgumentException($"A drive or volume root cannot be the client runtime trusted root: {Printable(root)}", nameof(trustedRoot)); + throw new ArgumentException($"A drive or volume root cannot be the client runtime trusted root: {root}", nameof(trustedRoot)); } Console.WriteLine($"Client runtime trusted root: {Printable(root)}"); @@ -83,7 +83,7 @@ public sealed class ClientRuntimeLoader : IDisposable try { - var assembly = loadContext.LoadFromAssemblyPath(assemblyPath); + var assembly = loadContext.LoadFromFileWithoutLock(assemblyPath); var factoryTypes = assembly.GetTypes() .Where(type => typeof(IAquariumRuntimeFactory).IsAssignableFrom(type) && type is { IsAbstract: false, IsInterface: false }) @@ -185,7 +185,7 @@ public sealed class ClientRuntimeLoader : IDisposable { if (!roots.Any(root => root is not null && IsUnder(root, fullPath))) { - throw new InvalidOperationException($"Client runtime path is outside the trusted root '{Printable(roots[0] ?? "")}': {Printable(fullPath)}"); + throw new InvalidOperationException($"Client runtime path is outside the trusted root '{roots[0]}': {fullPath}"); } } @@ -203,7 +203,7 @@ public sealed class ClientRuntimeLoader : IDisposable var resolved = ResolveLinks(Path.GetFullPath(path)); if (!IsUnder(resolvedRoot, resolved)) { - throw new InvalidOperationException($"Client runtime path is outside the trusted root '{Printable(resolvedRoot)}': {Printable(path)}"); + throw new InvalidOperationException($"Client runtime path is outside the trusted root '{resolvedRoot}': {path}"); } return resolved; @@ -213,29 +213,46 @@ public sealed class ClientRuntimeLoader : IDisposable { if (!Path.IsPathFullyQualified(path) || (path.Length >= 2 && IsSeparator(path[0]) && IsSeparator(path[1]))) { - throw new InvalidOperationException($"Client runtime path must be local and fully qualified: {Printable(path)}"); + throw new InvalidOperationException($"Client runtime path must be local and fully qualified: {path}"); } } - // Paths come from files other people can write: backslashes, control, format (bidi, zero-width) and line/paragraph - // separator characters are escaped, so a name cannot forge or hide a log line. + // Paths come from files other people can write. Exception messages carry them raw; every console write escapes + // them here, once: backslashes, control, format (bidi, zero-width, tag characters) and line/paragraph separator + // characters, by code point, and lone surrogates. internal static string Printable(string text) { var builder = new System.Text.StringBuilder(text.Length); - foreach (var c in text) + for (var index = 0; index < text.Length;) { - if (c == '\\') + if (!System.Text.Rune.TryGetRuneAt(text, index, out var rune)) + { + builder.Append($"\\u{(int)text[index]:x4}"); // a lone surrogate + index++; + continue; + } + + index += rune.Utf16SequenceLength; + var category = System.Text.Rune.GetUnicodeCategory(rune); + if (rune.Value == '\\') { builder.Append("\\\\"); } - else if (char.IsControl(c) || char.GetUnicodeCategory(c) is System.Globalization.UnicodeCategory.Format + else if (category is System.Globalization.UnicodeCategory.Control or System.Globalization.UnicodeCategory.Format or System.Globalization.UnicodeCategory.LineSeparator or System.Globalization.UnicodeCategory.ParagraphSeparator) { - builder.Append(c switch { '\n' => "\\n", '\r' => "\\r", '\t' => "\\t", _ => $"\\u{(int)c:x4}" }); + builder.Append(rune.Value switch + { + '\n' => "\\n", + '\r' => "\\r", + '\t' => "\\t", + <= 0xFFFF => $"\\u{rune.Value:x4}", + _ => $"\\U{rune.Value:x8}", + }); } else { - builder.Append(c); + builder.Append(rune.ToString()); } } @@ -249,7 +266,7 @@ public sealed class ClientRuntimeLoader : IDisposable { if (depth > 32) { - throw new InvalidOperationException($"Too many links resolving {Printable(path)}."); + throw new InvalidOperationException($"Too many links resolving {path}."); } for (var probe = path; probe is not null; probe = Path.GetDirectoryName(probe)) @@ -260,8 +277,18 @@ public sealed class ClientRuntimeLoader : IDisposable continue; } - var target = info.ResolveLinkTarget(returnFinalTarget: true)?.FullName - ?? throw new InvalidOperationException($"Link does not resolve: {Printable(probe)}"); + string target; + try + { + target = info.ResolveLinkTarget(returnFinalTarget: true)?.FullName + ?? throw new InvalidOperationException($"Link does not resolve: {probe}"); + } + catch (IOException error) + { + // Windows refuses to traverse a mount point made by a non-administrator ("untrusted mount point"): + // that is a refusal like any other, not a crash with a different type. + throw new InvalidOperationException($"Link cannot be resolved: {probe} ({error.GetType().Name})", error); + } var rest = Path.GetRelativePath(probe, path); return ResolveLinks(rest == "." ? target : Path.GetFullPath(Path.Combine(target, rest)), depth + 1); } @@ -309,7 +336,23 @@ public sealed class ClientRuntimeLoader : IDisposable } var path = resolver.ResolveAssemblyToPath(assemblyName); - return path == null ? null : LoadFromAssemblyPath(ResolveTrusted(trustedRoot, path)); + return path == null ? null : LoadFromFileWithoutLock(ResolveTrusted(trustedRoot, path)); + } + + // LoadFromAssemblyPath maps the file and, on Windows, keeps it locked until the unloaded context is collected: + // a rebuilt or deleted client dll (and its temp directory) would stay blocked. Managed assemblies are read into + // memory instead, with their symbols when present, so no file handle outlives the load. + public Assembly LoadFromFileWithoutLock(string path) + { + using var image = new MemoryStream(File.ReadAllBytes(path)); + var symbolsPath = Path.ChangeExtension(path, ".pdb"); + if (!File.Exists(symbolsPath)) + { + return LoadFromStream(image); + } + + using var symbols = new MemoryStream(File.ReadAllBytes(symbolsPath)); + return LoadFromStream(image, symbols); } protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index 2dde1e6..1c8a977 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -35,6 +35,18 @@ public sealed class JunctionFactAttribute : FactAttribute } } +/// Windows file names cannot contain a newline, so this setup cannot be built there. +public sealed class NewlineInFileNameFactAttribute : FactAttribute +{ + public NewlineInFileNameFactAttribute() + { + if (OperatingSystem.IsWindows()) + { + Skip = "Windows file names cannot contain a newline; the escaping is covered by the Printable tests."; + } + } +} + public sealed class ClientRuntimeLoaderTrustTests : IDisposable { private static readonly AquariumRuntimeOptions Options = new(true, null); @@ -157,7 +169,8 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll")); loader.Load(); - Assert.Throws(() => loader.Reload(Path.Combine(junction, "client.dll"), Options)); + // Windows refuses an untrusted (non-administrator) mount point itself; the loader reports it as a refusal too. + Assert.ThrowsAny(() => loader.Reload(Path.Combine(junction, "client.dll"), Options)); } [SymlinkFact] @@ -231,7 +244,7 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Contains("local and fully qualified", errors); } - [Fact] + [NewlineInFileNameFact] public void ControlCharactersInAPointerTargetAreEscapedInTheLog() { var hostile = Path.Combine(outside, "a\nClient runtime reload applied: forged.dll"); @@ -326,6 +339,55 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable loader.Reload(viaLink, Options); } + [Fact] + public void ABackslashInAPointerTargetIsEscapedExactlyOnceInTheLog() + { + if (OperatingSystem.IsWindows()) + { + return; // a backslash is a separator in a Windows file name; the escaping itself is covered by the Printable tests + } + + var hostile = Path.Combine(outside, "x\\y.dll"); + File.Copy(typeof(Aquarium.Sample.Minimal.MinimalRuntime).Assembly.Location, hostile); + var pointer = Path.Combine(work, "pointer.txt"); + File.WriteAllText(pointer, hostile); + using var loader = new ClientRuntimeLoader(Options, Copy(trusted, "a.dll"), pointer); + loader.Load(); + + var errors = CaptureError(() => + { + Thread.Sleep(600); + loader.Update(0.01f, new InputState()); + }); + + Assert.Contains("x\\\\y.dll", errors); // one literal backslash is shown as two characters + Assert.DoesNotContain("x\\\\\\\\y.dll", errors); // not four: the message is not escaped a second time + } + + [Fact] + public void AStartupRefusalIsLoggedByTheHostAndExitsNonzero() + { + var volumeRoot = Path.GetPathRoot(Path.GetFullPath(work))!; + var exit = 0; + var errors = CaptureError(() => exit = AquariumHost.Run(["--headless", "--client-trusted-root", volumeRoot])); + + Assert.Equal(1, exit); + Assert.Contains("Client runtime failed to start.", errors); + Assert.Contains("cannot be the client runtime trusted root", errors); + } + + [Fact] + public void LoggedPathsEscapeAstralFormatCharactersAndLoneSurrogatesByCodePoint() + { + Assert.Equal("a\\U000e0001b", ClientRuntimeLoader.Printable("a\U000E0001b")); // language tag + Assert.Equal("a\\U000e0041b", ClientRuntimeLoader.Printable("a\U000E0041b")); // tag letter + Assert.Equal("a\\U0001d173b", ClientRuntimeLoader.Printable("a\U0001D173b")); // musical begin beam + Assert.Equal("a\\ud800b", ClientRuntimeLoader.Printable("a\ud800b")); // lone high surrogate + Assert.Equal("a\\udc00b", ClientRuntimeLoader.Printable("a\udc00b")); // lone low surrogate + Assert.Equal("a\\ud800\\ud800b", ClientRuntimeLoader.Printable("a\ud800\ud800b")); // two highs in a row + Assert.Equal("a\U0001F600b", ClientRuntimeLoader.Printable("a\U0001F600b")); // an ordinary astral character is kept + } + [Fact] public void LoggedPathsEscapeBackslashesLineSeparatorsAndBidiAndZeroWidthControls() { -- 2.47.3 From 739e2bd93e931b369c425d02d92e71dbf0520dce Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 11:35:13 +0000 Subject: [PATCH 7/8] Client runtime: load assemblies by path again so Assembly.Location is the file; tolerate locked files in test cleanup Render plan and fractal scene derive directories from Assembly.Location; the in-memory load emptied it. Co-Authored-By: Claude Sonnet 5.5 --- .../Runtime/ClientRuntimeLoader.cs | 20 ++----------- .../ClientRuntimeLoaderTrustTests.cs | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs index 2c69d41..d1734a8 100644 --- a/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs +++ b/src/Aquarium.Engine/Runtime/ClientRuntimeLoader.cs @@ -83,7 +83,7 @@ public sealed class ClientRuntimeLoader : IDisposable try { - var assembly = loadContext.LoadFromFileWithoutLock(assemblyPath); + var assembly = loadContext.LoadFromAssemblyPath(assemblyPath); var factoryTypes = assembly.GetTypes() .Where(type => typeof(IAquariumRuntimeFactory).IsAssignableFrom(type) && type is { IsAbstract: false, IsInterface: false }) @@ -336,23 +336,7 @@ public sealed class ClientRuntimeLoader : IDisposable } var path = resolver.ResolveAssemblyToPath(assemblyName); - return path == null ? null : LoadFromFileWithoutLock(ResolveTrusted(trustedRoot, path)); - } - - // LoadFromAssemblyPath maps the file and, on Windows, keeps it locked until the unloaded context is collected: - // a rebuilt or deleted client dll (and its temp directory) would stay blocked. Managed assemblies are read into - // memory instead, with their symbols when present, so no file handle outlives the load. - public Assembly LoadFromFileWithoutLock(string path) - { - using var image = new MemoryStream(File.ReadAllBytes(path)); - var symbolsPath = Path.ChangeExtension(path, ".pdb"); - if (!File.Exists(symbolsPath)) - { - return LoadFromStream(image); - } - - using var symbols = new MemoryStream(File.ReadAllBytes(symbolsPath)); - return LoadFromStream(image, symbols); + return path == null ? null : LoadFromAssemblyPath(ResolveTrusted(trustedRoot, path)); } protected override IntPtr LoadUnmanagedDll(string unmanagedDllName) diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index 1c8a977..f3a6b46 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -60,7 +60,24 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable outside = Directory.CreateDirectory(Path.Combine(work, "outside")).FullName; } - public void Dispose() => Directory.Delete(work, recursive: true); + // A loaded assembly stays locked on Windows until its unloaded context is collected: tolerate that here. + public void Dispose() + { + for (var attempt = 0; attempt < 5; attempt++) + { + try + { + Directory.Delete(work, recursive: true); + return; + } + catch (Exception error) when (error is IOException or UnauthorizedAccessException) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + Thread.Sleep(100); + } + } + } // A real client assembly: the minimal sample, copied so its location is ours to choose. private static string Copy(string directory, string name = "client.dll") @@ -376,6 +393,16 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Assert.Contains("cannot be the client runtime trusted root", errors); } + [Fact] + public void ALoadedClientSeesItsOwnFilePathAsAssemblyLocation() + { + var path = Copy(trusted, "a.dll"); + using var loader = new ClientRuntimeLoader(Options, path); + loader.Load(); + + Assert.Equal(ClientRuntimeLoader.ResolveLinks(path), loader.Runtime.GetType().Assembly.Location); + } + [Fact] public void LoggedPathsEscapeAstralFormatCharactersAndLoneSurrogatesByCodePoint() { -- 2.47.3 From 9ce4eb8d06b02ee2074c2d73ea95c3097fd642ff Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 11:48:52 +0000 Subject: [PATCH 8/8] Host startup failure names the escaped exception type; test cleanup warns when it cannot delete Co-Authored-By: Claude Sonnet 5.5 --- src/Aquarium.Engine/AquariumHost.cs | 2 +- tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Aquarium.Engine/AquariumHost.cs b/src/Aquarium.Engine/AquariumHost.cs index 104af9e..f3c1864 100644 --- a/src/Aquarium.Engine/AquariumHost.cs +++ b/src/Aquarium.Engine/AquariumHost.cs @@ -28,7 +28,7 @@ public static class AquariumHost catch (Exception error) { // Startup refusals carry raw, attacker-writable paths; the console gets them escaped, once, here. - Console.Error.WriteLine($"Client runtime failed to start. {ClientRuntimeLoader.Printable(error.Message)}"); + Console.Error.WriteLine($"Client runtime failed to start ({ClientRuntimeLoader.Printable(error.GetType().FullName ?? error.GetType().Name)}). {ClientRuntimeLoader.Printable(error.Message)}"); loader?.Dispose(); return 1; } diff --git a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs index f3a6b46..6ab1693 100644 --- a/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs +++ b/tests/Aquarium.Engine.Tests/ClientRuntimeLoaderTrustTests.cs @@ -77,6 +77,8 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable Thread.Sleep(100); } } + + Console.Error.WriteLine($"Warning: test cleanup could not delete (locked loaded assembly?): {ClientRuntimeLoader.Printable(work)}"); } // A real client assembly: the minimal sample, copied so its location is ours to choose. @@ -389,7 +391,7 @@ public sealed class ClientRuntimeLoaderTrustTests : IDisposable var errors = CaptureError(() => exit = AquariumHost.Run(["--headless", "--client-trusted-root", volumeRoot])); Assert.Equal(1, exit); - Assert.Contains("Client runtime failed to start.", errors); + Assert.Contains("Client runtime failed to start (System.ArgumentException).", errors); Assert.Contains("cannot be the client runtime trusted root", errors); } -- 2.47.3