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

Separate out stabilization into configurable passes #132

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

msuozzo
Copy link
Member

@msuozzo msuozzo commented Oct 24, 2024

Working towards a solution for #39

@msuozzo msuozzo requested a review from wbxyz October 24, 2024 18:07
@msuozzo msuozzo changed the title Separate out normalization into configurable passes Separate out stabilization into configurable passes Oct 25, 2024
@msuozzo msuozzo force-pushed the push-znokvppnllot branch 3 times, most recently from f98b326 to 38c8171 Compare October 25, 2024 18:25
Copy link
Member

@wbxyz wbxyz left a comment

Choose a reason for hiding this comment

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

This is cool! It's really helpful to have the different considerations separated out and encapsulated with a description.

Do you think eventually we'll be able to tell which stabilizers actually made modifications? I was imagining the stabilizers could return a modified copy instead of modifying in place. That way the invocation of the stabilizer can compare before-after, but that would be expensive to copy the file on every stabilizer invocation.

I guess another option would be for the stabilizers to return a bool of whether they made a modification. It would be more efficient but potentially less robust and more complex.

docs/builds/ArtifactEquivalence@v0.1.md Outdated Show resolved Hide resolved
internal/verifier/attestation.go Outdated Show resolved Hide resolved
pkg/archive/tar.go Outdated Show resolved Hide resolved
pkg/archive/tar_test.go Outdated Show resolved Hide resolved
pkg/archive/zip.go Outdated Show resolved Hide resolved
pkg/archive/zip.go Outdated Show resolved Hide resolved
pkg/archive/zip.go Outdated Show resolved Hide resolved
@msuozzo
Copy link
Member Author

msuozzo commented Oct 28, 2024

Do you think eventually we'll be able to tell which stabilizers actually made modifications?

Yeah it's certainly possible but I don't think it's a particular priority. The current way, we provide the document the passes we do apply at a given time. Next, we would have to provide feedback from both the rebuilt and upstream artifacts, take the super-set of the applied passes, and report those. Even that could be confusing given we'd report the same set of passes for both while the changes would often just reflect variability in the upstream artifact's metadata.

Further, this wouldn't necessarily provide valuable feedback to our users or to upstream maintainers. If our argument is that these differences aren't security-critical, diverting time/resources to resolving them isn't productive.

pkg/archive/zip.go Outdated Show resolved Hide resolved
pkg/archive/zip.go Outdated Show resolved Hide resolved
@wbxyz
Copy link
Member

wbxyz commented Oct 28, 2024

I think I was thinking about #83 but agreed that it's not high priority at the moment.

@msuozzo
Copy link
Member Author

msuozzo commented Oct 28, 2024

I think I was thinking about #83 but agreed that it's not high priority at the moment.

Yeah I read that, as well. I think that's more the "next step" as I was saying and less a near-term priority.

@msuozzo msuozzo force-pushed the push-znokvppnllot branch 2 times, most recently from 1f933db to 18f3f90 Compare October 29, 2024 16:20
@msuozzo msuozzo merged commit 15e6dea into google:main Oct 29, 2024
3 checks passed
@msuozzo msuozzo deleted the push-znokvppnllot branch October 29, 2024 16:40
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.

2 participants