From bf44b1a983776c1fe97d189e88656ad6bbd174b8 Mon Sep 17 00:00:00 2001
From: Oliver Drobnik <oliver@cocoanetics.com>
Date: Sun, 10 May 2026 22:58:54 +0200
Subject: [PATCH] Sandbox.Denial: clean description, don't leak URLs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`Sandbox.Denial` is thrown by `Shell.authorize(_:)` when policy
rejects a URL. The struct carries `url`, `reason`, and an
implementer-defined `suggestion: URL?` hint — the suggestion
typically encodes "where this URL would have landed under the
first sandbox root", which means it embeds the embedder's host
sandbox root.

For an iOS-app-as-sandbox embedder (Cocoanetics/iBash) that's a
direct PII-shaped leak. ArgumentParser's `fullMessage(for:)`
falls through to `String(describing: error)` for unrecognised
error types, and Swift's default reflective dump for a plain
struct prints every stored property:

    Error: Denial(
        url: file:///Users/.../Containers/.../Documents/Foo.bar,
        reason: "file URL is outside sandbox root",
        suggestion: Optional(file:///Users/.../home))

A SwiftPorts CLI like `gh auth login`, registered as a builtin
in an in-process bash, would print that to the user's terminal
on a single denied call.

Add `CustomStringConvertible` + `LocalizedError` conformances
that return only `reason`. Same surface for `"\(denial)"`,
`String(describing: denial)`, and `denial.localizedDescription`.
Callers that want the URLs read `.url` and `.suggestion`
directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 Sources/ShellKit/Sandbox/Sandbox.swift | 20 ++++++++++++++++-
 Tests/ShellKitTests/ShellTests.swift   | 31 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Sources/ShellKit/Sandbox/Sandbox.swift b/Sources/ShellKit/Sandbox/Sandbox.swift
index a6d595d..f0fe258 100644
--- a/Sources/ShellKit/Sandbox/Sandbox.swift
+++ b/Sources/ShellKit/Sandbox/Sandbox.swift
@@ -114,7 +114,7 @@ public struct Sandbox: Sendable {
     /// guaranteed to succeed. Callers MAY inspect it for diagnostics
     /// or opt-in recovery; ShellKit / SwiftPorts internals never
     /// inspect it and never retry.
-    public struct Denial: Error, Sendable {
+    public struct Denial: Error, Sendable, CustomStringConvertible, LocalizedError {
         public let url: URL
         public let reason: String
         public let suggestion: URL?
@@ -124,5 +124,23 @@ public struct Sandbox: Sendable {
             self.reason = reason
             self.suggestion = suggestion
         }
+
+        /// Human-readable description that intentionally omits both
+        /// `url` and `suggestion`. The default `String(describing:)`
+        /// dump (which ArgumentParser's `fullMessage(for:)` falls
+        /// through to for unrecognised error types) would otherwise
+        /// expose the embedder's host sandbox root — for an
+        /// app-as-sandbox embedder that means the iOS container path
+        /// (`/Users/.../Containers/.../Documents/Foo.bar/...`) ends
+        /// up in user-visible stderr on a single denied call.
+        ///
+        /// Callers needing the URLs read `.url` and `.suggestion`
+        /// directly. The reason is the only safe-to-display string.
+        public var description: String { reason }
+
+        /// Same surface for `LocalizedError` consumers — keeps
+        /// `(error as NSError).localizedDescription` and
+        /// `error.localizedDescription` in sync with `description`.
+        public var errorDescription: String? { reason }
     }
 }
diff --git a/Tests/ShellKitTests/ShellTests.swift b/Tests/ShellKitTests/ShellTests.swift
index 7e12c2e..950f182 100644
--- a/Tests/ShellKitTests/ShellTests.swift
+++ b/Tests/ShellKitTests/ShellTests.swift
@@ -122,6 +122,37 @@ import Testing
             // expected
         }
     }
+
+    /// Default `String(describing:)` for a struct dumps every stored
+    /// property — including `suggestion`, which carries the host
+    /// sandbox root. ArgumentParser's `fullMessage(for:)` falls
+    /// through to that for unrecognised error types, so a denied
+    /// `gh issue list` would otherwise leak the embedder's
+    /// container path. Pin both `description` and
+    /// `errorDescription` so any consumer (string interpolation,
+    /// `localizedDescription`, ArgumentParser) gets the reason
+    /// only.
+    @Test func denialDescriptionDoesNotLeakUrls() {
+        let denial = Sandbox.Denial(
+            url: URL(fileURLWithPath: "/secret/host/root/Documents/Untitled.foo"),
+            reason: "host 'github.com' is not in the sandbox allowlist",
+            suggestion: URL(fileURLWithPath: "/secret/host/root/Documents/Untitled.foo/home"))
+
+        let viaInterpolation = "\(denial)"
+        let viaDescribing = String(describing: denial)
+        let viaLocalized = denial.errorDescription ?? ""
+
+        for s in [viaInterpolation, viaDescribing, viaLocalized] {
+            #expect(s.contains("host 'github.com' is not in the sandbox allowlist"),
+                    "expected reason in: \(s)")
+            #expect(!s.contains("/secret/host/root"),
+                    "host path leaked into: \(s)")
+            #expect(!s.contains("suggestion"),
+                    "field name leaked into: \(s)")
+            #expect(!s.contains("Denial("),
+                    "struct shape leaked into: \(s)")
+        }
+    }
 }
 
 @Suite struct ArgumentParserBridgeTests {