feature sensiolabs#27 Refactor the code to allow for a better output …

…(fabpot)

This PR was merged into the 1.3-dev branch.

Discussion
----------

Refactor the code to allow for a better output

Commits
-------

1813e4b refactored the code to allow for a better output
839c2c0 fixed typo

README.md

@@ -31,7 +31,7 @@ the checker into your own project:
         use SensioLabs\Security\SecurityChecker;
 
         $checker = new SecurityChecker();
-        $alerts = $checker->check('/path/to/composer.lock', 'json');
+        $alerts = $checker->check('/path/to/composer.lock');
 
 [1]: http://security.sensiolabs.org/
 [2]: https://github.com/sensiolabs/security-advisories

SensioLabs/Security/Command/SecurityCheckerCommand.php

@@ -17,6 +17,11 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputOption;
+use SensioLabs\Security\Exception\ExceptionInterface;
+
+if (!defined('JSON_PRETTY_PRINT')) {
+    define('JSON_PRETTY_PRINT', 0);
+}
 
 class SecurityCheckerCommand extends Command
 {
@@ -44,8 +49,12 @@ protected function configure()
             ))
             ->setDescription('Checks security issues in your project dependencies')
             ->setHelp(<<<EOF
-The <info>%command.name%</info> command checks a <info>composer.lock</info>
-file for security issues in the project dependencies:
+The <info>%command.name%</info> command looks for security issues in the
+project dependencies:
+
+<info>php %command.full_name%</info>
+
+You can also pass the path to a <info>composer.lock</info> file as an argument:
 
 <info>php %command.full_name% /path/to/composer.lock</info>
 
@@ -72,17 +81,71 @@ protected function execute(InputInterface $input, OutputInterface $output)
         }
 
         try {
-            $data = $this->checker->check($input->getArgument('lock'), $input->getOption('format'));
-        } catch (\Exception $e) {
+            $vulnerabilities = $this->checker->check($input->getArgument('lock'));
+        } catch (ExceptionInterface $e) {
             $output->writeln($this->getHelperSet()->get('formatter')->formatBlock($e->getMessage(), 'error', true));
 
             return 1;
         }
 
-        $output->write($data);
+         if ('json' === $input->getOption('format')) {
+             $output->write(json_encode($vulnerabilities, JSON_PRETTY_PRINT));
+         } else {
+             $this->displayResults($output, $input->getArgument('lock'), $vulnerabilities);
+         }
 
         if ($this->checker->getLastVulnerabilityCount() > 0) {
             return 1;
         }
     }
+
+    /**
+     * Displays a security report as plain text.
+     *
+     * @param OutputInterface $output
+     * @param string          $lockFilePath    The file path to the checked lock file
+     * @param array           $vulnerabilities An array of vulnerabilities
+     */
+    private function displayResults(OutputInterface $output, $lockFilePath, array $vulnerabilities)
+    {
+        $output->writeln("\n<fg=blue>Security Check Report\n~~~~~~~~~~~~~~~~~~~~~</>\n");
+        $output->writeln(sprintf('Checked file: <comment>%s</>', realpath($lockFilePath)));
+        $output->write("\n");
+
+        if ($count = count($vulnerabilities)) {
+            $status = 'CRITICAL';
+            $style = 'error';
+        } else {
+            $status = 'OK';
+            $style = 'bg=green;fg=white';
+        }
+
+        $output->writeln($this->getHelper('formatter')->formatBlock(array('['.$status.']', $count.' packages have known vulnerabilities'), $style, true));
+        $output->write("\n");
+
+        if (0 !== $count) {
+            foreach ($vulnerabilities as $dependency => $issues) {
+                $dependencyFullName = $dependency.' ('.$issues['version'].')';
+                $output->writeln('<info>'.$dependencyFullName."\n".str_repeat('-', strlen($dependencyFullName))."</>\n");
+
+                foreach ($issues['advisories'] as $issue => $details) {
+                    $output->write(' * ');
+                    if ($details['cve']) {
+                        $output->write('<comment>'.$details['cve'].': </comment>');
+                    }
+                    $output->writeln($details['title']);
+
+                    if ('' !== $details['link']) {
+                        $output->writeln('   '.$details['link']);
+                    }
+
+                    $output->writeln('');
+                }
+            }
+        }
+
+        $output->writeln("<bg=yellow;fg=white>            </> This checker can only detect vulnerabilities that are referenced");
+        $output->writeln("<bg=yellow;fg=white> Disclaimer </> in the SensioLabs security advisories database. Execute this");
+        $output->writeln("<bg=yellow;fg=white>            </> command regularly to check the newly discovered vulnerabilities.\n");
+    }
 }

SensioLabs/Security/Exception/ExceptionInterface.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the SensioLabs Security Checker.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace SensioLabs\Security\Exception;
+
+interface ExceptionInterface
+{
+}

SensioLabs/Security/Exception/RuntimeException.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the SensioLabs Security Checker.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace SensioLabs\Security\Exception;
+
+class RuntimeException extends \RuntimeException implements ExceptionInterface
+{
+}

SensioLabs/Security/SecurityChecker.php

@@ -11,10 +11,12 @@
 
 namespace SensioLabs\Security;
 
+use SensioLabs\Security\Exception\RuntimeException;
+
 class SecurityChecker
 {
     private $endPoint = 'https://security.sensiolabs.org/check_lock';
-    private $vulnerabilitiesCount;
+    private $vulnerabilityCount;
     private $timeout = 20;
 
     public function setTimeout($timeout)
@@ -30,24 +32,22 @@ public function setEndPoint($endPoint)
     /**
      * Checks a composer.lock file.
      *
-     * @param string $lock   The path to the composer.lock file
-     * @param string $format The return format
+     * @param string $lock The path to the composer.lock file
      *
-     * @return mixed The vulnerabilities
+     * @return array An array of vulnerabilities
      *
-     * @throws \InvalidArgumentException When the output format is unsupported
-     * @throws \RuntimeException         When the lock file does not exist
-     * @throws \RuntimeException         When curl does not work or is unavailable
-     * @throws \RuntimeException         When the certificate can not be copied
+     * @throws RuntimeException When the lock file does not exist
+     * @throws RuntimeException When curl does not work or is unavailable
+     * @throws RuntimeException When the certificate can not be copied
      */
-    public function check($lock, $format = 'text')
+    public function check($lock)
     {
         if (!function_exists('curl_init')) {
-            throw new \RuntimeException('Curl is required to use this command.');
+            throw new RuntimeException('Curl is required to use this command.');
         }
 
         if (false === $curl = curl_init()) {
-            throw new \RuntimeException('Unable to create a new curl handle.');
+            throw new RuntimeException('Unable to create a new curl handle.');
         }
 
         if (is_dir($lock) && file_exists($lock.'/composer.lock')) {
@@ -57,18 +57,7 @@ public function check($lock, $format = 'text')
         }
 
         if (!is_file($lock)) {
-            throw new \RuntimeException('Lock file does not exist.');
-        }
-
-        switch ($format) {
-            case 'text':
-                $accept = 'text/plain';
-                break;
-            case 'json':
-                $accept = 'application/json';
-                break;
-            default:
-                throw new \InvalidArgumentException(sprintf('Unsupported format "%s".', $format));
+            throw new RuntimeException('Lock file does not exist.');
         }
 
         $postFields = array('lock' => '@'.$lock);
@@ -80,7 +69,7 @@ public function check($lock, $format = 'text')
         curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
         curl_setopt($curl, CURLOPT_HEADER, true);
         curl_setopt($curl, CURLOPT_URL, $this->endPoint);
-        curl_setopt($curl, CURLOPT_HTTPHEADER, array('Accept: '.$accept));
+        curl_setopt($curl, CURLOPT_HTTPHEADER, array('Accept: application/json'));
         curl_setopt($curl, CURLOPT_POSTFIELDS, $postFields);
         curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, $this->timeout);
         curl_setopt($curl, CURLOPT_TIMEOUT, 10);
@@ -95,7 +84,7 @@ public function check($lock, $format = 'text')
         if ('phar://' === substr(__FILE__, 0, 7)) {
             $tmpFile = tempnam(sys_get_temp_dir(), 'sls');
             if (false === @copy($cert, $cert = $tmpFile)) {
-                throw new \RuntimeException(sprintf('Unable to copy the certificate in "%s".', $tmpFile));
+                throw new RuntimeException(sprintf('Unable to copy the certificate in "%s".', $tmpFile));
             }
         }
         curl_setopt($curl, CURLOPT_CAINFO, $cert);
@@ -109,7 +98,7 @@ public function check($lock, $format = 'text')
                 unlink($tmpFile);
             }
 
-            throw new \RuntimeException(sprintf('An error occurred: %s.', $error));
+            throw new RuntimeException(sprintf('An error occurred: %s.', $error));
         }
 
         $headersSize = curl_getinfo($curl, CURLINFO_HEADER_SIZE);
@@ -118,19 +107,15 @@ public function check($lock, $format = 'text')
 
         $statusCode = curl_getinfo($curl, CURLINFO_HTTP_CODE);
         if (400 == $statusCode) {
-            if ('text' == $format) {
-                $error = trim($body);
-            } else {
-                $data = json_decode($body, true);
-                $error = $data['error'];
-            }
+            $data = json_decode($body, true);
+            $error = $data['error'];
 
             curl_close($curl);
             if ($tmpFile) {
                 unlink($tmpFile);
             }
 
-            throw new \InvalidArgumentException($error);
+            throw new RuntimeException($error);
         }
 
         if (200 != $statusCode) {
@@ -139,7 +124,7 @@ public function check($lock, $format = 'text')
                 unlink($tmpFile);
             }
 
-            throw new \RuntimeException(sprintf('The web service failed for an unknown reason (HTTP %s).', $statusCode));
+            throw new RuntimeException(sprintf('The web service failed for an unknown reason (HTTP %s).', $statusCode));
         }
 
         curl_close($curl);
@@ -148,16 +133,16 @@ public function check($lock, $format = 'text')
         }
 
         if (!(preg_match('/X-Alerts: (\d+)/', $headers, $matches) || 2 == count($matches))) {
-            throw new \RuntimeException('The web service did not return alerts count.');
+            throw new RuntimeException('The web service did not return alerts count.');
         }
 
-        $this->vulnerabilitiesCount = intval($matches[1]);
+        $this->vulnerabilityCount = intval($matches[1]);
 
-        return $body;
+        return json_decode($body, true);
     }
 
     public function getLastVulnerabilityCount()
     {
-        return $this->vulnerabilitiesCount;
+        return $this->vulnerabilityCount;
     }
 }

composer.json

@@ -18,7 +18,7 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "1.3-dev"
+            "dev-master": "2.0-dev"
         }
     }
 }