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

Post modified artifacts to PRs for review #137

Open
scottbez1 opened this issue Mar 16, 2021 · 2 comments
Open

Post modified artifacts to PRs for review #137

scottbez1 opened this issue Mar 16, 2021 · 2 comments
Labels
area: tooling Tooling: scripts, continuous integration, etc type: enhancement

Comments

@scottbez1
Copy link
Owner

Set up a workflow_run workflow to download PR artifacts and post a comment back to the PR which shows artifacts that may have changed, as described here: https://securitylab.github.com/research/github-actions-preventing-pwn-requests

Could either run an image diff on the rasterized artifacts compared to the base revision's artifacts, or just review which file paths were modified in the diff and include all artifacts that depend on those files. Image diff increases security risk (e.g. potential malicious image artifact could be manually generated to compromise image diff program) and is more complicated, so probably best to just filter which artifacts to post based on which file paths were modified and accept that this may include artifacts that are unchanged.

@scottbez1 scottbez1 added type: enhancement area: tooling Tooling: scripts, continuous integration, etc labels Mar 16, 2021
@dmadison
Copy link
Contributor

Have you considered using hashing on the rasterized artifacts? So long as the outputs are deterministic that should give you a fast and robust way to determine which artifacts have changed and, to my knowledge at least, should come with no other security implications.

@scottbez1
Copy link
Owner Author

Yeah, that's definitely a possibility, though there are other issues besides raw file type determinism to address with that approach as well: artifact retention time limits (if there hasn't been a master build for a while), dates/commit hashes incorporated in the designs, etc. Not impossible to overcome, just potentially more work to think through and implement.

My planned approach is to implement the quick and "good enough" version based just on modified paths first, which I think should provide ~80% of the benefit even if it's potentially a bit noisy, and then follow up with a more correct version later as time permits or as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tooling Tooling: scripts, continuous integration, etc type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants