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 measure: fail PR if a lock file was changed #1344

Merged
merged 4 commits into from Nov 9, 2022

Conversation

kevinpapst
Copy link
Collaborator

@kevinpapst kevinpapst commented Nov 1, 2022

There were quite a lot of PRs in the past, which shipped changed lock files.

Those were mostly introduced by unexperienced contributors, but considering the popularity of Tabler and the amount of potential targets, this could be a real security risk:
https://snyk.io/blog/why-npm-lockfiles-can-be-a-security-blindspot-for-injecting-malicious-modules/

There is a simple way to prevent that, which I am using in all my projects: a workflow that checks the PR for changed lock files and fails directly if a changed lockfile was included. The only persons allowed to push changes for those files are @codecalm and @dependabot (see e.g. #1296).

The used workflow is: https://github.com/xalvarez/prevent-file-change-action

What do you think @codecalm about the idea in general?

This PR is a draft for now, because I am not sure about the required token permissions. I posted a question in the action repo and will update the workflow file accordingly if there are more required.

@vercel
Copy link

vercel bot commented Nov 1, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tabler ✅ Ready (Inspect) Visit Preview Nov 1, 2022 at 7:28PM (UTC)

@codecalm codecalm marked this pull request as ready for review November 1, 2022 12:10
@codecalm codecalm marked this pull request as draft November 1, 2022 12:11
@codecalm
Copy link
Member

codecalm commented Nov 1, 2022

nick, looks good! ❤️

@kevinpapst
Copy link
Collaborator Author

Updated permissions according to xalvarez/prevent-file-change-action#489

Ready for review.

@codecalm
Copy link
Member

codecalm commented Nov 9, 2022

for me its good! ❤️

@kevinpapst
Copy link
Collaborator Author

Your project => you merge 😄

@codecalm
Copy link
Member

codecalm commented Nov 9, 2022

take it 😂

@kevinpapst kevinpapst merged commit 153500f into tabler:dev Nov 9, 2022
@kevinpapst kevinpapst deleted the lockfiles branch November 9, 2022 20:03
@kevinpapst
Copy link
Collaborator Author

You called for it hahaha

@kevinpapst
Copy link
Collaborator Author

I don't quite understand why the merge run failed.

Can you merge dev into any branch that has a PR, so we can check if those fail as well.

@codecalm
Copy link
Member

codecalm commented Nov 9, 2022

I'll merge #1350 in few minutes

@kevinpapst
Copy link
Collaborator Author

That worked. I guess that the action somehow has an issue when itself is merged into a new branch.
I'll take care if that happens again.

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

Successfully merging this pull request may close these issues.

None yet

2 participants