From d3a1e30f9a50b653abb40d62c1f8ef391fdc76fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brian=20Sch=C3=A4ffner?= Date: Fri, 23 Oct 2026 23:45:32 +0200 Subject: [PATCH 1/2] fix(admin): carry package notices in a user transient instead of the URL The package manager rendered kernel-package-message from the query string as an admin notice. The text was escaped, but a crafted link could still show an administrator arbitrary notice text. Redirects now keep only the notice code in the URL and stash the message in a short-lived per-user transient. --- src/Admin/PackageManagerPage.php | 50 +++++++++++++++++++------- tests/Admin/PackageManagerPageTest.php | 50 ++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/Admin/PackageManagerPage.php b/src/Admin/PackageManagerPage.php index d95038a..a5cf9a2 100644 --- a/src/Admin/PackageManagerPage.php +++ b/src/Admin/PackageManagerPage.php @@ -14,7 +14,7 @@ final class PackageManagerPage private const string ACTION_QUERY_VAR = 'kernel-package-action'; private const string PACKAGE_QUERY_VAR = 'kernel-package'; private const string NOTICE_QUERY_VAR = 'kernel-package-notice'; - private const string MESSAGE_QUERY_VAR = 'kernel-package-message'; + private const string NOTICE_TRANSIENT_PREFIX = 'kernel_package_notice_'; private const string VIEW_QUERY_VAR = 'package_status'; private const string BULK_NONCE_ACTION = 'bulk-packages'; @@ -186,7 +186,7 @@ public function render(): void $packages = $this->packages->all(); $currentView = $this->currentView(); $notice = $this->requestString(self::NOTICE_QUERY_VAR); - $message = $this->noticeMessage($notice, $this->requestString(self::MESSAGE_QUERY_VAR)); + $message = $this->noticeMessage($notice, $this->takeNoticeMessage()); $this->view($currentView, $notice, $message)->render( $packages, @@ -400,23 +400,49 @@ private function nonceAction(string $action, PackageMetadata $package): string private function redirect(string $notice, string $message = ''): never { + // Free text never travels through the URL: a crafted link could otherwise + // show an administrator an arbitrary notice. + $this->stashNoticeMessage($message); wp_safe_redirect( $this->pageUrl( - array_filter( - array_merge( - $this->currentViewArgs(), - [ - self::NOTICE_QUERY_VAR => $notice, - self::MESSAGE_QUERY_VAR => $message, - ], - ), - static fn (string $value): bool => $value !== '', - ), + array_merge($this->currentViewArgs(), [self::NOTICE_QUERY_VAR => $notice]), ), ); exit; } + private function stashNoticeMessage(string $message): void + { + if ($message === '' || !function_exists('set_transient')) { + return; + } + + set_transient($this->noticeTransientKey(), $message, 60); + } + + private function takeNoticeMessage(): string + { + if (!function_exists('get_transient')) { + return ''; + } + + $key = $this->noticeTransientKey(); + $message = get_transient($key); + + if ($message !== false) { + delete_transient($key); + } + + return is_string($message) ? $message : ''; + } + + private function noticeTransientKey(): string + { + $user = function_exists('get_current_user_id') ? get_current_user_id() : 0; + + return self::NOTICE_TRANSIENT_PREFIX . $user; + } + private function isPostRequest(): bool { return ($_SERVER['REQUEST_METHOD'] ?? '') === 'POST'; diff --git a/tests/Admin/PackageManagerPageTest.php b/tests/Admin/PackageManagerPageTest.php index a1afb85..ffa3725 100644 --- a/tests/Admin/PackageManagerPageTest.php +++ b/tests/Admin/PackageManagerPageTest.php @@ -133,6 +133,38 @@ function wp_clean_plugins_cache(bool $clearUpdateCache = true): void $GLOBALS['kernel_test_plugins_cache_cleaned'] = $clearUpdateCache; } } + + if (!function_exists('get_current_user_id')) { + function get_current_user_id(): int + { + return 7; + } + } + + if (!function_exists('set_transient')) { + function set_transient(string $transient, mixed $value, int $expiration = 0): bool + { + $GLOBALS['kernel_test_transients'][$transient] = $value; + + return true; + } + } + + if (!function_exists('get_transient')) { + function get_transient(string $transient): mixed + { + return $GLOBALS['kernel_test_transients'][$transient] ?? false; + } + } + + if (!function_exists('delete_transient')) { + function delete_transient(string $transient): bool + { + unset($GLOBALS['kernel_test_transients'][$transient]); + + return true; + } + } } namespace SymPress\Kernel\Tests\Admin { @@ -199,6 +231,24 @@ public function testPackageManagerRegistersAdminPageWhenEnabled(): void ); } + public function testNoticeMessagesTravelThroughAUserTransientNotTheUrl(): void + { + $GLOBALS['kernel_test_transients'] = []; + $_GET['kernel-package-message'] = 'Crafted link text'; + + try { + $this->invokePageMethod('stashNoticeMessage', 'Package not found.'); + + self::assertSame(['kernel_package_notice_7' => 'Package not found.'], $GLOBALS['kernel_test_transients']); + self::assertSame('Package not found.', $this->invokePageMethod('takeNoticeMessage')); + self::assertSame([], $GLOBALS['kernel_test_transients']); + self::assertSame('', $this->invokePageMethod('takeNoticeMessage')); + self::assertSame('Package action failed.', $this->invokePageMethod('noticeMessage', 'error', '')); + } finally { + unset($GLOBALS['kernel_test_transients'], $_GET['kernel-package-message']); + } + } + public function testActionsAreBlockedWhenWordPressDisallowsFileModifications(): void { $GLOBALS['kernel_test_file_mod_allowed']['plugins'] = false; From 19c52cf52362cd6b84bef4e0c4682a69c762628c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brian=20Sch=C3=A4ffner?= Date: Sat, 24 Oct 2026 00:16:39 +0200 Subject: [PATCH 2/2] perf(admin): read the notice transient only when a notice is requested --- src/Admin/PackageManagerPage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Admin/PackageManagerPage.php b/src/Admin/PackageManagerPage.php index a5cf9a2..c0cee86 100644 --- a/src/Admin/PackageManagerPage.php +++ b/src/Admin/PackageManagerPage.php @@ -186,7 +186,7 @@ public function render(): void $packages = $this->packages->all(); $currentView = $this->currentView(); $notice = $this->requestString(self::NOTICE_QUERY_VAR); - $message = $this->noticeMessage($notice, $this->takeNoticeMessage()); + $message = $notice === '' ? '' : $this->noticeMessage($notice, $this->takeNoticeMessage()); $this->view($currentView, $notice, $message)->render( $packages,