Skip to content

Commit

Permalink
Resolve sandbox issues on external volumes
Browse files Browse the repository at this point in the history
The sandbox profile SwiftPM generates curates the directories plugins and manifests are able to write to. Several Foundation APIs use a so-called "item replacement directory" when doing atomic operations and this happens to be a special path on the same volume when performing operations on an external volume. This lead to those operations being blocked in plugins which does not seem right since those item replacement directories are temporary directories in spirit. We now automatically allow writes for those when generating a sandbox profile.

rdar://112217177
  • Loading branch information
neonichu committed Aug 8, 2023
1 parent 69dc9cb commit ebf511b
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 3 deletions.
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
)
}
}

0 comments on commit ebf511b

Please sign in to comment.