From 97f451cc2a2b0ad8f95d9de3dabab10c8fdb8573 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:09:47 +0200 Subject: [PATCH] fix(installer): change data directories only after checks and downloads install.sh created the resource directories and ran chown -R and chmod -R on source, ssh, and images before step 1. When a check or a CDN download failed, the changed owners and modes stayed although nothing was installed. Create only /data/coolify and source (for the log and the downloads) at the start. Create the other directories and set the permissions in the SSH key step, after the checks and downloads succeed. Closes #11973 Co-Authored-By: Claude Opus 5.5 --- other/nightly/install.sh | 26 ++++++---- scripts/install.sh | 26 ++++++---- .../InstallScriptDirectoryPermissionsTest.php | 47 +++++++++++++++++-- 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/other/nightly/install.sh b/other/nightly/install.sh index 5b5d74f1b1..f2bdb5830b 100755 --- a/other/nightly/install.sh +++ b/other/nightly/install.sh @@ -31,14 +31,21 @@ fi # Resource directories hold data whose owner and mode the resource images or a non-root SSH user need. # Set their owner and mode only when they are created, never on an upgrade (#11945). -for COOLIFY_DIRECTORY in /data/coolify /data/coolify/{applications,databases,backups,services,proxy,proxy/dynamic,sentinel}; do - if [ ! -d "$COOLIFY_DIRECTORY" ]; then - mkdir -p "$COOLIFY_DIRECTORY" - chown 9999:root "$COOLIFY_DIRECTORY" - chmod 700 "$COOLIFY_DIRECTORY" +create_data_directory() { + if [ ! -d "$1" ]; then + mkdir -p "$1" + chown 9999:root "$1" + chmod 700 "$1" fi -done -mkdir -p /data/coolify/source /data/coolify/images /data/coolify/ssh/{keys,mux} +} + +# Run only after the checks and downloads succeed, so a failed run does not change the data directories (#11973). +create_coolify_directories() { + for COOLIFY_DIRECTORY in /data/coolify/{applications,databases,backups,services,proxy,proxy/dynamic,sentinel}; do + create_data_directory "$COOLIFY_DIRECTORY" + done + mkdir -p /data/coolify/images /data/coolify/ssh/{keys,mux} +} # Coolify's own directories, used by the coolify container (UID 9999) set_coolify_directory_permissions() { @@ -46,7 +53,9 @@ set_coolify_directory_permissions() { chmod -R 700 /data/coolify/{source,ssh,images} } -set_coolify_directory_permissions +# The installation log and the downloaded files go to /data/coolify/source, so only it is created before the checks. +create_data_directory /data/coolify +mkdir -p /data/coolify/source INSTALLATION_LOG_WITH_DATE="/data/coolify/source/installation-${DATE}.log" @@ -1070,6 +1079,7 @@ step_done step_start "Checking SSH key for localhost access" SSH_KEY_DETAIL="existing key" +create_coolify_directories if [ ! -f ~/.ssh/authorized_keys ]; then mkdir -p ~/.ssh chmod 700 ~/.ssh diff --git a/scripts/install.sh b/scripts/install.sh index b40db37408..adc6f745c9 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -31,14 +31,21 @@ fi # Resource directories hold data whose owner and mode the resource images or a non-root SSH user need. # Set their owner and mode only when they are created, never on an upgrade (#11945). -for COOLIFY_DIRECTORY in /data/coolify /data/coolify/{applications,databases,backups,services,proxy,proxy/dynamic,sentinel}; do - if [ ! -d "$COOLIFY_DIRECTORY" ]; then - mkdir -p "$COOLIFY_DIRECTORY" - chown 9999:root "$COOLIFY_DIRECTORY" - chmod 700 "$COOLIFY_DIRECTORY" +create_data_directory() { + if [ ! -d "$1" ]; then + mkdir -p "$1" + chown 9999:root "$1" + chmod 700 "$1" fi -done -mkdir -p /data/coolify/source /data/coolify/images /data/coolify/ssh/{keys,mux} +} + +# Run only after the checks and downloads succeed, so a failed run does not change the data directories (#11973). +create_coolify_directories() { + for COOLIFY_DIRECTORY in /data/coolify/{applications,databases,backups,services,proxy,proxy/dynamic,sentinel}; do + create_data_directory "$COOLIFY_DIRECTORY" + done + mkdir -p /data/coolify/images /data/coolify/ssh/{keys,mux} +} # Coolify's own directories, used by the coolify container (UID 9999) set_coolify_directory_permissions() { @@ -46,7 +53,9 @@ set_coolify_directory_permissions() { chmod -R 700 /data/coolify/{source,ssh,images} } -set_coolify_directory_permissions +# The installation log and the downloaded files go to /data/coolify/source, so only it is created before the checks. +create_data_directory /data/coolify +mkdir -p /data/coolify/source INSTALLATION_LOG_WITH_DATE="/data/coolify/source/installation-${DATE}.log" @@ -1070,6 +1079,7 @@ step_done step_start "Checking SSH key for localhost access" SSH_KEY_DETAIL="existing key" +create_coolify_directories if [ ! -f ~/.ssh/authorized_keys ]; then mkdir -p ~/.ssh chmod 700 ~/.ssh diff --git a/tests/Unit/InstallScriptDirectoryPermissionsTest.php b/tests/Unit/InstallScriptDirectoryPermissionsTest.php index 812af5bde0..e43e0ccff2 100644 --- a/tests/Unit/InstallScriptDirectoryPermissionsTest.php +++ b/tests/Unit/InstallScriptDirectoryPermissionsTest.php @@ -4,16 +4,22 @@ * Runs the /data/coolify directory setup of the install script against a temporary data root. * chown is replaced with a logger because changing the owner to UID 9999 needs root. * + * @param bool $completeRun false runs only the setup before step 1, as when a check or download fails * @return list the paths that chown received */ -function runInstallScriptDirectorySetup(string $scriptPath, string $dataRoot): array +function runInstallScriptDirectorySetup(string $scriptPath, string $dataRoot, bool $completeRun = true): array { $script = file_get_contents(dirname(__DIR__, 2).'/'.$scriptPath); - preg_match('/^# Resource directories.*?^set_coolify_directory_permissions$/ms', $script, $matches); + preg_match('/^# Resource directories.*?^mkdir -p \/data\/coolify\/source$/ms', $script, $matches); expect($matches)->not->toBeEmpty(); - $setup = str_replace('/data/coolify', $dataRoot, $matches[0]); + $setup = $matches[0]; + if ($completeRun) { + $setup .= "\ncreate_coolify_directories\nset_coolify_directory_permissions"; + } + $setup = str_replace('/data/coolify', $dataRoot, $setup); $chownLog = "{$dataRoot}.chown"; + touch($chownLog); $process = proc_open( ['bash', '-c', "set -e\nchown() { printf '%s\\n' \"\$@\" >> ".escapeshellarg($chownLog)."; }\n{$setup}"], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], @@ -95,11 +101,42 @@ it('keeps existing resource directories and their content unchanged on an upgrad } })->with($installScripts); -it('does not change the whole data directory recursively', function (string $scriptPath) { +it('does not change an existing data directory before the checks and downloads succeed', function (string $scriptPath, array $coolifyDirectories) { + $dataRoot = sys_get_temp_dir().'/coolify-install-permissions-'.bin2hex(random_bytes(6)); + + try { + mkdir($dataRoot, 0750); + chmod($dataRoot, 0750); + foreach ($coolifyDirectories as $directory) { + mkdir("{$dataRoot}/{$directory}/nested", 0755, true); + chmod("{$dataRoot}/{$directory}", 0750); + touch("{$dataRoot}/{$directory}/nested/file"); + chmod("{$dataRoot}/{$directory}/nested/file", 0644); + } + + $chownedPaths = runInstallScriptDirectorySetup($scriptPath, $dataRoot, completeRun: false); + + expect($chownedPaths)->toBe([]) + ->and(installScriptFileMode($dataRoot))->toBe('0750'); + foreach ($coolifyDirectories as $directory) { + expect(installScriptFileMode("{$dataRoot}/{$directory}"))->toBe('0750') + ->and(installScriptFileMode("{$dataRoot}/{$directory}/nested"))->toBe('0755') + ->and(installScriptFileMode("{$dataRoot}/{$directory}/nested/file"))->toBe('0644'); + } + } finally { + exec('rm -rf '.escapeshellarg($dataRoot)); + } +})->with($installScripts); + +it('changes the data directories only after the checks and downloads', function (string $scriptPath) { $script = file_get_contents(dirname(__DIR__, 2).'/'.$scriptPath); + $downloadStep = strpos($script, 'step_start "Downloading required files from CDN"'); expect($script) ->not->toContain("chown -R 9999:root /data/coolify\n") ->not->toContain("chmod -R 700 /data/coolify\n") - ->and(substr_count($script, "\nset_coolify_directory_permissions\n"))->toBe(2); + ->and(substr_count($script, "\ncreate_coolify_directories\n"))->toBe(1) + ->and(substr_count($script, "\nset_coolify_directory_permissions\n"))->toBe(1) + ->and(strpos($script, "\ncreate_coolify_directories\n"))->toBeGreaterThan($downloadStep) + ->and(strpos($script, "\nset_coolify_directory_permissions\n"))->toBeGreaterThan($downloadStep); })->with(array_map(fn (array $script) => $script[0], $installScripts));