ContainerRegistry: Reject invalid repository names (#138)

Motivation
----------

`ImageReference` does not check for illegal characters in parsed image
references. This means that `containertool` will send illegal image
names to the registry. The registry will reject them, but the error
message might not explain why, so a generic error message will be
printed. Runtimes reject illegal image references immediately, without
sending them to the registry.

Modifications
-------------

* Introduce a `Repository` wrapper type
* Check validity when constructing the `Repository`
* Change the low-level API functions to accept `Repository` instead of
`String`.

Result
------

It is impossible to create a `Repository` object containing a malformed
name, because the constructor checks the string value. It is impossible
to send a malformed name to the registry because the API wrappers only
accept `Repository` objects.

Fixes #135 

Test Plan
---------

Existing tests continue to pass.
New tests exercise additional checks which were previously missing.

Author: euanh

Sources/ContainerRegistry/AuthHandler.swift

@@ -158,7 +158,7 @@ public struct AuthHandler {
 
     public func auth(
         registry: URL,
-        repository: String,
+        repository: ImageReference.Repository,
         actions: [String],
         withScheme scheme: AuthChallenge,
         usingClient client: HTTPClient

Sources/ContainerRegistry/Blobs.swift

@@ -28,9 +28,7 @@ public func digest<D: DataProtocol>(of data: D) -> String {
 
 extension RegistryClient {
     // Internal helper method to initiate a blob upload in 'two shot' mode
-    func startBlobUploadSession(repository: String) async throws -> URL {
-        precondition(repository.count > 0, "repository must not be an empty string")
-
+    func startBlobUploadSession(repository: ImageReference.Repository) async throws -> URL {
         // Upload in "two shot" mode.
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#post-then-put
         // - POST to obtain a session ID.
@@ -67,8 +65,7 @@ extension RegistryClient {
 extension HTTPField.Name { static let dockerContentDigest = Self("Docker-Content-Digest")! }
 
 public extension RegistryClient {
-    func blobExists(repository: String, digest: String) async throws -> Bool {
-        precondition(repository.count > 0, "repository must not be an empty string")
+    func blobExists(repository: ImageReference.Repository, digest: String) async throws -> Bool {
         precondition(digest.count > 0)
 
         do {
@@ -87,8 +84,7 @@ public extension RegistryClient {
     ///   - digest: Digest of the blob.
     /// - Returns: The downloaded data.
     /// - Throws: If the blob download fails.
-    func getBlob(repository: String, digest: String) async throws -> Data {
-        precondition(repository.count > 0, "repository must not be an empty string")
+    func getBlob(repository: ImageReference.Repository, digest: String) async throws -> Data {
         precondition(digest.count > 0, "digest must not be an empty string")
 
         return try await executeRequestThrowing(
@@ -110,8 +106,7 @@ public extension RegistryClient {
     /// in the registry as plain blobs with MIME type "application/octet-stream".
     /// This function attempts to decode the received data without reference
     /// to the MIME type.
-    func getBlob<Response: Decodable>(repository: String, digest: String) async throws -> Response {
-        precondition(repository.count > 0, "repository must not be an empty string")
+    func getBlob<Response: Decodable>(repository: ImageReference.Repository, digest: String) async throws -> Response {
         precondition(digest.count > 0, "digest must not be an empty string")
 
         return try await executeRequestThrowing(
@@ -132,11 +127,10 @@ public extension RegistryClient {
     /// - Returns: An ContentDescriptor object representing the
     ///            uploaded blob.
     /// - Throws: If the blob cannot be encoded or the upload fails.
-    func putBlob(repository: String, mediaType: String = "application/octet-stream", data: Data) async throws
+    func putBlob(repository: ImageReference.Repository, mediaType: String = "application/octet-stream", data: Data)
+        async throws
         -> ContentDescriptor
     {
-        precondition(repository.count > 0, "repository must not be an empty string")
-
         // Ask the server to open a session and tell us where to upload our data
         let location = try await startBlobUploadSession(repository: repository)
 
@@ -179,7 +173,11 @@ public extension RegistryClient {
     ///  Some JSON objects, such as ImageConfiguration, are stored
     /// in the registry as plain blobs with MIME type "application/octet-stream".
     /// This function encodes the data parameter and uploads it as a generic blob.
-    func putBlob<Body: Encodable>(repository: String, mediaType: String = "application/octet-stream", data: Body)
+    func putBlob<Body: Encodable>(
+        repository: ImageReference.Repository,
+        mediaType: String = "application/octet-stream",
+        data: Body
+    )
         async throws -> ContentDescriptor
     {
         let encoded = try encoder.encode(data)

Sources/ContainerRegistry/ImageReference.swift

@@ -14,13 +14,11 @@
 
 import RegexBuilder
 
-enum ReferenceError: Error { case unexpected(String) }
-
 // https://github.com/distribution/distribution/blob/v2.7.1/reference/reference.go
 // Split the image reference into a registry and a name part.
 func splitReference(_ reference: String) throws -> (String?, String) {
     let splits = reference.split(separator: "/", maxSplits: 1, omittingEmptySubsequences: false)
-    if splits.count == 0 { throw ReferenceError.unexpected("unexpected error") }
+    if splits.count == 0 { throw ImageReference.ValidationError.unexpected("unexpected error") }
 
     if splits.count == 1 { return (nil, reference) }
 
@@ -39,7 +37,7 @@ func splitName(_ name: String) throws -> (String, String) {
     if digestSplit.count == 2 { return (String(digestSplit[0]), String(digestSplit[1])) }
 
     let tagSplit = name.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false)
-    if tagSplit.count == 0 { throw ReferenceError.unexpected("unexpected error") }
+    if tagSplit.count == 0 { throw ImageReference.ValidationError.unexpected("unexpected error") }
 
     if tagSplit.count == 1 { return (name, "latest") }
 
@@ -52,10 +50,14 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
     /// The registry which contains this image
     public var registry: String
     /// The repository which contains this image
-    public var repository: String
+    public var repository: Repository
     /// The tag identifying the image.
     public var reference: String
 
+    public enum ValidationError: Error {
+        case unexpected(String)
+    }
+
     /// Creates an ImageReference from an image reference string.
     /// - Parameters:
     ///   - reference: The reference to parse.
@@ -72,19 +74,20 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
         // moby/moby assumes that these names refer to images in `library`: `library/swift` or `library/swift:slim`.
         // This special case only applies when using Docker Hub, so `example.com/swift` is not expanded `example.com/library/swift`
         if self.registry == "index.docker.io" && !repository.contains("/") {
-            self.repository = "library/\(repository)"
+            self.repository = try Repository("library/\(repository)")
         } else {
-            self.repository = repository
+            self.repository = try Repository(repository)
         }
         self.reference = reference
     }
 
     /// Creates an ImageReference from separate registry, repository and reference strings.
+    /// Used only in tests.
     /// - Parameters:
     ///   - registry: The registry which stores the image data.
     ///   - repository: The repository within the registry which holds the image.
     ///   - reference: The tag identifying the image.
-    public init(registry: String, repository: String, reference: String) {
+    init(registry: String, repository: Repository, reference: String) {
         self.registry = registry
         self.repository = repository
         self.reference = reference
@@ -104,3 +107,45 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
         "ImageReference(registry: \(registry), repository: \(repository), reference: \(reference))"
     }
 }
+
+extension ImageReference {
+    /// Repository refers a repository (image namespace) on a container registry
+    public struct Repository: Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
+        var value: String
+
+        public enum ValidationError: Error, Equatable {
+            case emptyString
+            case containsUppercaseLetters(String)
+            case invalidReferenceFormat(String)
+        }
+
+        public init(_ rawValue: String) throws {
+            // Reference handling in github.com/distribution reports empty and uppercase as specific errors.
+            // All other errors caused are reported as generic format errors.
+            guard rawValue.count > 0 else {
+                throw ValidationError.emptyString
+            }
+
+            if (rawValue.contains { $0.isUppercase }) {
+                throw ValidationError.containsUppercaseLetters(rawValue)
+            }
+
+            // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
+            let regex = /[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*/
+            if try regex.wholeMatch(in: rawValue) == nil {
+                throw ValidationError.invalidReferenceFormat(rawValue)
+            }
+
+            value = rawValue
+        }
+
+        public var description: String {
+            value
+        }
+
+        /// Printable description of an ImageReference in a form suitable for debugging.
+        public var debugDescription: String {
+            "Repository(\(value))"
+        }
+    }
+}

Sources/ContainerRegistry/Manifests.swift

@@ -13,10 +13,11 @@
 //===----------------------------------------------------------------------===//
 
 public extension RegistryClient {
-    func putManifest(repository: String, reference: String, manifest: ImageManifest) async throws -> String {
+    func putManifest(repository: ImageReference.Repository, reference: String, manifest: ImageManifest) async throws
+        -> String
+    {
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests
-        precondition(repository.count > 0, "repository must not be an empty string")
-        precondition(reference.count > 0, "reference must not be an empty string")
+        precondition("\(reference)".count > 0, "reference must not be an empty string")
 
         let httpResponse = try await executeRequestThrowing(
             // All blob uploads have Content-Type: application/octet-stream on the wire, even if mediatype is different
@@ -41,9 +42,8 @@ public extension RegistryClient {
             .absoluteString
     }
 
-    func getManifest(repository: String, reference: String) async throws -> ImageManifest {
+    func getManifest(repository: ImageReference.Repository, reference: String) async throws -> ImageManifest {
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
-        precondition(repository.count > 0, "repository must not be an empty string")
         precondition(reference.count > 0, "reference must not be an empty string")
 
         return try await executeRequestThrowing(
@@ -60,8 +60,7 @@ public extension RegistryClient {
         .data
     }
 
-    func getIndex(repository: String, reference: String) async throws -> ImageIndex {
-        precondition(repository.count > 0, "repository must not be an empty string")
+    func getIndex(repository: ImageReference.Repository, reference: String) async throws -> ImageIndex {
         precondition(reference.count > 0, "reference must not be an empty string")
 
         return try await executeRequestThrowing(

Sources/ContainerRegistry/RegistryClient.swift

@@ -127,7 +127,8 @@ extension URL {
     ///   - repository: The name of the repository.   May include path separators.
     ///   - endpoint: The distribution endpoint e.g. "tags/list"
     /// - Returns: A fully-qualified URL for the endpoint.
-    func distributionEndpoint(forRepository repository: String, andEndpoint endpoint: String) -> URL {
+    func distributionEndpoint(forRepository repository: ImageReference.Repository, andEndpoint endpoint: String) -> URL
+    {
         self.appendingPathComponent("/v2/\(repository)/\(endpoint)")
     }
 }
@@ -141,7 +142,7 @@ extension RegistryClient {
         }
 
         var method: HTTPRequest.Method  // HTTP method
-        var repository: String  // Repository path on the registry
+        var repository: ImageReference.Repository  // Repository path on the registry
         var destination: Destination  // Destination of the operation: can be a subpath or remote URL
         var actions: [String]  // Actions required by this operation
         var accepting: [String] = []  // Acceptable response types
@@ -156,7 +157,7 @@ extension RegistryClient {
 
         // Convenience constructors
         static func get(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             path: String,
             actions: [String]? = nil,
             accepting: [String] = [],
@@ -173,7 +174,7 @@ extension RegistryClient {
         }
 
         static func get(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             url: URL,
             actions: [String]? = nil,
             accepting: [String] = [],
@@ -190,7 +191,7 @@ extension RegistryClient {
         }
 
         static func head(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             path: String,
             actions: [String]? = nil,
             accepting: [String] = [],
@@ -208,7 +209,7 @@ extension RegistryClient {
 
         /// This handles the 'put' case where the registry gives us a location URL which we must not alter, aside from adding the digest to it
         static func put(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             url: URL,
             actions: [String]? = nil,
             accepting: [String] = [],
@@ -225,7 +226,7 @@ extension RegistryClient {
         }
 
         static func put(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             path: String,
             actions: [String]? = nil,
             accepting: [String] = [],
@@ -242,7 +243,7 @@ extension RegistryClient {
         }
 
         static func post(
-            _ repository: String,
+            _ repository: ImageReference.Repository,
             path: String,
             actions: [String]? = nil,
             accepting: [String] = [],

Sources/ContainerRegistry/Tags.swift

@@ -13,10 +13,8 @@
 //===----------------------------------------------------------------------===//
 
 public extension RegistryClient {
-    func getTags(repository: String) async throws -> Tags {
+    func getTags(repository: ImageReference.Repository) async throws -> Tags {
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-tags
-        precondition(repository.count > 0, "repository must not be an empty string")
-
-        return try await executeRequestThrowing(.get(repository, path: "tags/list"), decodingErrors: [.notFound]).data
+        try await executeRequestThrowing(.get(repository, path: "tags/list"), decodingErrors: [.notFound]).data
     }
 }

Sources/containertool/Extensions/Errors+CustomStringConvertible.swift

@@ -46,3 +46,17 @@ extension ContainerRegistry.DistributionErrors: Swift.CustomStringConvertible {
     /// A human-readable string describing a collection of unhandled distribution protocol errors
     public var description: String { errors.map { $0.description }.joined(separator: "\n") }
 }
+
+extension ContainerRegistry.ImageReference.Repository.ValidationError: Swift.CustomStringConvertible {
+    /// A human-readable string describing an image reference validation error
+    public var description: String {
+        switch self {
+        case .emptyString:
+            return "Invalid reference format: repository name cannot be empty"
+        case .containsUppercaseLetters(let rawValue):
+            return "Invalid reference format: repository name (\(rawValue)) must be lowercase"
+        case .invalidReferenceFormat(let rawValue):
+            return "Invalid reference format: repository name (\(rawValue)) contains invalid characters"
+        }
+    }
+}

Sources/containertool/Extensions/RegistryClient+CopyBlobs.swift

@@ -24,9 +24,9 @@ extension RegistryClient {
     /// - Throws: If the copy cannot be completed.
     func copyBlob(
         digest: String,
-        fromRepository sourceRepository: String,
+        fromRepository sourceRepository: ImageReference.Repository,
         toClient destClient: RegistryClient,
-        toRepository destRepository: String
+        toRepository destRepository: ImageReference.Repository
     ) async throws {
         if try await destClient.blobExists(repository: destRepository, digest: digest) {
             log("Layer \(digest): already exists")

Sources/containertool/Extensions/RegistryClient+Layers.swift

@@ -45,7 +45,7 @@ extension RegistryClient {
     // A layer is a tarball, optionally compressed using gzip or zstd
     // See https://github.com/opencontainers/image-spec/blob/main/media-types.md
     func uploadLayer(
-        repository: String,
+        repository: ImageReference.Repository,
         contents: [UInt8],
         mediaType: String = "application/vnd.oci.image.layer.v1.tar+gzip"
     ) async throws -> ImageLayer {