Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve sandbox issues on external volumes #6773

Merged
merged 1 commit into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Sources/Basics/FileSystem/FileSystem+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,12 @@ extension FileSystem {
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T {
try self.withLock(on: path.underlying, type: type, body)
}

/// Returns any known item replacement directories for a given path. These may be used by platform-specific
/// libraries to handle atomic file system operations, such as deletion.
func itemReplacementDirectories(for path: AbsolutePath) throws -> [AbsolutePath] {
return try self.itemReplacementDirectories(for: path.underlying).compactMap { AbsolutePath($0) }
}
}

// MARK: - user level
Expand Down
7 changes: 6 additions & 1 deletion Sources/Basics/Sandbox.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,21 @@ public enum Sandbox {
///
/// - Parameters:
/// - command: The command line to sandbox (including executable as first argument)
/// - fileSystem: The file system instance to use.
/// - 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],
fileSystem: FileSystem,
strictness: Strictness = .default,
writableDirectories: [AbsolutePath] = [],
readOnlyDirectories: [AbsolutePath] = [],
allowNetworkConnections: [SandboxNetworkPermission] = []
) throws -> [String] {
#if os(macOS)
let profile = try macOSSandboxProfile(
fileSystem: fileSystem,
strictness: strictness,
writableDirectories: writableDirectories,
readOnlyDirectories: readOnlyDirectories,
Expand Down Expand Up @@ -106,6 +109,7 @@ fileprivate let threadSafeDarwinCacheDirectories: [AbsolutePath] = {
}()

fileprivate func macOSSandboxProfile(
fileSystem: FileSystem,
strictness: Sandbox.Strictness,
writableDirectories: [AbsolutePath],
readOnlyDirectories: [AbsolutePath],
Expand Down Expand Up @@ -213,7 +217,8 @@ fileprivate func macOSSandboxProfile(
// 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 {
// For any explicit writable directories, also include the relevant item replacement directories so that Foundation APIs using atomic writes are not blocked by the sandbox.
for path in writableDirectories + Set(writableDirectories.compactMap { try? fileSystem.itemReplacementDirectories(for: $0) }.flatMap { $0}) {
contents += " (subpath \(try resolveSymlinks(path).quotedAsSubpathForSandboxProfile))\n"
}
contents += ")\n"
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ 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 !pluginConfiguration.disableSandbox {
commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory])
commandLine = try Sandbox.apply(command: commandLine, fileSystem: self.fileSystem, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory])
}
let processResult = try Process.popen(arguments: commandLine, environment: command.configuration.environment)
let output = try processResult.utf8Output() + processResult.utf8stderrOutput()
Expand Down
1 change: 1 addition & 0 deletions Sources/Build/LLBuildManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ extension LLBuildManifestBuilder {
if !self.disableSandboxForPluginCommands {
commandLine = try Sandbox.apply(
command: commandLine,
fileSystem: self.fileSystem,
strictness: .writableTemporaryDirectory,
writableDirectories: [result.pluginOutputDirectory]
)
Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageLoading/ManifestLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
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)
cmd = try Sandbox.apply(command: cmd, fileSystem: localFileSystem, strictness: strictness, writableDirectories: cacheDirectories)
} catch {
return completion(.failure(error))
}
Expand Down
1 change: 1 addition & 0 deletions Sources/Workspace/DefaultPluginScriptRunner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
do {
command = try Sandbox.apply(
command: command,
fileSystem: self.fileSystem,
strictness: .writableTemporaryDirectory,
writableDirectories: writableDirectories + [self.cacheDir],
readOnlyDirectories: readOnlyDirectories,
Expand Down
20 changes: 20 additions & 0 deletions Tests/BasicsTests/SandboxTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import XCTest
import Darwin
#endif

import class TSCBasic.InMemoryFileSystem
import class TSCBasic.Process
import struct TSCBasic.ProcessResult

Expand Down Expand Up @@ -221,3 +222,22 @@ final class SandboxTest: XCTestCase {
}
}
}

extension Sandbox {
public static func apply(
command: [String],
strictness: Strictness = .default,
writableDirectories: [AbsolutePath] = [],
readOnlyDirectories: [AbsolutePath] = [],
allowNetworkConnections: [SandboxNetworkPermission] = []
) throws -> [String] {
return try self.apply(
command: command,
fileSystem: InMemoryFileSystem(),
strictness: strictness,
writableDirectories: writableDirectories,
readOnlyDirectories: readOnlyDirectories,
allowNetworkConnections: allowNetworkConnections
)
}
}