Skip to content

Commit c373f1f

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 c373f1f

6 files changed

Lines changed: 318 additions & 0 deletions

File tree

‎src/DependencyInjection/Configurator.php‎

Lines changed: 61 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,13 @@
3844
final class Configurator extends \Nette\Bootstrap\Configurator
3945
{
4046

47+
/**
48+
* Env vars PHPStan reads at build time in a CompilerExtension (FnsrExtension reads PHPSTAN_FNSR),
49+
* so the container depends on them and they must stay in the cache key. ContainerCacheKeyEnvGuardTest
50+
* fails if a CompilerExtension reads an env var not listed here or referenced via %env.*%.
51+
*/
52+
public const BUILD_TIME_ENV_VARIABLES = ['PHPSTAN_FNSR'];
53+
4154
/** @var string[] */
4255
private array $allConfigFiles = [];
4356

@@ -97,6 +110,19 @@ public function loadContainer(): string
97110
// make sure invocations via blackfire use the same container
98111
unset($staticParameters['env']['BLACKFIRE_AGENT_SOCKET']);
99112

113+
// Keep only the env vars the container actually depends on in the cache key, so unrelated env
114+
// changes (CI/shell) don't force a full recompile - phpstan/phpstan#14072. The full env stays
115+
// in the container parameters, so %env.*% resolution is unaffected; this only narrows the key.
116+
if (isset($staticParameters['env'])) {
117+
$relevantEnvVariableNames = $this->relevantEnvVariableNamesForCacheKey();
118+
if ($relevantEnvVariableNames !== null) {
119+
$staticParameters['env'] = array_intersect_key(
120+
$staticParameters['env'],
121+
array_fill_keys($relevantEnvVariableNames, true),
122+
);
123+
}
124+
}
125+
100126
$containerKey = [
101127
$staticParameters,
102128
array_keys($this->dynamicParameters),
@@ -231,6 +257,41 @@ public function createContainer(bool $initialize = true): OriginalNetteContainer
231257
return $container;
232258
}
233259

260+
/**
261+
* Env vars that can change the generated container, so they must stay in the cache key: every
262+
* %env.NAME% referenced across the loaded configs, plus BUILD_TIME_ENV_VARIABLES. Returns null
263+
* when a config references the whole %env% array, in which case all of it must be kept.
264+
*
265+
* @return list<string>|null
266+
*/
267+
private function relevantEnvVariableNamesForCacheKey(): ?array
268+
{
269+
$names = self::BUILD_TIME_ENV_VARIABLES;
270+
foreach ($this->allConfigFiles as $file) {
271+
$contents = @file_get_contents($file);
272+
if ($contents === false || !str_contains($contents, '%env')) {
273+
continue;
274+
}
275+
276+
// A bare %env% reference exposes the whole environment to the container; keep all of it.
277+
if (preg_match('~%env%~', $contents) === 1) {
278+
return null;
279+
}
280+
281+
// Match Nette's parameter-name grammar (%([\w.-]*)% in NeonAdapter) so a valid reference like
282+
// %env.MY-VAR% is not missed, which would drop MY-VAR from the key and reuse a stale container.
283+
if (preg_match_all('~%env\.([\w.-]+)%~', $contents, $matches) === 0) {
284+
continue;
285+
}
286+
287+
foreach ($matches[1] as $name) {
288+
$names[] = $name;
289+
}
290+
}
291+
292+
return $names;
293+
}
294+
234295
/**
235296
* @return string[]
236297
*/

‎tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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+
* An env var a CompilerExtension reads at build time must be declared in
19+
* Configurator::BUILD_TIME_ENV_VARIABLES, or narrowing the container cache key would silently reuse a
20+
* stale container when it changes (phpstan/phpstan#14072). This fails on any undeclared build-time
21+
* read. Detection is deliberately simple: literal getenv()/$_ENV names in classes that directly
22+
* extend CompilerExtension.
23+
*/
24+
final class ContainerCacheKeyEnvGuardTest extends TestCase
25+
{
26+
27+
public function testBuildTimeEnvReadsInCompilerExtensionsAreDeclared(): void
28+
{
29+
$srcDir = __DIR__ . '/../../../src';
30+
$offenders = [];
31+
32+
/** @var SplFileInfo $file */
33+
foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($srcDir, RecursiveDirectoryIterator::SKIP_DOTS)) as $file) {
34+
if (!$file->isFile() || $file->getExtension() !== 'php') {
35+
continue;
36+
}
37+
38+
$contents = file_get_contents($file->getPathname());
39+
if ($contents === false || !str_contains($contents, 'extends CompilerExtension')) {
40+
continue;
41+
}
42+
43+
if (preg_match_all('~(?:getenv\(|\$_ENV\[)\s*[\'"]([A-Za-z_][A-Za-z0-9_]*)[\'"]~', $contents, $matches) === 0) {
44+
continue;
45+
}
46+
47+
foreach ($matches[1] as $envName) {
48+
if (in_array($envName, Configurator::BUILD_TIME_ENV_VARIABLES, true)) {
49+
continue;
50+
}
51+
52+
$offenders[] = $file->getFilename() . ': ' . $envName;
53+
}
54+
}
55+
56+
sort($offenders);
57+
58+
$this->assertSame(
59+
[],
60+
$offenders,
61+
'A CompilerExtension reads an environment variable at build time that is not declared in '
62+
. 'Configurator::BUILD_TIME_ENV_VARIABLES. Add it there (so it stays in the container cache '
63+
. 'key and a change recompiles), or read it via %env.* in a config instead:' . PHP_EOL
64+
. implode(PHP_EOL, $offenders),
65+
);
66+
}
67+
68+
}

‎tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
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 (grammar %([\w.-]*)%): extraction must keep dashed
85+
// names too, otherwise changing the referenced var would reuse a stale container.
86+
$envName = 'PHPSTAN_TEST_W1-DASH';
87+
$backup = getenv($envName);
88+
try {
89+
putenv(sprintf('%s=first', $envName));
90+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');
91+
92+
putenv(sprintf('%s=second', $envName));
93+
$this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon');
94+
95+
$this->assertSame(
96+
2,
97+
$this->countCompiledContainers(),
98+
'A %env.* reference whose name contains a dash must keep that var in the cache key.',
99+
);
100+
} finally {
101+
if ($backup === false) {
102+
putenv($envName);
103+
} else {
104+
putenv(sprintf('%s=%s', $envName, $backup));
105+
}
106+
}
107+
}
108+
109+
public function testBuildTimeEnvVariableStaysInCacheKey(): void
110+
{
111+
// FnsrExtension reads getenv('PHPSTAN_FNSR') at build time and rewires the container, so it
112+
// depends on the var even with no %env.*% in config; toggling it must recompile.
113+
$backup = getenv('PHPSTAN_FNSR');
114+
try {
115+
putenv('PHPSTAN_FNSR=0');
116+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
117+
118+
putenv('PHPSTAN_FNSR=1');
119+
$this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon');
120+
121+
$this->assertSame(
122+
2,
123+
$this->countCompiledContainers(),
124+
'Changing PHPSTAN_FNSR (read at build time by FnsrExtension) must recompile the container.',
125+
);
126+
} finally {
127+
if ($backup === false) {
128+
putenv('PHPSTAN_FNSR');
129+
} else {
130+
putenv(sprintf('PHPSTAN_FNSR=%s', $backup));
131+
}
132+
}
133+
}
134+
135+
private function buildContainer(string $additionalConfigFile): void
136+
{
137+
$rootDir = __DIR__ . '/../../..';
138+
$fileHelper = new FileHelper($rootDir);
139+
$rootDir = $fileHelper->normalizePath($rootDir, '/');
140+
$containerFactory = new ContainerFactory($rootDir);
141+
$containerFactory->create(
142+
$this->tmpDir,
143+
[
144+
$containerFactory->getConfigDirectory() . '/config.level8.neon',
145+
$additionalConfigFile,
146+
],
147+
[],
148+
);
149+
}
150+
151+
private function countCompiledContainers(): int
152+
{
153+
$containers = glob($this->tmpDir . '/cache/nette.configurator/Container_*.php');
154+
155+
return $containers === false ? 0 : count($containers);
156+
}
157+
158+
private function removeDirectory(string $directory): void
159+
{
160+
if (!is_dir($directory)) {
161+
return;
162+
}
163+
164+
$entries = glob($directory . DIRECTORY_SEPARATOR . '*');
165+
foreach ($entries === false ? [] : $entries as $entry) {
166+
if (is_dir($entry)) {
167+
$this->removeDirectory($entry);
168+
} else {
169+
@unlink($entry);
170+
}
171+
}
172+
173+
@rmdir($directory);
174+
}
175+
176+
}

‎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)