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: validate digest prior to layer download #1597

Merged
merged 6 commits into from
Aug 15, 2024
Merged

Conversation

RTann
Copy link
Collaborator

@RTann RTann commented Aug 9, 2024

No description provided.

Copy link

openshift-ci bot commented Aug 9, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@RTann RTann marked this pull request as ready for review August 9, 2024 23:00
@RTann RTann force-pushed the ross-validate-digest branch from a506129 to bd1bfb4 Compare August 12, 2024 16:28
go.mod Outdated Show resolved Hide resolved
@RTann
Copy link
Collaborator Author

RTann commented Aug 12, 2024

/retest

Copy link
Contributor

@dcaravel dcaravel left a comment

Choose a reason for hiding this comment

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

LGTM - assuming tests pass and the dep updated to a tagged version as mentioned.

@RTann RTann requested a review from dcaravel August 13, 2024 00:09
@RTann
Copy link
Collaborator Author

RTann commented Aug 13, 2024

@dcaravel I updated the PR to add some more validation, as I fear the original was not sufficient. I believe now I cover all bases (fetching both manifests and layers. before was just layers).

pkg/scan/scan.go Show resolved Hide resolved
pkg/scan/scan.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jvdm jvdm left a comment

Choose a reason for hiding this comment

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

LGTM, just curious about why you expected validation outside the function calls? Not a blocker. I just think when this is the case, functions/methods/routines should still protect themselves from callers, but they don't need to support the invalid input by returning an error -- since this is a programmer's kinda error, they could just enforce and panic (like some standard library methods do for invalid input that cannot be handled in runtime).

pkg/scan/scan.go Outdated Show resolved Hide resolved
@RTann RTann force-pushed the ross-validate-digest branch from 30a0090 to bf8ef9f Compare August 15, 2024 01:02
@RTann RTann enabled auto-merge (squash) August 15, 2024 19:38
@RTann RTann merged commit d4b374d into master Aug 15, 2024
18 of 21 checks passed
@RTann RTann deleted the ross-validate-digest branch August 15, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants