fix(publish): tolerate post-publish provenance flake in npm publish step - #304
Conversation
Run 30117865418 published every package (incl. the agentworkforce umbrella) to npm at 4.1.35, but the job still failed because npm's provenance attestation makes a second authenticated call after the tarball PUT, and that call 401'd with "Failed to generate Web Auth URLs" even though the publish itself had already landed. The failure skipped tag+push and the GitHub Release for an already-shipped version. Now a non-zero exit from npm publish is checked against the registry before failing the job: if the version is already live, warn and move on instead of aborting the whole lockstep run.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe publish workflow now handles ChangesPublish verification
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://cdn.github.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: 343068cb16
ℹ️ 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".
| PUBLISHED_VERSION=$(npm view "$NPM_NAME@$VERSION" version 2>/dev/null || true) | ||
| if [ "$PUBLISHED_VERSION" = "$VERSION" ]; then |
There was a problem hiding this comment.
If another workflow or one-off branch publishes the same name@version after the earlier preflight check (lines 234–252) but before this command, npm publish will correctly fail for a duplicate version, while npm view here returns that version and makes this run continue. The subsequent Tag + push and GitHub Release would then associate the current commit with an artifact produced by the other publisher; compare the packed tarball's integrity (or otherwise prove it is this run's artifact) before treating a nonzero publish as the provenance-only flake.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Around line 617-623: Replace the registry-existence fallback in the
publish/create-release flow with run-specific serialization or an integrity
check proving this run’s package was published, rather than relying solely on
npm view. Only continue for the known post-publish provenance/attestation
failure after confirming the published tarball or checksum matches this run;
otherwise preserve the failure path and prevent release creation.
- Around line 613-616: Update the shell condition in the publish failure
handling block to read the dry-run value from a step environment variable such
as DRY_RUN, rather than interpolating github.event.inputs.dry_run directly into
Bash source; compare the quoted "$DRY_RUN" value to "true" and configure the
environment assignment through the workflow mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a43aa1bb-9384-4325-b435-637edee16315
📒 Files selected for processing (1)
.github/workflows/publish.yml
| if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then | ||
| echo "::error::npm publish failed for $NPM_NAME@$VERSION (dry run)" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files around workflow =="
git ls-files .github/workflows | sed -n '1,120p'
echo
echo "== targeted publish.yml lines =="
sed -n '200,270p;600,630p' .github/workflows/publish.yml 2>/dev/null | nl -ba | sed -n '1,220p'
echo
echo "== input declarations and step run context =="
rg -n "inputs:|dry_run|if \[ \"\$\{\{ github\.event\.inputs\.dry_run \}\}\"" .github/workflows/publish.yml || true
echo
echo "== static shell injection probe for direct workflow input interpolation =="
python3 - <<'PY'
from pathlib import Path
p=Path('.github/workflows/publish.yml')
s=p.read_text()
needle='${{ github.event.inputs.dry_run }}'
print('needle_present=', needle in s)
idx=s.find(needle)
context=s[max(0, s.find('run: |',idx-300)):s.find('run: |',idx)+400] if idx!=-1 else ''
print('contains_direct_interpolation_in_run=', needle in context)
print('context_sample_800=', context[:800])
try:
# Simulate Bash expansion within the quoted test by constructing a value that would break the test.
# The expression `${{ github.event.inputs.dry_run }}` is not Bash syntax; when replaced with an exploitable input such as:
# if [ "$(command_injection)" = "true" ]; then
# Bash parses command_injection before comparing the command's stdout, which is unsafe regardless of the outer quotes.
input = '$(rm -rf /)'
line = f'if [ "{input}" = "true" ]; then'
print('injected_line=', line)
print('quoted_preserves_value_in_python_shell=False - this is why it is a shell injection probe, not executable.')
except Exception as e:
print('probe_error=', e)
PYRepository: AgentWorkforce/workforce
Length of output: 486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== publish.yml relevant sections =="
sed -n '220,260p;600,630p' .github/workflows/publish.yml
echo
echo "== input declarations and direct interpolation =="
grep -nE 'inputs:|dry_run|github\.event\.inputs\.dry_run' .github/workflows/publish.yml || true
echo
echo "== step context around publish name =="
grep -n -B10 -A20 'name: Pack + publish|Dry run|dry_run' .github/workflows/publish.yml || true
python3 - <<'PY'
from pathlib import Path
text = Path('.github/workflows/publish.yml').read_text()
lines = text.splitlines()
for i,line in enumerate(lines, 1):
if 'github.event.inputs.dry_run' in line:
print(f'direct_input_interpolation_line:{i}:{line}')
PYRepository: AgentWorkforce/workforce
Length of output: 5983
Do not interpolate dry_run directly into shell source.
${{ github.event.inputs.dry_run }} is expanded before Bash parses the script, so an unsafe input value can break the quoted condition and execute commands on the runner. Pass it through the step environment and compare "$DRY_RUN" instead.
Proposed fix
- name: Pack + publish
+ env:
+ DRY_RUN: ${{ github.event.inputs.dry_run }}
run: |
...
- if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then
+ if [ "$DRY_RUN" = "true" ]; then📝 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.
| if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then | |
| echo "::error::npm publish failed for $NPM_NAME@$VERSION (dry run)" >&2 | |
| exit 1 | |
| fi | |
| - name: Pack + publish | |
| env: | |
| DRY_RUN: ${{ github.event.inputs.dry_run }} | |
| run: | | |
| if [ "$DRY_RUN" = "true" ]; then | |
| echo "::error::npm publish failed for $NPM_NAME@$VERSION (dry run)" >&2 | |
| exit 1 | |
| fi |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 613-613: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/publish.yml around lines 613 - 616, Update the shell
condition in the publish failure handling block to read the dry-run value from a
step environment variable such as DRY_RUN, rather than interpolating
github.event.inputs.dry_run directly into Bash source; compare the quoted
"$DRY_RUN" value to "true" and configure the environment assignment through the
workflow mechanism.
Source: Linters/SAST tools
| PUBLISHED_VERSION=$(npm view "$NPM_NAME@$VERSION" version 2>/dev/null || true) | ||
| if [ "$PUBLISHED_VERSION" = "$VERSION" ]; then | ||
| echo "::warning::npm publish exited non-zero for $NPM_NAME@$VERSION but the registry already has that version (likely a post-publish attestation flake) - continuing" | ||
| else | ||
| echo "::error::npm publish failed for $NPM_NAME@$VERSION and the registry does not have it" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow file overview =="
wc -l .github/workflows/publish.yml
printf '\n== publish.yml outline / key occurrences =='
rg -n "PUBLISHED_VERSION|npm publish|npm view|unpublished|attestation|release|tag|workflow-run|concurrency|matrix" .github/workflows/publish.yml || true
printf '\n== relevant lines 560-650 =='
sed -n '560,650p' .github/workflows/publish.ymlRepository: AgentWorkforce/workforce
Length of output: 13474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== concurrency block =="
sed -n '1,80p' .github/workflows/publish.yml
printf '\n== publish job definition around pack+publish tagging =="
sed -n '380,450p' .github/workflows/publish.yml
printf '\n== create-release trigger/outputs wiring =="
sed -n '632,712p' .github/workflows/publish.ymlRepository: AgentWorkforce/workforce
Length of output: 2277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== publish job inputs/targets =="
sed -n '80,130p' .github/workflows/publish.yml
printf '\n== create-release workflow dependencies =='
sed -n '632,712p' .github/workflows/publish.yml
printf '\n== behavioral probe: compare npm publish exit path and registry check semantics =='
python3 - <<'PY'
from pathlib import Path
import textwrap
p = Path(".github/workflows/publish.yml")
s = p.read_text()
checks = {
"concurrency_cancel_in_progress_false": "cancel-in-progress: false" in s and "group: publish-${{ github.ref }}" in s,
"no_pnpm_rsync_mutex": "pnpm-rsync" not in s and "pnpm-lock.yaml" not in s.split("name: Pack + publish")[1].split("name: Tag + push")[0],
"only_registry_check_on_nonzero": "PUBLISHED_VERSION=$(npm view \"$NPM_NAME@$VERSION\" version 2>/dev/null || true)" in s,
"no_sha_or_tarball_integrity": "sha256" not in textwrap.dedent(s.split("name: Pack + publish")[1].split("name: Tag + push")[0]) and "npm pack" not in textwrap.dedent(s.split("name: Pack + publish")[1].split("name: Tag + push")[0]),
"creates_release": "softprops/action-gh-release@example" not in s and "softprops/action-gh-release@v3" in s,
}
for k,v in checks.items():
print(f"{k}={v}")
PYRepository: AgentWorkforce/workforce
Length of output: 6346
Do not treat registry existence as proof that this run published the package.
publish/create-release can still proceed after a failed publish if another concurrent or historical run already published npm view "$NPM_NAME@$VERSION" succeeds, then this run tags/releases the same version without this run’s tarball/publish attestation reaching the registry. Add a publish-run serializer or checksum/integrity check, and only tolerate the known post-publish provenance failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/publish.yml around lines 617 - 623, Replace the
registry-existence fallback in the publish/create-release flow with run-specific
serialization or an integrity check proving this run’s package was published,
rather than relying solely on npm view. Only continue for the known post-publish
provenance/attestation failure after confirming the published tarball or
checksum matches this run; otherwise preserve the failure path and prevent
release creation.
1 participant
Summary
agentworkforceumbrella — to npm at4.1.35, but the job still reported failure.--provenanceflow makes a second authenticated call after the tarballPUTto attach the attestation. That second call 401'd (Failed to generate Web Auth URLs ... token is invalid) even though the publish itself had already landed on the registry (confirmed vianpm view <pkg> version).Tag + pushand the GitHub Release step never ran, leavingmainwithout the version bump/changelog/tags for an already-shipped4.1.35.Fix
npm publishexits non-zero, check the registry for that exactname@versionbefore failing the run. If it's already live, warn and continue instead of aborting the whole multi-package publish. Dry runs are unaffected (nothing to check against, so a failure there still fails hard).Test plan