From 153b1e468974b5efbac5949cfe429423a4c26e78 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 11 Sep 2026 14:59:27 +0000 Subject: [PATCH] Add deployment-managed cache writer --- ...defer-cache-access-policy-to-deployment.md | 59 ++++++++ docs/roadmap/implementation.md | 2 + internal/core/fileio/fileio.go | 81 +++++++++++ .../fileio_process_permissions_unix_test.go | 131 ++++++++++++++++++ 4 files changed, 273 insertions(+) create mode 100644 docs/adr/0017-defer-cache-access-policy-to-deployment.md create mode 100644 internal/core/fileio/fileio_process_permissions_unix_test.go diff --git a/docs/adr/0017-defer-cache-access-policy-to-deployment.md b/docs/adr/0017-defer-cache-access-policy-to-deployment.md new file mode 100644 index 00000000..a3d49c28 --- /dev/null +++ b/docs/adr/0017-defer-cache-access-policy-to-deployment.md @@ -0,0 +1,59 @@ +# ADR-0017: Defer cache access policy to the deployment + +**Status:** Accepted +**Date:** 2026-09-11 + +## Context + +Canonical chunk plans and invocation checkpoints are reconstructible cache +state. They may include source-derived content, so access and retention matter, +but Notarius cannot determine whether a deployment needs a private cache or a +cache shared by a trusted group. The current application-enforced `0700` and +`0600` modes prevent ordinary Unix provisioning from making that choice. + +[ADR-0005](0005-cache-canonical-chunk-plans-by-source.md) governs canonical +chunk-plan reuse, while [ADR-0006](0006-separate-output-cache-and-debug-state.md) +established the output, cache, and debug separation refined by +[ADR-0007](0007-separate-checkpoint-recording-from-reuse.md). None of those +decisions requires Notarius to own exact cache access modes. + +## Decision + +Deployments own cache access policy through provisioned directories, process +umask, setgid inheritance, and default ACLs. Notarius requests conventional +maximum creation modes for cache entries and leaves the kernel to select their +effective permissions. + +Notarius continues to own confined paths, symlink rejection, validation, +atomic publication, cleanup, and recovery. Those correctness boundaries are +independent from deciding which accounts may access cache state. + +Source-derived cache contents can be sensitive. Operators own their access and +retention policy, while API credentials remain outside cache payloads under the +existing secret boundary. Existing cache inode modes are not migrated +automatically. + +This decision does not supersede ADR-0005, ADR-0006, or ADR-0007. + +## Alternatives considered + +- Continue enforcing private `0700` directory and `0600` file modes. This + makes one deployment choice for every operator and prevents trusted-group + provisioning. +- Add configurable Notarius permission modes or a private/group cache setting. + This preserves application ownership of a deployment concern and expands the + public configuration surface without a durable policy reason. +- Recursively converge existing cache ownership, modes, or ACLs. This would + make upgrades mutate operator-managed state and cannot safely infer an + intended policy. + +## Consequences + +Operators can use a restrictive umask for private cache state or provision a +setgid/default-ACL cache root for trusted-group access. They must deliberately +recreate or administratively migrate pre-existing cache trees when their +existing modes do not match the desired deployment policy. + +Notarius does not add user or group management, ACL mutation, permission +configuration, or recursive cache migration. Output and debug permission +contracts remain separate from this decision. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 309cd2b2..cecc267a 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -38,6 +38,8 @@ Every stage must preserve these decisions: ## Stage 1: Record The Decision And Add An Umask-Respecting Atomic Writer +**Completed:** 2026-09-11 + ### Goal Record the durable ownership boundary and add the narrow filesystem primitive diff --git a/internal/core/fileio/fileio.go b/internal/core/fileio/fileio.go index f1951543..60ceca0b 100644 --- a/internal/core/fileio/fileio.go +++ b/internal/core/fileio/fileio.go @@ -2,7 +2,10 @@ package fileio import ( + "crypto/rand" + "encoding/hex" "encoding/json" + "errors" "fmt" "os" "path" @@ -92,6 +95,16 @@ func WriteJSON(root, name string, payload any, dirMode, fileMode os.FileMode) er return WriteBytes(root, name, append(data, '\n'), dirMode, fileMode) } +// WriteJSONWithProcessPermissions writes indented JSON beneath root using the +// process and filesystem creation policy for new directories and files. +func WriteJSONWithProcessPermissions(root, name string, payload any) error { + data, err := json.MarshalIndent(payload, "", " ") + if err != nil { + return fmt.Errorf("marshal artifact %q: %w", name, err) + } + return WriteBytesWithProcessPermissions(root, name, append(data, '\n')) +} + func WriteBytes(root, name string, data []byte, dirMode, fileMode os.FileMode) error { target, err := SafePath(root, name) if err != nil { @@ -109,6 +122,26 @@ func WriteBytes(root, name string, data []byte, dirMode, fileMode os.FileMode) e return nil } +// WriteBytesWithProcessPermissions atomically writes data beneath root while +// allowing the process umask and filesystem default ACLs to select effective +// permissions for newly created entries. +func WriteBytesWithProcessPermissions(root, name string, data []byte) error { + target, err := SafePath(root, name) + if err != nil { + return err + } + if err := os.MkdirAll(filepath.Dir(target), 0o777); err != nil { + return fmt.Errorf("write artifact %q: %w", name, err) + } + if err := rejectSymlinkComponents(root, name); err != nil { + return err + } + if err := writeAtomicWithProcessPermissions(target, data); err != nil { + return fmt.Errorf("write artifact %q: %w", name, err) + } + return nil +} + func rejectSymlinkComponents(root, name string) error { absRoot, err := filepath.Abs(root) if err != nil { @@ -163,3 +196,51 @@ func writeAtomic(target string, data []byte, fileMode os.FileMode) error { keep = false return nil } + +func writeAtomicWithProcessPermissions(target string, data []byte) error { + directory := filepath.Dir(target) + base := filepath.Base(target) + seen := make(map[string]struct{}, 32) + for attempt := 0; attempt < 32; attempt++ { + suffix := make([]byte, 16) + if _, err := rand.Read(suffix); err != nil { + return fmt.Errorf("generate temporary file name: %w", err) + } + temporaryPath := filepath.Join(directory, "."+base+".tmp-"+hex.EncodeToString(suffix)) + if _, exists := seen[temporaryPath]; exists { + continue + } + seen[temporaryPath] = struct{}{} + + temporary, err := os.OpenFile(temporaryPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666) + if errors.Is(err, os.ErrExist) { + continue + } + if err != nil { + return err + } + return writeAndPublish(temporary, temporaryPath, target, data) + } + return fmt.Errorf("create temporary file for %q: exhausted unique names", target) +} + +func writeAndPublish(temporary *os.File, temporaryPath, target string, data []byte) error { + keep := true + defer func() { + if keep { + _ = os.Remove(temporaryPath) + } + }() + if _, err := temporary.Write(data); err != nil { + _ = temporary.Close() + return err + } + if err := temporary.Close(); err != nil { + return err + } + if err := os.Rename(temporaryPath, target); err != nil { + return err + } + keep = false + return nil +} diff --git a/internal/core/fileio/fileio_process_permissions_unix_test.go b/internal/core/fileio/fileio_process_permissions_unix_test.go new file mode 100644 index 00000000..90c15d93 --- /dev/null +++ b/internal/core/fileio/fileio_process_permissions_unix_test.go @@ -0,0 +1,131 @@ +//go:build linux || darwin + +package fileio + +import ( + "os" + "path/filepath" + "strings" + "syscall" + "testing" +) + +func TestWriteBytesWithProcessPermissionsHonorsUmask(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(), "cache") + if err := WriteBytesWithProcessPermissions(root, "nested/value", []byte("value")); err != nil { + t.Fatal(err) + } + for path, want := range map[string]os.FileMode{ + root: test.dirMode, + filepath.Join(root, "nested"): test.dirMode, + filepath.Join(root, "nested", "value"): test.fileMode, + } { + assertPermissions(t, path, want) + } + assertNoTemporarySiblings(t, filepath.Join(root, "nested")) + }) + } +} + +func TestWriteBytesWithProcessPermissionsLeavesExistingRootUntouched(t *testing.T) { + previousUmask := syscall.Umask(0o077) + defer syscall.Umask(previousUmask) + + root := filepath.Join(t.TempDir(), "cache") + syscall.Umask(0) + if err := os.Mkdir(root, 0o751); err != nil { + t.Fatal(err) + } + syscall.Umask(0o077) + if err := WriteBytesWithProcessPermissions(root, "nested/value", []byte("value")); err != nil { + t.Fatal(err) + } + assertPermissions(t, root, 0o751) +} + +func TestWriteBytesWithProcessPermissionsReplacementUsesCurrentPolicy(t *testing.T) { + root := filepath.Join(t.TempDir(), "cache") + previousUmask := syscall.Umask(0o077) + defer syscall.Umask(previousUmask) + if err := WriteBytesWithProcessPermissions(root, "value", []byte("before")); err != nil { + t.Fatal(err) + } + before, err := os.Stat(filepath.Join(root, "value")) + if err != nil { + t.Fatal(err) + } + + syscall.Umask(0o007) + if err := WriteBytesWithProcessPermissions(root, "value", []byte("after")); err != nil { + t.Fatal(err) + } + after, err := os.Stat(filepath.Join(root, "value")) + if err != nil { + t.Fatal(err) + } + if before.Sys().(*syscall.Stat_t).Ino == after.Sys().(*syscall.Stat_t).Ino { + t.Fatal("replacement retained the old inode") + } + assertPermissions(t, filepath.Join(root, "value"), 0o660) + data, err := os.ReadFile(filepath.Join(root, "value")) + if err != nil { + t.Fatal(err) + } + if string(data) != "after" { + t.Fatalf("replacement content = %q, want %q", data, "after") + } +} + +func TestWriteJSONWithProcessPermissionsFormatsJSON(t *testing.T) { + previousUmask := syscall.Umask(0o077) + defer syscall.Umask(previousUmask) + + root := filepath.Join(t.TempDir(), "cache") + if err := WriteJSONWithProcessPermissions(root, "value.json", map[string]string{"value": "ok"}); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(filepath.Join(root, "value.json")) + if err != nil { + t.Fatal(err) + } + if string(data) != "{\n \"value\": \"ok\"\n}\n" { + t.Fatalf("JSON = %q", data) + } +} + +func assertPermissions(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) + } +} + +func assertNoTemporarySiblings(t *testing.T, directory string) { + t.Helper() + entries, err := os.ReadDir(directory) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.Contains(entry.Name(), ".tmp-") { + t.Fatalf("temporary file remains: %s", entry.Name()) + } + } +}