diff --git a/internal/config/custom_service.go b/internal/config/custom_service.go index c92cb6a9..3d6f03a2 100644 --- a/internal/config/custom_service.go +++ b/internal/config/custom_service.go @@ -1,6 +1,8 @@ package config import ( + "crypto/sha256" + "encoding/hex" "fmt" "os" "path/filepath" @@ -747,12 +749,22 @@ func MaterializeServiceFilesChanged(svc *CustomService) (bool, error) { // importantly, avoids a needless service restart when nothing changed. The // mode is still enforced so a re-materialise stays idempotent on permissions, // but a mode-only fixup is not a content change and triggers no restart. - if existing, err := os.ReadFile(path); err == nil && string(existing) == content { + existing, readErr := os.ReadFile(path) + if readErr == nil && string(existing) == content { if info, statErr := os.Stat(path); statErr == nil && info.Mode().Perm() != mode.Perm() { if err := os.Chmod(path, mode); err != nil { return changed, fmt.Errorf("chmod %s: %w", path, err) } } + syncContentSidecar(path, content, f.Chown) + continue + } + // A restrictive chown:true mount is unreadable to us once podman's :U has + // re-owned it, so the compare above cannot run and every pass would rewrite + // the file, move its mtime and earn another restart. Compare against the + // hash instead. The cost is that bytes tampered with in place are no longer + // repaired while the file stays unreadable. + if readErr != nil && !os.IsNotExist(readErr) && contentMatchesSidecar(path, content) { continue } // Unlink first: with chown:true podman's :U flag re-owns the file to a @@ -770,10 +782,36 @@ func MaterializeServiceFilesChanged(svc *CustomService) (bool, error) { return changed, fmt.Errorf("chmod %s: %w", path, err) } changed = true + syncContentSidecar(path, content, f.Chown) } return changed, nil } +// contentSidecarPath is the hash file recording what was last materialised into +// path. It is never mounted, so podman's :U never re-owns it and it stays +// readable when the file it describes does not. +func contentSidecarPath(path string) string { return path + ".sha256" } + +// syncContentSidecar records the hash for the mounts that can become unreadable, +// so only those carry the extra file. A failed write costs one more rewrite on +// the next pass, which is not worth failing a materialise over. +func syncContentSidecar(path, content string, chown bool) { + if !chown || contentMatchesSidecar(path, content) { + return + } + sum := sha256.Sum256([]byte(content)) + _ = os.WriteFile(contentSidecarPath(path), []byte(hex.EncodeToString(sum[:])), 0644) +} + +func contentMatchesSidecar(path, content string) bool { + stored, err := os.ReadFile(contentSidecarPath(path)) + if err != nil { + return false + } + sum := sha256.Sum256([]byte(content)) + return string(stored) == hex.EncodeToString(sum[:]) +} + // customServicePath returns the on-disk definition path for a service name, or // an error when the name is not one SaveCustomService could have written. A // name indexes straight into a file path, and callers derive it from things diff --git a/internal/config/custom_service_test.go b/internal/config/custom_service_test.go index 503f7a33..01f8e7e0 100644 --- a/internal/config/custom_service_test.go +++ b/internal/config/custom_service_test.go @@ -189,6 +189,88 @@ func TestMaterializeServiceFilesChanged_DetectsContentDrift(t *testing.T) { } } +// podman :U re-owns the file so os.ReadFile EACCES; the sidecar hash must +// prevent a rewrite so mtime does not move. +func TestMaterializeServiceFilesChanged_UnreadableFileUsesSidecar(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_DATA_HOME", tmp) + svc := &CustomService{Name: "pgadmin", Image: "docker.io/dpage/pgadmin4:latest", Preset: "pgadmin"} + + changed, err := MaterializeServiceFilesChanged(svc) + if err != nil { + t.Fatalf("first materialize: %v", err) + } + if !changed { + t.Fatal("first materialize must report a change") + } + + // Simulate podman re-owning the file: make it unreadable by the host user. + path := ServiceFilePath(svc.Name, "/pgpass") + if err := os.Chmod(path, 0o000); err != nil { + t.Fatalf("chmod 000: %v", err) + } + + mtimeBefore, err := os.Stat(path) + if err != nil { + t.Fatalf("stat before: %v", err) + } + + changed, err = MaterializeServiceFilesChanged(svc) + if err != nil { + t.Fatalf("second materialize (unreadable): %v", err) + } + if changed { + t.Fatal("an unreadable file whose content has not changed must not be rewritten") + } + + mtimeAfter, err := os.Stat(path) + if err != nil { + t.Fatalf("stat after: %v", err) + } + if !mtimeAfter.ModTime().Equal(mtimeBefore.ModTime()) { + t.Fatal("mtime moved on an unchanged unreadable file, the drift loop is not fixed") + } + + // Restore so cleanup can remove it. + _ = os.Chmod(path, 0o600) +} + +// The other half of the sidecar: a shipped preset change must still reach an +// unreadable file, or the hash would hide the drift it exists to detect. +func TestMaterializeServiceFilesChanged_DriftReachesAnUnreadableFile(t *testing.T) { + tmp := t.TempDir() + t.Setenv("XDG_DATA_HOME", tmp) + svc := &CustomService{Name: "pgadmin", Image: "docker.io/dpage/pgadmin4:latest", Preset: "pgadmin"} + + if _, err := MaterializeServiceFilesChanged(svc); err != nil { + t.Fatalf("first materialize: %v", err) + } + path := ServiceFilePath(svc.Name, "/pgpass") + if err := os.Chmod(path, 0o000); err != nil { + t.Fatalf("chmod 000: %v", err) + } + + const updated = "lerd-postgres-18:5432:*:postgres:lerd\n" + prev := presetFileGenerators["pgadmin_pgpass"] + t.Cleanup(func() { presetFileGenerators["pgadmin_pgpass"] = prev }) + presetFileGenerators["pgadmin_pgpass"] = func(*CustomService) (string, error) { return updated, nil } + + changed, err := MaterializeServiceFilesChanged(svc) + if err != nil { + t.Fatalf("materialize after the rendering changed: %v", err) + } + if !changed { + t.Fatal("a new rendering must be written even when the old file cannot be read") + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + if string(got) != updated { + t.Errorf("content = %q, want %q", got, updated) + } +} + func TestValidateCustomService_rejectsEnvInjection(t *testing.T) { svc := &CustomService{Name: "evil", Image: "alpine", Environment: map[string]string{"X": "ok\nPodmanArgs=--privileged"}}