Should tags be considered sufficient pins for actions?

[funnelfiasco]

#161 introduced an audit for finding un-pinned actions. This is good. But I'm wondering if it's too accepting. It currently requires a pin based on branch, tag, or SHA. In most cases, that's fine. But what happens if upstream becomes malicious? Let's say Jia Tan becomes a maintainer of a popular (or unpopular for that matter) GitHub action. They can update the branch with a backdoor and force push a previously-good tag to now point to malicious code.

I see three possible approaches here:

1. I am totally right and the un-pinned action audit should only be satisfied if the pin is to a SHA. A new release drops the branch and tag checks and I am celebrated the world over as a genius.
2. I am thinking way too hard about this and the current state is the best approach because it provides the most protection to the most people.
3. It's a reasonable point and people need to determine their risk tolerance. The regular check is fine, but maybe --pedantic should require only SHAs.

I'm not sure what my position is here. 3 is in the lead at the moment, but I am open to being convinced of either 1 or 2. Part of it, I suppose, comes down to "how paranoid does this project want to be?" An argument for 2 is that the vast majority of actions (probably) come from well-managed providers (e.g. GitHub itself) that make the risk of compromise very low while also ensuring that people automatically get the latest bug fixes (which may also include security fixes) that they'd otherwise have to update manually.

[woodruffw]

Thanks for opening this @funnelfiasco!

I've gone back and forth on this as well: hash-pinned actions are definitely the gold standard, but even just branch/tag pinning is a significant improvement over leaving actions entirely unpinned.

3. It's a reasonable point and people need to determine their risk tolerance. The regular check is fine, but maybe --pedantic should require only SHAs.

I like this! If you're interested, I'd be more than happy to merge a PR that adds findings for non-hash-pinned actions when --pedantic is passed.

Longer term, this demonstrates that --pedantic is not a great categorization -- I've been thinking about refactoring the Determinations model to include a Sensitivity enumeration, which would then be something like:

enum Sensitivity {
  // The current default
  Normal,
  // For findings like these
  Pedantic,
  // For high false-positive findings, e.g. self-hosted runner detection
  Extreme,
}

...with that, each finding could mark its sensitivity and we everything would flow more gracefully into the current filtering system (which includes config rules).

[funnelfiasco]

I like this! If you're interested, I'd be more than happy to merge a PR that adds findings for non-hash-pinned actions when --pedantic is passed.

Do I know rust? No. Do I let little things like that stop me? Also no. 😄 I'll see if I can get something going in the next few days. There are examples for me to follow, so it shouldn't be too awful in theory.

Longer term, this demonstrates that --pedantic is not a great categorization -- I've been thinking about refactoring the Determinations model to include a Sensitivity enumeration

I think that general idea makes sense. My main concern is that it could get really hard to communicate the nuanced meaning to people. Perhaps associating the sensitivity levels with some kind of external standard would help there. 🤷 Just tossing some half-formed thoughts into the void.

[woodruffw]

I think that general idea makes sense. My main concern is that it could get really hard to communicate the nuanced meaning to people. Perhaps associating the sensitivity levels with some kind of external standard would help there. 🤷 Just tossing some half-formed thoughts into the void.

Good point; I'll try and find some prior art here.

[funnelfiasco]

FWIW, the OSPS Baseline that OpenSSF is working on might be reasonable at some point. It's still a work-in-progress, but it does have three maturity levels, so that's nice. I don't know that there's any real UX research that went into those levels, other than "it corresponds to the project lifecycle", which is perhaps not a great analog here.

Conversely, if your research turns up some useful sensitivity levels, that could make the Baseline more generally-applicable outside a Linux Foundation context. So something's getting improved, one way or another.

[woodruffw]

I was hoping SARIF would have a pre-defined list of common sensitivity settings, but unfortunately not.

Some notes:

• Most of the other static analysis tools I've worked with have at least two sensitivity levels: the 'normal' level (typically unnamed) and 'pedantic'.
• C/C++ compilers have -Wall/-Wextra/etc., but that's not super common outside of that space.
• cargo clippy has --all -- -W clippy::pedantic -W clippy::restriction -W clippy::nursery -D warnings to enable a bunch of high-sensitivity warnings and turn all warnings into denies.
• ruff doesn't appear to have a tool-wide sensitivity setting, besides setting select = ["ALL"] to enable everything
• bandit appears to have no sensitivity settings; only the ability to disable/suppress rules and their findings