ContainerRegistry: Reject invalid image tags and digests (#140)

Motivation
----------

`ImageReference` does not check for illegal characters in parsed image
digests and tags. 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.

Some desktop runtimes accept local image names which the registry will 
reject; other runtimes reject these names even for local names.
`containertool`
now also rejects them.

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

* Check validity of tags and digests when parsing image names
* Change the low-level API functions to accept `Digest` or `Reference`
instead of `String`.

Result
------

It is impossible to create a `Repository` object containing a malformed
tag or digest, because the constructor checks the string value. It is
impossible
to send a malformed name to the registry because the API wrappers only
accept `Digest` or `Reference (Digest | Tag)` objects.

Fixes #139 

Test Plan
---------

Existing tests continue to pass.
New tests exercise additional checks which were previously missing.
Removed tests which checked tags which seemed to be accepted by some
desktop runtimes, but which were not accepted by registries.

Author: euanh

Sources/ContainerRegistry/Blobs.swift

@@ -65,9 +65,7 @@ extension RegistryClient {
 extension HTTPField.Name { static let dockerContentDigest = Self("Docker-Content-Digest")! }
 
 public extension RegistryClient {
-    func blobExists(repository: ImageReference.Repository, digest: String) async throws -> Bool {
-        precondition(digest.count > 0)
-
+    func blobExists(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws -> Bool {
         do {
             let _ = try await executeRequestThrowing(
                 .head(repository, path: "blobs/\(digest)"),
@@ -84,10 +82,8 @@ public extension RegistryClient {
     ///   - digest: Digest of the blob.
     /// - Returns: The downloaded data.
     /// - Throws: If the blob download fails.
-    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(
+    func getBlob(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws -> Data {
+        try await executeRequestThrowing(
             .get(repository, path: "blobs/\(digest)", accepting: ["application/octet-stream"]),
             decodingErrors: [.notFound]
         )
@@ -106,10 +102,10 @@ 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: ImageReference.Repository, digest: String) async throws -> Response {
-        precondition(digest.count > 0, "digest must not be an empty string")
-
-        return try await executeRequestThrowing(
+    func getBlob<Response: Decodable>(repository: ImageReference.Repository, digest: ImageReference.Digest) async throws
+        -> Response
+    {
+        try await executeRequestThrowing(
             .get(repository, path: "blobs/\(digest)", accepting: ["application/octet-stream"]),
             decodingErrors: [.notFound]
         )

Sources/ContainerRegistry/ImageReference.swift

@@ -12,8 +12,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-import RegexBuilder
-
 // 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) {
@@ -30,29 +28,43 @@ func splitReference(_ reference: String) throws -> (String?, String) {
 }
 
 // Split the name into repository and tag parts
-// distribution/distribution defines regular expressions which validate names but these seem to be very strict
-// and reject names which clients accept
-func splitName(_ name: String) throws -> (String, String) {
+// distribution/distribution defines regular expressions which validate names
+// Some clients, such as docker CLI, accept names which violate these regular expressions for local images, but those images cannot be pushed.
+// Other clients, such as podman CLI, reject names which violate these regular expressions even for local images
+func parseName(_ name: String) throws -> (ImageReference.Repository, any ImageReference.Reference) {
     let digestSplit = name.split(separator: "@", maxSplits: 1, omittingEmptySubsequences: false)
-    if digestSplit.count == 2 { return (String(digestSplit[0]), String(digestSplit[1])) }
+    if digestSplit.count == 2 {
+        return (
+            try ImageReference.Repository(String(digestSplit[0])),
+            try ImageReference.Digest(String(digestSplit[1]))
+        )
+    }
 
     let tagSplit = name.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false)
-    if tagSplit.count == 0 { throw ImageReference.ValidationError.unexpected("unexpected error") }
+    if tagSplit.count == 0 {
+        throw ImageReference.ValidationError.unexpected("unexpected error")
+    }
 
-    if tagSplit.count == 1 { return (name, "latest") }
+    if tagSplit.count == 1 {
+        return (try ImageReference.Repository(name), try ImageReference.Tag("latest"))
+    }
 
     // assert splits == 2
-    return (String(tagSplit[0]), String(tagSplit[1]))
+    return (
+        try ImageReference.Repository(String(tagSplit[0])),
+        try ImageReference.Tag(String(tagSplit[1]))
+    )
 }
 
 /// ImageReference points to an image stored on a container registry
+/// This type is not found in the API - it is the reference string given by the user
 public struct ImageReference: Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
     /// The registry which contains this image
     public var registry: String
     /// The repository which contains this image
     public var repository: Repository
-    /// The tag identifying the image.
-    public var reference: String
+    /// The tag or digest identifying the image.
+    public var reference: Reference
 
     public enum ValidationError: Error {
         case unexpected(String)
@@ -65,18 +77,18 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
     /// - Throws: If `reference` cannot be parsed as an image reference.
     public init(fromString reference: String, defaultRegistry: String = "localhost:5000") throws {
         let (registry, remainder) = try splitReference(reference)
-        let (repository, reference) = try splitName(remainder)
+        let (repository, reference) = try parseName(remainder)
         self.registry = registry ?? defaultRegistry
         if self.registry == "docker.io" {
             self.registry = "index.docker.io"  // Special case for docker client, there is no network-level redirect
         }
         // As a special case, official images can be referred to by a single name, such as `swift` or `swift:slim`.
         // 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("/") {
+        if self.registry == "index.docker.io" && !repository.value.contains("/") {
             self.repository = try Repository("library/\(repository)")
         } else {
-            self.repository = try Repository(repository)
+            self.repository = repository
         }
         self.reference = reference
     }
@@ -87,19 +99,19 @@ public struct ImageReference: Sendable, Equatable, CustomStringConvertible, Cust
     ///   - registry: The registry which stores the image data.
     ///   - repository: The repository within the registry which holds the image.
     ///   - reference: The tag identifying the image.
-    init(registry: String, repository: Repository, reference: String) {
+    init(registry: String, repository: Repository, reference: Reference) {
         self.registry = registry
         self.repository = repository
         self.reference = reference
     }
 
+    public static func == (lhs: ImageReference, rhs: ImageReference) -> Bool {
+        "\(lhs)" == "\(rhs)"
+    }
+
     /// Printable description of an ImageReference in a form which can be understood by a runtime
     public var description: String {
-        if reference.starts(with: "sha256") {
-            return "\(registry)/\(repository)@\(reference)"
-        } else {
-            return "\(registry)/\(repository):\(reference)"
-        }
+        "\(registry)/\(repository)\(reference.separator)\(reference)"
     }
 
     /// Printable description of an ImageReference in a form suitable for debugging.
@@ -149,3 +161,98 @@ extension ImageReference {
         }
     }
 }
+
+extension ImageReference {
+    /// Reference refers to an image in a repository.  It can either be a tag or a digest.
+    public protocol Reference: Sendable, CustomStringConvertible, CustomDebugStringConvertible {
+        var separator: String { get }
+    }
+
+    /// Tag is a human-readable name for an image.
+    public struct Tag: Reference, Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
+        var value: String
+
+        public enum ValidationError: Error, Equatable {
+            case emptyString
+            case invalidReferenceFormat(String)
+            case tooLong(String)
+        }
+
+        public init(_ rawValue: String) throws {
+            guard rawValue.count > 0 else {
+                throw ValidationError.emptyString
+            }
+
+            guard rawValue.count <= 128 else {
+                throw ValidationError.tooLong(rawValue)
+            }
+
+            // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
+            let regex = /[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}/
+            if try regex.wholeMatch(in: rawValue) == nil {
+                throw ValidationError.invalidReferenceFormat(rawValue)
+            }
+
+            value = rawValue
+        }
+
+        public static func == (lhs: Tag, rhs: Tag) -> Bool {
+            lhs.value == rhs.value
+        }
+
+        public var separator: String = ":"
+
+        public var description: String {
+            "\(value)"
+        }
+
+        /// Printable description in a form suitable for debugging.
+        public var debugDescription: String {
+            "Tag(\(value))"
+        }
+    }
+
+    /// Digest identifies a specific blob by the hash of the blob's contents.
+    public struct Digest: Reference, Sendable, Equatable, CustomStringConvertible, CustomDebugStringConvertible {
+        var value: String
+
+        public enum ValidationError: Error, Equatable {
+            case emptyString
+            case invalidReferenceFormat(String)
+            case tooLong(String)
+        }
+
+        public init(_ rawValue: String) throws {
+            guard rawValue.count > 0 else {
+                throw ValidationError.emptyString
+            }
+
+            if rawValue.count > 7 + 64 {
+                throw ValidationError.tooLong(rawValue)
+            }
+
+            // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
+            let regex = /sha256:[a-fA-F0-9]{64}/
+            if try regex.wholeMatch(in: rawValue) == nil {
+                throw ValidationError.invalidReferenceFormat(rawValue)
+            }
+
+            value = rawValue
+        }
+
+        public static func == (lhs: Digest, rhs: Digest) -> Bool {
+            lhs.value == rhs.value
+        }
+
+        public var separator: String = "@"
+
+        public var description: String {
+            "\(value)"
+        }
+
+        /// Printable description in a form suitable for debugging.
+        public var debugDescription: String {
+            "Digest(\(value))"
+        }
+    }
+}

Sources/ContainerRegistry/Manifests.swift

@@ -13,12 +13,14 @@
 //===----------------------------------------------------------------------===//
 
 public extension RegistryClient {
-    func putManifest(repository: ImageReference.Repository, reference: String, manifest: ImageManifest) async throws
+    func putManifest(
+        repository: ImageReference.Repository,
+        reference: any ImageReference.Reference,
+        manifest: ImageManifest
+    ) async throws
         -> String
     {
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests
-        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
             .put(
@@ -42,11 +44,11 @@ public extension RegistryClient {
             .absoluteString
     }
 
-    func getManifest(repository: ImageReference.Repository, reference: String) async throws -> ImageManifest {
+    func getManifest(repository: ImageReference.Repository, reference: any ImageReference.Reference) async throws
+        -> ImageManifest
+    {
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
-        precondition(reference.count > 0, "reference must not be an empty string")
-
-        return try await executeRequestThrowing(
+        try await executeRequestThrowing(
             .get(
                 repository,
                 path: "manifests/\(reference)",
@@ -60,10 +62,11 @@ public extension RegistryClient {
         .data
     }
 
-    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(
+    func getIndex(repository: ImageReference.Repository, reference: any ImageReference.Reference) async throws
+        -> ImageIndex
+    {
+        // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests
+        try await executeRequestThrowing(
             .get(
                 repository,
                 path: "manifests/\(reference)",

Sources/ContainerRegistry/RegistryClient+ImageConfiguration.swift

@@ -21,7 +21,8 @@ extension RegistryClient {
     /// - Throws: If the blob cannot be decoded as an `ImageConfiguration`.
     ///
     /// Image configuration records are stored as blobs in the registry.  This function retrieves the requested blob and tries to decode it as a configuration record.
-    public func getImageConfiguration(forImage image: ImageReference, digest: String) async throws -> ImageConfiguration
+    public func getImageConfiguration(forImage image: ImageReference, digest: ImageReference.Digest) async throws
+        -> ImageConfiguration
     {
         try await getBlob(repository: image.repository, digest: digest)
     }

Sources/containertool/Extensions/Errors+CustomStringConvertible.swift

@@ -60,3 +60,31 @@ extension ContainerRegistry.ImageReference.Repository.ValidationError: Swift.Cus
         }
     }
 }
+
+extension ContainerRegistry.ImageReference.Tag.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: tag cannot be empty"
+        case .tooLong(let rawValue):
+            return "Invalid reference format: tag (\(rawValue)) is too long"
+        case .invalidReferenceFormat(let rawValue):
+            return "Invalid reference format: tag (\(rawValue)) contains invalid characters"
+        }
+    }
+}
+
+extension ContainerRegistry.ImageReference.Digest.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: digest cannot be empty"
+        case .tooLong(let rawValue):
+            return "Invalid reference format: digest (\(rawValue)) is too long"
+        case .invalidReferenceFormat(let rawValue):
+            return "Invalid reference format: digest (\(rawValue)) is not a valid digest"
+        }
+    }
+}

Sources/containertool/Extensions/RegistryClient+CopyBlobs.swift

@@ -23,7 +23,7 @@ extension RegistryClient {
     ///   - destRepository: The repository on this registry to which the blob should be copied.
     /// - Throws: If the copy cannot be completed.
     func copyBlob(
-        digest: String,
+        digest: ImageReference.Digest,
         fromRepository sourceRepository: ImageReference.Repository,
         toClient destClient: RegistryClient,
         toRepository destRepository: ImageReference.Repository
@@ -39,6 +39,6 @@ extension RegistryClient {
         log("Layer \(digest): pushing")
         let uploaded = try await destClient.putBlob(repository: destRepository, data: blob)
         log("Layer \(digest): done")
-        assert(digest == uploaded.digest)
+        assert("\(digest)" == uploaded.digest)
     }
 }

Sources/containertool/Extensions/RegistryClient+Layers.swift

@@ -26,13 +26,16 @@ extension RegistryClient {
             return try await getManifest(repository: image.repository, reference: image.reference)
         } catch {
             // Try again, treating the top level object as an index.
-            // This could be more efficient if the exception thrown by getManfiest() included the data it was unable to parse
+            // This could be more efficient if the exception thrown by getManifest() included the data it was unable to parse
             let index = try await getIndex(repository: image.repository, reference: image.reference)
             guard let manifest = index.manifests.first(where: { $0.platform?.architecture == architecture }) else {
                 throw "Could not find a suitable base image for \(architecture)"
             }
             // The index should not point to another index;   if it does, this call will throw a final error to be handled by the caller.
-            return try await getManifest(repository: image.repository, reference: manifest.digest)
+            return try await getManifest(
+                repository: image.repository,
+                reference: ImageReference.Digest(manifest.digest)
+            )
         }
     }