-
Notifications
You must be signed in to change notification settings - Fork 45
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
Barebones workflow to trigger pre-commit github action #64
base: main
Are you sure you want to change the base?
Conversation
.github/workflows/pre-commit.yml
Outdated
steps: | ||
- run: echo "🎉 The job was automatically triggered by a ${{ github.event_name }} event." | ||
- run: echo "🐧 This job is now running on a ${{ runner.os }} server hosted by GitHub!" | ||
- run: echo "🔎 The name of your branch is ${{ github.ref }} and your repository is ${{ github.repository }}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this line; it's vulnerable to script injection if someone opens a PR and names their repo in a malicious manner.
(I selected the wrong part - sorry! I mean L14 since it echos the repo name, but tbh we probably should just remove them all.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Any user modifiable field is at risk of this, repo is probably the least likely but still something we shouldn't open ourselves to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Honestly I just copied this example from a silly little boilerplate example in their docs. I'll make some meaningful changes here shortly.
Description
Barebones Github Action configuration to install and run
pre-commit
on the nv-ingest codebaseThis closes: #63
Checklist