Sandbox for manifests and plugins should block /tmp and instead provide a unique path specified by TMPDIR

The `/tmp` temporary directory is particularly unsafe because it is noisy — various processes with various ownerships write and read files there.  So it's better to use the per-user directory returned by `NSTemporaryDirectory()`.

We can improve several things here by passing through the result of calling `NSTemporaryDirectory()` in the sandbox instead of `/tmp`, and also `TMPDIR` in the environment (regardless of whether or not it was set in the inherited one).  This will cause the inferior's use of Foundation to respect this directory (the implementation of `NSTemporaryDirectory()` looks at `TMPDIR` on all platforms), and that should also be true for any command line tools it calls, as long as it passes through the environment.

Note:  if `TMPDIR` happens to be set in SwiftPM's own environment, it will control SwiftPM's own uses of `NSTemporaryDirectory()`, so the callers `TMPDIR` will be respected all the way through (down to manifests and plugins).

rdar://91575256

Author: abertelrud

Sources/Build/BuildOperation.swift

@@ -557,12 +557,20 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                     // Allow access to the plugin's output directory as well as to the local temporary directory.
                     let sandboxProfile = SandboxProfile([
                         .writable(pluginResult.pluginOutputDirectory),
-                        .writable(try AbsolutePath(validating: "/tmp")),
                         .writable(try self.fileSystem.tempDirectory)])
                     commandLine = try sandboxProfile.apply(to: commandLine)
                 }
-                let processResult = try TSCBasic.Process.popen(arguments: commandLine, environment: command.configuration.environment)
+
+                // Pass `TMPDIR` in the environment, in addition to anything the plugin specifies, in case we have an
+                // override in our own environment.
+                var environment = command.configuration.environment
+                environment["TMPDIR"] = try self.fileSystem.tempDirectory.pathString
+
+                // Run the command and wait for it to finish.
+                let processResult = try Process.popen(arguments: commandLine, environment: environment)
                 let output = try processResult.utf8Output() + processResult.utf8stderrOutput()
+
+                // Throw an error if it failed.
                 if processResult.exitStatus != .terminated(code: 0) {
                     throw StringError("failed: \(command)\n\n\(output)")
                 }

Sources/Build/LLBuildManifestBuilder.swift

@@ -626,17 +626,22 @@ extension LLBuildManifestBuilder {
                 if !self.disableSandboxForPluginCommands {
                     let sandboxProfile = SandboxProfile([
                         .writable(result.pluginOutputDirectory),
-                        .writable(try AbsolutePath(validating: "/tmp")),
                         .writable(try self.fileSystem.tempDirectory)])
                     commandLine = try sandboxProfile.apply(to: commandLine)
                 }
+
+                // Pass `TMPDIR` in the environment, in addition to anything the plugin specifies, in case we have an
+                // override in our own environment.
+                var environment = command.configuration.environment
+                environment["TMPDIR"] = try self.fileSystem.tempDirectory.pathString
+
                 manifest.addShellCmd(
                     name: displayName + "-" + ByteString(encodingAsUTF8: uniquedName).sha256Checksum,
                     description: displayName,
                     inputs: command.inputFiles.map{ .file($0) },
                     outputs: command.outputFiles.map{ .file($0) },
                     arguments: commandLine,
-                    environment: command.configuration.environment,
+                    environment: environment,
                     workingDirectory: command.configuration.workingDirectory?.pathString)
             }
         }

Sources/PackageLoading/ManifestLoader.swift

@@ -638,8 +638,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                             do {
                                 var sandbox = SandboxProfile()
 
-                                // Allow writing inside the temporary directories.
-                                sandbox.pathAccessRules.append(.writable(try AbsolutePath(validating: "/tmp")))
+                                // Allow writing inside the temporary directory.
                                 sandbox.pathAccessRules.append(.writable(try localFileSystem.tempDirectory))
 
                                 // Allow writing in the database cache directory, if we have one.
@@ -663,13 +662,22 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                             }
                         }
 
-                        // Run the compiled manifest.
+                        // Set up the environment so that the manifest inherits our own environment, but make sure that
+                        // `TMPDIR` is set to whatever the temporary directory is that is allowed in the sandbox.
                         var environment = ProcessEnv.vars
+                        do {
+                            environment["TMPDIR"] = try localFileSystem.tempDirectory.pathString
+                        } catch {
+                            return completion(.failure(error))
+                        }
+
+                        // On Windows, also make sure that the `Path` is set up correctly in the environment.
                         #if os(Windows)
                         let windowsPathComponent = runtimePath.pathString.replacingOccurrences(of: "/", with: "\\")
                         environment["Path"] = "\(windowsPathComponent);\(environment["Path"] ?? "")"
                         #endif
 
+                        // Run the compiled manifest.
                         let cleanupAfterRunning = cleanupIfError.delay()
                         TSCBasic.Process.popen(arguments: cmd, environment: environment, queue: callbackQueue) { result in
                             dispatchPrecondition(condition: .onQueue(callbackQueue))

Sources/Workspace/DefaultPluginScriptRunner.swift

@@ -425,8 +425,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
             do {
                 var sandbox = SandboxProfile()
 
-                // Allow writing inside the temporary directories.
-                sandbox.pathAccessRules.append(.writable(try AbsolutePath(validating: "/tmp")))
+                // Allow writing inside the temporary directory.
                 sandbox.pathAccessRules.append(.writable(try self.fileSystem.tempDirectory))
 
                 // But prevent writing in any read-only directories.
@@ -464,6 +463,16 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
 #endif
         process.currentDirectoryURL = workingDirectory.asURL
 
+        // Pass `TMPDIR` in the environment, in addition to anything the plugin specifies. This lets us respect our own
+        // override, and also makes sure that the plugin always knows what temporary directory we opened up for it.
+        var environment = ProcessInfo.processInfo.environment
+        do {
+            environment["TMPDIR"] = try localFileSystem.tempDirectory.pathString
+        } catch {
+            return completion(.failure(error))
+        }
+        process.environment = environment
+
         // Set up a pipe for sending structured messages to the plugin on its stdin.
         let stdinPipe = Pipe()
         let outputHandle = stdinPipe.fileHandleForWriting

Tests/CommandsTests/PackageToolTests.swift

@@ -1621,6 +1621,22 @@ final class PackageToolTests: CommandsTestCase {
                         let targetNames = argExtractor.extractOption(named: "target")
                         let targets = try context.package.targets(named: targetNames)
 
+                        // Check that we are able to write to the package plugin directory.
+                        print("Trying to write to the plugin output directory...")
+                        let outputPath = context.pluginWorkDirectory.appending("Foo")
+                        guard FileManager.default.createFile(atPath: outputPath.string, contents: Data("Hello".utf8)) else {
+                            throw "Couldn’t create file at path \\(outputPath)"
+                        }
+                        print("... successfully created file at path \\(outputPath)")
+
+                        // Check that we cannot write to `/tmp/`.
+                        print("Trying to write inside `/tmp/`...")
+                        let outputPath2 = Path("/tmp/_this_file_should_not_be_possible_to_create_")
+                        if FileManager.default.createFile(atPath: outputPath2.string, contents: Data("Hello".utf8)) {
+                            throw "Unexpectedly could create file at path \\(outputPath2)"
+                        }
+                        print("... correctly couldn't create file at path \\(outputPath2)")
+
                         // Print out the source files so that we can check them.
                         if let sourceFiles = (targets.first{ $0.name == "MyLibrary" } as? SourceModuleTarget)?.sourceFiles {
                             for file in sourceFiles {
@@ -1629,6 +1645,7 @@ final class PackageToolTests: CommandsTestCase {
                         }
                     }
                 }
+                extension String: Error {}
                 """
             }
 
@@ -1677,6 +1694,8 @@ final class PackageToolTests: CommandsTestCase {
                 let output = try result.utf8Output() + result.utf8stderrOutput()
                 XCTAssertEqual(result.exitStatus, .terminated(code: 0), "output: \(output)")
                 XCTAssertMatch(output, .contains("This is MyCommandPlugin."))
+                XCTAssertMatch(output, .contains("successfully created file at path \(try resolveSymlinks(packageDir).appending(components: ".build", "plugins", "MyPlugin", "outputs", "Foo"))"))
+                XCTAssertMatch(output, .contains("correctly couldn't create file at path /tmp/_this_file_should_not_be_possible_to_create_"))
             }
 
             // Check that we can also invoke it without the "plugin" subcommand.