Github action security hardening post-Ultralytics

[tim-schilling]

Hi all,

I was reading this update on the Ultralytics incident (https://blog.yossarian.net/2024/12/06/zizmor-ultralytics-injection) and I had a few thoughts/questions:

1. Should we enforce a limitation on what github actions all repos can use?
2. Should we adopt zizmor for our Django Commons top level actions in membership and controls?
3. Should we create a action that redefined zizmors default action (https://woodruffw.github.io/zizmor/usage/#use-in-github-actions) to make it easier for repos to be up to date?
4. Should we adopt zizmor into our pre-commit configs?

This exploit confirms our organization fits the profile of a target since it is designed around github actions and transparency.

[tim-schilling]

cc: @django-commons/admins @django-commons/security-team

[matthiask]

Thanks, I didn't know about this incident.

I'm not 100% sure what 1) would imply or if it would help or if it would restrict what people can do in django-commons. That said, if there's an easy way to enforce this and if we have a lightweight process around extending the allowlist I'd be fine with that.

2. sounds like a good idea.

3. could be a good idea, but we also have a level of indirection and some additional attack surface. That being said, repeating the same config everywhere is also error prone.

4. do we want people to run zizmor locally? Is it a trusted source? I think the bar should be even higher, since CI runners are a lower quality target than people's workstations.

[matthiask]

I have been browsing through https://lobste.rs/s/btagmw/maliciously_crafted_github_branch_name and have found:

https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git

It seems the problem was that the GitHub workflow allowed running PR tests with the target repository's secrets. A thing to look out for.

[marksweb]

Yeah that's it, pull_request_target essentially caused the issue and allowed initial access from what I've seen.

It's one to watch out for but not one you see often or need usually.

[tim-schilling]

GitHub allows you to allow-list an org's actions or a specific action (GitHub docs). Here's a screenshot of the area:

I do think it's feasible to configure it so that well-known providers are on the allow-list. Then any others would need to go through a request/approval process.

[Stormheg]

I'd rather not restrict what actions can be run. That sounds like extra workload for the admin team to investigate and approve and an annoyance for maintainers. The security benefit is not clear to me to justify all this.

[tim-schilling]

The security benefit would be slowing a person down and revisiting what they are trying to do so that other options could be considered when dealing with high risk areas.

[bckohan]

Yikes, CI injection by git branch name has me 🤯

• Should we enforce a limitation on what github actions all repos can use?

No, I think maintaining a allow list would be too much work and be too annoying to maintainers.

• Should we adopt zizmor for our Django Commons top level actions in membership and controls?

Yes!

• Should we create a action that redefined zizmors default action (https://woodruffw.github.io/zizmor/usage/#use-in-github-actions) to make it easier for repos to be up to date?

Yes, but see below.

• Should we adopt zizmor into our pre-commit configs?

If its fast enough!

---

I think we should strive for the confidence that a django-commons project is less vulnerable to a supply chain attack. I think this unfortunately necessitates a more active role for the security team because part of this story has to be offloading a little bit of work from maintainers. The good news is that I think there's a lot of low hanging fruit we can automate.

I propose the following:

1. Start a security repository.
2. Write a canonical SECURITY.md for this repo that contains the minimum required security policy for a django-commons project. I propose the following:
   • Private vulnerability reporting should be enabled.
   • Secret scanning should be enabled
   • Security advisories should be enabled
   • I don't think dependabot alerts need be enabled because these are middle-stream apps. It may make sense for some libraries that incorporate a dependency on popular third party libraries other than Django.
   • I don't think code scanning needs to be enabled.
   • Repos should have a SECURITY.md that at a minimum contains a link to file a private report.
3. Schedule a nightly github action that runs zizmor on the actions of all django-commons repositories and opens/maintains a private security ticket on the repos that have findings.

Doing it this way means that once your repo is onboarded at least you and the security team will automatically be apprised of any zizmor findings. No need to do per-repository work to add CI/CD scanning. I'd also imagine GitHub is going to incorporate CI/CD scanning into its security portfolio at some point at which point we can probably just adjust the security policy to say that must be enabled.

I'd be happy to take a stab at this if ya'll think its a good idea.

[tim-schilling]

I think I disagree regarding creating a security repository. The majority of vulnerabilities that will exist will require deep knowledge of the application, therefore the repository's admins should be the ones in charge of facilitation. There is the security team that can be brought in for outside help, but they shouldn't be relied on for purely logistical help.

If we're using the security repo to run private actions, then I can get more onboard. However, I think there are ways around that.

If zizmor is going to be run, then it should be on any PR that changes a file in the .github directory (or all PRs since I'm not great with GHA). This avoids us having to run it nightly and run it in private. If the concern is anyone can read the output of zizmor, that's kind of mood since they could pull the repo and run the command themselves.

[tim-schilling]

@bckohan I really like the structure of your security policy on django-typer. Maybe we can start there on the best-practices repo. Then when we figure out how to push files down, we'll do that for the Code of Conduct and Security Policy files?

[bckohan]

@tim-schilling I only suggest a security repository because I was working backwards from "How do you use GHA to run zizmor on all django-commons projects without having to do anything on a per repository basis?" It does not really matter that the output is secret because, as you say, anyone can run it themselves. You could use either normal issues or the private security reporting tool to report issues. I think the security tool makes sense because I presume the security team is notified anytime a security issue is opened on any repo? Or at least that that might be desirable?

[tim-schilling]

I think the security tool makes sense because I presume the security team is notified anytime a security issue is opened on any repo?

Correct. Just confirmed that we do have security managers set up (thanks @cunla!)

I think the security tool makes sense

Sorry, I'm not 100% sure what you're referring to here

[Stormheg]

I think we should strive for the confidence that a django-commons project is less vulnerable to a supply chain attack.

🙌 💯

I agree Django Commons should have an active role in making sure its open source projects follow security best practices. Zizmor sounds like an useful tool to help out with this goal.

Agree with @tim-schilling it would be helpful if Zizmor could be run on any change to a workflow file in a PR. Better to prevent introducing vulnerable workflows than to have to react to them on an organization-wide weekly run. Also agree with @bckohan that it's better to not have to set it up a per-repository level. But if that is complex to set up or manage, I'd settle for per-repository.