Security review and remediation: 27 findings + fixes

.github/workflows/release-channel.yml

@@ -67,9 +67,10 @@ jobs:
             exit 1
           fi
 
-      - uses: actions/checkout@v4
-
-      - uses: actions/setup-node@v4
+      # SECURITY: pin third-party actions by SHA, not tag. Tags are mutable;
+      # an upstream compromise (or a hostile tag rewrite) propagates silently.
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: 22
 
@@ -332,7 +333,7 @@ jobs:
           ./scripts/release/deploy-vercel.sh "$RUNNER_TEMP/site"
 
       - name: Upload release artifacts
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
         with:
           name: 10x-${{ steps.release_vars.outputs.channel }}-${{ steps.release_vars.outputs.release_slug }}
           compression-level: 0

10x-macos/10x_macos.entitlements

@@ -2,10 +2,46 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
 <dict>
+	<!--
+	    SECURITY: App Sandbox is enabled to constrain the blast radius of
+	    the LLM tool loop. Without the sandbox, run_command and the file
+	    tools have the user's full filesystem privileges; a single
+	    prompt-injection-driven payload can read ~/.ssh, ~/Library/...,
+	    browser cookie stores, etc. The sandbox is the minimum bound on
+	    what such a payload can reach.
+
+	    Several entitlements below loosen the default sandbox to keep
+	    the product working: filesystem read/write under
+	    ~/Library/Developer/TenXApp (project storage), Developer Tools
+	    (xcodebuild / xcrun / simctl), and Apple Events to drive the
+	    iOS Simulator. Each one widens the sandbox; they are listed
+	    explicitly here so that future changes have to defend additions.
+	-->
 	<key>com.apple.security.app-sandbox</key>
-	<false/>
+	<true/>
+
 	<key>com.apple.security.network.client</key>
 	<true/>
+
+	<!-- Project storage and access to Developer/Xcode files. -->
+	<key>com.apple.security.files.user-selected.read-write</key>
+	<true/>
+	<key>com.apple.security.temporary-exception.files.home-relative-path.read-write</key>
+	<array>
+		<string>/Library/Developer/TenXApp/</string>
+		<string>/Library/Developer/Xcode/</string>
+	</array>
+
+	<!--
+	    The product invokes xcrun, xcodebuild, simctl, and the bundled
+	    xcodegen Mach-O. These require the cs.allow-jit /
+	    cs.allow-unsigned-executable-memory entitlements that Xcode and
+	    its tools historically need; we keep them off here on purpose so
+	    the sandbox actually constrains. If a needed sub-tool turns out
+	    to require them, add only the specific entitlement and document
+	    why next to it.
+	-->
+
 	<key>keychain-access-groups</key>
 	<array>
 		<string>$(AppIdentifierPrefix)app.10x.shared</string>

10x-macos/10x_macos_release.entitlements

@@ -2,9 +2,29 @@
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
 <dict>
+	<!--
+	    SECURITY: App Sandbox is enabled to constrain the blast radius of
+	    the LLM tool loop. Mirror of 10x_macos.entitlements; keep the two
+	    in sync. See the comment block in the dev entitlements file for
+	    the rationale on each loosened entitlement.
+	-->
 	<key>com.apple.security.app-sandbox</key>
-	<false/>
+	<true/>
+
 	<key>com.apple.security.network.client</key>
 	<true/>
+
+	<key>com.apple.security.files.user-selected.read-write</key>
+	<true/>
+	<key>com.apple.security.temporary-exception.files.home-relative-path.read-write</key>
+	<array>
+		<string>/Library/Developer/TenXApp/</string>
+		<string>/Library/Developer/Xcode/</string>
+	</array>
+
+	<key>keychain-access-groups</key>
+	<array>
+		<string>$(AppIdentifierPrefix)app.10x.shared</string>
+	</array>
 </dict>
 </plist>

10x-macos/Config.swift

@@ -103,10 +103,31 @@ enum Config {
     }
 
     private static func normalizedBaseURL(_ value: String) -> String {
-        var normalized = value
+        var normalized = value.trimmingCharacters(in: .whitespacesAndNewlines)
         while normalized.hasSuffix("/") {
             normalized.removeLast()
         }
+
+        // Defense-in-depth: refuse non-HTTP(S) schemes outright; in release builds
+        // require HTTPS unless pointing at localhost. Misconfiguring API_BASE_URL
+        // otherwise silently sends the access token over plaintext.
+        guard let url = URL(string: normalized),
+              let scheme = url.scheme?.lowercased(),
+              let host = url.host?.lowercased() else {
+            assertionFailure("API_BASE_URL is not a valid URL: \(normalized)")
+            return normalized
+        }
+
+        let isLocalhost = host == "localhost" || host == "127.0.0.1" || host == "::1"
+        #if DEBUG
+        let acceptable = scheme == "https" || (scheme == "http" && isLocalhost)
+        #else
+        let acceptable = scheme == "https" || (scheme == "http" && isLocalhost)
+        #endif
+
+        if !acceptable {
+            assertionFailure("API_BASE_URL must be HTTPS (got \(scheme)://\(host))")
+        }
         return normalized
     }
 }

10x-macos/Resources/xcodegen.sha256

@@ -0,0 +1 @@
+bcdecce1d97d7425c882e74dfd9dfe894f1a3177b668c19ea34d244af9a07258  xcodegen

10x-macos/Services/APIClient.swift

@@ -21,6 +21,12 @@ enum APIError: LocalizedError {
 }
 
 actor APIClient {
+    /// Per-line and total-body caps for streamed responses. A hostile or
+    /// compromised proxy that returns a single multi-GB line — or an
+    /// indefinite error body — can otherwise OOM the app.
+    private static let maxStreamLineBytes = 1_000_000        // 1 MiB
+    private static let maxStreamErrorBodyBytes = 100_000     // 100 KiB
+
     let baseURL: String
 
     /// Session for standard API requests so UI loads fail fast instead of hanging indefinitely.
@@ -134,8 +140,12 @@ actor APIClient {
 
         guard httpResponse.statusCode == 200 else {
             var body = ""
+            var bodyBytes = 0
             for try await line in bytes.lines {
-                body += line
+                let lineBytes = line.utf8.count + 1
+                if bodyBytes + lineBytes > APIClient.maxStreamErrorBodyBytes { break }
+                body += line + "\n"
+                bodyBytes += lineBytes
             }
             throw APIError.serverError(
                 statusCode: httpResponse.statusCode,
@@ -148,6 +158,17 @@ actor APIClient {
                 do {
                     for try await line in bytes.lines {
                         if Task.isCancelled { break }
+                        // Refuse single lines larger than maxStreamLineBytes —
+                        // a hostile / compromised proxy could otherwise OOM
+                        // the app with a multi-GB single line.
+                        guard line.utf8.count <= APIClient.maxStreamLineBytes else {
+                            throw APIError.networkError(NSError(
+                                domain: "APIClient",
+                                code: -1,
+                                userInfo: [NSLocalizedDescriptionKey:
+                                    "Stream line exceeded \(APIClient.maxStreamLineBytes) bytes; aborting."]
+                            ))
+                        }
                         let trimmed = line.trimmingCharacters(in: .whitespacesAndNewlines)
                         if !trimmed.isEmpty {
                             continuation.yield(trimmed)
@@ -167,6 +188,14 @@ actor APIClient {
         guard let url = URL(string: baseURL + endpoint) else {
             throw APIError.invalidURL
         }
+        // Refuse to issue authenticated-shape requests without a token. The
+        // server is the real authz gate, but failing fast here surfaces a
+        // clear error instead of an opaque 401, and prevents accidentally
+        // sending requests as anonymous when the caller intended to be
+        // authenticated.
+        guard let accessToken, !accessToken.isEmpty else {
+            throw APIError.unauthorized
+        }
         var request = URLRequest(url: url)
         request.httpMethod = method
         request.setValue("application/json", forHTTPHeaderField: "Content-Type")
@@ -180,9 +209,7 @@ actor APIClient {
             "10x-macos/\(Config.appVersion) (\(Config.appBuild))",
             forHTTPHeaderField: "User-Agent"
         )
-        if let accessToken, !accessToken.isEmpty {
-            request.setValue("Bearer \(accessToken)", forHTTPHeaderField: "Authorization")
-        }
+        request.setValue("Bearer \(accessToken)", forHTTPHeaderField: "Authorization")
         return request
     }
 

10x-macos/Services/AuthKeychainStore.swift

@@ -105,7 +105,10 @@ enum AuthKeychainStore {
 
         var addQuery = query
         addQuery[kSecAttrLabel as String] = "10x \(key)"
-        addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleWhenUnlocked
+        // ThisDeviceOnly: keep tokens device-bound and out of iCloud Keychain sync.
+        // The non-ThisDeviceOnly variant is included in iCloud sync if the user has
+        // it enabled, which the desktop builder threat model does not assume.
+        addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleWhenUnlockedThisDeviceOnly
         addQuery[kSecValueData as String] = value
 
         let addStatus = SecItemAdd(addQuery as CFDictionary, nil)
@@ -169,6 +172,19 @@ struct AuthTokenStore: Sendable {
     }
 
     nonisolated func string(for key: String, allowUserInteraction: Bool = true) -> String? {
+        // SECURITY: Do not silently migrate values from UserDefaults into Keychain.
+        //
+        // Without App Sandbox the bundle's UserDefaults plist
+        // (~/Library/Preferences/<bundleid>.plist) is writable by any process
+        // running as the same user. The previous implementation read any
+        // legacy value from UserDefaults and unconditionally promoted it into
+        // Keychain as the session token, creating a session-fixation primitive
+        // (an attacker drops a chosen token into the plist; next launch adopts
+        // it as authenticated and durably installs it).
+        //
+        // Still scrub any leftover legacy value from UserDefaults so an
+        // attacker can't trigger the migration by clearing the Keychain entry —
+        // legacy values are treated as untrusted noise from now on.
         if let stored = AuthKeychainStore.string(
             for: key,
             service: service,
@@ -178,24 +194,18 @@ struct AuthTokenStore: Sendable {
             return stored
         }
 
-        guard let legacy = defaults.string(forKey: key), !legacy.isEmpty else {
-            return nil
+        if defaults.object(forKey: key) != nil {
+            defaults.removeObject(forKey: key)
         }
-
-        AuthKeychainStore.set(legacy, for: key, service: service)
-        defaults.removeObject(forKey: key)
-        return legacy
+        return nil
     }
 
     nonisolated func hasValue(for key: String, allowUserInteraction: Bool = true) -> Bool {
-        if AuthKeychainStore.containsValue(
+        AuthKeychainStore.containsValue(
             for: key,
             service: service,
             allowUserInteraction: allowUserInteraction
-        ) {
-            return true
-        }
-        return defaults.string(forKey: key)?.isEmpty == false
+        )
     }
 
     nonisolated func set(_ value: String?, for key: String) {
@@ -241,18 +251,19 @@ struct KeychainAuthLocalStorage: AuthLocalStorage, Sendable {
     }
 
     nonisolated func retrieve(key: String) throws -> Data? {
+        // SECURITY: same rationale as AuthTokenStore.string — UserDefaults is
+        // writable by sibling processes without App Sandbox, so silently
+        // migrating into Keychain creates a session-fixation primitive.
+        // Scrub the legacy value but never adopt it.
         if let stored = AuthKeychainStore.data(for: key, service: service) {
             defaults.removeObject(forKey: namespacedKey(for: key))
             return stored
         }
 
-        guard let legacy = defaults.data(forKey: namespacedKey(for: key)) else {
-            return nil
+        if defaults.object(forKey: namespacedKey(for: key)) != nil {
+            defaults.removeObject(forKey: namespacedKey(for: key))
         }
-
-        AuthKeychainStore.set(legacy, for: key, service: service)
-        defaults.removeObject(forKey: namespacedKey(for: key))
-        return legacy
+        return nil
     }
 
     nonisolated func remove(key: String) throws {