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

workflows/setup-commit-signing: exclude dependabot. #466

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

MikeMcQuaid
Copy link
Member

It doesn't have the necessary secrets set so will always fail.

It doesn't have the necessary secrets set so will always fail.
@MikeMcQuaid MikeMcQuaid merged commit 37bdcad into master Nov 28, 2023
3 checks passed
@MikeMcQuaid MikeMcQuaid deleted the gpg_dependabot branch November 28, 2023 09:04
@Bo98
Copy link
Member

Bo98 commented Nov 29, 2023

It passes if you label it update node_modules which we need to anyway for the dependabot changes to have any effect. GitHub Actions does not read package(-lock).json.

@MikeMcQuaid
Copy link
Member Author

@Bo98 Weird. Can you open an issue explaining what needs to be done to fix this in this repository? I'm not sure what's going on and have been happily just merging these PRs but sounds like they aren't doing anything.

I think ideally, with a @BrewTestBot PAT/@Homebrew GitHub App if necessary, we should automate these PRs so that they are 🔴 until they are ready to merge and 🟢 when ready to merge (and should go from 🔴 to 🟢 with no human intervention required).

I'm not sure I understand what's going on with package-lock.json because that's definitely being updated by @dependabot in a similar private repository I use...

@Bo98
Copy link
Member

Bo98 commented Nov 29, 2023

So basically how Actions work is a tarball of the repository is downloaded (codeload.github.com), the action.yml is read and if it's node it'll simply run node main.js (or whatever the main file is called). In the JS file, we include dependencies via require, e.g. require("@actions/core"), which works by searching through node_modules starting at the current directory and working its way back up the directory three (technically there's some differences between CommonJS and ESM but I'll ignore that for now): https://nodejs.org/api/modules.html#modules_loading_from_node_modules_folders.

package-lock.json is an npm file. Running npm install will read this and populate node_modules with the install of each of those dependencies. Npm is not run by GitHub Actions itself, so what we do is commit node_modules to the repository. An alternative is dist-style bundling that some of the official actions do but that's something that would need done on every commit so not exactly much better.

We have a workflow that does that npm install and commit. You can consider the process similar to brew vendor-gems. The reason it's done via a label and not automatically is for security reasons. I can detail this further on Slack. But there will be ways to improve this. Of particular interest is instead of node_modules we could use Yarn's PnP system which Dependabot apparently supports vendoring itself. It's somewhat non-standard as you need to add an extra require("../.pnp.cjs").setup() to every action but it could work - I'll need to experiment with it to make sure. If not, then we may need to investigate ways to better test actions that don't involve elevated permissions.

@MikeMcQuaid
Copy link
Member Author

Of particular interest is instead of node_modules we could use Yarn's PnP system which Dependabot apparently supports vendoring itself.

Yeh, I've been using yarn elsewhere and it seems to Just Work so that might be a good fit.

The reason it's done via a label and not automatically is for security reasons. I can detail this further on Slack.

Please, that would be good.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants