search

scottbez1 / splitflap

DIY split-flap display
https://scottbez1.github.io/splitflap
Other
3.09k stars 257 forks source link

Post modified artifacts to PRs for review #137

Open scottbez1 opened 3 years ago

scottbez1 commented 3 years ago

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.

dmadison commented 3 years ago

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 commented 3 years ago

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.