From 45b0b38a2e11e55a41d656ce6f47e5d23cb73eb5 Mon Sep 17 00:00:00 2001 From: Aqua Date: Sat, 7 Nov 2026 10:57:09 +0000 Subject: [PATCH] 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."); + } }