From 9033d790aeb1c6abcb4094aae5dbcbfabfb1ca40 Mon Sep 17 00:00:00 2001
From: Jack Freeman <110849539+jackfreem@users.noreply.github.com>
Date: Tue, 10 Mar 2026 15:33:54 -0400
Subject: [PATCH 1/5] Fix shell injection risk in AndroidEmulatorHelper.openUrl

---
 Package.swift                                 |   6 +
 .../qrgo/Helpers/AndroidEmulatorHelper.swift  |  24 +++-
 Sources/qrgoLib/URLSanitizer.swift            |  36 ++++++
 Tests/qrgoTests/URLSanitizerTests.swift       | 121 ++++++++++++++++++
 4 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 Sources/qrgoLib/URLSanitizer.swift
 create mode 100644 Tests/qrgoTests/URLSanitizerTests.swift

diff --git a/Package.swift b/Package.swift
index 886f897..50091ac 100644
--- a/Package.swift
+++ b/Package.swift
@@ -10,6 +10,12 @@ let package = Package(
     targets: [
         .executableTarget(
             name: "qrgo",
+            dependencies: ["qrgoLib"]),
+        .target(
+            name: "qrgoLib",
             dependencies: []),
+        .testTarget(
+            name: "qrgoTests",
+            dependencies: ["qrgoLib"]),
     ]
 )
diff --git a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
index f93df4d..19a46fa 100644
--- a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
+++ b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
@@ -1,4 +1,5 @@
 import Foundation
+import qrgoLib
 
 class AndroidEmulatorHelper {
     private static var _cachedAdbPath: String? = nil
@@ -107,6 +108,23 @@ class AndroidEmulatorHelper {
         return props
     }
 
+    private static func validateAndSanitizeUrl(_ urlString: String) -> String? {
+        guard let safe = sanitizeUrlForAndroidShell(urlString) else {
+            guard let url = URL(string: urlString) else {
+                printError("Malformed or unsupported URL, cannot open on Android device.")
+                return nil
+            }
+            let scheme = url.scheme?.lowercased() ?? ""
+            if !["http", "https", "cashme"].contains(scheme) {
+                printError("URL scheme '\(scheme.isEmpty ? "(none)" : scheme)' is not allowed. Only http, https, and cashme are permitted.")
+            } else {
+                printError("URL contains characters that are not permitted for Android shell.")
+            }
+            return nil
+        }
+        return safe
+    }
+
     @discardableResult
     static func openUrl(_ urlString: String, deviceId: String? = nil, validated: Bool = false) -> Bool {
         guard let adbPath = findAdbPath() else {
@@ -114,6 +132,10 @@ class AndroidEmulatorHelper {
             return false
         }
 
+        guard let safeUrlString = validateAndSanitizeUrl(urlString) else {
+            return false
+        }
+
         let targetDevice: String
         if let deviceId = deviceId, validated {
             targetDevice = deviceId
@@ -133,7 +155,7 @@ class AndroidEmulatorHelper {
 
         let result = Shell.runCommand(
             adbPath,
-            arguments: ["-s", targetDevice, "shell", "am", "start", "-a", "android.intent.action.VIEW", "-c", "android.intent.category.BROWSABLE", "-d", urlString],
+            arguments: ["-s", targetDevice, "shell", "am", "start", "-a", "android.intent.action.VIEW", "-c", "android.intent.category.BROWSABLE", "-d", safeUrlString],
             mergeStderr: true
         )
         let deviceName = getDeviceFriendlyName(targetDevice)
diff --git a/Sources/qrgoLib/URLSanitizer.swift b/Sources/qrgoLib/URLSanitizer.swift
new file mode 100644
index 0000000..c633ce7
--- /dev/null
+++ b/Sources/qrgoLib/URLSanitizer.swift
@@ -0,0 +1,36 @@
+import Foundation
+
+/// Validates and sanitizes a URL string for safe use with Android shell commands via `adb shell am start -d`.
+///
+/// Because `adb shell` invokes the Android `/bin/sh`, shell metacharacters in the URL would be
+/// interpreted as shell syntax rather than as URL data. This function:
+///   1. Rejects malformed URLs
+///   2. Enforces an allowlist of URL schemes
+///   3. Re-serializes via `URL` (which normalizes percent-encoding)
+///   4. Rejects any characters with special meaning to POSIX shells
+///
+/// Note: percent-encoded metacharacters (e.g. `%3B` for `;`) survive safely — they appear as
+/// literal `%`, digit, and letter characters in the serialized string, none of which are dangerous.
+///
+/// - Parameters:
+///   - urlString: The raw URL string to validate.
+///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `["http", "https", "cashme"]`.
+/// - Returns: The normalized URL string if valid, or `nil` if it should be rejected.
+public func sanitizeUrlForAndroidShell(
+    _ urlString: String,
+    allowedSchemes: Set<String> = ["http", "https", "cashme"]
+) -> String? {
+    guard let url = URL(string: urlString) else { return nil }
+
+    guard let scheme = url.scheme?.lowercased(), allowedSchemes.contains(scheme) else { return nil }
+
+    // Re-serialize to normalize percent-encoding
+    let sanitized = url.absoluteString
+
+    // Reject shell metacharacters. Note: space is included as a safety net even though
+    // URL(string:) percent-encodes spaces in path/query components.
+    let dangerous = CharacterSet(charactersIn: ";&|><`$()\\\"'\r\n\0 ")
+    guard !sanitized.unicodeScalars.contains(where: { dangerous.contains($0) }) else { return nil }
+
+    return sanitized
+}
diff --git a/Tests/qrgoTests/URLSanitizerTests.swift b/Tests/qrgoTests/URLSanitizerTests.swift
new file mode 100644
index 0000000..0bc94b9
--- /dev/null
+++ b/Tests/qrgoTests/URLSanitizerTests.swift
@@ -0,0 +1,121 @@
+import XCTest
+@testable import qrgoLib
+
+final class URLSanitizerTests: XCTestCase {
+
+    // MARK: - Valid URLs
+
+    func testHttpUrlPassesThrough() {
+        let result = sanitizeUrlForAndroidShell("http://example.com/path?query=value")
+        XCTAssertEqual(result, "http://example.com/path?query=value")
+    }
+
+    func testHttpsUrlPassesThrough() {
+        let result = sanitizeUrlForAndroidShell("https://cash.app/some/path")
+        XCTAssertEqual(result, "https://cash.app/some/path")
+    }
+
+    func testCashmeDeepLinkPassesThrough() {
+        let result = sanitizeUrlForAndroidShell("cashme://pay/john")
+        XCTAssertEqual(result, "cashme://pay/john")
+    }
+
+    func testUrlWithFragmentPassesThrough() {
+        let result = sanitizeUrlForAndroidShell("https://example.com/page#section")
+        XCTAssertEqual(result, "https://example.com/page#section")
+    }
+
+    func testUrlWithPortPassesThrough() {
+        let result = sanitizeUrlForAndroidShell("http://localhost:8080/path")
+        XCTAssertEqual(result, "http://localhost:8080/path")
+    }
+
+    // MARK: - Percent-encoded metacharacters are safe
+
+    func testPercentEncodedSemicolonIsAllowed() {
+        // %3B decodes to ';' but appears as literal %3B in the serialized string — safe
+        let result = sanitizeUrlForAndroidShell("https://example.com/path%3Breboot")
+        XCTAssertEqual(result, "https://example.com/path%3Breboot")
+    }
+
+    func testPercentEncodedDollarIsAllowed() {
+        let result = sanitizeUrlForAndroidShell("https://example.com/path%24var")
+        XCTAssertEqual(result, "https://example.com/path%24var")
+    }
+
+    func testPercentEncodedPipeIsAllowed() {
+        let result = sanitizeUrlForAndroidShell("https://example.com/%7Cpath")
+        XCTAssertEqual(result, "https://example.com/%7Cpath")
+    }
+
+    // MARK: - Scheme allowlist
+
+    func testJavascriptSchemeIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("javascript:alert(1)"))
+    }
+
+    func testFileSchemeIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("file:///etc/passwd"))
+    }
+
+    func testFtpSchemeIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("ftp://example.com/file"))
+    }
+
+    func testCustomSchemeNotInAllowlistIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("myapp://action"))
+    }
+
+    func testCustomSchemeInAllowlistIsAccepted() {
+        let result = sanitizeUrlForAndroidShell("myapp://action", allowedSchemes: ["myapp"])
+        XCTAssertEqual(result, "myapp://action")
+    }
+
+    // MARK: - Shell metacharacter injection
+
+    func testSemicolonInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com;reboot"))
+    }
+
+    func testAmpersandInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com&id"))
+    }
+
+    func testPipeInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com|cat /etc/passwd"))
+    }
+
+    func testBacktickInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com`id`"))
+    }
+
+    func testDollarSignInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com$(id)"))
+    }
+
+    func testCommandSubstitutionInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com$(reboot)"))
+    }
+
+    func testRedirectInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com>/tmp/out"))
+    }
+
+    func testNewlineInjectionIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com\nreboot"))
+    }
+
+    // MARK: - Malformed URLs
+
+    func testEmptyStringIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell(""))
+    }
+
+    func testNoSchemeIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("example.com/path"))
+    }
+
+    func testGibberishIsRejected() {
+        XCTAssertNil(sanitizeUrlForAndroidShell("not a url at all!!"))
+    }
+}

From 06bf07cef3470f3beae13cb09690691ba9269679 Mon Sep 17 00:00:00 2001
From: Jack Freeman <110849539+jackfreem@users.noreply.github.com>
Date: Tue, 10 Mar 2026 17:53:11 -0400
Subject: [PATCH 2/5] Move URL sanitizer into Helpers/ alongside other helpers

---
 Package.swift                                 |   6 -
 .../qrgo/Helpers/AndroidEmulatorHelper.swift  |   1 -
 .../Helpers}/URLSanitizer.swift               |   2 +-
 Tests/qrgoTests/URLSanitizerTests.swift       | 121 ------------------
 4 files changed, 1 insertion(+), 129 deletions(-)
 rename Sources/{qrgoLib => qrgo/Helpers}/URLSanitizer.swift (97%)
 delete mode 100644 Tests/qrgoTests/URLSanitizerTests.swift

diff --git a/Package.swift b/Package.swift
index 50091ac..886f897 100644
--- a/Package.swift
+++ b/Package.swift
@@ -10,12 +10,6 @@ let package = Package(
     targets: [
         .executableTarget(
             name: "qrgo",
-            dependencies: ["qrgoLib"]),
-        .target(
-            name: "qrgoLib",
             dependencies: []),
-        .testTarget(
-            name: "qrgoTests",
-            dependencies: ["qrgoLib"]),
     ]
 )
diff --git a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
index 19a46fa..2a1d452 100644
--- a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
+++ b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
@@ -1,5 +1,4 @@
 import Foundation
-import qrgoLib
 
 class AndroidEmulatorHelper {
     private static var _cachedAdbPath: String? = nil
diff --git a/Sources/qrgoLib/URLSanitizer.swift b/Sources/qrgo/Helpers/URLSanitizer.swift
similarity index 97%
rename from Sources/qrgoLib/URLSanitizer.swift
rename to Sources/qrgo/Helpers/URLSanitizer.swift
index c633ce7..6112155 100644
--- a/Sources/qrgoLib/URLSanitizer.swift
+++ b/Sources/qrgo/Helpers/URLSanitizer.swift
@@ -16,7 +16,7 @@ import Foundation
 ///   - urlString: The raw URL string to validate.
 ///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `["http", "https", "cashme"]`.
 /// - Returns: The normalized URL string if valid, or `nil` if it should be rejected.
-public func sanitizeUrlForAndroidShell(
+func sanitizeUrlForAndroidShell(
     _ urlString: String,
     allowedSchemes: Set<String> = ["http", "https", "cashme"]
 ) -> String? {
diff --git a/Tests/qrgoTests/URLSanitizerTests.swift b/Tests/qrgoTests/URLSanitizerTests.swift
deleted file mode 100644
index 0bc94b9..0000000
--- a/Tests/qrgoTests/URLSanitizerTests.swift
+++ /dev/null
@@ -1,121 +0,0 @@
-import XCTest
-@testable import qrgoLib
-
-final class URLSanitizerTests: XCTestCase {
-
-    // MARK: - Valid URLs
-
-    func testHttpUrlPassesThrough() {
-        let result = sanitizeUrlForAndroidShell("http://example.com/path?query=value")
-        XCTAssertEqual(result, "http://example.com/path?query=value")
-    }
-
-    func testHttpsUrlPassesThrough() {
-        let result = sanitizeUrlForAndroidShell("https://cash.app/some/path")
-        XCTAssertEqual(result, "https://cash.app/some/path")
-    }
-
-    func testCashmeDeepLinkPassesThrough() {
-        let result = sanitizeUrlForAndroidShell("cashme://pay/john")
-        XCTAssertEqual(result, "cashme://pay/john")
-    }
-
-    func testUrlWithFragmentPassesThrough() {
-        let result = sanitizeUrlForAndroidShell("https://example.com/page#section")
-        XCTAssertEqual(result, "https://example.com/page#section")
-    }
-
-    func testUrlWithPortPassesThrough() {
-        let result = sanitizeUrlForAndroidShell("http://localhost:8080/path")
-        XCTAssertEqual(result, "http://localhost:8080/path")
-    }
-
-    // MARK: - Percent-encoded metacharacters are safe
-
-    func testPercentEncodedSemicolonIsAllowed() {
-        // %3B decodes to ';' but appears as literal %3B in the serialized string — safe
-        let result = sanitizeUrlForAndroidShell("https://example.com/path%3Breboot")
-        XCTAssertEqual(result, "https://example.com/path%3Breboot")
-    }
-
-    func testPercentEncodedDollarIsAllowed() {
-        let result = sanitizeUrlForAndroidShell("https://example.com/path%24var")
-        XCTAssertEqual(result, "https://example.com/path%24var")
-    }
-
-    func testPercentEncodedPipeIsAllowed() {
-        let result = sanitizeUrlForAndroidShell("https://example.com/%7Cpath")
-        XCTAssertEqual(result, "https://example.com/%7Cpath")
-    }
-
-    // MARK: - Scheme allowlist
-
-    func testJavascriptSchemeIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("javascript:alert(1)"))
-    }
-
-    func testFileSchemeIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("file:///etc/passwd"))
-    }
-
-    func testFtpSchemeIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("ftp://example.com/file"))
-    }
-
-    func testCustomSchemeNotInAllowlistIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("myapp://action"))
-    }
-
-    func testCustomSchemeInAllowlistIsAccepted() {
-        let result = sanitizeUrlForAndroidShell("myapp://action", allowedSchemes: ["myapp"])
-        XCTAssertEqual(result, "myapp://action")
-    }
-
-    // MARK: - Shell metacharacter injection
-
-    func testSemicolonInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com;reboot"))
-    }
-
-    func testAmpersandInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com&id"))
-    }
-
-    func testPipeInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com|cat /etc/passwd"))
-    }
-
-    func testBacktickInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com`id`"))
-    }
-
-    func testDollarSignInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com$(id)"))
-    }
-
-    func testCommandSubstitutionInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com$(reboot)"))
-    }
-
-    func testRedirectInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com>/tmp/out"))
-    }
-
-    func testNewlineInjectionIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("http://example.com\nreboot"))
-    }
-
-    // MARK: - Malformed URLs
-
-    func testEmptyStringIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell(""))
-    }
-
-    func testNoSchemeIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("example.com/path"))
-    }
-
-    func testGibberishIsRejected() {
-        XCTAssertNil(sanitizeUrlForAndroidShell("not a url at all!!"))
-    }
-}

From f7bed580ce99df535e170241f78e501b3c9de9bc Mon Sep 17 00:00:00 2001
From: Jack Freeman <110849539+jackfreem@users.noreply.github.com>
Date: Wed, 11 Mar 2026 17:01:26 -0400
Subject: [PATCH 3/5] Address Copilot review: switch to shell-quoting and
 centralize scheme allowlist

- Replace broad shell-metacharacter denylist with single-quote approach:
  sanitizeUrlForAndroidShell now only rejects characters that can break
  out of POSIX single quotes (', \0, \r, \n), allowing legitimate URLs
  with & in query strings to pass through safely
- Wrap the sanitized URL in single quotes in the adb shell invocation
  so the Android /bin/sh treats & ; | etc. as literal URL data
- Extract allowedUrlSchemes constant so sanitizeUrlForAndroidShell and
  validateAndSanitizeUrl share a single source of truth for the allowlist
- Update PR description to reflect actual package structure (no qrgoLib
  library target; sanitizer lives alongside other helpers in qrgo target)
---
 .../qrgo/Helpers/AndroidEmulatorHelper.swift  |  4 +--
 Sources/qrgo/Helpers/URLSanitizer.swift       | 29 ++++++++++++-------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
index 2a1d452..03d2068 100644
--- a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
+++ b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
@@ -114,7 +114,7 @@ class AndroidEmulatorHelper {
                 return nil
             }
             let scheme = url.scheme?.lowercased() ?? ""
-            if !["http", "https", "cashme"].contains(scheme) {
+            if !allowedUrlSchemes.contains(scheme) {
                 printError("URL scheme '\(scheme.isEmpty ? "(none)" : scheme)' is not allowed. Only http, https, and cashme are permitted.")
             } else {
                 printError("URL contains characters that are not permitted for Android shell.")
@@ -154,7 +154,7 @@ class AndroidEmulatorHelper {
 
         let result = Shell.runCommand(
             adbPath,
-            arguments: ["-s", targetDevice, "shell", "am", "start", "-a", "android.intent.action.VIEW", "-c", "android.intent.category.BROWSABLE", "-d", safeUrlString],
+            arguments: ["-s", targetDevice, "shell", "am", "start", "-a", "android.intent.action.VIEW", "-c", "android.intent.category.BROWSABLE", "-d", "'\(safeUrlString)'"],
             mergeStderr: true
         )
         let deviceName = getDeviceFriendlyName(targetDevice)
diff --git a/Sources/qrgo/Helpers/URLSanitizer.swift b/Sources/qrgo/Helpers/URLSanitizer.swift
index 6112155..e3e29bf 100644
--- a/Sources/qrgo/Helpers/URLSanitizer.swift
+++ b/Sources/qrgo/Helpers/URLSanitizer.swift
@@ -1,24 +1,31 @@
 import Foundation
 
-/// Validates and sanitizes a URL string for safe use with Android shell commands via `adb shell am start -d`.
+/// Allowed URL schemes for opening on Android devices.
+let allowedUrlSchemes: Set<String> = ["http", "https", "cashme"]
+
+/// Validates and sanitizes a URL string for safe embedding in a single-quoted POSIX shell argument.
+///
+/// Because `adb shell` joins its arguments with spaces and passes the result to the Android
+/// device's `/bin/sh`, the caller must wrap the returned string in single quotes (e.g.
+/// `"-d", "'\(safe)'"`) to prevent shell interpretation of characters like `&`, `;`, `|`, etc.
 ///
-/// Because `adb shell` invokes the Android `/bin/sh`, shell metacharacters in the URL would be
-/// interpreted as shell syntax rather than as URL data. This function:
+/// This function:
 ///   1. Rejects malformed URLs
 ///   2. Enforces an allowlist of URL schemes
 ///   3. Re-serializes via `URL` (which normalizes percent-encoding)
-///   4. Rejects any characters with special meaning to POSIX shells
+///   4. Rejects characters that can break out of a single-quoted POSIX string: `'`, `\0`, `\r`, `\n`
 ///
-/// Note: percent-encoded metacharacters (e.g. `%3B` for `;`) survive safely — they appear as
-/// literal `%`, digit, and letter characters in the serialized string, none of which are dangerous.
+/// Note: Characters like `&`, `;`, `|`, `>`, `<` are safe to return — they cannot escape
+/// single quotes and will be treated literally by the shell when the caller wraps the URL
+/// in single quotes.
 ///
 /// - Parameters:
 ///   - urlString: The raw URL string to validate.
-///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `["http", "https", "cashme"]`.
+///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `allowedUrlSchemes`.
 /// - Returns: The normalized URL string if valid, or `nil` if it should be rejected.
 func sanitizeUrlForAndroidShell(
     _ urlString: String,
-    allowedSchemes: Set<String> = ["http", "https", "cashme"]
+    allowedSchemes: Set<String> = allowedUrlSchemes
 ) -> String? {
     guard let url = URL(string: urlString) else { return nil }
 
@@ -27,9 +34,9 @@ func sanitizeUrlForAndroidShell(
     // Re-serialize to normalize percent-encoding
     let sanitized = url.absoluteString
 
-    // Reject shell metacharacters. Note: space is included as a safety net even though
-    // URL(string:) percent-encodes spaces in path/query components.
-    let dangerous = CharacterSet(charactersIn: ";&|><`$()\\\"'\r\n\0 ")
+    // Reject characters that can break out of a single-quoted POSIX shell string.
+    // The caller is responsible for wrapping the result in single quotes.
+    let dangerous = CharacterSet(charactersIn: "'\r\n\0")
     guard !sanitized.unicodeScalars.contains(where: { dangerous.contains($0) }) else { return nil }
 
     return sanitized

From 4e1eb9e66635cd7e21b09c743703a225c505de34 Mon Sep 17 00:00:00 2001
From: Jack Freeman <110849539+jackfreem@users.noreply.github.com>
Date: Thu, 12 Mar 2026 09:54:29 -0400
Subject: [PATCH 4/5] Address second round of Copilot review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Pass am start as a single command string to adb shell so the Android
  device's /bin/sh interprets the single-quote wrapping around the URL,
  rather than relying on adb to preserve embedded quotes in argv
- Clarify URLSanitizer doc: the function does not reject shell
  metacharacters like & ; | — it relies on the caller single-quoting
  the URL in the command string
---
 Sources/qrgo/Helpers/AndroidEmulatorHelper.swift |  3 ++-
 Sources/qrgo/Helpers/URLSanitizer.swift          | 13 +++++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
index 03d2068..7db2ac5 100644
--- a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
+++ b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
@@ -152,9 +152,10 @@ class AndroidEmulatorHelper {
             }
         }
 
+        let shellCommand = "am start -a android.intent.action.VIEW -c android.intent.category.BROWSABLE -d '\(safeUrlString)'"
         let result = Shell.runCommand(
             adbPath,
-            arguments: ["-s", targetDevice, "shell", "am", "start", "-a", "android.intent.action.VIEW", "-c", "android.intent.category.BROWSABLE", "-d", "'\(safeUrlString)'"],
+            arguments: ["-s", targetDevice, "shell", shellCommand],
             mergeStderr: true
         )
         let deviceName = getDeviceFriendlyName(targetDevice)
diff --git a/Sources/qrgo/Helpers/URLSanitizer.swift b/Sources/qrgo/Helpers/URLSanitizer.swift
index e3e29bf..6d17431 100644
--- a/Sources/qrgo/Helpers/URLSanitizer.swift
+++ b/Sources/qrgo/Helpers/URLSanitizer.swift
@@ -3,11 +3,12 @@ import Foundation
 /// Allowed URL schemes for opening on Android devices.
 let allowedUrlSchemes: Set<String> = ["http", "https", "cashme"]
 
-/// Validates and sanitizes a URL string for safe embedding in a single-quoted POSIX shell argument.
+/// Validates and sanitizes a URL string for safe use inside a single-quoted POSIX shell argument.
 ///
-/// Because `adb shell` joins its arguments with spaces and passes the result to the Android
-/// device's `/bin/sh`, the caller must wrap the returned string in single quotes (e.g.
-/// `"-d", "'\(safe)'"`) to prevent shell interpretation of characters like `&`, `;`, `|`, etc.
+/// This function does NOT reject shell metacharacters like `&`, `;`, or `|` — those are safe
+/// when the URL is wrapped in single quotes by the caller (e.g. `"am start -d '\(safe)'"` passed
+/// as a single command string to `adb shell`). It only rejects characters that can break out of
+/// single-quoted strings themselves.
 ///
 /// This function:
 ///   1. Rejects malformed URLs
@@ -15,10 +16,6 @@ let allowedUrlSchemes: Set<String> = ["http", "https", "cashme"]
 ///   3. Re-serializes via `URL` (which normalizes percent-encoding)
 ///   4. Rejects characters that can break out of a single-quoted POSIX string: `'`, `\0`, `\r`, `\n`
 ///
-/// Note: Characters like `&`, `;`, `|`, `>`, `<` are safe to return — they cannot escape
-/// single quotes and will be treated literally by the shell when the caller wraps the URL
-/// in single quotes.
-///
 /// - Parameters:
 ///   - urlString: The raw URL string to validate.
 ///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `allowedUrlSchemes`.

From 7745ce819ae13d516a08930694d1968b491b7d87 Mon Sep 17 00:00:00 2001
From: Jack Freeman <110849539+jackfreem@users.noreply.github.com>
Date: Thu, 12 Mar 2026 15:23:10 -0400
Subject: [PATCH 5/5] Address third round of review feedback

- Return Result<String, URLSanitizationError> from sanitizeUrlForAndroidShell
  instead of String? so callers get a typed reason for rejection (.malformed,
  .disallowedScheme, .dangerousCharacters) without re-parsing the URL
- Update validateAndSanitizeUrl to switch on the error cases directly,
  removing the duplicated URL(string:) re-parse for error message selection
- Document allowedUrlSchemes as the single source of truth for scheme
  validation across the module
- Add inline comment above the adb shell invocation explaining the security
  model: adb shell passes the string to /bin/sh -c on device, single quotes
  neutralize metacharacters, and the sanitizer ensures no single-quote breakout
---
 .../qrgo/Helpers/AndroidEmulatorHelper.swift  | 27 ++++++++++---------
 Sources/qrgo/Helpers/URLSanitizer.swift       | 25 ++++++++++++-----
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
index 7db2ac5..c99bce7 100644
--- a/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
+++ b/Sources/qrgo/Helpers/AndroidEmulatorHelper.swift
@@ -108,20 +108,19 @@ class AndroidEmulatorHelper {
     }
 
     private static func validateAndSanitizeUrl(_ urlString: String) -> String? {
-        guard let safe = sanitizeUrlForAndroidShell(urlString) else {
-            guard let url = URL(string: urlString) else {
-                printError("Malformed or unsupported URL, cannot open on Android device.")
-                return nil
-            }
-            let scheme = url.scheme?.lowercased() ?? ""
-            if !allowedUrlSchemes.contains(scheme) {
-                printError("URL scheme '\(scheme.isEmpty ? "(none)" : scheme)' is not allowed. Only http, https, and cashme are permitted.")
-            } else {
-                printError("URL contains characters that are not permitted for Android shell.")
-            }
+        switch sanitizeUrlForAndroidShell(urlString) {
+        case .success(let safe):
+            return safe
+        case .failure(.malformed):
+            printError("Malformed or unsupported URL, cannot open on Android device.")
+            return nil
+        case .failure(.disallowedScheme(let scheme)):
+            printError("URL scheme '\(scheme.isEmpty ? "(none)" : scheme)' is not allowed. Only http, https, and cashme are permitted.")
+            return nil
+        case .failure(.dangerousCharacters):
+            printError("URL contains characters that are not permitted for Android shell.")
             return nil
         }
-        return safe
     }
 
     @discardableResult
@@ -152,6 +151,10 @@ class AndroidEmulatorHelper {
             }
         }
 
+        // `adb shell <string>` passes the string to `/bin/sh -c` on the Android device.
+        // The URL is wrapped in single quotes so the shell treats &, ;, |, etc. as literal
+        // URL data rather than shell operators. The sanitizer guarantees the URL contains no
+        // single quotes, making breakout from the single-quoted string impossible.
         let shellCommand = "am start -a android.intent.action.VIEW -c android.intent.category.BROWSABLE -d '\(safeUrlString)'"
         let result = Shell.runCommand(
             adbPath,
diff --git a/Sources/qrgo/Helpers/URLSanitizer.swift b/Sources/qrgo/Helpers/URLSanitizer.swift
index 6d17431..31547eb 100644
--- a/Sources/qrgo/Helpers/URLSanitizer.swift
+++ b/Sources/qrgo/Helpers/URLSanitizer.swift
@@ -1,8 +1,16 @@
 import Foundation
 
-/// Allowed URL schemes for opening on Android devices.
+/// The canonical allowlist of URL schemes permitted for opening on Android devices.
+/// This is the single source of truth used by both the sanitizer and error reporting.
 let allowedUrlSchemes: Set<String> = ["http", "https", "cashme"]
 
+/// Reasons a URL can be rejected by `sanitizeUrlForAndroidShell`.
+enum URLSanitizationError: Error {
+    case malformed
+    case disallowedScheme(String)
+    case dangerousCharacters
+}
+
 /// Validates and sanitizes a URL string for safe use inside a single-quoted POSIX shell argument.
 ///
 /// This function does NOT reject shell metacharacters like `&`, `;`, or `|` — those are safe
@@ -19,14 +27,15 @@ let allowedUrlSchemes: Set<String> = ["http", "https", "cashme"]
 /// - Parameters:
 ///   - urlString: The raw URL string to validate.
 ///   - allowedSchemes: Set of lowercase scheme strings to permit. Defaults to `allowedUrlSchemes`.
-/// - Returns: The normalized URL string if valid, or `nil` if it should be rejected.
+/// - Returns: The normalized URL string on success, or a `URLSanitizationError` describing the failure.
 func sanitizeUrlForAndroidShell(
     _ urlString: String,
     allowedSchemes: Set<String> = allowedUrlSchemes
-) -> String? {
-    guard let url = URL(string: urlString) else { return nil }
+) -> Result<String, URLSanitizationError> {
+    guard let url = URL(string: urlString) else { return .failure(.malformed) }
 
-    guard let scheme = url.scheme?.lowercased(), allowedSchemes.contains(scheme) else { return nil }
+    let scheme = url.scheme?.lowercased() ?? ""
+    guard allowedSchemes.contains(scheme) else { return .failure(.disallowedScheme(scheme)) }
 
     // Re-serialize to normalize percent-encoding
     let sanitized = url.absoluteString
@@ -34,7 +43,9 @@ func sanitizeUrlForAndroidShell(
     // Reject characters that can break out of a single-quoted POSIX shell string.
     // The caller is responsible for wrapping the result in single quotes.
     let dangerous = CharacterSet(charactersIn: "'\r\n\0")
-    guard !sanitized.unicodeScalars.contains(where: { dangerous.contains($0) }) else { return nil }
+    guard !sanitized.unicodeScalars.contains(where: { dangerous.contains($0) }) else {
+        return .failure(.dangerousCharacters)
+    }
 
-    return sanitized
+    return .success(sanitized)
 }