From ea503acd4213a9fdedd5f8b25aff4fc9d0b3065a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 11 Sep 2026 15:01:01 +0000 Subject: [PATCH] Respect deployment permissions for chunk plans --- docs/roadmap/implementation.md | 2 + internal/framework/chunkplan/store.go | 39 +--------- .../chunkplan/store_permissions_unix_test.go | 76 +++++++++++++++++++ internal/framework/chunkplan/store_test.go | 22 ------ 4 files changed, 81 insertions(+), 58 deletions(-) create mode 100644 internal/framework/chunkplan/store_permissions_unix_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index cecc267a..d153dc53 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -120,6 +120,8 @@ stage is small enough for one implementation prompt. ## Stage 2: Convert Canonical Chunk-Plan Storage +**Completed:** 2026-09-11 + ### Goal Make every chunk-plan creation and replacement path follow deployment-managed diff --git a/internal/framework/chunkplan/store.go b/internal/framework/chunkplan/store.go index 817962a8..50e4de9c 100644 --- a/internal/framework/chunkplan/store.go +++ b/internal/framework/chunkplan/store.go @@ -140,7 +140,7 @@ func (s *filesystemStore) openRoot(create bool) (*os.Root, error) { return nil, fmt.Errorf("chunk plan store must not be nil") } if create { - if err := os.MkdirAll(s.root, 0o700); err != nil { + if err := os.MkdirAll(s.root, 0o777); err != nil { return nil, err } } @@ -148,19 +148,6 @@ func (s *filesystemStore) openRoot(create bool) (*os.Root, error) { if err != nil { return nil, err } - if !create { - return root, nil - } - rootDirectory, err := root.Open(".") - if err != nil { - _ = root.Close() - return nil, err - } - defer rootDirectory.Close() - if err := rootDirectory.Chmod(0o700); err != nil { - _ = root.Close() - return nil, err - } return root, nil } @@ -207,7 +194,7 @@ func openDigestRootWithHooks(root *os.Root, digestDir string, create bool, hooks if !create { return nil, entryMissing, nil } - if err := root.Mkdir(digestDir, 0o700); err != nil && !errors.Is(err, os.ErrExist) { + if err := root.Mkdir(digestDir, 0o777); err != nil && !errors.Is(err, os.ErrExist) { return nil, entryPresent, err } continue @@ -238,22 +225,6 @@ func openDigestRootWithHooks(root *os.Root, digestDir string, create bool, hooks } continue } - if create { - directory, openErr := digestRoot.Open(".") - if openErr != nil { - _ = digestRoot.Close() - return nil, entryPresent, openErr - } - chmodErr := directory.Chmod(0o700) - closeErr := directory.Close() - if chmodErr != nil || closeErr != nil { - _ = digestRoot.Close() - if chmodErr != nil { - return nil, entryPresent, chmodErr - } - return nil, entryPresent, closeErr - } - } return digestRoot, entryPresent, nil } } @@ -433,10 +404,6 @@ func writeAtomicWithHooks(root *os.Root, target string, data []byte, hooks atomi _ = root.Remove(tempPath) } }() - if err := temp.Chmod(0o600); err != nil { - _ = temp.Close() - return err - } if _, err := temp.Write(data); err != nil { _ = temp.Close() return err @@ -467,7 +434,7 @@ func createTemporaryFile(root *os.Root, target string) (*os.File, string, error) return nil, "", err } path := strings.TrimSuffix(target, planFileName) + ".plan.json.tmp-" + hex.EncodeToString(suffix[:]) - file, err := root.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + file, err := root.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666) if errors.Is(err, os.ErrExist) { continue } diff --git a/internal/framework/chunkplan/store_permissions_unix_test.go b/internal/framework/chunkplan/store_permissions_unix_test.go new file mode 100644 index 00000000..b45114b4 --- /dev/null +++ b/internal/framework/chunkplan/store_permissions_unix_test.go @@ -0,0 +1,76 @@ +//go:build linux || darwin + +package chunkplan + +import ( + "os" + "path/filepath" + "strings" + "syscall" + "testing" +) + +func TestFilesystemStorePermissions(t *testing.T) { + for _, test := range []struct { + name string + umask int + dirMode os.FileMode + fileMode os.FileMode + }{ + {name: "private", umask: 0o077, dirMode: 0o700, fileMode: 0o600}, + {name: "group", umask: 0o007, dirMode: 0o770, fileMode: 0o660}, + } { + t.Run(test.name, func(t *testing.T) { + previousUmask := syscall.Umask(test.umask) + defer syscall.Umask(previousUmask) + + root := filepath.Join(t.TempDir(), "plans") + store := newStore(t, root) + if err := store.Save(testRecord(t, 1)); err != nil { + t.Fatal(err) + } + digestDir := filepath.Join(root, strings.TrimPrefix(testSourceDigest, "sha256:")) + for path, want := range map[string]os.FileMode{ + root: test.dirMode, + digestDir: test.dirMode, + filepath.Join(digestDir, "plan.json"): test.fileMode, + } { + assertStorePermissions(t, path, want) + } + }) + } +} + +func TestFilesystemStoreLeavesExistingDirectoriesUntouched(t *testing.T) { + previousUmask := syscall.Umask(0o077) + defer syscall.Umask(previousUmask) + + root := filepath.Join(t.TempDir(), "plans") + digestDir := filepath.Join(root, strings.TrimPrefix(testSourceDigest, "sha256:")) + syscall.Umask(0) + if err := os.Mkdir(root, 0o751); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(digestDir, 0o753); err != nil { + t.Fatal(err) + } + syscall.Umask(0o077) + + store := newStore(t, root) + if err := store.Save(testRecord(t, 1)); err != nil { + t.Fatal(err) + } + assertStorePermissions(t, root, 0o751) + assertStorePermissions(t, digestDir, 0o753) +} + +func assertStorePermissions(t *testing.T, path string, want os.FileMode) { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != want { + t.Fatalf("%s mode = %#o, want %#o", path, got, want) + } +} diff --git a/internal/framework/chunkplan/store_test.go b/internal/framework/chunkplan/store_test.go index a3b28d68..194aa505 100644 --- a/internal/framework/chunkplan/store_test.go +++ b/internal/framework/chunkplan/store_test.go @@ -61,28 +61,6 @@ func TestFilesystemStoreRoundTripAndExactPath(t *testing.T) { } } -func TestFilesystemStorePermissions(t *testing.T) { - root := filepath.Join(t.TempDir(), "plans") - store := newStore(t, root) - if err := store.Save(testRecord(t, 1)); err != nil { - t.Fatal(err) - } - digestDir := filepath.Join(root, strings.TrimPrefix(testSourceDigest, "sha256:")) - for path, want := range map[string]os.FileMode{ - root: 0o700, - digestDir: 0o700, - filepath.Join(digestDir, "plan.json"): 0o600, - } { - info, err := os.Stat(path) - if err != nil { - t.Fatal(err) - } - if got := info.Mode().Perm(); got != want { - t.Fatalf("%s mode = %04o, want %04o", path, got, want) - } - } -} - func TestFilesystemStoreMissingAndOperationalErrors(t *testing.T) { root := filepath.Join(t.TempDir(), "plans") store := newStore(t, root)