Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: openvex report oci id bug #928

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robert-cronin
Copy link
Contributor

Summary of changes

Adds logic to qualify the actual SHA of the patched image
Changes patch.go to uses the correct pURL format (e.g. nginx@sha256:...) for the namespace for oci container names
Falls back to patchedImageName if digest not found.

Fixes #667

Validation

Vex report output from a test run on nginx, the tail of the generated vex report:
image

@robert-cronin robert-cronin changed the title Fix openvex report oci id bug fix: openvex report oci id bug Feb 26, 2025
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 54.54545% with 15 lines in your changes missing coverage. Please review.

Project coverage is 48.25%. Comparing base (d13288e) to head (d9c1031).

Files with missing lines Patch % Lines
pkg/patch/patch.go 54.54% 14 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #928      +/-   ##
==========================================
+ Coverage   48.12%   48.25%   +0.13%     
==========================================
  Files          18       18              
  Lines        2261     2292      +31     
==========================================
+ Hits         1088     1106      +18     
- Misses       1114     1126      +12     
- Partials       59       60       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ashnamehrotra ashnamehrotra left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! Can we update the golang.org/x/crypto package for vuln-check and fix lint errs?

@sozercan
Copy link
Member

i think linter failure is due to incompat between go 1.24 and golang-ci lint version. opened #930 to bump linter

@sozercan sozercan requested a review from Copilot February 26, 2025 23:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR addresses the openvex report OCI ID bug by adding logic to qualify the patched image’s SHA and use the correct image format for generating the report.

  • Added logic to determine the repository name with digest using a new helper function.
  • Updated vex document generation to use the qualified image name.
  • Modified the error handling in patchWithContext to explicitly capture and return errors.

Reviewed Changes

File Description
pkg/patch/patch.go Introduces a new helper (getRepoNameWithDigest) to derive image names with digest and updates usage in vex report generation.

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

pkg/patch/patch.go:403

  • [nitpick] Consider adding tests for the new getRepoNameWithDigest function to verify its behavior in both success and failure cases.
// e.g. "docker.io/library/nginx:1.21.6-patched"

@robert-cronin robert-cronin force-pushed the fix/openvex-report-oci-id branch from 3bef974 to 21cc964 Compare February 27, 2025 01:23
Signed-off-by: robert-cronin <[email protected]>
@robert-cronin robert-cronin force-pushed the fix/openvex-report-oci-id branch from 21cc964 to d9c1031 Compare February 27, 2025 02:11

// e.g. "docker.io/library/nginx:1.21.6-patched".
func getRepoNameWithDigest(ctx context.Context, patchedImageName string) (string, error) {
cmd := execCommandContext(ctx, "docker", "image", "inspect",
Copy link
Member

Choose a reason for hiding this comment

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

can we do this without docker cli?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m wondering if we could make use of the BuildKit exporter response, since it should include the digest of the patched image. The challenge is that this information only becomes available once we’ve finished generating the VEX report, because everything happens within the same build command. I’d really appreciate any tips or guidance on how to make that work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

[BUG] fix openvex report oci id
3 participants