Security fixes - #24
Open
plastikdreams wants to merge 29 commits into
Open
plastikdreams wants to merge 29 commits into
plastikdreams wants to merge 29 commits into
Conversation
…multisite support
- Added extended tests for Plugin, CacheStore, CachingHeaders, FlushCache, Migrator, AdminPage, AdminBar, NetworkAdminPage, NetworkManager, SiteConfig, and NginxCommand classes. - Added WP_Post, WP_Comment, WP_Admin_Bar, WP_CLI and WP_CLI_Command stubs to tests/bootstrap.php. - Added is_file and unlink to patchwork.json redefinable-internals. - Installed Xdebug 3.5.3 for code coverage reporting. - Overall line coverage: 87.28% (549/629 lines).
feat: PHP 8.5 full compatibility & WordPress multisite support (v4.0.0)
Merge pull request #1 from plastikdreams/master
… global constants in namespaced classes
The v4.0.0 bootstrap never instantiated FlushCache and CachingHeaders,
so cache tracking, auto-flush, the AJAX flush endpoints, and
X-Accel-Expires headers never ran. The bootstrap also registered
Plugin::add_hook() on plugins_loaded at priority 10, whose own nested
plugins_loaded callback (migrations, textdomain) could never fire.
Bootstrap now runs at priority 1 and wires all components; AdminBar is
registered outside the is_admin() gate so it renders on the frontend.
Security hardening:
- Sanitize cache keys in CacheFileResolver before building unlink()
paths (path traversal via the reverse-proxy-key filter)
- Move settings save from admin_head-{hook} to load-{hook} so
wp_safe_redirect() runs before output starts
- Add manage_options capability check to AdminPage settings save and
use the capability instead of the 'administrator' role for the menu
- Validate X-Forwarded-For with FILTER_VALIDATE_IP before trusting it
as the commenter IP
- Require is_string() on all nonce / redirect_to inputs (PHP 8
TypeError via array query params)
- Add ABSPATH guards to admin view files and esc_url() on the
flush-all link
fix: wire core components in bootstrap and harden security (v4.0.0 rewrite + audit fixes)
dosubot
Bot
added
the
size:XXL
📝 WalkthroughWalkthroughThe plugin is fully rewritten from a legacy singleton architecture (nginx-champuru.php, includes/*.php) to a PSR-4 namespaced codebase under src/, adding PHP 8.5-oriented enums/value objects, DB migrations, multisite support, admin/CLI interfaces, comprehensive PHPUnit tests, Composer/PHPUnit tooling, and updated documentation. ChangesNginx Cache Controller v4.0.0 Rewrite
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (14)
docs/superpowers/specs/2026-06-10-php85-multisite-design.md (1)
72-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PageTypeenum values in the spec don't match the implementation.The spec defines
PageTypewith valueshome,archive,singular,feed,other, but the actual implementation (per the plan) usesis_home,is_archive,is_singular,is_feed,other. The implementation values match WordPress conditional tag names and are correct for DB storage compatibility. Update the spec to reflect the actual enum backing values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-06-10-php85-multisite-design.md` around lines 72 - 78, The PageType enum in the spec is using the wrong backing values, so update the documented enum to match the implementation’s WordPress-style names. In the PageType definition, replace the current home/archive/singular/feed values with the actual is_home/is_archive/is_singular/is_feed values while keeping other unchanged, so the spec aligns with the implementation and DB storage format.src/Multisite/NetworkManager.php (1)
31-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
createTableForSiteignores the injected$this->plugin.Unlike
flushAllSiteswhich falls back toPlugin::get_instance()only when$this->pluginis null,createTableForSitealways uses the singleton. This prevents unit-testing with a mock plugin and is inconsistent with the constructor's DI design.♻️ Proposed fix
public function createTableForSite(int $blogId): void { switch_to_blog($blogId); - Plugin::get_instance()->activation(); + ($this->plugin ?? Plugin::get_instance())->activation(); restore_current_blog(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Multisite/NetworkManager.php` around lines 31 - 36, createTableForSite currently bypasses the injected plugin instance and always calls Plugin::get_instance(), which breaks dependency injection and testability. Update NetworkManager::createTableForSite to use the same plugin selection pattern as flushAllSites: prefer $this->plugin when available and only fall back to Plugin::get_instance() if it is null, then call activation() on that resolved instance before restoring the blog.src/Cli/NginxCommand.php (2)
29-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd error handling for flush operations and empty URL guard.
If
esc_url_rawreturns an empty string (e.g., invalid protocol),flush_this('')returns silently but the success message still prints, misleading the user. Wrap flush calls in try/catch to produce user-friendly WP-CLI errors instead of uncaught exceptions.♻️ Proposed fix
public function flush(array $args, array $assocArgs): void { $plugin = Plugin::get_instance(); if (isset($assocArgs['cache'])) { $url = esc_url_raw($assocArgs['cache']); + if (empty($url)) { + \WP_CLI::error('Invalid URL provided.'); + return; + } - $plugin->flush_this($url); - \WP_CLI::success("Flushed cache for: {$url}"); + try { + $plugin->flush_this($url); + \WP_CLI::success("Flushed cache for: {$url}"); + } catch (\Throwable $e) { + \WP_CLI::error("Flush failed: {$e->getMessage()}"); + } } else { - $plugin->flush_cache('all', 0); - \WP_CLI::success('Flushed all caches.'); + try { + $plugin->flush_cache('all', 0); + \WP_CLI::success('Flushed all caches.'); + } catch (\Throwable $e) { + \WP_CLI::error("Flush failed: {$e->getMessage()}"); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Cli/NginxCommand.php` around lines 29 - 41, The flush flow in NginxCommand::flush needs both an empty-URL guard and explicit error handling. When handling the cache argument, validate the result of esc_url_raw before calling Plugin::flush_this so an invalid or empty URL does not print a success message; instead raise a WP_CLI error. Also wrap both flush_this and flush_cache calls in try/catch so any exception is converted into a friendly WP-CLI failure message rather than allowing uncaught errors or false success output.
58-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate the
--formatparameter before passing toformat_items.An invalid format string causes
WP_CLI\Utils\format_itemsto throw anInvalidArgumentException. Validating against the supported set (table,csv,json,yaml,ids,count) provides a cleaner error message.♻️ Proposed fix
public function list(array $args, array $assocArgs): void { $plugin = Plugin::get_instance(); $objects = $plugin->get_cached_objects(); $format = $assocArgs['format'] ?? 'table'; + $valid = ['table', 'csv', 'json', 'yaml', 'ids', 'count']; + if (!in_array($format, $valid, true)) { + \WP_CLI::error(sprintf('Invalid format "%s". Valid formats: %s', $format, implode(', ', $valid))); + return; + } $items = array_map(static fn(object $o) => [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Cli/NginxCommand.php` around lines 58 - 72, The NginxCommand::list method passes the raw --format value straight into WP_CLI\Utils\format_items, which can throw on unsupported formats. Add explicit validation for the format argument in list before calling format_items, accepting only the supported values (table, csv, json, yaml, ids, count) and returning a clear error for anything else. Use the existing list method and its $format handling to locate the change, and keep the valid path unchanged after validation.src/Admin/AdminPage.php (1)
119-129: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueDon't use translation function
__()for a URL.
esc_url(__('http://wpbooster.net/', 'nginxchampuru'))makes the URL translatable. A translator could accidentally or maliciously change the destination. Sinceesc_url()sanitizes the output, the risk is mitigated, but the translation hook is unnecessary for a fixed URL.♻️ Proposed fix
$links[] = sprintf( '<a href="%s">%s</a>', - esc_url(__('http://wpbooster.net/', 'nginxchampuru')), + esc_url('http://wpbooster.net/'), esc_html(__('Make WordPress Site Load Faster', 'nginxchampuru')), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Admin/AdminPage.php` around lines 119 - 129, The link in AdminPage::plugin_row_meta is using __() around a fixed URL, which makes the destination translatable unnecessarily. Update the anchor generation so the wpbooster.net URL is treated as a literal string and only sanitized with esc_url(), while keeping the label text translated with __() or esc_html__() as appropriate.src/Core/Plugin.php (1)
226-229: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
getStore()instantiates a newCacheStoreon every call.Every call to
getStore()creates a newCacheStoreobject. SincePlugin::flush_cacheandPlugin::flush_thiseach callgetStore()(some multiple times), consider caching the instance in a property for reuse.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Core/Plugin.php` around lines 226 - 229, The getStore() method in Plugin currently creates a new CacheStore on every call, including from flush_cache and flush_this, so update Plugin to cache and reuse a single CacheStore instance in a property instead of instantiating it repeatedly. Add or reuse a private property on Plugin to hold the CacheStore, initialize it lazily in getStore(), and have flush_cache/flush_this continue calling getStore() so they share the same instance.src/Core/CacheFileResolver.php (1)
25-28: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueHandle empty key after sanitization.
If sanitization produces an empty string (e.g., the filter returned only special characters),
substr('', -$offset, $level)returns'', producing paths with empty directory segments like/cache///key. While not a security issue, it will cause failedunlink()calls. Consider falling back to a hash of the original key when the sanitized result is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Core/CacheFileResolver.php` around lines 25 - 28, The CacheFileResolver path-building logic can end up with empty directory segments when sanitization leaves an empty key, so update the key normalization in CacheFileResolver to detect that case before the foreach over $levels uses substr(). Fall back to a stable hash derived from the original key when the sanitized value is empty, and keep the existing offset/parts logic working on that fallback so generated cache paths are always valid. Use the CacheFileResolver methods and the $key sanitization flow as the place to apply the fix.tests/Unit/Migration/MigratorExtendedTest.php (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTemp file creation uses overly permissive 0777 and lacks cleanup.
The stub file at
/tmp/wordpress/wp-admin/includes/upgrade.phpis created with0777permissions and never removed after the test. This can cause issues in CI environments with parallel test runs and leaves artifacts behind. Consider using0700and adding atearDowncleanup, or using a stream wrapper / virtual filesystem instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Migration/MigratorExtendedTest.php` around lines 24 - 31, The temporary stub setup in MigratorExtendedTest is too permissive and leaves filesystem artifacts behind. Update the setup that creates the wp-admin/includes/upgrade.php stub to use tighter directory permissions instead of 0777, and add cleanup in the test lifecycle so the temporary directory/file is removed after execution. If available, prefer isolating this in tearDown or switching the test to a virtual filesystem/stream wrapper to avoid touching /tmp directly.tests/Unit/Core/PluginExtendedTest.php (1)
334-341: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame temp file hygiene concern as MigratorExtendedTest.
The stub file creation at lines 334-341 (and duplicated at 418-425) uses
0777permissions and no cleanup. Consider extracting a shared helper that creates the stub with0700and registers cleanup intearDown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Core/PluginExtendedTest.php` around lines 334 - 341, The stub upgrade.php setup in PluginExtendedTest has unsafe temp-file hygiene and is duplicated in two places; extract it into a shared helper used by the test class, create the stub directory with 0700 instead of 0777, and register cleanup in tearDown so the temporary file is removed after tests. Use the existing test setup around the upgradeDir/upgradeFile creation to locate and replace both occurrences consistently.src/Cache/FlushCache.php (1)
16-19: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider skipping autosaves and revisions in
save_postflush.
save_postfires for autosaves and revisions (which havepost_status = 'inherit'), triggering unnecessary cache flushes. Addingwp_is_post_autosave()/wp_is_post_revision()guards would reduce spurious flushes.Optional guard
public function flush_by_post(int $id): void { $post = get_post($id); - if (!($post instanceof \WP_Post) || !in_array($post->post_status, $this->getFlushStatuses(), true)) { + if (!($post instanceof \WP_Post) + || wp_is_post_autosave($post) + || wp_is_post_revision($post) + || !in_array($post->post_status, $this->getFlushStatuses(), true)) { return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Cache/FlushCache.php` around lines 16 - 19, The save_post hook in FlushCache::flush_by_post is flushing cache for autosaves and revisions as well, causing unnecessary invalidations. Update the flush_by_post callback to return early when wp_is_post_autosave() or wp_is_post_revision() indicates the post is an autosave or revision, while keeping the existing publish_future_post, comment_post, and wp_set_comment_status behavior unchanged.src/Http/CachingHeaders.php (1)
131-137: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
wp_cron_cachingsendsX-Cachedon every request, not just during cron.The method is hooked on
plugins_loadedand always emits theX-Cachedtimestamp header. TheX-Accel-Expiresheader is correctly gated behindDOING_CRON, but the unconditionalX-Cachedheader leaks server time on every response. Consider gating theX-Cachedheader behind the cron check or renaming the method to reflect its actual behavior.Optional fix
public function wp_cron_caching(): void { if (defined('DOING_CRON') && DOING_CRON) { header('X-Accel-Expires: ' . self::WP_CRON_EXP); + header('X-Cached: ' . gmdate('D, d M Y H:i:s', time()) . ' GMT'); } - header('X-Cached: ' . gmdate('D, d M Y H:i:s', time()) . ' GMT'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Http/CachingHeaders.php` around lines 131 - 137, The wp_cron_caching method currently emits the X-Cached timestamp header on every request, while only X-Accel-Expires is meant for cron. Update wp_cron_caching in CachingHeaders so X-Cached is only sent inside the DOING_CRON branch, or otherwise adjust the method behavior/name to match its actual scope; use the wp_cron_caching and X-Cached symbols to locate the change.src/Cache/CacheStore.php (1)
23-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd explicit column list to
REPLACE INTOstatement.The
VALUES(%s, %d, %s, %s, CURRENT_TIMESTAMP)relies on implicit column ordering. If a future migration adds a column, this query will break silently — the VALUES count won't match the column count.🛡️ Proposed fix
- $sql = $this->wpdb->prepare( - "REPLACE INTO `{$this->table}` VALUES(%s, %d, %s, %s, CURRENT_TIMESTAMP)", + $sql = $this->wpdb->prepare( + "REPLACE INTO `{$this->table}` (`cache_key`, `cache_id`, `cache_type`, `cache_url`, `cache_saved`) VALUES(%s, %d, %s, %s, CURRENT_TIMESTAMP)",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Cache/CacheStore.php` around lines 23 - 33, The REPLACE INTO query in CacheStore::add relies on implicit table column order, so update it to name the target columns explicitly before VALUES. Use the existing symbols CacheStore::add and $this->table to locate the statement, and make sure the listed columns match the four bound values plus CURRENT_TIMESTAMP so future schema changes do not break the insert.tests/Unit/Core/PageTypeTest.php (1)
26-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test cases for
ArchiveandSingularbranches.Tests cover
Home,Feed, andOther(fallback) but not theis_archive→PageType::Archiveoris_singular→PageType::Singularpaths infromWP(). Adding these would complete branch coverage.♻️ Suggested additional tests
public function testFromWpReturnsArchive(): void { Functions\when('is_home')->justReturn(false); Functions\when('is_archive')->justReturn(true); Functions\when('is_singular')->justReturn(false); Functions\when('is_feed')->justReturn(false); Functions\when('apply_filters')->returnArg(2); self::assertSame(PageType::Archive, PageType::fromWP()); } public function testFromWpReturnsSingular(): void { Functions\when('is_home')->justReturn(false); Functions\when('is_archive')->justReturn(false); Functions\when('is_singular')->justReturn(true); Functions\when('is_feed')->justReturn(false); Functions\when('apply_filters')->returnArg(2); self::assertSame(PageType::Singular, PageType::fromWP()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Core/PageTypeTest.php` around lines 26 - 57, Add missing coverage in PageTypeTest for the PageType::fromWP() branches that map is_archive() to PageType::Archive and is_singular() to PageType::Singular. Follow the existing test pattern in testFromWpReturnsHome(), testFromWpReturnsFeed(), and testFromWpReturnsOtherWhenNoConditionMatches(): stub the WordPress condition functions with Functions\when(), keep apply_filters() returning arg 2, and add two new assertions against PageType::fromWP() to verify those paths.src/Migration/Migrator.php (1)
77-90: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider guarding ALTER TABLE migrations against partial-failure re-runs.
If
migrate121()adds thecache_savedcolumn but fails on the index, a subsequent invocation won't retry (becausedb_versionis already set topluginVersion). Adding existence checks before ALTER would make migrations idempotent and safe to re-run.♻️ Optional: idempotent migration guards
private function migrate115(): void { + $exists = $this->wpdb->get_var("SHOW COLUMNS FROM `{$this->table}` LIKE 'cache_url'"); + if (!$exists) { $this->wpdb->query("ALTER TABLE `{$this->table}` ADD COLUMN `cache_url` varchar(256)"); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Migration/Migrator.php` around lines 77 - 90, The migrate121() migration in Migrator is not safe to re-run after a partial failure, since it blindly adds the cache_saved column and indexes without checking whether they already exist. Update migrate115() and migrate121() to guard each ALTER TABLE step with existence checks on the target column/index before issuing the query, so a rerun can complete any missing steps without failing on already-applied changes. Use the existing Migrator methods and $this->wpdb/table references to keep the migration idempotent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.phpunit.result.cache:
- Line 1: The repository is tracking the PHPUnit-generated .phpunit.result.cache
file, which causes noisy diffs and should not be versioned. Remove this file
from source control and add it to .gitignore so future test runs do not
reintroduce it; use the .phpunit.result.cache entry as the target to clean up
and ignore.
In `@src/Admin/AdminBar.php`:
- Around line 23-29: The early return in the admin bar setup blocks network
admins from reaching the later `$canNet` path, so `AdminBar` should be updated
to allow `manage_network` users through even when they lack `flush_cache_single`
and `flush_cache_all`. Adjust the guard in the `AdminBar` logic so it only
returns when none of the relevant capabilities are present, and ensure the
network-scoped "Flush All Sites" menu item is still added for multisite admins
via the existing `$canNet` check.
- Around line 70-78: The URL built in getFlushThisUrl() is double-encoding query
separators because esc_url() is applied before urlencode(), which turns
ampersands into amp; and breaks redirects with multiple parameters. Update
getFlushThisUrl() in AdminBar to match the other URL builders (getFlushAllUrl
and getFlushAllSitesUrl) by removing esc_url() before urlencode(), or use
esc_url_raw() if you still want sanitization before encoding.
In `@src/Cache/FlushCache.php`:
- Around line 76-80: In wp_ajax_flushthis, the redirect handling does not fall
back when redirect_to is missing, so wp_safe_redirect may receive an empty URL.
Update the redirect logic in FlushCache::wp_ajax_flushthis to mirror
wp_ajax_flushcache by defaulting to admin_url() when the sanitized redirect
value is empty. Keep the existing sanitization with esc_url_raw and use the
resulting non-empty URL for both transientExec and wp_safe_redirect.
In `@src/Core/CacheFileResolver.php`:
- Line 19: The cache key normalization in CacheFileResolver is causing a
mismatch between PHP and Nginx because stripping characters changes the cache
filename. Update the key handling in the resolver logic that uses the
nginxchampuru_get_reverse_proxy_key filter so it preserves a stable one-to-one
mapping, for example by hashing the returned key instead of sanitizing it with
preg_replace. Keep the path traversal protection, but ensure unlink() and Nginx
resolve the same cache file path for any key value.
In `@src/Core/PageType.php`:
- Around line 27-28: The PageType filter handling in the method using
apply_filters() and BackedEnum::tryFrom() needs a guard before conversion.
Validate that the filtered result is a scalar string or int before passing it to
tryFrom(), and fall back to the original $type when the filter returns anything
else; if you need coercion, only cast after confirming it is safe to do so. Use
the existing PageType enum and the apply_filters('nginxchampuru_get_post_type',
...) flow to locate and update the logic.
In `@src/Core/Plugin.php`:
- Around line 160-171: The transientExec method leaves the nginxchampuru_flush
lock behind if the callback throws, because delete_transient is only reached on
success. Update transientExec in Plugin to wrap the dynamic callback invocation
($this->$callback(...$params)) in a try/finally so the transient is always
cleared, even when flush_cache or another callback fails. Keep the existing
is_enable_flush and wp_die guard behavior intact, but ensure the cleanup runs
unconditionally.
In `@src/Migration/Migrator.php`:
- Around line 44-55: The Migrator::run flow is marking migrations complete even
when a migration fails. Update Migrator::run and/or runMigration so the
$wpdb->query() result is checked and any false return aborts processing
immediately, preventing later migrations from running. Only call
update_option('nginxchampuru-db_version', ...) and assign $this->dbVersion after
all pending migrations succeed; if one fails, skip the version update so it can
be retried on the next run.
In `@src/Multisite/NetworkManager.php`:
- Around line 13-28: The multisite loop in NetworkManager::flushAllSites can
leave the request on the wrong blog if Plugin::transientExec (or any future call
inside the loop) throws or exits early, because restore_current_blog is not
guaranteed to run. Wrap the switch_to_blog/restore_current_blog pair in a
try/finally so the original blog context is always restored, and keep the
per-site flush guarded inside that structure; also account for
Plugin::transientExec potentially terminating the loop so subsequent sites are
not processed with a stale context.
In `@tests/Unit/Admin/AdminPageTest.php`:
- Around line 49-60:
`testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined` is
order-dependent because `AdminPage::plugin_row_meta` reads
`NGINX_CACHE_CONTROLER_BASE_NAME` directly, so define that constant in the test
setup (or in this test before calling `plugin_row_meta`) instead of relying on
`testPluginRowMetaAddsLinkForThisPlugin` to do it. Keep the assertion that the
links are unchanged for the non-matching plugin, but ensure the constant exists
before invoking `AdminPage` so the test can run independently.
In `@tests/Unit/Cli/NginxCommandTest.php`:
- Around line 45-102: The NginxCommand tests currently end with placeholder
assertions, so regressions in flush() and list() can still pass unnoticed.
Update the test methods testFlushAllCallsFlushCacheAll,
testFlushSingleUrlCallsFlushThis, testListReturnsCachedObjects, and
testListWithJsonFormat to assert the real side effects or CLI output produced by
NginxCommand rather than calling assertTrue(true). Use the existing NginxCommand
methods and mocked WP functions to verify expected behavior directly.
In `@uninstall.php`:
- Around line 9-18: The uninstall routine only removes the database tables and
leaves plugin settings behind. Update the uninstall logic in uninstall.php to
also delete any plugin options stored in wp_options, using the plugin’s option
key in the same cleanup flow that drops the nginxchampuru table. If there are
network-wide settings in multisite, handle them alongside the existing
is_multisite() / get_sites() path by removing the corresponding site option as
well.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-06-10-php85-multisite-design.md`:
- Around line 72-78: The PageType enum in the spec is using the wrong backing
values, so update the documented enum to match the implementation’s
WordPress-style names. In the PageType definition, replace the current
home/archive/singular/feed values with the actual
is_home/is_archive/is_singular/is_feed values while keeping other unchanged, so
the spec aligns with the implementation and DB storage format.
In `@src/Admin/AdminPage.php`:
- Around line 119-129: The link in AdminPage::plugin_row_meta is using __()
around a fixed URL, which makes the destination translatable unnecessarily.
Update the anchor generation so the wpbooster.net URL is treated as a literal
string and only sanitized with esc_url(), while keeping the label text
translated with __() or esc_html__() as appropriate.
In `@src/Cache/CacheStore.php`:
- Around line 23-33: The REPLACE INTO query in CacheStore::add relies on
implicit table column order, so update it to name the target columns explicitly
before VALUES. Use the existing symbols CacheStore::add and $this->table to
locate the statement, and make sure the listed columns match the four bound
values plus CURRENT_TIMESTAMP so future schema changes do not break the insert.
In `@src/Cache/FlushCache.php`:
- Around line 16-19: The save_post hook in FlushCache::flush_by_post is flushing
cache for autosaves and revisions as well, causing unnecessary invalidations.
Update the flush_by_post callback to return early when wp_is_post_autosave() or
wp_is_post_revision() indicates the post is an autosave or revision, while
keeping the existing publish_future_post, comment_post, and
wp_set_comment_status behavior unchanged.
In `@src/Cli/NginxCommand.php`:
- Around line 29-41: The flush flow in NginxCommand::flush needs both an
empty-URL guard and explicit error handling. When handling the cache argument,
validate the result of esc_url_raw before calling Plugin::flush_this so an
invalid or empty URL does not print a success message; instead raise a WP_CLI
error. Also wrap both flush_this and flush_cache calls in try/catch so any
exception is converted into a friendly WP-CLI failure message rather than
allowing uncaught errors or false success output.
- Around line 58-72: The NginxCommand::list method passes the raw --format value
straight into WP_CLI\Utils\format_items, which can throw on unsupported formats.
Add explicit validation for the format argument in list before calling
format_items, accepting only the supported values (table, csv, json, yaml, ids,
count) and returning a clear error for anything else. Use the existing list
method and its $format handling to locate the change, and keep the valid path
unchanged after validation.
In `@src/Core/CacheFileResolver.php`:
- Around line 25-28: The CacheFileResolver path-building logic can end up with
empty directory segments when sanitization leaves an empty key, so update the
key normalization in CacheFileResolver to detect that case before the foreach
over $levels uses substr(). Fall back to a stable hash derived from the original
key when the sanitized value is empty, and keep the existing offset/parts logic
working on that fallback so generated cache paths are always valid. Use the
CacheFileResolver methods and the $key sanitization flow as the place to apply
the fix.
In `@src/Core/Plugin.php`:
- Around line 226-229: The getStore() method in Plugin currently creates a new
CacheStore on every call, including from flush_cache and flush_this, so update
Plugin to cache and reuse a single CacheStore instance in a property instead of
instantiating it repeatedly. Add or reuse a private property on Plugin to hold
the CacheStore, initialize it lazily in getStore(), and have
flush_cache/flush_this continue calling getStore() so they share the same
instance.
In `@src/Http/CachingHeaders.php`:
- Around line 131-137: The wp_cron_caching method currently emits the X-Cached
timestamp header on every request, while only X-Accel-Expires is meant for cron.
Update wp_cron_caching in CachingHeaders so X-Cached is only sent inside the
DOING_CRON branch, or otherwise adjust the method behavior/name to match its
actual scope; use the wp_cron_caching and X-Cached symbols to locate the change.
In `@src/Migration/Migrator.php`:
- Around line 77-90: The migrate121() migration in Migrator is not safe to
re-run after a partial failure, since it blindly adds the cache_saved column and
indexes without checking whether they already exist. Update migrate115() and
migrate121() to guard each ALTER TABLE step with existence checks on the target
column/index before issuing the query, so a rerun can complete any missing steps
without failing on already-applied changes. Use the existing Migrator methods
and $this->wpdb/table references to keep the migration idempotent.
In `@src/Multisite/NetworkManager.php`:
- Around line 31-36: createTableForSite currently bypasses the injected plugin
instance and always calls Plugin::get_instance(), which breaks dependency
injection and testability. Update NetworkManager::createTableForSite to use the
same plugin selection pattern as flushAllSites: prefer $this->plugin when
available and only fall back to Plugin::get_instance() if it is null, then call
activation() on that resolved instance before restoring the blog.
In `@tests/Unit/Core/PageTypeTest.php`:
- Around line 26-57: Add missing coverage in PageTypeTest for the
PageType::fromWP() branches that map is_archive() to PageType::Archive and
is_singular() to PageType::Singular. Follow the existing test pattern in
testFromWpReturnsHome(), testFromWpReturnsFeed(), and
testFromWpReturnsOtherWhenNoConditionMatches(): stub the WordPress condition
functions with Functions\when(), keep apply_filters() returning arg 2, and add
two new assertions against PageType::fromWP() to verify those paths.
In `@tests/Unit/Core/PluginExtendedTest.php`:
- Around line 334-341: The stub upgrade.php setup in PluginExtendedTest has
unsafe temp-file hygiene and is duplicated in two places; extract it into a
shared helper used by the test class, create the stub directory with 0700
instead of 0777, and register cleanup in tearDown so the temporary file is
removed after tests. Use the existing test setup around the
upgradeDir/upgradeFile creation to locate and replace both occurrences
consistently.
In `@tests/Unit/Migration/MigratorExtendedTest.php`:
- Around line 24-31: The temporary stub setup in MigratorExtendedTest is too
permissive and leaves filesystem artifacts behind. Update the setup that creates
the wp-admin/includes/upgrade.php stub to use tighter directory permissions
instead of 0777, and add cleanup in the test lifecycle so the temporary
directory/file is removed after execution. If available, prefer isolating this
in tearDown or switching the test to a virtual filesystem/stream wrapper to
avoid touching /tmp directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a84e8253-c8e0-4f1e-b791-b146a22d3eb4
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (60)
.phpunit.result.cachecomposer.jsondocs/CHANGES.mddocs/README.mddocs/REFERENCE.mddocs/SPECIFICATIONS.mddocs/USAGE.mddocs/superpowers/plans/2026-06-10-php85-multisite.mddocs/superpowers/specs/2026-06-10-php85-multisite-design.mdincludes/admin.class.phpincludes/caching.class.phpincludes/class-addrewriterules.phpincludes/flush-cache.class.phpincludes/wp-cli.phpnginx-cache-controller.phpnginx-champuru.phppatchwork.jsonphpunit.xmlreadme.txtsrc/Admin/AdminBar.phpsrc/Admin/AdminPage.phpsrc/Admin/NetworkAdminPage.phpsrc/Admin/views/admin-panel.phpsrc/Admin/views/network-admin-panel.phpsrc/Cache/CacheStore.phpsrc/Cache/FlushCache.phpsrc/Cli/NginxCommand.phpsrc/Core/CacheConfig.phpsrc/Core/CacheFileResolver.phpsrc/Core/CacheKey.phpsrc/Core/FlushMode.phpsrc/Core/PageType.phpsrc/Core/Plugin.phpsrc/Http/CachingHeaders.phpsrc/Migration/Migrator.phpsrc/Multisite/NetworkManager.phpsrc/Multisite/SiteConfig.phptests/Unit/Admin/AdminBarTest.phptests/Unit/Admin/AdminPageTest.phptests/Unit/Admin/NetworkAdminPageTest.phptests/Unit/Cache/CacheStoreExtendedTest.phptests/Unit/Cache/CacheStoreTest.phptests/Unit/Cache/FlushCacheExtendedTest.phptests/Unit/Cache/FlushCacheTest.phptests/Unit/Cli/NginxCommandTest.phptests/Unit/Core/CacheConfigTest.phptests/Unit/Core/CacheFileResolverTest.phptests/Unit/Core/CacheKeyTest.phptests/Unit/Core/FlushModeTest.phptests/Unit/Core/PageTypeTest.phptests/Unit/Core/PluginExtendedTest.phptests/Unit/Core/PluginTest.phptests/Unit/Http/CachingHeadersExtendedTest.phptests/Unit/Http/CachingHeadersTest.phptests/Unit/Migration/MigratorExtendedTest.phptests/Unit/Migration/MigratorTest.phptests/Unit/Multisite/NetworkManagerTest.phptests/Unit/Multisite/SiteConfigTest.phptests/bootstrap.phpuninstall.php
💤 Files with no reviewable changes (6)
- includes/caching.class.php
- includes/flush-cache.class.php
- includes/class-addrewriterules.php
- includes/wp-cli.php
- includes/admin.class.php
- nginx-champuru.php
| @@ -0,0 +1 @@ | |||
| {"version":2,"defects":{"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testFromValidOption":8,"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testFromInvalidOptionReturnsNone":8,"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testValueReturnsBackingString":8,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsHome":8,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsFeed":8,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsOtherWhenNoConditionMatches":8,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testDbValueReturnsExpectedString":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testDefaultValues":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testWithExpiresReturnsNewInstance":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testMaxExpire":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testMaxExpireWithNoExpiresReturnsDefault":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheKeyTest::testGenerateReturnsMd5OfUrl":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheKeyTest::testGenerateAppliesFilter":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveWithLevels1Colon2":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveWithSingleLevel":8,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveMultipleKeys":8,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsWhenFreshInstall":8,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsWhenAlreadyUpToDate":8,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsPartialUpgrade":8,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testCreateTableSqlContainsExpectedColumns":8,"NginxCacheController\\Tests\\Unit\\Cache\\CacheStoreTest::testAddCallsQuery":8,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveReturnsCacheConfig":8,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveUsesNetworkCacheDir":8,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveSiteFlushEnabledOverridesDefault":8,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNocacheHeadersAddsXAccelExpires":8,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNonceLifeIsExtendedWhenCacheIsLonger":8,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNonceLifeIsNotReducedBelowOriginal":8,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testFlushByPostSkipsNonPublishedPost":8,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testGetFlushStatusesContainsPublishAndInherit":8,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testFlushCachesDoesNothingWhenModeIsNone":8,"NginxCacheController\\Tests\\Unit\\Multisite\\NetworkManagerTest::testFlushAllSitesDoesNothingOnSingleSite":8,"NginxCacheController\\Tests\\Unit\\Multisite\\NetworkManagerTest::testFlushAllSitesIteratesEachBlog":8},"times":{"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testFromValidOption":0,"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testFromInvalidOptionReturnsNone":0,"NginxCacheController\\Tests\\Unit\\Core\\FlushModeTest::testValueReturnsBackingString":0,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsHome":0.001,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsFeed":0,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testFromWpReturnsOtherWhenNoConditionMatches":0,"NginxCacheController\\Tests\\Unit\\Core\\PageTypeTest::testDbValueReturnsExpectedString":0,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testDefaultValues":0.001,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testWithExpiresReturnsNewInstance":0.003,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testMaxExpire":0,"NginxCacheController\\Tests\\Unit\\Core\\CacheConfigTest::testMaxExpireWithNoExpiresReturnsDefault":0,"NginxCacheController\\Tests\\Unit\\Core\\CacheKeyTest::testGenerateReturnsMd5OfUrl":0.001,"NginxCacheController\\Tests\\Unit\\Core\\CacheKeyTest::testGenerateAppliesFilter":0,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveWithLevels1Colon2":0.001,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveWithSingleLevel":0,"NginxCacheController\\Tests\\Unit\\Core\\CacheFileResolverTest::testResolveMultipleKeys":0.002,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsWhenFreshInstall":0.001,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsWhenAlreadyUpToDate":0.001,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testGetPendingMigrationsPartialUpgrade":0,"NginxCacheController\\Tests\\Unit\\Migration\\MigratorTest::testCreateTableSqlContainsExpectedColumns":0,"NginxCacheController\\Tests\\Unit\\Cache\\CacheStoreTest::testTableNameUsesWpdbPrefix":0.062,"NginxCacheController\\Tests\\Unit\\Cache\\CacheStoreTest::testTableNameWithMultisitePrefix":0,"NginxCacheController\\Tests\\Unit\\Cache\\CacheStoreTest::testAddCallsQuery":0.005,"NginxCacheController\\Tests\\Unit\\Cache\\CacheStoreTest::testGetExpireLimitReturnsFormattedDate":0.001,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveReturnsCacheConfig":0.002,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveUsesNetworkCacheDir":0,"NginxCacheController\\Tests\\Unit\\Multisite\\SiteConfigTest::testResolveSiteFlushEnabledOverridesDefault":0,"NginxCacheController\\Tests\\Unit\\Core\\PluginTest::testGetInstanceReturnsSameObject":0,"NginxCacheController\\Tests\\Unit\\Core\\PluginTest::testGetExpireReturnsConfiguredValue":0,"NginxCacheController\\Tests\\Unit\\Core\\PluginTest::testGetCacheKeyReturnsMd5":0,"NginxCacheController\\Tests\\Unit\\Core\\PluginTest::testGetFlushMethodReturnsString":0,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNocacheHeadersAddsXAccelExpires":0.003,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNonceLifeIsExtendedWhenCacheIsLonger":0,"NginxCacheController\\Tests\\Unit\\Http\\CachingHeadersTest::testNonceLifeIsNotReducedBelowOriginal":0,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testFlushByPostSkipsNonPublishedPost":0.004,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testGetFlushStatusesContainsPublishAndInherit":0.001,"NginxCacheController\\Tests\\Unit\\Cache\\FlushCacheTest::testFlushCachesDoesNothingWhenModeIsNone":0.001,"NginxCacheController\\Tests\\Unit\\Multisite\\NetworkManagerTest::testFlushAllSitesDoesNothingOnSingleSite":0.001,"NginxCacheController\\Tests\\Unit\\Multisite\\NetworkManagerTest::testFlushAllSitesIteratesEachBlog":0.001}} No newline at end of file | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove .phpunit.result.cache from version control.
This is a PHPUnit-generated cache file that changes on every test run. It should be added to .gitignore and removed from the repository to avoid unnecessary diffs and merge conflicts.
#!/bin/bash
# Check if .gitignore exists and whether this file is already ignored
if [ -f .gitignore ]; then
echo "--- .gitignore contents ---"
cat .gitignore
echo ""
echo "--- Is .phpunit.result.cache ignored? ---"
git check-ignore .phpunit.result.cache && echo "YES - already ignored" || echo "NO - not ignored"
else
echo ".gitignore does not exist"
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.phpunit.result.cache at line 1, The repository is tracking the
PHPUnit-generated .phpunit.result.cache file, which causes noisy diffs and
should not be versioned. Remove this file from source control and add it to
.gitignore so future test runs do not reintroduce it; use the
.phpunit.result.cache entry as the target to clean up and ignore.
Comment on lines
+23
to
+29
| $canSingle = current_user_can('flush_cache_single'); | ||
| $canAll = current_user_can('flush_cache_all'); | ||
| $canNet = is_multisite() && current_user_can('manage_network'); | ||
|
|
||
| if (!$canSingle && !$canAll) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Early return prevents network admins from seeing "Flush All Sites" without site-level flush capabilities.
The guard if (!$canSingle && !$canAll) { return; } exits before the $canNet check at line 51. A network admin who has manage_network but lacks flush_cache_single or flush_cache_all on the current site will never see the "Flush All Sites" menu item — even though that operation is network-scoped and doesn't require site-level capabilities.
🐛 Proposed fix
- if (!$canSingle && !$canAll) {
+ if (!$canSingle && !$canAll && !$canNet) {
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $canSingle = current_user_can('flush_cache_single'); | |
| $canAll = current_user_can('flush_cache_all'); | |
| $canNet = is_multisite() && current_user_can('manage_network'); | |
| if (!$canSingle && !$canAll) { | |
| return; | |
| } | |
| $canSingle = current_user_can('flush_cache_single'); | |
| $canAll = current_user_can('flush_cache_all'); | |
| $canNet = is_multisite() && current_user_can('manage_network'); | |
| if (!$canSingle && !$canAll && !$canNet) { | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Admin/AdminBar.php` around lines 23 - 29, The early return in the admin
bar setup blocks network admins from reaching the later `$canNet` path, so
`AdminBar` should be updated to allow `manage_network` users through even when
they lack `flush_cache_single` and `flush_cache_all`. Adjust the guard in the
`AdminBar` logic so it only returns when none of the relevant capabilities are
present, and ensure the network-scoped "Flush All Sites" menu item is still
added for multisite admins via the existing `$canNet` check.
Comment on lines
+70
to
+78
| private function getFlushThisUrl(): string | ||
| { | ||
| $plugin = Plugin::get_instance(); | ||
| return admin_url(sprintf( | ||
| '/admin-ajax.php?action=flushthis&redirect_to=%s&nonce=%s', | ||
| urlencode(esc_url($plugin->get_the_url())), | ||
| wp_create_nonce('flushthis'), | ||
| )); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove esc_url() before urlencode() — it double-encodes query parameter separators.
esc_url() converts & to & for HTML display. When the result is then passed through urlencode(), the & entity is encoded as %26amp%3B. After PHP decodes the redirect_to parameter, the URL contains & instead of &, breaking any redirect URL with multiple query parameters (e.g., ?foo=1&bar=2 becomes ?foo=1&bar=2).
The other two URL builders in this class (getFlushAllUrl and getFlushAllSitesUrl) correctly use urlencode() without esc_url(). Use esc_url_raw() if sanitization is desired, or simply remove esc_url().
🐛 Proposed fix
private function getFlushThisUrl(): string
{
$plugin = Plugin::get_instance();
return admin_url(sprintf(
'/admin-ajax.php?action=flushthis&redirect_to=%s&nonce=%s',
- urlencode(esc_url($plugin->get_the_url())),
+ urlencode(esc_url_raw($plugin->get_the_url())),
wp_create_nonce('flushthis'),
));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private function getFlushThisUrl(): string | |
| { | |
| $plugin = Plugin::get_instance(); | |
| return admin_url(sprintf( | |
| '/admin-ajax.php?action=flushthis&redirect_to=%s&nonce=%s', | |
| urlencode(esc_url($plugin->get_the_url())), | |
| wp_create_nonce('flushthis'), | |
| )); | |
| } | |
| private function getFlushThisUrl(): string | |
| { | |
| $plugin = Plugin::get_instance(); | |
| return admin_url(sprintf( | |
| '/admin-ajax.php?action=flushthis&redirect_to=%s&nonce=%s', | |
| urlencode(esc_url_raw($plugin->get_the_url())), | |
| wp_create_nonce('flushthis'), | |
| )); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Admin/AdminBar.php` around lines 70 - 78, The URL built in
getFlushThisUrl() is double-encoding query separators because esc_url() is
applied before urlencode(), which turns ampersands into amp; and breaks
redirects with multiple parameters. Update getFlushThisUrl() in AdminBar to
match the other URL builders (getFlushAllUrl and getFlushAllSitesUrl) by
removing esc_url() before urlencode(), or use esc_url_raw() if you still want
sanitization before encoding.
Comment on lines
+76
to
+80
| $redirect = $_GET['redirect_to'] ?? ''; | ||
| $url = esc_url_raw(is_string($redirect) ? $redirect : ''); | ||
| Plugin::get_instance()->transientExec('flush_this', $url); | ||
| wp_safe_redirect($url); | ||
| exit; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add redirect fallback for empty redirect_to in wp_ajax_flushthis.
Unlike wp_ajax_flushcache (line 94) which falls back to admin_url() when redirect_to is empty, this method passes an empty string to wp_safe_redirect(), which can produce an empty Location header and unpredictable browser behavior.
Proposed fix
- $redirect = $_GET['redirect_to'] ?? '';
- $url = esc_url_raw(is_string($redirect) ? $redirect : '');
+ $redirect = $_GET['redirect_to'] ?? '';
+ $url = esc_url_raw(is_string($redirect) && $redirect !== '' ? $redirect : admin_url());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $redirect = $_GET['redirect_to'] ?? ''; | |
| $url = esc_url_raw(is_string($redirect) ? $redirect : ''); | |
| Plugin::get_instance()->transientExec('flush_this', $url); | |
| wp_safe_redirect($url); | |
| exit; | |
| $redirect = $_GET['redirect_to'] ?? ''; | |
| $url = esc_url_raw(is_string($redirect) && $redirect !== '' ? $redirect : admin_url()); | |
| Plugin::get_instance()->transientExec('flush_this', $url); | |
| wp_safe_redirect($url); | |
| exit; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Cache/FlushCache.php` around lines 76 - 80, In wp_ajax_flushthis, the
redirect handling does not fall back when redirect_to is missing, so
wp_safe_redirect may receive an empty URL. Update the redirect logic in
FlushCache::wp_ajax_flushthis to mirror wp_ajax_flushcache by defaulting to
admin_url() when the sanitized redirect value is empty. Keep the existing
sanitization with esc_url_raw and use the resulting non-empty URL for both
transientExec and wp_safe_redirect.
| // Keys are normally md5 hex, but the 'nginxchampuru_get_reverse_proxy_key' | ||
| // filter can return arbitrary strings, so strip anything that could | ||
| // introduce path traversal ('/', '\', '..', null bytes). | ||
| $key = (string) preg_replace('/[^A-Za-z0-9_\-]/', '', $key); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sanitizing the key may cause cache path mismatch with Nginx.
The preg_replace strips non-alphanumeric characters from the key. While this correctly prevents path traversal, if the nginxchampuru_get_reverse_proxy_key filter returns a key containing characters like . or /, Nginx will have computed the cache file path using the original unsanitized key. The sanitized key will produce a different path, causing unlink() to miss the actual cache file. Consider hashing the key (e.g., md5) instead of stripping characters, which preserves the one-to-one mapping while still preventing traversal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Core/CacheFileResolver.php` at line 19, The cache key normalization in
CacheFileResolver is causing a mismatch between PHP and Nginx because stripping
characters changes the cache filename. Update the key handling in the resolver
logic that uses the nginxchampuru_get_reverse_proxy_key filter so it preserves a
stable one-to-one mapping, for example by hashing the returned key instead of
sanitizing it with preg_replace. Keep the path traversal protection, but ensure
unlink() and Nginx resolve the same cache file path for any key value.
Comment on lines
+44
to
+55
| public function run(): void | ||
| { | ||
| if ($this->tableExists()) { | ||
| foreach ($this->getPendingMigrations() as $version) { | ||
| $this->runMigration($version); | ||
| } | ||
| } else { | ||
| $this->createTable(); | ||
| } | ||
| update_option('nginxchampuru-db_version', $this->pluginVersion); | ||
| $this->dbVersion = $this->pluginVersion; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Failed migrations are silently ignored and marked as complete.
$wpdb->query() returns false on failure but the return value is never checked. After the migration loop, update_option('nginxchampuru-db_version', $this->pluginVersion) unconditionally marks all migrations as applied. If a migration fails midway, the schema is left inconsistent and the failed migration is never retried on subsequent runs.
🛡️ Proposed fix: abort on error and skip db_version update
public function run(): void
{
if ($this->tableExists()) {
foreach ($this->getPendingMigrations() as $version) {
$this->runMigration($version);
+ if ($this->wpdb->last_error) {
+ return;
+ }
}
} else {
$this->createTable();
+ if ($this->wpdb->last_error) {
+ return;
+ }
}
update_option('nginxchampuru-db_version', $this->pluginVersion);
$this->dbVersion = $this->pluginVersion;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function run(): void | |
| { | |
| if ($this->tableExists()) { | |
| foreach ($this->getPendingMigrations() as $version) { | |
| $this->runMigration($version); | |
| } | |
| } else { | |
| $this->createTable(); | |
| } | |
| update_option('nginxchampuru-db_version', $this->pluginVersion); | |
| $this->dbVersion = $this->pluginVersion; | |
| } | |
| public function run(): void | |
| { | |
| if ($this->tableExists()) { | |
| foreach ($this->getPendingMigrations() as $version) { | |
| $this->runMigration($version); | |
| if ($this->wpdb->last_error) { | |
| return; | |
| } | |
| } | |
| } else { | |
| $this->createTable(); | |
| if ($this->wpdb->last_error) { | |
| return; | |
| } | |
| } | |
| update_option('nginxchampuru-db_version', $this->pluginVersion); | |
| $this->dbVersion = $this->pluginVersion; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Migration/Migrator.php` around lines 44 - 55, The Migrator::run flow is
marking migrations complete even when a migration fails. Update Migrator::run
and/or runMigration so the $wpdb->query() result is checked and any false return
aborts processing immediately, preventing later migrations from running. Only
call update_option('nginxchampuru-db_version', ...) and assign $this->dbVersion
after all pending migrations succeed; if one fails, skip the version update so
it can be retried on the next run.
Comment on lines
+13
to
+28
| public function flushAllSites(): void | ||
| { | ||
| if (!is_multisite()) { | ||
| return; | ||
| } | ||
|
|
||
| $plugin = $this->plugin ?? Plugin::get_instance(); | ||
| $sites = get_sites(['number' => 0]); | ||
|
|
||
| foreach ($sites as $site) { | ||
| switch_to_blog((int) $site->blog_id); | ||
| if ($plugin->is_enable_flush()) { | ||
| $plugin->transientExec('flush_cache', 'all', 0); | ||
| } | ||
| restore_current_blog(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap switch_to_blog/restore_current_blog in try/finally to prevent stale blog context.
If transientExec or activation() throws an exception, restore_current_blog() is never called, leaving the request on the wrong blog for all subsequent operations — a data-integrity risk in multisite. Additionally, transientExec (per the plan's Plugin::transientExec) calls wp_die() when a flush transient is already set, which would halt the entire flushAllSites loop mid-iteration, skipping all remaining sites without restoring their blog context.
🔒 Proposed fix
public function flushAllSites(): void
{
if (!is_multisite()) {
return;
}
$plugin = $this->plugin ?? Plugin::get_instance();
$sites = get_sites(['number' => 0]);
foreach ($sites as $site) {
switch_to_blog((int) $site->blog_id);
- if ($plugin->is_enable_flush()) {
- $plugin->transientExec('flush_cache', 'all', 0);
+ try {
+ if ($plugin->is_enable_flush()) {
+ $plugin->transientExec('flush_cache', 'all', 0);
+ }
+ } finally {
+ restore_current_blog();
}
- restore_current_blog();
}
}
public function createTableForSite(int $blogId): void
{
switch_to_blog($blogId);
- Plugin::get_instance()->activation();
- restore_current_blog();
+ try {
+ ($this->plugin ?? Plugin::get_instance())->activation();
+ } finally {
+ restore_current_blog();
+ }
}Also applies to: 31-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Multisite/NetworkManager.php` around lines 13 - 28, The multisite loop in
NetworkManager::flushAllSites can leave the request on the wrong blog if
Plugin::transientExec (or any future call inside the loop) throws or exits
early, because restore_current_blog is not guaranteed to run. Wrap the
switch_to_blog/restore_current_blog pair in a try/finally so the original blog
context is always restored, and keep the per-site flush guarded inside that
structure; also account for Plugin::transientExec potentially terminating the
loop so subsequent sites are not processed with a stale context.
Comment on lines
+49
to
+60
| public function testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined(): void | ||
| { | ||
| Functions\when('__')->returnArg(1); | ||
| Functions\when('esc_url')->returnArg(1); | ||
| Functions\when('esc_html')->returnArg(1); | ||
|
|
||
| $page = new AdminPage(); | ||
| $links = ['settings', 'docs']; | ||
| $result = $page->plugin_row_meta($links, 'other-plugin/other.php'); | ||
|
|
||
| self::assertSame($links, $result); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined has a test-order dependency.
The test calls plugin_row_meta which references the NGINX_CACHE_CONTROLER_BASE_NAME constant directly. If this test runs before testPluginRowMetaAddsLinkForThisPlugin (which defines the constant), it will throw Error: Undefined constant in PHP 8.1+. The test bootstrap doesn't define this constant either.
💚 Proposed fix
public function testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined(): void
{
+ if (!defined('NGINX_CACHE_CONTROLER_BASE_NAME')) {
+ define('NGINX_CACHE_CONTROLER_BASE_NAME', 'nginx-champuru/nginx-champuru.php');
+ }
Functions\when('__')->returnArg(1);
Functions\when('esc_url')->returnArg(1);
Functions\when('esc_html')->returnArg(1);
$page = new AdminPage();
$links = ['settings', 'docs'];
$result = $page->plugin_row_meta($links, 'other-plugin/other.php');
self::assertSame($links, $result);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined(): void | |
| { | |
| Functions\when('__')->returnArg(1); | |
| Functions\when('esc_url')->returnArg(1); | |
| Functions\when('esc_html')->returnArg(1); | |
| $page = new AdminPage(); | |
| $links = ['settings', 'docs']; | |
| $result = $page->plugin_row_meta($links, 'other-plugin/other.php'); | |
| self::assertSame($links, $result); | |
| } | |
| public function testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined(): void | |
| { | |
| if (!defined('NGINX_CACHE_CONTROLER_BASE_NAME')) { | |
| define('NGINX_CACHE_CONTROLER_BASE_NAME', 'nginx-champuru/nginx-champuru.php'); | |
| } | |
| Functions\when('__')->returnArg(1); | |
| Functions\when('esc_url')->returnArg(1); | |
| Functions\when('esc_html')->returnArg(1); | |
| $page = new AdminPage(); | |
| $links = ['settings', 'docs']; | |
| $result = $page->plugin_row_meta($links, 'other-plugin/other.php'); | |
| self::assertSame($links, $result); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Unit/Admin/AdminPageTest.php` around lines 49 - 60,
`testPluginRowMetaReturnsLinksUnchangedWhenConstantNotDefined` is
order-dependent because `AdminPage::plugin_row_meta` reads
`NGINX_CACHE_CONTROLER_BASE_NAME` directly, so define that constant in the test
setup (or in this test before calling `plugin_row_meta`) instead of relying on
`testPluginRowMetaAddsLinkForThisPlugin` to do it. Keep the assertion that the
links are unchanged for the non-matching plugin, but ensure the constant exists
before invoking `AdminPage` so the test can run independently.
Comment on lines
+45
to
+102
| public function testFlushAllCallsFlushCacheAll(): void | ||
| { | ||
| $this->makePlugin(); | ||
|
|
||
| Functions\when('do_action')->justReturn(null); | ||
| Functions\when('has_filter')->justReturn(false); | ||
| Functions\when('apply_filters')->returnArg(2); | ||
| Functions\when('get_transient')->justReturn(false); | ||
| Functions\when('set_transient')->justReturn(true); | ||
| Functions\when('delete_transient')->justReturn(true); | ||
|
|
||
| $cmd = new NginxCommand(); | ||
| $cmd->flush([], []); | ||
| self::assertTrue(true); | ||
| } | ||
|
|
||
| public function testFlushSingleUrlCallsFlushThis(): void | ||
| { | ||
| $this->makePlugin(); | ||
|
|
||
| Functions\when('esc_url_raw')->returnArg(1); | ||
| Functions\when('do_action')->justReturn(null); | ||
| Functions\when('url_to_postid')->justReturn(0); | ||
| Functions\when('has_filter')->justReturn(false); | ||
| Functions\when('apply_filters')->returnArg(2); | ||
| Functions\when('get_transient')->justReturn(false); | ||
| Functions\when('set_transient')->justReturn(true); | ||
| Functions\when('delete_transient')->justReturn(true); | ||
|
|
||
| $cmd = new NginxCommand(); | ||
| $cmd->flush([], ['cache' => 'https://example.com/post/']); | ||
| self::assertTrue(true); | ||
| } | ||
|
|
||
| public function testListReturnsCachedObjects(): void | ||
| { | ||
| $this->makePlugin(); | ||
|
|
||
| if (!function_exists('WP_CLI\Utils\format_items')) { | ||
| // Safe: eval is used solely to declare a stub function in a | ||
| // namespaced scope (WP_CLI\Utils) that PHP cannot define any | ||
| // other way at runtime. No external input is involved. | ||
| eval('namespace WP_CLI\Utils; function format_items($format, $items, $cols): void {}'); // phpcs:ignore Squiz.PHP.Eval.Discouraged | ||
| } | ||
|
|
||
| $cmd = new NginxCommand(); | ||
| $cmd->list([], []); | ||
| self::assertTrue(true); | ||
| } | ||
|
|
||
| public function testListWithJsonFormat(): void | ||
| { | ||
| $this->makePlugin(); | ||
|
|
||
| $cmd = new NginxCommand(); | ||
| $cmd->list([], ['format' => 'json']); | ||
| self::assertTrue(true); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
These tests need real assertions.
Every case ends with assertTrue(true), so regressions in flush()/list() will still pass unnoticed. Please assert the expected side effects or CLI output instead of only checking that the method returned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/Unit/Cli/NginxCommandTest.php` around lines 45 - 102, The NginxCommand
tests currently end with placeholder assertions, so regressions in flush() and
list() can still pass unnoticed. Update the test methods
testFlushAllCallsFlushCacheAll, testFlushSingleUrlCallsFlushThis,
testListReturnsCachedObjects, and testListWithJsonFormat to assert the real side
effects or CLI output produced by NginxCommand rather than calling
assertTrue(true). Use the existing NginxCommand methods and mocked WP functions
to verify expected behavior directly.
Comment on lines
+9
to
+18
| if ( is_multisite() ) { | ||
| $site_ids = get_sites( array( 'fields' => 'ids', 'number' => 0 ) ); | ||
| foreach ( $site_ids as $site_id ) { | ||
| $table = $wpdb->get_blog_prefix( $site_id ) . 'nginxchampuru'; | ||
| $wpdb->query( "DROP TABLE IF EXISTS `{$table}`" ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared | ||
| } | ||
| } else { | ||
| $table = $wpdb->prefix . 'nginxchampuru'; | ||
| $wpdb->query( "DROP TABLE IF EXISTS `{$table}`" ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Consider cleaning up plugin options in addition to dropping tables.
The uninstall script drops the nginxchampuru table for each site but doesn't delete any options the plugin may have stored in wp_options (e.g., cache directory, flush mode, expires settings). Stale options would remain after uninstall.
If the plugin stores options, add cleanup like delete_option('nginxchampuru') (and delete_site_option for network-level settings in multisite).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@uninstall.php` around lines 9 - 18, The uninstall routine only removes the
database tables and leaves plugin settings behind. Update the uninstall logic in
uninstall.php to also delete any plugin options stored in wp_options, using the
plugin’s option key in the same cleanup flow that drops the nginxchampuru table.
If there are network-wide settings in multisite, handle them alongside the
existing is_multisite() / get_sites() path by removing the corresponding site
option as well.
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
1 participant
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.
コミット
de9c433「fix: wire core components in bootstrap and harden security」の内容を日本語で解説します。変更は 8 ファイル、+56 / −20 行です。1. 最重要バグ修正: プラグインの中核機能が一度も動いていなかった
nginx-cache-controller.php（ブートストラップ）
v4.0.0 のリライト時に、ブートストラップが
FlushCacheとCachingHeadersを一度もインスタンス化していませんでした。その結果、以下がすべて動作していませんでした。X-Accel-Expiresなどのキャッシュ制御ヘッダー送出さらに深い問題として、ブートストラップは
plugins_loadedの優先度 10 で実行されていましたが、その中で呼ばれるPlugin::add_hook()も同じplugins_loaded優先度 10 にコールバックを追加します。WordPress は「現在実行中の優先度」に追加されたコールバックを同一リクエスト内で実行しないため、DB マイグレーションと翻訳ファイルの読み込みも一度も実行されていませんでした。修正内容:
FlushCache・CachingHeadersを配線したAdminBarをis_admin()の外に移動した（管理バーはログインユーザーのフロントエンド表示でも描画されるため）2. セキュリティ強化
パストラバーサル対策 — CacheFileResolver.php
キャッシュキーはファイルパスに組み立てられ、後で
unlink()（ファイル削除）されます。通常は md5 の16進文字列ですが、nginxchampuru_get_reverse_proxy_keyフィルターは任意の文字列を返せて、その元になる URL には攻撃者が制御できるREQUEST_URIが含まれます。../を含むキーでキャッシュディレクトリ外のファイルを削除できる恐れがあったため、パス解決前にキーを[A-Za-z0-9_-]のみに正規化するようにしました。設定保存のタイミング修正 — AdminPage.php / NetworkAdminPage.php
設定保存処理が
admin_head-{hook}（HTML 出力開始後）で走っていたため、保存後のwp_safe_redirect()が「headers already sent」で失敗し、途中で切れたページが表示される状態でした。出力前に発火するload-{hook}へ移動しました。権限チェックの追加 — AdminPage.php
設定保存が nonce 検証のみで、権限チェックがありませんでした。
current_user_can('manage_options')を追加し、メニュー登録もロール名'administrator'から正規の権限'manage_options'に変更しました（WP のベストプラクティス）。IP スプーフィング緩和 — CachingHeaders.php
コメント投稿者の IP として
X-Forwarded-Forヘッダーの先頭要素を無検証で返していました。FILTER_VALIDATE_IPで構文検証し、不正な値ならREMOTE_ADDRにフォールバックするようにしました。型の堅牢化 — FlushCache.php / NetworkAdminPage.php / AdminPage.php
$_GET/$_POSTから読む nonce とredirect_toすべてにis_string()チェックを追加しました。以前は?nonce[]=xのように配列を渡すと PHP 8 でTypeError（500 エラー）になっていました。直接アクセス対策とエスケープ — views/admin-panel.php / views/network-admin-panel.php
両ビューファイルに
ABSPATHガードを追加し（直接アクセスでコードが実行され、エラー経由でパス情報が漏れる恐れがあった）、管理画面の「Flush All Caches」リンクをesc_url()で包みました。検証
変更した 8 ファイルすべてに
php -l（構文チェック）を実行し、PHPUnit のフルスイート（147 テスト / 194 アサーション）が変更前後ともにグリーンであることを確認済みです。なお、テストが配線バグを検出できなかったのは、テストが各クラスを直接インスタンス化していてブートストラップ自体を検証していないためです。この点は PR にも回帰防止テストの推奨として記載しています。Summary by CodeRabbit
New Features
Documentation
Tests