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 <noreply@anthropic.com>

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"

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"

src/Aquarium.Engine/AquariumHost.cs

@@ -258,20 +258,28 @@ public static class AquariumHost
             ?? Environment.GetEnvironmentVariable("AQUARIUM_LIVE_ASSEMBLY");
     }
 
-    private static string? ParseClientTrustedRoot(IReadOnlyCollection<string> args)
+    internal static string? ParseClientTrustedRoot(IReadOnlyCollection<string> 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;
     }
 

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);
         }

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<ArgumentException>(() => AquariumHost.ParseClientTrustedRoot(["--client-trusted-root", ""]));
+        Assert.Null(Environment.GetEnvironmentVariable(Name));
+
+        Environment.SetEnvironmentVariable(Name, "/from/env");
+        Assert.Throws<ArgumentException>(() => 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.");
+    }
 }