Sandbox blocks output to default plugin output directory when it's under <pkgdir>/.build (#4009)

The sandbox rules introduced in #3996 made the entire package directory read-only, but that isn't appropriate when `.build` is inside the package directory.

The rules for applying read-only directories were intended to shadow the implicit rules added by specifying a writable temporary directory, not to block the ones specified by explicit writable directories.  This fixes that ordering, and adds the missing unit test that would have caught the problem.

A future change reworks Sandbox completely so that it becomes a struct that can carry around the sandbox profile configuration until it is applied — this change is intended to be small enough to be nominatable

rdar://87417780

Author: abertelrud

Sources/Basics/Sandbox.swift

@@ -21,8 +21,8 @@ public enum Sandbox {
     /// - Parameters:
     ///   - command: The command line to sandbox (including executable as first argument)
     ///   - strictness: The basic strictness level of the standbox.
-    ///   - writableDirectories: Paths under which writing should be allowed.
-    ///   - readOnlyDirectories: Paths under which writing should be denied, even if they would have otherwise been allowed by either the strictness level or paths in `writableDirectories`.
+    ///   - writableDirectories: Paths under which writing should be allowed, even if they would otherwise be read-only based on the strictness or paths in `readOnlyDirectories`.
+    ///   - readOnlyDirectories: Paths under which writing should be denied, even if they would have otherwise been allowed by the rules implied by the strictness level.
     public static func apply(
         command: [String],
         strictness: Strictness = .default,
@@ -79,9 +79,8 @@ fileprivate func macOSSandboxProfile(
     }
 
     // Allow writing only to certain directories.
-    var writableDirectoriesExpression = writableDirectories.map {
-        "(subpath \(resolveSymlinks($0).quotedAsSubpathForSandboxProfile))"
-    }
+    var writableDirectoriesExpression: [String] = []
+
     // The following accesses are only needed when interpreting the manifest (versus running a compiled version).
     if strictness == .manifest_pre_53 {
         writableDirectoriesExpression += Platform.threadSafeDarwinCacheDirectories.get().map {
@@ -114,6 +113,15 @@ fileprivate func macOSSandboxProfile(
         contents += ")\n"
     }
 
+    // Emit rules for paths under which writing is allowed, even if they are descendants directories that are otherwise read-only.
+    if writableDirectories.count > 0 {
+        contents += "(allow file-write*\n"
+        for path in writableDirectories {
+            contents += "    (subpath \(resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
+        }
+        contents += ")\n"
+    }
+
     return contents
 }
 

Tests/BasicsTests/SandboxTests.swift

@@ -133,19 +133,20 @@ final class SandboxTest: XCTestCase {
 
     func testWritingToReadOnlyInsideWritableNotAllowed() throws {
         #if !os(macOS)
-        try XCTSkipIf(true, "test is only supported on macOS")
+        try XCTSkipIf(true, "sandboxing is only supported on macOS")
         #endif
 
         try withTemporaryDirectory { tmpDir in
-            // Check that we can write into it, but not to a read-only directory underneath it.
+            // Check that we can write into the temporary directory, but not into a read-only directory underneath it.
             let writableDir = tmpDir.appending(component: "ShouldBeWritable")
-            let allowedCommand = Sandbox.apply(command: ["mkdir", writableDir.pathString], strictness: .default, writableDirectories: [tmpDir])
+            try localFileSystem.createDirectory(writableDir)
+            let allowedCommand = Sandbox.apply(command: ["touch", writableDir.pathString], strictness: .default, writableDirectories: [writableDir])
             XCTAssertNoThrow(try Process.checkNonZeroExit(arguments: allowedCommand))
 
-            // Check that we cannot write into a read-only directory inside it.
+            // Check that we cannot write into a read-only directory inside a writable temporary directory.
             let readOnlyDir = writableDir.appending(component: "ShouldBeReadOnly")
             try localFileSystem.createDirectory(readOnlyDir)
-            let deniedCommand = Sandbox.apply(command: ["touch", readOnlyDir.pathString], strictness: .default, writableDirectories: [tmpDir], readOnlyDirectories: [readOnlyDir])
+            let deniedCommand = Sandbox.apply(command: ["touch", readOnlyDir.pathString], strictness: .writableTemporaryDirectory, readOnlyDirectories: [readOnlyDir])
             XCTAssertThrowsError(try Process.checkNonZeroExit(arguments: deniedCommand)) { error in
                 guard case ProcessResult.Error.nonZeroExit(let result) = error else {
                     return XCTFail("invalid error \(error)")
@@ -154,4 +155,29 @@ final class SandboxTest: XCTestCase {
             }
         }
     }
+
+    func testWritingToWritableInsideReadOnlyAllowed() throws {
+         #if !os(macOS)
+         try XCTSkipIf(true, "sandboxing is only supported on macOS")
+         #endif
+
+         try withTemporaryDirectory { tmpDir in
+             // Check that we cannot write into a read-only directory, but into a writable directory underneath it.
+             let readOnlyDir = tmpDir.appending(component: "ShouldBeReadOnly")
+             try localFileSystem.createDirectory(readOnlyDir)
+             let deniedCommand = Sandbox.apply(command: ["touch", readOnlyDir.pathString], strictness: .writableTemporaryDirectory, readOnlyDirectories: [readOnlyDir])
+             XCTAssertThrowsError(try Process.checkNonZeroExit(arguments: deniedCommand)) { error in
+                 guard case ProcessResult.Error.nonZeroExit(let result) = error else {
+                     return XCTFail("invalid error \(error)")
+                 }
+                 XCTAssertMatch(try! result.utf8stderrOutput(), .contains("Operation not permitted"))
+             }
+
+             // Check that we can write into a writable directory underneath it.
+             let writableDir = readOnlyDir.appending(component: "ShouldBeWritable")
+             try localFileSystem.createDirectory(writableDir)
+             let allowedCommand = Sandbox.apply(command: ["touch", writableDir.pathString], strictness: .default, writableDirectories:[writableDir], readOnlyDirectories: [readOnlyDir])
+             XCTAssertNoThrow(try Process.checkNonZeroExit(arguments: allowedCommand))
+         }
+     }
 }