Adds lock file linting to flows running with every PR. Closes #775 - #861
Adds lock file linting to flows running with every PR. Closes #775#861Adam-it wants to merge 2 commits into
Conversation
nicodecleyre
left a comment
nicodecleyre
left a comment
There was a problem hiding this comment.
Great work so far, can you check the comments i left? Thank you!
| npm_config_omit: optional | ||
|
|
||
| - name: Validate lockfile for SPFx Toolkit | ||
| run: .\node_modules\.bin\lockfile-lint --path npm-shrinkwrap.json --type npm --allowed-hosts npm --validate-https |
There was a problem hiding this comment.
Would you consider adding --validate-integrity and --validate-package-names to the lockfile-lint command? The current flags cover host and protocol validation, but the scenario described in the aim, a lockfile edit that swaps the package hash to a vulnerable repo or package, is specifically caught by --validate-integrity. --validate-package-names would additionally cover typosquatting or name mismatches between package.json and the lockfile.
There was a problem hiding this comment.
good comment. lets do that 👍, will do that
| "@tailwindcss/postcss": "4.1.4", | ||
| "@types/glob": "8.1.0", | ||
| "@types/mocha": "10.0.10", | ||
| "@types/node-forge": "^1.3.11", |
There was a problem hiding this comment.
Also noticed unrelated dependency changes (@types/node-forge. Removal of @types/sinon, @types/vscode, @typescript-eslint-plugin. Version swap on rehype-raw, remark-gfm, use-debounce). Were these intentional, or can they be split out of this PR since they're not needed for the lockfile-lint change?
There was a problem hiding this comment.
@nicodecleyre TBH those changes were just done 'automagically' after npm install. those are the same packages in the same versions just places in different order in the package.json. Not sure if the lint or npm install reoder this part 🙂.
I may revert to previous order but still nothing changed even now as the versions are the same
There was a problem hiding this comment.
Pull request overview
Adds CI validation of the npm shrinkwrap to reduce the risk of lockfile injection in PRs for this VS Code extension (SPFx Toolkit), aligning with the security goal in #775.
Changes:
- Adds
lockfile-lintas a dev dependency and updatesnpm-shrinkwrap.jsonaccordingly. - Updates the PR test workflow to run a lockfile validation step.
- Minor dependency list reordering/cleanup in
package.json.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
package.json |
Adds lockfile-lint to devDependencies (plus minor ordering changes). |
npm-shrinkwrap.json |
Updates shrinkwrap to include lockfile-lint and its transitive dependencies. |
.github/workflows/check-tests.yml |
Adds a lockfile validation step to the PR test workflow. |
Files not reviewed (1)
- npm-shrinkwrap.json: Generated file
🎯 Aim
The aim of this PR is to add a lint step for npm dependencies present in the lock file to every workflow which we run for PRs so that we try to shield ourselves from hacky updates done directly in the lock file in the PR that update the package hash to a vulnerable repo or package
📷 Result
✅ What was done
🔗 Related issue
Closes #775