diff --git a/Dockerfile b/Dockerfile index db52fc9b..20577904 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,6 +12,8 @@ RUN apt-get update \ && apt-get update \ && apt-get install -y --no-install-recommends nodejs \ && docker-php-ext-install intl pdo_mysql zip \ + && pecl install redis \ + && docker-php-ext-enable redis \ && a2enmod rewrite headers expires \ && sed -ri "s!/var/www/html!${APACHE_DOCUMENT_ROOT}!g" /etc/apache2/sites-available/000-default.conf /etc/apache2/apache2.conf \ && rm -rf /var/lib/apt/lists/* diff --git a/README.md b/README.md index f5bb2f35..c966918d 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,18 @@ This repository contains the code responsible for https://plugins.cakephp.org +## Production request protection + +The public package listing has query validation and an IP-based rate limit. It +uses Dokku's `REDIS_URL` by default. `RATE_LIMIT_CACHE_URL` can override it if +you later want a dedicated Redis database or service. + +The container includes the PHP Redis extension. `TRUSTED_PROXY_IPS` must list +only proxies that overwrite `X-Forwarded-For`; for the current Dokku setup its +default is `172.17.0.1`. Set `RATE_LIMIT_ENABLED=false` only for temporary +maintenance or local development. The defaults are 90 listing views/minute, +15 filtered listings/minute, and 30 autocomplete requests/minute per client IP. + ## Starting local development You need [DDEV](https://docs.ddev.com/en/stable/) installed and configured on your machine. diff --git a/composer.json b/composer.json index 461b0ff5..5aff599f 100644 --- a/composer.json +++ b/composer.json @@ -65,7 +65,7 @@ "illuminate": "bin/cake illuminate code", "phpstan": "phpstan analyse", "phpstan-baseline": "phpstan --generate-baseline", - "rector-setup": "cp composer.json composer.backup && composer require --dev rector/rector:\"~2.3.1\" && mv composer.backup composer.json", + "rector-setup": "cp composer.json composer.backup && composer require --dev rector/rector:\"~2.6.1\" && mv composer.backup composer.json", "rector-check": "vendor/bin/rector process --dry-run", "rector-fix": "vendor/bin/rector process" } diff --git a/composer.lock b/composer.lock index 3e1514e4..245517f1 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "4d01c1e1045275c0f4078565d31efb92", + "content-hash": "32ac1dbc66dbf810946aed3c4cc42a64", "packages": [ { "name": "admad/cakephp-social-auth", @@ -4659,6 +4659,66 @@ ], "time": "2025-08-19T18:57:03+00:00" }, + { + "name": "rector/rector", + "version": "2.6.1", + "source": { + "type": "git", + "url": "https://github.com/rectorphp/rector.git", + "reference": "b8e68f058bca43e01a2e1caa51ef022d6551ed95" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/rectorphp/rector/zipball/b8e68f058bca43e01a2e1caa51ef022d6551ed95", + "reference": "b8e68f058bca43e01a2e1caa51ef022d6551ed95", + "shasum": "" + }, + "require": { + "php": "^7.4|^8.0", + "phpstan/phpstan": "^2.2.6" + }, + "conflict": { + "rector/rector-doctrine": "*", + "rector/rector-downgrade-php": "*", + "rector/rector-phpunit": "*", + "rector/rector-symfony": "*" + }, + "suggest": { + "ext-dom": "To manipulate phpunit.xml via the custom-rule command" + }, + "bin": [ + "bin/rector" + ], + "type": "library", + "autoload": { + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Instant Upgrade and Automated Refactoring of any PHP code", + "homepage": "https://getrector.com/", + "keywords": [ + "automation", + "dev", + "migration", + "refactoring" + ], + "support": { + "issues": "https://github.com/rectorphp/rector/issues", + "source": "https://github.com/rectorphp/rector/tree/2.6.1" + }, + "funding": [ + { + "url": "https://github.com/tomasvotruba", + "type": "github" + } + ], + "time": "2026-08-03T17:30:34+00:00" + }, { "name": "sebastian/cli-parser", "version": "5.0.1", diff --git a/config/app.php b/config/app.php index 18d21a30..01a241aa 100644 --- a/config/app.php +++ b/config/app.php @@ -1,5 +1,7 @@ [ + 'rateLimitEnabled' => $rateLimitEnabled && is_string($rateLimitCacheUrl) && str_starts_with($rateLimitCacheUrl, 'redis://'), + 'trustedProxyIps' => $trustedProxyIps, + 'maxFilterValues' => max(1, (int)env('MAX_PACKAGE_FILTER_VALUES', 3)), + 'rateLimits' => [ + 'browse' => ['limit' => 90, 'window' => 60], + 'filtered' => ['limit' => 15, 'window' => 60], + 'autocomplete' => ['limit' => 30, 'window' => 60], + ], + ], + /* * Configure the cache adapters. */ @@ -122,6 +151,14 @@ 'url' => env('CACHE_DEFAULT_URL'), ], + 'rate_limit' => [ + 'className' => FileEngine::class, + 'path' => CACHE . 'rate_limit' . DS, + 'duration' => '+1 minute', + 'prefix' => 'plugins_rate_limit_', + 'fallback' => false, + ] + ($rateLimitCacheUrl ? ['url' => $rateLimitCacheUrl] : []), + /* * Configure the cache used for general framework caching. * Translation cache files are stored with this configuration. diff --git a/config/app_local.example.php b/config/app_local.example.php index bcc09034..39617d71 100644 --- a/config/app_local.example.php +++ b/config/app_local.example.php @@ -1,5 +1,7 @@ `[configuration options]`. - * - * Available options: - * - onlyDebug: Load the plugin only in debug mode. Default false. - * - onlyCli: Load the plugin only in CLI mode. Default false. - * - optional: Do not throw an exception if the plugin is not found. Default false. - */ +/* + * List of plugins to load in the form `PluginName` => `[configuration options]`. + * + * Available options: + * - onlyDebug: Load the plugin only in debug mode. Default false. + * - onlyCli: Load the plugin only in CLI mode. Default false. + * - optional: Do not throw an exception if the plugin is not found. Default false. + */ return [ 'DebugKit' => ['onlyDebug' => true], 'Bake' => ['onlyCli' => true, 'optional' => true], diff --git a/config/routes.php b/config/routes.php index 14ad6804..7e724f5f 100644 --- a/config/routes.php +++ b/config/routes.php @@ -1,4 +1,6 @@ */ class Application extends BaseApplication implements AuthenticationServiceProviderInterface { @@ -87,6 +88,8 @@ public function middleware(MiddlewareQueue $middlewareQueue): MiddlewareQueue return true; } }); + $requestProtection = (array)Configure::read('PublicRequestProtection', []); + $trustedProxyIps = (array)($requestProtection['trustedProxyIps'] ?? []); $middlewareQueue // Catch any exceptions in the lower layers, @@ -98,6 +101,12 @@ public function middleware(MiddlewareQueue $middlewareQueue): MiddlewareQueue 'cacheTime' => Configure::read('Asset.cacheTime'), ])) + // Reject malformed and unnecessarily expensive public search requests + // before routing or database work is performed. + ->add(new PublicRequestGuardMiddleware([ + 'maxFilterValues' => (int)($requestProtection['maxFilterValues'] ?? 3), + ])) + // Add routing middleware. // If you have a large number of routes connected, turning on routes // caching in production could improve performance. @@ -137,6 +146,54 @@ public function middleware(MiddlewareQueue $middlewareQueue): MiddlewareQueue ->add(new AuthenticationMiddleware($this)); + if ($requestProtection['rateLimitEnabled'] ?? false) { + $middlewareQueue->insertAfter( + PublicRequestGuardMiddleware::class, + new RateLimitMiddleware([ + 'cache' => 'rate_limit', + 'headers' => true, + 'strategyClass' => AtomicFixedWindowRateLimiter::class, + 'identifierCallback' => static function (ServerRequestInterface $request) use ($trustedProxyIps): string { + $remoteAddress = (string)($request->getServerParams()['REMOTE_ADDR'] ?? 'unknown'); + if (!in_array($remoteAddress, $trustedProxyIps, true)) { + return $remoteAddress; + } + + $forwardedFor = trim(explode(',', $request->getHeaderLine('X-Forwarded-For'))[0] ?? ''); + if (filter_var($forwardedFor, FILTER_VALIDATE_IP) === false) { + return $remoteAddress; + } + + return $forwardedFor; + }, + 'limiterResolver' => static function (ServerRequestInterface $request): string { + $path = rtrim($request->getUri()->getPath(), '/'); + if (str_ends_with($path, '.json')) { + $path = substr($path, 0, -5); + } + if ($path === '/autocomplete') { + return 'autocomplete'; + } + + return $request->getQueryParams() === [] ? 'browse' : 'filtered'; + }, + 'limiters' => $requestProtection['rateLimits'] ?? [], + 'skipCheck' => static function (ServerRequestInterface $request): bool { + if (!in_array($request->getMethod(), ['GET', 'HEAD'], true)) { + return true; + } + + $path = rtrim($request->getUri()->getPath(), '/'); + if (str_ends_with($path, '.json')) { + $path = substr($path, 0, -5); + } + + return !in_array($path, ['', '/packages', '/autocomplete'], true); + }, + ]), + ); + } + return $middlewareQueue; } diff --git a/src/Command/CleanCommand.php b/src/Command/CleanCommand.php index f8088fe9..4ae84724 100644 --- a/src/Command/CleanCommand.php +++ b/src/Command/CleanCommand.php @@ -3,6 +3,7 @@ namespace App\Command; +use Cake\Cache\Cache; use Cake\Command\Command; use Cake\Console\Arguments; use Cake\Console\ConsoleIo; @@ -61,5 +62,7 @@ public function execute(Arguments $args, ConsoleIo $io): void foreach ($allPackages as $package) { $packagesTable->delete($package); } + + Cache::delete('package_filter_tags_v1'); } } diff --git a/src/Command/SyncPackagesCommand.php b/src/Command/SyncPackagesCommand.php index c50ab432..a6999acd 100644 --- a/src/Command/SyncPackagesCommand.php +++ b/src/Command/SyncPackagesCommand.php @@ -3,6 +3,7 @@ namespace App\Command; +use Cake\Cache\Cache; use Cake\Command\Command; use Cake\Console\Arguments; use Cake\Console\CommandFactoryInterface; @@ -123,7 +124,7 @@ public function execute(Arguments $args, ConsoleIo $io): void $failed = 0; $i = 0; - /** @var \Cake\Command\Helper\ProgressHelper $progress */ + /** @var \Cake\Console\Helper\ProgressHelper $progress */ $progress = $io->helper('Progress'); $progress->init(['total' => $total, 'width' => 60]); $io->out('', 0); @@ -195,6 +196,8 @@ public function execute(Arguments $args, ConsoleIo $io): void $deleted, $deleteFailed, )); + + Cache::delete('package_filter_tags_v1'); } /** @@ -267,7 +270,7 @@ private function getDataForPackage(string $packageName): array } /** - * @return \Cake\I18n\Date|null + * Extract the release date */ private function extractReleaseDate(?Version $version): ?Date { diff --git a/src/Controller/PackagesController.php b/src/Controller/PackagesController.php index 88f88426..2d430714 100644 --- a/src/Controller/PackagesController.php +++ b/src/Controller/PackagesController.php @@ -3,6 +3,7 @@ namespace App\Controller; +use Cake\Cache\Cache; use Cake\Core\Configure; use Cake\Http\Response; use Cake\ORM\Query\SelectQuery; @@ -87,14 +88,22 @@ public function index(): Response } $packages = $this->paginate($query, ['limit' => 21]); - $cakephpTags = $this->Packages->Tags->find('list', keyField: 'slug') - ->where(['slug LIKE' => 'cakephp-%']) - ->toArray(); - $cakephpTags = $this->sortVersionTags($cakephpTags, 'CakePHP'); - $phpTags = $this->Packages->Tags->find('list', keyField: 'slug') - ->where(['slug LIKE' => 'php-%']) - ->toArray(); - $phpTags = $this->sortVersionTags($phpTags, 'PHP'); + /** @var array{cakephp: array, php: array} $filterTags */ + $filterTags = Cache::remember('package_filter_tags_v1', function (): array { + $cakephpTags = $this->Packages->Tags->find('list', keyField: 'slug') + ->where(['slug LIKE' => 'cakephp-%']) + ->toArray(); + $phpTags = $this->Packages->Tags->find('list', keyField: 'slug') + ->where(['slug LIKE' => 'php-%']) + ->toArray(); + + return [ + 'cakephp' => $this->sortVersionTags($cakephpTags, 'CakePHP'), + 'php' => $this->sortVersionTags($phpTags, 'PHP'), + ]; + }); + $cakephpTags = $filterTags['cakephp']; + $phpTags = $filterTags['php']; $this->set(compact('featuredPackages', 'packages', 'cakephpTags', 'phpTags')); diff --git a/src/Controller/PagesController.php b/src/Controller/PagesController.php index 7d370f7c..a87745b5 100644 --- a/src/Controller/PagesController.php +++ b/src/Controller/PagesController.php @@ -47,7 +47,6 @@ public function initialize(): void * Displays a view * * @param string ...$path Path segments. - * @return \Cake\Http\Response|null * @throws \Cake\Http\Exception\ForbiddenException When a directory traversal attempt. * @throws \Cake\View\Exception\MissingTemplateException When the view file could not * be found and in debug mode. diff --git a/src/Http/Middleware/PublicRequestGuardMiddleware.php b/src/Http/Middleware/PublicRequestGuardMiddleware.php new file mode 100644 index 00000000..ce618b26 --- /dev/null +++ b/src/Http/Middleware/PublicRequestGuardMiddleware.php @@ -0,0 +1,189 @@ +maxFilterValues = max(1, (int)($config['maxFilterValues'] ?? 3)); + } + + /** + * @inheritDoc + */ + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + if (!in_array($request->getMethod(), ['GET', 'HEAD'], true)) { + return $handler->handle($request); + } + + $path = $this->endpointPath($request); + if ($path === '' || $path === '/packages') { + $request = $this->validateIndexRequest($request); + } elseif ($path === '/autocomplete') { + $this->validateAutocompleteRequest($request); + } + + return $handler->handle($request); + } + + /** + * Validate and normalize package listing parameters. + */ + private function validateIndexRequest(ServerRequestInterface $request): ServerRequestInterface + { + $query = $request->getQueryParams(); + $this->assertAllowedParameters($query, self::INDEX_PARAMETERS); + + if (isset($query['search'])) { + $this->assertString($query['search'], 'search'); + if (mb_strlen($query['search']) > self::MAX_SEARCH_LENGTH) { + throw new BadRequestException('Search query is too long.'); + } + } + + foreach (['cakephp_slugs' => 'cakephp', 'php_slugs' => 'php'] as $key => $prefix) { + if (!isset($query[$key])) { + continue; + } + $query[$key] = $this->normaliseSlugValues($query[$key], $prefix); + } + + if (isset($query['sort'])) { + $this->assertString($query['sort'], 'sort'); + if (!in_array($query['sort'], self::SORT_FIELDS, true)) { + throw new BadRequestException('Unsupported sort field.'); + } + } + if (isset($query['direction'])) { + $this->assertString($query['direction'], 'direction'); + if (!in_array($query['direction'], self::DIRECTIONS, true)) { + throw new BadRequestException('Unsupported sort direction.'); + } + } + if (isset($query['page'])) { + $this->assertStringOrInteger($query['page'], 'page'); + if (!preg_match('/^[1-9][0-9]*$/', (string)$query['page']) || (int)$query['page'] > self::MAX_PAGE) { + throw new BadRequestException('Invalid page number.'); + } + } + + return $request->withQueryParams($query); + } + + /** + * Validate package autocomplete parameters. + */ + private function validateAutocompleteRequest(ServerRequestInterface $request): void + { + $query = $request->getQueryParams(); + $this->assertAllowedParameters($query, self::AUTOCOMPLETE_PARAMETERS); + + if (isset($query['q'])) { + $this->assertString($query['q'], 'q'); + if (mb_strlen($query['q']) > self::MAX_SEARCH_LENGTH) { + throw new BadRequestException('Search query is too long.'); + } + } + } + + /** + * @param array $query Query parameters. + * @param list $allowed Allowed parameter names. + */ + private function assertAllowedParameters(array $query, array $allowed): void + { + foreach (array_keys($query) as $key) { + if (!in_array($key, $allowed, true)) { + throw new BadRequestException('Unsupported query parameter.'); + } + } + } + + /** + * @return list + */ + private function normaliseSlugValues(mixed $value, string $prefix): array + { + $values = is_array($value) ? array_values($value) : [$value]; + if (count($values) > $this->maxFilterValues) { + throw new BadRequestException('Too many filter values.'); + } + + $normalised = []; + foreach ($values as $slug) { + $this->assertString($slug, 'filter'); + if (!preg_match('/^' . preg_quote($prefix, '/') . '-[1-9][0-9]*-[0-9]+$/', $slug)) { + throw new BadRequestException('Invalid filter value.'); + } + $normalised[$slug] = $slug; + } + + return array_values($normalised); + } + + /** + * Require a query parameter to be a string. + */ + private function assertString(mixed $value, string $parameter): void + { + if (!is_string($value)) { + throw new BadRequestException(sprintf('Invalid %s parameter.', $parameter)); + } + } + + /** + * Require a query parameter to be a scalar page number. + */ + private function assertStringOrInteger(mixed $value, string $parameter): void + { + if (!is_string($value) && !is_int($value)) { + throw new BadRequestException(sprintf('Invalid %s parameter.', $parameter)); + } + } + + /** + * Return the route path without an optional JSON extension. + */ + private function endpointPath(ServerRequestInterface $request): string + { + $path = rtrim($request->getUri()->getPath(), '/'); + + return str_ends_with($path, '.json') ? substr($path, 0, -5) : $path; + } +} diff --git a/src/Http/RateLimit/AtomicFixedWindowRateLimiter.php b/src/Http/RateLimit/AtomicFixedWindowRateLimiter.php new file mode 100644 index 00000000..380cf0aa --- /dev/null +++ b/src/Http/RateLimit/AtomicFixedWindowRateLimiter.php @@ -0,0 +1,71 @@ +cache = $cache; + } + + /** + * @inheritDoc + */ + public function attempt(string $identifier, int $limit, int $window, int $cost = 1): array + { + $now = time(); + $windowStart = intdiv($now, $window) * $window; + $key = $identifier . '_' . $windowStart; + + if ($this->cache->add($key, $cost)) { + $count = $cost; + } else { + try { + $count = $this->cache->increment($key, $cost); + } catch (LogicException $exception) { + throw new RuntimeException('Rate limiting requires a cache backend with atomic increments.', 0, $exception); + } + } + + if ($count === false) { + throw new RuntimeException('Unable to update rate-limit counter.'); + } + + return [ + 'allowed' => $count <= $limit, + 'limit' => $limit, + 'remaining' => max(0, $limit - $count), + 'reset' => $windowStart + $window, + ]; + } + + /** + * @inheritDoc + */ + public function reset(string $identifier): void + { + $this->cache->delete($identifier); + } +} diff --git a/src/View/Helper/UserHelper.php b/src/View/Helper/UserHelper.php index 3071281e..49857e4a 100644 --- a/src/View/Helper/UserHelper.php +++ b/src/View/Helper/UserHelper.php @@ -20,8 +20,6 @@ class UserHelper extends Helper /** * Get the username of the logged-in user. - * - * @return string|null */ public function username(): ?string { @@ -36,8 +34,6 @@ public function username(): ?string * Get the display name of the logged-in user. * * Falls back to username if first/last name are not set. - * - * @return string|null */ public function displayName(): ?string { @@ -56,7 +52,6 @@ public function displayName(): ?string * Get the GitHub avatar URL for the logged-in user. * * @param int $size Image size in pixels. - * @return string|null */ public function avatarUrl(int $size = 80): ?string { @@ -73,7 +68,6 @@ public function avatarUrl(int $size = 80): ?string * * @param int $size Image size in pixels. * @param array $attrs Additional HTML attributes for the img tag. - * @return string|null */ public function avatar(int $size = 80, array $attrs = []): ?string { diff --git a/tests/TestCase/ApplicationTest.php b/tests/TestCase/ApplicationTest.php index 3ec6acf7..1d1c1b85 100644 --- a/tests/TestCase/ApplicationTest.php +++ b/tests/TestCase/ApplicationTest.php @@ -17,6 +17,7 @@ namespace App\Test\TestCase; use App\Application; +use App\Http\Middleware\PublicRequestGuardMiddleware; use Cake\Core\Configure; use Cake\Error\Middleware\ErrorHandlerMiddleware; use Cake\Http\MiddlewareQueue; @@ -80,6 +81,8 @@ public function testMiddleware(): void $middleware->seek(1); $this->assertInstanceOf(AssetMiddleware::class, $middleware->current()); $middleware->seek(2); + $this->assertInstanceOf(PublicRequestGuardMiddleware::class, $middleware->current()); + $middleware->seek(3); $this->assertInstanceOf(RoutingMiddleware::class, $middleware->current()); } } diff --git a/tests/TestCase/Controller/PackagesControllerTest.php b/tests/TestCase/Controller/PackagesControllerTest.php index d2eaf3ac..a64b43fb 100644 --- a/tests/TestCase/Controller/PackagesControllerTest.php +++ b/tests/TestCase/Controller/PackagesControllerTest.php @@ -73,6 +73,26 @@ public function testIndexHidesFeaturedSliderWhenSearching(): void $this->assertResponseContains('vendor/package-02'); } + /** + * @return void + */ + public function testIndexRejectsUnknownQueryParameters(): void + { + $this->get('/?amp%3Bcakephp_slugs%5B%5D=cakephp-4-4'); + + $this->assertResponseCode(400); + } + + /** + * @return void + */ + public function testIndexRejectsExcessiveFilterValues(): void + { + $this->get('/?cakephp_slugs%5B%5D=cakephp-3-0&cakephp_slugs%5B%5D=cakephp-3-1&cakephp_slugs%5B%5D=cakephp-3-2&cakephp_slugs%5B%5D=cakephp-3-3'); + + $this->assertResponseCode(400); + } + /** * @return void */ diff --git a/tests/TestCase/Http/RateLimit/AtomicFixedWindowRateLimiterTest.php b/tests/TestCase/Http/RateLimit/AtomicFixedWindowRateLimiterTest.php new file mode 100644 index 00000000..b3e25295 --- /dev/null +++ b/tests/TestCase/Http/RateLimit/AtomicFixedWindowRateLimiterTest.php @@ -0,0 +1,28 @@ +setConfig('prefix', ''); + $limiter = new AtomicFixedWindowRateLimiter($cache); + + $this->assertTrue($limiter->attempt('visitor', 2, 60)['allowed']); + $this->assertTrue($limiter->attempt('visitor', 2, 60)['allowed']); + + $result = $limiter->attempt('visitor', 2, 60); + $this->assertFalse($result['allowed']); + $this->assertSame(0, $result['remaining']); + } +}