Skip to content

Commit 68703d0

Browse files
authored
assocaite signature warnings with the dependency that caused them (#6314)
motivation: better UX for warnings changes: add metadata with the package information to the warnings emitted when checking package signatures
1 parent 4006737 commit 68703d0

File tree

2 files changed

+43
-15
lines changed

2 files changed

+43
-15
lines changed

Sources/PackageRegistry/SignatureValidation.swift

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,21 +148,24 @@ struct SignatureValidation {
148148
observabilityScope: observabilityScope,
149149
completion: completion
150150
)
151-
} catch RegistryError.sourceArchiveNotSigned(let registry, let package, let version) {
152-
observabilityScope.emit(info: "\(package) \(version) from \(registry) is unsigned")
151+
} catch RegistryError.sourceArchiveNotSigned {
152+
observabilityScope.emit(
153+
info: "\(package) \(version) from \(registry) is unsigned",
154+
metadata: .registryPackageMetadata(identity: package)
155+
)
153156
guard let onUnsigned = configuration.onUnsigned else {
154157
return completion(.failure(RegistryError.missingConfiguration(details: "security.signing.onUnsigned")))
155158
}
156159

157160
let sourceArchiveNotSignedError = RegistryError.sourceArchiveNotSigned(
158161
registry: registry,
159-
package: package,
162+
package: package.underlying,
160163
version: version
161164
)
162165

163166
switch onUnsigned {
164167
case .prompt:
165-
self.delegate.onUnsigned(registry: registry, package: package, version: version) { `continue` in
168+
self.delegate.onUnsigned(registry: registry, package: package.underlying, version: version) { `continue` in
166169
if `continue` {
167170
completion(.success(.none))
168171
} else {
@@ -172,7 +175,10 @@ struct SignatureValidation {
172175
case .error:
173176
completion(.failure(sourceArchiveNotSignedError))
174177
case .warn:
175-
observabilityScope.emit(warning: "\(sourceArchiveNotSignedError)")
178+
observabilityScope.emit(
179+
warning: "\(sourceArchiveNotSignedError)",
180+
metadata: .registryPackageMetadata(identity: package)
181+
)
176182
completion(.success(.none))
177183
case .silentAllow:
178184
// Continue without logging
@@ -283,21 +289,24 @@ struct SignatureValidation {
283289
)
284290
} catch ManifestSignatureParser.Error.malformedManifestSignature {
285291
completion(.failure(RegistryError.invalidSignature(reason: "manifest signature is malformed")))
286-
} catch RegistryError.sourceArchiveNotSigned(let registry, let package, let version) {
287-
observabilityScope.emit(info: "\(package) \(version) from \(registry) is unsigned")
292+
} catch RegistryError.sourceArchiveNotSigned {
293+
observabilityScope.emit(
294+
info: "\(package) \(version) from \(registry) is unsigned",
295+
metadata: .registryPackageMetadata(identity: package)
296+
)
288297
guard let onUnsigned = configuration.onUnsigned else {
289298
return completion(.failure(RegistryError.missingConfiguration(details: "security.signing.onUnsigned")))
290299
}
291300

292301
let sourceArchiveNotSignedError = RegistryError.sourceArchiveNotSigned(
293302
registry: registry,
294-
package: package,
303+
package: package.underlying,
295304
version: version
296305
)
297306

298307
switch onUnsigned {
299308
case .prompt:
300-
self.delegate.onUnsigned(registry: registry, package: package, version: version) { `continue` in
309+
self.delegate.onUnsigned(registry: registry, package: package.underlying, version: version) { `continue` in
301310
if `continue` {
302311
completion(.success(.none))
303312
} else {
@@ -307,7 +316,10 @@ struct SignatureValidation {
307316
case .error:
308317
completion(.failure(sourceArchiveNotSignedError))
309318
case .warn:
310-
observabilityScope.emit(warning: "\(sourceArchiveNotSignedError)")
319+
observabilityScope.emit(
320+
warning: "\(sourceArchiveNotSignedError)",
321+
metadata: .registryPackageMetadata(identity: package)
322+
)
311323
completion(.success(.none))
312324
case .silentAllow:
313325
// Continue without logging
@@ -394,7 +406,10 @@ struct SignatureValidation {
394406
case .error:
395407
completion(.failure(signerNotTrustedError))
396408
case .warn:
397-
observabilityScope.emit(warning: "\(signerNotTrustedError)")
409+
observabilityScope.emit(
410+
warning: "\(signerNotTrustedError)",
411+
metadata: .registryPackageMetadata(identity: package)
412+
)
398413
completion(.success(.none))
399414
case .silentAllow:
400415
// Continue without logging
@@ -470,3 +485,12 @@ extension VerifierConfiguration {
470485
return verifierConfiguration
471486
}
472487
}
488+
489+
extension ObservabilityMetadata {
490+
public static func registryPackageMetadata(identity: PackageIdentity.RegistryIdentity) -> Self {
491+
var metadata = ObservabilityMetadata()
492+
metadata.packageIdentity = identity.underlying
493+
metadata.packageKind = .registry(identity.underlying)
494+
return metadata
495+
}
496+
}

Tests/PackageRegistryTests/SignatureValidationTests.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ final class SignatureValidationTests: XCTestCase {
161161
)
162162

163163
testDiagnostics(observability.diagnostics) { result in
164-
result.check(diagnostic: .contains("is not signed"), severity: .warning)
164+
let diagnostics = result.check(diagnostic: .contains("is not signed"), severity: .warning)
165+
XCTAssertEqual(diagnostics?.metadata?.packageIdentity, package.underlying)
165166
}
166167
}
167168

@@ -446,7 +447,8 @@ final class SignatureValidationTests: XCTestCase {
446447
)
447448

448449
testDiagnostics(observability.diagnostics) { result in
449-
result.check(diagnostic: .contains("is not signed"), severity: .warning)
450+
let diagnostics = result.check(diagnostic: .contains("is not signed"), severity: .warning)
451+
XCTAssertEqual(diagnostics?.metadata?.packageIdentity, package.underlying)
450452
}
451453
}
452454

@@ -1335,7 +1337,8 @@ final class SignatureValidationTests: XCTestCase {
13351337
)
13361338

13371339
testDiagnostics(observability.diagnostics) { result in
1338-
result.check(diagnostic: .contains("not trusted"), severity: .warning)
1340+
let diagnostics = result.check(diagnostic: .contains("not trusted"), severity: .warning)
1341+
XCTAssertEqual(diagnostics?.metadata?.packageIdentity, package.underlying)
13391342
}
13401343
}
13411344

@@ -1897,7 +1900,8 @@ final class SignatureValidationTests: XCTestCase {
18971900
)
18981901

18991902
testDiagnostics(observability.diagnostics) { result in
1900-
result.check(diagnostic: .contains("not trusted"), severity: .warning)
1903+
let diagnostics = result.check(diagnostic: .contains("not trusted"), severity: .warning)
1904+
XCTAssertEqual(diagnostics?.metadata?.packageIdentity, package.underlying)
19011905
}
19021906
}
19031907
#endif

0 commit comments

Comments
 (0)