diff --git a/src/Admin/PackageManagerPage.php b/src/Admin/PackageManagerPage.php index d95038a..c0cee86 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 = $notice === '' ? '' : $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;