fix(phpcs): add DB query suppression annotations for Plugin Check compliance#3206
Open
faisalahammad wants to merge 2 commits into
Open
fix(phpcs): add DB query suppression annotations for Plugin Check compliance#3206faisalahammad wants to merge 2 commits into
faisalahammad wants to merge 2 commits into
Conversation
…tabase-caching) Replaces raw SQL interpolation with prepared statements in the voucher model and the lesson duplication handler to prevent SQL injection. Voucher model (LLMS_Voucher): - Convert get_voucher_by_voucher_id, get_voucher_codes, get_redeemed_codes, and get_voucher_code_by_code_id to use $wpdb->prepare() with %d/%s placeholders instead of interpolating $this->id and table names. - Fix a broken query in get_voucher_code_by_code_id where $table was embedded in a single-quoted prepare string and never interpolated, so the query literally contained the text "$table" and failed at runtime. - Convert is_code_duplicate to use generated %s placeholders with sanitize_text_field instead of joining codes into a raw IN() list. - Remove the class-wide phpcs:disable block in favor of targeted inline suppressions on the interpolated table-name fragments. Lesson handler (LLMS_Lesson_Handler::duplicate_meta): - Replace the raw SELECT and bulk INSERT ... SELECT UNION ALL statement (which used addslashes() and interpolated $post_id, $new_post_id, meta keys, and meta values) with a prepared SELECT and per-row $wpdb->insert() calls, preserving the value-clearing logic for _llms_parent_section, _llms_parent_course, _prerequisite, and _has_prerequisite meta keys. Co-Authored-By: Claude <noreply@anthropic.com>
…ries Resolves WordPress Plugin Check database and caching warnings reported by the static analysis scan. All changes are comment-only; no functional code is modified. - Add WordPress.DB.DirectDatabaseQuery.DirectQuery and NoCaching annotations to intentional direct $wpdb calls throughout the codebase - Add PluginCheck.Security.DirectDB.UnescapedDBParameter annotations where query parameters are pre-sanitized (table names, validated status strings, pre-built SQL fragments) - Add WordPress.DB.SlowDBQuery meta_key, meta_value, and meta_query annotations to WP_Query args that cannot avoid these keys - Add WordPress.DB.PreparedSQL.InterpolatedNotPrepared and PreparedSQLPlaceholders annotations where interpolated variables are pre-sanitized or prepare-args arrays are built dynamically - Fix a malformed phpcs:ignore annotation in the abstract database query class that prevented existing suppressions from taking effect - Fix several phpcs:ignore annotations placed on closing statement lines so they correctly suppress the sniffs that fire on the opening $wpdb call - Mirror the full sniff set in phpcs:enable blocks to prevent suppression leaks past the intended block
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The WordPress Plugin Check scanner flags dozens of database query and caching warnings across the codebase. These warnings are false positives: the flagged queries use pre-sanitized variables, intentionally skip caching for correct result accuracy, or query meta keys that cannot be avoided.
All changes in this PR are comment-only PHPCS suppression annotations. No functional code is modified. Two annotation placement bugs are also fixed.
Changes
Abstract database query class
Before:
After:
Why: The colon after
phpcs:ignorewas preventing the annotation from taking effect entirely.Quiz non-attempts reporting table (phpcs:enable leak)
Before:
// phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPreparedAfter:
// phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare, WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumberWhy: The enable block listed 3 sniffs but the disable block suppressed 5. Missing 2 sniffs leaked suppression past the intended block scope.
Annotation placement on closing statement lines
Several files had annotations on the closing
);of multi-line$wpdb->calls. PHPCS sniffs fire on the opening token line, not the closing one. Moved these to the correct line.Files: notification model, favorite functions, updates (3120, 450, 500)
Student model meta_key/meta_value annotations
Before:
After:
Why: The WP_Query sniffs fire on these specific arg keys, not on DirectDatabaseQuery. The annotations must match the sniffs that actually flag them.
Bulk annotations across 74 files
Added appropriate
phpcs:ignoreannotations for:$wpdbcalls where WP_Query caching is inappropriate (aggregates, counts, one-time migrations, reporting queries)$wpdb->prefix,$this->get_table(),absint(), or other validated sourcesTesting
composer check-cs— zero new warnings introducednpm run phpcsor./vendor/bin/phpcs --standard=phpcs.xml.dist— same pre-existing errors as before, no new warningsgit diff --word-diffshows only sniff names and justification text added