From 8d2049d2a3aa67bcba2354070f00d662044f6958 Mon Sep 17 00:00:00 2001 From: Phil Young Date: Fri, 19 Jul 2024 16:36:14 +0100 Subject: [PATCH 1/4] Fix the PackageSecurityHealthCheck Swap sensiolabs/security-checker for enlightn/security-checker as the sensiolabs one is now abandoned. Co-authored-by: Steve Richter --- CHANGELOG.md | 4 ++ composer.json | 3 +- config/healthcheck.php | 9 +-- src/Checks/PackageSecurityHealthCheck.php | 26 +++++--- .../Checks/PackageSecurityHealthCheckTest.php | 61 +++++++++++-------- ...curityCheckerPackageHasVulnerability.json} | 5 +- 6 files changed, 66 insertions(+), 42 deletions(-) rename tests/json/{sensiolabsPackageHasVulnerability.json => securityCheckerPackageHasVulnerability.json} (74%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f2e7ec..612a1fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Dropped support for < Laravel 10 **(breaking change)** - The `context` property for `UKFast\HealthCheck\Checks\CrossServiceHealthCheck` is now an array, not a string, matching other checks **(breaking change)** +### Fixed + +- Fix PackageSecurityHealthCheck by using the `enlightn/security-checker` package instead of the archived `sensiolabs/security-checker` package **(breaking change)** + ## [1.15.0] - 2024-07-17 ### Added diff --git a/composer.json b/composer.json index 5e7e731..779cc86 100644 --- a/composer.json +++ b/composer.json @@ -57,6 +57,7 @@ "require": { "php": "^8.1", "ext-json": "*", + "enlightn/security-checker": "^2.0", "illuminate/console": "^10.0|^11.0", "illuminate/http": "^10.0|^11.0", "illuminate/support": "^10.0|^11.0" @@ -76,6 +77,6 @@ "illuminate/redis": "Allows the redis health check to function.", "league/flysystem": "Allows the ftp health check to function.", "guzzlehttp/guzzle": "Allows the http health check to function.", - "sensiolabs/security-checker": "Allows the package security health check to function." + "enlightn/security-checker": "Allows the package security health check to function." } } diff --git a/config/healthcheck.php b/config/healthcheck.php index 5242f15..7b9bcc2 100644 --- a/config/healthcheck.php +++ b/config/healthcheck.php @@ -82,7 +82,7 @@ * Will be used in the HealthCheckController's response. */ 'default-problem-http-code' => 500, - + /* * Default timeout for cURL requests for HTTP health check. */ @@ -114,10 +114,11 @@ ], /* - * A list of packages to be ignored by the Package Security health check + * Settings for the Package Security health check */ 'package-security' => [ - 'ignore' => [], + 'exclude-dev' => false, // Exclude dev dependencies from the security check + 'ignore' => [], // Packages to ignore ], 'scheduler' => [ @@ -127,7 +128,7 @@ /* * Default value for env checks. - * For each key, the check will call `env(KEY, config('healthcheck.env-default-key'))` + * For each key, the check will call `env(KEY, config('healthcheck.env-default-key'))` * to avoid false positives when `env(KEY)` is defined but is null. */ 'env-check-key' => 'HEALTH_CHECK_ENV_DEFAULT_VALUE', diff --git a/src/Checks/PackageSecurityHealthCheck.php b/src/Checks/PackageSecurityHealthCheck.php index 9d89494..5888a7f 100644 --- a/src/Checks/PackageSecurityHealthCheck.php +++ b/src/Checks/PackageSecurityHealthCheck.php @@ -4,7 +4,7 @@ use Exception; use Illuminate\Support\Collection; -use SensioLabs\Security\SecurityChecker; +use Enlightn\SecurityChecker\SecurityChecker; use UKFast\HealthCheck\HealthCheck; use UKFast\HealthCheck\Status; @@ -22,23 +22,33 @@ public function __construct() $this->vulnerablePackages = collect(); } - public static function checkDependency(): bool + /** + * @param class-string $class + */ + public static function checkDependency(string $class): bool { - return class_exists(SecurityChecker::class); + return class_exists($class); } public function status(): Status { try { - if (! static::checkDependency()) { - throw new Exception('You need to install the sensiolabs/security-checker package to use this check.'); + if (! static::checkDependency(SecurityChecker::class)) { + if (static::checkDependency('SensioLabs\Security\SecurityChecker')) { + throw new Exception('The sensiolabs/security-checker package has been archived. Install enlightn/security-checker instead.'); + } + throw new Exception('You need to install the enlightn/security-checker package to use this check.'); } $checker = new SecurityChecker(); - $result = $checker->check(base_path('composer.lock'), 'json'); + $result = $checker->check( + base_path('composer.lock'), + config('healthcheck.package-security.exclude-dev', false), + ); + + $vulnerabilities = collect($result); - if ($result->count() > 0) { - $vulnerabilities = collect(json_decode((string) $result, true)); + if ($vulnerabilities->count() > 0) { $this->vulnerablePackages = $vulnerabilities->reject(function ($vulnerability, $package) { return in_array($package, config('healthcheck.package-security.ignore')); }); diff --git a/tests/Checks/PackageSecurityHealthCheckTest.php b/tests/Checks/PackageSecurityHealthCheckTest.php index 35b601a..be9aa95 100644 --- a/tests/Checks/PackageSecurityHealthCheckTest.php +++ b/tests/Checks/PackageSecurityHealthCheckTest.php @@ -36,30 +36,43 @@ protected function partialMock($abstract, Closure $mock = null): MockInterface public function testShowsProblemIfRequiredPackageNotLoaded() { - $status = (new PackageSecurityHealthCheck($this->app))->status(); + $status = (new MockedPackageSecurityHealthCheck)->status(); $this->assertTrue($status->isProblem()); + $this->assertSame('You need to install the enlightn/security-checker package to use this check.', $status->context()['exception']['error']); } - public function shows_problem_if_cannot_check_packages(): void + public function testShowsProblemIfIncorrectPackageLoaded(): void { - $this->partialMock('overload:SensioLabs\Security\SecurityChecker', function ($mock) { - $mock->shouldReceive('check')->andThrow(new Exception('Lock file does not exist.')); + MockedPackageSecurityHealthCheck::$classResults = [ + 'Enlightn\SecurityChecker\SecurityChecker' => false, + 'SensioLabs\Security\SecurityChecker' => true, + ]; + $status = (new MockedPackageSecurityHealthCheck)->status(); + + $this->assertTrue($status->isProblem()); + $this->assertSame('The sensiolabs/security-checker package has been archived. Install enlightn/security-checker instead.', $status->context()['exception']['error']); + } + + public function testShowsProblemIfCannotCheckPackages(): void + { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { + $mock->shouldReceive('check')->andThrow(new Exception('File not found at [/tmp/composer.lock]')); }); - $status = (new PackageSecurityHealthCheck($this->app))->status(); + $status = (new PackageSecurityHealthCheck)->status(); $this->assertTrue($status->isProblem()); } public function testShowsProblemIfPackageHasVulnerability(): void { - $this->partialMock('overload:SensioLabs\Security\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { $mock->shouldReceive('check') - ->andReturn(new MockResult(1, file_get_contents('tests/json/sensiolabsPackageHasVulnerability.json'))); + ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)); }); - $status = (new PackageSecurityHealthCheck($this->app))->status(); + $status = (new PackageSecurityHealthCheck)->status(); $this->assertTrue($status->isProblem()); } @@ -72,44 +85,38 @@ public function testIgnoresPackageIfInConfig(): void ], ]); - $this->partialMock('overload:SensioLabs\Security\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { $mock->shouldReceive('check') - ->andReturn(new MockResult(1, file_get_contents('tests/json/sensiolabsPackageHasVulnerability.json'))); + ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)); }); - $status = (new PackageSecurityHealthCheck($this->app))->status(); + $status = (new PackageSecurityHealthCheck)->status(); $this->assertTrue($status->isOkay()); } public function testShowsOkayIfNoPackagesHaveVulnerabilities(): void { - $this->partialMock('overload:SensioLabs\Security\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { $mock->shouldReceive('check') - ->andReturn(new MockResult(0, '{}')); + ->andReturn([]); }); - $status = (new PackageSecurityHealthCheck($this->app))->status(); + $status = (new PackageSecurityHealthCheck)->status(); $this->assertTrue($status->isOkay()); } } -class MockResult +class MockedPackageSecurityHealthCheck extends \UKFast\HealthCheck\Checks\PackageSecurityHealthCheck { - public function __construct( - public readonly int $count, - private readonly string $vulnerabilities, - ) { - } - - public function __toString(): string - { - return $this->vulnerabilities; - } + public static $classResults = [ + 'Enlightn\SecurityChecker\SecurityChecker' => false, + 'SensioLabs\Security\SecurityChecker' => false, + ]; - public function count(): int + public static function checkDependency(string $class): bool { - return $this->count; + return static::$classResults[$class]; } } diff --git a/tests/json/sensiolabsPackageHasVulnerability.json b/tests/json/securityCheckerPackageHasVulnerability.json similarity index 74% rename from tests/json/sensiolabsPackageHasVulnerability.json rename to tests/json/securityCheckerPackageHasVulnerability.json index ffff729..08020fc 100644 --- a/tests/json/sensiolabsPackageHasVulnerability.json +++ b/tests/json/securityCheckerPackageHasVulnerability.json @@ -1,6 +1,7 @@ { "example/package": { - "version": "v1.2.3", + "version": "1.2.3", + "time": "2021-10-01T00:00:00+00:00", "advisories": [ { "title": "Example Vulnerability", @@ -9,4 +10,4 @@ } ] } -} \ No newline at end of file +} From 4bfaae303341c1299733b23ab0470ca2e4d549b3 Mon Sep 17 00:00:00 2001 From: Phil Young Date: Fri, 19 Jul 2024 16:40:38 +0100 Subject: [PATCH 2/4] Tidy up code Very minor updates to make the code a bit cleaner --- src/Checks/PackageSecurityHealthCheck.php | 2 +- .../Checks/PackageSecurityHealthCheckTest.php | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/Checks/PackageSecurityHealthCheck.php b/src/Checks/PackageSecurityHealthCheck.php index 5888a7f..a695e8c 100644 --- a/src/Checks/PackageSecurityHealthCheck.php +++ b/src/Checks/PackageSecurityHealthCheck.php @@ -48,7 +48,7 @@ public function status(): Status $vulnerabilities = collect($result); - if ($vulnerabilities->count() > 0) { + if ($vulnerabilities->isNotEmpty()) { $this->vulnerablePackages = $vulnerabilities->reject(function ($vulnerability, $package) { return in_array($package, config('healthcheck.package-security.ignore')); }); diff --git a/tests/Checks/PackageSecurityHealthCheckTest.php b/tests/Checks/PackageSecurityHealthCheckTest.php index be9aa95..dc2cebc 100644 --- a/tests/Checks/PackageSecurityHealthCheckTest.php +++ b/tests/Checks/PackageSecurityHealthCheckTest.php @@ -56,9 +56,9 @@ public function testShowsProblemIfIncorrectPackageLoaded(): void public function testShowsProblemIfCannotCheckPackages(): void { - $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { - $mock->shouldReceive('check')->andThrow(new Exception('File not found at [/tmp/composer.lock]')); - }); + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', fn (MockInterface $mock) => + $mock->shouldReceive('check')->andThrow(new Exception('File not found at [/tmp/composer.lock]')) + ); $status = (new PackageSecurityHealthCheck)->status(); @@ -67,10 +67,10 @@ public function testShowsProblemIfCannotCheckPackages(): void public function testShowsProblemIfPackageHasVulnerability(): void { - $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', fn (MockInterface $mock) => $mock->shouldReceive('check') - ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)); - }); + ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)) + ); $status = (new PackageSecurityHealthCheck)->status(); @@ -85,10 +85,10 @@ public function testIgnoresPackageIfInConfig(): void ], ]); - $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', fn (MockInterface $mock) => $mock->shouldReceive('check') - ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)); - }); + ->andReturn(json_decode(file_get_contents('tests/json/securityCheckerPackageHasVulnerability.json'), true)) + ); $status = (new PackageSecurityHealthCheck)->status(); @@ -97,10 +97,10 @@ public function testIgnoresPackageIfInConfig(): void public function testShowsOkayIfNoPackagesHaveVulnerabilities(): void { - $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', function ($mock) { + $this->partialMock('overload:Enlightn\SecurityChecker\SecurityChecker', fn(MockInterface $mock) => $mock->shouldReceive('check') - ->andReturn([]); - }); + ->andReturn([]) + ); $status = (new PackageSecurityHealthCheck)->status(); @@ -108,9 +108,12 @@ public function testShowsOkayIfNoPackagesHaveVulnerabilities(): void } } -class MockedPackageSecurityHealthCheck extends \UKFast\HealthCheck\Checks\PackageSecurityHealthCheck +class MockedPackageSecurityHealthCheck extends PackageSecurityHealthCheck { - public static $classResults = [ + /** + * @var array + */ + public static array $classResults = [ 'Enlightn\SecurityChecker\SecurityChecker' => false, 'SensioLabs\Security\SecurityChecker' => false, ]; From 4543868d2e45d544843897dde414e92560ef9394 Mon Sep 17 00:00:00 2001 From: Phil Young Date: Fri, 19 Jul 2024 16:45:46 +0100 Subject: [PATCH 3/4] Remove the security checker as a hard dependency This was me commiting test code by accident --- composer.json | 1 - 1 file changed, 1 deletion(-) diff --git a/composer.json b/composer.json index 779cc86..58bd92c 100644 --- a/composer.json +++ b/composer.json @@ -57,7 +57,6 @@ "require": { "php": "^8.1", "ext-json": "*", - "enlightn/security-checker": "^2.0", "illuminate/console": "^10.0|^11.0", "illuminate/http": "^10.0|^11.0", "illuminate/support": "^10.0|^11.0" From 66d9567cf32c9b9b16a965da18996677bd0d7d36 Mon Sep 17 00:00:00 2001 From: Phil Young Date: Fri, 19 Jul 2024 16:46:12 +0100 Subject: [PATCH 4/4] Add the security checker package to CI We now have all the suggested packages --- .github/workflows/run-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index c23663e..a7aa80a 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -43,6 +43,7 @@ jobs: rm -f composer.lock sudo composer self-update composer require laravel/framework:^${{ matrix.environment.laravel_version }}.0 -W + composer require enlightn/security-checker - name: Execute tests run: vendor/bin/phpunit