phpro / grumphp

A PHP code-quality tool
MIT License
4.13k stars 430 forks source link

Replaced abandoned Sensiolabs security checker with Enlightn security checker #870

Closed paras-malhotra closed 3 years ago

paras-malhotra commented 3 years ago
Q A
Branch master for features and deprecations
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets https://github.com/phpro/grumphp/issues/865

This PR replaces the abandoned Sensiolabs security checker with the Enlightn security checker. Fixes #865

New Task Checklist:

Even though this is not a new task, I've still filled the checklist below

Landerstraeten commented 3 years ago

Hi @paras-malhotra. Thank you for your time and effort in the PR. Unfortunately, we prefer to would prefer to recommend the official tools: https://github.com/fabpot/local-php-security-checker and https://symfony.com/download.

Feel free to create configurable tasks that use these tools.

paras-malhotra commented 3 years ago

@Landerstraeten does this mean you're removing security checking from GrumPHP or that you're replacing it with the local-php-security-checker? I hope you saw this comment on the licensing issues with the local-php-security-checker.

/cc @JeppeKnockaert

veewee commented 3 years ago

That does make sense. Would it be an option to build 1 task with the bare minimum of configurable options. In this task, you are able to choose an executable that fits your needs: symfony / local-php-security-checker / enligthn version?

paras-malhotra commented 3 years ago

@veewee, the 3 of them differ in arguments. For instance, the local-php-security-checker has a --path option for the composer lock file whereas the Enlightn security checker has an optional argument for the composer lock path. The Symfony security checker seems to have no arguments.

So, would it be possible to combine them in a single task? If yes, I'd be happy to modify this PR to do that.

veewee commented 3 years ago

Gonna let it sink in over the weekend. Reopening for now.

paras-malhotra commented 3 years ago

@veewee since the Sensiolabs security checker and API has stopped working since Monday, I think we should make a decision soon. GrumPHP users are currently without security checkers.

veewee commented 3 years ago

I was kinda waiting for a comment on this issue you posted to see where to go.

Bit I assume that fabpot is nog going to advise you use a package he does not own...

https://github.com/FriendsOfPHP/security-advisories/pull/526#issuecomment-770609275

paras-malhotra commented 3 years ago

Not sure about that. I'm hopeful he will add it. The package is backed with tests and Symfony and Fabien have supported many community packages before. Just need more support on adding it (more comments or likes perhaps). To be honest, there are several use cases that none of the other recommended packages currently support.

Anyway, I guess we shouldn't hold up on that for this PR.