fix(composer-update): extract update logic to script + fix no-widen abort + tests#31
Merged
Merged
Conversation
…abort Two problems, one root area: 1. The inline "Update packages" run block grew past GitHub's 21000-char compiled-expression limit (`The template is not valid ... Exceeded max expression length 21000`), so the action failed to even start — breaking every repo's scheduled scan. Extract the ~470-line block to composer-update/scripts/update.sh, invoked via `bash "$GITHUB_ACTION_PATH/scripts/update.sh"`. Inputs pass through env (PACKAGES, VULNS_JSON); the script keeps `set -eo pipefail` to mirror the composite shell. No more inline-expression size ceiling, and the logic is now unit-testable. 2. When every constraint-blocked package is in extra.vuln-scan.no-widen, the widen-targets dedupe ran `grep -v '^$'` over empty input, which exits 1 and (under pipefail) aborted the step — dropping the PR even when other packages updated cleanly. Guard with `|| true` (same fix on find_direct_ancestors, whose empty-result grep had the identical property). Add a regression test (composer-update/tests/no-widen-still-creates-pr.test.sh) that drives the real script with a fake composer and asserts a safe package still updates while a no-widen package is skipped without aborting. Verified it fails on the pre-fix logic and passes after. Wire it into CI via .github/workflows/composer-update-tests.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oxyc
added a commit
that referenced
this pull request
May 29, 2026
… (+ test suite) (#35) * test(composer-update): unit + integration tests; extract helpers to lib.sh The composer-update logic had no tests and was fragile (every recent fix — #20–#31 — was a production-only discovery). Add a real test suite and make the logic testable. - Extract the helper functions (build_pkg_arg, build_widen_arg, find_direct_ancestors, loosen_constraint, is_still_vulnerable, expand_args_for, get_lock_version) from update.sh into a sourceable scripts/lib.sh. update.sh sources it; behavior is unchanged (the existing no-widen integration test still passes). - Unit tests for the PHP helpers (the fragile version-range logic): - compute-min-safe-constraints: exclusive/inclusive bounds, missing version components, multi-range selection, no-entry fall-throughs. - is-still-vulnerable: in/out of range, multi-range, junk-input fail-safe. - Unit tests for the bash helpers, each tied to the edge case it guards: #27 build_pkg_arg trailing newline, #29 build_widen_arg caret widen, #28 loosen_constraint, #22/#26 find_direct_ancestors BFS + expand_args_for. - Integration tests driving the real update.sh with a fake composer: #24 downgrade revert, #21 dev-* revert, #23 per-package retry isolation, #20 no-widen honored as a JSON array. (Plus the existing no-widen test.) - Fix surfaced by the tests: compute-min-safe-constraints emitted `[]` for an empty result, but update.sh string-indexes that map with `jq '.[$pkg]'`, which errors on a JSON array under `set -eo pipefail`. Encode as `{}`. - CI: composer-update-tests.yml now sets up PHP and runs tests/run.sh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(composer-update): inclusive-bound (<=) safe-version derivation skips 4-segment hotfixes compute-min-safe-constraints derived ~X.Y.(Z+1) for a <=X.Y.Z advisory. That skips a 4-segment hotfix like X.Y.Z.1, which is >X.Y.Z but <X.Y.(Z+1): the constraint matched no published version, the update no-opped, and the vuln went unpatched (no PR opened). Real case: suomentyokalu / seo-by-rank-math, advisory <=1.0.271, fixed in 1.0.271.1. The derived ~1.0.272 could not resolve. Emit >X.Y.Z,<X.(Y+1).0 instead — a strict-greater lower bound that matches the hotfix while still excluding the vulnerable boundary X.Y.Z, still minor-capped. Teach loosen_constraint() and build_widen_arg() the new range shape (major-cap widen/loosen, boundary stays excluded). Tests: regression case asserting <=1.0.271 -> >1.0.271,<1.1.0 plus Semver checks that 1.0.271.1 is reachable and 1.0.271 is excluded; updated existing <= assertions; helper-fn coverage for the range shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: test <test@example.com> 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
1 participant
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.
Why
Two issues in
composer-update, plus the testing you asked about.1.
masteris currently broken (regression from #30)#30 added explanatory comments to the inline "Update packages"
run:block, which pushed it past GitHub's 21000-char compiled-expression limit. Every scheduled scan now fails before running:(Seen when re-triggering the solarplexius scan after pointing
v2at #30.)Fix: extract the ~470-line block into
composer-update/scripts/update.sh, invoked asbash "$GITHUB_ACTION_PATH/scripts/update.sh". Inputs pass throughenv(PACKAGES,VULNS_JSON); the script setsset -eo pipefailto mirror the composite shell exactly. This removes the inline-size ceiling for good and makes the logic testable. Net behavior is unchanged — only 3 expression refs existed in the block (github.action_path→$GITHUB_ACTION_PATH,inputs.packages→$PACKAGES,inputs.vulns_json→ existing env).2. The original no-widen abort (the bug #30 set out to fix)
When every constraint-blocked package is in
extra.vuln-scan.no-widen, the widen-targets dedupe runsgrep -v '^$'over empty input → exit 1 → underpipefailthe step dies and no PR is raised, even when other packages (e.g. symfony CVEs) updated cleanly. Guarded with|| true(same onfind_direct_ancestors, identical empty-result property). This is the real fix that #30 intended but couldn't deliver because of #1.3. Tests (per request)
composer-update/tests/no-widen-still-creates-pr.test.sh— drives the realupdate.shwith a fakecomposer/git(no network), asserting a safe package still updates andchanged=trueis emitted while a no-widen package is skipped without aborting..github/workflows/composer-update-tests.ymlrunscomposer-update/tests/*.test.shon changes to the action. The extraction makes adding more cases straightforward.Note
My apologies — the size regression in #30 was mine (the verbose comments). This supersedes it and hardens against recurrence. After merge,
v2needs re-pointing to include this commit.🤖 Generated with Claude Code