From c4f1e8d7cb1f7b630ced50fa3086eb7b5cd1db22 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 24 Apr 2026 09:21:42 +0100 Subject: [PATCH] Avoid tenant runtime image copy-up --- docker-entrypoint.sh | 31 +++++++++-- .../v6/internal/subsystems/cloud-paid.md | 5 ++ .../subsystems/deployment-installability.md | 4 ++ .../v6/internal/subsystems/monitoring.md | 4 ++ .../installtests/docker_entrypoint_test.go | 54 +++++++++++++++++++ 5 files changed, 94 insertions(+), 4 deletions(-) diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 7ff1a9a61..8322d9d93 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -19,6 +19,10 @@ is_immutable_ownership_path() { return 1 } +path_owner() { + stat -c '%u:%g' "$1" 2>/dev/null || stat -f '%u:%g' "$1" 2>/dev/null || echo "" +} + chown_tree_skipping_immutable_paths() { owner="$1" root="$2" @@ -43,6 +47,24 @@ chown_tree_skipping_immutable_paths() { chown "$owner" "$root" } +chown_tree_if_owner_mismatch() { + owner="$1" + uid="$2" + gid="$3" + root="$4" + + if [ ! -e "$root" ]; then + return 0 + fi + + current_owner=$(path_owner "$root") + if [ "$current_owner" = "$uid:$gid" ]; then + return 0 + fi + + chown -R "$owner" "$root" +} + # Only adjust permissions if running as root if [ "$(id -u)" = "0" ]; then echo "Starting with UID: $PUID, GID: $PGID" @@ -50,8 +72,7 @@ if [ "$(id -u)" = "0" ]; then # If PUID is 0 (root), don't create a new user, just run as root if [ "$PUID" = "0" ]; then echo "Running as root user" - # Fix ownership to root - chown -R root:root /data /app /opt/pulse + chown_tree_if_owner_mismatch root:root 0 0 /data chown_tree_skipping_immutable_paths root:root /etc/pulse exec "$@" fi @@ -71,8 +92,10 @@ if [ "$(id -u)" = "0" ]; then adduser -D -u "$PUID" -G pulse pulse fi - # Fix ownership of data directory - chown -R pulse:pulse /data /app /opt/pulse + # Only mutable data paths are recursively fixed at startup. Immutable image + # paths such as /app and /opt/pulse are owned during the image build; chowning + # them here causes overlayfs copy-up for every tenant container. + chown_tree_if_owner_mismatch pulse:pulse "$PUID" "$PGID" /data chown_tree_skipping_immutable_paths pulse:pulse /etc/pulse # Switch to pulse user diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 3753b028b..d60439cc4 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -1623,6 +1623,11 @@ control plane may bind-mount billing and handoff files into `/etc/pulse` as read-only inputs, but runtime startup ownership repair must treat those paths as immutable and skip `chown` attempts against them instead of aborting tenant provisioning. +That startup repair boundary also treats image-owned runtime paths as +immutable. The container entrypoint may repair writable tenant data paths, but +it must not recursively `chown` built image paths such as `/app` or +`/opt/pulse`, because doing so copy-ups the runtime image into every hosted +tenant writable layer and turns fleet reconciliation into host-disk pressure. That same immutable-file boundary now also owns write-time runtime ownership: control-plane provisioning and later billing-state rewrites must leave `billing.json`, `secrets/handoff.key`, and `.cloud_handoff_key` readable by diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 300420212..44db92ebb 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -545,6 +545,10 @@ batch-reconcile path that preserves each tenant runtime's current image line, supports dry-run planning before mutation, and converges existing hosted tenants onto the canonical runtime contract without relying on ad hoc host scripts or one-off manual tenant loops. +That same hosted runtime container boundary owns startup ownership repair: +entrypoints may repair writable runtime data paths, but must not recursively +`chown` immutable image paths such as `/app` or `/opt/pulse`, because overlayfs +copy-up makes every tenant recreate consume image-sized writable disk. That same rule applies to live runtime behavior too: config loading and reload watching may not treat `mock.env` as a parallel primary-path control surface. Supported mock-mode runtime state must come from the canonical `.env` contract, diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index f11552168..1c83426be 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -448,6 +448,10 @@ boundary. Hosted or managed tenant bootstrap changes must preserve safe startup when immutable read-only mounts are layered into `/etc/pulse`; the entrypoint may not reintroduce ownership mutation against those read-only files during container boot. +That same startup path must avoid recursive ownership mutation of image-owned +runtime directories such as `/app` and `/opt/pulse`; those paths are build-time +artifacts, and copy-up into per-container writable layers is a monitoring and +host-health regression, not a valid runtime repair. That same monitoring boundary now also owns Docker Swarm runtime truth at the collection seam. `internal/dockeragent/swarm.go` is the canonical manager-side filter for live Swarm services and tasks, so monitoring consumers do not ingest diff --git a/scripts/installtests/docker_entrypoint_test.go b/scripts/installtests/docker_entrypoint_test.go index ca1450ddf..fb8599139 100644 --- a/scripts/installtests/docker_entrypoint_test.go +++ b/scripts/installtests/docker_entrypoint_test.go @@ -102,6 +102,60 @@ chown_tree_skipping_immutable_paths pulse:pulse "$root" } } +func TestDockerEntrypointDoesNotChownAlreadyOwnedTree(t *testing.T) { + repoRoot := repoRoot(t) + entrypointPath := filepath.Join(repoRoot, "docker-entrypoint.sh") + + entrypoint, err := os.ReadFile(entrypointPath) + if err != nil { + t.Fatalf("read docker-entrypoint.sh: %v", err) + } + + const marker = "# Only adjust permissions if running as root" + prefix, _, found := strings.Cut(string(entrypoint), marker) + if !found { + t.Fatalf("docker-entrypoint.sh missing marker %q", marker) + } + + root := t.TempDir() + binDir := filepath.Join(t.TempDir(), "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatalf("mkdir bin dir: %v", err) + } + chownLog := filepath.Join(t.TempDir(), "chown.log") + chownStub := filepath.Join(binDir, "chown") + chownScript := `#!/bin/sh +set -eu +printf '%s\n' "$*" >> "$CHOWN_LOG" +exit 0 +` + if err := os.WriteFile(chownStub, []byte(chownScript), 0o755); err != nil { + t.Fatalf("write chown stub: %v", err) + } + + shell := prefix + ` +set -eu +root="` + root + `" +uid="$(id -u)" +gid="$(id -g)" +export CHOWN_LOG="` + chownLog + `" +export PATH="` + binDir + `:$PATH" +chown_tree_if_owner_mismatch pulse:pulse "$uid" "$gid" "$root" +` + + cmd := exec.Command("sh", "-c", shell) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("run helper: %v\n%s", err, out) + } + + if logData, err := os.ReadFile(chownLog); err == nil && len(logData) > 0 { + t.Fatalf("already-owned tree was chowned:\n%s", logData) + } else if err != nil && !os.IsNotExist(err) { + t.Fatalf("read chown log: %v", err) + } +} + func repoRoot(t *testing.T) string { t.Helper()