This PR can be summarized in the following changelog entry:
N/A
Relevant technical choices:
A while back the roave/security-advisories dependency was added to protect against installing dependencies with known security issues.
The roave/security-advisories dependency is supposed to be at dev-master to always have the latest information on security issues available.
However, this repository has a composer.lock file, which means that the roave/security-advisories dependency is locked at the version of last update and the dependency is not updated often enough to provide adequate protection.
This application bases its information on the same source data as the roave/security-advisories, but is a tool which checks the information live, i.e. even if the application would be out-of-date, it will still use the latest information available about security vulnerabilities when running the check.
I've chosen not to make this application a dependency (which would need to be updated regularly again), but instead to get the phar version during the Travis build, effectively making sure that the check will always use the latest version.
As the composer.lock doesn't change between builds, this check only needs to run on one build.
This does mean that the security check will only be done during the Travis build, not during a composer update, but at least it will not use out-of-date information which might miss newly discovered security vulnerabilities.
This does mean that a build may fail for an unrelated PR if a security issue has been discovered in one of the dependencies, but rather that, than releasing a new version of the plugin with an insecure dependency which hasn't been noticed because of the security-advisories were out of date.
As an alternative, it could be elected to use the Symfony Security Monitoring service, which is based on the same code and will pro-actively monitor the composer.lock file, even for inactive/less active repositories, but that is a paid outside service, so would need a management decision.
Dieterrr
commented
5 years ago
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
A while back the
roave/security-advisoriesdependency was added to protect against installing dependencies with known security issues.The
roave/security-advisoriesdependency is supposed to be atdev-masterto always have the latest information on security issues available.However, this repository has a
composer.lockfile, which means that theroave/security-advisoriesdependency is locked at the version of last update and the dependency is not updated often enough to provide adequate protection.So, having said all that, I'm proposing to change over to using the
sensiolabs/security-checkerinstead.This application bases its information on the same source data as the
roave/security-advisories, but is a tool which checks the information live, i.e. even if the application would be out-of-date, it will still use the latest information available about security vulnerabilities when running the check.I've chosen not to make this application a dependency (which would need to be updated regularly again), but instead to get the
pharversion during the Travis build, effectively making sure that the check will always use the latest version.As the
composer.lockdoesn't change between builds, this check only needs to run on one build.This does mean that the security check will only be done during the Travis build, not during a
composer update, but at least it will not use out-of-date information which might miss newly discovered security vulnerabilities.This does mean that a build may fail for an unrelated PR if a security issue has been discovered in one of the dependencies, but rather that, than releasing a new version of the plugin with an insecure dependency which hasn't been noticed because of the
security-advisorieswere out of date.As an alternative, it could be elected to use the Symfony Security Monitoring service, which is based on the same code and will pro-actively monitor the
composer.lockfile, even for inactive/less active repositories, but that is a paid outside service, so would need a management decision.Test instructions
I have tested the principle of this PR on Travis already - see: https://travis-ci.org/Yoast/yoastseo-amp/builds/496340692 As a secondary test, I introduced a package with a security vulnerability into the repo (of course removed again before I pulled the PR). The Travis build for the commit which included the insecure package failed as expected - see: https://travis-ci.org/Yoast/yoastseo-amp/builds/496347854.