From 3e66127b94fdebd4a6d52f77f4361c83c3292c93 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 9 Aug 2026 01:41:37 +0000 Subject: [PATCH] Write durable outputs through confined file writer --- internal/cli/run.go | 51 ++++--------------------------------- internal/cli/run_id_test.go | 23 +++++++++++++++-- 2 files changed, 26 insertions(+), 48 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index 00ecd7e..609b302 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -19,6 +19,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/artifacts" "gitea.maximumdirect.net/eric/notarius/internal/core/config" "gitea.maximumdirect.net/eric/notarius/internal/core/debugbundle" + "gitea.maximumdirect.net/eric/notarius/internal/core/fileio" "gitea.maximumdirect.net/eric/notarius/internal/framework/checkpoint" "gitea.maximumdirect.net/eric/notarius/internal/framework/chunkplan" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -757,17 +758,10 @@ func configSource(configPath string) string { } func writeOutputFiles(runOutputDir string, files []contracts.OutputFile) error { - type outputTarget struct { - path string - file contracts.OutputFile - } - targets := make([]outputTarget, 0, len(files)) for _, file := range files { - targetPath, err := outputFilePath(runOutputDir, file.Name) - if err != nil { + if _, err := outputFilePath(runOutputDir, file.Name); err != nil { return err } - targets = append(targets, outputTarget{path: targetPath, file: file}) } outputParent := filepath.Dir(runOutputDir) @@ -780,12 +774,9 @@ func writeOutputFiles(runOutputDir string, files []contracts.OutputFile) error { } return fmt.Errorf("create output run directory %q: %w", runOutputDir, err) } - for _, target := range targets { - if err := os.MkdirAll(filepath.Dir(target.path), 0o755); err != nil { - return fmt.Errorf("create output directory %q: %w", filepath.Dir(target.path), err) - } - if err := writeFileAtomic(target.path, target.file.Bytes, 0o644); err != nil { - return fmt.Errorf("write output file %q: %w", target.file.Name, err) + for _, file := range files { + if err := fileio.WriteBytes(runOutputDir, file.Name, file.Bytes, 0o755, 0o644); err != nil { + return fmt.Errorf("write output file %q: %w", file.Name, err) } } return nil @@ -828,38 +819,6 @@ func outputFilePath(runOutputDir, logicalName string) (string, error) { return target, nil } -func writeFileAtomic(path string, data []byte, perm os.FileMode) error { - dir := filepath.Dir(path) - temp, err := os.CreateTemp(dir, "."+filepath.Base(path)+".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, path); err != nil { - return err - } - removeTemp = false - return nil -} - func reorderRunArgs(args []string) []string { var flags []string var positionals []string diff --git a/internal/cli/run_id_test.go b/internal/cli/run_id_test.go index a4c08bb..ae0f9d9 100644 --- a/internal/cli/run_id_test.go +++ b/internal/cli/run_id_test.go @@ -38,10 +38,29 @@ func TestWriteOutputFilesSupportsNestedLogicalPaths(t *testing.T) { if err := writeOutputFiles(runPath, []contracts.OutputFile{{Name: "nested/result.json", Bytes: []byte("result")}}); err != nil { t.Fatal(err) } - data, err := os.ReadFile(filepath.Join(runPath, "nested", "result.json")) + resultPath := filepath.Join(runPath, "nested", "result.json") + data, err := os.ReadFile(resultPath) if err != nil || string(data) != "result" { t.Fatalf("nested output = %q, %v", data, err) } + for path, want := range map[string]os.FileMode{runPath: 0o755, filepath.Join(runPath, "nested"): 0o755, resultPath: 0o644} { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != want { + t.Fatalf("%s mode = %#o, want %#o", path, info.Mode().Perm(), want) + } + } + entries, err := os.ReadDir(filepath.Join(runPath, "nested")) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.Contains(entry.Name(), ".tmp-") { + t.Fatalf("temporary file remains: %s", entry.Name()) + } + } } func TestWriteOutputFilesRejectsUnsafeNamesBeforeAllocatingRunDirectory(t *testing.T) { @@ -75,7 +94,7 @@ func TestWriteOutputFilesRetainsNewPartialDirectoryAndPreservesSibling(t *testin {Name: "blocked", Bytes: []byte("partial output")}, {Name: "blocked/nested.json", Bytes: []byte("unreachable")}, }) - if err == nil || !strings.Contains(err.Error(), "create output directory") { + if err == nil || !strings.Contains(err.Error(), `write output file "blocked/nested.json"`) { t.Fatalf("writeOutputFiles() error = %v, want later directory failure", err) } if got, err := os.ReadFile(filepath.Join(runPath, "blocked")); err != nil || string(got) != "partial output" {