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 GHA workflows #18

Open
1 of 14 tasks
legoktm opened this issue Dec 11, 2024 · 2 comments
Open
1 of 14 tasks

Lint our GHA workflows #18

legoktm opened this issue Dec 11, 2024 · 2 comments

Comments

@legoktm
Copy link
Member

legoktm commented Dec 11, 2024

We don't really have any credentials in CI, the main thing is that the client and workstation repos have push tokens for apt-test/yum-test/build-logs (the last one is probably the most sensitive).

https://woodruffw.github.io/zizmor/ is one I had seen previously on lobsters and now it's mentioned on https://blog.pypi.org/posts/2024-12-11-ultralytics-attack-analysis/ so we should give it a shot.

It's written in Rust and the recommendation is to cargo install it, so we can do something like https://github.com/freedomofpress/securedrop/blob/develop/.github/workflows/cargo-vet.yml

Primary repositories

Docs repos

  • securedrop-dev-docs
  • securedrop-docs
  • securedrop-workstation-docs

Package repositories

n.b. these don't have a Python toolchain setup

  • securedrop-yum-test
  • securedrop-yum-prod
  • securedrop-apt-test
  • securedrop-apt-prod

Misc

  • kernel-builder
  • securedrop-supply-chain
@legoktm legoktm moved this to Ready to go in SecureDrop dev cycle Dec 11, 2024
legoktm added a commit to freedomofpress/securedrop-client that referenced this issue Dec 12, 2024
zizmor is a new tool to lint GitHub Actions workflows. For the most part
our workflows are pretty low risk since we don't give it a bunch of
credentials, but we can avoid issues in the future by locking them down
now.

Two overall issues needed to be fixed:
* setting persist-credentials: false for actions/checkout, which we do
everywhere except in the workflows that need to push.
* Don't use template expansion when we can use a normal bash variable.

While zizmor is written in Rust, it is also shipped as a prebuilt binary
via PyPI, so we can set it as a poetry dependency and run it as part of
our normal lint CI.

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

legoktm commented Dec 12, 2024

I submitted an initial PR for securedrop-client since that repo isn't currently going through a release. Overall I found the explanations at https://woodruffw.github.io/zizmor/audits/ pretty useful in figuring out the solution. I think it's silly that actions/checkout does the insecure thing by default.

One interesting part of zizmor is the "personas" configuration it has: https://woodruffw.github.io/zizmor/usage/#using-personas. When I use --persona=pedantic it just additionally flags our two uses of persist-credentials: true (OK) and every instance of:

help[unpinned-uses]: unpinned action reference
  --> /home/user/github/freedomofpress/securedrop-client/.github/workflows/security.yml:12:9
   |
12 |       - uses: actions/checkout@v4
   |         ------------------------- help: action is not pinned to a hash ref
   |
   = note: audit confidence → High

which I understand is a security risk in theory, but I don't think hash pinning is anything other than security theater since we're not even auditing the actions or checking what's changed. IMO it's more important that we only use actions we trust (i.e. official GitHub ones and similar).

So I think sticking with the default persona is probably how we should go with to begin.

It's written in Rust and the recommendation is to cargo install it

It's also built and published to PyPI as documented in https://woodruffw.github.io/zizmor/usage/#use-in-github-actions, so I think that's an easier way for us to integrate for now, and freedomofpress/securedrop-client#2331 demonstrates how trivial it was to add to poetry.

P.S. It feels kind of stupid to add this type of linting to literally every repository we maintain that has CI, but at the same time once I think of them as just code instead of CI manifests, it seems more logical that all the code we use/ship should be linted. 🤷🏾

legoktm added a commit to freedomofpress/securedrop-workstation that referenced this issue Dec 20, 2024
zizmor is a new tool to lint GitHub Actions workflows. For the most part
our workflows are pretty low risk since we don't give it a bunch of
credentials, but we can avoid issues in the future by locking them down
now.

The only issue that needed fixing was setting persist-credentials: false
for actions/checkout, which we do everywhere except in the workflows
that need to push.

While zizmor is written in Rust, it is also shipped as a prebuilt binary
via PyPI, so we can set it as a poetry dependency and run it as part of
our normal lint CI.

Refs <freedomofpress/securedrop-tooling#18>.
legoktm added a commit to freedomofpress/securedrop that referenced this issue Dec 20, 2024
zizmor is a new tool to lint GitHub Actions workflows. For the most part
our workflows are pretty low risk since we don't give it a bunch of
credentials, but we can avoid issues in the future by locking them down
now.

Two overall issues needed to be fixed:
* setting persist-credentials: false for actions/checkout, which we do
everywhere except in the workflows that need to push.
* Don't use template expansion when we can use a normal bash variable.

While zizmor is written in Rust, it is also shipped as a prebuilt binary
via PyPI, so we can set it as a pip dependency and run it as part of
our normal lint CI.

Refs <freedomofpress/securedrop-tooling#18>.
@legoktm legoktm moved this from Ready to go to In Progress in SecureDrop dev cycle Dec 21, 2024
@legoktm
Copy link
Member Author

legoktm commented Dec 21, 2024

Thanks to Ro for reviewing and approving the client PR, I've submitted two more PRs for the workstation and server repos, and created a checklist in the issue description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

No branches or pull requests

1 participant