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

Add VirusTotal scanning of bootloaders to CI #8408

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

danyeaw
Copy link
Contributor

@danyeaw danyeaw commented Apr 14, 2024

Closes #8372 by matrix building all the Windows bootloaders and uploading them to VirusTotal for each push to the develop branch.

@danyeaw danyeaw force-pushed the virus-check branch 2 times, most recently from bcb6e66 to b0a472f Compare April 14, 2024 14:40
@bwoodsend
Copy link
Member

If you remember, can you add [skip ci] to your commit message title so that the main CI test job isn't spinning away on a change that doesn't affect it.

@danyeaw
Copy link
Contributor Author

danyeaw commented Apr 14, 2024

@bwoodsend It looks like [skip ci] disables all workflows, so I'll just temporarily disable the main CI workflow and drop the commit later

@danyeaw danyeaw force-pushed the virus-check branch 5 times, most recently from cb125d7 to 1b4e82f Compare April 14, 2024 17:31
@bwoodsend
Copy link
Member

I've added a VIRUSTOTAL_API_KEY to this repo's secrets although, since secrets are hidden for PRs from forks, the on: pull_request trigger is effectively a no go. Perhaps a workflow_dispatch (for verifying) plus a schedule trigger would be better? Then you could add your own key to your fork, trigger it there and post a link to the job here?

@danyeaw
Copy link
Contributor Author

danyeaw commented Apr 14, 2024

Hi @bwoodsend, thanks for creating the secret!

I was thinking once this is tested we make it only on push to develop, but I'm good with workflow_dispatch and a schedule. Maybe we should run it nightly?

@danyeaw danyeaw force-pushed the virus-check branch 2 times, most recently from ccd3357 to 9c20b48 Compare April 14, 2024 18:20
@danyeaw
Copy link
Contributor Author

danyeaw commented Apr 14, 2024

Workflow run completed successfully: https://github.com/danyeaw/pyinstaller/actions/runs/8680718643/job/23801928189
It looks like you can only do workflow_dispatch on the main branch of a repo, so I merged these changes in to my fork's develop branch and I can force push the develop branch later to sync back up.

@bwoodsend
Copy link
Member

Is there a reason for the exact pins on actions/{download,upload}-artifact? Looks good otherwise.

@danyeaw
Copy link
Contributor Author

danyeaw commented Apr 14, 2024

@bwoodsend I am actually used to pinning to the hash for security, but pins to the major versions updated to be consistent. Thanks for the feedback!

@rokm
Copy link
Member

rokm commented Apr 15, 2024

The workflow looks good to me, but I'm not sure about the news fragment. If this will appear under "Features" section under PyInstaller's release changelog, it will likely give incorrect impression that this is a build-time feature.

There is a process news category, but I think we should do without news fragment at all, to minimize confusion.

@danyeaw
Copy link
Contributor Author

danyeaw commented Apr 16, 2024

Hi @rokm, thanks for the feedback! I dropped the news fragment 👍

@rokm rokm merged commit ca1ec5b into pyinstaller:develop Apr 16, 2024
17 of 18 checks passed
@bwoodsend
Copy link
Member

Thanks again

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.

Upload PyInstaller Bootloaders to VirusTotal Automatically
3 participants