From a1a3f985bbab46992cdf09f56db48591d5e5ee37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Pi=C3=B1era?= Date: Thu, 4 Dec 2025 17:24:42 +0100 Subject: [PATCH] Handle invalid source control URLs in registry identity lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add validation for source control URLs at multiple levels: 1. Add `isValid` property to `SourceControlURL` that checks: - URL doesn't contain whitespace (indicates malformed URL) - URL is parseable as a standard URL with a host, OR - URL matches SSH-style git format (user@host:path) 2. Validate URLs early in `mapRegistryIdentity` before making registry requests 3. Handle HTTP 400 responses from the registry server by throwing `RegistryError.invalidSourceControlURL` This handles cases where malformed URLs (e.g., containing git credential error messages like "'URL': failed looking up identity...") are passed to the registry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- Sources/Basics/SourceControlURL.swift | 26 +++++++++++ Sources/PackageRegistry/RegistryClient.swift | 10 ++++- Sources/Workspace/Workspace+Registry.swift | 6 +++ Tests/BasicsTests/SourceControlURLTests.swift | 44 +++++++++++++++++++ .../RegistryClientTests.swift | 33 ++++++++++++++ 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 Tests/BasicsTests/SourceControlURLTests.swift diff --git a/Sources/Basics/SourceControlURL.swift b/Sources/Basics/SourceControlURL.swift index 398c2f89ddf..af797c88696 100644 --- a/Sources/Basics/SourceControlURL.swift +++ b/Sources/Basics/SourceControlURL.swift @@ -38,6 +38,32 @@ public struct SourceControlURL: Codable, Equatable, Hashable, Sendable { public var url: URL? { return URL(string: self.urlString) } + + /// Whether this URL appears to be a valid source control URL. + /// + /// Valid source control URLs must: + /// - Be parseable as a URL (or match SSH-style git URL format) + /// - Have a non-empty host + /// - Not contain whitespace (which would indicate a malformed URL, + /// e.g., one concatenated with an error message) + public var isValid: Bool { + // URLs with whitespace are invalid (typically indicates concatenated error messages) + guard !self.urlString.contains(where: \.isWhitespace) else { + return false + } + + // Check for standard URL format (http://, https://, ssh://, etc.) + if let url = self.url, + let host = url.host, + !host.isEmpty { + return true + } + + // Check for SSH-style git URLs: git@host:path or user@host:path + // These don't parse as standard URLs but are valid git URLs + let sshPattern = #/^[\w.-]+@[\w.-]+:.+/# + return self.urlString.contains(sshPattern) + } } extension SourceControlURL: CustomStringConvertible { diff --git a/Sources/PackageRegistry/RegistryClient.swift b/Sources/PackageRegistry/RegistryClient.swift index 320297a0c30..125ef4f0f3d 100644 --- a/Sources/PackageRegistry/RegistryClient.swift +++ b/Sources/PackageRegistry/RegistryClient.swift @@ -1066,6 +1066,11 @@ public final class RegistryClient: AsyncCancellable { ) observabilityScope.emit(debug: "matched identities for \(scmURL): \(packageIdentities)") return Set(packageIdentities.identifiers.map(PackageIdentity.plain)) + case 400: + // 400 indicates the server rejected the URL as invalid. + // This can happen when malformed URLs (e.g., containing git credential error messages) + // are passed to the registry. + throw RegistryError.invalidSourceControlURL(scmURL) case 404: // 404 is valid, no identities mapped return [] @@ -1073,7 +1078,7 @@ public final class RegistryClient: AsyncCancellable { throw RegistryError.failedIdentityLookup( registry: registry, scmURL: scmURL, - error: self.unexpectedStatusError(response, expectedStatus: [200, 404]) + error: self.unexpectedStatusError(response, expectedStatus: [200, 400, 404]) ) } } @@ -1503,6 +1508,7 @@ public enum RegistryError: Error, CustomStringConvertible { case registryNotConfigured(scope: PackageIdentity.Scope?) case invalidPackageIdentity(PackageIdentity) case invalidURL(URL) + case invalidSourceControlURL(SourceControlURL) case invalidResponseStatus(expected: [Int], actual: Int) case invalidContentVersion(expected: String, actual: String?) case invalidContentType(expected: String, actual: String?) @@ -1580,6 +1586,8 @@ public enum RegistryError: Error, CustomStringConvertible { return "invalid package identifier '\(packageIdentity)'" case .invalidURL(let url): return "invalid URL '\(url)'" + case .invalidSourceControlURL(let scmURL): + return "invalid source control URL '\(scmURL)'" case .invalidResponseStatus(let expected, let actual): return "invalid registry response status '\(actual)', expected '\(expected)'" case .invalidContentVersion(let expected, let actual): diff --git a/Sources/Workspace/Workspace+Registry.swift b/Sources/Workspace/Workspace+Registry.swift index c9b3feebed1..ee3fa3ccc8f 100644 --- a/Sources/Workspace/Workspace+Registry.swift +++ b/Sources/Workspace/Workspace+Registry.swift @@ -19,6 +19,7 @@ import class Basics.ObservabilityScope import struct Basics.SourceControlURL import class Basics.ThreadSafeKeyValueStore import class PackageGraph.ResolvedPackagesStore +import enum PackageRegistry.RegistryError import protocol PackageLoading.ManifestLoaderProtocol import protocol PackageModel.DependencyMapper import protocol PackageModel.IdentityResolver @@ -332,6 +333,11 @@ extension Workspace { url: SourceControlURL, observabilityScope: ObservabilityScope ) async throws -> PackageIdentity? { + // Validate URL before attempting registry lookup + guard url.isValid else { + throw RegistryError.invalidSourceControlURL(url) + } + if let cached = self.identityLookupCache[url], cached.expirationTime > .now() { switch cached.result { case .success(let identity): diff --git a/Tests/BasicsTests/SourceControlURLTests.swift b/Tests/BasicsTests/SourceControlURLTests.swift new file mode 100644 index 00000000000..efdf5a2505a --- /dev/null +++ b/Tests/BasicsTests/SourceControlURLTests.swift @@ -0,0 +1,44 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2025 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 Basics +import Testing + +@Suite("SourceControlURL") +struct SourceControlURLTests { + @Test func validURLs() { + #expect(SourceControlURL("https://github.com/owner/repo").isValid) + #expect(SourceControlURL("https://github.com/owner/repo.git").isValid) + #expect(SourceControlURL("git@github.com:owner/repo.git").isValid) + #expect(SourceControlURL("ssh://git@github.com/owner/repo.git").isValid) + #expect(SourceControlURL("http://example.com/path/to/repo").isValid) + } + + @Test func invalidURLs_withWhitespace() { + // URLs containing whitespace are invalid (typically indicates concatenated error messages) + #expect(!SourceControlURL("https://github.com/owner/repo.git': failed looking up identity").isValid) + #expect(!SourceControlURL("https://github.com/owner/repo error message").isValid) + #expect(!SourceControlURL("https://github.com/owner/repo\there").isValid) + #expect(!SourceControlURL("https://github.com/owner/repo\nhere").isValid) + } + + @Test func invalidURLs_unparseable() { + // URLs that can't be parsed + #expect(!SourceControlURL("not a url").isValid) + #expect(!SourceControlURL("").isValid) + } + + @Test func invalidURLs_noHost() { + // URLs without a host + #expect(!SourceControlURL("file:///path/to/repo").isValid) + } +} diff --git a/Tests/PackageRegistryTests/RegistryClientTests.swift b/Tests/PackageRegistryTests/RegistryClientTests.swift index 32e65c1f150..e5c19a4960e 100644 --- a/Tests/PackageRegistryTests/RegistryClientTests.swift +++ b/Tests/PackageRegistryTests/RegistryClientTests.swift @@ -3148,6 +3148,39 @@ fileprivate var availabilityURL = URL("\(registryURL)/availability") let identities = try await registryClient.lookupIdentities(scmURL: packageURL) #expect([PackageIdentity.plain("mona.LinkedList")] == identities) } + + @Test func serverReturns400_throwsInvalidSourceControlURL() async throws { + // Test that when the server returns 400 Bad Request, the client throws invalidSourceControlURL + let scmURL = packageURL + + let handler: HTTPClient.Implementation = { request, _ in + // Server returns 400 Bad Request for invalid URLs + let data = #"{"message": "Invalid repository URL"}"#.data(using: .utf8)! + return .init( + statusCode: 400, + headers: .init([ + .init(name: "Content-Length", value: "\(data.count)"), + .init(name: "Content-Type", value: "application/json"), + .init(name: "Content-Version", value: "1"), + ]), + body: data + ) + } + + let httpClient = HTTPClient(implementation: handler) + var configuration = RegistryConfiguration() + configuration.defaultRegistry = Registry(url: registryURL, supportsAvailability: false) + + let registryClient = makeRegistryClient(configuration: configuration, httpClient: httpClient) + await #expect { + try await registryClient.lookupIdentities(scmURL: scmURL) + } throws: { error in + if case RegistryError.invalidSourceControlURL(scmURL) = error { + return true + } + return false + } + } } @Suite("Login") struct Login {