From 97868c32640a9875f1f6f0e4d215d2ad6655e65a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 16 Oct 2025 09:08:54 +0200 Subject: [PATCH] feat: allow safe environment variable defaults in array-format volumes Changes: - Extended validateDockerComposeForInjection to recognize env vars with defaults - Added pattern check for ${VAR:-default} format alongside simple ${VAR} check - Maintains consistency with parseDockerVolumeString behavior for string format Test coverage: - Added test for safe environment variable defaults in array format - Verifies ${DATA_PATH:-./data} is allowed in array-format volumes - All 79 security tests pass (215 assertions) This allows users to specify environment variables with safe default values in array-format Docker Compose volumes, matching the behavior already supported in string-format volumes. --- bootstrap/helpers/parsers.php | 5 ++++- tests/Unit/VolumeArrayFormatSecurityTest.php | 23 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index f3d71c83d..f2260f0c6 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -59,8 +59,11 @@ function validateDockerComposeForInjection(string $composeYaml): void if (isset($volume['source'])) { $source = $volume['source']; if (is_string($source)) { + // Allow simple env vars and env vars with defaults (validated in parseDockerVolumeString) $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source); - if (! $isSimpleEnvVar) { + $isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $source); + + if (! $isSimpleEnvVar && ! $isEnvVarWithDefault) { try { validateShellSafePath($source, 'volume source'); } catch (\Exception $e) { diff --git a/tests/Unit/VolumeArrayFormatSecurityTest.php b/tests/Unit/VolumeArrayFormatSecurityTest.php index b8e5a7f7a..97a6819b2 100644 --- a/tests/Unit/VolumeArrayFormatSecurityTest.php +++ b/tests/Unit/VolumeArrayFormatSecurityTest.php @@ -171,6 +171,29 @@ expect($isSimpleEnvVar)->toBe(1); // preg_match returns 1 on success, not true }); +test('array-format with safe environment variable default', function () { + $dockerComposeYaml = <<<'YAML' +services: + web: + image: nginx + volumes: + - type: bind + source: '${DATA_PATH:-./data}' + target: /app/data +YAML; + + $parsed = Yaml::parse($dockerComposeYaml); + $source = $parsed['services']['web']['volumes'][0]['source']; + + // Parse correctly extracts the source value + expect($source)->toBe('${DATA_PATH:-./data}'); + + // Safe environment variable with benign default should be allowed + // The pre-save validation skips env vars with safe defaults + expect(fn () => validateDockerComposeForInjection($dockerComposeYaml)) + ->not->toThrow(Exception::class); +}); + test('array-format with malicious environment variable default', function () { $dockerComposeYaml = <<<'YAML' services: