From 3a5bb98b70528fe2be1bc5b409aa3de1bb9c8afe Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:29:35 +0100 Subject: [PATCH] refactor(cache): generalize content-type guard and gate bypass to privileged callers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on the prior commit: - The shared cache middleware should not hardcode `image/*`. Replace the inline content-type check with a generic `cache.contentType` route label (a prefix match). Each route that opts into caching now declares `->label('cache.contentType', 'image/')` alongside `->label('cache', true)`. - A public `Cache-Control: no-cache` bypass is a DDoS vector on expensive pipelines (Imagick transforms, Chromium screenshots). Restrict the bypass to privileged callers — API keys, admin console, and other server-side contexts — so only trusted traffic can force-miss the cache. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/controllers/shared/api.php | 26 +++++++++++-------- .../Modules/Avatars/Http/Browsers/Get.php | 1 + .../Avatars/Http/Cards/Cloud/Back/Get.php | 1 + .../Avatars/Http/Cards/Cloud/Front/Get.php | 1 + .../Avatars/Http/Cards/Cloud/OG/Get.php | 1 + .../Modules/Avatars/Http/CreditCards/Get.php | 1 + .../Modules/Avatars/Http/Favicon/Get.php | 1 + .../Modules/Avatars/Http/Flags/Get.php | 1 + .../Modules/Avatars/Http/Image/Get.php | 1 + .../Modules/Avatars/Http/Screenshots/Get.php | 1 + .../Http/Buckets/Files/Preview/Get.php | 1 + 11 files changed, 25 insertions(+), 11 deletions(-) diff --git a/app/controllers/shared/api.php b/app/controllers/shared/api.php index a0a0da2e0b..72c101ce95 100644 --- a/app/controllers/shared/api.php +++ b/app/controllers/shared/api.php @@ -631,11 +631,14 @@ Http::init() $isImageTransformation = $route->getPath() === '/v1/storage/buckets/:bucketId/files/:fileId/preview'; $isDisabled = isset($plan['imageTransformations']) && $plan['imageTransformations'] === -1 && ! $user->isPrivileged($authorization->getRoles()); - // Allow clients/operators to bypass a poisoned cache entry by sending - // `Cache-Control: no-cache`. The save path still runs on success and - // overwrites the existing entry with the freshly rendered response. + // Allow privileged callers (API key / admin / server-side) to bypass a + // poisoned cache entry by sending `Cache-Control: no-cache`. Restricted + // to privileged callers so public traffic cannot force-miss the cache + // and overload the underlying pipeline (e.g. Imagick transformations). + // The save path still runs on success and overwrites the stored entry. $cacheControl = \strtolower($request->getHeader('cache-control', '')); - $bypassCache = \str_contains($cacheControl, 'no-cache') || \str_contains($cacheControl, 'no-store'); + $bypassRequested = \str_contains($cacheControl, 'no-cache') || \str_contains($cacheControl, 'no-store'); + $bypassCache = $bypassRequested && ($isAppUser || $user->isPrivileged($authorization->getRoles())); $key = $request->cacheIdentifier(); Span::add('storage.cache.key', $key); @@ -988,13 +991,14 @@ Http::shutdown() $resource = $resourceType = null; $data = $response->getPayload(); $statusCode = $response->getStatusCode(); - // All routes that opt into label('cache', true) produce binary images - // via Response::file(). Refuse to cache anything else — this stops - // error pages, JSON error bodies, or partial/empty outputs from being - // persisted and served back on subsequent requests. - $contentType = (string) $response->getContentType(); - $isImagePayload = \str_starts_with(\strtolower($contentType), 'image/'); - if ($isImagePayload && ! empty($data['payload']) && $statusCode >= 200 && $statusCode < 300) { + // Routes may declare a `cache.contentType` prefix (e.g. `image/`) that + // the response must match to be cacheable. This stops error pages, + // fallback bodies, or partial outputs that slipped through with a 2xx + // status from being persisted and served back on later requests. + $contentTypePrefix = $route->getLabel('cache.contentType', ''); + $contentType = \strtolower((string) $response->getContentType()); + $contentTypeOk = empty($contentTypePrefix) || \str_starts_with($contentType, \strtolower($contentTypePrefix)); + if ($contentTypeOk && ! empty($data['payload']) && $statusCode >= 200 && $statusCode < 300) { $pattern = $route->getLabel('cache.resource', null); if (! empty($pattern)) { $resource = $parseLabel($pattern, $responsePayload, $requestParams, $user); diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Browsers/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Browsers/Get.php index 637ea647ef..04e31b6111 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Browsers/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Browsers/Get.php @@ -33,6 +33,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resource', 'avatar/browser') ->label('sdk', new Method( namespace: 'avatars', diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Back/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Back/Get.php index a6a013ef21..4f24b3bff2 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Back/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Back/Get.php @@ -36,6 +36,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resourceType', 'cards/cloud-back') ->label('cache.resource', 'card-back/{request.userId}') ->label('docs', false) diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Front/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Front/Get.php index d0c600192b..de075a8599 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Front/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/Front/Get.php @@ -36,6 +36,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resourceType', 'cards/cloud') ->label('cache.resource', 'card/{request.userId}') ->label('docs', false) diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/OG/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/OG/Get.php index ad74d6c192..fb004d4024 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/OG/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Cards/Cloud/OG/Get.php @@ -36,6 +36,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resourceType', 'cards/cloud-og') ->label('cache.resource', 'card-og/{request.userId}') ->label('docs', false) diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/CreditCards/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/CreditCards/Get.php index 87357f14c7..6bdea97f21 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/CreditCards/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/CreditCards/Get.php @@ -33,6 +33,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resource', 'avatar/credit-card') ->label('sdk', new Method( namespace: 'avatars', diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Favicon/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Favicon/Get.php index a41d0f81da..9c2dce68fa 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Favicon/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Favicon/Get.php @@ -40,6 +40,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resource', 'avatar/favicon') ->label('sdk', new Method( namespace: 'avatars', diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Flags/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Flags/Get.php index 8230b15f50..e4a427b9e5 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Flags/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Flags/Get.php @@ -33,6 +33,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resource', 'avatar/flag') ->label('sdk', new Method( namespace: 'avatars', diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Image/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Image/Get.php index 77ff9dd8ce..e5b8447516 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Image/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Image/Get.php @@ -36,6 +36,7 @@ class Get extends Action ->groups(['api', 'avatars']) ->label('scope', 'avatars.read') ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resource', 'avatar/image') ->label('sdk', new Method( namespace: 'avatars', diff --git a/src/Appwrite/Platform/Modules/Avatars/Http/Screenshots/Get.php b/src/Appwrite/Platform/Modules/Avatars/Http/Screenshots/Get.php index c43c0fc4bf..daa893806c 100644 --- a/src/Appwrite/Platform/Modules/Avatars/Http/Screenshots/Get.php +++ b/src/Appwrite/Platform/Modules/Avatars/Http/Screenshots/Get.php @@ -46,6 +46,7 @@ class Get extends Action ->label('usage.metric', METRIC_AVATARS_SCREENSHOTS_GENERATED) ->label('abuse-limit', 60) ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resourceType', 'avatar/screenshot') ->label('cache.resource', 'screenshot/{request.url}/{request.width}/{request.height}/{request.scale}/{request.theme}/{request.userAgent}/{request.fullpage}/{request.locale}/{request.timezone}/{request.latitude}/{request.longitude}/{request.accuracy}/{request.touch}/{request.permissions}/{request.sleep}/{request.quality}/{request.output}') ->label('sdk', new Method( diff --git a/src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/Preview/Get.php b/src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/Preview/Get.php index 4fa5006db8..834821b65f 100644 --- a/src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/Preview/Get.php +++ b/src/Appwrite/Platform/Modules/Storage/Http/Buckets/Files/Preview/Get.php @@ -52,6 +52,7 @@ class Get extends Action ->label('scope', 'files.read') ->label('resourceType', RESOURCE_TYPE_BUCKETS) ->label('cache', true) + ->label('cache.contentType', 'image/') ->label('cache.resourceType', 'bucket/{request.bucketId}') ->label('cache.resource', 'file/{request.fileId}') ->label('cache.params', ['width', 'height', 'gravity', 'quality', 'borderWidth', 'borderColor', 'borderRadius', 'opacity', 'rotation', 'background', 'output', 'project'])