feat: npm build command input#19
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional GitHub Action input to run a user-specified npm build step during Phase 2 (finalize release) before publishing to an npm registry.
Changes:
- Introduces a new
npm-build-commandinput exposed viaaction.yml. - Runs an optional build step (currently
npm install+ command execution) beforenpm publishinscripts/finalize-release.sh. - Documents the new input and usage examples in
README.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/finalize-release.sh | Adds conditional pre-publish build execution when npm-build-command is provided. |
| action.yml | Defines and wires the new npm-build-command input into the composite action environment. |
| README.md | Documents the new input and shows usage examples for npm publish flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [[ -n "${INPUT_NPM_BUILD_COMMAND:-}" ]]; then | ||
| echo "Running build: ${INPUT_NPM_BUILD_COMMAND}" | ||
| npm install | ||
| eval "$INPUT_NPM_BUILD_COMMAND" |
There was a problem hiding this comment.
Using eval to run the build command adds an extra round of shell parsing/expansion (beyond what bash -c would do) and can lead to unexpected behavior or command injection if the input ever becomes attacker-controlled. Prefer invoking a shell with -c (or restricting this input to an npm run <script> pattern) instead of eval.
| eval "$INPUT_NPM_BUILD_COMMAND" | |
| bash -c "$INPUT_NPM_BUILD_COMMAND" |
| echo "${registry_host}/:_authToken=${INPUT_NPM_TOKEN}" > .npmrc | ||
| echo "registry=${registry}" >> .npmrc | ||
|
|
||
| if [[ -n "${INPUT_NPM_BUILD_COMMAND:-}" ]]; then | ||
| echo "Running build: ${INPUT_NPM_BUILD_COMMAND}" | ||
| npm install | ||
| eval "$INPUT_NPM_BUILD_COMMAND" |
There was a problem hiding this comment.
The .npmrc containing the publish token is written before npm install / the build command runs. That means dependency lifecycle scripts (and any build tooling) can read/exfiltrate the token, and if the build fails the .npmrc cleanup won’t run. Consider (1) moving .npmrc creation to immediately before npm publish (or using NODE_AUTH_TOKEN just for publish), and (2) adding an EXIT trap to reliably remove .npmrc even on failures.
No description provided.