From c217c4a41708f1e8f535dfcf82b5e607ac645f80 Mon Sep 17 00:00:00 2001 From: Pi Web Agent Date: Mon, 13 Jul 2026 14:52:46 +0000 Subject: [PATCH] fix(docker): refuse unsafe development updates --- docker/README.md | 6 +- docker/pi-web-docker | 61 ++++++++++++++++ src/server/dockerControlAssets.test.ts | 99 +++++++++++++++++++++++++- 3 files changed, 164 insertions(+), 2 deletions(-) diff --git a/docker/README.md b/docker/README.md index ff5bda8..72bdafc 100644 --- a/docker/README.md +++ b/docker/README.md @@ -73,7 +73,7 @@ From a production/runtime install directory, run `./pi-web-docker `. Fr | `restart` | `./pi-web-docker restart` | `./docker/pi-web-docker --dev restart` | Restarts `web` and `sessiond`. | | `restart-web` | `./pi-web-docker restart-web` | `./docker/pi-web-docker --dev restart-web` | Restarts only the web/API service. | | `restart-sessiond` | `./pi-web-docker restart-sessiond` | `./docker/pi-web-docker --dev restart-sessiond` | Restarts the session daemon; active agent runtimes may stop in that Docker stack. | -| `update` | `./pi-web-docker update` | `./docker/pi-web-docker --dev update` | Rebuilds/recreates the stack. Runtime host updates rerun the installer to refresh Docker assets first. | +| `update` | `./pi-web-docker update` | `./docker/pi-web-docker --dev update` | Rebuilds/recreates the stack. Runtime host updates rerun the installer to refresh Docker assets first. Development updates require a clean Git checkout with no Git operation in progress. | | `status` | `./pi-web-docker status` | `./docker/pi-web-docker --dev status` | Shows Docker Compose service status. | | `logs` | `./pi-web-docker logs [web\|sessiond]` | `./docker/pi-web-docker --dev logs [web\|sessiond\|data-init]` | Follows logs; omitting a target follows all services. | | `shell` | `./pi-web-docker shell [web\|sessiond]` | `./docker/pi-web-docker --dev shell [web\|sessiond]` | Opens Bash in `web` by default. | @@ -281,6 +281,10 @@ PI_WEB_DEV_BIND_ADDR=0.0.0.0 \ ./docker/pi-web-docker --dev start ``` +Development `update` is intentionally fail-closed. Before starting a Docker helper or build, it requires this repository to be a clean Git checkout, including no staged, modified, or untracked files, and no merge, rebase, cherry-pick, revert, sequenced operation, or bisect in progress. It never stashes, removes, or rewrites developer work; resolve, commit, stash, or remove that work explicitly and rerun the update. This guard applies only to `update`: `start` and restart commands remain available for normal development against an intentionally dirty checkout. + +The Docker command rebuilds the current checkout; it does not merge branches or resolve source updates. Perform any Git integration separately, then run the guarded Docker update after the checkout is clean. + You can run the dev stack in the background with: ```bash diff --git a/docker/pi-web-docker b/docker/pi-web-docker index 0880d9d..c0d8e64 100755 --- a/docker/pi-web-docker +++ b/docker/pi-web-docker @@ -25,6 +25,7 @@ Commands: restart-web Restart only the web service restart-sessiond Restart only the session daemon update Rebuild/update and recreate the Docker stack + (development mode requires a clean Git checkout) status Show Docker Compose service status logs [web|sessiond|data-init] Follow Docker Compose logs @@ -229,6 +230,60 @@ enforce_dev_root_safety() { [ "$uid" != 0 ] || die "refusing to run Docker development mode as root; retry with --allow-root if this is intentional" } +dev_git_operation() { + git_dir=$1 + if [ -f "$git_dir/MERGE_HEAD" ]; then + printf '%s\n' merge + elif [ -d "$git_dir/rebase-merge" ] || [ -d "$git_dir/rebase-apply" ] || [ -f "$git_dir/REBASE_HEAD" ]; then + printf '%s\n' rebase + elif [ -f "$git_dir/CHERRY_PICK_HEAD" ]; then + printf '%s\n' cherry-pick + elif [ -f "$git_dir/REVERT_HEAD" ]; then + printf '%s\n' revert + elif [ -d "$git_dir/sequencer" ]; then + printf '%s\n' sequenced-operation + elif [ -f "$git_dir/BISECT_LOG" ]; then + printf '%s\n' bisect + else + return 1 + fi +} + +require_clean_dev_update_checkout() { + [ "$(docker_mode)" = dev ] || return 0 + root=$(dev_root) + require_command git + + git_root=$(git -C "$root" rev-parse --show-toplevel 2>/dev/null) \ + || die "Docker development update requires a Git checkout at $root" + git_root=$(absolute_existing_dir "$git_root") \ + || die "could not resolve Git checkout root: $git_root" + [ "$git_root" = "$root" ] \ + || die "Docker development root $root must be the Git checkout root ($git_root)" + git_dir=$(git -C "$root" rev-parse --absolute-git-dir 2>/dev/null) \ + || die "could not resolve Git metadata for $root" + + operation=$(dev_git_operation "$git_dir" 2>/dev/null || true) + if [ -n "$operation" ]; then + log "pi-web-docker: refusing to update the Docker development stack while a Git $operation is in progress: $root" + checkout_status=$(git -C "$root" status --porcelain=v1 --untracked-files=all 2>/dev/null || true) + if [ -n "$checkout_status" ]; then + log "Checkout status:" + printf '%s\n' "$checkout_status" >&2 + fi + die "resolve or abort the Git $operation before rerunning pi-web-docker --dev update" + fi + + checkout_status=$(git -C "$root" status --porcelain=v1 --untracked-files=all) \ + || die "could not inspect Git checkout status at $root" + if [ -n "$checkout_status" ]; then + log "pi-web-docker: refusing to update the Docker development stack because the checkout has uncommitted changes: $root" + log "Checkout status:" + printf '%s\n' "$checkout_status" >&2 + die "commit, stash, or remove these changes before rerunning pi-web-docker --dev update; no files were changed" + fi +} + enforce_container_mode_match() { is_truthy "${PI_WEB_DOCKER_RUNTIME:-}" || return 0 runtime_mode=${PI_WEB_DOCKER_MODE:-} @@ -401,6 +456,7 @@ run_runtime_host_update() { run_update() { assert_no_args update "$@" + require_clean_dev_update_checkout case "$(docker_mode)" in runtime) if ! is_truthy "${PI_WEB_DOCKER_RUNTIME:-}"; then @@ -729,6 +785,11 @@ run_restart_or_update() { shift assert_no_args "$action" "$@" if is_truthy "${PI_WEB_DOCKER_RUNTIME:-}"; then + # Fail before scheduling a helper, then recheck inside the helper in + # run_update so a checkout change cannot race the detached operation. + if [ "$action" = update ]; then + require_clean_dev_update_checkout + fi start_detached_helper "$action" return 0 fi diff --git a/src/server/dockerControlAssets.test.ts b/src/server/dockerControlAssets.test.ts index 4502c7a..12bb43c 100644 --- a/src/server/dockerControlAssets.test.ts +++ b/src/server/dockerControlAssets.test.ts @@ -259,6 +259,84 @@ describe("Docker command assets", () => { expect(await readFile(helperLog, "utf8")).toBe("allow=1 args=ps\n"); }); + dockerCommandIt("refuses development updates when the checkout has uncommitted files", async () => { + const helperLog = join(tempDir, "dev-helper.log"); + const devRoot = await createCleanDevGitRepoWithFakeHelper(helperLog); + const fakeDocker = await installFakeDocker(); + await installFakeId(fakeDocker.binDir, 1234, 2345); + await writeFile(join(devRoot, "staged.txt"), "changed\n", "utf8"); + await execUtf8("git", ["-C", devRoot, "add", "staged.txt"], cleanProcessEnv()); + await writeFile(join(devRoot, "modified.txt"), "changed\n", "utf8"); + await writeFile(join(devRoot, "untracked.txt"), "untracked\n", "utf8"); + + const result = await runDockerCommandAllowFailure( + ["--dev", "update"], + devHostEnv(fakeDocker, devRoot, join(tempDir, "home")), + ); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain("refusing to update the Docker development stack because the checkout has uncommitted changes"); + expect(result.stderr).toContain("staged.txt"); + expect(result.stderr).toContain("modified.txt"); + expect(result.stderr).toContain("?? untracked.txt"); + expect(result.stderr).toContain("commit, stash, or remove these changes"); + await expect(readFile(helperLog, "utf8")).rejects.toThrow(); + }); + + dockerCommandIt("refuses dirty development updates before scheduling a detached helper", async () => { + const helperLog = join(tempDir, "dev-helper.log"); + const devRoot = await createCleanDevGitRepoWithFakeHelper(helperLog); + const fakeDocker = await installFakeDocker(); + await installFakeId(fakeDocker.binDir, 1234, 2345); + await writeFile(join(devRoot, "untracked.txt"), "untracked\n", "utf8"); + + const result = await runDockerCommandAllowFailure(["--dev", "update"], devRuntimeEnv(fakeDocker, devRoot)); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain("checkout has uncommitted changes"); + expect(result.stdout).not.toContain("Started detached PI WEB Docker helper"); + await expect(readFile(fakeDocker.logPath, "utf8")).rejects.toThrow(); + await expect(readFile(helperLog, "utf8")).rejects.toThrow(); + }); + + dockerCommandIt("refuses development updates while a Git operation is in progress", async () => { + const helperLog = join(tempDir, "dev-helper.log"); + const devRoot = await createCleanDevGitRepoWithFakeHelper(helperLog); + const fakeDocker = await installFakeDocker(); + await installFakeId(fakeDocker.binDir, 1234, 2345); + const head = (await execUtf8("git", ["-C", devRoot, "rev-parse", "HEAD"], cleanProcessEnv())).stdout.trim(); + await writeFile(join(devRoot, ".git", "MERGE_HEAD"), `${head}\n`, "utf8"); + + const result = await runDockerCommandAllowFailure( + ["--dev", "update"], + devHostEnv(fakeDocker, devRoot, join(tempDir, "home")), + ); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain("while a Git merge is in progress"); + expect(result.stderr).toContain("resolve or abort the Git merge"); + await expect(readFile(helperLog, "utf8")).rejects.toThrow(); + }); + + dockerCommandIt("allows clean development updates and dirty development starts", async () => { + const helperLog = join(tempDir, "dev-helper.log"); + const devRoot = await createCleanDevGitRepoWithFakeHelper(helperLog); + const fakeDocker = await installFakeDocker(); + await installFakeId(fakeDocker.binDir, 1234, 2345); + const env = devHostEnv(fakeDocker, devRoot, join(tempDir, "home")); + + await runDockerCommand(["--dev", "update"], env); + await writeFile(join(devRoot, "in-progress-work.txt"), "dirty by design\n", "utf8"); + await runDockerCommand(["--dev", "start"], env); + + expect(await readFile(helperLog, "utf8")).toBe([ + "allow=0 args=build --pull", + "allow=0 args=up -d --force-recreate --remove-orphans", + "allow=0 args=up -d --build", + "", + ].join("\n")); + }); + dockerCommandIt("starts development detached helpers as the generated dev user", async () => { const devRoot = await createDevGeneratedEnv({ uid: 1234, gid: 2345, dockerGid: 3456 }); const fakeDocker = await installFakeDocker(); @@ -443,12 +521,31 @@ async function createDevRepoFixtureWithFakeHelper(logPath: string): Promise${shellSingleQuote(logPath)} +printf 'allow=%s args=%s\n' "\${PI_WEB_DOCKER_ALLOW_ROOT:-}" "$*" >>${shellSingleQuote(logPath)} `, "utf8"); await chmod(helperPath, 0o755); return devRoot; } +async function createCleanDevGitRepoWithFakeHelper(logPath: string): Promise { + const devRoot = await createDevRepoFixtureWithFakeHelper(logPath); + await Promise.all([ + writeFile(join(devRoot, "staged.txt"), "clean\n", "utf8"), + writeFile(join(devRoot, "modified.txt"), "clean\n", "utf8"), + ]); + const env = cleanProcessEnv(); + await execUtf8("git", ["init", "--quiet", devRoot], env); + await execUtf8("git", ["-C", devRoot, "add", "."], env); + await execUtf8("git", [ + "-C", devRoot, + "-c", "user.name=PI WEB Test", + "-c", "user.email=pi-web-test@example.invalid", + "-c", "core.hooksPath=/dev/null", + "commit", "--quiet", "--no-gpg-sign", "-m", "test fixture", + ], env); + return devRoot; +} + async function createDevGeneratedEnv(ids: { uid: number; gid: number; dockerGid: number }): Promise { const devRoot = join(tempDir, "dev-runtime"); await mkdir(join(devRoot, ".pi-web"), { recursive: true });