From b5d3eb591346345bbb9ff579a8f46231a0baa449 Mon Sep 17 00:00:00 2001 From: Anders Bertelrud Date: Tue, 11 Jan 2022 19:14:17 -0800 Subject: [PATCH] Improve flexibility of sandbox path rules, and make sandbox profile a struct that's easier to manage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turn the stateless `Sandbox` enum into a `SandboxProfile` struct that's a bit more flexible: - there is a list of path rules allowing a mixed order of `writable`, `readonly`, and `noaccess` rules; this addresses an issue where writable directories were always applied after readable directories, preventing arrangements where, for example, a source directory inside the temporary directory was properly made readonly - the defaults are built into the initializer rather than a separate `strictness` parameter — this means that they can be queried once the SandboxProfile has been created and are expressed in terms of the choices available to all sandbox profiles - it's a bit more ergonomic (as a struct, it can be copied, mutated, and carried around in a property before being used), and an optional `SandboxProfile` can be used to indicate both whether or not to apply a profile and if so what to apply. Having the sandbox profile be a struct that generates the platform specifics as needed also could also provide a place with which to associate any cached/compiled representation for profiles that are largely static, for any platform that supports that. As cleanup, this commit also removes the pre-SwiftPM 5.3 specialities which were specific to running the package manifest in an interpreter rather than compiling and executing it. This functionality is no longer relevant since it isn't possible to run any manifest in the interpreter, no matter what tools version the package specifies. In this commit, the call sites have been adjusted so that they use the modified SandboxProfile API, but they still construct the profiles on-the-fly as before. A future change will make them instead be configured at an early point and then applied later when the sandboxed process is actually launched. Note also that the existing call sites still pass `/tmp` because that it was the sandbox has allowed up until now. A separate PR will deal with whether or not allowing writes to `/tmp` is apprporiate. Another future change could add the sandbox profile to `Process` as a property so that there is no API assumption that applying a sandbox necessarily involves just modifying the command line. --- CHANGELOG.md | 4 + Sources/Basics/CMakeLists.txt | 2 +- Sources/Basics/Sandbox.swift | 144 ------------------ Sources/Basics/SandboxProfile.swift | 95 ++++++++++++ Sources/Build/BuildOperation.swift | 7 +- Sources/Build/LLBuildManifestBuilder.swift | 8 +- Sources/PackageLoading/ManifestLoader.swift | 25 ++- .../Workspace/DefaultPluginScriptRunner.swift | 18 ++- Tests/BasicsTests/SandboxTests.swift | 79 ++++++---- Tests/CommandsTests/PackageToolTests.swift | 30 +++- 10 files changed, 221 insertions(+), 191 deletions(-) delete mode 100644 Sources/Basics/Sandbox.swift create mode 100644 Sources/Basics/SandboxProfile.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index a0da7767cd8..ceb7aaa8bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ Swift 5.8 In packages that specify resources using tools version 5.8 or later, the generated resource bundle accessor will import `Foundation.Bundle` for its own implementation only. _Clients_ of such packages therefore no longer silently import `Foundation`, preventing inadvertent use of Foundation extensions to standard library APIs, which helps to avoid unexpected code size increases. +* [#5857] + + When running a compiler package manifest, the sandbox on Darwin platforms no longer allows writing to the compiler's module cache directories. These directories were originally writable when running Swift package manifests in the interpreter, but the Swift interpreter has not been used for package manifests (regardless of tools version) for several releases now, so this should have no noticeable impact on existing packages. + Swift 5.7 ----------- diff --git a/Sources/Basics/CMakeLists.txt b/Sources/Basics/CMakeLists.txt index 89924d1c5c9..dc6cf867e59 100644 --- a/Sources/Basics/CMakeLists.txt +++ b/Sources/Basics/CMakeLists.txt @@ -27,7 +27,7 @@ add_library(Basics NSLock+Extensions.swift Observability.swift SQLite.swift - Sandbox.swift + SandboxProfile.swift String+Extensions.swift Triple+Extensions.swift SwiftVersion.swift diff --git a/Sources/Basics/Sandbox.swift b/Sources/Basics/Sandbox.swift deleted file mode 100644 index af083fa7855..00000000000 --- a/Sources/Basics/Sandbox.swift +++ /dev/null @@ -1,144 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift open source project -// -// Copyright (c) 2021-2022 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See http://swift.org/LICENSE.txt for license information -// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// - -import Foundation -import TSCBasic - -import enum TSCUtility.Platform - -public enum Sandbox { - - /// Applies a sandbox invocation to the given command line (if the platform supports it), - /// and returns the modified command line. On platforms that don't support sandboxing, the - /// command line is returned unmodified. - /// - /// - 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, 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, - writableDirectories: [AbsolutePath] = [], - readOnlyDirectories: [AbsolutePath] = [] - ) throws -> [String] { - #if os(macOS) - let profile = try macOSSandboxProfile(strictness: strictness, writableDirectories: writableDirectories, readOnlyDirectories: readOnlyDirectories) - return ["/usr/bin/sandbox-exec", "-p", profile] + command - #else - // rdar://40235432, rdar://75636874 tracks implementing sandboxes for other platforms. - return command - #endif - } - - /// Basic strictness level of a sandbox applied to a command line. - public enum Strictness: Equatable { - /// Blocks network access and all file system modifications. - case `default` - /// More lenient restrictions than the default, for compatibility with SwiftPM manifests using a tools version older than 5.3. - case manifest_pre_53 // backwards compatibility for manifests - /// Like `default`, but also makes temporary-files directories (such as `/tmp`) on the platform writable. - case writableTemporaryDirectory - } -} - -// MARK: - macOS - -#if os(macOS) -fileprivate func macOSSandboxProfile( - strictness: Sandbox.Strictness, - writableDirectories: [AbsolutePath], - readOnlyDirectories: [AbsolutePath] -) throws -> String { - var contents = "(version 1)\n" - - // Deny everything by default. - contents += "(deny default)\n" - - // Import the system sandbox profile. - contents += "(import \"system.sb\")\n" - - // Allow reading all files; ideally we'd only allow the package directory and any dependencies, - // but all kinds of system locations need to be accessible. - contents += "(allow file-read*)\n" - - // This is needed to launch any processes. - contents += "(allow process*)\n" - - // The following accesses are only needed when interpreting the manifest (versus running a compiled version). - if strictness == .manifest_pre_53 { - // This is required by the Swift compiler. - contents += "(allow sysctl*)\n" - } - - // Allow writing only to certain directories. - 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 { - ##"(regex #"^\##($0.pathString)/org\.llvm\.clang.*")"## - } - } - // Optionally allow writing to temporary directories (a lot of use of Foundation requires this). - else if strictness == .writableTemporaryDirectory { - // Add `subpath` expressions for the regular and the Foundation temporary directories. - for tmpDir in ["/tmp", NSTemporaryDirectory()] { - writableDirectoriesExpression += try ["(subpath \(resolveSymlinks(AbsolutePath(validating: tmpDir)).quotedAsSubpathForSandboxProfile))"] - } - } - - // Emit rules for paths under which writing is allowed. Some of these expressions may be regular expressions and others literal subpaths. - if writableDirectoriesExpression.count > 0 { - contents += "(allow file-write*\n" - for expression in writableDirectoriesExpression { - contents += " \(expression)\n" - } - contents += ")\n" - } - - // Emit rules for paths under which writing should be disallowed, even if they would be covered by a previous rule to allow writing to them. A classic case is a package which is located under the temporary directory, which should be read-only even though the temporary directory as a whole is writable. - if readOnlyDirectories.count > 0 { - contents += "(deny file-write*\n" - for path in readOnlyDirectories { - contents += " (subpath \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n" - } - 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 \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n" - } - contents += ")\n" - } - - return contents -} - -fileprivate extension AbsolutePath { - /// Private computed property that returns a version of the path as a string quoted for use as a subpath in a .sb sandbox profile. - var quotedAsSubpathForSandboxProfile: String { - return "\"" + self.pathString - .replacingOccurrences(of: "\\", with: "\\\\") - .replacingOccurrences(of: "\"", with: "\\\"") - + "\"" - } -} - -extension TSCUtility.Platform { - fileprivate static let threadSafeDarwinCacheDirectories = ThreadSafeArrayStore((try? Self.darwinCacheDirectories()) ?? []) -} -#endif diff --git a/Sources/Basics/SandboxProfile.swift b/Sources/Basics/SandboxProfile.swift new file mode 100644 index 00000000000..88c537f0fb6 --- /dev/null +++ b/Sources/Basics/SandboxProfile.swift @@ -0,0 +1,95 @@ +/* + This source file is part of the Swift.org open source project + + Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors + Licensed under Apache License v2.0 with Runtime Library Exception + + See http://swift.org/LICENSE.txt for license information + See http://swift.org/CONTRIBUTORS.txt for Swift project authors + */ + +import Foundation +import TSCBasic + +/// A sandbox profile representing the desired restrictions. The implementation can vary between platforms. Currently +/// the only control a client has is in the path rules, but in the future there should also be options for controlling +/// blocking of network access and process launching. +public struct SandboxProfile: Equatable { + /// An ordered list of path rules, where the last rule to cover a particular path "wins". These will be resolved + /// to absolute paths at the time the profile is applied. They are applied after any of the implicit directories + /// referenced by other sandbox profile settings. + public var pathAccessRules: [PathAccessRule] + + /// Represents a rule for access to a path and everything under it. + public enum PathAccessRule: Equatable { + case noaccess(AbsolutePath) + case readonly(AbsolutePath) + case writable(AbsolutePath) + } + + /// Configures a SandboxProfile for blocking network access and writing to the file system except to specifically + /// permitted locations. + public init(_ pathAccessRules: [PathAccessRule] = []) { + self.pathAccessRules = pathAccessRules + } +} + +extension SandboxProfile { + /// Applies the sandbox profile to the given command line (if the platform supports it), and returns the modified + /// command line. On platforms that don't support sandboxing, the unmodified command line is returned. + public func apply(to command: [String]) throws -> [String] { + #if os(macOS) + return ["/usr/bin/sandbox-exec", "-p", try self.generateMacOSSandboxProfileString()] + command + #else + // rdar://40235432, rdar://75636874 tracks implementing sandboxes for other platforms. + return command + #endif + } +} + +// MARK: - macOS + +#if os(macOS) +fileprivate extension SandboxProfile { + /// Private function that generates a Darwin sandbox profile suitable for passing to `sandbox-exec(1)`. + func generateMacOSSandboxProfileString() throws -> String { + var contents = "(version 1)\n" + + // Deny everything by default. + contents += "(deny default)\n" + + // Import the system sandbox profile. + contents += "(import \"system.sb\")\n" + + // Allow operations on subprocesses. + contents += "(allow process*)\n" + + // Allow reading any file that isn't protected by TCC or permissions (ideally we'd only allow a specific set + // of readable locations, and can maybe tighten this in the future). + contents += "(allow file-read*)\n" + + // Apply customized rules for specific file system locations. Everything is readonly by default, so we just + // either allow or deny writing, in order. Later rules have precedence over earlier rules. + for rule in pathAccessRules { + switch rule { + case .noaccess(let path): + contents += "(deny file-* (subpath \(try resolveSymlinksAndQuotePath(path))))\n" + case .readonly(let path): + contents += "(deny file-write* (subpath \(try resolveSymlinksAndQuotePath(path))))\n" + case .writable(let path): + contents += "(allow file-write* (subpath \(try resolveSymlinksAndQuotePath(path))))\n" + } + } + return contents + } + + /// Private helper function that resolves an AbsolutePath and returns it as a string quoted for use as a subpath + /// in a .sb sandbox profile. + func resolveSymlinksAndQuotePath(_ path: AbsolutePath) throws -> String { + return try "\"" + resolveSymlinks(path).pathString + .replacingOccurrences(of: "\\", with: "\\\\") + .replacingOccurrences(of: "\"", with: "\\\"") + + "\"" + } +} +#endif diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift index c266fa81eb0..8114a0c27bb 100644 --- a/Sources/Build/BuildOperation.swift +++ b/Sources/Build/BuildOperation.swift @@ -554,7 +554,12 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS // TODO: We need to also use any working directory, but that support isn't yet available on all platforms at a lower level. var commandLine = [command.configuration.executable.pathString] + command.configuration.arguments if !self.disableSandboxForPluginCommands { - commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory]) + // 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) let output = try processResult.utf8Output() + processResult.utf8stderrOutput() diff --git a/Sources/Build/LLBuildManifestBuilder.swift b/Sources/Build/LLBuildManifestBuilder.swift index 83530e24bae..e62de4f631c 100644 --- a/Sources/Build/LLBuildManifestBuilder.swift +++ b/Sources/Build/LLBuildManifestBuilder.swift @@ -621,8 +621,14 @@ extension LLBuildManifestBuilder { let uniquedName = ([execPath.pathString] + command.configuration.arguments).joined(separator: "|") let displayName = command.configuration.displayName ?? execPath.basename var commandLine = [execPath.pathString] + command.configuration.arguments + + // Apply the sandbox, if appropriate. if !self.disableSandboxForPluginCommands { - commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [result.pluginOutputDirectory]) + let sandboxProfile = SandboxProfile([ + .writable(result.pluginOutputDirectory), + .writable(try AbsolutePath(validating: "/tmp")), + .writable(try self.fileSystem.tempDirectory)]) + commandLine = try sandboxProfile.apply(to: commandLine) } manifest.addShellCmd( name: displayName + "-" + ByteString(encodingAsUTF8: uniquedName).sha256Checksum, diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift index 1120c9878d8..59c4fff827d 100644 --- a/Sources/PackageLoading/ManifestLoader.swift +++ b/Sources/PackageLoading/ManifestLoader.swift @@ -708,10 +708,29 @@ public final class ManifestLoader: ManifestLoaderProtocol { // This provides some safety against arbitrary code execution when parsing manifest files. // We only allow the permissions which are absolutely necessary. if self.isManifestSandboxEnabled { - let cacheDirectories = [self.databaseCacheDir, moduleCachePath].compactMap{ $0 } - let strictness: Sandbox.Strictness = toolsVersion < .v5_3 ? .manifest_pre_53 : .default do { - cmd = try Sandbox.apply(command: cmd, strictness: strictness, writableDirectories: cacheDirectories) + var sandbox = SandboxProfile() + + // Allow writing inside the temporary directories. + sandbox.pathAccessRules.append(.writable(try AbsolutePath(validating: "/tmp"))) + sandbox.pathAccessRules.append(.writable(try localFileSystem.tempDirectory)) + + // Allow writing in the database cache directory, if we have one. + if let databaseCacheDir = self.databaseCacheDir { + sandbox.pathAccessRules.append(.writable(databaseCacheDir)) + } + + // Allow writing in the module cache path, if there is one. + if let moduleCachePath = moduleCachePath { + sandbox.pathAccessRules.append(.writable(moduleCachePath)) + } + + // But do not allow writing in the directory that contains the manifest, even if it is + // inside one of the otherwise writable directories. + sandbox.pathAccessRules.append(.readonly(manifestPath.parentDirectory)) + + // Finally apply the sandbox. + cmd = try sandbox.apply(to: cmd) } catch { return completion(.failure(error)) } diff --git a/Sources/Workspace/DefaultPluginScriptRunner.swift b/Sources/Workspace/DefaultPluginScriptRunner.swift index dcdfe6c0aca..977bb29a48f 100644 --- a/Sources/Workspace/DefaultPluginScriptRunner.swift +++ b/Sources/Workspace/DefaultPluginScriptRunner.swift @@ -423,7 +423,23 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable { // Optionally wrap the command in a sandbox, which places some limits on what it can do. In particular, it blocks network access and restricts the paths to which the plugin can make file system changes. It does allow writing to temporary directories. if self.enableSandbox { do { - command = try Sandbox.apply(command: command, strictness: .writableTemporaryDirectory, writableDirectories: writableDirectories + [self.cacheDir], readOnlyDirectories: readOnlyDirectories) + var sandbox = SandboxProfile() + + // Allow writing inside the temporary directories. + sandbox.pathAccessRules.append(.writable(try AbsolutePath(validating: "/tmp"))) + sandbox.pathAccessRules.append(.writable(try self.fileSystem.tempDirectory)) + + // But prevent writing in any read-only directories. + sandbox.pathAccessRules.append(contentsOf: readOnlyDirectories.map{ .readonly($0) }) + + // But allow writing in any writable directories. + sandbox.pathAccessRules.append(contentsOf: writableDirectories.map{ .writable($0) }) + + // And always allow writing to the cache directory, even if it is inside one of the readonly directories. + sandbox.pathAccessRules.append(.writable(self.cacheDir)) + + // Apply the sandbox to the command. + command = try sandbox.apply(to: command) } catch { return callbackQueue.async { completion(.failure(error)) diff --git a/Tests/BasicsTests/SandboxTests.swift b/Tests/BasicsTests/SandboxTests.swift index 25c3aee7f72..a060e01d085 100644 --- a/Tests/BasicsTests/SandboxTests.swift +++ b/Tests/BasicsTests/SandboxTests.swift @@ -16,12 +16,19 @@ import TSCBasic import XCTest final class SandboxTest: XCTestCase { + + func testDefaults() throws { + let sandboxProfile = SandboxProfile() + XCTAssertEqual(sandboxProfile.pathAccessRules, []) + } + func testSandboxOnAllPlatforms() throws { try withTemporaryDirectory { path in + let profile = SandboxProfile() #if os(Windows) - let command = try Sandbox.apply(command: ["tar.exe", "-h"], strictness: .default, writableDirectories: []) + let command = try profile.apply(to: ["tar.exe", "-h"]) #else - let command = try Sandbox.apply(command: ["echo", "0"], strictness: .default, writableDirectories: []) + let command = try profile.apply(to: ["echo", "0"]) #endif XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command)) } @@ -29,10 +36,12 @@ final class SandboxTest: XCTestCase { func testNetworkNotAllowed() throws { #if !os(macOS) - try XCTSkipIf(true, "test is only supported on macOS") + try XCTSkipIf(true, "sandboxing is only supported on macOS") #endif - let command = try Sandbox.apply(command: ["ping", "-t", "1", "localhost"], strictness: .default, writableDirectories: []) + /// Check that network access isn't allowed by default. + let profile = SandboxProfile() + let command = try profile.apply(to: ["ping", "-t", "1", "localhost"]) XCTAssertThrowsError(try TSCBasic.Process.checkNonZeroExit(arguments: command)) { error in guard case ProcessResult.Error.nonZeroExit(let result) = error else { @@ -44,22 +53,24 @@ final class SandboxTest: XCTestCase { func testWritableAllowed() 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 { path in - let command = try Sandbox.apply(command: ["touch", path.appending(component: UUID().uuidString).pathString], strictness: .default, writableDirectories: [path]) + let profile = SandboxProfile([.writable(path)]) + let command = try profile.apply(to: ["touch", path.appending(component: UUID().uuidString).pathString]) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command)) } } func testWritableNotAllowed() 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 { path in - let command = try Sandbox.apply(command: ["touch", path.appending(component: UUID().uuidString).pathString], strictness: .default, writableDirectories: []) + let profile = SandboxProfile() + let command = try profile.apply(to: ["touch", path.appending(component: UUID().uuidString).pathString]) XCTAssertThrowsError(try TSCBasic.Process.checkNonZeroExit(arguments: command)) { error in guard case ProcessResult.Error.nonZeroExit(let result) = error else { return XCTFail("invalid error \(error)") @@ -71,14 +82,15 @@ final class SandboxTest: XCTestCase { func testRemoveNotAllowed() 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 { path in let file = path.appending(component: UUID().uuidString) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["touch", file.pathString])) - let command = try Sandbox.apply(command: ["rm", file.pathString], strictness: .default, writableDirectories: []) + let profile = SandboxProfile() + let command = try profile.apply(to: ["rm", file.pathString]) XCTAssertThrowsError(try TSCBasic.Process.checkNonZeroExit(arguments: command)) { error in guard case ProcessResult.Error.nonZeroExit(let result) = error else { return XCTFail("invalid error \(error)") @@ -91,14 +103,15 @@ final class SandboxTest: XCTestCase { // FIXME: rdar://75707545 this should not be allowed outside very specific read locations func testReadAllowed() 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 { path in let file = path.appending(component: UUID().uuidString) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["touch", file.pathString])) - let command = try Sandbox.apply(command: ["cat", file.pathString], strictness: .default, writableDirectories: []) + let profile = SandboxProfile() + let command = try profile.apply(to: ["cat", file.pathString]) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command)) } } @@ -106,34 +119,32 @@ final class SandboxTest: XCTestCase { // FIXME: rdar://75707545 this should not be allowed outside very specific programs func testExecuteAllowed() 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 { path in - let file = path.appending(component: UUID().uuidString) - XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["touch", file.pathString])) - XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["chmod", "+x", file.pathString])) + let scriptFile = path.appending(component: UUID().uuidString) + XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["touch", scriptFile.pathString])) + XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: ["chmod", "+x", scriptFile.pathString])) - let command = try Sandbox.apply(command: [file.pathString], strictness: .default, writableDirectories: []) + let profile = SandboxProfile() + let command = try profile.apply(to: [scriptFile.pathString]) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command)) } } func testWritingToTemporaryDirectoryAllowed() throws { #if !os(macOS) - try XCTSkipIf(true, "test is only supported on macOS") + try XCTSkipIf(true, "sandboxing is only supported on macOS") #endif - // Try writing to the per-user temporary directory, which is under /var/folders/.../TemporaryItems. - let tmpFile1 = NSTemporaryDirectory() + "/" + UUID().uuidString - let command1 = try Sandbox.apply(command: ["touch", tmpFile1], strictness: .writableTemporaryDirectory) - XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command1)) - try? FileManager.default.removeItem(atPath: tmpFile1) - - let tmpFile2 = "/tmp" + "/" + UUID().uuidString - let command2 = try Sandbox.apply(command: ["touch", tmpFile2], strictness: .writableTemporaryDirectory) - XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: command2)) - try? FileManager.default.removeItem(atPath: tmpFile2) + // Try writing to the per-user temporary directory, which is under `/var/folders/.../TemporaryItems` unless it + // is overridden by the TMPDIR environment variable. + let tmpFile = try localFileSystem.tempDirectory.appending(component: UUID().uuidString) + let profile = SandboxProfile([.writable(try localFileSystem.tempDirectory)]) + let command = try profile.apply(to: ["touch", tmpFile.pathString]) + XCTAssertNoThrow(try Process.checkNonZeroExit(arguments: command)) + try? FileManager.default.removeItem(atPath: tmpFile.pathString) } func testWritingToReadOnlyInsideWritableNotAllowed() throws { @@ -145,13 +156,15 @@ final class SandboxTest: XCTestCase { // Check that we can write into the temporary directory, but not into a read-only directory underneath it. let writableDir = tmpDir.appending(component: "ShouldBeWritable") try localFileSystem.createDirectory(writableDir) - let allowedCommand = try Sandbox.apply(command: ["touch", writableDir.pathString], strictness: .default, writableDirectories: [writableDir]) + let allowanceProfile = SandboxProfile([.writable(writableDir)]) + let allowedCommand = try allowanceProfile.apply(to: ["touch", writableDir.pathString]) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: allowedCommand)) // 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 = try Sandbox.apply(command: ["touch", readOnlyDir.pathString], strictness: .writableTemporaryDirectory, readOnlyDirectories: [readOnlyDir]) + let denialProfile = SandboxProfile([.writable(tmpDir), .readonly(readOnlyDir)]) + let deniedCommand = try denialProfile.apply(to: ["touch", readOnlyDir.pathString]) XCTAssertThrowsError(try TSCBasic.Process.checkNonZeroExit(arguments: deniedCommand)) { error in guard case ProcessResult.Error.nonZeroExit(let result) = error else { return XCTFail("invalid error \(error)") @@ -170,7 +183,8 @@ final class SandboxTest: XCTestCase { // 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 = try Sandbox.apply(command: ["touch", readOnlyDir.pathString], strictness: .writableTemporaryDirectory, readOnlyDirectories: [readOnlyDir]) + let denialProfile = SandboxProfile([.writable(tmpDir), .readonly(readOnlyDir)]) + let deniedCommand = try denialProfile.apply(to: ["touch", readOnlyDir.pathString]) XCTAssertThrowsError(try TSCBasic.Process.checkNonZeroExit(arguments: deniedCommand)) { error in guard case ProcessResult.Error.nonZeroExit(let result) = error else { return XCTFail("invalid error \(error)") @@ -181,7 +195,8 @@ final class SandboxTest: XCTestCase { // Check that we can write into a writable directory underneath it. let writableDir = readOnlyDir.appending(component: "ShouldBeWritable") try localFileSystem.createDirectory(writableDir) - let allowedCommand = try Sandbox.apply(command: ["touch", writableDir.pathString], strictness: .default, writableDirectories:[writableDir], readOnlyDirectories: [readOnlyDir]) + let allowanceProfile = SandboxProfile([.readonly(readOnlyDir), .writable(writableDir)]) + let allowedCommand = try allowanceProfile.apply(to: ["touch", writableDir.pathString]) XCTAssertNoThrow(try TSCBasic.Process.checkNonZeroExit(arguments: allowedCommand)) } } diff --git a/Tests/CommandsTests/PackageToolTests.swift b/Tests/CommandsTests/PackageToolTests.swift index 679f3689557..ddef271140c 100644 --- a/Tests/CommandsTests/PackageToolTests.swift +++ b/Tests/CommandsTests/PackageToolTests.swift @@ -1609,6 +1609,15 @@ 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)") + // 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 { @@ -1617,6 +1626,7 @@ final class PackageToolTests: CommandsTestCase { } } } + extension String: Error {} """ } @@ -1665,6 +1675,7 @@ 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"))")) } // Check that we can also invoke it without the "plugin" subcommand. @@ -1761,8 +1772,9 @@ final class PackageToolTests: CommandsTestCase { ) throws { // Check that we can write to the package directory. print("Trying to write to the package directory...") - guard FileManager.default.createFile(atPath: context.package.directory.appending("Foo").string, contents: Data("Hello".utf8)) else { - throw "Couldn’t create file at path \\(context.package.directory.appending("Foo"))" + let outputPath = context.package.directory.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 it") } @@ -1786,18 +1798,20 @@ final class PackageToolTests: CommandsTestCase { // Check that we don't get an error (and also are allowed to write to the package directory) if we pass `--allow-writing-to-package-directory`. do { let result = try SwiftPMProduct.SwiftPackage.executeProcess(["plugin", "--allow-writing-to-package-directory", "PackageScribbler"], packagePath: packageDir, env: ["DECLARE_PACKAGE_WRITING_PERMISSION": "1"]) - XCTAssertEqual(result.exitStatus, .terminated(code: 0)) - XCTAssertMatch(try result.utf8Output(), .contains("successfully created it")) - XCTAssertNoMatch(try result.utf8stderrOutput(), .contains("error: Couldn’t create file at path")) + let output = try result.utf8Output() + result.utf8stderrOutput() + XCTAssertEqual(result.exitStatus, .terminated(code: 0), "output: \(output)") + XCTAssertMatch(output, .contains("successfully created it")) + XCTAssertNoMatch(output, .contains("error: Couldn’t create file at path")) } // Check that we get an error if the plugin doesn't declare permission but tries to write anyway. Note that sandboxing is only currently supported on macOS. #if os(macOS) do { let result = try SwiftPMProduct.SwiftPackage.executeProcess(["plugin", "PackageScribbler"], packagePath: packageDir, env: ["DECLARE_PACKAGE_WRITING_PERMISSION": "0"]) - XCTAssertNotEqual(result.exitStatus, .terminated(code: 0)) - XCTAssertNoMatch(try result.utf8Output(), .contains("successfully created it")) - XCTAssertMatch(try result.utf8stderrOutput(), .contains("error: Couldn’t create file at path")) + let output = try result.utf8Output() + result.utf8stderrOutput() + XCTAssertNotEqual(result.exitStatus, .terminated(code: 0), "output: \(output)") + XCTAssertNoMatch(output, .contains("successfully created it")) + XCTAssertMatch(output, .contains("error: Couldn’t create file at path")) } #endif }