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

Ignore development-only packages in security check #229

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

fredden
Copy link
Member

@fredden fredden commented Nov 27, 2024

Proposed Changes

Only perform security check on actual dependencies / ignore development-only dependencies for this check.

Related Issues

#228 (comment)

jrfnl
jrfnl previously approved these changes Nov 27, 2024
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Thanks @fredden!

@jrfnl
Copy link
Member

jrfnl commented Nov 27, 2024

🤔 Hmm... based on the build on your fork, this doesn't solve it for PHP 5.4...? Composer still creates the lock file, even if the packages aren't being installed and the lock file is what's used by the Security check...

@fredden
Copy link
Member Author

fredden commented Nov 27, 2024

🤔 Hmm... based on the build on your fork, this doesn't solve it for PHP 5.4...? Composer still creates the lock file, even if the packages aren't being installed and the lock file is what's used by the Security check...

I've updated the check command to use the actually-installed packages instead.

@fredden fredden requested a review from jrfnl November 27, 2024 13:33
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Interesting. Looks like checking the installed.json file is an undocumented option.

Also noticed that the Local security checker package is now deprecated in favour of the composer audit command.

We should probably update this workflow to use that once support for PHP < 7.2 has been dropped (issue #221).

@fredden
Copy link
Member Author

fredden commented Nov 28, 2024

It turns out that this checker tool also has a --no-dev flag which could have been used here. If you would prefer that method, let me know and I will update this pull request accordingly.

@fredden
Copy link
Member Author

fredden commented Jan 13, 2025

The failing tests here will be solved by #221

@jrfnl
Copy link
Member

jrfnl commented Jan 14, 2025

@Potherca Do you still want to have a look at this or can I merge it ?

@Potherca
Copy link
Member

Merge at will.

@jrfnl jrfnl merged commit c7b4e64 into PHPCSStandards:main Jan 14, 2025
66 of 76 checks passed
@fredden fredden deleted the feature/security-check-no-dev branch January 14, 2025 19:19
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.

None yet

3 participants