Repository navigation
Phpcs reduce ignores - #955
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reduces inline PHPCS suppressions by restructuring input handling and nonce validation so that WordPress security sniffs can be satisfied without phpcs:ignore/phpcs:disable, while preserving existing behavior in the Two-Factor plugin’s settings and revalidation flows.
Changes:
- Replaces
$_GET['page']usage (with nonce-verification ignore) by usingget_current_screen()to detect the Two-Factor settings screen. - Introduces
two_factor_is_valid_user_action()and updates core logic/tests to use it, while deprecatingTwo_Factor_Core::is_valid_user_action(). - Sanitizes posted provider arrays earlier (settings page + user profile options) and adds PHPCS config for custom nonce verification helper.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
two-factor.php |
Uses get_current_screen() for settings-page detection and adds two_factor_is_valid_user_action() helper. |
class-two-factor-core.php |
Deprecates is_valid_user_action() in favor of the new helper; adjusts request parsing and revalidation nonce checks. |
settings/class-two-factor-settings.php |
Sanitizes enabled provider inputs inline to remove PHPCS suppressions. |
tests/class-two-factor-core.php |
Updates coverage and assertions to target two_factor_is_valid_user_action(). |
phpcs.xml.dist |
Configures PHPCS nonce verification sniff to recognize the new helper as a custom nonce verification function. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
masteradhoc
left a comment
There was a problem hiding this comment.
LGTM! Just added the final versions to finalize this :)
Co-authored-by: Brian <brian@brianhaas.li>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
What?
Reducing
phpcs:ignoreandphpcs:disableinstances.Fixes #
Why?
It's not unknown that these come back cause problems later as code is removed.
How?
Modifies code to remove the need where possible.
Commonly:
Use of AI Tools
N/A
Testing Instructions
TBA once this comes out of draft.
Screenshots or screencast
Changelog Entry