From 35024d1b4abeb13fcff3e2f5e98451c2770fc5c0 Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:16:11 +0200 Subject: [PATCH 1/8] [ENG-2586] Harden version comparison with stricter normalization and expanded tests - Extract normalizeVersion() to safely strip v-prefix without corrupting versions containing 'v' in body (e.g., 6.3-revision-0, 5.0.37.preview) - Use strict === comparisons and explicit empty checks instead of loose == - Fix range detection regex to avoid misinterpreting dashed versions as ranges - Replace strpos/stripos with str_contains/str_starts_with (PHP 8.1+) - Add edge case tests: scientific notation coercion, whitespace-only input, malformed operators, compound dashes, decimal strategy coverage --- src/CheckVersionVulnerability.php | 33 ++++++---- .../CheckDrupalVersionVulnerability.php | 31 ++++++---- tests/CheckVersionVulnerabilityTest.php | 61 +++++++++++++++++++ .../CheckDrupalVersionVulnerabilityTest.php | 33 ++++++++++ tests/VersionStrategyTest.php | 10 +++ 5 files changed, 145 insertions(+), 23 deletions(-) create mode 100644 tests/VersionStrategyTest.php diff --git a/src/CheckVersionVulnerability.php b/src/CheckVersionVulnerability.php index 538ecad..8742b8b 100644 --- a/src/CheckVersionVulnerability.php +++ b/src/CheckVersionVulnerability.php @@ -17,8 +17,9 @@ public function __construct(private readonly CompareVersions $compareVersions) { public function execute(string $currentVersion, string $affectedIn, VersionStrategy $strategy = VersionStrategy::Standard): bool { $affectedIn = trim($affectedIn); + $currentVersion = trim($currentVersion); - if (empty($affectedIn) || empty($currentVersion)) { + if ($affectedIn === '' || $currentVersion === '') { return false; } @@ -26,20 +27,17 @@ public function execute(string $currentVersion, string $affectedIn, VersionStrat return true; } - $currentVersion = str_replace('v', '', strtolower($currentVersion)); + $currentVersion = $this->normalizeVersion($currentVersion); - if (strpos($affectedIn, '<') !== false) { - $affectedIn = preg_replace('/(<=?) *([0-9a-zA-Z\.]*)/m', '$1 $2', $affectedIn); - $parts = explode(' ', $affectedIn); - - return $this->compareVersions->execute($currentVersion, $parts[1], $parts[0], $strategy); + if (preg_match('/^(<=?)\s*(\S+)/', $affectedIn, $matches)) { + return $this->compareVersions->execute($currentVersion, $matches[2], $matches[1], $strategy); } - if (strpos($affectedIn, ',') !== false) { + if (str_contains($affectedIn, ',')) { $versions = explode(',', $affectedIn); foreach ($versions as $version) { - if (trim($version) == $currentVersion) { + if ($this->normalizeVersion($version) === $currentVersion) { return true; } } @@ -47,13 +45,24 @@ public function execute(string $currentVersion, string $affectedIn, VersionStrat return false; } - if (strpos($affectedIn, '-') !== false) { - $parts = explode('-', $affectedIn); + if (preg_match('/^\d+(?:\.\d+)*\s*-\s*\d+(?:\.\d+)*$/', $affectedIn)) { + $parts = explode('-', $affectedIn, 2); return $this->compareVersions->execute($currentVersion, trim($parts[0]), '>=', $strategy) && $this->compareVersions->execute($currentVersion, trim($parts[1]), '<=', $strategy); } - return $currentVersion == $affectedIn; + return $currentVersion === $this->normalizeVersion($affectedIn); + } + + private function normalizeVersion(string $version): string + { + $version = strtolower(trim($version)); + + if (str_starts_with($version, 'v')) { + $version = substr($version, 1); + } + + return $version; } } diff --git a/src/Drupal/CheckDrupalVersionVulnerability.php b/src/Drupal/CheckDrupalVersionVulnerability.php index 014fa31..7bb09b0 100644 --- a/src/Drupal/CheckDrupalVersionVulnerability.php +++ b/src/Drupal/CheckDrupalVersionVulnerability.php @@ -23,8 +23,9 @@ public function __construct( public function execute(string $currentVersion, string $affectedIn, VersionStrategy $strategy = VersionStrategy::Standard): bool { $affectedIn = trim($affectedIn); + $currentVersion = trim($currentVersion); - if (empty($affectedIn) || empty($currentVersion)) { + if ($affectedIn === '' || $currentVersion === '') { return false; } @@ -32,7 +33,7 @@ public function execute(string $currentVersion, string $affectedIn, VersionStrat return true; } - if (stripos($affectedIn, ',') !== false) { + if (str_contains($affectedIn, ',')) { $versions = explode(',', $affectedIn); foreach ($versions as $version) { @@ -44,7 +45,7 @@ public function execute(string $currentVersion, string $affectedIn, VersionStrat return false; } - $currentVersion = str_replace('v', '', strtolower($currentVersion)); + $currentVersion = $this->normalizeVersion($currentVersion); if (! $this->normalizer->majorVersionsMatch($affectedIn, $currentVersion)) { return false; @@ -53,20 +54,28 @@ public function execute(string $currentVersion, string $affectedIn, VersionStrat $affectedIn = $this->normalizer->normalize($affectedIn); $currentVersion = $this->normalizer->normalize($currentVersion); - if (strpos($affectedIn, '<') !== false) { - $affectedIn = preg_replace('/(<=?) *([0-9a-zA-Z\.]*)/m', '$1 $2', $affectedIn); - $parts = explode(' ', $affectedIn); - - return $this->compareVersions->execute($currentVersion, $parts[1], $parts[0], $strategy); + if (preg_match('/^(<=?)\s*(\S+)/', $affectedIn, $matches)) { + return $this->compareVersions->execute($currentVersion, $matches[2], $matches[1], $strategy); } - if (strpos($affectedIn, '-') !== false) { - $parts = explode('-', $affectedIn); + if (preg_match('/^\d+(?:\.\d+)*\s*-\s*\d+(?:\.\d+)*$/', $affectedIn)) { + $parts = explode('-', $affectedIn, 2); return $this->compareVersions->execute($currentVersion, trim($parts[0]), '>=', $strategy) && $this->compareVersions->execute($currentVersion, trim($parts[1]), '<=', $strategy); } - return $currentVersion == $affectedIn; + return $currentVersion === $this->normalizeVersion($affectedIn); + } + + private function normalizeVersion(string $version): string + { + $version = strtolower(trim($version)); + + if (str_starts_with($version, 'v')) { + $version = substr($version, 1); + } + + return $version; } } diff --git a/tests/CheckVersionVulnerabilityTest.php b/tests/CheckVersionVulnerabilityTest.php index 1ee3ec8..86392fa 100644 --- a/tests/CheckVersionVulnerabilityTest.php +++ b/tests/CheckVersionVulnerabilityTest.php @@ -64,6 +64,51 @@ expect($this->action->execute('v1.5', '1.5'))->toBeTrue(); }); +it('does not corrupt versions containing v in body', function (string $currentVersion, string $affectedIn, bool $expected) { + expect($this->action->execute($currentVersion, $affectedIn))->toBe($expected); +})->with([ + 'revision exact match' => ['6.3-revision-0', '6.3-revision-0', true], + 'dev exact match' => ['1.0-dev', '1.0-dev', true], + 'preview exact match' => ['5.0.37.preview', '5.0.37.preview', true], +]); + +it('handles exact match for versions with dashes', function (string $currentVersion, string $affectedIn, bool $expected) { + expect($this->action->execute($currentVersion, $affectedIn))->toBe($expected); +})->with([ + 'beta suffix: match' => ['4.5.5-beta', '4.5.5-beta', true], + 'beta suffix: no match' => ['4.5.6', '4.5.5-beta', false], + 'revision suffix: match' => ['6.3-revision-0', '6.3-revision-0', true], + 'revision suffix: no match' => ['6.3-revision-1', '6.3-revision-0', false], + 'alpha segment: match' => ['12.8-a.1', '12.8-a.1', true], + 'alpha segment: no match' => ['12.8-a.3', '12.8-a.1', false], +]); + +it('does not match versions via scientific notation coercion', function () { + expect($this->action->execute('10', '1e1'))->toBeFalse() + ->and($this->action->execute('10', '1e1, 2.0'))->toBeFalse() + ->and($this->action->execute('100', '1e2'))->toBeFalse(); +}); + +it('handles version zero as valid input', function () { + expect($this->action->execute('0', '<= 1.0'))->toBeTrue() + ->and($this->action->execute('0', '*'))->toBeTrue(); +}); + +it('normalizes v prefix in affectedIn for comma and exact paths', function () { + expect($this->action->execute('v1.5', 'v1.0, v1.5, v2.0'))->toBeTrue() + ->and($this->action->execute('V1.5', 'V1.5'))->toBeTrue(); +}); + +it('rejects whitespace-only current version', function () { + expect($this->action->execute(' ', '<= 3.0'))->toBeFalse() + ->and($this->action->execute(' ', '*'))->toBeFalse(); +}); + +it('returns false for malformed operator expressions', function () { + expect($this->action->execute('3.5', '3.41<'))->toBeFalse() + ->and($this->action->execute('3.5', 'version < 3.41'))->toBeFalse(); +}); + it('handles real-world version formats', function (string $currentVersion, string $affectedIn, bool $expected) { expect($this->action->execute($currentVersion, $affectedIn))->toBe($expected); })->with([ @@ -72,5 +117,21 @@ 'suffixed decaf: vulnerable' => ['5.0.37.decaf', '<= 5.0.37.decaf', true], 'suffixed decaf: fixed' => ['5.0.53.decaf', '<= 5.0.37.decaf', false], 'pre-release beta: vulnerable' => ['4.5.5-beta', '<= 4.5.5-beta', true], + 'pre-release beta: fixed' => ['4.5.6', '<= 4.5.5-beta', false], + 'alpha segment: vulnerable' => ['12.8-a.1', '<= 12.8-a.1', true], + 'alpha segment: fixed' => ['12.8-a.3', '<= 12.8-a.1', false], + 'compound dash: vulnerable' => ['25.07010000-WP6.8.1-JB5.11.5', '<= 25.07010000-WP6.8.1-JB5.11.5', true], + 'compound dash: fixed' => ['25.09000000-WP6.8.2-JB5.12.0', '<= 25.07010000-WP6.8.1-JB5.11.5', false], 'revision-based: vulnerable' => ['6.3-revision-0', '<= 6.3-revision-0', true], + 'revision-based: fixed' => ['6.3-revision-1', '<= 6.3-revision-0', false], ]); + +it('applies decimal strategy to strict less-than operator', function () { + expect($this->action->execute('3.5', '< 3.41', VersionStrategy::DecimalNormalized))->toBeFalse() + ->and($this->action->execute('3.40', '< 3.41', VersionStrategy::DecimalNormalized))->toBeTrue(); +}); + +it('applies decimal strategy to range format', function () { + expect($this->action->execute('3.5', '3.41-3.6', VersionStrategy::DecimalNormalized))->toBeTrue() + ->and($this->action->execute('3.39', '3.41-3.6', VersionStrategy::DecimalNormalized))->toBeFalse(); +}); diff --git a/tests/Drupal/CheckDrupalVersionVulnerabilityTest.php b/tests/Drupal/CheckDrupalVersionVulnerabilityTest.php index 5800eff..f64fc4f 100644 --- a/tests/Drupal/CheckDrupalVersionVulnerabilityTest.php +++ b/tests/Drupal/CheckDrupalVersionVulnerabilityTest.php @@ -52,3 +52,36 @@ expect($this->action->execute('8.x-3.5', '8.x-3.5'))->toBeTrue() ->and($this->action->execute('8.x-3.6', '8.x-3.5'))->toBeFalse(); }); + +it('does not corrupt Drupal versions containing v in body', function () { + expect($this->action->execute('8.x-3.5-dev', '<= 8.x-3.5-dev'))->toBeTrue(); +}); + +it('handles exact match for Drupal versions with dashes after prefix stripping', function () { + expect($this->action->execute('8.x-3.5-beta', '8.x-3.5-beta'))->toBeTrue() + ->and($this->action->execute('8.x-3.6', '8.x-3.5-beta'))->toBeFalse(); +}); + +it('does not match Drupal versions via scientific notation coercion', function () { + expect($this->action->execute('10', '1e1'))->toBeFalse(); +}); + +it('handles version zero as valid Drupal input', function () { + expect($this->action->execute('0', '<= 1.0'))->toBeTrue() + ->and($this->action->execute('0', '*'))->toBeTrue(); +}); + +it('rejects whitespace-only Drupal current version', function () { + expect($this->action->execute(' ', '<= 3.0'))->toBeFalse() + ->and($this->action->execute(' ', '*'))->toBeFalse(); +}); + +it('returns false for malformed Drupal operator expressions', function () { + expect($this->action->execute('3.5', '3.41<'))->toBeFalse() + ->and($this->action->execute('3.5', 'version < 3.41'))->toBeFalse(); +}); + +it('handles Drupal with decimal normalized strategy', function () { + expect($this->action->execute('8.x-3.5', '<= 8.x-3.41', \Patchstack\VersionCompare\VersionStrategy::DecimalNormalized))->toBeFalse() + ->and($this->action->execute('8.x-3.40', '<= 8.x-3.41', \Patchstack\VersionCompare\VersionStrategy::DecimalNormalized))->toBeTrue(); +}); diff --git a/tests/VersionStrategyTest.php b/tests/VersionStrategyTest.php new file mode 100644 index 0000000..df2f39a --- /dev/null +++ b/tests/VersionStrategyTest.php @@ -0,0 +1,10 @@ +toBe(VersionStrategy::DecimalNormalized) + ->and(VersionStrategy::fromAbnormalFlag(false))->toBe(VersionStrategy::Standard); +}); From 1b123cbc9397ff0692f26a8d58edda544825d07a Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:40:03 +0200 Subject: [PATCH 2/8] Fix CI: update PHP matrix to 8.2+ for Pest v3 compatibility Pest v3 requires PHP 8.2+. The default template had PHP 8.1 in the matrix which caused composer install to fail. Also adds Composer dependency caching and PHP 8.4 to the matrix. --- .github/workflows/tests.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5ace847..bceb117 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php: ['8.1', '8.2', '8.3'] + php: ['8.2', '8.3', '8.4'] name: PHP ${{ matrix.php }} @@ -24,8 +24,16 @@ jobs: php-version: ${{ matrix.php }} coverage: none + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: vendor + key: ${{ runner.os }}-composer-${{ hashFiles('composer.lock') }} + restore-keys: | + ${{ runner.os }}-composer- + - name: Install dependencies - run: composer install --no-interaction --prefer-dist + run: composer install --no-progress --no-interaction --prefer-dist - name: Run tests run: vendor/bin/pest From e5fba21b01b98322e69797dc2d82d62a68807318 Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:46:44 +0200 Subject: [PATCH 3/8] Add phpunit.xml for Pest v3 CI compatibility Pest v3 requires a phpunit.xml config in CI environments. Without it, the --cache-directory flag is misinterpreted as an XML config path. --- .gitignore | 1 + phpunit.xml | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 phpunit.xml diff --git a/.gitignore b/.gitignore index 0794651..fa88824 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /vendor/ composer.lock .phpunit.result.cache +.phpunit.cache diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..5df23e0 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,16 @@ + + + + + tests + + + + + src + + + From 08ab1e41d3b0ac1affc1d6f5901a93a2ecb92b47 Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:51:48 +0200 Subject: [PATCH 4/8] Add PHP 8.1 back to CI matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit composer.lock is gitignored so CI resolves fresh — PHP 8.1 gets Pest v2, PHP 8.2+ gets Pest v3. Tests the full declared range. --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bceb117..31f670f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php: ['8.2', '8.3', '8.4'] + php: ['8.1', '8.2', '8.3', '8.4'] name: PHP ${{ matrix.php }} From 00f68c8836d0abea9246aa5409910666aab62054 Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:52:44 +0200 Subject: [PATCH 5/8] Bump minimum PHP to 8.2 Pest v2.36.1+ and v3 both require PHP 8.2. PHP 8.1 reached EOL Nov 2024. Update composer.json and CI matrix accordingly. --- .github/workflows/tests.yml | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 31f670f..bceb117 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php: ['8.1', '8.2', '8.3', '8.4'] + php: ['8.2', '8.3', '8.4'] name: PHP ${{ matrix.php }} diff --git a/composer.json b/composer.json index 09dd9f8..d8f6cd6 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "library", "license": "MIT", "require": { - "php": "^8.1" + "php": "^8.2" }, "require-dev": { "pestphp/pest": "^2.0|^3.0" From f1e9d405ef5f81a2a64d63ef529acc38998c319b Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:54:55 +0200 Subject: [PATCH 6/8] Require PHP 8.4+ and simplify CI to single version Package will be used in Hub (Laravel 12, PHP 8.4). No need to support older versions. --- .github/workflows/tests.yml | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bceb117..819c272 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php: ['8.2', '8.3', '8.4'] + php: ['8.4'] name: PHP ${{ matrix.php }} diff --git a/composer.json b/composer.json index d8f6cd6..c564317 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "library", "license": "MIT", "require": { - "php": "^8.2" + "php": "^8.4" }, "require-dev": { "pestphp/pest": "^2.0|^3.0" From 9ccb9979e9bdb71f47099b9e7b02f062447e4f7a Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 15:56:39 +0200 Subject: [PATCH 7/8] Rename CI job to Tests and drop matrix --- .github/workflows/tests.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 819c272..6adfcf8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,11 +9,7 @@ on: jobs: test: runs-on: ubuntu-latest - strategy: - matrix: - php: ['8.4'] - - name: PHP ${{ matrix.php }} + name: Tests steps: - uses: actions/checkout@v4 @@ -21,7 +17,7 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: ${{ matrix.php }} + php-version: '8.4' coverage: none - name: Cache Composer dependencies From 19128761b76def9495728296ed7a4df72e8ad37a Mon Sep 17 00:00:00 2001 From: Renato Hysa Date: Wed, 15 Apr 2026 16:30:00 +0200 Subject: [PATCH 8/8] Lower minimum PHP to 8.2 and restore CI matrix Hub and saas run PHP 8.2, so this package must support it. No 8.4-specific features are used in the source code. --- .github/workflows/tests.yml | 7 +++++-- composer.json | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6adfcf8..9d059f6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,7 +9,10 @@ on: jobs: test: runs-on: ubuntu-latest - name: Tests + strategy: + matrix: + php: ['8.2', '8.3', '8.4'] + name: PHP ${{ matrix.php }} steps: - uses: actions/checkout@v4 @@ -17,7 +20,7 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 with: - php-version: '8.4' + php-version: ${{ matrix.php }} coverage: none - name: Cache Composer dependencies diff --git a/composer.json b/composer.json index c564317..d8f6cd6 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,7 @@ "type": "library", "license": "MIT", "require": { - "php": "^8.4" + "php": "^8.2" }, "require-dev": { "pestphp/pest": "^2.0|^3.0"