From 5319bce5257c6c13f6198edd9df59e134f2e5481 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 14:56:45 -0400 Subject: [PATCH 01/10] fix(git): refuse a RAM mount inside a disk or pool The path guard dropped RAM mounts before picking the deepest mount that holds a path, so a tmpfs mounted inside a pool (a RAM transcode folder in appdata, say) was taken for the pool: a clones root there was accepted and clones written to RAM. findMountFor() now picks the deepest mount from the full table and returns null when that one is in RAM. Unassigned Devices' tmpfs over /mnt/disks is still refused as before. A deploy's bind mount check still accepts a folder on such a tmpfs, which the user mounted to be in RAM; a missing one there is left for Docker to create, as below an existing share. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- .../include/GitDeployCheck.php | 9 +++ .../compose.manager/include/GitPathGuard.php | 68 ++++++++++++++++--- tests/unit/GitDeployCheckTest.php | 21 ++++++ tests/unit/GitPathGuardTest.php | 26 +++++++ 4 files changed, 115 insertions(+), 9 deletions(-) diff --git a/source/compose.manager/include/GitDeployCheck.php b/source/compose.manager/include/GitDeployCheck.php index 11db8f19..8cbf3593 100644 --- a/source/compose.manager/include/GitDeployCheck.php +++ b/source/compose.manager/include/GitDeployCheck.php @@ -254,6 +254,15 @@ private function checkBindSource(string $source, string $cloneReal): ?string } if (GitPathGuard::findMountFor($source) === null) { + // A tmpfs the user mounted inside a disk or pool (a RAM transcode folder, say) is + // meant to be in RAM. A missing folder in it, empty after every reboot, is left for + // Docker to create, as below an existing share. + if (GitPathGuard::isOnRamMountInsideADisk($source)) { + if (!file_exists($source)) { + $this->foldersDockerWillCreate[] = $source; + } + return null; + } return "the bind mount $source is not on a mounted disk, pool or share. " . 'Docker would create it in RAM. Check the path for a typo.'; } diff --git a/source/compose.manager/include/GitPathGuard.php b/source/compose.manager/include/GitPathGuard.php index b4556860..56f56f79 100644 --- a/source/compose.manager/include/GitPathGuard.php +++ b/source/compose.manager/include/GitPathGuard.php @@ -47,43 +47,60 @@ public static function isArrayStarted(): bool * @return string[] */ public static function listMountPoints(): array + { + $mountPoints = []; + foreach (self::listMounts() as $mountPoint => $filesystem) { + if (!in_array($filesystem, self::RAM_FILESYSTEMS, true)) { + $mountPoints[] = $mountPoint; + } + } + return $mountPoints; + } + + /** + * Every mount listed in the mounts file, RAM filesystems included. + * + * @return array mount point => filesystem type + */ + private static function listMounts(): array { $content = @file_get_contents(COMPOSE_MOUNTS_FILE); if ($content === false) { return []; } - $mountPoints = []; + $mounts = []; foreach (explode("\n", $content) as $line) { $fields = explode(' ', trim($line)); if (count($fields) < 3) { continue; } - if (in_array($fields[2], self::RAM_FILESYSTEMS, true)) { - continue; - } // The kernel writes spaces, tabs, newlines and backslashes in mount // points as octal escapes, for example "\040" for a space. - $mountPoints[] = preg_replace_callback( + $mountPoint = preg_replace_callback( '/\\\\([0-7]{3})/', static fn(array $match): string => chr((int) octdec($match[1])), $fields[1] ); + $mounts[$mountPoint] = $fields[2]; } - return $mountPoints; + return $mounts; } /** * The mount under /mnt that holds a path, or null when the path is not on one. * - * Picks the longest listed mount point that contains the path. /mnt itself - * and / never count: on Unraid both are RAM. + * Picks the longest listed mount point that contains the path, RAM mounts + * included, and returns null when that one is in RAM: a tmpfs mounted inside + * a disk or pool keeps its contents in RAM even though the disk is below it. + * /mnt itself and / never count: on Unraid both are RAM. */ public static function findMountFor(string $path): ?string { $mntDir = rtrim(COMPOSE_GIT_MNT_DIR, '/'); $best = null; - foreach (self::listMountPoints() as $mountPoint) { + $bestFilesystem = null; + foreach (self::listMounts() as $mountPoint => $filesystem) { $mountPoint = rtrim($mountPoint, '/'); if (!str_starts_with($mountPoint, $mntDir . '/')) { continue; @@ -93,11 +110,44 @@ public static function findMountFor(string $path): ?string } if ($best === null || strlen($mountPoint) > strlen($best)) { $best = $mountPoint; + $bestFilesystem = $filesystem; } } + if ($bestFilesystem !== null && in_array($bestFilesystem, self::RAM_FILESYSTEMS, true)) { + return null; + } return $best; } + /** + * Whether a path is on a RAM mount that sits inside a disk, pool or share mount, + * such as a tmpfs transcode folder in appdata. Unassigned Devices' tmpfs over + * /mnt/disks is not: nothing real is mounted below it. + */ + public static function isOnRamMountInsideADisk(string $path): bool + { + $mntDir = rtrim(COMPOSE_GIT_MNT_DIR, '/'); + $deepestRam = null; + $diskBelow = false; + foreach (self::listMounts() as $mountPoint => $filesystem) { + $mountPoint = rtrim($mountPoint, '/'); + if (!str_starts_with($mountPoint, $mntDir . '/')) { + continue; + } + if ($path !== $mountPoint && !str_starts_with($path, $mountPoint . '/')) { + continue; + } + if (in_array($filesystem, self::RAM_FILESYSTEMS, true)) { + if ($deepestRam === null || strlen($mountPoint) > strlen($deepestRam)) { + $deepestRam = $mountPoint; + } + } else { + $diskBelow = true; + } + } + return $deepestRam !== null && $diskBelow && self::findMountFor($path) === null; + } + /** * Check the shape of an absolute path without touching the filesystem. * diff --git a/tests/unit/GitDeployCheckTest.php b/tests/unit/GitDeployCheckTest.php index e5f8478d..dad6d34e 100644 --- a/tests/unit/GitDeployCheckTest.php +++ b/tests/unit/GitDeployCheckTest.php @@ -228,6 +228,27 @@ public function testBindSourceOnAMissingMountIsCalledOutAsRam(): void $this->assertStringContainsString('RAM', $this->check()[0]); } + public function testBindSourceOnARamMountInsideAPoolIsTheUsersChoice(): void + { + // A tmpfs in appdata, such as a RAM transcode folder, is meant to be in RAM. A missing + // folder in it (empty after every reboot) is left for Docker to create. + mkdir($this->mnt . '/cache/appdata/ram/transcode', 0755, true); + file_put_contents( + COMPOSE_MOUNTS_FILE, + "rootfs {$this->mnt} rootfs rw 0 0\nshfs {$this->mnt}/user fuse.shfs rw 0 0\n" + . "/dev/nvme0n1p1 {$this->mnt}/cache xfs rw 0 0\ntmpfs {$this->mnt}/cache/appdata/ram tmpfs rw 0 0\n" + ); + $missing = $this->mnt . '/cache/appdata/ram/missing'; + $this->docker->setConfig($this->config(['image' => 'busybox', 'volumes' => [ + ['type' => 'bind', 'source' => $this->mnt . '/cache/appdata/ram/transcode', 'target' => '/transcode'], + ['type' => 'bind', 'source' => $missing, 'target' => '/cache'], + ]])); + $check = new GitDeployCheck('whoami', ['-f', $this->clone . '/whoami/compose.yaml'], $this->clone, $this->clone . '/whoami', null); + + $this->assertSame([], $check->run()); + $this->assertSame([$missing], $check->foldersDockerWillCreate()); + } + public function testExistingBindSourceAndMissingSourceInsideTheCloneAreFine(): void { $this->docker->setConfig($this->config(['image' => 'busybox', 'volumes' => [ diff --git a/tests/unit/GitPathGuardTest.php b/tests/unit/GitPathGuardTest.php index 5cdd6f93..4fde95e9 100644 --- a/tests/unit/GitPathGuardTest.php +++ b/tests/unit/GitPathGuardTest.php @@ -100,6 +100,32 @@ public function testMountNameThatOnlySharesAPrefixDoesNotMatch(): void $this->assertNull(GitPathGuard::findMountFor($this->mnt . '/user0/appdata')); } + public function testRamMountInsideADiskMountIsRefused(): void + { + // A tmpfs mounted inside a pool (a RAM transcode folder, say) keeps its contents + // in RAM, even though the pool is mounted below it. + mkdir($this->mnt . '/cache/appdata/ram/git', 0755, true); + file_put_contents( + COMPOSE_MOUNTS_FILE, + "rootfs / rootfs rw 0 0\n" + . "rootfs {$this->mnt} rootfs rw 0 0\n" + . "/dev/nvme0n1p1 {$this->mnt}/cache xfs rw 0 0\n" + . "tmpfs {$this->mnt}/cache/appdata/ram tmpfs rw 0 0\n" + ); + + $this->assertNull(GitPathGuard::findMountFor($this->mnt . '/cache/appdata/ram/git')); + $this->assertSame($this->mnt . '/cache', GitPathGuard::findMountFor($this->mnt . '/cache/appdata/git')); + + try { + GitPathGuard::assertSafeToWrite($this->mnt . '/cache/appdata/ram/git'); + $this->fail('A folder on a tmpfs inside a pool was accepted for writing.'); + } catch (RuntimeException $error) { + $this->assertStringContainsString('not on a mounted disk', $error->getMessage()); + } + $this->expectException(InvalidArgumentException::class); + GitPathGuard::assertValidClonesRoot($this->mnt . '/cache/appdata/ram/git'); + } + // ----- clones root ----- public function testUsualClonesRootsAreAccepted(): void From 437b9b2e0be3c54f66d28056dcffb183719f94ff Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 14:59:39 -0400 Subject: [PATCH 02/10] fix(git): stop a git deploy when a missing network cannot be created With Create Missing External Networks on, a git deploy's checks let a missing external network through, for the deploy to create just before up. When that creation failed, it was only logged and up ran anyway: up then failed before touching a container, but as a failed up, so the commit was recorded as failed and the previous one was not put back. create_missing_external_networks now tries every network and returns 1 if any could not be created. gitdeploy stops there and puts the previous commit back, as it does for a failed pull or build. Start and Update ignore the result, as before. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- source/compose.manager/scripts/common.sh | 5 ++++- source/compose.manager/scripts/compose.sh | 13 +++++++++++-- tests/unit/common.bats | 20 ++++++++++++++++++++ tests/unit/gitdeploy.bats | 18 ++++++++++++++++-- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/source/compose.manager/scripts/common.sh b/source/compose.manager/scripts/common.sh index 2f83b178..f89efd54 100644 --- a/source/compose.manager/scripts/common.sh +++ b/source/compose.manager/scripts/common.sh @@ -135,8 +135,9 @@ foreach ($config["networks"] ?? [] as $key => $network) { # Arguments: the docker compose command that reads the stack's config, for example # create_missing_external_networks docker compose -f compose.yaml -p mystack # A config that does not parse is skipped here; the compose command that follows reports it. +# Returns 1 if any network could not be created (after trying them all, so each is named). create_missing_external_networks() { - local config_json network + local config_json network failed=0 if ! config_json=$("$@" config --format json 2>/dev/null); then return 0 fi @@ -151,8 +152,10 @@ create_missing_external_networks() { else echo "✗ Failed to create external network: $network" composeLogger "Failed to create external network: $network" error compose + failed=1 fi done < <(printf '%s' "$config_json" | external_network_names) + return $failed } # Resolve effective env file for a stack. diff --git a/source/compose.manager/scripts/compose.sh b/source/compose.manager/scripts/compose.sh index 649e4a0c..2da09fde 100755 --- a/source/compose.manager/scripts/compose.sh +++ b/source/compose.manager/scripts/compose.sh @@ -644,9 +644,18 @@ case $command in # Optionally create missing `external: true` networks, as up and update do # (Settings > Compose > Create Missing External Networks). This is done # here, after every check, pull and build has passed, so a deploy that - # stops earlier creates nothing. + # stops earlier creates nothing. A network that cannot be created would make + # up fail, so stop here, while the previous commit can still be put back. if plugin_setting_enabled CREATE_MISSING_EXTERNAL_NETWORKS; then - create_missing_external_networks "${git_compose[@]}" -p "$name" + if ! create_missing_external_networks "${git_compose[@]}" -p "$name"; then + save_result "failed" 1 "gitdeploy" + log_msg "ERROR" "A missing external network could not be created for git stack $name" + echo "" + echo "✗ Stack $name was not deployed: a missing external network could not be created. No container was changed." + echo "Any network created above is left in place." + put_back_previous_commit + exit 1 + fi fi # 4. Start the stack at the new commit. diff --git a/tests/unit/common.bats b/tests/unit/common.bats index 32559fa5..6cf31f6f 100644 --- a/tests/unit/common.bats +++ b/tests/unit/common.bats @@ -228,6 +228,26 @@ NETWORKS_CONFIG_JSON='{"networks":{"internalnet":{"name":"stack_internalnet"},"s [[ "$(cat "$TEST_TEMP_DIR/created")" == "svc-real-name" ]] } +@test "create_missing_external_networks tries every network and fails when one cannot be created" { + # shellcheck disable=SC1090 + source "$COMMON_SCRIPT" + # Neither network exists; creating "svc-real-name" fails, creating "web" works. + docker() { + case "$*" in + "compose -p teststack config --format json") echo "$NETWORKS_CONFIG_JSON" ;; + "network inspect "*) return 1 ;; + "network create svc-real-name") return 1 ;; + "network create "*) echo "$3" >> "$TEST_TEMP_DIR/created" ;; + esac + } + + run create_missing_external_networks docker compose -p teststack + + assert_failure + assert_output_contains "Failed to create external network: svc-real-name" + [[ "$(cat "$TEST_TEMP_DIR/created")" == "web" ]] +} + @test "create_missing_external_networks does nothing when the compose config does not parse" { # shellcheck disable=SC1090 source "$COMMON_SCRIPT" diff --git a/tests/unit/gitdeploy.bats b/tests/unit/gitdeploy.bats index a1c5b8a5..852acb7d 100644 --- a/tests/unit/gitdeploy.bats +++ b/tests/unit/gitdeploy.bats @@ -26,8 +26,9 @@ test_setup() { export PREPARE_OUTPUT="0000000000000000000000000000000000000001" # The plugin's settings: none, so Create Missing External Networks is off unless a test turns it on. export COMPOSE_MANAGER_CFG_FILE="$TEST_TEMP_DIR/compose.manager.cfg" - # What "docker compose config --format json" prints, and whether "docker network inspect" finds the network. - export CONFIG_JSON='{}' NETWORK_INSPECT_EXIT=0 + # What "docker compose config --format json" prints, whether "docker network inspect" finds the network, + # and whether "docker network create" works. + export CONFIG_JSON='{}' NETWORK_INSPECT_EXIT=0 NETWORK_CREATE_EXIT=0 STACK="$TEST_TEMP_DIR/whoami" mkdir -p "$STACK" "$TEST_TEMP_DIR/bin" @@ -84,6 +85,7 @@ for arg in "$@"; do up) exit "$UP_EXIT" ;; config) printf '%s' "$CONFIG_JSON"; exit 0 ;; inspect) exit "$NETWORK_INSPECT_EXIT" ;; + create) exit "$NETWORK_CREATE_EXIT" ;; esac done exit 0 @@ -280,6 +282,18 @@ calls_matching() { [ "$create_line" -lt "$up_line" ] } +@test "gitdeploy puts the previous commit back when a missing network cannot be created, and never runs up" { + echo 'CREATE_MISSING_EXTERNAL_NETWORKS="true"' > "$COMPOSE_MANAGER_CFG_FILE" + export CONFIG_JSON='{"networks":{"proxy":{"name":"zz-proxy","external":true}}}' NETWORK_INSPECT_EXIT=1 + export NETWORK_CREATE_EXIT=1 + run_gitdeploy + [ "$status" -eq 1 ] + [[ "$output" == *"a missing external network could not be created"* ]] + grep -q "git_stack restore $STACK 0000000000000000000000000000000000000001" "$CALLS" + [ "$(calls_matching ' up ')" -eq 0 ] + [ "$(calls_matching 'finish')" -eq 0 ] +} + @test "gitdeploy creates no network when the setting is off" { export CONFIG_JSON='{"networks":{"proxy":{"name":"zz-proxy","external":true}}}' NETWORK_INSPECT_EXIT=1 run_gitdeploy From 6cf338afa3327aaf3fe14d885d9ad48c8f387eca Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:01:04 -0400 Subject: [PATCH 03/10] fix(git): never build or pull again during a git deploy's up A git deploy pulls and builds as steps of their own, so that a failure there puts the previous commit back. But up still followed each service's pull_policy: "build" built again and "always" pulled again during up, where a failure is recorded as a failed up and nothing is put back. up now gets --no-build and --pull missing: it uses the images the earlier steps prepared, and still fetches one they did not. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- docs/git-stacks.md | 4 ++-- source/compose.manager/scripts/compose.sh | 6 +++++- tests/unit/gitdeploy.bats | 11 +++++++---- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/git-stacks.md b/docs/git-stacks.md index 82c4cf7a..69e400ce 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -41,8 +41,8 @@ to your shell profile. pulling new commits never touches them. - **A deploy changes no container until everything else has worked.** It fetches the commit, checks it out, checks it (see [below](#what-is-checked-before-a-deploy)), pulls and builds images, and only then runs - `docker compose up`. If anything before `up` fails, the previous commit is put back and your containers are - exactly as they were. + `docker compose up`, which neither builds nor pulls again, whatever a service's `pull_policy` says. If + anything before `up` fails, the previous commit is put back and your containers are exactly as they were. - **Nothing of yours is deleted.** Files the plugin replaces are moved into a backup folder, local changes are saved as a patch before they are discarded, and an old clone is moved aside rather than removed. The plugin removes only a clone it has just made itself, when setting it up fails (for example, the branch has no diff --git a/source/compose.manager/scripts/compose.sh b/source/compose.manager/scripts/compose.sh index 2da09fde..05090688 100755 --- a/source/compose.manager/scripts/compose.sh +++ b/source/compose.manager/scripts/compose.sh @@ -574,7 +574,11 @@ case $command in # A deploy always builds services that have a build section (the new # commit may change their Dockerfile), as a step of its own before up. - up_args=("-d" "--remove-orphans") + # up itself neither builds nor pulls again, whatever a service's + # pull_policy says: a failure there would come after the point where the + # previous commit can be put back. --pull missing still fetches an image + # the pull step did not. + up_args=("-d" "--remove-orphans" "--no-build" "--pull" "missing") if [ "$wait_for_healthy" = true ]; then up_args+=("--wait") if [ -n "$wait_timeout" ]; then diff --git a/tests/unit/gitdeploy.bats b/tests/unit/gitdeploy.bats index 852acb7d..c292162a 100644 --- a/tests/unit/gitdeploy.bats +++ b/tests/unit/gitdeploy.bats @@ -218,7 +218,7 @@ calls_matching() { [ "$build_line" -lt "$up_line" ] [ "$up_line" -lt "$finish_line" ] - grep -q "docker compose -f /clone/whoami/compose.yaml --env-file /stack/.env -p whoami up -d --remove-orphans$" "$CALLS" + grep -q "docker compose -f /clone/whoami/compose.yaml --env-file /stack/.env -p whoami up -d --remove-orphans --no-build --pull missing$" "$CALLS" grep -q "git_stack finish $STACK success" "$CALLS" grep -q "docker rmi img-old$" "$CALLS" grep -q '"operation":"gitdeploy"' "$STACK/last_result.json" @@ -234,7 +234,7 @@ calls_matching() { @test "gitdeploy waits for healthy containers when asked" { run_gitdeploy --wait --wait-timeout 90 [ "$status" -eq 0 ] - grep -q -- "up -d --remove-orphans --wait --wait-timeout 90" "$CALLS" + grep -q -- "up -d --remove-orphans --no-build --pull missing --wait --wait-timeout 90" "$CALLS" } @test "gitdeploy recreates every container when the stack's folder changed" { @@ -242,7 +242,7 @@ calls_matching() { run_gitdeploy --wait [ "$status" -eq 0 ] grep -q "git_stack up-arguments $STACK 0000000000000000000000000000000000000001" "$CALLS" - grep -q -- "up -d --remove-orphans --wait --force-recreate$" "$CALLS" + grep -q -- "up -d --remove-orphans --no-build --pull missing --wait --force-recreate$" "$CALLS" } @test "gitdeploy puts the previous commit back when it cannot tell whether to recreate, and never runs up" { @@ -262,10 +262,13 @@ calls_matching() { [ "$(calls_matching ' up ')" -eq 0 ] } -@test "gitdeploy never rebuilds during up, so a build failure cannot happen part-way" { +@test "gitdeploy tells up never to build and to pull only missing images" { + # A service's pull_policy (build or always) would otherwise build or pull again during up, + # after the point where the previous commit can be put back. run_gitdeploy --build [ "$status" -eq 0 ] [ "$(grep -c -- ' up .*--build' "$CALLS" || true)" -eq 0 ] + grep -q -- ' up .*--no-build --pull missing' "$CALLS" } @test "gitdeploy creates a missing external network after the build and before up when the setting is on" { From fa14f6688267362445e529de04f747add1a07bfd Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:07:55 -0400 Subject: [PATCH 04/10] fix(git): check the projects folder before compose-git add clones add() made the stack folder and wrote its first files before anything checked that the projects folder is on a mounted disk; only saving the git settings refused. With a projects folder left in RAM (a pool that did not mount), add wrote there and left a clone and a partly made stack folder behind. The stack folder is now checked first, before the clone, as convert already does. The test that relied on the old order to reach the "only partly made" message is replaced by one that expects nothing written. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- .../include/GitStackManager.php | 2 ++ tests/unit/GitStackManagerTest.php | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/source/compose.manager/include/GitStackManager.php b/source/compose.manager/include/GitStackManager.php index 847ff566..8f624618 100644 --- a/source/compose.manager/include/GitStackManager.php +++ b/source/compose.manager/include/GitStackManager.php @@ -57,6 +57,8 @@ public function add( if (file_exists($stackDir) || @readlink($stackDir) !== false) { throw new RuntimeException("A stack folder named '$folder' already exists. Choose another name, or use convert for that stack."); } + // Before the clone, so a projects folder whose mount is gone leaves nothing behind. + $this->assertSafeToWriteStackFolder($stackDir); $settings = GitStackSettings::createNew($url, $branch, $composePath, $clonesRoot ?? COMPOSE_GIT_DEFAULT_CLONES_ROOT, $folder); ($this->say)("Cloning $url ($branch) into {$settings->cloneDir}..."); diff --git a/tests/unit/GitStackManagerTest.php b/tests/unit/GitStackManagerTest.php index ec913a65..6a74ca55 100644 --- a/tests/unit/GitStackManagerTest.php +++ b/tests/unit/GitStackManagerTest.php @@ -106,29 +106,30 @@ public function testAddWithAComposePathNotOnTheBranchLeavesNothingBehind(): void $this->assertDirectoryDoesNotExist($this->composeRoot . '/whoami'); } - public function testAddThatFailsAfterMakingTheStackFolderSaysToRemoveIt(): void + public function testAddDoesNothingWhileTheArrayIsStopped(): void + { + file_put_contents(COMPOSE_UNRAID_VAR_INI, "mdState=\"STOPPED\"\n"); + $this->expectException(RuntimeException::class); + $this->manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + } + + public function testAddToAProjectsFolderWhoseMountIsGoneWritesNothing(): void { - // A projects folder on the array's root file system (RAM): the stack - // folder is made, then saving its git settings there is refused. + // A projects folder that is not on a mount (a pool that did not mount, while the + // folder is still there, in RAM): neither the clone nor the stack folder may be made. + // This used to make the stack folder first and fail only when saving its settings. $composeRoot = $this->mnt . '/projects'; mkdir($composeRoot); $manager = new GitStackManager($composeRoot); try { $manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); - $this->fail('Added a stack whose settings could not be saved'); + $this->fail('The stack was added to a projects folder that is not on a mounted disk.'); } catch (RuntimeException $error) { - $this->assertStringContainsString("the stack folder $composeRoot/whoami was only partly made", $error->getMessage()); - $this->assertStringContainsString('Remove both folders', $error->getMessage()); + $this->assertStringContainsString('not on a mounted disk', $error->getMessage()); } - $this->assertDirectoryExists($composeRoot . '/whoami'); - } - - public function testAddDoesNothingWhileTheArrayIsStopped(): void - { - file_put_contents(COMPOSE_UNRAID_VAR_INI, "mdState=\"STOPPED\"\n"); - $this->expectException(RuntimeException::class); - $this->manager->add('whoami', $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + $this->assertDirectoryDoesNotExist($composeRoot . '/whoami'); + $this->assertSame([], glob($this->clonesRoot . '/*') ?: []); } // ----- convert ----- From c32965d5a183d6400396089a9911766ab6a68e09 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:10:26 -0400 Subject: [PATCH 05/10] fix(git): keep the .env in use when converting with a stale env path convert copies the .env next to an indirect stack's old compose file into the stack folder, since that one stops being used once the compose file is in the clone. It skipped the copy whenever the stack had an env file setting at all. But a setting that names a missing file is ignored, and the .env next to the compose file is used instead: after the convert that .env was no longer used, and its values were lost. The copy is now decided by which .env is actually in use. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- .../include/GitStackManager.php | 7 +++++-- tests/unit/GitStackManagerTest.php | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/source/compose.manager/include/GitStackManager.php b/source/compose.manager/include/GitStackManager.php index 8f624618..e7ef4d05 100644 --- a/source/compose.manager/include/GitStackManager.php +++ b/source/compose.manager/include/GitStackManager.php @@ -109,7 +109,10 @@ public function convert(string $folder, string $url, string $branch, string $com return $this->withStackLock($stack, function () use ($stack, $stackDir, $folder, $url, $branch, $composePath, $clonesRoot): string { $oldOverride = $stack->getOverridePath(); $oldEnv = $stack->getEffectiveEnvFilePath(); - $hasExplicitEnvPath = trim((string) @file_get_contents($stackDir . '/envpath')) !== ''; + // Whether the .env in use is the one next to the old compose file. That one stops + // being used once the compose file is in the clone. An envpath setting is not enough + // to tell: one that names a missing file is ignored, and that .env is used instead. + $usesEnvNextToComposeFile = $oldEnv !== null && Path::refersToSamePath($oldEnv, $stack->composeSource . '/.env'); $oldComposeFile = $stack->isIndirect ? null : $stack->composeFilePath; $foldersNextToOldComposeFile = $this->foldersIn($stack->composeSource); @@ -154,7 +157,7 @@ public function convert(string $folder, string $url, string $branch, string $com // An indirect stack may have used a .env next to its old compose // file. Keep using it: copy it into the stack folder. $stackEnv = $stackDir . '/.env'; - if (!$hasExplicitEnvPath && $oldEnv !== null && $oldEnv !== $stackEnv && !is_file($stackEnv)) { + if ($usesEnvNextToComposeFile && $oldEnv !== null && $oldEnv !== $stackEnv && !is_file($stackEnv)) { if (!@copy($oldEnv, $stackEnv)) { throw new RuntimeException("Could not copy $oldEnv to $stackEnv."); } diff --git a/tests/unit/GitStackManagerTest.php b/tests/unit/GitStackManagerTest.php index 6a74ca55..43321d88 100644 --- a/tests/unit/GitStackManagerTest.php +++ b/tests/unit/GitStackManagerTest.php @@ -195,6 +195,25 @@ public function testConvertOfAStackWithACustomEnvFileCopiesNoEnv(): void $this->assertSame($customEnv, \StackInfo::fromProject($this->composeRoot, $stack->projectFolder)->getEffectiveEnvFilePath()); } + public function testConvertWithAnEnvPathToAMissingFileCopiesTheEnvInUse(): void + { + // An env file setting that names a file which is gone is ignored, and the .env next + // to the old compose file is used. That is the one to keep using after the convert. + $oldFolder = $this->mnt . '/user/appdata/legacy'; + mkdir($oldFolder); + file_put_contents($oldFolder . '/compose.yaml', "services:\n whoami:\n image: traefik/whoami\n"); + file_put_contents($oldFolder . '/.env', "NEXT_TO_COMPOSE=1\n"); + $stack = \StackInfo::createNew($this->composeRoot, 'legacy', '', $oldFolder); + $stackDir = $this->composeRoot . '/' . $stack->projectFolder; + file_put_contents($stackDir . '/envpath', $this->mnt . '/user/appdata/moved-away.env'); + + $this->manager->convert($stack->projectFolder, $this->upstream, 'main', 'whoami/compose.yaml', $this->clonesRoot); + + $this->assertSame("NEXT_TO_COMPOSE=1\n", file_get_contents($stackDir . '/.env')); + \StackInfo::clearCache(); + $this->assertSame($stackDir . '/.env', \StackInfo::fromProject($this->composeRoot, $stack->projectFolder)->getEffectiveEnvFilePath()); + } + public function testConvertOfAnIndirectStackCarriesItsOverrideIntoTheStackFolder(): void { $oldFolder = $this->mnt . '/user/appdata/legacy'; From a24f9ef3dd5bb8db31417a4aa3744e6b4d2f5003 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:13:48 -0400 Subject: [PATCH 06/10] fix(git): recreate containers after discarding local changes in the stack folder A deploy with --save-local-changes saves edits made in the clone as a patch and discards them. When that happened at the same commit as the one deployed, no commit differed, so nothing was recreated: a container with a single-file mount of an edited config kept the edit, while the deploy reported success. When the discarded changes include files in the stack's folder, the deploy now leaves a git_discarded_changes file in the stack folder (before discarding, so a failure to write it stops the deploy with the changes in place). While it is there, every deploy recreates the containers; a successful one removes it. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- docs/git-stacks.md | 12 +++-- source/compose.manager/include/GitDeploy.php | 56 ++++++++++++++++++++ tests/unit/GitDeployTest.php | 33 ++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/docs/git-stacks.md b/docs/git-stacks.md index 69e400ce..f4554d97 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -37,8 +37,9 @@ to your shell profile. - **The clone** lives on your array, by default under `/mnt/user/appdata/compose.manager/git/-`, never on the flash drive. Each git stack has its own clone of the whole repository. - **The stack folder** in the projects folder holds the stack's settings (`git.json`), what was last deployed - (`git_state.json`), the stack's `.env` and the plugin's override file. These are never inside the clone, so - pulling new commits never touches them. + (`git_state.json`), the stack's `.env` and the plugin's override file (and, until the next deploy that works, + `git_discarded_changes`, see [below](#config-files-and-recreating-containers)). These are never inside the + clone, so pulling new commits never touches them. - **A deploy changes no container until everything else has worked.** It fetches the commit, checks it out, checks it (see [below](#what-is-checked-before-a-deploy)), pulls and builds images, and only then runs `docker compose up`, which neither builds nor pulls again, whatever a service's `pull_policy` says. If @@ -184,7 +185,8 @@ unchanged, the stack keeps its volumes and networks. Check that it works. ### 8. Tidy up Once you are happy, delete the `pre-git-` folder. Until then it is your way back: to undo the -conversion, delete `git.json`, `git_state.json`, `indirect` and `indirect_mode` from the stack folder, then +conversion, delete `git.json`, `git_state.json`, `git_discarded_changes` (if there is one), `indirect` and +`indirect_mode` from the stack folder, then copy the backup folder's files back into it (they include the old `indirect` settings, if the stack had any). ## Day to day @@ -286,6 +288,10 @@ a config file the container mounts would otherwise be checked out but never take So by default, **when a deploy changes anything in the stack's folder in the repository, every container in the stack is recreated**, and the new config takes effect. +A deploy that saves local changes in the stack's folder as a patch and discards them recreates every +container too, even at the same commit: a container may still hold an edited file. Until a deploy works, the +stack folder holds a `git_discarded_changes` file that says so. + Only the stack's own folder counts: a change to a shared folder elsewhere in the repository does not recreate anything yet. To turn the behaviour off, set `"recreateOnFolderChange": false` in the stack's `git.json`. diff --git a/source/compose.manager/include/GitDeploy.php b/source/compose.manager/include/GitDeploy.php index dda59159..6a9c352f 100644 --- a/source/compose.manager/include/GitDeploy.php +++ b/source/compose.manager/include/GitDeploy.php @@ -21,6 +21,15 @@ */ final class GitDeploy { + /** + * A file in the stack folder that says local changes to files in the stack's + * folder in the clone were discarded, and no deploy has succeeded since. The + * containers may still hold the discarded files (a single-file bind mount keeps + * the old copy), but no commit differs, so only this tells the next "up" to + * recreate them. + */ + public const DISCARDED_CHANGES_FILE = 'git_discarded_changes'; + /** @var callable(string): void */ private $say; @@ -113,6 +122,11 @@ public function upArguments(string $previousCommit): array $clone = new GitClone($settings); $current = $clone->checkedOutCommit(); + if (is_file($this->stackDir . '/' . self::DISCARDED_CHANGES_FILE)) { + ($this->say)("Local changes in the stack's folder were discarded, so every container is recreated."); + return ['--force-recreate']; + } + $compareFrom = [$previousCommit]; $deployed = GitStackState::load($this->stackDir)->deployedCommit; if ($deployed !== null && $deployed !== $previousCommit) { @@ -156,6 +170,12 @@ public function finish(bool $succeeded): void ? new GitStackState($commit, null) : new GitStackState($state->deployedCommit, $commit); $newState->save($this->stackDir); + if ($succeeded) { + // The containers were just made from the clone as it is. If this cannot be + // removed, every deploy recreates the containers until it is (it is in the + // stack folder, and removing it by hand is safe). + @unlink($this->stackDir . '/' . self::DISCARDED_CHANGES_FILE); + } } /** @@ -283,9 +303,45 @@ private function dealWithLocalChanges(GitClone $clone, GitStackState $state, str if ($patch !== null) { ($this->say)("Saved the local changes to $patch"); } + // Recorded before the discard, so a failure here leaves the changes in place. + if ($this->anyInStackFolder($clone, $changedFiles)) { + $this->recordDiscardedChanges(); + } $clone->discardLocalChanges(); } + /** + * Whether any of these paths (relative to the repository) is in the stack's folder. + * + * @param string[] $paths + */ + private function anyInStackFolder(GitClone $clone, array $paths): bool + { + $stackFolder = $clone->stackFolderInRepo(); + if ($stackFolder === '') { + return $paths !== []; + } + foreach ($paths as $path) { + if (str_starts_with($path, $stackFolder . '/')) { + return true; + } + } + return false; + } + + private function recordDiscardedChanges(): void + { + $stackDir = rtrim($this->stackDir, '/'); + if (str_starts_with($stackDir, rtrim(COMPOSE_GIT_MNT_DIR, '/') . '/')) { + GitPathGuard::assertSafeToWrite($stackDir); + } + $file = $stackDir . '/' . self::DISCARDED_CHANGES_FILE; + $content = date('c') . "\n"; + if (file_put_contents($file, $content) !== strlen($content)) { + throw new RuntimeException("Could not write $file, so the local changes were not discarded."); + } + } + /** * @param string[] $profiles * @throws RuntimeException listing every problem found diff --git a/tests/unit/GitDeployTest.php b/tests/unit/GitDeployTest.php index 2f76a976..a07b6a36 100644 --- a/tests/unit/GitDeployTest.php +++ b/tests/unit/GitDeployTest.php @@ -331,6 +331,39 @@ public function testRedeployOfTheSameCommitRecreatesNothing(): void $this->assertSame([], $this->deploy()->upArguments($previous)); } + public function testDiscardedLocalChangesInTheStackFolderRecreateEveryContainer(): void + { + // A config file edited in the clone, then discarded at the same commit: no commit + // differs, but a container with a single-file mount of it still holds the edit. + $this->writeAndPush(['whoami/app.conf' => "from-repo\n"], 'config'); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->deploy()->finish(true); + file_put_contents($this->clone->settings()->cloneDir . '/whoami/app.conf', "edited-locally\n"); + + $previous = $this->deploy()->prepare('whoami', [], null, true); + $this->assertSame(['--force-recreate'], $this->deploy()->upArguments($previous)); + + // Until a deploy succeeds, every deploy recreates: the containers may still hold the edit. + $this->deploy()->finish(false); + $this->assertSame(['--force-recreate'], $this->deploy()->upArguments($previous)); + $this->deploy()->finish(true); + $this->assertFileDoesNotExist($this->stackDir . '/' . GitDeploy::DISCARDED_CHANGES_FILE); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->assertSame([], $this->deploy()->upArguments($previous)); + } + + public function testDiscardedLocalChangesOutsideTheStackFolderRecreateNothing(): void + { + $this->writeAndPush(['other/readme.txt' => "from-repo\n"], 'another folder'); + $previous = $this->deploy()->prepare('whoami', [], null, false); + $this->deploy()->finish(true); + file_put_contents($this->clone->settings()->cloneDir . '/other/readme.txt', "edited-locally\n"); + + $previous = $this->deploy()->prepare('whoami', [], null, true); + + $this->assertSame([], $this->deploy()->upArguments($previous)); + } + public function testWithTheSettingOffNothingIsRecreated(): void { GitStackSettings::load($this->stackDir)->withRecreateOnFolderChange(false)->save($this->stackDir); From a02b855183dc547d266820ba776f466a246174d8 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:17:02 -0400 Subject: [PATCH 07/10] fix(git): deploy with the same environment the checks used A git deploy's checks run docker compose with a minimal environment, in the compose file's folder. compose-git then started compose.sh with the caller's whole shell environment, from the caller's folder. Variables exported in that shell win over the stack's .env in compose, and ${PWD} was the caller's folder, so the checks could pass for one port or bind path while up deployed another. DOCKER_HOST could even point up at another daemon than the one the checks asked. compose-git now starts compose.sh with the checks' environment (plus the documented COMPOSE_LOCK_TIMEOUT and COMPOSE_LOCK_DIR, and DOCKER_CONFIG) in the compose file's folder, and the checks set PWD to their folder as a shell would. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- docs/git-stacks.md | 3 ++ .../compose.manager/include/DockerCommand.php | 3 ++ .../compose.manager/scripts/compose_git.php | 48 ++++++++++++++++++- tests/unit/GitStackManagerTest.php | 39 +++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) diff --git a/docs/git-stacks.md b/docs/git-stacks.md index f4554d97..36fadd99 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -231,6 +231,9 @@ With no custom env file set for the stack, a git stack uses, in order: Only one of the two is used; they are not merged. Editing a `.env` that came from the repository changes the clone, so the next deploy stops with "local changes": make the change in the repository instead. +Variables exported in the shell you run `compose-git deploy` from are not passed on, and `${PWD}` in the compose +file is the compose file's folder in the clone: a deploy uses only what its checks saw. Put values in the `.env`. + If the `.env` sets `COMPOSE_FILE` with relative paths, they are relative to the folder the `.env` is in. For the stack folder's `.env` that is the stack folder, not the clone, so use paths relative to the clone only in a `.env` committed next to the compose file. diff --git a/source/compose.manager/include/DockerCommand.php b/source/compose.manager/include/DockerCommand.php index f19cb809..dddd6e04 100644 --- a/source/compose.manager/include/DockerCommand.php +++ b/source/compose.manager/include/DockerCommand.php @@ -28,6 +28,9 @@ public static function run(array $args, ?string $workingDirectory = null, int $t $environment['DOCKER_CONFIG'] = $dockerConfig; } + // As a shell would set it: a compose file can use ${PWD}, and compose.sh's runs see it too. + $environment['PWD'] = $workingDirectory ?? '/'; + return ProcessRunner::run( array_merge([COMPOSE_DOCKER_BIN], $args), $workingDirectory ?? '/', diff --git a/source/compose.manager/scripts/compose_git.php b/source/compose.manager/scripts/compose_git.php index 7d732395..a0d5adaf 100644 --- a/source/compose.manager/scripts/compose_git.php +++ b/source/compose.manager/scripts/compose_git.php @@ -329,6 +329,43 @@ function compose_git_profile_arguments(StackInfo $stack, array $options): array return $args; } +/** + * The environment compose.sh gets for a git deploy: what DockerCommand gives the deploy's + * checks, plus the documented lock settings. Nothing else of the caller's shell is passed on. + * + * @param array $current The caller's environment + * @return array + */ +function compose_git_deploy_environment(array $current): array +{ + $environment = [ + 'PATH' => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', + 'HOME' => $current['HOME'] ?? '/root', + 'LC_ALL' => 'C', + ]; + // TERM only changes how Compose draws its progress. + foreach (['DOCKER_CONFIG', 'COMPOSE_LOCK_TIMEOUT', 'COMPOSE_LOCK_DIR', 'TERM'] as $name) { + if (isset($current[$name]) && $current[$name] !== '') { + $environment[$name] = $current[$name]; + } + } + return $environment; +} + +/** + * The folder compose.sh runs in for a git deploy: the compose file's folder, where the + * deploy's checks run too. + */ +function compose_git_deploy_directory(?string $composeFilePath): string +{ + $composeFile = $composeFilePath === null ? false : realpath($composeFilePath); + if ($composeFile === false) { + // The compose file is checked again by the deploy, which stops before up if it is missing. + return '/'; + } + return dirname($composeFile); +} + /** * Deploy through compose.sh, so the deploy takes the same lock, credentials * and logging as every other stack action. Its output goes straight to ours. @@ -365,7 +402,16 @@ function compose_git_deploy(GitStackManager $manager, string $folder, array $opt $command[] = $arg; } - $process = proc_open($command, [0 => ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], $pipes); + // compose.sh runs with the same environment and folder as the deploy's checks, so a + // variable exported in the caller's shell (or the folder it was run from, ${PWD}) + // cannot make up deploy something other than what was checked. + $process = proc_open( + $command, + [0 => ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], + $pipes, + compose_git_deploy_directory($stack->composeFilePath), + compose_git_deploy_environment(getenv()) + ); if (!is_resource($process)) { fwrite($errors, "Could not start compose.sh.\n"); return COMPOSE_GIT_EXIT_FAILED; diff --git a/tests/unit/GitStackManagerTest.php b/tests/unit/GitStackManagerTest.php index 43321d88..2eab84b3 100644 --- a/tests/unit/GitStackManagerTest.php +++ b/tests/unit/GitStackManagerTest.php @@ -484,6 +484,45 @@ public function testDeployWaitsAsTheStacksSettingSaysUnlessTheCommandLineOverrid $this->assertSame(['--wait'], compose_git_wait_arguments(['enabled' => true, 'timeout' => '5m'], [])); } + public function testDeployRunsComposeWithTheSameEnvironmentAsTheChecks(): void + { + // Exported variables win over the stack's .env in compose, and PWD is the caller's + // folder: neither may reach the deploy, since the checks never saw them. + $callersShell = [ + 'PATH' => '/root/bin:/usr/bin', + 'HOME' => '/root', + 'PWD' => '/root/somewhere', + 'PORT' => '8080', + 'DOCKER_HOST' => 'tcp://elsewhere:2375', + 'COMPOSE_PROFILES' => 'debug', + 'COMPOSE_LOCK_TIMEOUT' => '5', + 'DOCKER_CONFIG' => '/tmp/registry-login', + ]; + + $this->assertSame([ + 'PATH' => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', + 'HOME' => '/root', + 'LC_ALL' => 'C', + 'DOCKER_CONFIG' => '/tmp/registry-login', + 'COMPOSE_LOCK_TIMEOUT' => '5', + ], compose_git_deploy_environment($callersShell)); + } + + public function testDeployRunsComposeInTheComposeFilesFolder(): void + { + $folder = $this->mnt . '/user/appdata/git/whoami/whoami'; + mkdir($folder, 0755, true); + file_put_contents($folder . '/compose.yaml', "services: {}\n"); + // A compose file that is a symlink: the folder of the file it leads to, as the checks use. + mkdir($this->mnt . '/user/appdata/git/whoami/linked', 0755, true); + symlink($folder . '/compose.yaml', $this->mnt . '/user/appdata/git/whoami/linked/compose.yaml'); + + $this->assertSame(realpath($folder), compose_git_deploy_directory($folder . '/compose.yaml')); + $this->assertSame(realpath($folder), compose_git_deploy_directory($this->mnt . '/user/appdata/git/whoami/linked/compose.yaml')); + $this->assertSame('/', compose_git_deploy_directory($folder . '/missing.yaml')); + $this->assertSame('/', compose_git_deploy_directory(null)); + } + public function testDeployRefusesWaitAndNoWaitTogether(): void { $this->expectException(\ComposeGitUsageError::class); From a40b670bb35f941b74fe470ab8be853905b32ade Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:17:16 -0400 Subject: [PATCH 08/10] docs(git): say that mounted files from the clone change before up A deploy checks the new commit out in the clone, then runs the checks, pull and build, then up. Containers are not touched before up, but one that mounts files from the clone sees the new commit's files during that time, and the old ones again if the deploy is put back. An app that reloads its config on change can pick up a commit that is then put back. Say so, with what to do about it. Raised by Copilot on #172. Checking the new commit out only right before up (a staging copy for the checks and build) would narrow this, but is a change of its own. Co-Authored-By: Claude Opus 5.5 --- docs/git-stacks.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/git-stacks.md b/docs/git-stacks.md index 36fadd99..9445c128 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -317,6 +317,13 @@ is, the deploy stops instead. - **Before `up`** (a check fails, the pull or build fails): the previous commit is put back and no container was changed. Fix the cause, usually in the repository, and deploy again. + + "No container was changed" is about the containers themselves. A running container that mounts a file or + folder from the clone (`./config`, say) sees the new commit's files from the moment it is checked out, while + the checks, pull and build run, and the old ones again once it is put back. Most apps read their config only + when they start, so this does not matter to them. For an app that reloads its config when the file changes + (a proxy watching its config folder, say), keep that config at an absolute path outside the clone if a + deploy that is put back must not reach it. - **During `up`** (a container fails to start, or does not get healthy with `--wait`): a half-finished `up` cannot be undone safely, so it is not rolled back. The commit is recorded as failed and shown by `compose-git status`. Fix it in the repository and deploy again, or deploy a known good commit with From b59c877a899539fdddc4583af2d47a87962d2279 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:21:12 -0400 Subject: [PATCH 09/10] fix(git): tell a missing network or volume from one Docker could not be asked about A deploy's check read any failed "docker network inspect" or "docker volume inspect" as "does not exist". With the daemon not answering, the deploy still stopped, but told the user to create a network or volume that may well exist. Only Docker's own "not found" answer now means missing; any other failure says the check could not be made, with Docker's message, as the container name check already does. The test stand-in now prints Docker's real "not found" messages, so the tests reach both answers. Found while checking Copilot's findings on #172. Co-Authored-By: Claude Opus 5.5 --- docs/git-stacks.md | 3 +- .../include/GitDeployCheck.php | 35 ++++++++++++++++++- tests/unit/GitDeployCheckTest.php | 35 +++++++++++++++++-- tests/unit/fixtures/fake-docker.sh | 7 +++- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/docs/git-stacks.md b/docs/git-stacks.md index 9445c128..cd48a88e 100644 --- a/docs/git-stacks.md +++ b/docs/git-stacks.md @@ -352,7 +352,8 @@ without changing anything when: folder, is missing; - a `container_name` or published port is already taken by another stack or container, or two services of the stack publish the same port; -- Docker could not be asked whether a name or port is free (the daemon is not answering, say). +- Docker could not be asked whether a name or port is free, or whether an external network or volume exists + (the daemon is not answering, say). The deploy log says which `.env` was used. diff --git a/source/compose.manager/include/GitDeployCheck.php b/source/compose.manager/include/GitDeployCheck.php index 8cbf3593..d7498ec4 100644 --- a/source/compose.manager/include/GitDeployCheck.php +++ b/source/compose.manager/include/GitDeployCheck.php @@ -173,8 +173,14 @@ private function missingExternalResources(array $resolved): array continue; } $name = (string) ($definition['name'] ?? $key); - if (!DockerCommand::run([$kind, 'inspect', '--', $name], null, 30)->succeeded()) { + $inspect = DockerCommand::run([$kind, 'inspect', '--', $name], null, 30); + if ($inspect->succeeded()) { + continue; + } + if (self::failedOnlyOnMissing($inspect, $kind)) { $problems[] = "The external $kind '$name' does not exist. Create it before deploying."; + } else { + $problems[] = "Could not check whether the external $kind '$name' exists: " . self::failureText($inspect); } } } @@ -529,6 +535,33 @@ private static function failedOnlyOnMissingContainers(ProcessResult $inspect): b return true; } + /** + * Whether "docker network inspect" or "docker volume inspect" failed only because + * nothing has that name, rather than because Docker could not be asked. + */ + private static function failedOnlyOnMissing(ProcessResult $inspect, string $kind): bool + { + if ($inspect->timedOut) { + return false; + } + $lines = array_filter(array_map('trim', explode("\n", $inspect->stderr)), static fn(string $line): bool => $line !== ''); + if ($lines === []) { + return false; + } + foreach ($lines as $line) { + // Docker today: "Error response from daemon: network x not found" and + // "Error response from daemon: get x: no such volume". Older Docker: + // "Error: No such network: x" and "Error: No such volume: x". + $missing = $kind === 'network' + ? preg_match('/network .+ not found$/i', $line) === 1 || stripos($line, 'no such network') !== false + : stripos($line, 'no such volume') !== false; + if (!$missing) { + return false; + } + } + return true; + } + private static function failureText(ProcessResult $result): string { return trim($result->stderr) !== '' ? trim($result->stderr) : $result->errorSummary(); diff --git a/tests/unit/GitDeployCheckTest.php b/tests/unit/GitDeployCheckTest.php index dad6d34e..0678eb4e 100644 --- a/tests/unit/GitDeployCheckTest.php +++ b/tests/unit/GitDeployCheckTest.php @@ -123,14 +123,45 @@ public function testMissingExternalNetworkAndVolumeAreProblems(): void $problems = $this->check(); $this->assertCount(2, $problems); - $this->assertStringContainsString("network 'zz-proxy'", $problems[0]); - $this->assertStringContainsString("volume 'zz-ext'", $problems[1]); + $this->assertStringContainsString("The external network 'zz-proxy' does not exist", $problems[0]); + $this->assertStringContainsString("The external volume 'zz-ext' does not exist", $problems[1]); $this->docker->addNetwork('zz-proxy'); $this->docker->addVolume('zz-ext'); $this->assertSame([], $this->check()); } + public function testExternalNetworkAndVolumeThatDockerCannotBeAskedAboutAreNotCalledMissing(): void + { + $config = $this->config(['image' => 'busybox']); + $config['networks'] = ['proxy' => ['name' => 'zz-proxy', 'external' => true]]; + $config['volumes'] = ['ext' => ['name' => 'zz-ext', 'external' => true]]; + $this->docker->setConfig($config); + $daemonDown = 'Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?'; + $this->docker->fail('network', $daemonDown); + $this->docker->fail('volume', $daemonDown); + + $problems = $this->check(); + $this->assertCount(2, $problems); + $this->assertStringContainsString("Could not check whether the external network 'zz-proxy' exists: Cannot connect", $problems[0]); + $this->assertStringContainsString("Could not check whether the external volume 'zz-ext' exists: Cannot connect", $problems[1]); + } + + public function testOlderDockersWordingForAMissingNetworkOrVolumeCountsAsMissing(): void + { + $config = $this->config(['image' => 'busybox']); + $config['networks'] = ['proxy' => ['name' => 'zz-proxy', 'external' => true]]; + $config['volumes'] = ['ext' => ['name' => 'zz-ext', 'external' => true]]; + $this->docker->setConfig($config); + $this->docker->fail('network', 'Error: No such network: zz-proxy'); + $this->docker->fail('volume', 'Error: No such volume: zz-ext'); + + $problems = $this->check(); + $this->assertCount(2, $problems); + $this->assertStringContainsString("The external network 'zz-proxy' does not exist", $problems[0]); + $this->assertStringContainsString("The external volume 'zz-ext' does not exist", $problems[1]); + } + public function testMissingExternalNetworkIsNotAProblemWhenItWillBeCreated(): void { $config = $this->config(['image' => 'busybox']); diff --git a/tests/unit/fixtures/fake-docker.sh b/tests/unit/fixtures/fake-docker.sh index 278218bc..9775a782 100644 --- a/tests/unit/fixtures/fake-docker.sh +++ b/tests/unit/fixtures/fake-docker.sh @@ -56,7 +56,12 @@ case "$1" in cat "$here/$kind/$name" exit 0 fi - echo "Error: No such $1: $name" >&2 + # What Docker prints for a network or volume that does not exist. + if [ "$1" = network ]; then + echo "Error response from daemon: network $name not found" >&2 + else + echo "Error response from daemon: get $name: no such volume" >&2 + fi exit 1 ;; ps) From 7b065ce59ba45728e39362839ae577c5c2fbd3e4 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Thu, 8 Oct 2026 15:52:23 -0400 Subject: [PATCH 10/10] fix(git): refuse a git home folder someone else made first Git runs for git stacks use a home folder in /var/tmp, where anyone can create things, and the global config git reads (the list of trusted repositories) is kept there. A folder already there was used as it was: if another user made it first, or put a symlink there, they could give git a config of their own, which root then ran with. The folder is now used only when it is a plain folder owned by whoever runs the plugin (root on Unraid), and made private; otherwise git is not run and the result says why. Found while checking Copilot's findings on #174, which raised the same pattern for the folder that holds git credentials. Co-Authored-By: Claude Opus 5.5 --- source/compose.manager/include/GitCommand.php | 35 +++++++++++++++---- tests/unit/GitCommandTest.php | 24 +++++++++++++ 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/source/compose.manager/include/GitCommand.php b/source/compose.manager/include/GitCommand.php index 5ea5bddb..b6688e75 100644 --- a/source/compose.manager/include/GitCommand.php +++ b/source/compose.manager/include/GitCommand.php @@ -62,7 +62,13 @@ public static function run( array $extraEnvironment = [], array $trustedRepositories = [] ): ProcessResult { - $homeDirectory = self::prepareHomeDirectory(); + $homeDirectory = COMPOSE_GIT_HOME_DIR; + $homeProblem = self::homeDirectoryProblem($homeDirectory); + if ($homeProblem !== null) { + // 128, git's own code for "could not run": some callers read exit code 1 as an + // answer (merge-base --is-ancestor says "no" with 1). + return new ProcessResult(128, '', $homeProblem, false); + } $command = ['git']; foreach (self::FORCED_CONFIG as $setting) { @@ -142,14 +148,29 @@ private static function trustFile(string $homeDirectory, ?string $workingDirecto } /** - * The empty folder git runs with as HOME, so no personal config is read. + * Make the folder git runs with as HOME, so no personal config is read, and + * say what is wrong with it, if anything. + * + * It is in /var/tmp, where anyone can create things, and the global config + * git reads is kept in it: a folder someone else made first (or a symlink + * there) would let its owner give git a config of their own. So only a plain + * folder owned by whoever runs this (root on Unraid) is used. */ - private static function prepareHomeDirectory(): string + private static function homeDirectoryProblem(string $home): ?string { - $home = COMPOSE_GIT_HOME_DIR; - if (!is_dir($home)) { - @mkdir($home, 0700, true); + if (!file_exists($home) && @readlink($home) === false && !@mkdir($home, 0700, true) && !is_dir($home)) { + return "Could not create $home for git."; + } + clearstatcache(true, $home); + if (@readlink($home) !== false || !is_dir($home)) { + return "$home is not a plain folder, so git was not run. Remove it."; + } + if (@fileowner($home) !== (function_exists('posix_geteuid') ? posix_geteuid() : 0)) { + return "$home belongs to another user, so git was not run. Remove it."; + } + if (!chmod($home, 0700)) { + return "Could not make $home private, so git was not run."; } - return $home; + return null; } } diff --git a/tests/unit/GitCommandTest.php b/tests/unit/GitCommandTest.php index a70af62d..cd36d875 100644 --- a/tests/unit/GitCommandTest.php +++ b/tests/unit/GitCommandTest.php @@ -39,6 +39,30 @@ public function testRunsGitAndCapturesOutput(): void $this->assertStringStartsWith('git version', $result->stdout); } + public function testAGitHomeThatIsASymlinkIsRefused(): void + { + // The home folder is in /var/tmp, where anyone can create things first, and holds the + // global config git reads. (One another user owns is refused too, but a test that does + // not run as root cannot make one.) + $home = rtrim(COMPOSE_GIT_HOME_DIR, '/'); + $target = sys_get_temp_dir() . '/compose_git_home_elsewhere'; + @mkdir($target, 0755); + $moved = $home . '.real'; + @rename($home, $moved); + symlink($target, $home); + try { + $result = GitCommand::run(['--version']); + } finally { + unlink($home); + @rename($moved, $home); + } + + $this->assertFalse($result->succeeded()); + $this->assertStringContainsString('is not a plain folder', $result->errorSummary()); + $this->assertSame(0755, fileperms($target) & 0777); + rmdir($target); + } + public function testFailureReportsTheLastErrorLine(): void { $result = GitCommand::run(['rev-parse', '--verify', 'no-such-ref'], $this->makeRepo());