Skip to content

Conversation

@fghanmi
Copy link
Collaborator

@fghanmi fghanmi commented Nov 6, 2025

Summary by Sourcery

Add support for listing image cryptographic signatures and signed attestations in the GetImageMetadata endpoint

New Features:

  • Extend the image metadata API to include lists of cryptographic signatures and signed attestations in the response

Enhancements:

  • Implement getImageSignatures and getImageAttestations functions to fetch and dedupe signature (.sig) and attestation (.att) manifests using crane and v1 APIs
  • Add error constants ErrArtifactFailedToFetchSignatures and ErrArtifactFailedToFetchAttestations for signature and attestation retrieval failures

Documentation:

  • Update the OpenAPI spec to document the new signatures and attestations fields in ImageMetadataResponse

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 6, 2025

Reviewer's Guide

This PR enhances the ImageMetadata endpoint to list cryptographic signatures and signed attestations by introducing helper functions that fetch and dedupe manifests via the go-containerregistry APIs, integrates these lists into the service response, and updates associated errors, models, and the OpenAPI spec.

Sequence diagram for fetching image signatures and attestations in GetImageMetadata

sequenceDiagram
participant S as "artifactService"
participant G as "getImageSignatures()"
participant A as "getImageAttestations()"
S->>G: getImageSignatures(digest, ref)
G-->>S: signatureList ([]string)
S->>A: getImageAttestations(digest, ref)
A-->>S: attestationList ([]string)
S->>S: Build ImageMetadataResponse with signatures and attestations
Loading

ER diagram for updated ImageMetadataResponse data structure

erDiagram
IMAGE_METADATA_RESPONSE {
  string digest
  string[] signatures
  string[] attestations
  Metadata metadata
}
METADATA {
}
IMAGE_METADATA_RESPONSE ||--|| METADATA : contains
Loading

Class diagram for updated ImageMetadataResponse model

classDiagram
class ImageMetadataResponse {
  +string Digest
  +Metadata Metadata
  +string[] Signatures
  +string[] Attestations
}
class Metadata {
  ...
}
ImageMetadataResponse --> Metadata
Loading

File-Level Changes

Change Details Files
Integrate signature and attestation retrieval in GetImageMetadata
  • Invoke getImageSignatures and getImageAttestations in GetImageMetadata
  • Wrap fetch errors using new console_errors types
  • Assign Signatures and Attestations fields on the response
internal/services/artifact.go
Add helper functions to fetch image signatures and attestations
  • Import crane, name and v1 packages
  • Implement getImageSignatures to construct .sig tag, fetch and parse manifest, dedupe layer digests
  • Implement getImageAttestations to construct .att tag, fetch and parse manifest, dedupe layer digests
internal/services/artifact.go
Define new error types for signature and attestation fetch failures
  • Add ErrArtifactFailedToFetchSignatures constant
  • Add ErrArtifactFailedToFetchAttestations constant
internal/errors/errors.go
Extend OpenAPI spec with signatures and attestations fields
  • Add signatures array property to ImageMetadataResponse schema
  • Add attestations array property to ImageMetadataResponse schema
internal/api/openapi/rhtas-console.yaml
Update ImageMetadataResponse model to include signatures and attestations
  • Add Signatures *[]string field with JSON tag
  • Add Attestations *[]string field with JSON tag
internal/models/models.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 6, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing auditing: Newly added critical actions fetching signatures and attestations are not accompanied by
any audit logging of the action, user, or outcome.

Referred Code
signatureList, err := getImageSignatures(digest, ref)
if err != nil {
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}

attestationList, err := getImageAttestations(digest, ref)
if err != nil {
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}

response := models.ImageMetadataResponse{
	Image: &image,
	Metadata: models.Metadata{
		MediaType: string(descriptor.MediaType),
		Size:      descriptor.Size,
		Created:   createdTime,
		Labels:    &labels,
	},
	Digest:       digest.String(),
	Signatures:   &signatureList,
	Attestations: &attestationList,


 ... (clipped 1 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Error wrapping: Errors from fetching signatures/attestations are wrapped with generic messages but do not
distinguish not-found vs absent artifacts and may surface low-level details without
explicit handling of empty/optional cases.

Referred Code
signatureList, err := getImageSignatures(digest, ref)
if err != nil {
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}

attestationList, err := getImageAttestations(digest, ref)
if err != nil {
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Detailed errors: Returned errors include underlying error text via fmt.Errorf("%w: %v", ...)
which could expose internal details if propagated to users.

Referred Code
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}

attestationList, err := getImageAttestations(digest, ref)
if err != nil {
	return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
}
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input handling: New functions construct remote references and pull manifests without visible validation or
authorization checks for external inputs and do not handle the case where
signatures/attestations are intentionally absent.

Referred Code
// getImageSignatures returns the list of unique cryptographic signatures associated to the imageDigest
func getImageSignatures(imageDigest v1.Hash, ref name.Reference) ([]string, error) {
	digest := ref.Context().Digest(imageDigest.String())
	h, err := v1.NewHash(digest.Identifier())
	if err != nil {
		return nil, fmt.Errorf("error getting hash: %w", err)
	}
	// Construct the signature reference - sha256-<hash>.sig
	sigTag := digest.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".sig"))
	// Get the manifest of the signature
	mf, err := crane.Manifest(sigTag.Name())
	if err != nil {
		return nil, fmt.Errorf("error getting signature manifest: %w", err)
	}
	sigManifest, err := v1.ParseManifest(bytes.NewReader(mf))
	if err != nil {
		return nil, fmt.Errorf("error parsing signature manifest: %w", err)
	}
	signatureList := []string{}
	seen := make(map[string]struct{}) // to track signature duplicates



 ... (clipped 40 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 6, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Return signature/attestation content, not digests

The implementation should be updated to return the actual content of signatures
and attestations, rather than just their digests. This change would align the
implementation with the OpenAPI specification and provide more useful data to
API consumers.

Examples:

internal/services/artifact.go [235-241]
	for _, layer := range sigManifest.Layers {
		digestStr := layer.Digest.String()
		if _, exists := seen[digestStr]; !exists {
			seen[digestStr] = struct{}{}
			signatureList = append(signatureList, digestStr)
		}
	}

Solution Walkthrough:

Before:

func getImageSignatures(imageDigest, ref) ([]string, error) {
  // ... construct signature tag and get manifest
  mf, err := crane.Manifest(sigTag.Name())
  // ...
  sigManifest, err := v1.ParseManifest(bytes.NewReader(mf))
  // ...
  signatureList := []string{}
  
  for _, layer := range sigManifest.Layers {
    // Appends the digest of the layer, not its content
    digestStr := layer.Digest.String()
    signatureList = append(signatureList, digestStr)
  }
  return signatureList, nil
}

After:

func getImageSignatures(imageDigest, ref) ([]string, error) {
  // ... construct signature tag and get manifest
  mf, err := crane.Manifest(sigTag.Name())
  // ...
  sigManifest, err := v1.ParseManifest(bytes.NewReader(mf))
  // ...
  signatureList := []string{}
  
  for _, layer := range sigManifest.Layers {
    // Fetches the actual layer content
    layerContent, err := fetchLayerContent(sigTag, layer.Digest)
    // Encodes content to string (e.g., base64)
    contentStr := encodeToString(layerContent)
    signatureList = append(signatureList, contentStr)
  }
  return signatureList, nil
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw where the implementation returns layer digests instead of the actual content, which contradicts the API specification's description and severely limits the feature's utility.

High
General
Refactor duplicated code into one function

Refactor the duplicated logic in getImageSignatures and getImageAttestations
into a single getImageArtifacts function. Additionally, handle "not found"
errors gracefully by returning an empty list instead of failing.

internal/services/artifact.go [214-272]

-// getImageSignatures returns the list of unique cryptographic signatures associated to the imageDigest
-func getImageSignatures(imageDigest v1.Hash, ref name.Reference) ([]string, error) {
+// getImageArtifacts returns the list of unique artifacts (signatures or attestations) associated with an image digest.
+func getImageArtifacts(imageDigest v1.Hash, ref name.Reference, tagSuffix string) ([]string, error) {
 	digest := ref.Context().Digest(imageDigest.String())
 	h, err := v1.NewHash(digest.Identifier())
 	if err != nil {
 		return nil, fmt.Errorf("error getting hash: %w", err)
 	}
-	// Construct the signature reference - sha256-<hash>.sig
-	sigTag := digest.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".sig"))
-	// Get the manifest of the signature
-	mf, err := crane.Manifest(sigTag.Name())
+
+	// Construct the artifact reference, e.g., sha256-<hash>.sig
+	artifactTag := digest.Context().Tag(fmt.Sprintf("%s-%s.%s", h.Algorithm, h.Hex, tagSuffix))
+
+	// Get the manifest of the artifact
+	mf, err := crane.Manifest(artifactTag.Name())
 	if err != nil {
-		return nil, fmt.Errorf("error getting signature manifest: %w", err)
+		if isNotFound(err) {
+			return []string{}, nil // No artifacts found, return empty list
+		}
+		return nil, fmt.Errorf("error getting %s manifest: %w", tagSuffix, err)
 	}
-	sigManifest, err := v1.ParseManifest(bytes.NewReader(mf))
+
+	manifest, err := v1.ParseManifest(bytes.NewReader(mf))
 	if err != nil {
-		return nil, fmt.Errorf("error parsing signature manifest: %w", err)
+		return nil, fmt.Errorf("error parsing %s manifest: %w", tagSuffix, err)
 	}
-	signatureList := []string{}
-	seen := make(map[string]struct{}) // to track signature duplicates
 
-	for _, layer := range sigManifest.Layers {
+	var artifactList []string
+	seen := make(map[string]struct{}) // to track duplicates
+
+	for _, layer := range manifest.Layers {
 		digestStr := layer.Digest.String()
 		if _, exists := seen[digestStr]; !exists {
 			seen[digestStr] = struct{}{}
-			signatureList = append(signatureList, digestStr)
+			artifactList = append(artifactList, digestStr)
 		}
 	}
-	return signatureList, nil
+	return artifactList, nil
+}
+
+// getImageSignatures returns the list of unique cryptographic signatures associated to the imageDigest
+func getImageSignatures(imageDigest v1.Hash, ref name.Reference) ([]string, error) {
+	return getImageArtifacts(imageDigest, ref, "sig")
 }
 
 // getImageAttestations returns the unique list of signed attestations associated with the imageDigest
 func getImageAttestations(imageDigest v1.Hash, ref name.Reference) ([]string, error) {
-	digest := ref.Context().Digest(imageDigest.String())
-	h, err := v1.NewHash(digest.Identifier())
-	if err != nil {
-		return nil, fmt.Errorf("error getting hash: %w", err)
-	}
-	// Construct the attestation reference - sha256-<hash>.att
-	attTag := digest.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".att"))
-	// Get the manifest of the attestation
-	mf, err := crane.Manifest(attTag.Name())
-	if err != nil {
-		return nil, fmt.Errorf("error getting attestation manifest: %w", err)
-	}
-	sigManifest, err := v1.ParseManifest(bytes.NewReader(mf))
-	if err != nil {
-		return nil, fmt.Errorf("error parsing attestation manifest: %w", err)
-	}
-	attestationList := []string{}
-	seen := make(map[string]struct{}) // to track attestation duplicates
-
-	for _, layer := range sigManifest.Layers {
-		digestStr := layer.Digest.String()
-		if _, exists := seen[digestStr]; !exists {
-			seen[digestStr] = struct{}{}
-			attestationList = append(attestationList, digestStr)
-		}
-	}
-	return attestationList, nil
+	return getImageArtifacts(imageDigest, ref, "att")
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies significant code duplication and proposes a sound refactoring that improves maintainability. It also introduces a crucial improvement by gracefully handling "not found" errors, which prevents failures when signatures or attestations are absent.

Medium
Possible issue
Use correct error for attestations

In the error handling for getImageAttestations, replace
ErrArtifactFailedToFetchSignatures with the correct
ErrArtifactFailedToFetchAttestations for accurate error reporting.

internal/services/artifact.go [194-197]

 	attestationList, err := getImageAttestations(digest, ref)
 	if err != nil {
-		return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchSignatures, err)
+		return models.ImageMetadataResponse{}, fmt.Errorf("%w: %v", console_errors.ErrArtifactFailedToFetchAttestations, err)
 	}
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a copy-paste bug where the wrong error constant is used, which would result in misleading error messages and make debugging more difficult.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant