feat(npm-pr-version): add tarball support#45
Conversation
📝 WalkthroughWalkthroughAdds a tarball publishing mode to npm-publish-pr that validates and extracts package metadata from a tarball, publishes using secure flags (e.g., --ignore-scripts, --provenance), expands documentation and workflows to a two-job artifact pattern, updates code fence language hints, and adds integration tests and helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as "GitHub Workflow"
participant BuildJob as "build-and-test Job"
participant Artifacts as "GitHub Artifacts"
participant PublishJob as "publish Job"
participant Action as "npm-publish-pr Action"
participant Registry as "npm Registry"
Workflow->>BuildJob: trigger (pull_request / workflow_run)
BuildJob->>BuildJob: build & pack (create tarball)
BuildJob->>Artifacts: upload tarball artifact
Workflow->>PublishJob: triggered after build job
PublishJob->>Artifacts: download tarball artifact
PublishJob->>Action: run with `tarball` input
Action->>Action: resolve & validate tarball path(s)
Action->>Action: extract package.json → derive name & version
Action->>Registry: npm publish (--ignore-scripts [--provenance])
Registry-->>Action: publish result
Action-->>PublishJob: set outputs (`package-name`, `version`, `error-message`)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Integration Test Resultsnpm-pr-version action test:
The action is working correctly! 🎉 |
🧪 Test Results✅ All tests passed successfully!
|
|
Claude finished @codfish's task in 3m 8s —— View job Comprehensive Code Review Complete ✅I've completed a thorough review of the tarball support feature. This is a valuable security enhancement that addresses real risks in SummaryOverall Assessment: ✅ Approve with Minor Fixes Required This PR adds important security functionality with solid documentation and test coverage. The implementation is sound, but there are a few security and code quality issues that should be addressed before merging. 🔒 Security ReviewCritical Issues
Positive Security Aspects ✅
🧪 Testing ReviewTest Coverage: Good ✅The new
Test Quality Issues
📚 Documentation ReviewExcellent Documentation ✅
Documentation Issues
💻 Code Quality ReviewImplementation: Very Good ✅Strengths:
Code Quality Suggestions
📊 Specific Findings by Category1. Code Quality ✅
2. Security
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SECURITY.md (1)
77-86:⚠️ Potential issue | 🔴 CriticalBroken markdown fences render most of the document as a single code block.
Line 77 opens a 4-backtick fence (
````yaml), but line 86 uses only 3 backticks (```yaml) which cannot close a 4-backtick fence. The only matching 4-backtick close is line 277, so everything from line 78 to line 276 renders as one giant code block in GitHub's markdown renderer.Additionally, closing fences should not have a language hint —
```yamlon a closing fence is invalid markdown and will be treated as content or a new opening fence depending on context.The same closing-fence-with-language-hint pattern repeats at lines 99, 116, 186, 195, and 236.
Proposed fix — use consistent 3-backtick fences with plain closers
-````yaml +```yaml # ✅ Good - Using secrets properly - uses: codfish/actions/npm-pr-version@v3 with: npm-token: ${{ secrets.NPM_TOKEN }} # ❌ Bad - Exposing secrets - name: Debug run: echo "Token: ${{ secrets.NPM_TOKEN }}" -```yaml +```Apply the same fix at every closing fence (lines 99, 116, 186, 195, 236) — remove the language hint from the closing
```. And change line 277 from````back to a regular document-end (or remove it if nothing needs closing).
🤖 Fix all issues with AI agents
In `@npm-publish-pr/action.yml`:
- Around line 99-106: The tarball glob from the tarball input is never expanded
because the script treats INPUT_TARBALL as a literal (used as "$INPUT_TARBALL"
in the tarball_mode branch and in the existence check [ ! -f "$INPUT_TARBALL"
]); resolve the glob immediately after tarball_mode is detected by expanding
INPUT_TARBALL into a concrete filename (for example enable globbing/nullglob,
expand the pattern and set INPUT_TARBALL to the first match) before running the
file-exists check and the rest of the logic so subsequent uses of
"$INPUT_TARBALL" are safe and refer to a real file.
In `@npm-publish-pr/README.md`:
- Line 252: Replace the incorrect lockfile name `pnpm-lock.yml` with the correct
`pnpm-lock.yaml` in the README entry (the line showing "`pnpm-lock.yml` →
pnpm"); ensure the README text matches the action's check (action.yml uses
`pnpm-lock.yaml`) so both references are consistent.
In `@README.md`:
- Line 6: Replace the doctoc block wrappers that currently use "<!--
eslint-disable -->" / "<!-- eslint-enable -->" with Prettier ignore markers:
wrap the entire doctoc TOC section using "<!-- prettier-ignore-start -->" before
the block and "<!-- prettier-ignore-end -->" after it (ensure you update both
occurrences noted around the block, e.g., the markers currently at lines around
the top and the one at 22), so Prettier will skip formatting the
doctoc-generated TOC instead of disabling ESLint.
In `@SECURITY.md`:
- Around line 180-195: Two JSON example blocks showing malicious
"prepublishOnly" scripts use incorrect closing fences: the blocks start with
"```json" but are improperly closed with "```json" which opens new blocks;
locate the two example code blocks containing the "scripts": { "prepublishOnly":
... } JSON (the block that references $NPM_TOKEN and the block that references
$ACTIONS_ID_TOKEN_REQUEST_URL/$ACTIONS_ID_TOKEN_REQUEST_TOKEN) and replace their
closing fences from ```json to plain ``` so each JSON block is properly closed.
- Line 238: In SECURITY.md update the broken link: decide whether it should
point to npm-pr-version/README.md or npm-publish-pr/README.md (the diff
currently uses ./npm-publish-pr/README.md but the text says
npm-pr-version/README.md), then change the URL to the correct README and either
add or correct the anchor (replace or create the fragment matching the actual
heading name instead of `#security-considerations`) or remove the fragment
entirely so the link resolves; ensure the link text and target file
(npm-pr-version/README.md or npm-publish-pr/README.md) match and that the
referenced heading exists.
🧹 Nitpick comments (4)
tests/integration/npm-pr-version/tarball.bats (2)
16-32: Tests re-implement action logic inline rather than testing the actual action script.These tests validate the pattern of tarball mode detection by reimplementing the logic in
bash -cblocks, but they don't exercise the actual shell code fromnpm-publish-pr/action.yml. If the action's implementation drifts from these patterns, the tests will still pass while the action is broken. Consider extracting shared logic into a sourced script that both the action and tests use, or at minimum add a comment clarifying these are "logic pattern" tests, not integration tests of the action itself.
47-47: Hardcoded/tmp/package.json.testpath could collide across parallel test runs.Multiple tests write to
/tmp/package.json.test. While BATS defaults to sequential execution, usingmktempwould be more robust and consistent with the$TEST_DIRpattern already insetup().Example fix (apply similarly to all occurrences)
- tar -xzf "$INPUT_TARBALL" -O package/package.json > /tmp/package.json.test 2>/dev/null + tmpfile=$(mktemp) + tar -xzf "$INPUT_TARBALL" -O package/package.json > "$tmpfile" 2>/dev/null - if [ -s /tmp/package.json.test ]; then - package_name=$(jq -r ".name // empty" /tmp/package.json.test) + if [ -s "$tmpfile" ]; then + package_name=$(jq -r ".name // empty" "$tmpfile") echo "package-name=$package_name" else echo "error=Could not extract package.json" fi - rm -f /tmp/package.json.test + rm -f "$tmpfile"Also applies to: 75-75, 112-112, 199-199, 223-223
npm-publish-pr/action.yml (2)
110-119: Temporary file/tmp/package.json.tarballis never cleaned up.The extracted file is written to
/tmp/package.json.tarballbut never removed after use. Consider addingrm -f /tmp/package.json.tarballafter extracting the needed values (after line 137).
185-201: Package manager detection runs in tarball mode but the result is unused.When
tarball_mode=true, the publish path (lines 224-246) always usesnpmregardless of the detectedpackage_manager. The detection block at lines 192-201 still runs and logs "Detected package manager: yarn/pnpm", which would be misleading. Consider skipping detection in tarball mode or logging that tarball mode forces npm.Proposed fix
if [ "$INPUT_NPM_TOKEN" ]; then echo "🔐 Using token-based authentication" # Token mode: set NODE_AUTH_TOKEN so publishing works export NODE_AUTH_TOKEN="$INPUT_NPM_TOKEN" - # Detect package manager for token-based publishing - if [ -f "./yarn.lock" ]; then - package_manager="yarn" - echo "📦 Detected package manager: yarn" - elif [ -f "./pnpm-lock.yaml" ]; then - package_manager="pnpm" - echo "📦 Detected package manager: pnpm" - else - package_manager="npm" - echo "📦 Detected package manager: npm" - fi + if [ "$tarball_mode" = true ]; then + echo "📦 Using npm for tarball publishing (tarball mode always uses npm)" + else + # Detect package manager for token-based publishing + if [ -f "./yarn.lock" ]; then + package_manager="yarn" + echo "📦 Detected package manager: yarn" + elif [ -f "./pnpm-lock.yaml" ]; then + package_manager="pnpm" + echo "📦 Detected package manager: pnpm" + else + package_manager="npm" + echo "📦 Detected package manager: npm" + fi + fi fi
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Line 5: The doctoc TOC currently uses <!-- eslint-disable -->/<!--
eslint-enable --> which removed the Prettier guard; update the doctoc block to
include <!-- prettier-ignore-start --> and <!-- prettier-ignore-end --> around
the TOC and, if ESLint disabling is still required, stack the markers so both
appear (e.g., place <!-- prettier-ignore-start --> before <!-- eslint-disable
--> and <!-- prettier-ignore-end --> after <!-- eslint-enable -->) ensuring the
TOC is wrapped by the prettier-ignore markers while preserving any
eslint-disable lines.
In `@npm-publish-pr/action.yml`:
- Around line 66-69: The current unquoted glob expansion
tarball_files=($INPUT_TARBALL) will split filenames on spaces; change the logic
that builds tarball_files to use a safe glob expansion (e.g., use compgen -G
"$INPUT_TARBALL" or a loop that reads compgen output and appends each match as a
single quoted element) so filenames containing spaces are preserved, keeping the
surrounding shopt -s nullglob / shopt -u nullglob lines intact and ensuring each
entry in the tarball_files array is a single properly quoted filename.
In `@tests/integration/npm-pr-version/tarball.bats`:
- Around line 245-287: The test "npm-pr-version: tarball mode uses secure
temporary files (mktemp)" is fragile because the mktemp path regex and hardcoded
"permissions=600" assertion assume specific platforms; update the test to stop
asserting a specific TMPDIR pattern and exact permissions: in the subshell that
sets temp_file_path, keep the mktemp invocation and checks that produce
"mktemp-created=true" and "file-exists=true" but remove or relax the regex match
for temp file path and drop the strict "permissions=600" assertion; instead
assert only that the temp file was created and that the trap removed it (retain
the "trap-cleanup=true" check), or optionally gate the strict checks behind a
platform detection (e.g., check uname and only assert permissions on Linux/macOS
where known). Reference symbols: the test name, the temp_file_path variable, the
emitted markers "mktemp-created=true", "file-exists=true", "permissions=600",
and "trap-cleanup=true".
🧹 Nitpick comments (3)
tests/integration/npm-pr-version/tarball.bats (1)
16-32: Tests re-implement action logic inline rather than exercising the actual action code.All tests in this file replicate shell snippets from
action.ymlinstead of sourcing/invoking the real action script. If the action's logic diverges from these copies, the tests will still pass while the action is broken. Consider extracting the core tarball logic into a sourceable shell script that both the action and tests can share, or at minimum add a comment acknowledging this is intentional (testing the pattern, not the implementation).This is fine for an initial PR — just flagging the drift risk.
npm-publish-pr/README.md (1)
218-226: Thetarballinput is documented in the table but has no usage example in this README.The inputs table correctly lists the
tarballinput, and SECURITY.md has the two-job workflow example. However, users looking for how to use this action's tarball mode would naturally look here first. Consider adding a "Tarball Mode (Secure)" usage section alongside the existing OIDC and Token examples — even a brief one that references SECURITY.md for the full workflow.SECURITY.md (1)
173-176: Nit: "Open Source" as a compound adjective modifying "Projects" could be hyphenated.Per the static analysis hint, "open source project" used as a compound adjective before a noun is conventionally hyphenated: "open-source project" (line 175) and "Open-Source Projects" (line 173 heading).
codfish
force-pushed
the
feat/tarballs
branch
from
e179214 to
d2a4507
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@npm-publish-pr/action.yml`:
- Around line 118-166: Tarball mode currently reads version from the archive
(variables package_name/version) and publishes that unmodified; change it to
produce the PR-specific version like normal mode by computing the PR version
(e.g., "0.0.0-PR-{number}--{short-sha}") and applying it to the extracted
package.json before publishing: after extracting to temp_pkg_json update its
"version" field to the computed pr_version (or regenerate/rewrap the tarball
with the modified package.json), then set version=$pr_version and write
"version=$version" to GITHUB_OUTPUT; alternatively, if you prefer option B/C
implement a validation branch that checks extracted version matches the
0.0.0-PR-* pattern (using package_name/version) and fails with a clear error if
not, but do not leave the action publishing the original tarball version.
In `@npm-publish-pr/README.md`:
- Around line 129-168: The README's Tarball Mode example doesn't show
PR-specific versioning and thus will pack the original package.json version;
update the docs to either state explicitly that tarball mode uses the tarball's
existing package.json version (i.e., it does NOT auto-generate PR-specific
versions) or add a step in the build job before running npm pack that sets a
PR-specific version (e.g., run npm version with --no-git-tag-version using
GitHub variables) so the tarball contains the expected 0.0.0-PR-... format;
reference the build job steps (npm pack) and the publish step using
codfish/actions/npm-pr-version@v3 with tarball to make clear which behavior
applies.
- Around line 154-168: The publish job requires npm >=11.5.1 for OIDC but
doesn't upgrade npm; add a step in the "publish" job (before the
codfish/actions/npm-pr-version@v3 step and after the download-artifact step)
that upgrades/activates npm 11.5.1+, e.g. run Corepack to prepare/activate npm
11.5.1 (corepack enable && corepack prepare npm@11.5.1 --activate) or otherwise
install npm@11.5.1 globally so the npm-pr-version action
(codfish/actions/npm-pr-version@v3) can use OIDC successfully.
In `@SECURITY.md`:
- Around line 241-256: The publish job currently uses id-token: write for OIDC
but misses the setup step that ensures a new enough npm; add a step calling
setup-node-and-install@v3 (or the project's equivalent setup action) immediately
before the codfish/actions/npm-pr-version@v3 step in the publish job so npm is
upgraded to ^11.5.1 and OIDC publishing will succeed; ensure the step targets
the desired node version (e.g., node-version input) and appears before the uses:
codfish/actions/npm-pr-version@v3 entry.
🧹 Nitpick comments (3)
SECURITY.md (1)
173-176: Nit: "open source" → "open-source" when used as a compound adjective.Static analysis flags "open source project" — hyphenate when it modifies a noun.
-If you're an **open source project** using `pull_request_target` to publish PR packages from external contributors, +If you're an **open-source project** using `pull_request_target` to publish PR packages from external contributors,npm-publish-pr/action.yml (2)
74-74: Quote$GITHUB_OUTPUTfor robustness.Throughout the script,
$GITHUB_OUTPUTis used unquoted in>> $GITHUB_OUTPUT. While GitHub controls this path, quoting is a shell best practice to guard against paths with spaces.Example fix (apply throughout)
-echo "error-message=$error_message" >> $GITHUB_OUTPUT +echo "error-message=$error_message" >> "$GITHUB_OUTPUT"
249-271: Tarball mode always usesnpmeven when token-based auth with yarn/pnpm is detected.Lines 263–264 have a comment "Token mode with tarball - always use npm for tarball publishing", but this isn't explained to the user. If someone uses pnpm and provides an npm-token, they might expect pnpm to be used. Consider adding a log message explaining why npm is used (e.g., pnpm/yarn don't support publishing pre-built tarballs the same way), or documenting this in the README.
| # In tarball mode, extract package info from tarball | ||
| # In normal mode, validate package.json | ||
| if [ "$tarball_mode" = true ]; then | ||
| # Validate tarball exists | ||
| if [ ! -f "$INPUT_TARBALL" ]; then | ||
| error_message="❌ ERROR: Tarball not found at path: $INPUT_TARBALL" | ||
| echo "$error_message" | ||
| echo "error-message=$error_message" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate package.json is valid JSON | ||
| if ! jq empty package.json 2>/dev/null; then | ||
| error_message="❌ ERROR: package.json is not valid JSON" | ||
| echo "$error_message" | ||
| echo "error-message=$(extract_error "$error_message")" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
| echo "📦 Extracting package info from tarball: $INPUT_TARBALL" | ||
|
|
||
| # Check if package has a name | ||
| package_name=$(jq -r '.name // empty' package.json) | ||
| if [ -z "$package_name" ] || [ "$package_name" = "null" ]; then | ||
| error_message="❌ ERROR: package.json must have a 'name' field" | ||
| echo "$error_message" | ||
| echo "error-message=$(extract_error "$error_message")" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
| # Create secure temporary file with proper permissions | ||
| temp_pkg_json=$(mktemp) | ||
| trap 'rm -f "$temp_pkg_json"' EXIT | ||
|
|
||
| # Extract package.json from tarball to get name and version | ||
| # Note: Use -O without --wildcards for BSD tar (macOS) compatibility | ||
| tar -xzf "$INPUT_TARBALL" -O package/package.json > "$temp_pkg_json" 2>/dev/null | ||
|
|
||
| # Output package name for use in error handling | ||
| echo "package-name=$package_name" >> $GITHUB_OUTPUT | ||
| if [ ! -s "$temp_pkg_json" ]; then | ||
| error_message="❌ ERROR: Could not extract package.json from tarball" | ||
| echo "$error_message" | ||
| echo "error-message=$error_message" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Get package name from tarball | ||
| package_name=$(jq -r '.name // empty' "$temp_pkg_json") | ||
| if [ -z "$package_name" ] || [ "$package_name" = "null" ]; then | ||
| error_message="❌ ERROR: Tarball's package.json must have a 'name' field" | ||
| echo "$error_message" | ||
| echo "error-message=$error_message" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Get version from tarball | ||
| version=$(jq -r '.version // empty' "$temp_pkg_json") | ||
| if [ -z "$version" ] || [ "$version" = "null" ]; then | ||
| error_message="❌ ERROR: Tarball's package.json must have a 'version' field" | ||
| echo "$error_message" | ||
| echo "error-message=$error_message" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "📦 Tarball package: $package_name@$version" | ||
| echo "package-name=$package_name" >> $GITHUB_OUTPUT | ||
| echo "version=$version" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Tarball mode publishes the tarball's original version, not a PR-specific version.
In normal mode, the action generates 0.0.0-PR-{number}--{short-sha} and mutates package.json. In tarball mode (lines 155–166), the version is extracted from the tarball as-is — no PR-specific version is generated.
This means if the tarball contains "version": "1.0.0", the action publishes 1.0.0 to the pr tag, which:
- Breaks the action's core promise of PR-specific versions (
0.0.0-PR-…). - Could overwrite a real published version (version immutability violation on npm).
- None of the workflow examples (SECURITY.md, README.md) show the user mutating the version before
npm pack.
Consider either:
- (A) Generating the PR version inside the action and re-packing/rewriting the tarball's
package.json, or - (B) Requiring the user to pass the version pre-baked, and validating it matches the
0.0.0-PR-*pattern, or - (C) At minimum, clearly documenting that the user must set the version before
npm pack, and adding a warning if the extracted version doesn't look like a PR version.
🤖 Prompt for AI Agents
In `@npm-publish-pr/action.yml` around lines 118 - 166, Tarball mode currently
reads version from the archive (variables package_name/version) and publishes
that unmodified; change it to produce the PR-specific version like normal mode
by computing the PR version (e.g., "0.0.0-PR-{number}--{short-sha}") and
applying it to the extracted package.json before publishing: after extracting to
temp_pkg_json update its "version" field to the computed pr_version (or
regenerate/rewrap the tarball with the modified package.json), then set
version=$pr_version and write "version=$version" to GITHUB_OUTPUT;
alternatively, if you prefer option B/C implement a validation branch that
checks extracted version matches the 0.0.0-PR-* pattern (using
package_name/version) and fails with a clear error if not, but do not leave the
action publishing the original tarball version.
| ### Tarball Mode (Secure for pull_request_target) | ||
|
|
||
| For `pull_request_target` workflows, use tarball mode to prevent execution of malicious lifecycle scripts from untrusted | ||
| PRs: | ||
|
|
||
| ```yml | ||
| on: pull_request_target | ||
|
|
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
|
|
||
| - uses: codfish/actions/setup-node-and-install@v3 | ||
| - run: npm run build | ||
| - run: npm pack | ||
|
|
||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: package-tarball | ||
| path: '*.tgz' | ||
|
|
||
| publish: | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| id-token: write | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| name: package-tarball | ||
|
|
||
| - uses: codfish/actions/npm-pr-version@v3 | ||
| with: | ||
| tarball: '*.tgz' # Publishes with --ignore-scripts | ||
| ``` |
There was a problem hiding this comment.
Tarball mode examples don't show PR-specific version generation.
As noted in the action.yml review, tarball mode uses the version from the tarball's package.json as-is. The build job (lines 141–152) runs npm pack without first setting a PR-specific version, so the published package will have whatever version is in the source package.json (e.g., 1.0.0), not the expected 0.0.0-PR-123--abc1234 format.
If this is by design, the documentation should explicitly state that tarball mode does not generate PR-specific versions, and show how users should set the version before packing (e.g., npm version 0.0.0-PR-${{ github.event.number }}--... --no-git-tag-version).
🤖 Prompt for AI Agents
In `@npm-publish-pr/README.md` around lines 129 - 168, The README's Tarball Mode
example doesn't show PR-specific versioning and thus will pack the original
package.json version; update the docs to either state explicitly that tarball
mode uses the tarball's existing package.json version (i.e., it does NOT
auto-generate PR-specific versions) or add a step in the build job before
running npm pack that sets a PR-specific version (e.g., run npm version with
--no-git-tag-version using GitHub variables) so the tarball contains the
expected 0.0.0-PR-... format; reference the build job steps (npm pack) and the
publish step using codfish/actions/npm-pr-version@v3 with tarball to make clear
which behavior applies.
| publish: | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| id-token: write | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| name: package-tarball | ||
|
|
||
| - uses: codfish/actions/npm-pr-version@v3 | ||
| with: | ||
| tarball: '*.tgz' # Publishes with --ignore-scripts | ||
| ``` |
There was a problem hiding this comment.
Tarball publish job needs npm 11.5.1+ for OIDC but has no upgrade step.
The publish job uses OIDC (id-token: write) which requires npm 11.5.1+. GitHub-hosted runners ship with an older npm. Without a checkout step, setup-node-and-install@v3 can't be used (it needs package.json).
Consider adding a manual npm upgrade:
Proposed fix
steps:
- uses: actions/download-artifact@v4
with:
name: package-tarball
+ - name: Upgrade npm for OIDC support
+ run: npm install -g npm@^11.5.1
+
- uses: codfish/actions/npm-pr-version@v3
with:
tarball: '*.tgz' # Publishes with --ignore-scriptsBased on learnings: "The npm-pr-version action requires npm 11.5.1+ for OIDC (automatically provided by setup-node-and-install v3)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publish: | |
| needs: build | |
| runs-on: ubuntu-latest | |
| permissions: | |
| id-token: write | |
| pull-requests: write | |
| steps: | |
| - uses: actions/download-artifact@v4 | |
| with: | |
| name: package-tarball | |
| - uses: codfish/actions/npm-pr-version@v3 | |
| with: | |
| tarball: '*.tgz' # Publishes with --ignore-scripts | |
| ``` | |
| publish: | |
| needs: build | |
| runs-on: ubuntu-latest | |
| permissions: | |
| id-token: write | |
| pull-requests: write | |
| steps: | |
| - uses: actions/download-artifact@v4 | |
| with: | |
| name: package-tarball | |
| - name: Upgrade npm for OIDC support | |
| run: npm install -g npm@^11.5.1 | |
| - uses: codfish/actions/npm-pr-version@v3 | |
| with: | |
| tarball: '*.tgz' # Publishes with --ignore-scripts |
🤖 Prompt for AI Agents
In `@npm-publish-pr/README.md` around lines 154 - 168, The publish job requires
npm >=11.5.1 for OIDC but doesn't upgrade npm; add a step in the "publish" job
(before the codfish/actions/npm-pr-version@v3 step and after the
download-artifact step) that upgrades/activates npm 11.5.1+, e.g. run Corepack
to prepare/activate npm 11.5.1 (corepack enable && corepack prepare npm@11.5.1
--activate) or otherwise install npm@11.5.1 globally so the npm-pr-version
action (codfish/actions/npm-pr-version@v3) can use OIDC successfully.
| publish: | ||
| needs: build | ||
| # Publish with secrets and --ignore-scripts | ||
| permissions: | ||
| contents: read | ||
| id-token: write # For OIDC (or omit if using npm-token) | ||
| pull-requests: write # For commenting | ||
| steps: | ||
| - uses: actions/checkout@v6 # Trusted base branch | ||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| name: package-tarball | ||
| - uses: codfish/actions/npm-pr-version@v3 | ||
| with: | ||
| tarball: '*.tgz' | ||
| ``` |
There was a problem hiding this comment.
Publish job example may need setup-node-and-install@v3 for OIDC.
The publish job uses id-token: write (OIDC), which requires npm 11.5.1+. GitHub-hosted runners ship with an older npm version. Without setup-node-and-install@v3 (which auto-upgrades npm to ^11.5.1), OIDC publishing will fail.
Consider adding the setup step before the publish step, consistent with the npm-publish-pr/README.md tarball example:
Proposed fix
steps:
- uses: actions/checkout@v6 # Trusted base branch
+ - uses: codfish/actions/setup-node-and-install@v3
- uses: actions/download-artifact@v4
with:
name: package-tarball
- uses: codfish/actions/npm-pr-version@v3
with:
tarball: '*.tgz'Based on learnings: "The npm-pr-version action requires npm 11.5.1+ for OIDC (automatically provided by setup-node-and-install v3)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publish: | |
| needs: build | |
| # Publish with secrets and --ignore-scripts | |
| permissions: | |
| contents: read | |
| id-token: write # For OIDC (or omit if using npm-token) | |
| pull-requests: write # For commenting | |
| steps: | |
| - uses: actions/checkout@v6 # Trusted base branch | |
| - uses: actions/download-artifact@v4 | |
| with: | |
| name: package-tarball | |
| - uses: codfish/actions/npm-pr-version@v3 | |
| with: | |
| tarball: '*.tgz' | |
| ``` | |
| publish: | |
| needs: build | |
| # Publish with secrets and --ignore-scripts | |
| permissions: | |
| contents: read | |
| id-token: write # For OIDC (or omit if using npm-token) | |
| pull-requests: write # For commenting | |
| steps: | |
| - uses: actions/checkout@v6 # Trusted base branch | |
| - uses: codfish/actions/setup-node-and-install@v3 | |
| - uses: actions/download-artifact@v4 | |
| with: | |
| name: package-tarball | |
| - uses: codfish/actions/npm-pr-version@v3 | |
| with: | |
| tarball: '*.tgz' |
🤖 Prompt for AI Agents
In `@SECURITY.md` around lines 241 - 256, The publish job currently uses id-token:
write for OIDC but misses the setup step that ensures a new enough npm; add a
step calling setup-node-and-install@v3 (or the project's equivalent setup
action) immediately before the codfish/actions/npm-pr-version@v3 step in the
publish job so npm is upgraded to ^11.5.1 and OIDC publishing will succeed;
ensure the step targets the desired node version (e.g., node-version input) and
appears before the uses: codfish/actions/npm-pr-version@v3 entry.
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Note
Medium Risk
Changes the
npm-pr-versionpublishing path and adds new tarball-based behavior that affects how packages are built/versioned/published; mistakes could break PR publishing or alter what gets released, though the change is scoped and includes new integration tests.Overview
Adds a new
tarballinput tonpm-pr-versionto publish a pre-built*.tgzvianpm publish ... --ignore-scripts, extractingname/versionfrom the tarball and bypassingpackage.jsonversion mutation when enabled.Updates docs to recommend a two-job
pull_request_targetworkflow (build untrusted code →npm/pnpm packartifact → publish from trusted base) and expands security guidance around lifecycle-script credential exfiltration; also adjusts doc generation to preferymlcode fences and adds Bats integration tests plus arefute_output_containshelper.Written by Cursor Bugbot for commit 3048dc9. Configure here.
Summary by CodeRabbit
New Features
Documentation
Tests
Style