diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 7cc084530..f3d71c83d 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -104,53 +104,8 @@ function validateDockerComposeForInjection(string $composeYaml): void */ function validateVolumeStringForInjection(string $volumeString): void { - // Parse the volume string to extract source and target - $parts = explode(':', $volumeString); - if (count($parts) < 2) { - // Named volume without target - only validate the name - try { - validateShellSafePath($parts[0], 'volume name'); - } catch (\Exception $e) { - throw new \Exception( - 'Invalid Docker volume definition: '.$e->getMessage(). - ' Please use safe names without shell metacharacters.', - 0, - $e - ); - } - - return; - } - - $source = $parts[0]; - $target = $parts[1]; - - // Validate source (but allow simple environment variables) - $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source); - if (! $isSimpleEnvVar) { - try { - validateShellSafePath($source, 'volume source'); - } catch (\Exception $e) { - throw new \Exception( - 'Invalid Docker volume definition: '.$e->getMessage(). - ' Please use safe path names without shell metacharacters.', - 0, - $e - ); - } - } - - // Validate target - try { - validateShellSafePath($target, 'volume target'); - } catch (\Exception $e) { - throw new \Exception( - 'Invalid Docker volume definition: '.$e->getMessage(). - ' Please use safe path names without shell metacharacters.', - 0, - $e - ); - } + // Canonical parsing also validates and throws on unsafe input + parseDockerVolumeString($volumeString); } function parseDockerVolumeString(string $volumeString): array diff --git a/tests/Unit/VolumeArrayFormatSecurityTest.php b/tests/Unit/VolumeArrayFormatSecurityTest.php index f52c9bd11..b8e5a7f7a 100644 --- a/tests/Unit/VolumeArrayFormatSecurityTest.php +++ b/tests/Unit/VolumeArrayFormatSecurityTest.php @@ -168,7 +168,7 @@ expect($source)->toBe('${DATA_PATH}'); // Our validation allows simple env var references $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source); - expect($isSimpleEnvVar)->toBeTrue(); + expect($isSimpleEnvVar)->toBe(1); // preg_match returns 1 on success, not true }); test('array-format with malicious environment variable default', function () { diff --git a/tests/Unit/WindowsPathVolumeTest.php b/tests/Unit/WindowsPathVolumeTest.php new file mode 100644 index 000000000..1d3af6abd --- /dev/null +++ b/tests/Unit/WindowsPathVolumeTest.php @@ -0,0 +1,64 @@ +toBe('C:\\host\\path'); + expect((string) $result['target'])->toBe('/container'); +}); + +test('validateVolumeStringForInjection correctly handles Windows paths via parseDockerVolumeString', function () { + $windowsVolume = 'C:\\Users\\Data:/app/data'; + + // Should not throw an exception + validateVolumeStringForInjection($windowsVolume); + + // If we get here, the test passed + expect(true)->toBeTrue(); +}); + +test('validateVolumeStringForInjection rejects malicious Windows-like paths', function () { + $maliciousVolume = 'C:\\host\\`whoami`:/container'; + + expect(fn () => validateVolumeStringForInjection($maliciousVolume)) + ->toThrow(\Exception::class); +}); + +test('validateDockerComposeForInjection handles Windows paths in compose files', function () { + $dockerComposeYaml = <<<'YAML' +services: + web: + image: nginx + volumes: + - C:\Users\Data:/app/data +YAML; + + // Should not throw an exception + validateDockerComposeForInjection($dockerComposeYaml); + + expect(true)->toBeTrue(); +}); + +test('validateDockerComposeForInjection rejects Windows paths with injection', function () { + $dockerComposeYaml = <<<'YAML' +services: + web: + image: nginx + volumes: + - C:\Users\$(whoami):/app/data +YAML; + + expect(fn () => validateDockerComposeForInjection($dockerComposeYaml)) + ->toThrow(\Exception::class); +}); + +test('Windows paths with complex paths and spaces are handled correctly', function () { + $windowsVolume = 'C:\\Program Files\\MyApp:/app'; + + $result = parseDockerVolumeString($windowsVolume); + + expect((string) $result['source'])->toBe('C:\\Program Files\\MyApp'); + expect((string) $result['target'])->toBe('/app'); +});