Extend ManifestLoader with the ability to restrict imports (#5888)

This allows passing a minimal list of allowed modules and the minimum tools-version for applying the restricton to `ManifestLoader` and `WorkspaceConfiguration` (which passes it to the manifest loader). This allows clients of libSwiftPM to choose to be more restrictive about the modules package manifests are allowed to import.

rdar://92879696

Author: neonichu

Sources/Basics/CMakeLists.txt

@@ -20,6 +20,7 @@ add_library(Basics
   FileSystem+Extensions.swift
   HTTPClient+URLSession.swift
   HTTPClient.swift
+  ImportScanning.swift
   JSON+Extensions.swift
   JSONDecoder+Extensions.swift
   Netrc.swift

Sources/Basics/ImportScanning.swift

@@ -0,0 +1,65 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 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 Dispatch
+
+import class Foundation.JSONDecoder
+import struct TSCBasic.AbsolutePath
+import class TSCBasic.Process
+
+private let defaultImports = ["Swift", "SwiftOnoneSupport", "_Concurrency", "_StringProcessing"]
+
+private struct Imports: Decodable {
+    let imports: [String]
+}
+
+public protocol ImportScanner {
+    func scanImports(_ filePathToScan: AbsolutePath, callbackQueue: DispatchQueue, completion: @escaping (Result<[String], Error>) -> Void)
+}
+
+public struct SwiftcImportScanner: ImportScanner {
+    private let swiftCompilerEnvironment: EnvironmentVariables
+    private let swiftCompilerFlags: [String]
+    private let swiftCompilerPath: AbsolutePath
+
+    public init(swiftCompilerEnvironment: EnvironmentVariables, swiftCompilerFlags: [String], swiftCompilerPath: AbsolutePath) {
+        self.swiftCompilerEnvironment = swiftCompilerEnvironment
+        self.swiftCompilerFlags = swiftCompilerFlags
+        self.swiftCompilerPath = swiftCompilerPath
+    }
+
+    public func scanImports(_ filePathToScan: AbsolutePath,
+                            callbackQueue: DispatchQueue,
+                            completion: @escaping (Result<[String], Error>) -> Void) {
+        let cmd = [swiftCompilerPath.pathString,
+                   filePathToScan.pathString,
+                   "-scan-dependencies", "-Xfrontend", "-import-prescan"] + self.swiftCompilerFlags
+
+        TSCBasic.Process.popen(arguments: cmd, environment: self.swiftCompilerEnvironment, queue: callbackQueue) { result in
+            dispatchPrecondition(condition: .onQueue(callbackQueue))
+            
+            do {
+                let stdout = try result.get().utf8Output()
+                let imports = try JSONDecoder.makeWithDefaults().decode(Imports.self, from: stdout).imports
+                    .filter { !defaultImports.contains($0) }
+                
+                callbackQueue.async {
+                    completion(.success(imports))
+                }
+            } catch {
+                callbackQueue.async {
+                    completion(.failure(error))
+                }
+            }
+        }
+    }
+}

Sources/CoreCommands/SwiftTool.swift

@@ -378,7 +378,8 @@ public final class SwiftTool {
                 additionalFileRules: isXcodeBuildSystemEnabled ? FileRuleDescription.xcbuildFileTypes : FileRuleDescription.swiftpmFileTypes,
                 sharedDependenciesCacheEnabled: self.options.caching.useDependenciesCache,
                 fingerprintCheckingMode: self.options.security.fingerprintCheckingMode,
-                sourceControlToRegistryDependencyTransformation: self.options.resolver.sourceControlToRegistryDependencyTransformation.workspaceConfiguration
+                sourceControlToRegistryDependencyTransformation: self.options.resolver.sourceControlToRegistryDependencyTransformation.workspaceConfiguration,
+                restrictImports: .none
             ),
             cancellator: self.cancellator,
             initializationWarningHandler: { self.observabilityScope.emit(warning: $0) },
@@ -704,7 +705,8 @@ public final class SwiftTool {
                 toolchain: self.getHostToolchain(),
                 isManifestSandboxEnabled: !self.options.security.shouldDisableSandbox,
                 cacheDir: cachePath,
-                extraManifestFlags: extraManifestFlags
+                extraManifestFlags: extraManifestFlags,
+                restrictImports: nil
             )
         })
     }()

Sources/PackageLoading/ManifestLoader.swift

@@ -25,6 +25,9 @@ public enum ManifestParseError: Swift.Error, Equatable {
     // TODO: Test this error.
     /// The manifest was successfully loaded by swift interpreter but there were runtime issues.
     case runtimeManifestErrors([String])
+
+    /// The manifest loader specified import restrictions that the given manifest violated.
+    case importsRestrictedModules([String])
 }
 
 // used to output the errors via the observability system
@@ -37,6 +40,8 @@ extension ManifestParseError: CustomStringConvertible {
             return "invalid manifest\n\(error)"
         case .runtimeManifestErrors(let errors):
             return "invalid manifest (evaluation failed)\n\(errors.joined(separator: "\n"))"
+        case .importsRestrictedModules(let modules):
+            return "invalid manifest, imports restricted modules: \(modules.joined(separator: ", "))"
         }
     }
 }
@@ -149,6 +154,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     private let isManifestSandboxEnabled: Bool
     private let delegate: ManifestLoaderDelegate?
     private let extraManifestFlags: [String]
+    private let restrictImports: (startingToolsVersion: ToolsVersion, allowedImports: [String])?
 
     private let databaseCacheDir: AbsolutePath?
 
@@ -165,13 +171,15 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         isManifestSandboxEnabled: Bool = true,
         cacheDir: AbsolutePath? = nil,
         delegate: ManifestLoaderDelegate? = nil,
-        extraManifestFlags: [String] = []
+        extraManifestFlags: [String] = [],
+        restrictImports: (startingToolsVersion: ToolsVersion, allowedImports: [String])? = .none
     ) {
         self.toolchain = toolchain
         self.serializedDiagnostics = serializedDiagnostics
         self.isManifestSandboxEnabled = isManifestSandboxEnabled
         self.delegate = delegate
         self.extraManifestFlags = extraManifestFlags
+        self.restrictImports = restrictImports
 
         self.databaseCacheDir = try? cacheDir.map(resolveSymlinks)
 
@@ -426,6 +434,53 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         }
     }
 
+    private func validateImports(
+        manifestPath: AbsolutePath,
+        toolsVersion: ToolsVersion,
+        callbackQueue: DispatchQueue,
+        completion: @escaping (Result<Void, Error>) -> Void) {
+            // If there are no import restrictions, we do not need to validate.
+            guard let restrictImports = restrictImports, toolsVersion >= restrictImports.startingToolsVersion else {
+                return callbackQueue.async {
+                    completion(.success(()))
+                }
+            }
+
+            // Allowed are the expected defaults, plus anything allowed by the configured restrictions.
+            let allowedImports = ["PackageDescription", "Swift", "SwiftOnoneSupport"] + restrictImports.allowedImports
+
+            // wrap the completion to free concurrency control semaphore
+            let completion: (Result<Void, Error>) -> Void = { result in
+                self.concurrencySemaphore.signal()
+                callbackQueue.async {
+                    completion(result)
+                }
+            }
+
+            // we must not block the calling thread (for concurrency control) so nesting this in a queue
+            self.evaluationQueue.addOperation {
+                do {
+                    // park the evaluation thread based on the max concurrency allowed
+                    self.concurrencySemaphore.wait()
+
+                    let importScanner = SwiftcImportScanner(swiftCompilerEnvironment: self.toolchain.swiftCompilerEnvironment,
+                                                            swiftCompilerFlags: self.extraManifestFlags,
+                                                            swiftCompilerPath: self.toolchain.swiftCompilerPathForManifests)
+
+                    importScanner.scanImports(manifestPath, callbackQueue: callbackQueue) { result in
+                        do {
+                            let imports = try result.get().filter { !allowedImports.contains($0) }
+                            guard imports.isEmpty else {
+                                throw ManifestParseError.importsRestrictedModules(imports)
+                            }
+                        } catch {
+                            completion(.failure(error))
+                        }
+                    }
+                }
+            }
+        }
+
     /// Compiler the manifest at the given path and retrieve the JSON.
     fileprivate func evaluateManifest(
         packageIdentity: PackageIdentity,
@@ -454,17 +509,30 @@ public final class ManifestLoader: ManifestLoaderProtocol {
                     VFSOverlay.File(name: manifestPath.pathString, externalContents: manifestTempFilePath.pathString)
                 ]).write(to: vfsOverlayTempFilePath, fileSystem: localFileSystem)
 
-                try self.evaluateManifest(
-                    at: manifestPath,
-                    vfsOverlayPath: vfsOverlayTempFilePath,
-                    packageIdentity: packageIdentity,
-                    toolsVersion: toolsVersion,
-                    delegateQueue: delegateQueue,
-                    callbackQueue: callbackQueue
-                ) { result in
+                validateImports(manifestPath: manifestTempFilePath, toolsVersion: toolsVersion, callbackQueue: callbackQueue) { result in
                     dispatchPrecondition(condition: .onQueue(callbackQueue))
-                    cleanupTempDir(tempDir)
-                    completion(result)
+
+                    do {
+                        try result.get()
+
+                        try self.evaluateManifest(
+                            at: manifestPath,
+                            vfsOverlayPath: vfsOverlayTempFilePath,
+                            packageIdentity: packageIdentity,
+                            toolsVersion: toolsVersion,
+                            delegateQueue: delegateQueue,
+                            callbackQueue: callbackQueue
+                        ) { result in
+                            dispatchPrecondition(condition: .onQueue(callbackQueue))
+                            cleanupTempDir(tempDir)
+                            completion(result)
+                        }
+                    } catch {
+                        cleanupTempDir(tempDir)
+                        callbackQueue.async {
+                            completion(.failure(error))
+                        }
+                    }
                 }
             }
         } catch {

Sources/SPMTestSupport/MockWorkspace.swift

@@ -266,7 +266,8 @@ public final class MockWorkspace {
                 additionalFileRules: WorkspaceConfiguration.default.additionalFileRules,
                 sharedDependenciesCacheEnabled: WorkspaceConfiguration.default.sharedDependenciesCacheEnabled,
                 fingerprintCheckingMode: .strict,
-                sourceControlToRegistryDependencyTransformation: self.sourceControlToRegistryDependencyTransformation
+                sourceControlToRegistryDependencyTransformation: self.sourceControlToRegistryDependencyTransformation,
+                restrictImports: .none
             ),
             customFingerprints: self.fingerprints,
             customMirrors: self.mirrors,

Sources/Workspace/Workspace+Configuration.swift

@@ -663,6 +663,9 @@ public struct WorkspaceConfiguration {
     /// Whether to create a product for use in the Swift REPL
     public var createREPLProduct: Bool
 
+    /// Whether or not there should be import restrictions applied when loading manifests
+    public var restrictImports: (startingToolsVersion: ToolsVersion, allowedImports: [String])?
+
     public init(
         skipDependenciesUpdates: Bool,
         prefetchBasedOnResolvedFile: Bool,
@@ -671,7 +674,8 @@ public struct WorkspaceConfiguration {
         additionalFileRules: [FileRuleDescription],
         sharedDependenciesCacheEnabled: Bool,
         fingerprintCheckingMode: FingerprintCheckingMode,
-        sourceControlToRegistryDependencyTransformation: SourceControlToRegistryDependencyTransformation
+        sourceControlToRegistryDependencyTransformation: SourceControlToRegistryDependencyTransformation,
+        restrictImports: (startingToolsVersion: ToolsVersion, allowedImports: [String])?
     ) {
         self.skipDependenciesUpdates = skipDependenciesUpdates
         self.prefetchBasedOnResolvedFile = prefetchBasedOnResolvedFile
@@ -681,6 +685,7 @@ public struct WorkspaceConfiguration {
         self.sharedDependenciesCacheEnabled = sharedDependenciesCacheEnabled
         self.fingerprintCheckingMode = fingerprintCheckingMode
         self.sourceControlToRegistryDependencyTransformation = sourceControlToRegistryDependencyTransformation
+        self.restrictImports = restrictImports
     }
 
     /// Default instance of WorkspaceConfiguration
@@ -693,7 +698,8 @@ public struct WorkspaceConfiguration {
             additionalFileRules: [],
             sharedDependenciesCacheEnabled: true,
             fingerprintCheckingMode: .strict,
-            sourceControlToRegistryDependencyTransformation: .disabled
+            sourceControlToRegistryDependencyTransformation: .disabled,
+            restrictImports: .none
         )
     }
 

Sources/Workspace/Workspace.swift

@@ -437,7 +437,8 @@ public class Workspace {
         let location = try Location(forRootPackage: packagePath, fileSystem: fileSystem)
         let manifestLoader = ManifestLoader(
             toolchain: customHostToolchain,
-            cacheDir: location.sharedManifestsCacheDirectory
+            cacheDir: location.sharedManifestsCacheDirectory,
+            restrictImports: configuration?.restrictImports
         )
         try self.init(
             fileSystem: fileSystem,
@@ -541,7 +542,8 @@ public class Workspace {
         let hostToolchain = try customHostToolchain ?? UserToolchain(destination: .hostDestination())
         var manifestLoader = customManifestLoader ?? ManifestLoader(
             toolchain: hostToolchain,
-            cacheDir: location.sharedManifestsCacheDirectory
+            cacheDir: location.sharedManifestsCacheDirectory,
+            restrictImports: configuration?.restrictImports
         )
 
         let configuration = configuration ?? .default

Tests/PackageLoadingTests/PDLoadingTests.swift

@@ -52,6 +52,7 @@ class PackageDescriptionLoadingTests: XCTestCase, ManifestLoaderDelegate {
         _ bytes: ByteString,
         toolsVersion: ToolsVersion? = nil,
         packageKind: PackageReference.Kind? = nil,
+        customManifestLoader: ManifestLoader? = nil,
         observabilityScope: ObservabilityScope,
         file: StaticString = #file,
         line: UInt = #line
@@ -73,7 +74,7 @@ class PackageDescriptionLoadingTests: XCTestCase, ManifestLoaderDelegate {
         let fileSystem = InMemoryFileSystem()
         let manifestPath = packagePath.appending(component: Manifest.filename)
         try fileSystem.writeFileContents(manifestPath, bytes: bytes)
-        let manifest = try manifestLoader.load(
+        let manifest = try (customManifestLoader ?? manifestLoader).load(
             manifestPath: manifestPath,
             packageKind: packageKind,
             toolsVersion: toolsVersion,

Tests/PackageLoadingTests/PD_5_7_LoadingTests .swift

@@ -172,4 +172,22 @@ class PackageDescription5_7LoadingTests: PackageDescriptionLoadingTests {
             PlatformDescription(name: "driverkit", version: "22.0"),
         ])
     }
+
+    func testImportRestrictions() throws {
+        let content =  """
+            import PackageDescription
+            import BestModule
+            let package = Package(name: "Foo")
+            """
+
+        let observability = ObservabilitySystem.makeForTesting()
+        let manifestLoader = ManifestLoader(toolchain: try UserToolchain.default, restrictImports: (.v5_7, []))
+        XCTAssertThrowsError(try loadAndValidateManifest(ByteString(encodingAsUTF8: content), customManifestLoader: manifestLoader, observabilityScope: observability.topScope)) { error in
+            if case ManifestParseError.importsRestrictedModules(let modules) = error {
+                XCTAssertEqual(modules, ["BestModule"])
+            } else {
+                XCTFail("unexpected error: \(error)")
+            }
+        }
+    }
 }