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

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
 

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
             }

src/Aquarium.Engine/AquariumHost.cs

@@ -258,30 +258,9 @@ public static class AquariumHost
             ?? Environment.GetEnvironmentVariable("AQUARIUM_LIVE_ASSEMBLY");
     }
 
-    internal static string? ParseClientTrustedRoot(IReadOnlyCollection<string> 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<string> args) => TrustedRoot.Resolve(args);
 
     private static string? ParseClientReloadPointerPath(IReadOnlyCollection<string> args)
     {

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

src/Aquarium.Engine/Runtime/TrustedRootSetting.cs

@@ -0,0 +1,48 @@
+namespace Aquarium.Engine.Runtime;
+
+/// <summary>
+/// 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.
+/// </summary>
+internal sealed class TrustedRootSetting(Func<string?> takeEnvironment)
+{
+    private const string Flag = "--client-trusted-root";
+    private const string Variable = "AQUARIUM_CLIENT_TRUSTED_ROOT";
+    private readonly Lazy<string?> fromEnvironment = new(takeEnvironment);
+
+    public static TrustedRootSetting FromProcessEnvironment() => new(() =>
+    {
+        var value = Environment.GetEnvironmentVariable(Variable);
+        Environment.SetEnvironmentVariable(Variable, null);
+        return value;
+    });
+
+    public string? Resolve(IReadOnlyCollection<string> 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;
+    }
+}

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