Skip to content

MAINT: Audit CIs using zizmor - #13011

Merged
drammock merged 3 commits into
mne-tools:mainfrom
larsoner:zizmor
Dec 10, 2024
Merged

MAINT: Audit CIs using zizmor#13011
drammock merged 3 commits into
mne-tools:mainfrom
larsoner:zizmor

Conversation

@larsoner

@larsoner larsoner commented Dec 6, 2024

Copy link
Copy Markdown
Member

Not 100% sure about adding this as a pre-commit hook because at least on latest Ubuntu 24.10 I needed a newer rust than was installed on the system, but it was pretty easy to install. An alternative would be to add it as a GitHub action.

@larsoner

larsoner commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

Opened zizmorcore/zizmor#236 about pre-commit.ci, we'll see what they say...

@drammock drammock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need to first decide whether there's value in running locally via pre-commit. If not, then we might as well just run it as a GHA instead of through pre-commit.ci.

I don't like the idea of all contributors (not just maintainers!) needing to install or update Rust just to make a change to MNE. That alone sways me pretty strongly toward GHA / running zizmor only in cloud. The only hesitation I have is that, if someone did introduce a security hole into one of our GHAs, it would be nice to catch that hole locally before it got pushed into a PR and executed.

@larsoner

larsoner commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

Actually they responded to the issue and might try to make it Rust 1.75-compatible, which would work with at least Ubuntu 22.04+. Not sure about macOS or Windows, though.

Could add it to the Style GHA run I suppose

Comment thread .pre-commit-config.yaml Outdated
@graingert

Copy link
Copy Markdown
Contributor

I don't like the idea of all contributors (not just maintainers!) needing to install or update Rust just to make a change to MNE. That alone sways me pretty strongly toward GHA / running zizmor only in cloud. The only hesitation I have is that, if someone did introduce a security hole into one of our GHAs, it would be nice to catch that hole locally before it got pushed into a PR and executed.

no need to install rust anymore! It's a regular python hook - suspiciously similar to ruff

Comment thread .pre-commit-config.yaml Outdated
Co-authored-by: Thomas Grainger <tagrain@gmail.com>
@larsoner larsoner added this to the 1.9 milestone Dec 9, 2024
@drammock
drammock merged commit b329515 into mne-tools:main Dec 10, 2024
@drammock

Copy link
Copy Markdown
Member

thanks @larsoner and @graingert !

@drammock
drammock deleted the zizmor branch December 10, 2024 23:30
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