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

Lint our GitHub Actions workflows with zizmor #215

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Lint our GitHub Actions workflows with zizmor #215

merged 1 commit into from
Jan 9, 2025

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Jan 3, 2025

Status

Ready for review

Description

We just need to set persist-credentials: false in all of our workflows.

Introduce the standard make lint target that runs all of our linters.

Refs <freedomofpress/securedrop-tooling#18>.

We just need to set persist-credentials: false in all of our workflows.

Introduce the standard `make lint` target that runs all of our linters.

Refs <freedomofpress/securedrop-tooling#18>.
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Thanks, @legoktm. This follows the precedent established in other pull requests towards freedomofpress/securedrop-tooling#18 and runs as expected.

Do you think any of these checks, currently disabled in our environment, are worth enabling in follow-up work?

2025-01-03T18:04:43.735635Z  WARN zizmor: skipping impostor-commit: can't run without a GitHub API token
2025-01-03T18:04:43.735660Z  WARN zizmor: skipping ref-confusion: can't run without a GitHub API token
2025-01-03T18:04:43.735666Z  WARN zizmor: skipping known-vulnerable-actions: can't run without a GitHub API token

@cfm cfm added this pull request to the merge queue Jan 9, 2025
Merged via the queue into main with commit 470ac85 Jan 9, 2025
4 checks passed
@legoktm
Copy link
Member Author

legoktm commented Jan 9, 2025

Do you think any of these checks, currently disabled in our environment, are worth enabling in follow-up work?

I would like to enable them, but since they need a GitHub API token, then we're adding a new secret to CI, which means we've expanded our CI attack surface :/

I'm kind of hoping that woodruffw/zizmor#278 will address some of the things that currently need the GH API.

@legoktm legoktm deleted the zizmor branch January 9, 2025 21:26
@cfm
Copy link
Member

cfm commented Jan 9, 2025

Yep, that's what I was wondering about! Thanks for the pointer.

@woodruffw
Copy link

Hi! I saw this backlinked from woodruffw/zizmor#278; thanks for trying out zizmor!

FWIW, you don't need to configure a new secret in CI to use zizmor's online features: you can use the default secrets.GITHUB_TOKEN, which you can even scope down to permissions: {} (i.e. no permissions needed at all). The main reason zizmor needs a token is to avoid getting throttled with rate limits, not because it needs any special access to anything 🙂

(Long term the plan is to move to the static API however, so this will certainly become less relevant!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants