fix(composer-update): don't abort PR when all unhandled packages are no-widen#30
Merged
Merged
Conversation
…no-widen When the constraint-blocked set (Step 2) consists entirely of packages listed in `extra.vuln-scan.no-widen`, they're all skipped and `TARGETS` ends up empty. The dedupe line then runs `grep -v '^$'` over empty input, which exits 1 on no match. Composite-action shells run with `set -eo pipefail`, so that non-zero exit propagated and killed the whole "Update packages" step — aborting PR creation even when Step 1 had already updated other vulnerable packages within their constraints. Real-world effect: a repo with safely-updatable symfony CVEs *and* a few deliberately pinned (no-widen) WooCommerce plugins got NO security PR at all — the safe fixes were silently dropped along with the skipped ones. Swallow the empty-match exit on the dedupe line so the step falls through to the git-diff check and still raises a PR for whatever updated. Apply the same `|| true` guard to find_direct_ancestors, whose final `grep -v '^$'` has the identical "empty result is valid, not an error" property and is consumed in pipefail contexts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the vulnerability scan finds packages it can't update within their
composer.jsonconstraints (Step 2 / widen), and every one of them is listed inextra.vuln-scan.no-widen, they all get skipped andTARGETSends up empty. The dedupe line then runs:TARGETS=$(echo "$TARGETS" | tr ' ' '\n' | grep -v '^$' | sort -u | tr '\n' ' ')grep -v '^$'over empty input matches nothing and exits 1. Composite-action steps run withset -eo pipefail, so that non-zero exit propagates and kills the whole "Update packages" step — aborting PR creation entirely, even when Step 1 already updated other vulnerable packages within their constraints.Real-world impact
Seen on
generoi/solarplexius: the scan flagged 4 safely-updatablesymfony/*CVEs and 3 deliberately-pinned (no-widen) WooCommerce plugins. The 3 pins were correctly skipped — but the empty-TARGETSgrepthen exited 1, so the step failed and no security PR was opened at all. The safe symfony fixes were silently dropped along with the intentionally-skipped ones. (I raised that repo's symfony PR manually in the meantime: generoi/solarplexius#83.)Fix
Add
|| trueto the dedupe pipeline so the empty-match exit is swallowed and the step falls through to the existinggit diffcheck — which still sees Step 1's changes and raises a PR for whatever updated.Also hardened
find_direct_ancestors, whose finalgrep -v '^$'has the identical "empty result is a valid outcome, not an error" property and is consumed in$(...)/pipe contexts under pipefail (latent sibling of the same bug).Verification
Reproduced the failure and confirmed the fix with a minimal harness mirroring the Step 2 path under
set -eo pipefail:TARGETS→grep -v '^$'→ step exits 1 (PR aborted).git diff/changed=truebranch, PR is created for the packages that did update.Note for release
The scheduled scans consume this via
composer-update@v2(andvulnerability-scan.yml@v2). Thev2tag needs to be re-pointed to include this commit for the fix to take effect in running workflows.🤖 Generated with Claude Code