From 11b055756e4fe0564be65daf4d870e0313befb8a Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Fri, 22 Jan 2021 16:29:44 +0200 Subject: [PATCH 1/7] TE-8136 / TE-8201 checker --- .github/workflows/ci.yml | 55 ++++++++ README.md | 14 ++ composer.json | 5 +- phpstan-bootstrap.php | 2 +- phpstan.neon | 4 +- .../Command/SecurityCheckerCommand.php | 130 +++++++++++++++++- src/SecurityChecker/Result.php | 58 ++++++++ 7 files changed, 259 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 src/SecurityChecker/Result.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..24f629a --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,55 @@ +name: CI + +on: + pull_request: + push: + branches: + - master + schedule: + - cron: "0 0 * * *" + branches: + - 'master' + +jobs: + checks: + runs-on: ubuntu-latest + strategy: + matrix: + php-version: ['7.3', '7.4'] + + steps: + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} + extensions: mbstring, intl + + - name: Checkout + uses: actions/checkout@v1 + with: + fetch-depth: 1 + + - name: Composer validate + run: composer validate + + - name: Composer get cache directory + id: composer-cache + run: | + echo "::set-output name=dir::$(composer config cache-files-dir)" + + - name: Composer cache + uses: actions/cache@v2 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: | + ${{ runner.os }}-composer- + + - name: Composer install + run: composer install --optimize-autoloader + + - name: CodeStyle checks + run: composer cs-check + + - name: PHPStan checks + run: composer stan diff --git a/README.md b/README.md index d8a8415..1658527 100644 --- a/README.md +++ b/README.md @@ -4,3 +4,17 @@ [![Minimum PHP Version](https://img.shields.io/badge/php-%3E%3D%207.3-8892BF.svg)](https://php.net/) [![PHPStan](https://img.shields.io/badge/PHPStan-level%208-brightgreen.svg?style=flat)](https://phpstan.org/) +Checks security issues in your project dependencies + +## Installation + +`composer require --dev spryker-sdk/security-checker` + +## Configuration + +After the installation you will need to enable it in the `ConsoleDependencyProvider`. + +## Commands + +Security shecker offer the following command: +- `console security:check` - check security issue in composer.lock file. \ No newline at end of file diff --git a/composer.json b/composer.json index 8b082ad..ea01622 100644 --- a/composer.json +++ b/composer.json @@ -4,7 +4,10 @@ "description": "A security checker for your composer.lock", "license": "proprietary", "require": { - "php": ">=7.3" + "php": ">=7.3", + "symfony/console": "^5.0", + "symfony/options-resolver": "^5.0", + "symfony/process": "^5.0" }, "require-dev": { "phpstan/phpstan": "^0.12.18", diff --git a/phpstan-bootstrap.php b/phpstan-bootstrap.php index 03900ea..a83ad84 100644 --- a/phpstan-bootstrap.php +++ b/phpstan-bootstrap.php @@ -1,3 +1,3 @@ setName(static::COMMAND_NAME) + ->setDescription('Checks security issues in your project dependencies'); + } + + /** + * @param \Symfony\Component\Console\Input\InputInterface $input + * @param \Symfony\Component\Console\Output\OutputInterface $output + * + * @return int + */ + public function execute(InputInterface $input, OutputInterface $output): int + { + $this->loadFile(); + $this->changeFileMode(); + $commandOutput = $this->runCommand(); + $commandOutput = $this->markFalsePositiveResults($commandOutput); + + $result = $this->createResultFromCommandOutput($commandOutput); + + $output->writeln($result->getVulnerabilities()); + + if ($result->count() === 0) { + return 0; + } + + if ($result->count() === 1 && strpos($result->getVulnerabilities(), static::FALSE_POSITIVE_ISSUE_NUMBER) !== false) { + $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); + $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); + + return 0; + } + + return 1; + } + + /** + * @return void + */ + protected function loadFile(): void + { + $process = new Process(['wget', static::BINARY_CHECKER, '-O', static::FILE_NAME]); + $process->run(); + } + + /** + * @return void + */ + protected function changeFileMode(): void + { + chmod(static::FILE_NAME, 0777); + } + + /** + * @throws \Symfony\Component\Process\Exception\ProcessFailedException + * + * @return string + */ + protected function runCommand(): string + { + $process = new Process([static::FILE_NAME, 'check:security']); + $process->run(); + + if (!empty($process->getErrorOutput())) { + throw new ProcessFailedException($process); + } + + return $process->getOutput(); + } + + /** + * @param string $commandOutput + * + * @return \SecurityChecker\Result + */ + protected function createResultFromCommandOutput(string $commandOutput): Result + { + $count = 0; + preg_match('/\d\spackage/m', $commandOutput, $matches); + if (!empty($matches)) { + $count = (int)$matches[0]; + } + + return new Result($count, $commandOutput); + } + + /** + * @param string $commandOutput + * + * @return string + */ + protected function markFalsePositiveResults(string $commandOutput): string + { + if (strpos($commandOutput, static::FALSE_POSITIVE_ISSUE_NUMBER) !== false) { + $commandOutput = str_replace( + static::FALSE_POSITIVE_ISSUE_NUMBER, + sprintf('%s - is a false positive', static::FALSE_POSITIVE_ISSUE_NUMBER), + $commandOutput + ); + } -} \ No newline at end of file + return $commandOutput; + } +} diff --git a/src/SecurityChecker/Result.php b/src/SecurityChecker/Result.php new file mode 100644 index 0000000..c6f3a49 --- /dev/null +++ b/src/SecurityChecker/Result.php @@ -0,0 +1,58 @@ +count = $count; + $this->vulnerabilities = $vulnerabilities; + } + + /** + * @return string + */ + public function getVulnerabilities(): string + { + return $this->vulnerabilities; + } + + /** + * @return string + */ + public function __toString(): string + { + return $this->vulnerabilities; + } + + /** + * @return int + */ + public function count(): int + { + return $this->count; + } +} From 993bc066ce57cbc7a3d0012fd0ac72be11e78996 Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Fri, 22 Jan 2021 16:39:15 +0200 Subject: [PATCH 2/7] TE-8136 / TE-8201 fix actions --- composer.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 8b082ad..437964a 100644 --- a/composer.json +++ b/composer.json @@ -27,9 +27,9 @@ "config": { "sort-packages": true }, - "scripts": { - "cs-check": "phpcs --colors -p -s --extensions=php --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml --ignore=/tests/_data/,/_support/ src/ tests/", - "cs-fix": "phpcbf --colors -p --extensions=php --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml --ignore=/tests/_data/,/_support/ src/ tests/", - "stan": "phpstan analyze -l 8 src/" - } + "scripts": { + "cs-check": "phpcs --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml -v src/", + "cs-fix": "phpcbf --standard=vendor/spryker/code-sniffer/Spryker/ruleset.xml -v src/", + "stan": "phpstan analyze -l 8 src/" + } } From 280b5ca94890bb43d5c693af80b2f5dbb38b817f Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Fri, 22 Jan 2021 17:07:02 +0200 Subject: [PATCH 3/7] TE-8136 / TE-8201 symfony 4 support --- composer.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 5daaba6..b8f2841 100644 --- a/composer.json +++ b/composer.json @@ -5,9 +5,9 @@ "license": "proprietary", "require": { "php": ">=7.3", - "symfony/console": "^5.0", - "symfony/options-resolver": "^5.0", - "symfony/process": "^5.0" + "symfony/console": "^4.0.0 || ^5.0.0", + "symfony/options-resolver": "^4.0.0 || ^5.0.0", + "symfony/process": "^4.0.0 || ^5.0.0" }, "require-dev": { "phpstan/phpstan": "^0.12.18", From d30621663a8caeb2dd971e05c7325e46d8792d36 Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Mon, 25 Jan 2021 14:07:55 +0200 Subject: [PATCH 4/7] TE-8136 review fix --- .../Command/SecurityCheckerCommand.php | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/SecurityChecker/Command/SecurityCheckerCommand.php b/src/SecurityChecker/Command/SecurityCheckerCommand.php index 89416de..097ee22 100644 --- a/src/SecurityChecker/Command/SecurityCheckerCommand.php +++ b/src/SecurityChecker/Command/SecurityCheckerCommand.php @@ -50,18 +50,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $output->writeln($result->getVulnerabilities()); - if ($result->count() === 0) { - return 0; - } - - if ($result->count() === 1 && strpos($result->getVulnerabilities(), static::FALSE_POSITIVE_ISSUE_NUMBER) !== false) { - $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); - $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); - - return 0; - } - - return 1; + return $this->convertResultToExitCode($result, $output); } /** @@ -131,4 +120,36 @@ protected function markFalsePositiveResults(string $commandOutput): string return $commandOutput; } + + /** + * @param \SecurityChecker\Result $result + * @param \Symfony\Component\Console\Output\OutputInterface $output + * + * @return int + */ + protected function convertResultToExitCode(Result $result, OutputInterface $output): int + { + if ($result->count() === 0) { + return 0; + } + + if ($this->checkResultForFalsePositiveCase($result)) { + $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); + $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); + + return 0; + } + + return 1; + } + + /** + * @param \SecurityChecker\Result $result + * + * @return bool + */ + protected function checkResultForFalsePositiveCase(Result $result): bool + { + return $result->count() === 1 && strpos($result->getVulnerabilities(), static::FALSE_POSITIVE_ISSUE_NUMBER) !== false; + } } From 08a969b5aace72f094d131b8650b6e893389c871 Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Mon, 25 Jan 2021 14:18:08 +0200 Subject: [PATCH 5/7] TE-8136 review fix --- src/SecurityChecker/Command/SecurityCheckerCommand.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/SecurityChecker/Command/SecurityCheckerCommand.php b/src/SecurityChecker/Command/SecurityCheckerCommand.php index 097ee22..399f369 100644 --- a/src/SecurityChecker/Command/SecurityCheckerCommand.php +++ b/src/SecurityChecker/Command/SecurityCheckerCommand.php @@ -134,8 +134,10 @@ protected function convertResultToExitCode(Result $result, OutputInterface $outp } if ($this->checkResultForFalsePositiveCase($result)) { - $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); - $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); + if ($output->isVerbose()) { + $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); + $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); + } return 0; } From b96c46f832cd26ca9075cf13b576f2c467ec4e40 Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Mon, 25 Jan 2021 16:42:07 +0200 Subject: [PATCH 6/7] TE-8136 review fix --- .../Command/SecurityCheckerCommand.php | 28 +++++++++++-------- src/SecurityChecker/Result.php | 4 +-- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/SecurityChecker/Command/SecurityCheckerCommand.php b/src/SecurityChecker/Command/SecurityCheckerCommand.php index 399f369..194aa3c 100644 --- a/src/SecurityChecker/Command/SecurityCheckerCommand.php +++ b/src/SecurityChecker/Command/SecurityCheckerCommand.php @@ -17,6 +17,9 @@ class SecurityCheckerCommand extends Command { + protected const CODE_SUCCESS = 0; + protected const CODE_ERROR = 1; + protected const COMMAND_NAME = 'security:check'; protected const BINARY_CHECKER = 'https://github.com/fabpot/local-php-security-checker/releases/download/v1.0.0/local-php-security-checker_1.0.0_linux_amd64'; @@ -42,7 +45,6 @@ protected function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { $this->loadFile(); - $this->changeFileMode(); $commandOutput = $this->runCommand(); $commandOutput = $this->markFalsePositiveResults($commandOutput); @@ -58,8 +60,12 @@ public function execute(InputInterface $input, OutputInterface $output): int */ protected function loadFile(): void { - $process = new Process(['wget', static::BINARY_CHECKER, '-O', static::FILE_NAME]); - $process->run(); + if (!file_exists(static::FILE_NAME)) { + $process = new Process(['wget', static::BINARY_CHECKER, '-O', static::FILE_NAME]); + $process->run(); + + $this->changeFileMode(); + } } /** @@ -130,19 +136,19 @@ protected function markFalsePositiveResults(string $commandOutput): string protected function convertResultToExitCode(Result $result, OutputInterface $output): int { if ($result->count() === 0) { - return 0; + return static::CODE_SUCCESS; } - if ($this->checkResultForFalsePositiveCase($result)) { - if ($output->isVerbose()) { - $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); - $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); - } + if (!$this->checkResultForFalsePositiveCase($result)) { + return static::CODE_ERROR; + } - return 0; + if ($output->isVerbose()) { + $output->writeln(sprintf('The issue about %s is a false positive result', static::FALSE_POSITIVE_ISSUE_NUMBER)); + $output->writeln('Check https://github.com/FriendsOfPHP/security-advisories/issues/511 for details'); } - return 1; + return static::CODE_SUCCESS; } /** diff --git a/src/SecurityChecker/Result.php b/src/SecurityChecker/Result.php index c6f3a49..d95a3bf 100644 --- a/src/SecurityChecker/Result.php +++ b/src/SecurityChecker/Result.php @@ -15,12 +15,12 @@ class Result implements Countable /** * @var int */ - private $count; + protected $count; /** * @var string */ - private $vulnerabilities; + protected $vulnerabilities; /** * @param int $count From 8c3f1bdfeeb0a7f15b919344d3e976fb3b4a428f Mon Sep 17 00:00:00 2001 From: Dimitriy Kravchenko Date: Mon, 25 Jan 2021 17:08:07 +0200 Subject: [PATCH 7/7] TE-8136 review fix --- .../Command/SecurityCheckerCommand.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/SecurityChecker/Command/SecurityCheckerCommand.php b/src/SecurityChecker/Command/SecurityCheckerCommand.php index 194aa3c..8dd6b59 100644 --- a/src/SecurityChecker/Command/SecurityCheckerCommand.php +++ b/src/SecurityChecker/Command/SecurityCheckerCommand.php @@ -56,16 +56,24 @@ public function execute(InputInterface $input, OutputInterface $output): int } /** + * @throws \Symfony\Component\Process\Exception\ProcessFailedException + * * @return void */ protected function loadFile(): void { - if (!file_exists(static::FILE_NAME)) { - $process = new Process(['wget', static::BINARY_CHECKER, '-O', static::FILE_NAME]); - $process->run(); + if (file_exists(static::FILE_NAME)) { + return; + } - $this->changeFileMode(); + $process = new Process(['wget', static::BINARY_CHECKER, '-O', static::FILE_NAME]); + $process->run(); + + if ($process->getExitCode() === static::CODE_ERROR) { + throw new ProcessFailedException($process); } + + $this->changeFileMode(); } /**