Skip to content

Commit

Permalink
Disable fingerprint checking when storage is not available (#3928)
Browse files Browse the repository at this point in the history
Motivation:

Source compat test failure: https://ci.swift.org/job/swift-PR-source-compat-suite/5701/artifact/swift-source-compat-suite/

```
error: Failed to get source control fingerprint for swift-log remoteSourceControl https://github.com/apple/swift-log.git version 1.4.2 from storage: Error Domain=NSCocoaErrorDomain Code=513 "You don't have permission to save the file "fingerprints" in the folder "security"." UserInfo={NSFilePath=/Users/buildnode/.swiftpm/security/fingerprints, NSUnderlyingError=0x7feaae439370 {Error Domain=NSPOSIXErrorDomain Code=1 "Operation not permitted"}}
error: Error Domain=NSCocoaErrorDomain Code=513 "You don't have permission to save the file "fingerprints" in the folder "security"." UserInfo={NSFilePath=/Users/buildnode/.swiftpm/security/fingerprints, NSUnderlyingError=0x7feaae439370 {Error Domain=NSPOSIXErrorDomain Code=1 "Operation not permitted"}}
```

Modifications:
- Make `PackageFingerprintStorage` optional in `RegistryClient` and `SourceControlPackageContainer`, which would turn off fingerprint read/write and essentially disable the TOFU feature.
- `SwiftTool` will try to create the shared security directory (under which fingerprints are stored), and if it fails (e.g., permission errors) set `PackageFingerprintStorage` to none.
- Don't perform integrity check on fingerprint write. The validation failure will happen on read.
  • Loading branch information
yim-lee authored Dec 10, 2021
1 parent e4de038 commit e0b1bd2
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 140 deletions.
19 changes: 17 additions & 2 deletions Sources/Commands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,20 @@ public class SwiftTool {
return .none
}
}

private func getSharedSecurityDirectory() throws -> AbsolutePath? {
do {
let fileSystem = localFileSystem
let sharedSecurityDirectory = fileSystem.swiftPMSecurityDirectory
if !fileSystem.exists(sharedSecurityDirectory) {
try fileSystem.createDirectory(sharedSecurityDirectory, recursive: true)
}
return sharedSecurityDirectory
} catch {
self.observabilityScope.emit(warning: "Failed creating shared security directory: \(error)")
return .none
}
}

/// Returns the currently active workspace.
func getActiveWorkspace() throws -> Workspace {
Expand All @@ -649,7 +663,8 @@ public class SwiftTool {

let delegate = ToolWorkspaceDelegate(self.outputStream, logLevel: self.logLevel, observabilityScope: self.observabilityScope)
let provider = GitRepositoryProvider(processSet: processSet)
let sharedCacheDirectory = try self.getSharedCacheDirectory()
let sharedSecurityDirectory = try self.getSharedSecurityDirectory()
let sharedCacheDirectory = try self.getSharedCacheDirectory()
let sharedConfigurationDirectory = try self.getSharedConfigurationDirectory()
let isXcodeBuildSystemEnabled = self.options.buildSystem == .xcode
let workspace = try Workspace(
Expand All @@ -658,7 +673,7 @@ public class SwiftTool {
workingDirectory: buildPath,
editsDirectory: self.editsDirectory(),
resolvedVersionsFile: self.resolvedVersionsFile(),
sharedSecurityDirectory: localFileSystem.swiftPMSecurityDirectory,
sharedSecurityDirectory: sharedSecurityDirectory,
sharedCacheDirectory: sharedCacheDirectory,
sharedConfigurationDirectory: sharedConfigurationDirectory
),
Expand Down
1 change: 0 additions & 1 deletion Sources/PackageFingerprint/Model.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,4 @@ public typealias PackageFingerprints = [Version: [Fingerprint.Kind: Fingerprint]
public enum FingerprintCheckingMode: String {
case strict
case warn
case none
}
11 changes: 10 additions & 1 deletion Sources/PackageFingerprint/PackageFingerprintStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,16 @@ public extension PackageFingerprintStorage {
}
}

public enum PackageFingerprintStorageError: Error, Equatable {
public enum PackageFingerprintStorageError: Error, Equatable, CustomStringConvertible {
case conflict(given: Fingerprint, existing: Fingerprint)
case notFound

public var description: String {
switch self {
case .conflict(let given, let existing):
return "Fingerprint \(given) is different from previously recorded value \(existing)"
case .notFound:
return "Not found"
}
}
}
78 changes: 40 additions & 38 deletions Sources/PackageRegistry/RegistryClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ public final class RegistryClient {
private let archiverProvider: (FileSystem) -> Archiver
private let httpClient: HTTPClient
private let authorizationProvider: HTTPClientAuthorizationProvider?
private let fingerprintStorage: PackageFingerprintStorage
private let fingerprintStorage: PackageFingerprintStorage?
private let fingerprintCheckingMode: FingerprintCheckingMode
private let jsonDecoder: JSONDecoder

public init(
configuration: RegistryConfiguration,
identityResolver: IdentityResolver,
fingerprintStorage: PackageFingerprintStorage,
fingerprintStorage: PackageFingerprintStorage? = .none,
fingerprintCheckingMode: FingerprintCheckingMode,
authorizationProvider: HTTPClientAuthorizationProvider? = .none,
customHTTPClient: HTTPClient? = .none,
Expand Down Expand Up @@ -423,27 +423,23 @@ public final class RegistryClient {
throw RegistryError.invalidSourceArchive
}

self.fingerprintStorage.put(package: package,
version: version,
fingerprint: .init(origin: .registry(registry.url), value: checksum),
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { storageResult in
switch storageResult {
case .success:
completion(.success(checksum))
case .failure(PackageFingerprintStorageError.conflict(_, let existing)):
switch self.fingerprintCheckingMode {
case .strict:
completion(.failure(RegistryError.checksumChanged(latest: checksum, previous: existing.value)))
case .warn:
observabilityScope.emit(warning: "The checksum \(checksum) from \(registry.url.absoluteString) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))")
if let fingerprintStorage = self.fingerprintStorage {
fingerprintStorage.put(package: package,
version: version,
fingerprint: .init(origin: .registry(registry.url), value: checksum),
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { storageResult in
switch storageResult {
case .success:
completion(.success(checksum))
case .none:
case .failure(let error):
// Don't throw write errors
observabilityScope.emit(warning: "Failed to save checksum '\(checksum) from \(registry.url) to fingerprints storage: \(error)")
completion(.success(checksum))
}
case .failure(let error):
completion(.failure(error))
}
} else {
completion(.success(checksum))
}
} catch {
completion(.failure(RegistryError.failedRetrievingReleaseChecksum(error)))
Expand Down Expand Up @@ -535,8 +531,6 @@ public final class RegistryClient {
return completion(.failure(RegistryError.invalidChecksum(expected: expectedChecksum, actual: actualChecksum)))
case .warn:
observabilityScope.emit(warning: "The checksum \(actualChecksum) does not match previously recorded value \(expectedChecksum)")
case .none:
break
}
}

Expand Down Expand Up @@ -567,25 +561,33 @@ public final class RegistryClient {

// We either use a previously recorded checksum, or fetch it from the registry
func withExpectedChecksum(body: @escaping (Result<String, Error>) -> Void) {
self.fingerprintStorage.get(package: package,
version: version,
kind: .registry,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { result in
switch result {
case .success(let fingerprint):
body(.success(fingerprint.value))
case .failure(let error):
if error as? PackageFingerprintStorageError != .notFound {
observabilityScope.emit(error: "Failed to get registry fingerprint for \(package) \(version) from storage: \(error)")
if let fingerprintStorage = self.fingerprintStorage {
fingerprintStorage.get(package: package,
version: version,
kind: .registry,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue) { result in
switch result {
case .success(let fingerprint):
body(.success(fingerprint.value))
case .failure(let error):
if error as? PackageFingerprintStorageError != .notFound {
observabilityScope.emit(error: "Failed to get registry fingerprint for \(package) \(version) from storage: \(error)")
}
// Try fetching checksum from registry again no matter which kind of error it is
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
// Try fetching checksum from registry again no matter which kind of error it is
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
} else {
self.fetchSourceArchiveChecksum(package: package,
version: version,
observabilityScope: observabilityScope,
callbackQueue: callbackQueue,
completion: body)
}
}
}
Expand Down
26 changes: 10 additions & 16 deletions Sources/Workspace/SourceControlPackageContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
private let manifestLoader: ManifestLoaderProtocol
private let toolsVersionLoader: ToolsVersionLoaderProtocol
private let currentToolsVersion: ToolsVersion
private let fingerprintStorage: PackageFingerprintStorage
private let fingerprintStorage: PackageFingerprintStorage?
private let fingerprintCheckingMode: FingerprintCheckingMode
private let observabilityScope: ObservabilityScope

Expand All @@ -79,7 +79,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
manifestLoader: ManifestLoaderProtocol,
toolsVersionLoader: ToolsVersionLoaderProtocol,
currentToolsVersion: ToolsVersion,
fingerprintStorage: PackageFingerprintStorage,
fingerprintStorage: PackageFingerprintStorage?,
fingerprintCheckingMode: FingerprintCheckingMode,
observabilityScope: ObservabilityScope
) throws {
Expand Down Expand Up @@ -150,6 +150,10 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
}

func checkIntegrity(version: Version, revision: Revision) throws {
guard let fingerprintStorage = self.fingerprintStorage else {
return
}

guard case .remoteSourceControl(let sourceControlURL) = self.package.kind else {
return
}
Expand All @@ -158,7 +162,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
let fingerprint: Fingerprint
do {
fingerprint = try temp_await {
self.fingerprintStorage.get(
fingerprintStorage.get(
package: packageIdentity,
version: version,
kind: .sourceControl,
Expand All @@ -172,7 +176,7 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
// Write to storage if fingerprint not yet recorded
do {
try temp_await {
self.fingerprintStorage.put(
fingerprintStorage.put(
package: packageIdentity,
version: version,
fingerprint: fingerprint,
Expand All @@ -181,16 +185,8 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
callback: $0
)
}
} catch PackageFingerprintStorageError.conflict(_, let existing) {
let message = "Revision \(revision.identifier) for \(self.package) version \(version) does not match previously recorded value \(existing.value) from \(String(describing: existing.origin.url?.absoluteString))"
switch self.fingerprintCheckingMode {
case .strict:
throw StringError(message)
case .warn:
observabilityScope.emit(warning: message)
case .none:
return
}
} catch {
observabilityScope.emit(warning: "Failed to save revision '\(revision.identifier) from \(sourceControlURL) to fingerprints storage: \(error)")
}
} catch {
self.observabilityScope.emit(error: "Failed to get source control fingerprint for \(self.package) version \(version) from storage: \(error)")
Expand All @@ -205,8 +201,6 @@ internal final class SourceControlPackageContainer: PackageContainer, CustomStri
throw StringError(message)
case .warn:
observabilityScope.emit(warning: message)
case .none:
return
}
}
}
Expand Down
13 changes: 8 additions & 5 deletions Sources/Workspace/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public class Workspace {
fileprivate let checksumAlgorithm: HashAlgorithm

/// The package fingerprint storage
fileprivate let fingerprintStorage: PackageFingerprintStorage
fileprivate let fingerprintStorage: PackageFingerprintStorage?

/// Enable prefetching containers in resolver.
fileprivate let resolverPrefetchingEnabled: Bool
Expand Down Expand Up @@ -259,6 +259,7 @@ public class Workspace {
/// - customHTTPClient: A custom http client.
/// - customArchiver: A custom archiver.
/// - customChecksumAlgorithm: A custom checksum algorithm.
/// - customFingerprintStorage: A custom fingerprint storage.
/// - additionalFileRules: File rules to determine resource handling behavior.
/// - resolverUpdateEnabled: Enables the dependencies resolver automatic version update check. Enabled by default. When disabled the resolver relies only on the resolved version file
/// - resolverPrefetchingEnabled: Enables the dependencies resolver prefetching based on the resolved version file. Enabled by default.
Expand Down Expand Up @@ -306,10 +307,12 @@ public class Workspace {
delegate: delegate.map(WorkspaceRepositoryManagerDelegate.init(workspaceDelegate:)),
cachePath: sharedRepositoriesCacheEnabled ? location.sharedRepositoriesCacheDirectory : .none
)
let fingerprintStorage = customFingerprintStorage ?? FilePackageFingerprintStorage(
fileSystem: fileSystem,
directoryPath: location.sharedFingerprintsDirectory
)
let fingerprintStorage = customFingerprintStorage ?? location.sharedFingerprintsDirectory.map {
FilePackageFingerprintStorage(
fileSystem: fileSystem,
directoryPath: $0
)
}

let registryClient = customRegistryClient ?? registries.map { configuration in
RegistryClient(
Expand Down
8 changes: 4 additions & 4 deletions Sources/Workspace/WorkspaceConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extension Workspace {
public var resolvedVersionsFile: AbsolutePath

/// Path to the shared security directory
public var sharedSecurityDirectory: AbsolutePath
public var sharedSecurityDirectory: AbsolutePath?

/// Path to the shared cache directory
public var sharedCacheDirectory: AbsolutePath?
Expand Down Expand Up @@ -61,8 +61,8 @@ extension Workspace {
}

/// Path to the shared fingerprints directory.
public var sharedFingerprintsDirectory: AbsolutePath {
self.sharedSecurityDirectory.appending(component: "fingerprints")
public var sharedFingerprintsDirectory: AbsolutePath? {
self.sharedSecurityDirectory.map { $0.appending(component: "fingerprints") }
}

/// Path to the shared repositories cache.
Expand Down Expand Up @@ -103,7 +103,7 @@ extension Workspace {
workingDirectory: AbsolutePath,
editsDirectory: AbsolutePath,
resolvedVersionsFile: AbsolutePath,
sharedSecurityDirectory: AbsolutePath,
sharedSecurityDirectory: AbsolutePath?,
sharedCacheDirectory: AbsolutePath?,
sharedConfigurationDirectory: AbsolutePath?
) {
Expand Down
Loading

0 comments on commit e0b1bd2

Please sign in to comment.