Skip to content

Commit 8a335c8

Browse files
SanderMullerclaude
SanderMuller
and
claude
committed
Keep only the relevant environment variables in the container cache key
The container cache key included the entire environment (minus a hand-maintained denylist - #4924, #4925), so any unrelated env var that differs between invocations (CI runners inject per-job vars on every run) flipped the key and forced a full container recompile + ignoreErrors re-validation. The container only depends on the environment through (a) %env.X% references in loaded configs and (b) env read at build time by a CompilerExtension (PHPStan reads only PHPSTAN_FNSR, in FnsrExtension::beforeCompile()). Keep exactly those env vars in the cache key - the %env.NAME% names (matching Nette's %([\w.-]*)% grammar) plus self::BUILD_TIME_ENV_VARIABLES - and drop the rest. The container parameters still carry the full env, so %env.*% resolution is unaffected; this only narrows the key. ContainerCacheKeyEnvGuardTest enforces that every env var read at build time by a CompilerExtension is declared, so a future build-time getenv() cannot silently reuse a stale container. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5bcff71 commit 8a335c8

6 files changed

Lines changed: 332 additions & 0 deletions

File tree

‎src/DependencyInjection/Configurator.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,25 @@
1212
use PHPStan\File\FileReader;
1313
use PHPStan\File\FileWriter;
1414
use PHPStan\Turbo\TurboExtensionEnabler;
15+
use function array_fill_keys;
16+
use function array_intersect_key;
1517
use function array_keys;
1618
use function count;
1719
use function error_reporting;
1820
use function explode;
21+
use function file_get_contents;
1922
use function hash_file;
2023
use function implode;
2124
use function in_array;
2225
use function is_dir;
2326
use function is_file;
2427
use function ksort;
28+
use function preg_match;
29+
use function preg_match_all;
2530
use function restore_error_handler;
2631
use function set_error_handler;
2732
use function sprintf;
33+
use function str_contains;
2834
use function str_ends_with;
2935
use function substr;
3036
use function time;
@@ -38,6 +44,14 @@
3844
final class Configurator extends \Nette\Bootstrap\Configurator
3945
{
4046

47+
/**
48+
* Environment variables PHPStan reads at build time inside a CompilerExtension, so the compiled
49+
* container depends on them and they must stay in the container cache key. FnsrExtension reads
50+
* PHPSTAN_FNSR in beforeCompile(). Enforced by ContainerCacheKeyEnvGuardTest, which fails if any
51+
* CompilerExtension reads an env var that is neither listed here nor referenced via %env.*%.
52+
*/
53+
public const BUILD_TIME_ENV_VARIABLES = ['PHPSTAN_FNSR'];
54+
4155
/** @var string[] */
4256
private array $allConfigFiles = [];
4357

@@ -97,6 +111,23 @@ public function loadContainer(): string
97111
// make sure invocations via blackfire use the same container
98112
unset($staticParameters['env']['BLACKFIRE_AGENT_SOCKET']);
99113

114+
// The container only depends on the environment through (a) %env.X% references in loaded
115+
// configs and (b) env read at build time inside a CompilerExtension (PHPStan itself reads only
116+
// PHPSTAN_FNSR, in FnsrExtension). Keep exactly those env vars in the cache key and drop the
117+
// rest, so unrelated env vars (CI/terminal/session-injected) no longer flip the key and force a
118+
// full container recompile + ignoreErrors re-validation - phpstan/phpstan#14072. (The container
119+
// parameters still carry the full env via $this->staticParameters, so %env.*% resolution is
120+
// unaffected; this only narrows the cache key.)
121+
if (isset($staticParameters['env'])) {
122+
$relevantEnvVariableNames = $this->relevantEnvVariableNamesForCacheKey();
123+
if ($relevantEnvVariableNames !== null) {
124+
$staticParameters['env'] = array_intersect_key(
125+
$staticParameters['env'],
126+
array_fill_keys($relevantEnvVariableNames, true),
127+
);
128+
}
129+
}
130+
100131
$containerKey = [
101132
$staticParameters,
102133
array_keys($this->dynamicParameters),
@@ -231,6 +262,45 @@ public function createContainer(bool $initialize = true): OriginalNetteContainer
231262
return $container;
232263
}
233264

265+
/**
266+
* Names of the environment variables whose value can change the generated container, so they must
267+
* stay in the container cache key: every %env.NAME% referenced across the loaded configs, plus the
268+
* env vars PHPStan reads at build time itself (self::BUILD_TIME_ENV_VARIABLES). All other env vars
269+
* are dropped from the key. Returns null when a config references the whole %env% array, in which
270+
* case the entire environment must be kept.
271+
*
272+
* @return list<string>|null
273+
*/
274+
private function relevantEnvVariableNamesForCacheKey(): ?array
275+
{
276+
$names = self::BUILD_TIME_ENV_VARIABLES;
277+
foreach ($this->allConfigFiles as $file) {
278+
$contents = @file_get_contents($file);
279+
if ($contents === false || !str_contains($contents, '%env')) {
280+
continue;
281+
}
282+
283+
// A bare %env% reference exposes the whole environment to the container; keep all of it.
284+
if (preg_match('~%env%~', $contents) === 1) {
285+
return null;
286+
}
287+
288+
// Match Nette's parameter-name grammar (%([\w.-]*)% in Config\Adapters\NeonAdapter), so a
289+
// valid reference like %env.MY-VAR% is not missed (which would drop MY-VAR from the key and
290+
// reuse a stale container on change). The %, space etc. terminating the name keep comment
291+
// prose like "%env.* foo" from being captured.
292+
if (preg_match_all('~%env\.([\w.-]+)%~', $contents, $matches) === 0) {
293+
continue;
294+
}
295+
296+
foreach ($matches[1] as $name) {
297+
$names[] = $name;
298+
}
299+
}
300+
301+
return $names;
302+
}
303+
234304
/**
235305
* @return string[]
236306
*/

‎tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\DependencyInjection;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use RecursiveDirectoryIterator;
7+
use RecursiveIteratorIterator;
8+
use SplFileInfo;
9+
use function file_get_contents;
10+
use function implode;
11+
use function in_array;
12+
use function preg_match_all;
13+
use function sort;
14+
use function str_contains;
15+
use const PHP_EOL;
16+
17+
/**
18+
* Guards the container-cache-key narrowing in Configurator (phpstan/phpstan#14072): when no config
19+
* references %env.*%, env vars are dropped from the cache key, so any env var a CompilerExtension
20+
* reads at *build time* (getenv()/$_ENV) must be declared in Configurator::BUILD_TIME_ENV_VARIABLES -
21+
* otherwise changing it would silently reuse a stale container. A new build-time env read in any
22+
* extension fails this test instead of re-opening that hole. Detection is intentionally simple, with
23+
* documented limits (acceptable - PHPStan's build-time env access goes through these patterns):
24+
* literal env-var names only (route dynamic ones via %env.*), getenv()/$_ENV reads (not $_SERVER),
25+
* and classes that directly extend CompilerExtension (not env read in a helper they delegate to).
26+
*/
27+
final class ContainerCacheKeyEnvGuardTest extends TestCase
28+
{
29+
30+
public function testBuildTimeEnvReadsInCompilerExtensionsAreDeclared(): void
31+
{
32+
$srcDir = __DIR__ . '/../../../src';
33+
$offenders = [];
34+
35+
/** @var SplFileInfo $file */
36+
foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($srcDir, RecursiveDirectoryIterator::SKIP_DOTS)) as $file) {
37+
if (!$file->isFile() || $file->getExtension() !== 'php') {
38+
continue;
39+
}
40+
41+
$contents = file_get_contents($file->getPathname());
42+
if ($contents === false || !str_contains($contents, 'extends CompilerExtension')) {
43+
continue;
44+
}
45+
46+
if (preg_match_all('~(?:getenv\(|\$_ENV\[)\s*[\'"]([A-Za-z_][A-Za-z0-9_]*)[\'"]~', $contents, $matches) === 0) {
47+
continue;
48+
}
49+
50+
foreach ($matches[1] as $envName) {
51+
if (in_array($envName, Configurator::BUILD_TIME_ENV_VARIABLES, true)) {
52+
continue;
53+
}
54+
55+
$offenders[] = $file->getFilename() . ': ' . $envName;
56+
}
57+
}
58+
59+
sort($offenders);
60+
61+
$this->assertSame(
62+
[],
63+
$offenders,
64+
'A CompilerExtension reads an environment variable at build time that is not declared in '
65+
. 'Configurator::BUILD_TIME_ENV_VARIABLES. Add it there (so it stays in the container cache '
66+
. 'key and a change recompiles), or read it via %env.* in a config instead:' . PHP_EOL
67+
. implode(PHP_EOL, $offenders),
68+
);
69+
}
70+
71+
}

‎tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\DependencyInjection;
4+
5+
use Override;
6+
use PHPStan\File\FileHelper;
7+
use PHPUnit\Framework\TestCase;
8+
use function count;
9+
use function getenv;
10+
use function glob;
11+
use function is_dir;
12+
use function md5;
13+
use function putenv;
14+
use function rmdir;
15+
use function sprintf;
16+
use function sys_get_temp_dir;
17+
use function uniqid;
18+
use function unlink;
19+
use const DIRECTORY_SEPARATOR;
20+
21+
final class ContainerCacheKeyEnvTest extends TestCase
22+
{
23+
24+
private const ENV_VAR = 'PHPSTAN_TEST_W1_CACHE_KEY';
25+
26+
private string $tmpDir;
27+
28+
private string|false $envBackup;
29+
30+
#[Override]
31+
protected function setUp(): void
32+
{
33+
$this->envBackup = getenv(self::ENV_VAR);
34+
$this->tmpDir = sys_get_temp_dir() . '/phpstan-container-cache-key-' . md5(uniqid());
35+
$this->removeDirectory($this->tmpDir);
36+
}
37+
38+
#[Override]
39+
protected function tearDown(): void
40+
{
41+
if ($this->envBackup === false) {
42+
putenv(self::ENV_VAR);
43+
} else {
44+
putenv(sprintf('%s=%s', self::ENV_VAR, $this->envBackup));
45+
}
46+
47+
$this->removeDirectory($this->tmpDir);
48+
}
49+
50+
public function testEnvironmentIsDroppedFromCacheKeyWhenNoConfigReferencesIt(): void
51+
{
52+
// No loaded config uses %env.*%, so an unrelated env change must NOT recompile the container.
53+
putenv(sprintf('%s=first', self::ENV_VAR));
54+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
55+
56+
putenv(sprintf('%s=second', self::ENV_VAR));
57+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
58+
59+
$this->assertSame(
60+
1,
61+
$this->countCompiledContainers(),
62+
'Changing an unrelated environment variable should reuse the cached container.',
63+
);
64+
}
65+
66+
public function testEnvironmentStaysInCacheKeyWhenAConfigReferencesIt(): void
67+
{
68+
// A loaded config uses %env.*%, so the environment is relevant and a change must recompile.
69+
putenv(sprintf('%s=first', self::ENV_VAR));
70+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon');
71+
72+
putenv(sprintf('%s=second', self::ENV_VAR));
73+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon');
74+
75+
$this->assertSame(
76+
2,
77+
$this->countCompiledContainers(),
78+
'When a config references %env.*%, changing it must recompile the container.',
79+
);
80+
}
81+
82+
public function testDashedEnvVariableReferenceStaysInCacheKey(): void
83+
{
84+
// %env.MY-VAR% is a valid Nette reference (parameter-name grammar %([\w.-]*)%). The env-name
85+
// extraction must keep dash/dot/digit-start names too - otherwise changing such a referenced
86+
// var would reuse a stale container, the regression class W1 fixes.
87+
$envName = 'PHPSTAN_TEST_W1-DASH';
88+
$backup = getenv($envName);
89+
try {
90+
putenv(sprintf('%s=first', $envName));
91+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');
92+
93+
putenv(sprintf('%s=second', $envName));
94+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');
95+
96+
$this->assertSame(
97+
2,
98+
$this->countCompiledContainers(),
99+
'A %env.* reference whose name contains a dash must keep that var in the cache key.',
100+
);
101+
} finally {
102+
if ($backup === false) {
103+
putenv($envName);
104+
} else {
105+
putenv(sprintf('%s=%s', $envName, $backup));
106+
}
107+
}
108+
}
109+
110+
public function testBuildTimeEnvVariableStaysInCacheKey(): void
111+
{
112+
// FnsrExtension reads getenv('PHPSTAN_FNSR') in beforeCompile() and rewires the container, so
113+
// the compiled container depends on it even though no config references %env.*%. Toggling it
114+
// must recompile - otherwise the change would be silently ignored (a stale container).
115+
$backup = getenv('PHPSTAN_FNSR');
116+
try {
117+
putenv('PHPSTAN_FNSR=0');
118+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
119+
120+
putenv('PHPSTAN_FNSR=1');
121+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
122+
123+
$this->assertSame(
124+
2,
125+
$this->countCompiledContainers(),
126+
'Changing PHPSTAN_FNSR (read at build time by FnsrExtension) must recompile the container.',
127+
);
128+
} finally {
129+
if ($backup === false) {
130+
putenv('PHPSTAN_FNSR');
131+
} else {
132+
putenv(sprintf('PHPSTAN_FNSR=%s', $backup));
133+
}
134+
}
135+
}
136+
137+
private function buildContainer(string $additionalConfigFile): void
138+
{
139+
$rootDir = __DIR__ . '/../../..';
140+
$fileHelper = new FileHelper($rootDir);
141+
$rootDir = $fileHelper->normalizePath($rootDir, '/');
142+
$containerFactory = new ContainerFactory($rootDir);
143+
$containerFactory->create(
144+
$this->tmpDir,
145+
[
146+
$containerFactory->getConfigDirectory() . '/config.level8.neon',
147+
$additionalConfigFile,
148+
],
149+
[],
150+
);
151+
}
152+
153+
private function countCompiledContainers(): int
154+
{
155+
$containers = glob($this->tmpDir . '/cache/nette.configurator/Container_*.php');
156+
157+
return $containers === false ? 0 : count($containers);
158+
}
159+
160+
private function removeDirectory(string $directory): void
161+
{
162+
if (!is_dir($directory)) {
163+
return;
164+
}
165+
166+
$entries = glob($directory . DIRECTORY_SEPARATOR . '*');
167+
foreach ($entries === false ? [] : $entries as $entry) {
168+
if (is_dir($entry)) {
169+
$this->removeDirectory($entry);
170+
} else {
171+
@unlink($entry);
172+
}
173+
}
174+
175+
@rmdir($directory);
176+
}
177+
178+
}

‎tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
parameters:
2+
customRulesetUsed: true

‎tests/PHPStan/DependencyInjection/containerCacheKey-withenv-dashed.neon‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# References an env var whose name contains a dash (%env.PHPSTAN_TEST_W1-DASH%) - a valid Nette
2+
# reference (Nette's parameter-name grammar is %([\w.-]*)%). The env-name extraction must keep such
3+
# names in the container cache key too, otherwise changing this var would reuse a stale container.
4+
parameters:
5+
tmpDir: %env.PHPSTAN_TEST_W1-DASH%

‎tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# References an environment variable via %env.<name>%, so that env var must stay in the container
2+
# cache key - otherwise changing it would wrongly reuse a stale container. The resolved tmpDir value
3+
# is irrelevant here (ContainerFactory::create() overrides tmpDir with its own argument); the
4+
# reference itself is what keeps PHPSTAN_TEST_W1_CACHE_KEY in the cache key.
5+
parameters:
6+
tmpDir: %env.PHPSTAN_TEST_W1_CACHE_KEY%

0 commit comments

Comments
 (0)