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

Security: v2 action cannot be pinned effectively #574

Closed
mhils opened this issue Oct 29, 2021 · 6 comments
Closed

Security: v2 action cannot be pinned effectively #574

mhils opened this issue Oct 29, 2021 · 6 comments

Comments

@mhils
Copy link

mhils commented Oct 29, 2021

The individual jobs in a GitHub Actions workflow have access to all secrets configured in the repository. Consequently, there is significant risk in sourcing actions from third-party repositories on GitHub. To mitigate this risk, GitHub recommends the following:

Pin actions to a full length commit SHA

Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.

Source: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions

We've been using this approach successfully to pin a specific commit of codecov-action@v1 in our repositories. However, this version will be deprecated on February 1, 2022.

codecov-action@v2 introduces new behavior in that it fetches the uploader binary from uploader.codecov.io and then verifies that is has been signed using Codecov's PGP key. While I appreciate that you do perform signature checks, this behavior effectively undermines pinning as additional code is fetched and executed dynamically. While I may trust Codecov, I don't trust the >500 NPM dependencies included in the new uploader.

Is there any possiblity that you could ship a version of your action where the uploader binaries are bundled or pinned?

/cc @briansmith, who has done some great research on GitHub Actions security at briansmith/untrusted#50.
@drazisil-codecov
Copy link
Contributor

Is there any possibility that you could ship a version of your action where the uploader binaries are bundled or pinned?

Hi @mhils 👋
The binaries are all kept at https://uploader.codecov.io/, so you can pin to any one you want.

While I may trust Codecov, I don't trust the >500 NPM dependencies included in the new uploader.

😬 I hope most of those are dev dependencies. The majority of the size comes from bundling Node.js in so it's a static standalone. That being said, I do have a goal to try and reduce the dependency tree. PRs welcome! ;)

@mhils
Copy link
Author

mhils commented Nov 22, 2021

The binaries are all kept at https://uploader.codecov.io/, so you can pin to any one you want.

To clarify, you recommend that we download, verify, and invoke the binaries manually (without using codecov-action)? Happy to go that route unless there is another better/blessed way. :)

😬 I hope most of those are dev dependencies.

According to https://npmgraph.js.org/, it's 689 modules from 387 maintainers; or 64 modules from 100 maintainers if you exclude dev dependencies. I don't think you can exclude dev dependencies though, they could compromise binaries just as well.

The majority of the size comes from bundling Node.js in so it's a static standalone. That being said, I do have a goal to try and reduce the dependency tree. PRs welcome! ;)

I'm mostly concerned about the security aspects here, but I can't deny that I'm also a bit disappointed to see that the new uploader clocks in at 60MB. I'm surprised you picked Node.js given the supply chain security + file size issues, but I guess you've thought about the pros and cons longer than I did. So I don't want to sidetrack this here. :)

@drazisil-codecov
Copy link
Contributor

To clarify, do you recommend that we download, verify, and invoke the binaries manually (without using codecov-action)? Happy to go that route unless there is another better/blessed way. :)

Yes. It was pointed out to me that I misunderstood your request / forgot which repo I was on. :D

I'm mostly concerned about the security aspects here, but I can't deny that I'm also a bit disappointed to see that the new uploader clocks in at 60MB. I'm surprised you picked Node.js given the supply chain security + file size issues, but I guess you've thought about the pros and cons longer than I did. So I don't want to sidetrack this here. :)

Well. we weren't aware that on Alpine we also have to bundle the c lib, since that doesn't come installed by default. 😬

Given that we pin the dependency and use Renovate for updates, do you still have outstanding concerns regarding the binary in general? I can tag on our security engineer if you'd like to start a thread over on the uploader repo. https://github.com/codecov/uploader

@mhils
Copy link
Author

mhils commented Nov 22, 2021

Yes. It was pointed out to me that I misunderstood your request / forgot which repo I was on. :D

All good. Thanks for confirming. :-)

Given that we pin the dependency and use Renovate for updates, do you still have outstanding concerns regarding the binary in general?

We'll pin the binaries for now, which satisfies my use case. I wish pinning would be supported by the action as it's a bit cumbersome to do manually (different binary and hash for each platform), but that's not a roadblocker for me. For everyone who's not pinning I just hope that you sign manually/offline and not automatically from some pubsub consumer. :-)

Thanks again!

@drazisil-codecov
Copy link
Contributor

I wish pinning would be supported by the action as it's a bit cumbersome to do manually

Actually ... Since we are now on versions, does setting version to the version you want (they are all at uploader.codecov.io for browsing and selection) count as allowing you to pin? https://github.com/codecov/codecov-action/blob/f32b3a3741e1053eb607407145bc9619351dc93b/CHANGELOG.md#features

@mhils
Copy link
Author

mhils commented Nov 22, 2021

That's a great first step, but unless the binary hashes are checked the action still depends on the integrity of https://uploader.codecov.io/. For example, someone could sign a malicious binary with the codecov keys tomorrow, replace https://uploader.codecov.io/linux/v0.1.9, and my CI jobs would happily execute it as the signature is valid.

For proper pinning, codecov-action would ideally hardcode a specific version with specific binary hashes (and then verify that they match on download). The upside is that you don't have to deal with PGP complexity, the downside is that you need to bump codecov-action when you do a new uploader release (although that could be easily automated).

mhils added a commit to mhils/pdoc that referenced this issue Mar 31, 2022
codecov's new uploader cannot be used securely [1],
so we take that opportunity to switch to something simpler.

[1] codecov/codecov-action#574
mhils added a commit to mhils/pdoc that referenced this issue Mar 31, 2022
codecov's new uploader cannot be used securely [1],
so we take that opportunity to switch to something simpler.

[1] codecov/codecov-action#574
mhils added a commit to mitmproxy/pdoc that referenced this issue Mar 31, 2022
codecov's new uploader cannot be used securely [1],
so we take that opportunity to switch to something simpler.

[1] codecov/codecov-action#574
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