From cfe8a800cf56544b6697315cc0b3191a8b2d001c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:34:38 +0200 Subject: [PATCH] Improve raw Compose volume path handling --- app/Models/Application.php | 9 +- bootstrap/helpers/shared.php | 38 ++++ tests/Unit/RawComposeVolumePathTest.php | 261 ++++++++++++++++++++++++ 3 files changed, 305 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/RawComposeVolumePathTest.php diff --git a/app/Models/Application.php b/app/Models/Application.php index 264d6e38b0..3ef2b28813 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -2103,6 +2103,7 @@ class Application extends BaseModel $source = data_get_str($volume, 'source'); } if ($type?->value() === 'bind') { + $source = str($source); if ($source->value() === '/var/run/docker.sock') { continue; } @@ -2110,10 +2111,12 @@ class Application extends BaseModel continue; } if ($source->startsWith('.')) { - $source = $source->after('.'); - $source = $workdir.$source; + $source = str($workdir.$source->after('.')); + } + $mkdirCommand = rawComposeBindMkdirCommand($source->value()); + if ($mkdirCommand !== null) { + $commands->push($mkdirCommand); } - $commands->push("mkdir -p $source > /dev/null 2>&1 || true"); } } } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index dbe8814bd7..009166e5ab 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -165,6 +165,44 @@ function validateShellSafePath(string $input, string $context = 'path'): string return $input; } +/** + * Build the remote mkdir command for a raw Compose bind volume source. + * + * Keep volume paths as single arguments when creating bind directories. + * + * Compose environment interpolations are left to Docker Compose. They are + * not expanded by the destination server shell. + * + * @throws Exception If the source is invalid + */ +function rawComposeBindMkdirCommand(string $source): ?string +{ + if (preg_match('/[\x00-\x1F\x7F]/', $source)) { + throw new Exception('Invalid volume source: contains a control character.'); + } + + $source = trim($source); + if ($source === '') { + throw new Exception('Invalid volume source: path is empty.'); + } + + $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source) === 1; + $isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}(?:\/[\w.\-]+)*\/?$/', $source) === 1; + if ($isSimpleEnvVar || $isEnvVarWithPath) { + return null; + } + + if (preg_match('/^\$\{([a-zA-Z_][a-zA-Z0-9_]*):-(.*)\}$/', $source, $matches) === 1) { + validateShellSafePath($matches[2], 'volume source'); + + return null; + } + + validateShellSafePath($source, 'volume source'); + + return 'mkdir -p -- '.escapeshellarg($source).' > /dev/null 2>&1 || true'; +} + /** * Validate that a filename is safe for use as a plain file name (no path components). * diff --git a/tests/Unit/RawComposeVolumePathTest.php b/tests/Unit/RawComposeVolumePathTest.php new file mode 100644 index 0000000000..b486f63ecd --- /dev/null +++ b/tests/Unit/RawComposeVolumePathTest.php @@ -0,0 +1,261 @@ +uuid = 'compose-volume-test'; + $application->docker_compose_raw = $compose; + + $server = new Server; + $server->ip = '127.0.0.1'; + $server->user = 'root'; + + $destination = new StandaloneDocker; + $destination->setRelation('server', $server); + $application->setRelation('destination', $destination); + + return $application; +} + +function rawComposeBindSource(string $source, string $syntax = 'long'): string +{ + if ($syntax === 'short') { + $volume = trim($source, '"'); + + return << $application->oldRawParser()) + ->toThrow(Exception::class, 'forbidden character'); + + Process::assertNothingRan(); +}); + +it('rejects invalid short-form bind sources before remote work', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('"/srv/data;extra"', 'short')); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class, 'forbidden character'); + + Process::assertNothingRan(); +}); + +it('rejects unsupported expansion syntax in long-form bind sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('"/srv/data$(value)"')); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class, 'command substitution'); + + Process::assertNothingRan(); +}); + +it('rejects unsupported quoted syntax in long-form bind sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('"/srv/data`value`"')); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class, 'backtick'); + + Process::assertNothingRan(); +}); + +it('rejects unsupported operators in bind sources', function (string $source) { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource($source)); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class, 'forbidden character'); + + Process::assertNothingRan(); +})->with([ + '"/srv/data | extra"', + '"/srv/data && extra"', + '"/srv/data || extra"', + '"/srv/data > extra"', + '"/srv/data < extra"', +]); + +it('rejects newlines and control characters in volume sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(<<<'YAML' +services: + web: + image: nginx:alpine + volumes: + - type: bind + source: "/srv/data +extra" + target: /usr/share/nginx/html +YAML); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class); + + Process::assertNothingRan(); +}); + +it('rejects invalid Compose variable defaults in bind sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('"${DATA:-$(value)}"')); + + expect(fn () => $application->oldRawParser()) + ->toThrow(Exception::class); + + Process::assertNothingRan(); +}); + +it('quotes safe absolute bind sources at the mkdir execution boundary', function () { + expect(rawComposeBindMkdirCommand('/srv/appdata')) + ->toBe("mkdir -p -- '/srv/appdata' > /dev/null 2>&1 || true"); + + expect(fn () => rawComposeBindMkdirCommand('/srv/appdata;extra')) + ->toThrow(Exception::class); +}); + +it('quotes paths that begin with a command option', function () { + expect(rawComposeBindMkdirCommand('-evil')) + ->toBe("mkdir -p -- '-evil' > /dev/null 2>&1 || true"); +}); + +it('rejects blank and null-byte bind sources before a remote command runs', function (string $source) { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource(json_encode($source))); + + expect(fn () => $application->oldRawParser())->toThrow(Exception::class, 'Invalid volume source'); + Process::assertNothingRan(); +})->with([' ', "\0"]); + +it('quotes quotes and backslashes so they cannot change the mkdir command', function () { + expect(rawComposeBindMkdirCommand("/tmp/foo'bar")) + ->toBe("mkdir -p -- '/tmp/foo'\\''bar' > /dev/null 2>&1 || true") + ->and(rawComposeBindMkdirCommand('/tmp/foo\\bar')) + ->toBe("mkdir -p -- '/tmp/foo\\bar' > /dev/null 2>&1 || true") + ->and(rawComposeBindMkdirCommand('$HOME')) + ->toBe("mkdir -p -- '\$HOME' > /dev/null 2>&1 || true"); +}); + +it('does not expand compose environment interpolations through the server shell', function () { + expect(rawComposeBindMkdirCommand('${DATA_PATH}'))->toBeNull() + ->and(rawComposeBindMkdirCommand('${DATA_PATH}/mysql'))->toBeNull() + ->and(rawComposeBindMkdirCommand('${DATA_PATH:-/srv/appdata}'))->toBeNull(); +}); + +it('preserves legitimate long-form bind mounts', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('"/srv/appdata"')); + + $application->oldRawParser(); + + expect($application->docker_compose_raw)->toContain('/srv/appdata'); +}); + +it('preserves legitimate short-form relative bind mounts', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('./data', 'short')); + + $application->oldRawParser(); + + expect($application->docker_compose_raw)->toContain('./data'); +}); + +it('preserves nested relative bind mounts', function () { + Process::fake(); + + $application = makeRawComposeApplication(rawComposeBindSource('./data/nested/dir', 'short')); + + $application->oldRawParser(); + + expect($application->docker_compose_raw)->toContain('./data/nested/dir'); +}); + +it('does not treat named volumes as bind sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(<<<'YAML' +services: + web: + image: nginx:alpine + volumes: + - appdata:/var/lib/data +volumes: + appdata: +YAML); + + $application->oldRawParser(); + + expect($application->docker_compose_raw)->toContain('appdata:/var/lib/data'); +}); + +it('does not treat long-form named volumes as bind sources', function () { + Process::fake(); + + $application = makeRawComposeApplication(<<<'YAML' +services: + web: + image: nginx:alpine + volumes: + - type: volume + source: appdata + target: /var/lib/data +volumes: + appdata: +YAML); + + $application->oldRawParser(); + + expect($application->docker_compose_raw)->toContain('appdata'); +}); + +it('still uses oldRawParser for raw compose deployments after git compose load', function () { + $jobSource = file_get_contents(base_path('app/Jobs/ApplicationDeploymentJob.php')); + $parserSource = file_get_contents(base_path('app/Models/Application.php')); + + expect($jobSource) + ->toContain('loadComposeFile(isInit: false)') + ->toContain('is_raw_compose_deployment_enabled') + ->toContain('oldRawParser()') + ->and($parserSource) + ->toContain('rawComposeBindMkdirCommand(') + ->toContain('escapeshellarg'); +});