Skip to content

Add allow-list functionality to securitychecker_enlightn #1161

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

Merged
merged 6 commits into from
Mar 21, 2025

Conversation

peterjaap
Copy link
Contributor

Q A
Branch v2.x
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Documented? yes

Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice addition, thanks!
Can you make sure to add a test-case for this new allow_list argument? Otherwise we won't be able to merge this.

@peterjaap
Copy link
Contributor Author

@veewee I've added a test case and used addArgumentArrayWithSeparatedValue().

Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. It's curently not working as expected.
I've added some additional pointers in code.

@@ -24,10 +24,12 @@ public static function getConfigurableOptions(): ConfigOptionsResolver
$resolver->setDefaults([
'lockfile' => './composer.lock',
'run_always' => false,
'allow_list' => []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method provideConfigurableOptions in the test should be altered to cover this newly added default.

@leonhelmus
Copy link

@peterjaap nice addition. This is something i could use as well. Thanks for adding it. When will you have time to look at the changes needed?

- Document new option in enlightn.md with example usage

- Fix argument formatting to use comma-separated values for --allow-list

- Update unit test to include the new option in default configuration

This change allows users to specify CVEs that should be ignored during security checks when they've been assessed and determined not to pose a risk.
@peterjaap
Copy link
Contributor Author

@veewee I processed your remarks

@veewee veewee added this to the 2.12.0 milestone Mar 21, 2025
@veewee veewee merged commit f18e860 into phpro:v2.x Mar 21, 2025
26 checks passed
@veewee
Copy link
Contributor

veewee commented Mar 21, 2025

Thanks

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.

3 participants