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

Recommend pinning to commit SHA or release tag #265

Open
joebowbeer opened this issue May 19, 2022 · 3 comments
Open

Recommend pinning to commit SHA or release tag #265

joebowbeer opened this issue May 19, 2022 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@joebowbeer
Copy link

To protect against latest version incompatibility and potential supply chain attack (e.g. codecov), please recommend pinning to a commit SHA, or at least a release tag.

Also, consider packaging this in a GitHub Action.

https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions

@quinnturner
Copy link
Member

quinnturner commented May 19, 2022

Hi @joebowbeer, thanks for the issue report!

Could you expand on what value you'd like to gain with a GitHub Action over the existing bash approach? The workflow to run audit-ci is one line of bash: npx audit-ci@{version} --config ./audit-ci.jsonc.

I could also use some clarification on how you'd like to update the documentation. Notably, in the docs, I do recommend using npx audit-ci@^6 --config ./audit-ci.jsonc instead of npx audit-ci@6.2.1 --config ./audit-ci.jsonc. If that's what you're referring to, I've done that intentionally to try and find a good balance of not breaking pipelines and trying and help bring new updates to consumers conveniently. Yes, if this project went rogue and published malware, non-pinning consumers may be at risk. However, I am comfortable with that trade-off for this project as we have a small and trusted group of developers managing it (a total of two) with MFA on GitHub and NPM.

On the other hand, itmake sense is to strictly pin our dependencies. While I've personally vetted every line of code of the distributed JS of our dependencies for the versions that we depend on, they aren't pinned (they use ^), so my vetting may be moot by now since the dependencies may update. I may set our package.json's dependencies to use version pinning on versions that I've personally vetted. This kind of goes along with #124 where I was wondering if I can pin transitive dependencies by publishing the package-lock.json. I don't think that's how it works, but I am open to ideas to pinning transitive dependencies.

@quinnturner quinnturner added enhancement New feature or request question Further information is requested labels May 19, 2022
@joebowbeer
Copy link
Author

I think GitHub Actions provides a better onboarding experience. In the future, GitHub may differentiate code scanning actions from others, providing further benefits, but this may be wishful thinking.

In addition, npx is susceptible to typosquatting.

I do see an instance or two of a tag in the README, now that you mention it, but I didn't see any security-related justification, and there are many instances without, including the one I was interested in:

https://github.com/IBM/audit-ci#github-actions

I used to trust repos, especially ones that were created to tighten security, but now I prefer trusting as little as possible (zero trust).

Pinning all dependencies would be great. This is an advantage of installing audit as a dev dependency, because in that case its deps are locked and it can check its own deps for recently-reported threats. But that has other issues as you point out.

@quinnturner
Copy link
Member

You are absolutely right about missing pinning. I've updated all documented references for npx/{yarn|pnpm} dlx to use audit-ci@^6 here: #273. I still do see the point and appreciate pinning to a specific version. I may follow up with a note about using a pinned version, but at least #273 starts to cover your suggestion.

but I didn't see any security-related justification

Fair! Open to improvements in this area.

npx is susceptible to typosquatting

Yeah, I can appreciate that as well. Typosquatting can only really be prevented with the usage of an SHA, as you mentioned. Reasonable to consider documenting that tbh, have to think about that a bit more and find a convenient way to provide that SHA as documentation. Open to ideas!

I used to trust repos, especially ones that were created to tighten security, but now I prefer trusting as little as possible (zero trust).

Agreed 👍🏻

Pinning all dependencies would be great.

I've already pinned the "less common" dependencies, but definitely am considering pinning all dependencies. I'd also consider finding a way to pin transitive dependencies. They were originally not pinned to support deduplication when performing --save-dev. However, I've since moved off suggesting --save-dev to ensure that we run an audit before the CI/user runs postinstall. Now that we suggest npx, deduplication is not as important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants