From 28884ec40d1dca333f01da891a36c8bb385aaf8e Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 17:01:53 +0600 Subject: [PATCH 01/23] Remove Help tab from MotionKit admin page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No real support/community links exist yet — motionkit.io/support, motionkit.io/community, and motionkit.io/docs are either 404 or unready. Rather than ship dead links, remove the Help tab entirely: sidebar nav entry, switch-case dispatch, and render_help_tab() (the Support banner + Community/Documentation cards). ?tab=help now falls through to the default Connect tab instead of erroring. Only Connect and Tools remain as navigable tabs. CLAUDE.md synced (file-tree comment + Auth section), and corrected a stale claim that License was a separate tab — it's actually rendered inline inside Connect via the private render_license_tab(). --- CLAUDE.md | 6 ++- includes/Auth/ConnectPage.php | 72 ----------------------------------- 2 files changed, 4 insertions(+), 74 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6266b88..a47d478 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ includes/ ├── Auth/ │ ├── JwtTokenManager.php ← Mint + validate editor session JWTs │ ├── OAuthHandler.php ← OAuth connect/disconnect to motionkit.io -│ └── ConnectPage.php ← Admin "MotionKit" menu (Connect/License/Tools/Help tabs) +│ └── ConnectPage.php ← Admin "MotionKit" menu (Connect/Tools tabs) │ ├── Admin/ │ └── PermalinkNotice.php ← Plain-permalinks nag notice @@ -181,7 +181,9 @@ settings via `settings_config()` helper. - `includes/Auth/OAuthHandler.php` — OAuth connect/disconnect flow to motionkit.io. Fires `do_action('motionkit/oauth/connected')` on successful connect. - `includes/Auth/ConnectPage.php` — top-level admin menu ("MotionKit") with tabs: - Connect / License / Tools / Help. Render-methods per tab. + Connect / Tools. Render-methods per tab. License status renders inline inside + the Connect tab (`render_license_tab()` is a private method embedded in + `render_connect_tab()`, not a separate navigable tab). ### Permission model (RestApi.php) diff --git a/includes/Auth/ConnectPage.php b/includes/Auth/ConnectPage.php index 6ddc5b1..3719370 100644 --- a/includes/Auth/ConnectPage.php +++ b/includes/Auth/ConnectPage.php @@ -152,8 +152,6 @@ public function render_page(): void 'tools' => ['label' => __('Tools', 'motionkit'), 'icon' => '🔧'], ]; - $tabs['help'] = ['label' => __('Help', 'motionkit'), 'icon' => 'ⓘ']; - ?>
@@ -209,9 +207,6 @@ class="motionkit-sidebar-link render_tools_tab(); break; - case 'help': - $this->render_help_tab(); - break; case 'connect': default: $this->render_connect_tab($current_user); @@ -690,73 +685,6 @@ private function render_disconnected_state(): void - -
-
-

- ⚡ -

-

- -

-
- - - -
- - -
-
-
- - - - - - - -
-

-

- -

- - - -
- -
-
- - - - - - - - - -
-

-

- -

- - - -
-
- - Date: Thu, 6 Aug 2026 17:24:40 +0600 Subject: [PATCH 02/23] Remove unused Plugin::should_skip_init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dead code: its only call site was already commented out, and the method itself had a real bug — DOING_CRON would return true unconditionally, which (if ever re-enabled) would skip Plugin::init() on cron requests entirely. That would break LicenseStatus's daily refresh hook registration, which init_auth() explicitly registers outside the is_admin() branch specifically so cron keeps working (see the comment there). Removed the method, its commented call, and the matching phpstan-baseline.neon suppression entry. PHPStan/phpcs clean. --- includes/Plugin.php | 25 ------------------------- phpstan-baseline.neon | 5 ----- 2 files changed, 30 deletions(-) diff --git a/includes/Plugin.php b/includes/Plugin.php index 2f72f06..0344de8 100644 --- a/includes/Plugin.php +++ b/includes/Plugin.php @@ -209,11 +209,6 @@ public function maybe_fix_autoload_flags(): void */ public function init(): void { - // Performance optimization: Skip initialization on certain requests - // if ($this->should_skip_init()) { - // return; - // } - // Send platform identification header for MotionKit detect-platform add_action('send_headers', [$this, 'send_platform_header']); @@ -232,26 +227,6 @@ public function init(): void do_action('MOTIONKIT_LOADED'); } - /** - * Check if initialization should be skipped - * - * @return bool True if should skip, false otherwise - */ - private function should_skip_init(): bool - { - // Skip on AJAX requests unless it's our AJAX - if (defined('DOING_AJAX') && DOING_AJAX) { - return !isset($_REQUEST['action']) || strpos(sanitize_text_field( wp_unslash($_REQUEST['action'] )), 'motionkit_') === false; - } - - // Skip on cron requests - if (defined('DOING_CRON') && DOING_CRON) { - return true; - } - - return false; - } - /** * Initialize frontend functionality * diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a5fa18c..88da158 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -15,11 +15,6 @@ parameters: count: 3 path: includes/Frontend/Frontend.php - - - message: "#^Method MotionKit\\\\Plugin\\:\\:should_skip_init\\(\\) is unused\\.$#" - count: 1 - path: includes/Plugin.php - - message: "#^Unreachable statement \\- code above always terminates\\.$#" count: 1 From abe53d94e8722f2ddb378aadc6262d655ddfd6bd Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 17:33:52 +0600 Subject: [PATCH 03/23] upate --- README.md | 132 ------------------------------------------------------ 1 file changed, 132 deletions(-) delete mode 100644 README.md diff --git a/README.md b/README.md deleted file mode 100644 index 9611dfb..0000000 --- a/README.md +++ /dev/null @@ -1,132 +0,0 @@ -# GSAP Animation Builder for WordPress - -A powerful and customizable GSAP animation builder plugin for WordPress that allows you to create stunning animations with ease. - -## Features - -- 🎨 **Easy Animation Creation** - Build animations with a visual interface -- ⚡ **Performance Optimized** - Lazy loading, caching, and conditional asset loading -- 🏗️ **Modern Architecture** - Built with design patterns (Factory, Strategy, Decorator, Singleton) -- 🔒 **Secure** - Comprehensive security measures and data sanitization -- 🌐 **Extensible** - Easy to extend with custom strategies and decorators -- 📚 **Well Documented** - Comprehensive developer documentation - -## Requirements - -- WordPress 6.7+ -- PHP 7.4+ -- GSAP Library (loaded separately or included) - -## Installation - -1. Upload the plugin folder to `/wp-content/plugins/` -2. Activate the plugin through the 'Plugins' menu in WordPress -3. Configure settings as needed - -## Quick Start - -### Basic Usage - -```php -// Get plugin instance -$plugin = \WcfAnimationBuilder\Plugin::get_instance(WCF_ANIMATION_BUILDER_PLUGIN_FILE); - -// Access helpers -$option = \WcfAnimationBuilder\Helpers\Helper::get_option('my_key', 'default'); -``` - -### Using Animation Strategies - -```php -use WcfAnimationBuilder\Strategy\AnimationStrategyFactory; - -// Create fade animation -$fade_strategy = AnimationStrategyFactory::create('fade'); -$config = ['duration' => 1, 'ease' => 'power2.out']; -$animation_code = $fade_strategy->execute($config); - -// Create slide animation -$slide_strategy = AnimationStrategyFactory::create('slide'); -$config = ['duration' => 1, 'direction' => 'left']; -$animation_code = $slide_strategy->execute($config); -``` - -### Conditional Asset Loading - -```php -use WcfAnimationBuilder\Decorator\ConditionalAssetLoaderDecorator; -use WcfAnimationBuilder\Factory\ComponentFactory; - -$base_loader = ComponentFactory::create_asset_loader(); -$conditional_loader = new ConditionalAssetLoaderDecorator( - $base_loader, - fn() => is_single() -); - -$conditional_loader->enqueue_style('my-style', 'assets/style.css'); -``` - -## Performance Optimizations - -The plugin includes several performance optimizations: - -- **Caching System** - Object caching for options and expensive operations -- **Lazy Loading** - Components loaded only when needed -- **Conditional Loading** - Assets loaded based on context -- **Duplicate Prevention** - Prevents duplicate asset enqueueing -- **Skip Unnecessary Requests** - Skips initialization on AJAX/cron requests - -### Using Cache - -```php -use WcfAnimationBuilder\Helpers\Cache; - -// Cache expensive operation -$data = Cache::remember('key', function() { - return expensive_operation(); -}, 3600); - -// Get/Set cache -Cache::set('key', $value, 3600); -$value = Cache::get('key', 'default'); -``` - -## Developer Documentation - -For detailed developer documentation, see [DEVELOPER.md](./DEVELOPER.md) - -## Hooks & Filters - -### Actions - -- `WCF_ANIMATION_BUILDER_LOADED` - Fired after plugin initialization -- `wcf_animation_builder_activated` - Fired on activation -- `wcf_animation_builder_deactivated` - Fired on deactivation - -### Usage Example - -```php -add_action('WCF_ANIMATION_BUILDER_LOADED', function() { - // Your code here -}); -``` - -## Constants - -- `WCF_ANIMATION_BUILDER_VERSION` - Plugin version -- `WCF_ANIMATION_BUILDER_PLUGIN_DIR` - Plugin directory path -- `WCF_ANIMATION_BUILDER_PLUGIN_URL` - Plugin URL -- `WCF_ANIMATION_BUILDER_PLUGIN_FILE` - Main plugin file - -## Support - -For support, feature requests, or bug reports, please visit the plugin repository. - -## License - -GPL v2 or later - -## Credits - -Built with modern PHP practices and WordPress coding standards. - From 33712524604ce73c1c9ea53cdb0bb3dd737e8993 Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 17:39:54 +0600 Subject: [PATCH 04/23] Move admin menu icon CSS to enqueued file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConnectPage::print_menu_icon_style() echoed a raw '; + $version = defined('MOTIONKIT_VERSION') ? MOTIONKIT_VERSION : '1.0.0'; + + wp_enqueue_style( + 'motionkit-admin-menu-icon', + plugins_url('assets/build/admin-menu-icon.css', MOTIONKIT_PLUGIN_FILE), + [], + $version + ); } private function get_menu_icon(): string diff --git a/src/css/admin-menu-icon.css b/src/css/admin-menu-icon.css new file mode 100644 index 0000000..2df4a33 --- /dev/null +++ b/src/css/admin-menu-icon.css @@ -0,0 +1,9 @@ +/* MotionKit top-level admin menu icon sizing (applies on every wp-admin + page, since the admin menu sidebar is global — unlike admin.css, which + only loads on the MotionKit settings page itself). */ + +#adminmenu #toplevel_page_motionkit-connect .wp-menu-image img { + width: 23px; + height: 23px; + padding: 7px 0 0; +} diff --git a/webpack.config.js b/webpack.config.js index 3abb61e..ee1d608 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -83,6 +83,7 @@ const mainConfig = { admin: "./src/css/admin.css", "admin-tools": "./src/css/admin-tools.css", "admin-bar": "./src/css/admin-bar.css", + "admin-menu-icon": "./src/css/admin-menu-icon.css", ...getPresetEntries({ folder: "./src/modules/animation-builder/frontend/animation-type/preset", From 5a24119c82e142bc22397ebe8bd2c81bb8b249ab Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 18:10:35 +0600 Subject: [PATCH 05/23] Fix Plugin Check (PCP) findings: name trademark, Tested up to, nonce unslash - Plugin name contained "WordPress" (wp.org naming policy disallows this anywhere in a plugin's display name): "Motionkit - Visual Animation with GSAP for WordPress" -> "Motionkit - Visual Animation with GSAP" in both readme.txt and motionkit.php. - Tested up to must be major.minor only, no patch version: 7.0.2 -> 7.0 in both files (the version was right, the granularity wasn't). - ConnectPage.php: $_POST['_wpnonce'] now sanitized+unslashed before wp_verify_nonce(), matching the pattern used elsewhere in the class. - Removed scripts/copy-to-editor copy.js, a stray unreferenced duplicate (older hardcoded-path version of copy-to-editor.js). - phpstan-bootstrap.php: synced stale MOTIONKIT_VERSION 1.5.1 -> 1.0.0. Investigated but did not change: PCP's hidden-files/markdown/.claude/ .github findings are dist-scope false positives (all already excluded via .distignore, PCP scans the raw working dir not the filtered zip); NonceVerification.Recommended warnings are all read-only $_GET display flags with no state-changing side effect; DirectDatabaseQuery warnings are correctly wp->prepare()'d one-shot admin/uninstall queries. Documented in WPORG-SUBMISSION.md's new "Fifth pass" section. --- WPORG-SUBMISSION.md | 71 +++++++++++++++++++++- includes/Auth/ConnectPage.php | 3 +- motionkit.php | 4 +- phpstan-bootstrap.php | 2 +- readme.txt | 4 +- scripts/copy-to-editor copy.js | 108 --------------------------------- 6 files changed, 77 insertions(+), 115 deletions(-) delete mode 100644 scripts/copy-to-editor copy.js diff --git a/WPORG-SUBMISSION.md b/WPORG-SUBMISSION.md index 0ceee0c..4b44e43 100644 --- a/WPORG-SUBMISSION.md +++ b/WPORG-SUBMISSION.md @@ -426,6 +426,72 @@ will ever exist in the wild). so a clean write-then-read under the new key was the correct verification, not a migration check). +### Fifth pass — Plugin Check (PCP) live scan (2026-08-06) + +Ran the official Plugin Check scanner against the working directory. Real, +actionable findings and fixes: + +- **Trademark: plugin name contained "WordPress"** — wp.org's naming policy + disallows "WordPress" anywhere in a plugin's display name, no exceptions. + Both `readme.txt`'s `=== ... ===` title and `motionkit.php`'s `Plugin Name:` + said "Motionkit – Visual Animation with GSAP for WordPress" → renamed to + "Motionkit – Visual Animation with GSAP" in both places. (Historical + references to the old name elsewhere in this doc's earlier dated passes are + left as-is — they're a record of what the name was at that point in time.) +- **`Tested up to` had a patch version** — wp.org only accepts `major.minor` + for this header (e.g. `7.0`, never `7.0.2`). Had been set to `7.0.2` in an + earlier pass to match WordPress.org's actual current release number, which + was the right *version* but the wrong *granularity* — fixed to `7.0` in + both `readme.txt` and `motionkit.php`. +- **`ConnectPage.php:737`** — `$_POST['_wpnonce']` was passed straight into + `wp_verify_nonce()` without `wp_unslash()`/sanitization first. Low + functional risk (nonces are alphanumeric, unaffected by magic-quotes + slashing in practice) but a real gap against the sanitize-input pattern + used everywhere else in this file — fixed to sanitize+unslash before the + `wp_verify_nonce()` call, matching the pattern already used for every other + `$_POST`/`$_GET` read in this class. +- **Stray duplicate file**: `scripts/copy-to-editor copy.js` (space in the + filename — an accidental OS-level copy of `copy-to-editor.js`, an older, + hardcoded-path version of the same script vs. the current `.env`-driven + one). Unreferenced anywhere, dev-only (`scripts/` already excluded via + `.distignore`), deleted. +- **`phpstan-bootstrap.php` stale version**: still said `1.5.1` after the + `1.0.0` version-rename pass — synced. (PCP's separate "missing ABSPATH + guard" flag on this file was **not** acted on: this is a dev-only PHPStan + CLI stub that fakes plugin constants for static analysis, never included by + a real WP request, and already `.distignore`d — adding an ABSPATH check + would be following the rule's letter against a file the rule doesn't + actually apply to.) + +**Findings investigated and NOT changed** (confirmed false-positives for this +scan mode, not real defects): + +- `.env`, `.env.example`, `phpcs.xml.dist`, `.distignore`, `.gitignore`, + `.claude`, `.github`, `CLAUDE.md`, `flow.md`, `USAGE.md`, + `WPORG-SUBMISSION.md` — PCP scans the raw working directory, not the + `.distignore`-filtered dist ZIP. Confirmed (Third pass, "GitHub Actions CI" + section below, and independently re-checked this pass) that every one of + these is already listed in `.distignore` and would not ship. Re-verify with + an actual `wp dist-archive` build before the real upload, per the + pre-submission checklist below. +- All `WordPress.Security.NonceVerification.Recommended` warnings (~40+ + across `ConnectPage.php`/`OAuthHandler.php`/`Frontend.php`/ + `ScrollSmoother.php`) — every flagged read is a display-only `$_GET` flag + (active tab, error message, connected/disconnected banner state) with no + state-changing side effect; nonce verification protects actions that + *change* something, not read-only navigation params. Already + individually verified and excluded in this project's own `phpcs.xml.dist` + (see the "wp.org standard" section below) — PCP just doesn't honor that + local ruleset since it runs its own fixed one. +- `WordPress.DB.DirectDatabaseQuery.*` warnings on the Tools tab bulk-delete + queries and `uninstall.php` — all use `$wpdb->prepare()` correctly; these + are one-shot admin-triggered/uninstall-time queries, not hot-path reads + that would benefit from `wp_cache_*` wrapping. +- `PrefixAllGlobals.NonPrefixedVariableFound` on `uninstall.php`'s local + variables — WP core guarantees `uninstall.php` runs in an isolated, + single-execution scope; local variable names there can't collide with + anything. + ### Account/ownership — check before submitting - A separate `wealcoder`-account plugin ("bricksfly") received a real wp.org @@ -501,7 +567,10 @@ will ever exist in the wild). ## Pre-submission checklist -- [ ] Run Plugin Check (PCP) locally against a clean build, fix everything it flags. +- [x] Run Plugin Check (PCP) locally — done (2026-08-06), see "Fifth pass" + above. Real findings fixed (trademark name, Tested-up-to granularity, + nonce unslash, stray file); dist-scope/NonceVerification/DirectQuery + findings investigated and confirmed false-positives for this scan mode. - [ ] Sync version number across readme.txt / plugin header / `MOTIONKIT_VERSION` / package.json. - [x] Rewrite readme.txt — done, verified current (2026-08-04); still needs the `== External services ==` section extended to cover the GSAP CDN diff --git a/includes/Auth/ConnectPage.php b/includes/Auth/ConnectPage.php index 86aab4e..ba9fa32 100644 --- a/includes/Auth/ConnectPage.php +++ b/includes/Auth/ConnectPage.php @@ -734,7 +734,8 @@ public function handle_tools_actions(): void if (!current_user_can('manage_options')) { return; } - if (!wp_verify_nonce($_POST['_wpnonce'] ?? '', 'motionkit_tools_nonce')) { + $nonce = isset($_POST['_wpnonce']) ? sanitize_text_field(wp_unslash($_POST['_wpnonce'])) : ''; + if (!wp_verify_nonce($nonce, 'motionkit_tools_nonce')) { wp_safe_redirect(admin_url('admin.php?page=motionkit-connect&tab=tools&error=nonce_failed')); exit; } diff --git a/motionkit.php b/motionkit.php index fd91bc0..277c304 100644 --- a/motionkit.php +++ b/motionkit.php @@ -1,7 +1,7 @@ { - try { return fs.existsSync(path.dirname(d)); } catch { return false; } - }); -} - -function copyToEditor({ quiet = false } = {}) { - const dest = resolveDest(); - if (!dest) { - if (!quiet) { - console.warn('[copy-to-editor] Editor server path not found. Set MOTIONKIT_EDITOR_PATH env var.'); - console.warn('[copy-to-editor] Tried:', DEST_CANDIDATES); - } - return { copied: 0, skipped: 0, dest: null }; - } - - let copied = 0; - let skipped = 0; - - for (const file of FILES) { - const src = path.join(SRC, file); - const out = path.join(dest, file); - - if (!fs.existsSync(src)) { - skipped++; - continue; - } - - const outDir = path.dirname(out); - if (!fs.existsSync(outDir)) { - fs.mkdirSync(outDir, { recursive: true }); - } - - fs.copyFileSync(src, out); - copied++; - } - - if (!quiet) { - console.log(`[copy-to-editor] Copied ${copied} files, skipped ${skipped} (not found in build).`); - console.log(`[copy-to-editor] Destination: ${dest}`); - } else if (copied > 0) { - console.log(`[copy-to-editor] Synced ${copied} file${copied === 1 ? '' : 's'} → editor.`); - } - - return { copied, skipped, dest }; -} - -module.exports = { copyToEditor }; - -if (require.main === module) { - copyToEditor({ quiet: false }); -} From d3f10abbf6da6ddd4237e7cb01452521b707db1d Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 18:15:33 +0600 Subject: [PATCH 06/23] Silence PCP false-positive on maybe_fix_autoload_flags()'s IN clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WordPress.DB.PreparedSQL.InterpolatedNotPrepared / PreparedSQLPlaceholders.UnfinishedPrepare flagged the dynamic IN ({$placeholders}) clause — the sniff can't trace that $placeholders is itself built from %s tokens sized to $hot_options (a hardcoded literal, never external input), which is the standard $wpdb->prepare() pattern for a variable-length IN (...) list. Added a targeted phpcs:ignore on the flagged line rather than another blanket phpcs.xml.dist exclusion, since PCP runs its own fixed ruleset and doesn't honor that file. Verified clean against the unmodified WordPress-Extra standard directly (not just this project's lenient ruleset), and PHPStan stays clean too. Documented in WPORG-SUBMISSION.md's "Fifth pass" section. --- WPORG-SUBMISSION.md | 13 +++++++++++++ includes/Plugin.php | 7 ++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/WPORG-SUBMISSION.md b/WPORG-SUBMISSION.md index 4b44e43..66f2191 100644 --- a/WPORG-SUBMISSION.md +++ b/WPORG-SUBMISSION.md @@ -487,6 +487,19 @@ scan mode, not real defects): queries and `uninstall.php` — all use `$wpdb->prepare()` correctly; these are one-shot admin-triggered/uninstall-time queries, not hot-path reads that would benefit from `wp_cache_*` wrapping. +- `Plugin.php:197` — `PreparedSQL.InterpolatedNotPrepared` / + `PreparedSQLPlaceholders.UnfinishedPrepare` on + `maybe_fix_autoload_flags()`'s dynamic `IN ({$placeholders})` clause. The + sniff can't trace that `$placeholders` is itself built from `%s` tokens + (`implode(',', array_fill(0, count($hot_options), '%s'))`) sized to match + the hardcoded `$hot_options` literal — the standard pattern for a + variable-length `IN (...)` with `$wpdb->prepare()`'s variadic args. Real + code, not user input; confirmed false-positive same as the equivalent + finding already excluded in `phpcs.xml.dist` for `ConnectPage.php`'s bulk + queries. Added a targeted `// phpcs:ignore` on the flagged line (rather + than another blanket `phpcs.xml.dist` exclusion) since PCP doesn't honor + that file — verified clean against the *unmodified* `WordPress-Extra` + standard directly, not just this project's lenient ruleset. - `PrefixAllGlobals.NonPrefixedVariableFound` on `uninstall.php`'s local variables — WP core guarantees `uninstall.php` runs in an isolated, single-execution scope; local variable names there can't collide with diff --git a/includes/Plugin.php b/includes/Plugin.php index 0344de8..6bd4496 100644 --- a/includes/Plugin.php +++ b/includes/Plugin.php @@ -189,12 +189,17 @@ public function maybe_fix_autoload_flags(): void global $wpdb; $hot_options = ['motionkit_page_settings_updated_at']; + // Dynamic %s,%s,... placeholder list sized to $hot_options — this IS + // the prepare() placeholder syntax, not unescaped SQL; $hot_options is + // a hardcoded literal above, never external input. The sniff can't + // trace that {$placeholders} expands to valid %s placeholders before + // prepare() consumes them via the variadic ...$hot_options args below. $placeholders = implode(',', array_fill(0, count($hot_options), '%s')); // Single UPDATE rather than per-option get/delete/add cycles. $wpdb->query( $wpdb->prepare( "UPDATE {$wpdb->options} SET autoload = 'yes' - WHERE option_name IN ({$placeholders}) AND autoload != 'yes'", + WHERE option_name IN ({$placeholders}) AND autoload != 'yes'", // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare ...$hot_options ) ); From bfcb6dd474d181959cca99fa31f837a6505e9262 Mon Sep 17 00:00:00 2001 From: Rayhan Uddin Date: Thu, 6 Aug 2026 18:25:54 +0600 Subject: [PATCH 07/23] Add real nonce to sidebar tab navigation; suppress redirect-flag warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves ConnectPage.php's WordPress.Security.NonceVerification.Recommended findings from Plugin Check with two different fixes depending on what the data actually is: - ?tab= (enqueue_admin_styles, render_page): now genuinely nonce-gated. wp_nonce_url() stamps the sidebar tab links with a 'motionkit_tab_nav' nonce; both methods verify it and fall back to the default 'connect' tab on a missing/invalid nonce rather than hard-failing (an expired bookmark shouldn't lock out navigation, and nothing here changes state either way — this is defense-in-depth, not a functional requirement). Verified live: valid nonce opens the requested tab, missing/garbage nonce falls back correctly in both cases. - render_notices()'s ?error=/?connected=/?disconnected=/?license_refresh=/ ?tools_deleted=/?verify= flags and get_error_message()'s ?error_message=: these are appended by server-side redirects *after* an action that already verified its own nonce (OAuth callback, handle_tools_actions()). They only pick which notice banner to show — a crafted URL at worst displays a fake success banner with no effect on actual state — so method-level phpcs:disable/enable blocks suppress these with the reasoning documented inline, rather than nonce-stamping every redirect target for no real security benefit. Verified against the unmodified WordPress-Extra standard directly (not just this project's lenient phpcs.xml.dist, since Plugin Check doesn't honor that file): 24 warnings on 12 lines -> 0. phpcs.xml.dist and PHPStan both stay clean too. --- includes/Auth/ConnectPage.php | 39 ++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/includes/Auth/ConnectPage.php b/includes/Auth/ConnectPage.php index ba9fa32..ea4ed7c 100644 --- a/includes/Auth/ConnectPage.php +++ b/includes/Auth/ConnectPage.php @@ -74,7 +74,14 @@ public function enqueue_admin_styles(string $hook): void $version ); - $active_tab = isset($_GET['tab']) ? sanitize_text_field(wp_unslash($_GET['tab'])) : 'connect'; + // Same nonce as the sidebar tab links (see render_page()) — mirrors its + // fallback-to-default behavior so the enqueued CSS always matches what + // render_page() actually draws. + $tab_nonce_valid = isset($_GET['_wpnonce']) + && wp_verify_nonce(sanitize_text_field(wp_unslash($_GET['_wpnonce'])), 'motionkit_tab_nav'); + $active_tab = ($tab_nonce_valid && isset($_GET['tab'])) + ? sanitize_text_field(wp_unslash($_GET['tab'])) + : 'connect'; if ($active_tab === 'tools') { wp_enqueue_style( 'motionkit-admin-tools', @@ -155,7 +162,14 @@ public function render_page(): void wp_die(esc_html__('You do not have permission to access this page.', 'motionkit')); } - $active_tab = isset($_GET['tab']) ? sanitize_text_field(wp_unslash($_GET['tab'])) : 'connect'; + // The sidebar nonces this URL (see the tab loop below); a missing/expired + // nonce (e.g. an old bookmark) just falls back to the default tab rather + // than hard-failing, since nothing here changes state either way. + $tab_nonce_valid = isset($_GET['_wpnonce']) + && wp_verify_nonce(sanitize_text_field(wp_unslash($_GET['_wpnonce'])), 'motionkit_tab_nav'); + $active_tab = ($tab_nonce_valid && isset($_GET['tab'])) + ? sanitize_text_field(wp_unslash($_GET['tab'])) + : 'connect'; $current_user = wp_get_current_user(); $connection_info = OAuthHandler::get_connection_info(); $user_email = !empty($connection_info['email']) ? $connection_info['email'] : $current_user->user_email; @@ -202,7 +216,13 @@ public function render_page(): void