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

direct use of branches #59

Closed
hellodword opened this issue Oct 28, 2024 · 7 comments
Closed

direct use of branches #59

hellodword opened this issue Oct 28, 2024 · 7 comments

Comments

@hellodword
Copy link

$ curl -fsSLo pr.yml https://github.com/graphql/graphiql/raw/c500e825094acac01d65840943d5ccd80e7de18a/.github/workflows/pr.yml

$ zizmor --gh-token 'foo' pr.yml 
🌈 completed pr.yml
warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> pr.yml:136:9
    |
136 |       - uses: actions/checkout@v4
    |         ------------------------- does not set persist-credentials: false
    |

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> pr.yml:114:9
    |
114 |       - uses: actions/checkout@v4
    |         ------------------------- does not set persist-credentials: false
    |

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> pr.yml:77:9
   |
77 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> pr.yml:198:9
    |
198 |         - uses: actions/checkout@v4
    |  _________-
199 | |         with:
200 | |           fetch-depth: 0
    | |________________________- does not set persist-credentials: false
    |

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> pr.yml:157:9
    |
157 |       - uses: actions/checkout@v4
    |         ------------------------- does not set persist-credentials: false
    |

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> pr.yml:257:9
    |
257 |       - uses: actions/checkout@v4
    |         ------------------------- does not set persist-credentials: false
    |

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> pr.yml:41:9
   |
41 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> pr.yml:16:9
   |
16 |         - name: Checkout Code
   |  _________-
17 | |         uses: actions/checkout@v4
   | |_________________________________- does not set persist-credentials: false
   |

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> pr.yml:62:9
   |
62 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> pr.yml:92:9
   |
92 |       - uses: actions/checkout@v4
   |         ------------------------- does not set persist-credentials: false
   |

10 findings (0 unknown, 0 informational, 0 low, 10 medium, 0 high)

While I think the direct use of branches may be vulnerable:

And this type of code is quite common.

@woodruffw
Copy link
Owner

Thanks for the issue. Yeah, I'm trying to strike a balance on flagging branches -- using a "version" branch is so common that flagging it will lower the S/N ratio too much, but OTOH there are branches that people should almost never use (like master and main). #1 contains an audit idea for the latter.

@hellodword
Copy link
Author

hellodword commented Oct 28, 2024

Cool. Besides guessing conventional branch names, I think another approach could be to scrape all actions in the build marketplace1, recording details in a database, such as branches, tags, any force-push activity, etc.

What do you think?

Footnotes

  1. https://github.com/marketplace?query=sort%3Acreated-desc&type=actions

@woodruffw
Copy link
Owner

Cool. Besides guessing conventional branch names, I think another approach could be to scrape all actions in the build marketplace1, recording details in a database, such as branches, tags, any force-push activity, etc.

I think that's a good idea, but probably out of scope for this tool -- managing datasets/behavioral trends fits better into trying to supply open source security intelligence, rather than pointing out individual fixable problems in a user's workflows.

OTOH, the baseline audit here is a good idea: we can use the GitHub API to get the repo's default branch, and then flag a finding if a uses: clause contains that branch. That should be sufficiently general to catch main, master, trunk, etc.

@woodruffw woodruffw mentioned this issue Oct 30, 2024
28 tasks
@leighmcculloch
Copy link

using a "version" branch is so common that flagging it will lower the S/N ratio too much, but OTOH there are branches that people should almost never use (like master and main)

Something to consider in this balance is that GitHub explicitly recommends against referencing balances or tags for third party actions:

It would be helpful if you could allow it on actions from specific orgs, and then not allow it from all other orgs. For example, in the repos I work in I would allow it for @github and my employer's org, but then not any other org.

@woodruffw
Copy link
Owner

It would be helpful if you could allow it on actions from specific orgs, and then not allow it from all other orgs. For example, in the repos I work in I would allow it for @github and my employer's org, but then not any other org.

FWIW, you can accomplish this by checking the results of the unpinned-uses audit (in "pedantic" mode, this will also list branch/tag pins) and then filtering the JSON or SARIF output down to the orgs/repos you care about 🙂

(I've thought about making this more configurable e.g. allowing the config file to include per-audit sections, but there's a lot of design space there.)

@woodruffw
Copy link
Owner

This has been implemented in the unpinned-uses audit, so closing!

@leighmcculloch
Copy link

FWIW, you can accomplish this by checking the results of the unpinned-uses audit (in "pedantic" mode, this will also list branch/tag pins) and then filtering the JSON or SARIF output down to the orgs/repos you care about 🙂

(I've thought about making this more configurable e.g. allowing the config file to include per-audit sections, but there's a lot of design space there.)

I'll give that a go, but one argument for supporting distinguishing third party vs first/second party actions, is that it's an explicit recommendation from GitHub.

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

No branches or pull requests

3 participants