Reinstate manifest sandboxing (#2848)

* Reinstate manifest sandboxing

Manifest loading has been sandboxed on macOS for a while, but the change
in #2518 broke it for 5.3.

https://bugs.swift.org/browse/SR-13346
rdar://problem/66586184

* Test manifest sandboxing

This adds tests for manifest sandboxing. Since we made the sandbox more
restrictive in 5.3, there are two tests:
- in 5.2 we check if we can make a network request
- in 5.3 we check if we can launch a process

* Fix 5.3 sandbox profile

* Use 127.0.0.1 for the attempted network request

It doesn't actually matter whether or not the request would succeed
since the test assumes it will always be blocked.

Author: neonichu

Sources/PackageLoading/ManifestLoader.swift

@@ -711,7 +711,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                         moduleCachePath.map({ AbsolutePath($0) })
                     ].compactMap({ $0 })
                     let profile = sandboxProfile(toolsVersion: toolsVersion, cacheDirectories: cacheDirectories)
-                    cmd += ["sandbox-exec", "-p", profile]
+                    cmd = ["sandbox-exec", "-p", profile] + cmd
                 }
               #endif
 
@@ -818,13 +818,14 @@ private func sandboxProfile(toolsVersion: ToolsVersion, cacheDirectories: [Absol
     stream <<< "(deny default)" <<< "\n"
     // Import the system sandbox profile.
     stream <<< "(import \"system.sb\")" <<< "\n"
+    // Allow reading all files. Even in 5.3 we need to be able to read the PD dylibs.
+    stream <<< "(allow file-read*)" <<< "\n"
+    // This is needed to launch any processes.
+    stream <<< "(allow process*)" <<< "\n"
 
     // The following accesses are only needed when interpreting the manifest (versus running a compiled version).
     if toolsVersion < .v5_3 {
-        // Allow reading all files.
-        stream <<< "(allow file-read*)" <<< "\n"
-        // These are required by the Swift compiler.
-        stream <<< "(allow process*)" <<< "\n"
+        // This is required by the Swift compiler.
         stream <<< "(allow sysctl*)" <<< "\n"
         // Allow writing in temporary locations.
         stream <<< "(allow file-write*" <<< "\n"
@@ -834,9 +835,9 @@ private func sandboxProfile(toolsVersion: ToolsVersion, cacheDirectories: [Absol
         for directory in cacheDirectories {
             stream <<< "    (subpath \"\(directory.pathString)\")" <<< "\n"
         }
+        stream <<< ")" <<< "\n"
     }
 
-    stream <<< ")" <<< "\n"
     return stream.bytes.description
 }
 

Tests/PackageLoadingTests/PD5_2LoadingTests.swift

@@ -345,4 +345,28 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
             }
         }
     }
+
+    func testManifestLoadingIsSandboxed() throws {
+        #if os(macOS) // Sandboxing is only done on macOS today.
+        let stream = BufferedOutputByteStream()
+        stream <<< """
+            import Foundation
+
+            try! String(contentsOf:URL(string: "http://127.0.0.1")!)
+
+            import PackageDescription
+            let package = Package(
+                name: "Foo",
+                targets: [
+                    .target(name: "Foo"),
+                ]
+            )
+            """
+
+        XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
+            guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
+            XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
+        }
+        #endif
+    }
 }

Tests/PackageLoadingTests/PD5_3LoadingTests.swift

@@ -406,4 +406,28 @@ class PackageDescriptionNextLoadingTests: PackageDescriptionLoadingTests {
             XCTAssertTrue(error is ManifestParseError, "unexpected error: \(error)")
         }
     }
+
+    func testManifestLoadingIsSandboxed() throws {
+        #if os(macOS) // Sandboxing is only done on macOS today.
+        let stream = BufferedOutputByteStream()
+        stream <<< """
+            import Foundation
+
+            try! "should not be allowed".write(to: URL(fileURLWithPath: "/tmp/file.txt"), atomically: true, encoding: String.Encoding.utf8)
+
+            import PackageDescription
+            let package = Package(
+                name: "Foo",
+                targets: [
+                    .target(name: "Foo"),
+                ]
+            )
+            """
+
+        XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
+            guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
+            XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
+        }
+        #endif
+    }
 }