PL-916 [Vanta] Remediate High vulnerabilities identified in packages are addressed (GitHub Repo) - #14
Open
anush wants to merge 2 commits into
Open
PL-916 [Vanta] Remediate High vulnerabilities identified in packages are addressed (GitHub Repo)#14anush wants to merge 2 commits into
anush wants to merge 2 commits into
Conversation
Raises the phpunit/phpunit dev constraint from "^6" to "^8.5.52" to clear GHSA-vvj3-c3rp-c85p (unsafe deserialization in PHPT code coverage handling, high, vulnerable range "< 8.5.52"). This is a direct cross-major bump (6 -> 8) of a require-dev dependency, approved as an explicit exception: phpunit is a test runner that never reaches consumers of this bundle, and the suite was verified green on 8.5.53 beforehand. There is no patch on the 6.x line -- the highest available 6.x is 6.5.14, so no in-major fix exists. This repo has no composer.lock (it is gitignored), so the manifest constraint is the only available lever. This bump also REPAIRS an existing failure on master: phpunit 6.5's phpunit-mock-objects generator calls ReflectionType::__toString(), which is deprecated in PHP 7.4, and convertNoticesToExceptions turns each into an error. Tests go from 21 tests / 19 errors before to 21/21 passing after. No source changes were required. Note: phpunit 8 rejects the "syntaxCheck" attribute at phpunit.xml.dist line 11 and prints a config-validation warning. Tests still run and pass. Left untouched here; recommended as a follow-up cleanup. Fixes: https://app.vanta.com/c/xola.com/tests/packages-checked-for-vulnerabilities-v2-records-closed-github-dependabot-high Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe development dependency constraint for ChangesPHPUnit upgrade
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@composer.json`:
- Line 21: Align the PHP platform constraint with the phpunit/phpunit ^8.5.52
requirement by raising the supported PHP minimum to >=7.2, or instead pin
PHPUnit to a compatible 7.x release if PHP 7.0/7.1 support must remain. Update
the relevant PHP constraint in composer.json while preserving the intended
runtime support policy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2e428f6-d7ae-4728-b097-b10001e69d32
📒 Files selected for processing (1)
composer.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
xola/x2-seller(manual) → reviewed against open PR#6794vanta/dependabot-high-2026-07-24instead of the default branchxola/x2-checkout(manual)
📜 Review details
🔇 Additional comments (1)
composer.json (1)
21-21: 🗄️ Data Integrity & Integration
composer.lockisn’t tracked hereThis repo only includes
composer.json, so there’s no lockfile to regenerate.> Likely an incorrect or invalid review comment.
require-dev pins phpunit/phpunit ^8.5.52, which requires PHP >=7.2, and .travis.yml only tests PHP 7.2/7.3/7.4. The previous php:">=7" constraint falsely advertised support for PHP 7.0/7.1, which are neither tested nor compatible with the dev dependency. composer.lock is gitignored in this repo so no lockfile change is needed. Co-Authored-By: Claude Opus 5 (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.
Summary
Clears the single open high Dependabot alert on this repo by raising the
phpunit/phpunitdev constraint. As a bonus, it also fixes the currently-broken test suite onmaster— see below.Important
This is a direct cross-major bump (6 → 8) of a
require-devdependency, landed as an explicit, deliberately-granted exception to our usual "no direct major bumps" rule. It is safe here because phpunit is a test runner that never reaches consumers of this bundle, and the suite was verified green on 8.5.53 before this PR was opened.Packages upgraded
phpunit/phpunit^6(resolved 6.5.14)^8.5.52(resolves 8.5.53)require-devComposer also drops five now-unneeded transitive dev packages that phpunit 6 pulled in and phpunit 8 does not:
phpunit/phpunit-mock-objects(already marked abandoned; its mock generator moved in-tree in phpunit 7+),phpspec/prophecy,phpdocumentor/reflection-docblock,phpdocumentor/reflection-common,phpdocumentor/type-resolver. No production dependency changes.Advisory: PHPUnit vulnerable to unsafe deserialization in PHPT code coverage handling. Severity high. Vulnerable range
< 8.5.52; first patched8.5.52.Why the major bump was unavoidable
6.5.14, which is still inside the vulnerable range — so no in-major fix exists.composer.lock(it is gitignored), so there is no lockfile-only lever.composer update phpunit/phpunit --with-dependenciesrefuses outright: "Cannot update only a partial set of packages without a lock file present." The manifest constraint is the only thing that can move.8.5.53was chosen over the bare8.5.52floor and checked against every known phpunit advisory — the two 2026-04 advisories (GHSA-mh6w-vxff-9wqp,GHSA-qrr6-mg7r-m243) only affect the 12.5.21 / 13.1.5 lines, so 8.5.53 is clean.This PR repairs a pre-existing failure on
mastermasteris currently red, independently of this change. On PHP 7.4, phpunit 6.5'sphpunit-mock-objectsgenerator callsReflectionType::__toString(), which PHP 7.4 deprecated;phpunit.xml.distsetsconvertNoticesToExceptions="true", so every one of those deprecations becomes a test error:Baseline on
origin/master: 21 tests, 2 assertions, 19 errors.On this branch: 21 tests, 72 assertions, 0 errors.
So this is not merely a version-number change — it takes the suite from 19 errors to fully green, and restores 70 assertions that were never actually reached before.
Residual
0. This was the only open high-severity Dependabot alert on the repo.
Behavioural caveats
1.
phpunit.xml.dist:11uses asyntaxCheckattribute that phpunit 8 rejects. Running the suite now prints:This is cosmetic — the attribute was a no-op removed in phpunit 7, all 21 tests still run and pass. I have deliberately not edited it in this PR to keep the diff to the single dependency line. Recommended follow-up: delete the
syntaxCheckattribute fromphpunit.xml.distto silence the warning.2. No
: voidsignature problem here. phpunit 8's main breaking change is requiring: voidonsetUp()/tearDown()/setUpBeforeClass(). This repo's tests are unaffected, so no source changes were needed and none were made — the diff is one line incomposer.json.Smoke test for the reviewer
Expect
OK (21 tests, 72 assertions)andPHPUnit 8.5.53in the header (plus the cosmeticsyntaxCheckwarning above). The meaningful check isTests/Service/OmnipayTest.php, which exercises real gateway construction across the Omnipay drivers (authorizenet,stripe,paypal, …) viagetMockBuilder()— precisely the mock machinery that phpunit 8 rewrote and that was erroring out onmaster.Verification
All commands run locally on PHP 7.4.14 / Composer 2.2.26, baseline captured on
origin/masterfirst, then compared like-for-like.origin/master, phpunit 6.5.14)composer validatesymfony/framework-bundleunbound>=2.1)composer install --no-scriptsvendor/bin/phpunitSemver proof (the actual success measure, not an audit tail): the resolved version was checked against the advisory's
vulnerable_version_rangeon both sides. Baseline resolves6.5.14, which satisfies< 8.5.52→ vulnerable, confirming the check is not vacuous. This branch resolves8.5.53, which does not satisfy< 8.5.52→ cleared.Fixes: https://app.vanta.com/c/xola.com/tests/packages-checked-for-vulnerabilities-v2-records-closed-github-dependabot-high
🤖 Generated with Claude Code
Summary by CodeRabbit