From 78fe54e883176263e8cc10871c4b46dd67bebfd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brian=20Sch=C3=A4ffner?= Date: Sat, 3 Oct 2026 15:58:28 +0200 Subject: [PATCH 1/2] fix(cache): preserve upgrades and stable signed cache identity --- .github/workflows/dependency-canary.yml | 2 +- .github/workflows/qa.yml | 4 +-- CHANGELOG.md | 10 +++--- README.md | 32 ++++++++++++------- Resources/config/services.php | 2 +- dropin/object-cache.php | 3 +- src/Cache/FrameworkCacheConfigurationPass.php | 2 +- .../SymPressFrameworkExtension.php | 5 --- src/ObjectCache/CacheAdapterFactory.php | 5 ++- src/ObjectCache/CacheConfig.php | 2 ++ src/ObjectCache/CacheConfigFactory.php | 23 ++++++++++--- src/ObjectCache/ObjectCacheValueCodec.php | 9 ++++-- .../FrameworkCacheConfigurationPassTest.php | 22 +++++++++++++ .../SymPressFrameworkExtensionTest.php | 9 +++--- tests/ObjectCache/CacheConfigFactoryTest.php | 19 +++++++++++ tests/ObjectCache/DropInInstallerTest.php | 5 +++ .../ObjectCache/ObjectCacheValueCodecTest.php | 16 ++++++++++ 17 files changed, 133 insertions(+), 37 deletions(-) diff --git a/.github/workflows/dependency-canary.yml b/.github/workflows/dependency-canary.yml index 7ab4ffb..2e41e25 100644 --- a/.github/workflows/dependency-canary.yml +++ b/.github/workflows/dependency-canary.yml @@ -13,6 +13,6 @@ jobs: permissions: contents: read issues: write - uses: sympress/workflows/.github/workflows/dependency-canary.yml@177fa0d727b278d2103052ec77c102b4a4c492a0 + uses: sympress/workflows/.github/workflows/dependency-canary.yml@78d083a9a0176cf0252318b0c69fc64938a727d6 with: php_version: '8.5' diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index f43b763..6c2a686 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -12,10 +12,10 @@ permissions: jobs: workflows: - uses: sympress/workflows/.github/workflows/lint-workflows.yml@177fa0d727b278d2103052ec77c102b4a4c492a0 + uses: sympress/workflows/.github/workflows/lint-workflows.yml@78d083a9a0176cf0252318b0c69fc64938a727d6 qa: - uses: sympress/workflows/.github/workflows/sympress-qa.yml@177fa0d727b278d2103052ec77c102b4a4c492a0 + uses: sympress/workflows/.github/workflows/sympress-qa.yml@78d083a9a0176cf0252318b0c69fc64938a727d6 with: php_version: '8.5' secrets: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c40511..0d9326c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html) where applicable. +## Unreleased + +- Keep upgrades with an existing short APP_SECRET available while disabling persistent object caching until a strong signing secret is provided; report the production requirement through validation without per-request warning noise. +- Use a stable literal SYMPRESS_PROJECT_DIR for WordPress signing/namespaces and Symfony cache pool seeds while retaining active-release package, configuration and autoloader discovery. +- Allow explicitly reviewed plugin/WooCommerce object classes only after signed payload authentication. + ## 1.0.3 — 2026-10-02 - Scope WordPress object-cache namespaces and signing keys to the project and environment, including explicitly configured prefix labels. - Authenticate filesystem, SQLite/PDO and APCu serialized payloads before decoding, matching the signed native Redis/Memcached boundary. - Require a strong independent application/cache secret and isolate default temporary stores by user and namespace; weak configuration falls back safely to request memory. - -## Unreleased - -- Initial Framework Bundle package, QA workflow and repository documentation. diff --git a/README.md b/README.md index eeab0f9..b775942 100644 --- a/README.md +++ b/README.md @@ -26,9 +26,11 @@ Require the package in a SymPress project: composer require sympress/framework-bundle ``` -Projects must provide a randomly generated `APP_SECRET` of at least 32 bytes. Missing or shorter -secrets fail FrameworkBundle compilation. The bundle passes `APP_SECRET` directly to Symfony's -FrameworkBundle and does not fall back to predictable project paths. +Projects must provide a randomly generated `APP_SECRET` of at least 32 bytes for production. +An existing shorter value remains usable during an upgrade so container compilation can finish; +persistent WordPress object caching stays disabled until a strong application/cache secret is supplied. +Runtime Doctor checks this production requirement before deployment. The bundle passes `APP_SECRET` +directly to Symfony's FrameworkBundle and does not derive a secret from project paths or `AUTH_KEY`. The bundle is discovered through Composer metadata: @@ -150,23 +152,27 @@ Projects without SymPress Runtime can explicitly call `DropInInstaller::install( Managed SymPress drop-ins are only rewritten when their contents change and third-party drop-ins without the `sympress-framework-object-cache` marker are left untouched by the runtime installer. SymPress Runtime remains the preferred owner in SymPress Runtime projects because it publishes the drop-in during Composer/project setup instead of an explicit application setup step. -The delegator resolves the Composer autoloader from `SYMPRESS_PROJECT_DIR`, `APP_PROJECT_DIR`, `WP_CONTENT_DIR`, `ABSPATH`, or nearby parent directories; `SYMPRESS_COMPOSER_AUTOLOAD` and `SYMPRESS_OBJECT_CACHE_FUNCTIONS` can override those paths for custom layouts. +The delegator resolves the Composer autoloader from `APP_PROJECT_DIR`, `WP_CONTENT_DIR`, `ABSPATH`, or nearby active release directories, then uses `SYMPRESS_PROJECT_DIR` as a compatibility fallback; `SYMPRESS_COMPOSER_AUTOLOAD` and `SYMPRESS_OBJECT_CACHE_FUNCTIONS` can override those paths for custom layouts. This makes the same file work when copied by SymPress Runtime, symlinked from `content-dev`, or installed by the runtime fallback. If a persistent backend cannot be initialized, the drop-in logs or warns about the backend failure before falling back to request-local array cache. WP-CLI cache flushes run directly in the current CLI process and never create temporary PHP endpoints in the web root. Redis and Memcached use native object-cache backends instead of Symfony internals for WordPress counter semantics. `add`, `replace`, `incr` and `decr` are mapped to backend-native atomic operations where the backend supports them; Redis counters use a Lua script so missing keys are not created and decrements clamp to zero like WordPress expects. Existing non-numeric counter values follow WordPress core semantics and are treated as zero. Flushes use versioned namespaces, so group/runtime invalidation does not depend on scanning or reflecting backend internals. The other Symfony-backed drivers keep best-effort semantics because PSR-6 does not expose cross-process compare-and-swap primitives. All persistent WordPress drivers require `SYMPRESS_CACHE_SECRET` or `APP_SECRET` of at least 32 bytes; -`AUTH_KEY` is not a signing-secret fallback. Missing or short object-cache secrets disable persistence -and report the initialization failure before using request-local cache. Redis and Memcached retain +`AUTH_KEY` is not a signing-secret fallback. Missing or short object-cache secrets use request-local +cache without repeating a warning on every request; production validation reports the missing +requirement. Redis and Memcached retain native counter operations; APCu, filesystem and SQLite/PDO authenticate serialized values with the same signed codec before reconstructing objects. Treat persistent cache backends and the kernel cache directory as trusted infrastructure; do not expose them to untrusted writers. -WordPress cache prefixes include a hash of the project root and environment, including when -`SYMPRESS_CACHE_PREFIX` is set. Roots resolve from `SYMPRESS_PROJECT_DIR`, `APP_PROJECT_DIR`, -`ABSPATH`, `WP_CONTENT_DIR`, then the working directory. Set a stable project root before loading -the drop-in in custom layouts. Environment resolves from `APP_ENV`, `APP_RUNTIME_ENV`, +WordPress cache prefixes include a hash of the project identity and environment, including when +`SYMPRESS_CACHE_PREFIX` is set. Identity resolves from the literal `SYMPRESS_PROJECT_DIR`, then +`APP_PROJECT_DIR`, `ABSPATH`, `WP_CONTENT_DIR`, then the working directory. Set `SYMPRESS_PROJECT_DIR` +to a persistent deployment base or project identifier before the kernel/drop-in boots and retain it +across release directories. Symfony's default `framework.cache.prefix_seed` uses the same identity, +falling back to `kernel.project_dir` when it is absent. Package/configuration discovery and the +kernel's actual project directory still use the active release. Environment resolves from `APP_ENV`, `APP_RUNTIME_ENV`, `WP_ENVIRONMENT_TYPE`, then `production`. The signing key is also bound to this scoped prefix. This changes the cache identity on upgrade; existing cache records remain unused and refill normally. @@ -191,7 +197,11 @@ validate the HMAC before deserializing. The codec permits only `stdClass`, `WP_Post`, `WP_Term`, `WP_Comment`, `WP_User`, `WP_Error`, `WP_Site`, `WP_Network`, `DateTime`, `DateTimeImmutable`, and `DateTimeZone`. Unsupported objects (including nested objects) are rejected rather than invoking application magic methods. Custom -application cache objects need a reviewed codec extension or plain data arrays. +application cache objects can be admitted with `SYMPRESS_CACHE_ALLOWED_CLASSES`, a comma-separated +list of exact class names such as `WC_Product,Vendor\\Plugin\\CacheValue`. Wildcards and permissive +boolean values are rejected. Additional classes apply only to authenticated signed payloads; +review their deserialization/magic methods before adding them. Classes may load later during +WordPress/plugin bootstrap. Prefer plain data arrays when object restoration is unnecessary. Filesystem and default SQLite cache paths use a private per-user temporary directory with a separate project/environment namespace and mode 0700. APCu still requires trusted PHP-FPM pools: diff --git a/Resources/config/services.php b/Resources/config/services.php index d6fc188..c877b14 100644 --- a/Resources/config/services.php +++ b/Resources/config/services.php @@ -56,7 +56,7 @@ return static function (ContainerConfigurator $container): void { $parameters = $container->parameters(); - $parameters->set('cache.prefix.seed', 'sympress.%kernel.project_dir%.%kernel.environment%'); + $parameters->set('cache.prefix.seed', 'sympress.%env(default:kernel.project_dir:SYMPRESS_PROJECT_DIR)%.%kernel.environment%'); $parameters->set('framework.cache.version', '%kernel.environment%'); $parameters->set('framework.cache', []); diff --git a/dropin/object-cache.php b/dropin/object-cache.php index bd52eb1..336da78 100644 --- a/dropin/object-cache.php +++ b/dropin/object-cache.php @@ -47,7 +47,6 @@ $projectDirs[] = $dir; }; - $addProjectDir($path($value('SYMPRESS_PROJECT_DIR'))); $addProjectDir($path($value('APP_PROJECT_DIR'))); $contentDir = defined('WP_CONTENT_DIR') ? $path(constant('WP_CONTENT_DIR')) : $path(__DIR__); @@ -79,6 +78,8 @@ $scanDir = $parent; } + // Stable cache identity must not select a stale deployment-base autoloader. + $addProjectDir($path($value('SYMPRESS_PROJECT_DIR'))); $projectDirs = array_values(array_unique($projectDirs)); $autoload = $file($path($value('SYMPRESS_COMPOSER_AUTOLOAD'))); diff --git a/src/Cache/FrameworkCacheConfigurationPass.php b/src/Cache/FrameworkCacheConfigurationPass.php index 74edb30..e636eb1 100644 --- a/src/Cache/FrameworkCacheConfigurationPass.php +++ b/src/Cache/FrameworkCacheConfigurationPass.php @@ -35,7 +35,7 @@ final class FrameworkCacheConfigurationPass implements CompilerPassInterface * @var array */ private const array DEFAULTS = [ - 'prefix_seed' => 'sympress.%kernel.project_dir%.%kernel.environment%', + 'prefix_seed' => 'sympress.%env(default:kernel.project_dir:SYMPRESS_PROJECT_DIR)%.%kernel.environment%', 'app' => 'cache.adapter.filesystem', 'system' => 'cache.adapter.system', 'directory' => '%kernel.cache_dir%/pools/app', diff --git a/src/DependencyInjection/SymPressFrameworkExtension.php b/src/DependencyInjection/SymPressFrameworkExtension.php index caadba4..ff04e18 100644 --- a/src/DependencyInjection/SymPressFrameworkExtension.php +++ b/src/DependencyInjection/SymPressFrameworkExtension.php @@ -30,11 +30,6 @@ public function load(array $configs, ContainerBuilder $container): void if ($configuration !== null) { $config = (new Processor())->processConfiguration($configuration, $configs); - $secret = $config['secret'] ?? null; - $resolved = is_string($secret) ? $container->resolveEnvPlaceholders($secret, true) : null; - if (!is_string($resolved) || strlen($resolved) < 32) { - throw new \InvalidArgumentException('FrameworkBundle requires APP_SECRET or framework.secret of at least 32 bytes.'); - } $container->setParameter('framework.cache', $config['cache'] ?? []); } diff --git a/src/ObjectCache/CacheAdapterFactory.php b/src/ObjectCache/CacheAdapterFactory.php index 3e8d822..6283de8 100644 --- a/src/ObjectCache/CacheAdapterFactory.php +++ b/src/ObjectCache/CacheAdapterFactory.php @@ -17,6 +17,9 @@ final class CacheAdapterFactory { public function createPersistent(CacheConfig $config): ObjectCacheBackendInterface { + if (in_array($config->driver, ['apcu', 'filesystem', 'redis', 'memcached', 'pdo', 'sqlite'], true) && (!is_string($config->secret) || strlen($config->secret) < 32)) { + return $this->createRuntime(); + } try { return $this->createAdapter($config); } catch (\Throwable $exception) { @@ -215,7 +218,7 @@ private function codec(CacheConfig $config): ObjectCacheValueCodec ? hash_hmac('sha256', 'sympress.object-cache:' . $config->prefix, $config->secret) : null; - return new ObjectCacheValueCodec($secret); + return new ObjectCacheValueCodec($secret, $config->allowedClasses); } private function assertRedisConnection(object $connection): void diff --git a/src/ObjectCache/CacheConfig.php b/src/ObjectCache/CacheConfig.php index 5398cfd..69ff84a 100644 --- a/src/ObjectCache/CacheConfig.php +++ b/src/ObjectCache/CacheConfig.php @@ -8,6 +8,7 @@ final class CacheConfig { /** * @param array $driverArgs + * @param list $allowedClasses */ public function __construct( public readonly string $driver, @@ -17,6 +18,7 @@ public function __construct( public readonly int $purgeInterval, public readonly string $prefix, public readonly ?string $secret = null, + public readonly array $allowedClasses = [], ) { } } diff --git a/src/ObjectCache/CacheConfigFactory.php b/src/ObjectCache/CacheConfigFactory.php index e4f37b8..c8acc9b 100644 --- a/src/ObjectCache/CacheConfigFactory.php +++ b/src/ObjectCache/CacheConfigFactory.php @@ -24,17 +24,32 @@ public function create(): CacheConfig $this->scopedPrefix(), $this->stringValue('SYMPRESS_CACHE_SECRET') ?? $this->stringValue('APP_SECRET'), + $this->allowedClasses(), ); } + /** @return list */ + private function allowedClasses(): array + { + $configured = $this->stringValue('SYMPRESS_CACHE_ALLOWED_CLASSES'); + $classes = []; + foreach ($configured === null ? [] : explode(',', $configured) as $class) { + $class = ltrim(trim($class), '\\'); + if ($class !== '' && !in_array(strtolower($class), ['true', 'false', 'null', 'self', 'static', 'parent'], true) && preg_match('/^[A-Za-z_][A-Za-z0-9_]*(?:\\\\[A-Za-z_][A-Za-z0-9_]*)*$/D', $class)) { + $classes[] = $class; + } + } + return array_values(array_unique($classes)); + } + private function scopedPrefix(): string { - $project = $this->stringValue('SYMPRESS_PROJECT_DIR') - ?? $this->stringValue('APP_PROJECT_DIR') - ?? $this->stringValue('ABSPATH') + $identity = $this->stringValue('SYMPRESS_PROJECT_DIR') + ?? $this->stringValue('APP_PROJECT_DIR'); + $project = $identity ?? $this->stringValue('ABSPATH') ?? $this->stringValue('WP_CONTENT_DIR') ?? (getcwd() ?: __DIR__); - $project = realpath($project) ?: $project; + $project = $identity === null ? (realpath($project) ?: $project) : rtrim($project, '/\\'); $environment = $this->stringValue('APP_ENV') ?? $this->stringValue('APP_RUNTIME_ENV') ?? $this->stringValue('WP_ENVIRONMENT_TYPE') diff --git a/src/ObjectCache/ObjectCacheValueCodec.php b/src/ObjectCache/ObjectCacheValueCodec.php index 4057c4a..9959ad4 100644 --- a/src/ObjectCache/ObjectCacheValueCodec.php +++ b/src/ObjectCache/ObjectCacheValueCodec.php @@ -9,8 +9,10 @@ final class ObjectCacheValueCodec private const string LEGACY_SERIALIZED_PREFIX = 'sympress-cache-v1:'; private const string SIGNED_SERIALIZED_PREFIX = 'sympress-cache-v2:'; + /** @param list $allowedClasses */ public function __construct( private readonly ?string $secret = null, + private readonly array $allowedClasses = [], ) { } @@ -93,9 +95,12 @@ private function normalizedSecret(): ?string private function unserializePayload(string $payload): mixed { - // Only inert core cache data objects may be reconstructed. Application classes - // must opt into a separate codec rather than executing arbitrary magic methods. + // Additional application classes require explicit trusted configuration and + // a valid signed payload. Unsigned legacy payloads keep the core allowlist. $allowed = ['stdClass', 'WP_Post', 'WP_Term', 'WP_Comment', 'WP_User', 'WP_Error', 'WP_Site', 'WP_Network', 'DateTime', 'DateTimeImmutable', 'DateTimeZone']; + if ($this->normalizedSecret() !== null) { + $allowed = array_values(array_unique([...$allowed, ...$this->allowedClasses])); + } try { $value = @unserialize($payload, ['allowed_classes' => $allowed]); } catch (\Throwable) { diff --git a/tests/Cache/FrameworkCacheConfigurationPassTest.php b/tests/Cache/FrameworkCacheConfigurationPassTest.php index d1d3809..19c65a7 100644 --- a/tests/Cache/FrameworkCacheConfigurationPassTest.php +++ b/tests/Cache/FrameworkCacheConfigurationPassTest.php @@ -17,6 +17,28 @@ final class FrameworkCacheConfigurationPassTest extends TestCase { + public function testStableProjectIdentityKeepsSymfonyPoolPrefixAcrossReleases(): void + { + $previous = $_ENV['SYMPRESS_PROJECT_DIR'] ?? null; + $_ENV['SYMPRESS_PROJECT_DIR'] = '/deploy/stable-site'; + try { + foreach (['/deploy/stable-site/releases/one', '/deploy/stable-site/releases/two'] as $release) { + $container = $this->container(); + $container->setParameter('kernel.project_dir', $release); + (new FrameworkCacheConfigurationPass())->process($container); + self::assertSame('sympress./deploy/stable-site.test', $container->getParameter('cache.prefix.seed')); + self::assertSame($release, $container->getParameter('kernel.project_dir')); + } + } finally { + if ($previous === null) { + unset($_ENV['SYMPRESS_PROJECT_DIR']); + } + if ($previous !== null) { + $_ENV['SYMPRESS_PROJECT_DIR'] = $previous; + } + } + } + public function testRegistersDefaultAndCustomCachePools(): void { $container = $this->container(); diff --git a/tests/DependencyInjection/SymPressFrameworkExtensionTest.php b/tests/DependencyInjection/SymPressFrameworkExtensionTest.php index 2b6ce55..c5e57bd 100644 --- a/tests/DependencyInjection/SymPressFrameworkExtensionTest.php +++ b/tests/DependencyInjection/SymPressFrameworkExtensionTest.php @@ -29,11 +29,12 @@ public function testLoadsSymfonyFrameworkBundleServicesBeyondCache(): void self::assertIsArray($container->getParameter('framework.cache')); } - public function testRejectsShortSecretWithoutDisclosingIt(): void + public function testShortExistingSecretPreservesContainerBuildAvailability(): void { - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessageMatches('/at least 32 bytes/'); - (new SymPressFrameworkExtension())->load([['secret' => 'test-only-short']], $this->container()); + $container = $this->container(); + (new SymPressFrameworkExtension())->load([['secret' => 'test-only-short']], $container); + self::assertTrue($container->hasDefinition('http_kernel')); + self::assertSame('test-only-short', $container->getParameter('kernel.secret')); } private function container(): ContainerBuilder diff --git a/tests/ObjectCache/CacheConfigFactoryTest.php b/tests/ObjectCache/CacheConfigFactoryTest.php index 85cf8eb..877bd00 100644 --- a/tests/ObjectCache/CacheConfigFactoryTest.php +++ b/tests/ObjectCache/CacheConfigFactoryTest.php @@ -24,6 +24,8 @@ protected function tearDown(): void 'AUTH_KEY', 'APP_PROJECT_DIR', 'APP_ENV', + 'SYMPRESS_PROJECT_DIR', + 'SYMPRESS_CACHE_ALLOWED_CLASSES', ] as $name ) { putenv($name); @@ -31,6 +33,23 @@ protected function tearDown(): void } } + public function testExplicitStableIdentitySurvivesReleaseChangesAndDoesNotResolveSymlinks(): void + { + $_ENV['SYMPRESS_PROJECT_DIR'] = '/deploy/site/current'; + $_ENV['APP_PROJECT_DIR'] = '/deploy/site/releases/one'; + $first = (new CacheConfigFactory())->create()->prefix; + $_ENV['APP_PROJECT_DIR'] = '/deploy/site/releases/two'; + self::assertSame($first, (new CacheConfigFactory())->create()->prefix); + $_ENV['SYMPRESS_PROJECT_DIR'] = '/deploy/another/current'; + self::assertNotSame($first, (new CacheConfigFactory())->create()->prefix); + } + + public function testExplicitClassAllowlistRejectsWildcardsAndAcceptsUnloadedApplicationNames(): void + { + $_ENV['SYMPRESS_CACHE_ALLOWED_CLASSES'] = 'WC_Product,Vendor\\Plugin\\CacheData,*,true'; + self::assertSame(['WC_Product', 'Vendor\\Plugin\\CacheData'], (new CacheConfigFactory())->create()->allowedClasses); + } + public function testNamespaceSeparatesProjectsAndEnvironmentsEvenWithExplicitPrefix(): void { $_ENV['APP_PROJECT_DIR'] = '/sites/first'; diff --git a/tests/ObjectCache/DropInInstallerTest.php b/tests/ObjectCache/DropInInstallerTest.php index 946c1c6..cc15da4 100644 --- a/tests/ObjectCache/DropInInstallerTest.php +++ b/tests/ObjectCache/DropInInstallerTest.php @@ -155,6 +155,9 @@ public function testDropInCopiedBySymPressRuntimeCanResolveProjectLayout(): void } mkdir($projectDir . '/packages'); + $identity = $projectDir . '/stable-identity'; + mkdir($identity . '/vendor', 0700, true); + file_put_contents($identity . '/vendor/autoload.php', 'modify('+1 day')->format('Y-m-d')); } + + public function testConfiguredPluginClassesRequireAuthenticationBeforeTheirMagicMethodsRun(): void + { + CacheWakeupProbe::$woken = 0; + $codec = new ObjectCacheValueCodec('secret', [CacheWakeupProbe::class]); + $payload = $codec->encode(new CacheWakeupProbe()); + self::assertInstanceOf(CacheWakeupProbe::class, $codec->decode($payload)); + self::assertSame(1, CacheWakeupProbe::$woken); + $tampered = str_replace('sympress-cache-v2:', 'sympress-cache-v2:bad', $payload); + self::assertFalse($codec->decode($tampered)); + self::assertSame(1, CacheWakeupProbe::$woken); + $unsigned = new ObjectCacheValueCodec(null, [CacheWakeupProbe::class]); + self::assertFalse($unsigned->decode($unsigned->encode(new CacheWakeupProbe()))); + self::assertSame(1, CacheWakeupProbe::$woken); + CacheWakeupProbe::$woken = 0; + } } final class CacheWakeupProbe From 62ec6ba5693dac4ea7584b0a18e5ca76ef096e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brian=20Sch=C3=A4ffner?= Date: Sat, 3 Oct 2026 16:21:22 +0200 Subject: [PATCH 2/2] fix: guard and authenticate Symfony application cache pools --- CHANGELOG.md | 3 +- README.md | 11 ++- Resources/config/packages/framework.php | 2 +- Resources/config/services.php | 6 +- src/Cache/FrameworkCacheConfigurationPass.php | 13 +++- src/Cache/GuardApplicationCachePoolsPass.php | 35 +++++++++ src/Cache/GuardedCachePoolFactory.php | 18 +++++ src/Cache/SignedMarshaller.php | 47 +++++++++++ .../SymPressFrameworkExtension.php | 1 + src/SymPressFrameworkBundle.php | 2 + .../FrameworkCacheConfigurationPassTest.php | 25 ++++++ tests/Cache/SignedMarshallerTest.php | 43 +++++++++++ tests/Fixtures/cache-secret-upgrade.php | 77 +++++++++++++++++++ tests/Kernel/SymPressFrameworkBundleTest.php | 33 ++++++++ 14 files changed, 309 insertions(+), 7 deletions(-) create mode 100644 src/Cache/GuardApplicationCachePoolsPass.php create mode 100644 src/Cache/GuardedCachePoolFactory.php create mode 100644 src/Cache/SignedMarshaller.php create mode 100644 tests/Cache/SignedMarshallerTest.php create mode 100644 tests/Fixtures/cache-secret-upgrade.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d9326c..0971952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ where applicable. ## Unreleased -- Keep upgrades with an existing short APP_SECRET available while disabling persistent object caching until a strong signing secret is provided; report the production requirement through validation without per-request warning noise. +- Keep upgrades with missing or short APP_SECRET available on existing kernels: WordPress object caches and Symfony application pools use request memory until a strong signing secret is provided; report the production requirement through validation without per-request warning noise. +- Authenticate Symfony application pool payloads before deserialization across persistent adapters and configured marshallers; preserve trusted Symfony system/compiled-code caches and discard unsigned legacy entries as misses. - Use a stable literal SYMPRESS_PROJECT_DIR for WordPress signing/namespaces and Symfony cache pool seeds while retaining active-release package, configuration and autoloader discovery. - Allow explicitly reviewed plugin/WooCommerce object classes only after signed payload authentication. diff --git a/README.md b/README.md index b775942..961b668 100644 --- a/README.md +++ b/README.md @@ -27,11 +27,18 @@ composer require sympress/framework-bundle ``` Projects must provide a randomly generated `APP_SECRET` of at least 32 bytes for production. -An existing shorter value remains usable during an upgrade so container compilation can finish; -persistent WordPress object caching stays disabled until a strong application/cache secret is supplied. +An existing missing or shorter value remains usable during an upgrade: WordPress object caching, +`cache.app` and custom application pools use request memory until a strong secret is supplied. +This also works when updating the bundle with an existing Kernel 1.1.3 installation. Runtime Doctor checks this production requirement before deployment. The bundle passes `APP_SECRET` directly to Symfony's FrameworkBundle and does not derive a secret from project paths or `AUTH_KEY`. +Application pools authenticate serialized payloads before restoring values, including filesystem, +PDO/SQLite, APCu, Redis and Memcached adapters and configured custom marshallers. Existing unsigned +entries become cache misses and refill normally after the upgrade. Application pools configured with +PHP file or system adapters use signed filesystem storage instead. Symfony's own `cache.system` +retains its trusted compiled-code storage and remains available without an application secret. + The bundle is discovered through Composer metadata: ```json diff --git a/Resources/config/packages/framework.php b/Resources/config/packages/framework.php index dbfe4c6..f615fd4 100644 --- a/Resources/config/packages/framework.php +++ b/Resources/config/packages/framework.php @@ -6,6 +6,6 @@ return static function (ContainerConfigurator $container): void { $container->extension('framework', [ - 'secret' => '%env(APP_SECRET)%', + 'secret' => '%env(default::APP_SECRET)%', ]); }; diff --git a/Resources/config/services.php b/Resources/config/services.php index c877b14..422482d 100644 --- a/Resources/config/services.php +++ b/Resources/config/services.php @@ -11,6 +11,7 @@ use SymPress\Framework\Cache\CachePoolClearer; use SymPress\Framework\Cache\CachePoolPruner; use SymPress\Framework\Cache\CachePoolRegistry; +use SymPress\Framework\Cache\SignedMarshaller; use SymPress\Framework\Cache\Command\CachePoolClearCommand; use SymPress\Framework\Cache\Command\CachePoolDeleteCommand; use SymPress\Framework\Cache\Command\CachePoolInvalidateTagsCommand; @@ -59,6 +60,7 @@ $parameters->set('cache.prefix.seed', 'sympress.%env(default:kernel.project_dir:SYMPRESS_PROJECT_DIR)%.%kernel.environment%'); $parameters->set('framework.cache.version', '%kernel.environment%'); $parameters->set('framework.cache', []); + $parameters->set('framework.cache.secret', '%env(default::APP_SECRET)%'); $services = $container->services(); $services->defaults() @@ -267,8 +269,8 @@ ]) ->tag('monolog.logger', ['channel' => 'cache']); - $services->set('cache.default_marshaller', DefaultMarshaller::class) - ->args([null, '%kernel.debug%']); + $services->set('cache.default_marshaller', SignedMarshaller::class) + ->args(['%framework.cache.secret%', '%cache.prefix.seed%', inline_service(DefaultMarshaller::class)->args([null, '%kernel.debug%'])]); $services->set('cache.early_expiration_handler', EarlyExpirationHandler::class) ->args([service('reverse_container')]) diff --git a/src/Cache/FrameworkCacheConfigurationPass.php b/src/Cache/FrameworkCacheConfigurationPass.php index e636eb1..faa120a 100644 --- a/src/Cache/FrameworkCacheConfigurationPass.php +++ b/src/Cache/FrameworkCacheConfigurationPass.php @@ -82,10 +82,21 @@ public function process(ContainerBuilder $container): void } $pools[$name] = $this->normalizePool($pool); + // App values use authenticated data stores; PHP bytecode caches remain a system-only surface. + foreach ($pools[$name]['adapters'] as $provider => $adapter) { + if (in_array($adapter, ['cache.adapter.php_files', 'cache.adapter.system', 'cache.system'], true)) { + $pools[$name]['adapters'][$provider] = 'cache.adapter.filesystem'; + } + } + if (isset($pools[$name]['marshaller']) && $pools[$name]['marshaller'] !== 'cache.default_marshaller') { + $marshaller = '.sympress.signed_marshaller.' . $name; + $container->register($marshaller, SignedMarshaller::class)->setArguments(['%framework.cache.secret%', '%cache.prefix.seed%', new Reference($pools[$name]['marshaller'])]); + $pools[$name]['marshaller'] = $marshaller; + } } $pools['cache.app'] = $this->normalizePool([ - 'adapters' => [$config['app']], + 'adapters' => [in_array($config['app'], ['cache.adapter.php_files', 'cache.adapter.system', 'cache.system'], true) ? 'cache.adapter.filesystem' : $config['app']], 'public' => true, 'tags' => false, 'clearer' => 'cache.app_clearer', diff --git a/src/Cache/GuardApplicationCachePoolsPass.php b/src/Cache/GuardApplicationCachePoolsPass.php new file mode 100644 index 0000000..2a52369 --- /dev/null +++ b/src/Cache/GuardApplicationCachePoolsPass.php @@ -0,0 +1,35 @@ +hasParameter('framework.cache.secret')) { + return; + } + foreach ($container->findTaggedServiceIds('cache.pool') as $id => $tags) { + $definition = $container->getDefinition($id); + // Symfony's system PHP caches are trusted compiled application code, not serialized app payloads. + if ($definition->isAbstract() || $definition->getFactory() === [AbstractAdapter::class, 'createSystemCache']) { + continue; + } + $inner = '.sympress.persistent.' . $id; + $persistent = clone $definition; + $persistent->setPublic(false)->setTags([]); + $container->setDefinition($inner, $persistent); + $definition->setFactory([GuardedCachePoolFactory::class, 'create']); + $definition->setArguments(['%framework.cache.secret%', new ServiceClosureArgument(new Reference($inner))]); + $definition->setMethodCalls([]); + } + } +} diff --git a/src/Cache/GuardedCachePoolFactory.php b/src/Cache/GuardedCachePoolFactory.php new file mode 100644 index 0000000..9556764 --- /dev/null +++ b/src/Cache/GuardedCachePoolFactory.php @@ -0,0 +1,18 @@ +key = hash_hmac('sha256', 'sympress.symfony-cache:' . $namespace, $secret); + } + + /** @param array $values @param array|null $failed @return array */ + public function marshall(array $values, ?array &$failed): array + { + $encoded = $this->inner->marshall($values, $failed); + foreach ($encoded as $id => $payload) { + $encoded[$id] = self::PREFIX . hash_hmac('sha256', $payload, $this->key) . ':' . base64_encode($payload); + } + return $encoded; + } + + public function unmarshall(string $value): mixed + { + if (!str_starts_with($value, self::PREFIX)) { + throw new CacheException('Unsigned application cache payload.'); + } + $payload = substr($value, strlen(self::PREFIX)); + $signature = substr($payload, 0, 64); + $encoded = substr($payload, 65); + $decoded = base64_decode($encoded, true); + if (($payload[64] ?? null) !== ':' || !is_string($decoded) || !hash_equals(hash_hmac('sha256', $decoded, $this->key), $signature)) { + throw new CacheException('Application cache authentication failed.'); + } + return $this->inner->unmarshall($decoded); + } +} diff --git a/src/DependencyInjection/SymPressFrameworkExtension.php b/src/DependencyInjection/SymPressFrameworkExtension.php index ff04e18..70399eb 100644 --- a/src/DependencyInjection/SymPressFrameworkExtension.php +++ b/src/DependencyInjection/SymPressFrameworkExtension.php @@ -34,6 +34,7 @@ public function load(array $configs, ContainerBuilder $container): void } parent::load($configs, $container); + $container->setParameter('framework.cache.secret', $container->hasParameter('kernel.secret') ? $container->getParameter('kernel.secret') : '%env(default::APP_SECRET)%'); $this->removeUnavailableAutoconfiguration($container); $this->removeSymfonyKernelConsoleCommands($container); diff --git a/src/SymPressFrameworkBundle.php b/src/SymPressFrameworkBundle.php index 803cbda..2763872 100644 --- a/src/SymPressFrameworkBundle.php +++ b/src/SymPressFrameworkBundle.php @@ -5,6 +5,7 @@ namespace SymPress\Framework; use SymPress\Framework\Cache\FrameworkCacheConfigurationPass; +use SymPress\Framework\Cache\GuardApplicationCachePoolsPass; use SymPress\Framework\Cache\RemoveAbstractResetTagsPass; use SymPress\Framework\DependencyInjection\Compiler\RegisterFallbackRouterPass; use SymPress\Kernel\Bundle\AbstractBundle; @@ -20,6 +21,7 @@ final class SymPressFrameworkBundle extends AbstractBundle public function build(ContainerBuilder $container): void { parent::build($container); + $container->addCompilerPass(new GuardApplicationCachePoolsPass(), PassConfig::TYPE_BEFORE_REMOVING, 100); if (class_exists(SymfonyFrameworkBundle::class)) { (new SymfonyFrameworkBundle())->build($container); diff --git a/tests/Cache/FrameworkCacheConfigurationPassTest.php b/tests/Cache/FrameworkCacheConfigurationPassTest.php index 19c65a7..ddee85b 100644 --- a/tests/Cache/FrameworkCacheConfigurationPassTest.php +++ b/tests/Cache/FrameworkCacheConfigurationPassTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use SymPress\Framework\Cache\FrameworkCacheConfigurationPass; +use SymPress\Framework\Cache\SignedMarshaller; use Symfony\Component\Cache\Adapter\ChainAdapter; use Symfony\Component\Cache\Adapter\TagAwareAdapter; use Symfony\Component\Cache\DependencyInjection\CachePoolPass; @@ -62,6 +63,30 @@ public function testRegistersDefaultAndCustomCachePools(): void self::assertTrue($container->getDefinition('cache.marketing')->isPublic()); } + public function testApplicationPhpPoolsUseAuthenticatedStorageAndWrapCustomMarshallers(): void + { + $container = $this->container(); + $container->setParameter('framework.cache', [ + 'app' => 'cache.adapter.php_files', + 'pools' => [ + 'cache.custom' => ['adapter' => 'cache.adapter.system', 'marshaller' => 'custom.marshaller'], + ], + ]); + (new FrameworkCacheConfigurationPass())->process($container); + + foreach (['cache.app', 'cache.custom'] as $name) { + $pool = $container->getDefinition($name); + self::assertInstanceOf(ChildDefinition::class, $pool); + self::assertSame('cache.adapter.filesystem', $pool->getParent()); + } + $marshaller = $container->getDefinition('cache.custom')->getTag('cache.pool')[0]['marshaller']; + self::assertSame(SignedMarshaller::class, $container->getDefinition($marshaller)->getClass()); + self::assertSame('custom.marshaller', (string) $container->getDefinition($marshaller)->getArgument(2)); + $system = $container->getDefinition('cache.system'); + self::assertInstanceOf(ChildDefinition::class, $system); + self::assertSame('cache.adapter.system', $system->getParent()); + } + public function testRegistersFrameworkBundleCachePools(): void { $container = $this->container(); diff --git a/tests/Cache/SignedMarshallerTest.php b/tests/Cache/SignedMarshallerTest.php new file mode 100644 index 0000000..78c4214 --- /dev/null +++ b/tests/Cache/SignedMarshallerTest.php @@ -0,0 +1,43 @@ +marshall(['value' => new SignedCacheFixture()], $failed); + SignedCacheFixture::$wakes = 0; + foreach ([serialize(new SignedCacheFixture()), $encoded['value'] . 'tampered'] as $untrusted) { + try { + $marshaller->unmarshall($untrusted); + self::fail('Unsigned/tampered payload was accepted.'); + } catch (CacheException) { + self::assertSame(0, SignedCacheFixture::$wakes); + } + } + self::assertInstanceOf(SignedCacheFixture::class, $marshaller->unmarshall($encoded['value'])); + self::assertSame(1, SignedCacheFixture::$wakes); + $this->expectException(CacheException::class); + (new SignedMarshaller(str_repeat('test-secret-', 4), 'other-project', new DefaultMarshaller()))->unmarshall($encoded['value']); + } +} + +final class SignedCacheFixture +{ + public static int $wakes = 0; + + public function __wakeup(): void + { + self::$wakes++; + } +} diff --git a/tests/Fixtures/cache-secret-upgrade.php b/tests/Fixtures/cache-secret-upgrade.php new file mode 100644 index 0000000..2cfa3d6 --- /dev/null +++ b/tests/Fixtures/cache-secret-upgrade.php @@ -0,0 +1,77 @@ + $arguments */ +$arguments = $_SERVER['argv'] ?? []; +$directory = $arguments[1]; +$mode = $arguments[2]; +$poison = ($arguments[3] ?? null) === 'poison'; +if ($poison) { + foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($directory . '/pools/app', FilesystemIterator::SKIP_DOTS)) as $file) { + if (!$file->isFile()) { + continue; + } + $bytes = file_get_contents($file->getPathname()); + $offset = strpos($bytes, 'sympress-symfony-cache-v1:'); + if ($offset !== false) { + file_put_contents($file->getPathname(), substr($bytes, 0, $offset) . serialize((object) ['untrusted' => true])); + } + } + $pdo = new PDO('sqlite:' . $directory . '/payload.sqlite'); + $query = $pdo->prepare('UPDATE cache_items SET item_data = ?'); + $query->execute([serialize((object) ['untrusted' => true])]); +} +unset($_ENV['APP_SECRET'], $_SERVER['APP_SECRET']); +putenv('APP_SECRET'); +if ($mode !== 'missing') { + $_ENV['APP_SECRET'] = $mode === 'short' ? 'old-short-secret' : str_repeat('test-secret-', 4); +} +$_ENV['SYMPRESS_PROJECT_DIR'] = $directory; +$kernel = new class ($root, 'test', false, $directory) extends AbstractKernel { + public function __construct(string $root, string $environment, bool $debug, private readonly string $directory) + { + parent::__construct($root, $environment, $debug); + } + + public function getCacheDir(): string + { + return $this->directory; + } +}; +$container = $kernel->createContainer(); +$registry = (new BundleRegistry())->add(new BundleMetadata('sympress/framework-bundle', 'library', 'framework-bundle', $root, $root . '/composer.json', new SymPressFrameworkBundle())); +$files = $kernel->configureContainer($container->builder(), $container, $registry); +$container->builder()->setParameter('framework.cache', ['pools' => [ + 'cache.files' => ['adapter' => 'cache.adapter.filesystem', 'public' => true], + 'cache.sqlite' => ['adapter' => 'cache.adapter.pdo', 'provider' => 'sqlite:' . $directory . '/payload.sqlite', 'public' => true], + 'cache.apcu' => ['adapter' => 'cache.adapter.apcu', 'public' => true], +]]); +$kernel->createRuntimeContainer($container, $registry, $files); +$hits = []; +$apcuTamperMiss = false; +foreach (['cache.app', 'cache.files', 'cache.sqlite', 'cache.apcu'] as $name) { + $pool = $container->get($name); + $item = $pool->getItem('upgrade-value'); + $hits[$name] = $item->isHit(); + $pool->save($item->set('current')); + if ($poison && $name === 'cache.apcu') { + foreach (apcu_cache_info()['cache_list'] as $entry) { + $payload = apcu_fetch($entry['info']); + if (is_string($payload) && str_starts_with($payload, 'sympress-symfony-cache-v1:')) { + apcu_store($entry['info'], serialize((object) ['untrusted' => true])); + } + } + $apcuTamperMiss = !$pool->getItem('upgrade-value')->isHit(); + } +} +$system = $container->get('cache.system'); +$saved = $system->save($system->getItem('compiled')->set(['metadata' => 'current'])); +echo json_encode(['hits' => $hits, 'system' => $saved, 'apcu_tamper_miss' => $apcuTamperMiss], JSON_THROW_ON_ERROR); diff --git a/tests/Kernel/SymPressFrameworkBundleTest.php b/tests/Kernel/SymPressFrameworkBundleTest.php index b288273..6e46c9e 100644 --- a/tests/Kernel/SymPressFrameworkBundleTest.php +++ b/tests/Kernel/SymPressFrameworkBundleTest.php @@ -10,6 +10,7 @@ use SymPress\Kernel\Bundle\BundleRegistry; use SymPress\Kernel\Kernel\AbstractKernel; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Process\Process; final class SymPressFrameworkBundleTest extends TestCase { @@ -56,6 +57,38 @@ public function testBuildsRuntimeContainerWithSymfonyFrameworkBundleServices(): self::assertTrue($container->has('cache.app')); } + public function testMissingAndShortSecretsUseRequestCacheOnExistingKernelAndStrongValuesPersist(): void + { + foreach (['missing', 'short', 'strong'] as $mode) { + $directory = $this->cacheDir . '/' . $mode; + $run = new Process([PHP_BINARY, '-d', 'apc.enable_cli=1', dirname(__DIR__) . '/Fixtures/cache-secret-upgrade.php', $directory, $mode]); + $run->mustRun(); + $first = json_decode($run->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(['cache.app' => false, 'cache.files' => false, 'cache.sqlite' => false, 'cache.apcu' => false], $first['hits']); + self::assertTrue($first['system']); + $run->mustRun(); + $second = json_decode($run->getOutput(), true, flags: JSON_THROW_ON_ERROR); + foreach (['cache.app', 'cache.files', 'cache.sqlite'] as $pool) { + self::assertSame($mode === 'strong', $second['hits'][$pool], $mode . '/' . $pool); + } + if ($mode !== 'strong') { + self::assertDirectoryDoesNotExist($directory . '/pools/app'); + self::assertFileDoesNotExist($directory . '/payload.sqlite'); + } + } + } + + public function testUnsignedExistingApplicationCacheEntriesAreRejectedBeforeRestoration(): void + { + $run = new Process([PHP_BINARY, '-d', 'apc.enable_cli=1', dirname(__DIR__) . '/Fixtures/cache-secret-upgrade.php', $this->cacheDir, 'strong']); + $run->mustRun(); + $run = new Process([PHP_BINARY, '-d', 'apc.enable_cli=1', dirname(__DIR__) . '/Fixtures/cache-secret-upgrade.php', $this->cacheDir, 'strong', 'poison']); + $run->mustRun(); + $result = json_decode($run->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(['cache.app' => false, 'cache.files' => false, 'cache.sqlite' => false, 'cache.apcu' => false], $result['hits']); + self::assertTrue($result['apcu_tamper_miss']); + } + private function kernel(string $projectDir): AbstractKernel { return new class ($projectDir, 'test', false, $this->cacheDir) extends AbstractKernel {