Commit 905ffefc authored by Mahmoud Aglan's avatar Mahmoud Aglan

fix(cache): stop refusing to restore the objects we cache on purpose

Every branded page on the live tenant answered 500 with
"BrandingService::for(): Return value must be of type BrandProfile,
__PHP_Incomplete_Class returned".

config/cache.php shipped Laravel's default serializable_classes => false,
which unserializes cache values with allowed_classes: false. That is safe for
an app that caches only scalars and arrays, and fatal for one that does not.
We cache whole value objects deliberately: BrandProfile is the entire tenant
brand, resolved once and held until branding changes, and read by every admin,
portal and print layout. With classes refused it came back as
__PHP_Incomplete_Class, the return type threw, and the admin went dark.

The setting exists to stop a gadget chain in a cache an attacker can already
write to. Ours is the tenant's own Postgres, reachable only by the app;
anyone who can write there can do worse directly. CACHE_SERIALIZABLE_CLASSES
lets a deployment pass its own allowlist without a code change.

BrandingService now also checks what the cache handed back before trusting it,
and rebuilds when it is not a profile. A cache that cannot return this class
should cost a rebuild per request, never a 500 — the failure has to degrade,
not detonate.

Verified: every admin screen and the group roster render against a restored
copy of the live tenant with CACHE_STORE=database.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent 55b03a06
...@@ -67,11 +67,21 @@ public function for(?int $academyId): BrandProfile ...@@ -67,11 +67,21 @@ public function for(?int $academyId): BrandProfile
} }
$version = (int) ($academy->branding_version ?? 1); $version = (int) ($academy->branding_version ?? 1);
$key = "branding:{$academyId}:v{$version}";
$profile = Cache::get($key);
if (! $profile instanceof BrandProfile) {
// Either nothing is cached, or what came back is not a profile —
// which is what a cache configured to refuse this class returns.
// Rebuild rather than hand back a corpse: a wrong-typed value in a
// cache is not a cache hit, and letting it reach the return type
// below turns a cache problem into a 500 on every branded page.
$profile = $this->build($academy, $version);
Cache::forever($key, $profile);
}
return $this->memo[$academyId] = Cache::rememberForever( return $this->memo[$academyId] = $profile;
"branding:{$academyId}:v{$version}",
fn () => $this->build($academy, $version)
);
} }
/** /**
......
...@@ -126,11 +126,26 @@ ...@@ -126,11 +126,26 @@
|-------------------------------------------------------------------------- |--------------------------------------------------------------------------
| |
| This value determines the classes that can be unserialized from cache | This value determines the classes that can be unserialized from cache
| storage. By default, no PHP classes will be unserialized from your | storage. Laravel ships this as false — no class is restored, everything
| cache to prevent gadget chain attacks if your APP_KEY is leaked. | comes back as __PHP_Incomplete_Class — which is safe for an app that only
| ever caches scalars and arrays, and fatal for one that does not.
|
| We cache whole value objects on purpose: BrandProfile is the entire
| tenant brand, resolved once and held until branding changes, and every
| admin, portal and print layout reads it. With this false it came back as
| __PHP_Incomplete_Class, the return type on BrandingService::for() threw,
| and every page that draws the tenant's colours answered 500.
|
| What this protects against is a gadget chain in a cache an attacker can
| already write to. Ours is the tenant's own Postgres, reachable only by
| the app itself; anyone who can write to it can do far worse directly.
| CACHE_SERIALIZABLE_CLASSES is here so a deployment that wants the
| narrower posture can pass its own allowlist without a code change.
| |
*/ */
'serializable_classes' => false, 'serializable_classes' => env('CACHE_SERIALIZABLE_CLASSES')
? explode(',', (string) env('CACHE_SERIALIZABLE_CLASSES'))
: true,
]; ];
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment