ci: monitor npm install scripts#3882
Conversation
|
No new npm install scripts detected. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a CI workflow and Deno script that detect when a PR introduces npm dependencies with hasInstallScript: true (or causes existing dependencies to start declaring install scripts), and posts a summary comment on the PR.
Changes:
- New Deno/TypeScript script
scripts/monitor-npm-has-install-script.mtsthat diffspackage-lock.jsonsnapshots between the merge-base and PR head, classifying findings asintroducedorexisting_new_script, and emits a TSV findings file plus a Markdown comment. - New workflow
.github/workflows/monitor-npm-hasInstallScript.ymlthat runs onpull_request, executes the script with restricted Deno permissions, appends the result to the workflow summary, and creates/updates a marker-tagged PR comment viaactions/github-script.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/monitor-npm-has-install-script.mts | Core detection logic: parses lockfiles at base/head SHAs, identifies new install-script usage, and renders findings + package.json diff snippets. |
| .github/workflows/monitor-npm-hasInstallScript.yml | CI wiring: checks out repo, sets up Deno, runs the detection script, and posts/updates a PR comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Add result to workflow summary | ||
| run: cat "${{ steps.detect.outputs.comment_path }}" >> "${GITHUB_STEP_SUMMARY}" | ||
|
|
||
| - name: Create or update PR comment |
There was a problem hiding this comment.
I want it to add a "No problems found" comment so I know it's working :). Maybe it could be quieted later if desired.
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] |
| const value: string | undefined = args[i + 1]; | ||
| if (value == null || value.startsWith('--')) { | ||
| fail(`Missing value for --${key}`); | ||
| } |
There was a problem hiding this comment.
If the input is in this format, fail will exit the program. I don't think it's important right now to account for paths beginning with --.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3882 +/- ##
=======================================
Coverage 81.03% 81.03%
=======================================
Files 630 630
Lines 40684 40684
Branches 6602 6597 -5
=======================================
Hits 32967 32967
Misses 6681 6681
Partials 1036 1036 ☔ View full report in Codecov by Sentry. |
This patch adds a workflow which looks to see if a PR changes package-lock.json files such that a dependency newly has an install script that didn't before. Or if there is a new dependency added that has an install script. I read that this can be a suspicious sign of possible trouble to pay attention to.
I apologize that the script is so long.
I suggest that the workflow just comment on our PRs for now without blocking them.
Some example findings can be seen below, as run from repo root.
No finding:
Introduced package with install script:
Existing package starts to have an install script:
A finding where there are RTS existing dependencies that now have an install script, and ClientApp existing packages that now have an insall script, and ClientApp has new dependencies that have install scripts. This example was made by using a large range of commits, which would presumably not be the normal situation.
This change is