Skip to content

Commit c131bec

Browse files
authored
Reinstate manifest sandboxing (#2852)
Manifest loading has been sandboxed on macOS for a while, but the change in #2518 broke it for 5.3. https://bugs.swift.org/browse/SR-13346 rdar://problem/66586184 (cherry picked from commit 4d720d6)
1 parent 975523d commit c131bec

File tree

3 files changed

+55
-6
lines changed

3 files changed

+55
-6
lines changed

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
613613
moduleCachePath.map({ AbsolutePath($0) })
614614
].compactMap({ $0 })
615615
let profile = sandboxProfile(toolsVersion: toolsVersion, cacheDirectories: cacheDirectories)
616-
cmd += ["sandbox-exec", "-p", profile]
616+
cmd = ["sandbox-exec", "-p", profile] + cmd
617617
}
618618
#endif
619619

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

743747
// The following accesses are only needed when interpreting the manifest (versus running a compiled version).
744748
if toolsVersion < .v5_3 {
745-
// Allow reading all files.
746-
stream <<< "(allow file-read*)" <<< "\n"
747-
// These are required by the Swift compiler.
748-
stream <<< "(allow process*)" <<< "\n"
749+
// This is required by the Swift compiler.
749750
stream <<< "(allow sysctl*)" <<< "\n"
750751
// Allow writing in temporary locations.
751752
stream <<< "(allow file-write*" <<< "\n"
@@ -755,9 +756,9 @@ private func sandboxProfile(toolsVersion: ToolsVersion, cacheDirectories: [Absol
755756
for directory in cacheDirectories {
756757
stream <<< " (subpath \"\(directory.pathString)\")" <<< "\n"
757758
}
759+
stream <<< ")" <<< "\n"
758760
}
759761

760-
stream <<< ")" <<< "\n"
761762
return stream.bytes.description
762763
}
763764

Tests/PackageLoadingTests/PD5_2LoadingTests.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,4 +345,28 @@ class PackageDescription5_2LoadingTests: PackageDescriptionLoadingTests {
345345
}
346346
}
347347
}
348+
349+
func testManifestLoadingIsSandboxed() throws {
350+
#if os(macOS) // Sandboxing is only done on macOS today.
351+
let stream = BufferedOutputByteStream()
352+
stream <<< """
353+
import Foundation
354+
355+
try! String(contentsOf:URL(string: "http://127.0.0.1")!)
356+
357+
import PackageDescription
358+
let package = Package(
359+
name: "Foo",
360+
targets: [
361+
.target(name: "Foo"),
362+
]
363+
)
364+
"""
365+
366+
XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
367+
guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
368+
XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
369+
}
370+
#endif
371+
}
348372
}

Tests/PackageLoadingTests/PD5_3LoadingTests.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,4 +406,28 @@ class PackageDescriptionNextLoadingTests: PackageDescriptionLoadingTests {
406406
XCTAssertTrue(error is ManifestParseError, "unexpected error: \(error)")
407407
}
408408
}
409+
410+
func testManifestLoadingIsSandboxed() throws {
411+
#if os(macOS) // Sandboxing is only done on macOS today.
412+
let stream = BufferedOutputByteStream()
413+
stream <<< """
414+
import Foundation
415+
416+
try! "should not be allowed".write(to: URL(fileURLWithPath: "/tmp/file.txt"), atomically: true, encoding: String.Encoding.utf8)
417+
418+
import PackageDescription
419+
let package = Package(
420+
name: "Foo",
421+
targets: [
422+
.target(name: "Foo"),
423+
]
424+
)
425+
"""
426+
427+
XCTAssertManifestLoadThrows(stream.bytes) { error, _ in
428+
guard case ManifestParseError.invalidManifestFormat(let msg, _) = error else { return XCTFail("unexpected error: \(error)") }
429+
XCTAssertTrue(msg.contains("Operation not permitted"), "unexpected error message: \(msg)")
430+
}
431+
#endif
432+
}
409433
}

0 commit comments

Comments
 (0)