EZP-31821: Failing security-checker results in errored installation

[glye]

https://jira.ez.no/browse/EZP-31821
For all supported branches.

There have been repeated requests for the security checker not to break the installation process. If we accept the premise that the checker should not lead to errors, but should still run and display packages with vulnerabilites, then this is a simple way to do it. This shows the output from composer install && echo "SUCCESS!" in v1.13 (and v2.5).

In v3.x Symfony scripts are run in a different way, and if we apply the same fix there it will not show the output from security-checker, just a green OK. If we're not going to fork the relevant Symfony repo, and if there's no easy way to overload it, then we have to go back to the old way of running security checker. And that requires moving it out from the auto-scripts section, as is shown here. With that fix, there is no green OK, but it works the same way as in v2/v1.

Yes, it's hackish, and the || true is visible in the output. But it seems to be the simplest possible solution, and is easily reversible for those who may want to keep the old behaviour.

[alongosz]

Failed installation means that a Developer should immediately do composer update. This approach contradicts basic security principles and creates unnecessary liability IMHO.
If you're all aware of that, I can accept the PR. I'm not gonna accept responsibility for consequences of that change though :)

[andrerom]

@alongosz Welcome to the real world. The thing is this will block any kind of deploy or CI job in the way it is now. So we either need to ignore exit code like done here, or we need to remove the check from install / update as it blocks usage of composer in a lot of cases.

So we went this way to make sure users are ALWAYS notified about possible issues, but never blocked from deploying what might be a critical fix for them.

[glye]

I agree there are very good security reasons for blocking on vulnerabilities. I don't like this change. But then again, what André said.

[alongosz]

Sure, no problem.

[konradoboza]

No QA? 😨

[andrerom]

@glye Will reopen this PR and send that to QA as this was not merged up to other branches yet, good catch @konradoboza

---

In the meantime, an possible example of where this can benefit us as well:

install_dependencies_1_9a031fbab15c | !!  Symfony Security Check Report
install_dependencies_1_9a031fbab15c | !!  =============================
install_dependencies_1_9a031fbab15c | !!  
install_dependencies_1_9a031fbab15c | !!  1 packages have known vulnerabilities.
install_dependencies_1_9a031fbab15c | !!  
install_dependencies_1_9a031fbab15c | !!  symfony/http-kernel (v5.0.11)
install_dependencies_1_9a031fbab15c | !!  -----------------------------
install_dependencies_1_9a031fbab15c | !!  
install_dependencies_1_9a031fbab15c | !!   * [CVE-2020-15094][]: Prevent RCE when calling untrusted remote with CachingHttpClient

https://travis-ci.org/github/ezsystems/ezplatform-http-cache/jobs/734422273

TL;DR; REST test failing due to security notice, as Symfony 5.0 is not maintained anymore it would basically block any kind of fixes against this branch, no matter if you use CachingHttpClient or not. In this case we can rebase PR to 3.1 instead, but we can't always do that (e.g security fix on branch in EOM but not yet EOL).

---

Said in another way, almost all security advisories are conditional, that is why they are called advisory as the user of the software will need to consider it on a case by case basis.