From 93653cccb8ac0722762b55e2c5eace18333e3c2b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 8 Jul 2026 02:23:20 +0000 Subject: [PATCH] Add workspace filesystem helpers --- docs/internal/overview.md | 3 + internal/core/workspace/files.go | 114 +++++++++++++++++ internal/core/workspace/files_test.go | 148 +++++++++++++++++++++++ internal/core/workspace/settings.go | 76 ++++++++++++ internal/core/workspace/settings_test.go | 127 +++++++++++++++++++ 5 files changed, 468 insertions(+) create mode 100644 internal/core/workspace/files.go create mode 100644 internal/core/workspace/files_test.go create mode 100644 internal/core/workspace/settings.go create mode 100644 internal/core/workspace/settings_test.go diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 6acf0cd..a58104f 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -29,6 +29,9 @@ belongs in modules, not in command handlers. diagnostics artifact writers, atomic writes, and retention decisions. - `internal/core/source`: source documents, source units, source references, and validation. +- `internal/core/workspace`: effective workspace roots, enabled-state helpers, + safe workspace-relative path construction, and atomic workspace artifact + writes. Core packages should remain deterministic and concrete. They should not import production modules. diff --git a/internal/core/workspace/files.go b/internal/core/workspace/files.go new file mode 100644 index 0000000..ac4cb53 --- /dev/null +++ b/internal/core/workspace/files.go @@ -0,0 +1,114 @@ +package workspace + +import ( + "encoding/json" + "fmt" + "os" + "path" + "path/filepath" + "strings" +) + +func SafePath(root string, name string) (string, error) { + root = strings.TrimSpace(root) + if root == "" { + return "", fmt.Errorf("workspace root must not be empty") + } + name = strings.TrimSpace(name) + if name == "" { + return "", fmt.Errorf("workspace artifact name must not be empty") + } + if strings.Contains(name, `\`) { + return "", fmt.Errorf("workspace artifact name %q must use slash-separated relative paths", name) + } + if path.IsAbs(name) || filepath.IsAbs(name) { + return "", fmt.Errorf("workspace artifact name %q must be relative", name) + } + if name == "." || strings.Contains(name, "..") { + return "", fmt.Errorf("workspace artifact name %q must not contain ..", name) + } + cleaned := path.Clean(name) + if cleaned != name { + return "", fmt.Errorf("workspace artifact name %q must be clean", name) + } + + absRoot, err := filepath.Abs(root) + if err != nil { + return "", fmt.Errorf("resolve workspace root %q: %w", root, err) + } + target, err := filepath.Abs(filepath.Join(absRoot, filepath.FromSlash(cleaned))) + if err != nil { + return "", fmt.Errorf("resolve workspace artifact %q: %w", name, err) + } + rel, err := filepath.Rel(absRoot, target) + if err != nil { + return "", fmt.Errorf("resolve workspace artifact %q: %w", name, err) + } + if rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("workspace artifact name %q resolves outside workspace root", name) + } + return target, nil +} + +func WriteJSON(root string, name string, payload any) error { + target, err := SafePath(root, name) + if err != nil { + return err + } + data, err := json.MarshalIndent(payload, "", " ") + if err != nil { + return fmt.Errorf("marshal workspace artifact %q: %w", name, err) + } + data = append(data, '\n') + if err := writeFileAtomic(target, data, 0o644); err != nil { + return fmt.Errorf("write workspace artifact %q: %w", name, err) + } + return nil +} + +func WriteBytes(root string, name string, data []byte) error { + target, err := SafePath(root, name) + if err != nil { + return err + } + if err := writeFileAtomic(target, data, 0o644); err != nil { + return fmt.Errorf("write workspace artifact %q: %w", name, err) + } + return nil +} + +func writeFileAtomic(target string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(target) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + + temp, err := os.CreateTemp(dir, "."+filepath.Base(target)+".tmp-*") + if err != nil { + return err + } + tempPath := temp.Name() + removeTemp := true + defer func() { + if removeTemp { + _ = os.Remove(tempPath) + } + }() + + if _, err := temp.Write(data); err != nil { + _ = temp.Close() + return err + } + if err := temp.Chmod(perm); err != nil { + _ = temp.Close() + return err + } + if err := temp.Close(); err != nil { + return err + } + if err := os.Rename(tempPath, target); err != nil { + return err + } + removeTemp = false + return nil +} diff --git a/internal/core/workspace/files_test.go b/internal/core/workspace/files_test.go new file mode 100644 index 0000000..d09c91e --- /dev/null +++ b/internal/core/workspace/files_test.go @@ -0,0 +1,148 @@ +package workspace + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSafePathAcceptsCleanRelativePaths(t *testing.T) { + root := t.TempDir() + + got, err := SafePath(root, "source/manifest.json") + if err != nil { + t.Fatalf("SafePath: %v", err) + } + + want := filepath.Join(root, "source", "manifest.json") + if got != want { + t.Fatalf("SafePath = %q, want %q", got, want) + } +} + +func TestSafePathRejectsUnsafeNames(t *testing.T) { + root := t.TempDir() + tests := []struct { + name string + path string + want string + }{ + {name: "empty", path: " ", want: "empty"}, + {name: "absolute", path: filepath.Join(root, "artifact.json"), want: "relative"}, + {name: "parent segment", path: "../artifact.json", want: ".."}, + {name: "embedded parent", path: "source/../artifact.json", want: ".."}, + {name: "backslash", path: `source\artifact.json`, want: "slash-separated"}, + {name: "unclean", path: "source//artifact.json", want: "clean"}, + {name: "dot", path: ".", want: ".."}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := SafePath(root, tc.path) + if err == nil { + t.Fatalf("SafePath returned %q, want error", got) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("SafePath error = %v, want containing %q", err, tc.want) + } + }) + } +} + +func TestSafePathRejectsEmptyRoot(t *testing.T) { + got, err := SafePath(" ", "artifact.json") + if err == nil { + t.Fatalf("SafePath returned %q, want error", got) + } + if !strings.Contains(err.Error(), "root") { + t.Fatalf("SafePath error = %v, want root error", err) + } +} + +func TestSafePathDoesNotPermitEscapingRoot(t *testing.T) { + root := t.TempDir() + for _, name := range []string{ + "..", + "../outside.json", + "nested/../../outside.json", + } { + t.Run(name, func(t *testing.T) { + got, err := SafePath(root, name) + if err == nil { + t.Fatalf("SafePath returned %q, want error", got) + } + }) + } +} + +func TestWriteJSONWritesIndentedAtomicArtifact(t *testing.T) { + root := t.TempDir() + + err := WriteJSON(root, "source/manifest.json", map[string]any{ + "status": "succeeded", + "count": 2, + }) + if err != nil { + t.Fatalf("WriteJSON: %v", err) + } + + got := string(readFile(t, filepath.Join(root, "source", "manifest.json"))) + if !strings.HasSuffix(got, "\n") { + t.Fatalf("expected trailing newline, got %q", got) + } + if !strings.Contains(got, `"status": "succeeded"`) || !strings.Contains(got, `"count": 2`) { + t.Fatalf("unexpected JSON: %s", got) + } + assertNoTempFiles(t, filepath.Join(root, "source")) +} + +func TestWriteBytesWritesNestedArtifact(t *testing.T) { + root := t.TempDir() + + if err := WriteBytes(root, "chunk/chunks.json", []byte("payload")); err != nil { + t.Fatalf("WriteBytes: %v", err) + } + + got := string(readFile(t, filepath.Join(root, "chunk", "chunks.json"))) + if got != "payload" { + t.Fatalf("bytes = %q, want payload", got) + } + assertNoTempFiles(t, filepath.Join(root, "chunk")) +} + +func TestWritersRejectUnsafePaths(t *testing.T) { + root := t.TempDir() + + if err := WriteBytes(root, "../outside.json", []byte("payload")); err == nil { + t.Fatalf("WriteBytes accepted unsafe path") + } + if err := WriteJSON(root, `debug\trace.json`, map[string]string{"x": "y"}); err == nil { + t.Fatalf("WriteJSON accepted unsafe path") + } + if _, err := os.Stat(filepath.Join(root, "..", "outside.json")); !os.IsNotExist(err) { + t.Fatalf("outside path stat err = %v, want not exist", err) + } +} + +func readFile(t *testing.T, path string) []byte { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %q: %v", path, err) + } + return data +} + +func assertNoTempFiles(t *testing.T, dir string) { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read dir %q: %v", dir, err) + } + for _, entry := range entries { + if strings.Contains(entry.Name(), ".tmp-") { + t.Fatalf("temporary file was not cleaned up: %s", entry.Name()) + } + } +} diff --git a/internal/core/workspace/settings.go b/internal/core/workspace/settings.go new file mode 100644 index 0000000..421170e --- /dev/null +++ b/internal/core/workspace/settings.go @@ -0,0 +1,76 @@ +package workspace + +import ( + "fmt" + "path/filepath" + "strings" + + "gitea.maximumdirect.net/eric/notarius/internal/core/config" +) + +type Settings struct { + RootDir string + DiagnosticsRoot string + CheckpointsRoot string + DebugRoot string + DiagnosticsEnabled bool + ResumeEnabled bool + DebugEnabled bool +} + +func FromConfig(cfg config.Config) Settings { + root := cleanPath(cfg.Workspace.Directory) + settings := Settings{ + RootDir: root, + DiagnosticsEnabled: cfg.DiagnosticsEnabled(), + } + if settings.DiagnosticsEnabled { + settings.DiagnosticsRoot = cleanPath(cfg.Diagnostics.WorkDir) + } + if root == "" { + return settings + } + + settings.CheckpointsRoot = filepath.Join(root, "checkpoints") + settings.DebugRoot = filepath.Join(root, "debug") + settings.ResumeEnabled = cfg.Workspace.Resume.Enabled + settings.DebugEnabled = cfg.Workspace.Debug.Enabled + return settings +} + +func (s Settings) DiagnosticsRunDirectory(runID string) (string, error) { + if !s.DiagnosticsEnabled || strings.TrimSpace(s.DiagnosticsRoot) == "" { + return "", nil + } + return safeSingleDirectory(s.DiagnosticsRoot, runID, "diagnostics run ID") +} + +func (s Settings) CheckpointIdentityDirectory(identity string) (string, error) { + if !s.ResumeEnabled || strings.TrimSpace(s.CheckpointsRoot) == "" { + return "", nil + } + return SafePath(s.CheckpointsRoot, identity) +} + +func (s Settings) DebugRunDirectory(runID string) (string, error) { + if !s.DebugEnabled || strings.TrimSpace(s.DebugRoot) == "" { + return "", nil + } + return safeSingleDirectory(s.DebugRoot, runID, "debug run ID") +} + +func cleanPath(path string) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + return filepath.Clean(path) +} + +func safeSingleDirectory(root string, name string, label string) (string, error) { + name = strings.TrimSpace(name) + if strings.Contains(name, "/") || strings.Contains(name, `\`) { + return "", fmt.Errorf("%s %q must be a single directory name", label, name) + } + return SafePath(root, name) +} diff --git a/internal/core/workspace/settings_test.go b/internal/core/workspace/settings_test.go new file mode 100644 index 0000000..3d7c727 --- /dev/null +++ b/internal/core/workspace/settings_test.go @@ -0,0 +1,127 @@ +package workspace + +import ( + "path/filepath" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/core/config" +) + +func TestFromConfigBuildsWorkspaceRoots(t *testing.T) { + cfg := config.Default() + cfg.Workspace.Directory = "/var/lib/notarius" + cfg.Workspace.Resume.Enabled = true + cfg.Workspace.Debug.Enabled = true + cfg.RecomputeEffectiveDiagnostics() + + settings := FromConfig(cfg) + + if settings.RootDir != "/var/lib/notarius" { + t.Fatalf("RootDir = %q, want /var/lib/notarius", settings.RootDir) + } + if settings.DiagnosticsRoot != "/var/lib/notarius/diagnostics" || !settings.DiagnosticsEnabled { + t.Fatalf("diagnostics settings = %+v, want workspace diagnostics root enabled", settings) + } + if settings.CheckpointsRoot != "/var/lib/notarius/checkpoints" || !settings.ResumeEnabled { + t.Fatalf("checkpoint settings = %+v, want workspace checkpoints root enabled", settings) + } + if settings.DebugRoot != "/var/lib/notarius/debug" || !settings.DebugEnabled { + t.Fatalf("debug settings = %+v, want workspace debug root enabled", settings) + } +} + +func TestFromConfigKeepsLegacyDiagnosticsRootWithoutWorkspaceRoot(t *testing.T) { + cfg := config.Default() + cfg.Diagnostics.WorkDir = "/tmp/notarius-legacy" + cfg.Workspace.Resume.Enabled = true + cfg.Workspace.Debug.Enabled = true + + settings := FromConfig(cfg) + + if settings.RootDir != "" { + t.Fatalf("RootDir = %q, want empty", settings.RootDir) + } + if settings.DiagnosticsRoot != "/tmp/notarius-legacy" || !settings.DiagnosticsEnabled { + t.Fatalf("diagnostics settings = %+v, want legacy diagnostics root enabled", settings) + } + if settings.CheckpointsRoot != "" || settings.ResumeEnabled { + t.Fatalf("checkpoint settings = %+v, want disabled empty root", settings) + } + if settings.DebugRoot != "" || settings.DebugEnabled { + t.Fatalf("debug settings = %+v, want disabled empty root", settings) + } +} + +func TestPathConstructors(t *testing.T) { + root := t.TempDir() + settings := Settings{ + RootDir: root, + DiagnosticsRoot: filepath.Join(root, "diagnostics"), + CheckpointsRoot: filepath.Join(root, "checkpoints"), + DebugRoot: filepath.Join(root, "debug"), + DiagnosticsEnabled: true, + ResumeEnabled: true, + DebugEnabled: true, + } + + diagnosticsDir, err := settings.DiagnosticsRunDirectory("run-123") + if err != nil { + t.Fatalf("DiagnosticsRunDirectory: %v", err) + } + if diagnosticsDir != filepath.Join(root, "diagnostics", "run-123") { + t.Fatalf("diagnostics dir = %q", diagnosticsDir) + } + + checkpointDir, err := settings.CheckpointIdentityDirectory("pipeline/input-digest/pipeline-digest") + if err != nil { + t.Fatalf("CheckpointIdentityDirectory: %v", err) + } + if checkpointDir != filepath.Join(root, "checkpoints", "pipeline", "input-digest", "pipeline-digest") { + t.Fatalf("checkpoint dir = %q", checkpointDir) + } + + debugDir, err := settings.DebugRunDirectory("run-456") + if err != nil { + t.Fatalf("DebugRunDirectory: %v", err) + } + if debugDir != filepath.Join(root, "debug", "run-456") { + t.Fatalf("debug dir = %q", debugDir) + } +} + +func TestDisabledPathConstructorsReturnEmptyPaths(t *testing.T) { + settings := Settings{} + + for name, call := range map[string]func() (string, error){ + "diagnostics": func() (string, error) { return settings.DiagnosticsRunDirectory("run-1") }, + "checkpoint": func() (string, error) { return settings.CheckpointIdentityDirectory("identity") }, + "debug": func() (string, error) { return settings.DebugRunDirectory("run-1") }, + } { + t.Run(name, func(t *testing.T) { + got, err := call() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "" { + t.Fatalf("path = %q, want empty", got) + } + }) + } +} + +func TestRunDirectoryConstructorsRejectNestedNames(t *testing.T) { + root := t.TempDir() + settings := Settings{ + DiagnosticsRoot: filepath.Join(root, "diagnostics"), + DebugRoot: filepath.Join(root, "debug"), + DiagnosticsEnabled: true, + DebugEnabled: true, + } + + if got, err := settings.DiagnosticsRunDirectory("run-1/nested"); err == nil { + t.Fatalf("DiagnosticsRunDirectory returned %q, want error", got) + } + if got, err := settings.DebugRunDirectory("run-1/nested"); err == nil { + t.Fatalf("DebugRunDirectory returned %q, want error", got) + } +}