Merge nucleic/vivid-glass-urchin-xoym into main

Sources/RunnerCore/LocalNetworkPolicy.swift

@@ -50,6 +50,12 @@ public enum LocalNetworkPolicy {
     /// The domain is written with `sudo`, so which preferences directory it
     /// lands in depends on whether that `sudo` preserved `HOME`. Rather than
     /// guess at the host's sudoers configuration, check each candidate.
+    ///
+    /// In practice the first candidate is where it lands, and `/var/root` is
+    /// mode 700 — so an unprivileged process cannot read back what it just
+    /// wrote. That is what ``Status/unreadablePaths`` exists to report, and
+    /// what `RunnerHost`'s `LocalNetworkPermission.observedStatus()` works
+    /// around by re-reading as root.
     public static func preferenceCandidates() -> [String] {
         [
             "/var/root/Library/Preferences/\(domain).plist",
@@ -112,47 +118,103 @@ public enum LocalNetworkPolicy {
         /// Whether at least one entry covers the whole vmnet range.
         public let coversGuestRange: Bool
 
+        /// Candidate files this process was refused permission to read.
+        ///
+        /// Not the same as "absent". `sudo defaults write` normally lands in
+        /// `/var/root/Library/Preferences`, which is mode 700, so an ordinary
+        /// user is refused before it can find out whether the file is even
+        /// there. An empty ``allowlist`` with a non-empty `unreadablePaths`
+        /// means *unknown*, not *unconfigured*, and must not be reported as
+        /// the latter.
+        public let unreadablePaths: [String]
+
         /// Whether anything is configured at all.
         public var isConfigured: Bool { !allowlist.isEmpty }
 
-        public init(allowlist: [String], sourcePaths: [String], coversGuestRange: Bool) {
+        /// Whether nothing was found and something could not be read, so the
+        /// answer is genuinely unknown without administrator rights.
+        public var isIndeterminate: Bool { allowlist.isEmpty && !unreadablePaths.isEmpty }
+
+        public init(
+            allowlist: [String],
+            sourcePaths: [String],
+            coversGuestRange: Bool,
+            unreadablePaths: [String] = []
+        ) {
             self.allowlist = allowlist
             self.sourcePaths = sourcePaths
             self.coversGuestRange = coversGuestRange
+            self.unreadablePaths = unreadablePaths
         }
     }
 
-    /// Reads the host's current allowlist.
+    /// Reads the host's current allowlist with this process's own privileges.
     ///
-    /// Best effort and never fatal: an unreadable or absent preferences file
-    /// simply reads as "no allowlist".
+    /// Best effort and never fatal: an absent preferences file reads as "no
+    /// allowlist", and one that exists but cannot be opened is recorded in
+    /// ``Status/unreadablePaths`` rather than being mistaken for absent.
     public static func status() -> Status {
-        var found: [String] = []
-        var sources: [String] = []
+        var sources: [(path: String, data: Data)] = []
+        var unreadable: [String] = []
 
         for path in preferenceCandidates() {
-            guard let data = FileManager.default.contents(atPath: path),
-                let plist = try? PropertyListSerialization.propertyList(
-                    from: data, options: [], format: nil) as? [String: Any]
-            else { continue }
-
-            var contributed = false
-            for key in keys {
-                for entry in (plist[key] as? [String] ?? []) where !found.contains(entry) {
-                    found.append(entry)
-                    contributed = true
-                }
+            if let data = FileManager.default.contents(atPath: path) {
+                sources.append((path, data))
+            } else if access(path, R_OK) != 0, errno == EACCES {
+                // Refused, not missing — including when the refusal is on a
+                // parent directory, which is exactly the /var/root case.
+                unreadable.append(path)
             }
-            if contributed { sources.append(path) }
+        }
+
+        return status(fromContentsOf: sources, unreadablePaths: unreadable)
+    }
+
+    /// Builds a ``Status`` from preferences files already read, however they
+    /// were obtained.
+    ///
+    /// Split out from ``status()`` so the privileged read-back in `RunnerHost`
+    /// — which has to shell out to `sudo` to see root's copy — shares this
+    /// parsing rather than reimplementing it.
+    public static func status(
+        fromContentsOf sources: [(path: String, data: Data)],
+        unreadablePaths: [String] = []
+    ) -> Status {
+        var found: [String] = []
+        var paths: [String] = []
+
+        for source in sources {
+            let fresh = entries(inPreferences: source.data).filter { !found.contains($0) }
+            guard !fresh.isEmpty else { continue }
+            found.append(contentsOf: fresh)
+            paths.append(source.path)
         }
 
         return Status(
             allowlist: found,
-            sourcePaths: sources,
-            coversGuestRange: found.contains(where: coversVMNetRange)
+            sourcePaths: paths,
+            coversGuestRange: found.contains(where: coversVMNetRange),
+            unreadablePaths: unreadablePaths
         )
     }
 
+    /// Every allowlist entry in one preferences file, across both keys, in the
+    /// order encountered and without duplicates. Unparseable data reads empty.
+    public static func entries(inPreferences data: Data) -> [String] {
+        guard
+            let plist = try? PropertyListSerialization.propertyList(
+                from: data, options: [], format: nil) as? [String: Any]
+        else { return [] }
+
+        var found: [String] = []
+        for key in keys {
+            for entry in (plist[key] as? [String] ?? []) where !found.contains(entry) {
+                found.append(entry)
+            }
+        }
+        return found
+    }
+
     /// Subnets pre-authorized for local network access on this host, if any.
     public static func allowlist() -> [String] { status().allowlist }
 

Sources/RunnerHost/Doctor.swift

@@ -771,7 +771,7 @@ public enum Doctor {
     /// does not apply (Apple, TN3179).
     public static func localNetworkNote() -> DoctorCheck {
         let name = "local network access"
-        let status = LocalNetworkPolicy.status()
+        let status = LocalNetworkPermission.observedStatus()
 
         if status.isConfigured {
             if status.coversGuestRange {
@@ -796,10 +796,26 @@ public enum Doctor {
             )
         }
 
+        // Nothing found. On all but an unusual host that means *not visible*
+        // rather than *not set*: the allowlist is written as root and lands in
+        // /var/root, which is mode 700. Say which of the two this is, because
+        // "no allowlist" would otherwise be asserted on a host that has one.
+        let caveat =
+            status.isIndeterminate
+            ? """
+                 This cannot see the setting itself — it lives in \
+                \(status.unreadablePaths[0]), which only root can read — so treat the above as \
+                "not visible", not "not set". `sudo gitea-macos-runner permissions status` \
+                answers definitively, and `permissions grant` verifies its own write.
+                """
+            : ""
+
         return DoctorCheck(
             name: name,
             result: .info,
-            detail: "guests are reached over the host-private NAT link",
+            detail: status.isIndeterminate
+                ? "no allowlist visible; guests are reached over the host-private NAT link"
+                : "guests are reached over the host-private NAT link",
             remediation: """
                 on macOS 15+ the first connection to a guest can be blocked by the Local Network \
                 privacy prompt, and frequently there is nothing able to answer it. A LaunchAgent \
@@ -810,7 +826,7 @@ public enum Doctor {
                 `gitea-macos-runner permissions grant`, which writes a subnet allowlist that \
                 needs no prompt, covers every process, and survives rebuilds — then reboot. \
                 `permissions status` explains both routes. See docs/setup.md §2.6.
-                """
+                """ + caveat
         )
     }
 

Sources/RunnerHost/LocalNetworkPermission.swift

@@ -133,7 +133,66 @@ public enum LocalNetworkPermission {
             }
         }
 
-        return AllowlistResult(requested: subnets, observed: LocalNetworkPolicy.status())
+        return AllowlistResult(requested: subnets, observed: observedStatus())
+    }
+
+    /// The host's allowlist, read with root's privileges when this process's
+    /// own are not enough.
+    ///
+    /// `sudo defaults write <domain>` lands in `/var/root/Library/Preferences`
+    /// on a stock host, and that directory is mode 700 — so the plain read in
+    /// ``LocalNetworkPolicy/status()`` is refused and a write that worked
+    /// perfectly looks like it vanished. Re-read the refused candidates as
+    /// root instead.
+    ///
+    /// Always `sudo -n`, so this can never turn a status query into a password
+    /// prompt. Right after a write the credentials are still cached and it
+    /// simply works; later — after a reboot, say — it fails and the result
+    /// stays ``LocalNetworkPolicy/Status/isIndeterminate``, which callers
+    /// report as "cannot tell without root" rather than as "not configured".
+    public static func observedStatus() -> LocalNetworkPolicy.Status {
+        let unprivileged = LocalNetworkPolicy.status()
+        guard !unprivileged.unreadablePaths.isEmpty else { return unprivileged }
+
+        var sources: [(path: String, data: Data)] = []
+        for path in LocalNetworkPolicy.preferenceCandidates() {
+            if let data = FileManager.default.contents(atPath: path) {
+                sources.append((path, data))
+            } else if let data = readAsRoot(path) {
+                sources.append((path, data))
+            }
+        }
+
+        let recovered = LocalNetworkPolicy.status(fromContentsOf: sources)
+        // Nothing came back from the privileged read either: keep the
+        // unprivileged answer, which still carries why it could not tell.
+        guard recovered.isConfigured else { return unprivileged }
+        return recovered
+    }
+
+    /// `sudo -n cat <path>`, or nil if that fails for any reason.
+    ///
+    /// Both failure modes are ordinary rather than exceptional — the candidate
+    /// usually does not exist, and `sudo -n` legitimately refuses when no
+    /// credentials are cached — so stderr is discarded instead of being shown
+    /// to the operator. `Process` is fine here, unlike in ``runInForeground``:
+    /// `-n` never touches the terminal.
+    private static func readAsRoot(_ path: String) -> Data? {
+        let process = Process()
+        process.executableURL = URL(fileURLWithPath: "/usr/bin/sudo")
+        process.arguments = ["-n", "/bin/cat", path]
+
+        let output = Pipe()
+        process.standardOutput = output
+        process.standardError = FileHandle.nullDevice
+        process.standardInput = FileHandle.nullDevice
+
+        guard (try? process.run()) != nil else { return nil }
+        let data = output.fileHandleForReading.readDataToEndOfFile()
+        process.waitUntilExit()
+
+        guard process.terminationStatus == 0, !data.isEmpty else { return nil }
+        return data
     }
 
     /// Reboots the host. Only ever called from an explicit confirmation — the

Sources/gitea-macos-runner/CommandPermissions.swift

@@ -47,7 +47,7 @@ struct PermissionsCommand: AsyncParsableCommand {
                 Doctor.checkCodeSignature(),
             ]))
 
-            let status = LocalNetworkPolicy.status()
+            let status = LocalNetworkPermission.observedStatus()
             if !status.sourcePaths.isEmpty {
                 print("")
                 for path in status.sourcePaths {
@@ -55,9 +55,19 @@ struct PermissionsCommand: AsyncParsableCommand {
                 }
             }
 
+            if status.isIndeterminate {
+                print("")
+                print("The allowlist lives in root's preferences, which only root can read, so")
+                print("this cannot tell whether it is already set. For a definitive answer:")
+                print("  sudo gitea-macos-runner permissions status")
+            }
+
             guard !status.coversGuestRange else { return }
             print("")
-            print("to fix:")
+            // Not visible is not the same as not set, and an unconfigured host
+            // looks identical to a configured one from an ordinary login — so
+            // offer the commands without asserting anything is broken.
+            print(status.isIndeterminate ? "if it is not set, either of these sets it:" : "to fix:")
             print("  gitea-macos-runner permissions grant                  # subnet allowlist, needs a reboot")
             print("  gitea-macos-runner permissions grant --method prompt  # system prompt, takes effect at once")
         }
@@ -185,7 +195,29 @@ enum LocalNetworkGrantFlow {
         // `defaults` reports success regardless of which preferences directory
         // the write landed in, so report what was read back rather than what
         // was asked for. See LocalNetworkPermission.grantViaAllowlist.
-        guard result.verified else {
+        if result.verified {
+            print("granted: \(result.observed.allowlist.joined(separator: ", "))")
+            for path in result.observed.sourcePaths {
+                print("written to: \(path)")
+            }
+            if !result.observed.coversGuestRange {
+                CLI.note("""
+                    warning: none of these cover the whole guest range (192.168.64.0/18), \
+                    so guests will still be blocked once the NAT subnet shifts
+                    """)
+            }
+        } else if result.observed.isIndeterminate {
+            // The write succeeded but there is no way to look: the allowlist
+            // lands in root's preferences, and this host does not keep sudo
+            // credentials cached long enough for the read-back to use them.
+            // Unknown is not failure — say so plainly rather than either
+            // claiming success or crying wolf.
+            CLI.note("""
+                wrote \(subnets.joined(separator: ", ")), but could not read it back to \
+                confirm — that needs administrator rights this process no longer holds. \
+                Check it with: sudo defaults read \(LocalNetworkPolicy.domain)
+                """)
+        } else {
             CLI.error("""
                 the write reported success but the values could not be read back. \
                 Check by hand: sudo defaults read \(LocalNetworkPolicy.domain)
@@ -193,17 +225,6 @@ enum LocalNetworkGrantFlow {
             throw ExitCode(1)
         }
 
-        print("granted: \(result.observed.allowlist.joined(separator: ", "))")
-        for path in result.observed.sourcePaths {
-            print("written to: \(path)")
-        }
-        if !result.observed.coversGuestRange {
-            CLI.note("""
-                warning: none of these cover the whole guest range (192.168.64.0/18), \
-                so guests will still be blocked once the NAT subnet shifts
-                """)
-        }
-
         print("")
         print("This is read at boot, so it does nothing until the host reboots.")