mirror of
https://github.com/appwrite/appwrite.git
synced 2026-05-26 13:51:13 +00:00
refactor(cache): generalize content-type guard and gate bypass to privileged callers
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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'])
|
||||
|
||||
Reference in New Issue
Block a user