Skip to content

Reinstate manifest sandboxing #2852

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

Merged
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
13 changes: 7 additions & 6 deletions Sources/PackageLoading/ManifestLoader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
moduleCachePath.map({ AbsolutePath($0) })
].compactMap({ $0 })
let profile = sandboxProfile(toolsVersion: toolsVersion, cacheDirectories: cacheDirectories)
cmd += ["sandbox-exec", "-p", profile]
cmd = ["sandbox-exec", "-p", profile] + cmd
}
#endif

Expand Down Expand Up @@ -739,13 +739,14 @@ private func sandboxProfile(toolsVersion: ToolsVersion, cacheDirectories: [Absol
stream <<< "(deny default)" <<< "\n"
// Import the system sandbox profile.
stream <<< "(import \"system.sb\")" <<< "\n"
// Allow reading all files. Even in 5.3 we need to be able to read the PD dylibs.
stream <<< "(allow file-read*)" <<< "\n"
// This is needed to launch any processes.
stream <<< "(allow process*)" <<< "\n"

// The following accesses are only needed when interpreting the manifest (versus running a compiled version).
if toolsVersion < .v5_3 {
// Allow reading all files.
stream <<< "(allow file-read*)" <<< "\n"
// These are required by the Swift compiler.
stream <<< "(allow process*)" <<< "\n"
// This is required by the Swift compiler.
stream <<< "(allow sysctl*)" <<< "\n"
// Allow writing in temporary locations.
stream <<< "(allow file-write*" <<< "\n"
Expand All @@ -755,9 +756,9 @@ private func sandboxProfile(toolsVersion: ToolsVersion, cacheDirectories: [Absol
for directory in cacheDirectories {
stream <<< " (subpath \"\(directory.pathString)\")" <<< "\n"
}
stream <<< ")" <<< "\n"
}

stream <<< ")" <<< "\n"
return stream.bytes.description
}

Expand Down
24 changes: 24 additions & 0 deletions Tests/PackageLoadingTests/PD5_2LoadingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,28 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
}
}
}

func testManifestLoadingIsSandboxed() throws {
#if os(macOS) // Sandboxing is only done on macOS today.
let stream = BufferedOutputByteStream()
stream <<< """
import Foundation

try! String(contentsOf:URL(string: "http://127.0.0.1")!)

import PackageDescription
let package = Package(
name: "Foo",
targets: [
.target(name: "Foo"),
]
)
"""

XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
}
#endif
}
}
24 changes: 24 additions & 0 deletions Tests/PackageLoadingTests/PD5_3LoadingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -406,4 +406,28 @@ class PackageDescriptionNextLoadingTests: PackageDescriptionLoadingTests {
XCTAssertTrue(error is ManifestParseError, "unexpected error: \(error)")
}
}

func testManifestLoadingIsSandboxed() throws {
#if os(macOS) // Sandboxing is only done on macOS today.
let stream = BufferedOutputByteStream()
stream <<< """
import Foundation

try! "should not be allowed".write(to: URL(fileURLWithPath: "/tmp/file.txt"), atomically: true, encoding: String.Encoding.utf8)

import PackageDescription
let package = Package(
name: "Foo",
targets: [
.target(name: "Foo"),
]
)
"""

XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
}
#endif
}
}