Harden npm package publishing - #82
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48be0be54d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "tarball=${TARBALLS[0]}" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Publish with OIDC provenance | ||
| run: npm publish "$tarball" --provenance --access public |
There was a problem hiding this comment.
When this job reaches the publish step, $tarball comes from find package ..., so its value is a bare relative path like package/formo-analytics-react-native-1.0.2.tgz. npm does not treat that shape as a local tarball path unless it is path-qualified (for example ./package/... or an absolute path); it is parsed as a package/GitHub spec and the publish fails before reaching the registry. I reproduced the same parsing with npm by publishing out/foo.tgz versus ./out/foo.tgz, so release tags will build and validate but never publish unless this writes or passes a path-qualified tarball.
Useful? React with 👍 / 👎.
| for (const hook of ['preinstall', 'install', 'postinstall']) { | ||
| if (pkg.scripts?.[hook]) throw new Error(`Forbidden lifecycle hook: ${hook}`) | ||
| } |
There was a problem hiding this comment.
If a package candidate ever contains a root binding.gyp without an explicit install/preinstall, npm's lifecycle rules synthesize an install step that runs node-gyp rebuild, so this check can pass a tarball that still executes install-time code for consumers. Since this publish job is meant to enforce the no-lifecycle-hook boundary, also reject binding.gyp or otherwise account for npm's implicit install behavior before approving the tarball.
Useful? React with 👍 / 👎.
| name: npm-package | ||
| path: ${{ runner.temp }}/package/*.tgz | ||
| if-no-files-found: error | ||
| retention-days: 1 |
There was a problem hiding this comment.
Because the publish job waits on the protected npm-publish environment, any required reviewer delay or wait timer longer than one day lets this uploaded candidate expire before actions/download-artifact runs. In that case a valid tag build becomes unreleasable solely because approval took too long, so use the repository default or a longer retention window for the package artifact.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aeb93ab22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Reject newly introduced vulnerable dependencies | ||
| uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 | ||
| with: | ||
| fail-on-severity: high |
There was a problem hiding this comment.
For PRs that add a vulnerable dev/build dependency, this configuration still passes because dependency-review-action defaults fail-on-scopes to runtime unless development/unknown are listed (see the action's configuration docs: https://github.com/actions/dependency-review-action#configuration-options). The release workflow installs dev dependencies before building the published tarball, so high-severity issues in build tooling are still able to enter the release path unchecked; add fail-on-scopes for the build-time scopes if this check is meant to reject newly introduced vulnerable dependencies.
Useful? React with 👍 / 👎.
1 participant
What changed
npm-publishenvironmentWhy
The new release boundary prevents dependency code from running with npm publishing identity. The lockfile repair restores reproducible frozen installs.
Validation
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.