Onboarding substrate: harden evidence containment and gated-start rechecks

src/Capacitor.Cli.Core/Config/ConfigMutator.cs

@@ -44,24 +44,29 @@ public static ProfileConfig LoadPure(string path) {
     /// silently treat it the same as absence (e.g. a gated identity check) use this instead of
     /// <see cref="LoadPure"/>.
     public static bool TryLoadPure(string path, out ProfileConfig config) {
-        // A directory sitting at the config path is NOT absence — File.Exists alone would say
-        // false and this would silently fall through to "nothing configured yet" for what is
-        // actually an unreadable/misconfigured location.
-        if (Directory.Exists(path)) {
-            config = new() { Profiles = new() { ["default"] = new() } };
-            return false;
-        }
-
-        if (!File.Exists(path)) {
-            config = new() { Profiles = new() { ["default"] = new() } };
-            return true;
-        }
-
+        // Attempt the read directly rather than probing Directory.Exists/File.Exists first: BOTH
+        // return false on a permission-denied path (or an unreadable ancestor directory), which
+        // would silently degrade "inaccessible" into "absent, defaults are fine" — exactly wrong
+        // for a caller (the start gate) that must fail closed on unreadable evidence rather than
+        // proceed as though nothing were configured.
         string json;
         try {
             json = File.ReadAllText(path);
-        } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) {
-            config = new() { Profiles = new() { ["default"] = new() } };
+        } catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) {
+            // A link at the exact path (dangling included), or a file/link ancestor, is
+            // structural evidence — unreadable, not absence.
+            if (PathEvidence.PathBlockedByFileOrLink(path)) {
+                config = FreshDefault();
+                return false;
+            }
+
+            config = FreshDefault();
+            return true;
+        } catch {
+            // A directory sitting at the config path (UnauthorizedAccessException on read), a
+            // permission error, or any other I/O failure — unreadable EVIDENCE, never silently
+            // folded into absence.
+            config = FreshDefault();
             return false;
         }
 
@@ -71,25 +76,39 @@ public static bool TryLoadPure(string path, out ProfileConfig config) {
         // contract.
         try {
             using var doc = JsonDocument.Parse(json);
-            if (doc.RootElement.ValueKind != JsonValueKind.Object) throw new JsonException("config root is not a JSON object");
+            if (!doc.RootElement.IsObject) throw new JsonException("config root is not a JSON object");
         } catch (JsonException) {
-            config = new() { Profiles = new() { ["default"] = new() } };
+            config = FreshDefault();
             return false;
         }
 
         try {
             config = ConfigMigration.MigrateIfNeeded(json).Config;
             return true;
         } catch (JsonException) {
-            config = new() { Profiles = new() { ["default"] = new() } };
+            config = FreshDefault();
             return false;
         }
     }
 
+    static ProfileConfig FreshDefault() => new() { Profiles = new() { ["default"] = new() } };
+
     static void Publish(string path, ProfileConfig config) {
         var tmp = path + ".tmp-" + Guid.NewGuid().ToString("N")[..8];
         File.WriteAllBytes(tmp,
             JsonSerializer.SerializeToUtf8Bytes(config, ProfileConfigJsonContextIndented.Default.ProfileConfig));
-        File.Move(tmp, path, overwrite: true);
+        // Windows denies replace-into-place while any reader holds the destination without
+        // FILE_SHARE_DELETE; readers are short-lived, so retry briefly before surfacing.
+        for (var attempt = 0; ; attempt++) {
+            try {
+                File.Move(tmp, path, overwrite: true);
+                return;
+            } catch (Exception e) when (e is UnauthorizedAccessException or IOException && attempt < 49) {
+                Thread.Sleep(20);
+            } catch {
+                try { File.Delete(tmp); } catch { /* best effort */ }
+                throw;
+            }
+        }
     }
 }

src/Capacitor.Cli.Core/PathEvidence.cs

@@ -0,0 +1,24 @@
+namespace Capacitor.Cli.Core;
+
+/// Structural path-evidence probe shared by not-found-classification callers.
+public static class PathEvidence {
+    /// True when a not-found read failure at <paramref name="path"/> is explained by a link at the
+    /// exact path, or a file/link ancestor, rather than a chain of directories never created.
+    public static bool PathBlockedByFileOrLink(string path) {
+        if (IsLink(path)) return true;
+
+        var current = Path.GetDirectoryName(path);
+        while (!string.IsNullOrEmpty(current)) {
+            if (File.Exists(current)) return true;
+            if (Directory.Exists(current)) return false;
+            if (IsLink(current)) return true;
+            current = Path.GetDirectoryName(current);
+        }
+        return false;
+    }
+
+    static bool IsLink(string path) {
+        try { return File.ResolveLinkTarget(path, returnFinalTarget: false) is not null; }
+        catch { return false; }
+    }
+}

src/Capacitor.Cli.Core/Setup/AgentDetection.cs

@@ -111,14 +111,22 @@ public static AgentDetectionResult Detect(AgentDetectionInputs i) {
     /// null/empty PATH. On Unix, requires at least one of the user/group/other execute bits; on
     /// Windows, walks PATHEXT (defaulting to .EXE/.CMD/.BAT) and accepts any file that exists.
     /// </summary>
-    public static bool BinaryOnPath(string binaryName, AgentDetectionInputs i) {
+    public static bool BinaryOnPath(string binaryName, AgentDetectionInputs i) =>
+        BinaryOnPath(binaryName, i, IsExecutable);
+
+    /// <summary>Stubbing seam for <see cref="BinaryOnPath(string, AgentDetectionInputs)"/>; separator/
+    /// extensions/comparer are all derived from <paramref name="i"/>'s platform, never the host's.</summary>
+    internal static bool BinaryOnPath(string binaryName, AgentDetectionInputs i, Func<string, bool, bool> isExecutable) {
         if (string.IsNullOrEmpty(i.PathEnv)) return false;
 
-        var paths      = i.PathEnv.Split(Path.PathSeparator);
+        var separator  = i.IsWindows ? ';' : ':';
+        var paths      = i.PathEnv.Split(separator);
         var extensions = i.IsWindows ? WindowsExtensions(i.PathExt) : [""];
+        var comparer   = i.IsWindows ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal;
 
         return paths.Where(dir => !string.IsNullOrEmpty(dir))
-            .Any(dir => extensions.Select(ext => Path.Combine(dir, binaryName + ext)).Any(path => IsExecutable(path, i.IsWindows)));
+            .Distinct(comparer)
+            .Any(dir => extensions.Select(ext => Path.Combine(dir, binaryName + ext)).Any(path => isExecutable(path, i.IsWindows)));
     }
 
     /// <summary>Convenience overload for CLI call sites that only need a single-binary current-

src/Capacitor.Cli/Services/IServiceManager.cs

@@ -73,11 +73,9 @@ interface IVerifyServiceManager {
     void         WriteAndBootstrap(ServiceSpec spec, TimeSpan timeout);
     bool         Uninstall(string serviceId, TimeSpan timeout, out string? error);
     bool         Start(string serviceId, TimeSpan timeout, out string? error);
-    /// <summary>Bootstrap-only start for the gated start path: activate the ALREADY-WRITTEN unit
-    /// without the generic <see cref="Start"/>'s Loaded→kickstart branch. If the label turns out
-    /// Loaded (a foreign writer raced the gate's own confirmed-absent check), this must fail rather
-    /// than kickstart it — kickstarting an unverified loaded definition is exactly what the gate
-    /// exists to prevent.</summary>
+    /// <summary>Bootstrap-only start for the gated path: activates the already-written unit, but
+    /// fails — never kickstarts — when the label reads Loaded, closing the race with the gate's own
+    /// confirmed-absent check.</summary>
     bool         StartBootstrapOnly(string serviceId, TimeSpan timeout, out string? error);
     bool         Stop(string serviceId, TimeSpan timeout, out string? error);
 }

src/Capacitor.Cli/Services/LaunchdServiceManager.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 
 namespace Capacitor.Cli.Services;
@@ -48,7 +49,7 @@ public IReadOnlyList<string> ListInstalled() {
     public ServiceStatus Status(string serviceId) {
         var path = LaunchdUnit.PlistPath(serviceId);
         if (!File.Exists(path)) return new ServiceStatus(ServiceState.NotInstalled, null);
-        var bin = LaunchdUnit.BinaryFromPlist(File.ReadAllText(path)); // ProgramArguments[0], not the Label
+        var bin = ReadBinaryPathSafe(path); // ProgramArguments[0], not the Label
         var (code, stdout, _) = _runProcess("launchctl", LaunchdUnit.PrintArgs(Uid(), serviceId));
         return new ServiceStatus(LaunchdUnit.StatusFromPrint(code, stdout), bin);
     }
@@ -57,9 +58,11 @@ public ServiceStatus Status(string serviceId) {
     public ServiceQuery Query(string serviceId, TimeSpan t)     => QueryCore(serviceId, t);
 
     ServiceQuery QueryCore(string serviceId, TimeSpan? timeout) {
-        var path        = LaunchdUnit.PlistPath(serviceId);
-        var unitPresent = File.Exists(path);
-        var bin         = unitPresent ? LaunchdUnit.BinaryFromPlist(File.ReadAllText(path)) : null;
+        var path = LaunchdUnit.PlistPath(serviceId);
+        // File.Exists alone reads a DIRECTORY at the path (or an inaccessible ancestor) as
+        // absent — open directly so presence and unreadable-but-present are never conflated.
+        var unitPresent = LaunchdUnit.TryReadPlist(path, out _) != LaunchdUnit.PlistRead.Absent;
+        var bin         = unitPresent ? ReadBinaryPathSafe(path) : null;
         var (code, stdout, stderr, timedOut) = RunCtl(timeout, LaunchdUnit.PrintArgs(Uid(), serviceId));
         // A killed-on-timeout print told us nothing about the label — never let its kill exit code
         // masquerade as a real classification; report Unknown so nothing destructive follows.
@@ -68,6 +71,17 @@ ServiceQuery QueryCore(string serviceId, TimeSpan? timeout) {
         return new ServiceQuery(probe, unitPresent, state, bin, probe == LabelProbe.Loaded ? LaunchdUnit.PidFromPrint(stdout) : null);
     }
 
+    /// <summary>Total plist-evidence read: <c>File.ReadAllText</c> + <see cref="LaunchdUnit.BinaryFromPlist"/>
+    /// together, contained so that ANY failure — an I/O error, malformed XML, a duplicate
+    /// <c>ProgramArguments</c> key — yields null rather than escaping. <see cref="Query(string)"/>
+    /// and <see cref="Status"/> are total, never-throwing probes; the gate's own contained Phase-A
+    /// parse (<c>ServiceVerify</c>'s <c>_readPlist</c> path) remains the sole authority for coded
+    /// evidence classification (malformed → <c>evidence_unreadable</c>).</summary>
+    static string? ReadBinaryPathSafe(string path) {
+        try { return LaunchdUnit.BinaryFromPlist(File.ReadAllText(path)); }
+        catch { return null; }
+    }
+
     public void Install(ServiceSpec spec, bool startNow) {
         var plistPath = LaunchdUnit.PlistPath(spec.ServiceId);
         // idempotent: bootout an existing job (ignore failure), then rewrite + bootstrap.
@@ -163,14 +177,13 @@ bool StartCore(string serviceId, TimeSpan? timeout, out string? error) {
         return true;
     }
 
-    /// <summary>
-    /// The gated start path's bootstrap-only verb (verify-only, no legacy no-timeout overload):
-    /// re-probes the label immediately before acting, and ONLY bootstraps when it reads Absent —
-    /// unlike <see cref="StartCore"/>, a Loaded probe here is a FAILURE, never a kickstart. This is
-    /// the re-check that closes the race between the gate's own confirmed-absent wait and this call:
-    /// a foreign writer that loaded the label in that window must not get silently kickstarted.
-    /// </summary>
+    /// <summary>launchd implementation of <see cref="IVerifyServiceManager.StartBootstrapOnly"/>:
+    /// re-probes the label immediately before acting. Both launchctl calls share ONE deadline —
+    /// the probe gets the full <paramref name="timeout"/>, and the bootstrap gets only what's left
+    /// of it — rather than each getting the full budget (which would let the pair invade up to 2x
+    /// the caller's forward remainder, including its separately reserved rollback budget).</summary>
     public bool StartBootstrapOnly(string serviceId, TimeSpan timeout, out string? error) {
+        var sw = Stopwatch.StartNew();
         var (probeExit, probeOut, probeErr, probeTimedOut) = RunCtl(timeout, LaunchdUnit.PrintArgs(Uid(), serviceId));
         var probe = probeTimedOut ? LabelProbe.Unknown : LaunchdUnit.ClassifyPrint(probeExit, probeOut, probeErr);
 
@@ -179,7 +192,13 @@ public bool StartBootstrapOnly(string serviceId, TimeSpan timeout, out string? e
             return false;
         }
 
-        var (bootstrapExit, _, bootstrapErr, bootstrapTimedOut) = RunCtl(timeout, LaunchdUnit.BootstrapArgs(Uid(), LaunchdUnit.PlistPath(serviceId)));
+        var remaining = timeout - sw.Elapsed;
+        if (remaining <= TimeSpan.Zero) {
+            error = "launchctl bootstrap timed out and was terminated";
+            return false;
+        }
+
+        var (bootstrapExit, _, bootstrapErr, bootstrapTimedOut) = RunCtl(remaining, LaunchdUnit.BootstrapArgs(Uid(), LaunchdUnit.PlistPath(serviceId)));
         if (bootstrapTimedOut) { error = "launchctl bootstrap timed out and was terminated"; return false; }
         if (bootstrapExit != 0) {
             error = $"launchctl bootstrap failed (exit {bootstrapExit}): {bootstrapErr.Trim()}";