Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced abandoned Sensiolabs security checker with Enlightn security checker #870

Merged
merged 2 commits into from
Feb 4, 2021

Conversation

paras-malhotra
Copy link
Contributor

@paras-malhotra paras-malhotra commented Jan 28, 2021

Q A
Branch master for features and deprecations
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Documented? yes
Fixed tickets #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

  • Are the dependencies added to the composer.json suggestions?
  • Is the doc/tasks.md file updated?
  • Are the task parameters documented?
  • Is the task registered in the tasks.yml file?
  • Does the task contains phpunit tests?
  • Is the configuration having logical allowed types?
  • Does the task run in the correct context?
  • Is the run() method readable?
  • Is the run() method using the configuration correctly?
  • Are all CI services returning green?

@Landerstraeten
Copy link
Contributor

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
Copy link
Contributor Author

paras-malhotra commented Jan 29, 2021

@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
Copy link
Contributor

veewee commented Jan 29, 2021

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
Copy link
Contributor Author

paras-malhotra commented Jan 29, 2021

@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 veewee reopened this Jan 29, 2021
@veewee
Copy link
Contributor

veewee commented Jan 29, 2021

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

@paras-malhotra
Copy link
Contributor Author

@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
Copy link
Contributor

veewee commented Feb 3, 2021

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...

FriendsOfPHP/security-advisories#526 (comment)

@paras-malhotra
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sensiolabs/security-checker is abandoned and replaced by fabpot/local-php-security-checker
3 participants