CI: pin GitHub actions to SHAs and use Dependabot to update them

[topimiettinen]

See:
https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
and:
https://docs.github.com/en/code-security/supply-chain-security/keeping-your-dependencies-updated-automatically/keeping-your-actions-up-to-date-with-dependabot

[reinerh]

Is this really necessary for actions that come from Github?
I wouldn't expect them to be malicious (and even if they were, we don't use them to ship artefacts).

I personally would be more annoyed by regular merge requests from a bot that just bumps commit IDs.

[topimiettinen]

Is this really necessary for actions that come from Github? I wouldn't expect them to be malicious (and even if they were, we don't use them to ship artefacts).

Maybe, but there might still be problems with forks and PRs. We already have also LGTM, which is 3rd party. I'd expect the CI setup to expand to further 3rd party systems (for example Coveralls could be interesting) and maybe one day there are Python dependencies (like Meson and Ninja). The small downside is that simple version IDs like v1 became much longer hashes. The benefit (although a bit theoretical ATM) is that it's hard to forge SHAs while simple strings can be copied easily.

I personally would be more annoyed by regular merge requests from a bot that just bumps commit IDs.

The frequency can be tuned to monthly but I haven't found weekly check to be too noisy. The PRs are rather trivial and it's also easy to be vigilant and check the SHAs.

[rusty-snake]

Did I miss understand something or where's the issue with semver dependencies if we are not using ${{ secrets.GITHUB_TOKEN }}?

[topimiettinen]

Did I miss understand something or where's the issue with semver dependencies if we are not using ${{ secrets.GITHUB_TOKEN }}?

There are other problems mentioned in the page, like "pull_request_target still has the read/write repository token in memory that is potentially available to any running program". Maybe other problems will rise in the future.

The actions could be also buggy and keeping them relatively recent could help with that, like with any software. Using unmaintained 'v1' forever is asking for problems.

[kmk3]

@topimiettinen commented on Dec 14:

Is this really necessary for actions that come from Github? I wouldn't
expect them to be malicious (and even if they were, we don't use them to
ship artefacts).

Maybe, but there might still be problems with forks and PRs. We already have
also LGTM, which is 3rd party. I'd expect the CI setup to expand to further
3rd party systems (for example Coveralls could be interesting) and maybe one
day there are Python dependencies (like Meson and Ninja). The small downside
is that simple version IDs like v1 became much longer hashes. The benefit
(although a bit theoretical ATM) is that it's hard to forge SHAs while simple
strings can be copied easily.

Since AFAIK GitHub does not verify the GPG signature of a tag after cloning it,
pinning to a SHA does seem more secure.

I personally would be more annoyed by regular merge requests from a bot
that just bumps commit IDs.

The frequency can be tuned to monthly but I haven't found weekly check to be
too noisy. The PRs are rather trivial and it's also easy to be vigilant and
check the SHAs.

The dependabot spam is also a concern of mine, though I suppose that it's
partly because I've mostly seen it on e.g.: nodejs-based projects that have
hundreds of dependencies, which are also updated rather frequently. If it can
be made to alert only weekly/monthly, then that doesn't seem too bad,
especially when using only a handful of GitHub Actions.

[kmk3]

It's unclear which version a SHA represents.

How about putting each intended version in a comment?

Suggested change

	- uses: actions/checkout@@ec3a7ce113134d7a93b817d10a8272cb61118579
	# v2
	- uses: actions/checkout@@ec3a7ce113134d7a93b817d10a8272cb61118579

Or maybe:

Suggested change

	- uses: actions/checkout@@ec3a7ce113134d7a93b817d10a8272cb61118579
	# actions/checkout@v2
	- uses: actions/checkout@@ec3a7ce113134d7a93b817d10a8272cb61118579

[topimiettinen]

Would the comment be updated by Dependabot? Otherwise it will bitrot.

[kmk3]

@topimiettinen on Dec 14:

Would the comment be updated by Dependabot? Otherwise it will bitrot.

With the current configuration, does Dependabot put something like "update
foo@v1 to foo@v2" in the commit message (I know that it can do that, but I'm
not sure if it's the default)? If the equivalent tags can be discovered
locally/without having to search for them on GitHub, I think that would be good
enough.

If not, instead of a comment on each item, to avoid bitrot and duplication I
think we could have a mapping on a separate file. It would only need updating
when adding a completely new "action@version" combination (i.e.: one that was
never part of any committed workflow file). Kind of like a "package.lock"
file. Example:

actions/checkout@v2: ec3a7ce113134d7a93b817d10a8272cb61118579
actions/checkout@v3: 1231231231231231231231231231231231231231
github/codeql-action/init@v1: e095058bfa09de8070f94e98f5dc059531bc6235

By the way, is Dependabot smart enough to update a SHA to a new SHA or would it
open a PR to bump actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579
to actions/checkout@v3 in this case?

[topimiettinen]

@topimiettinen on Dec 14:

Would the comment be updated by Dependabot? Otherwise it will bitrot.

With the current configuration, does Dependabot put something like "update foo@v1 to foo@v2" in the commit message (I know that it can do that, but I'm not sure if it's the default)? If the equivalent tags can be discovered locally/without having to search for them on GitHub, I think that would be good enough.

Yes, here's an example commit/PR with the same settings: topimiettinen/libaslrmalloc@0aeb663

If not, instead of a comment on each item, to avoid bitrot and duplication I think we could have a mapping on a separate file. It would only need updating when adding a completely new "action@version" combination (i.e.: one that was never part of any committed workflow file). Kind of like a "package.lock" file. Example:

actions/checkout@v2: ec3a7ce113134d7a93b817d10a8272cb61118579
actions/checkout@v3: 1231231231231231231231231231231231231231
github/codeql-action/init@v1: e095058bfa09de8070f94e98f5dc059531bc6235

For Python, there's requirements.txt with similar syntax (https://github.com/topimiettinen/libaslrmalloc/blob/master/.github/workflows/requirements.txt) but I don't know if that would work for GH actions. Example of Dependabot updating it: topimiettinen/libaslrmalloc@248495b

By the way, is Dependabot smart enough to update a SHA to a new SHA or would it open a PR to bump actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 to actions/checkout@v3 in this case?

Only SHA is updated. There's also a GH action to verify that PRs don't change SHAs to semvers: https://github.com/zgosalvez/github-actions-ensure-sha-pinned-actions but I don't know if there's an action to verify that it actually does what it says it does and so forth.

[kmk3]

Sounds good then; thanks for the examples.

[topimiettinen]

@topimiettinen commented on Dec 14:

Is this really necessary for actions that come from Github? I wouldn't
expect them to be malicious (and even if they were, we don't use them to
ship artefacts).

Maybe, but there might still be problems with forks and PRs. We already have
also LGTM, which is 3rd party. I'd expect the CI setup to expand to further
3rd party systems (for example Coveralls could be interesting) and maybe one
day there are Python dependencies (like Meson and Ninja). The small downside
is that simple version IDs like v1 became much longer hashes. The benefit
(although a bit theoretical ATM) is that it's hard to forge SHAs while simple
strings can be copied easily.

Since AFAIK GitHub does not verify the GPG signature of a tag after cloning it, pinning to a SHA does seem more secure.

A GPG signature is also a hash, signed with a key. Instead of pinning the SHA, only the public key used for signing would need to be pinned. But which key? It would also need updating when the keys expire, there's maybe also CRL, OCSP, master keys, CAs etc. Cryptographic signatures could be better but they're also more complex.

I personally would be more annoyed by regular merge requests from a bot
that just bumps commit IDs.

The frequency can be tuned to monthly but I haven't found weekly check to be
too noisy. The PRs are rather trivial and it's also easy to be vigilant and
check the SHAs.

The dependabot spam is also a concern of mine, though I suppose that it's partly because I've mostly seen it on e.g.: nodejs-based projects that have hundreds of dependencies, which are also updated rather frequently. If it can be made to alert only weekly/monthly, then that doesn't seem too bad, especially when using only a handful of GitHub Actions.

Yeah, I can only imagine the noise with JS or Python. This shouldn't be as bad. Continuing the comparison of the Actions to other software: an LTS version of the Action with only security updates could be nice but I guess such things don't exist.

[kmk3]

+1 for having Dependabot check the dependencies.

As for the pinning, maybe using exact versions (such as @2.1.0) would be a
secure enough alternative (not sure), but I don't feel strongly about it either
way. Might expand on this on a discussion later.

[topimiettinen]

+1 for having Dependabot check the dependencies.

Great! What about @reinerh and @rusty-snake, are you now OK with this or do you have further questions?

As for the pinning, maybe using exact versions (such as @2.1.0) would be a secure enough alternative (not sure), but I don't feel strongly about it either way. Might expand on this on a discussion later.

The problem is that PRs and forks could still forge the same exact version numbers but with SHAs its very hard.

[rusty-snake]

I still don't understand why it is necessary (we don't use pull_request_target either), but don't take this as blocking a merge.