diff --git a/internal/core/debugbundle/bundle_test.go b/internal/core/debugbundle/bundle_test.go index da52a86..6ca7646 100644 --- a/internal/core/debugbundle/bundle_test.go +++ b/internal/core/debugbundle/bundle_test.go @@ -5,6 +5,9 @@ import ( "path/filepath" "testing" "time" + + "gitea.maximumdirect.net/eric/notarius/internal/core/artifacts" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) func TestAllocateCreatesRestrictiveSummaryAndTrace(t *testing.T) { @@ -61,6 +64,80 @@ func TestAllocateRetriesAndDoesNotDeleteBundle(t *testing.T) { t.Fatal(err) } } + +func TestAllocateReturnsErrorAfterRunIDCollisionsAreExhausted(t *testing.T) { + parent := t.TempDir() + fixed := time.Unix(0, 99).UTC() + if err := os.Mkdir(filepath.Join(parent, "run-99"), 0o700); err != nil { + t.Fatal(err) + } + previous := utcNow + utcNow = func() time.Time { return fixed } + defer func() { utcNow = previous }() + + if _, err := Allocate(parent); err == nil { + t.Fatal("Allocate succeeded after exhausting run ID collisions") + } +} + +func TestSummaryWriterWritesEverySummaryArtifact(t *testing.T) { + bundle, err := Allocate(t.TempDir()) + if err != nil { + t.Fatal(err) + } + summary := bundle.Summary() + if err := summary.WriteInvocation(Invocation{Operation: "run"}); err != nil { + t.Fatal(err) + } + if err := summary.WriteRedactedEffectiveConfig(testRedactedSummaryPayload{}); err != nil { + t.Fatal(err) + } + if err := summary.WriteResolvedPipeline(map[string]any{}); err != nil { + t.Fatal(err) + } + if err := summary.WriteResolvedReferences(nil); err != nil { + t.Fatal(err) + } + if err := summary.WriteCheckpointEvents(nil); err != nil { + t.Fatal(err) + } + if err := summary.WriteRunManifest(artifacts.RunManifest{RunID: bundle.RunID()}); err != nil { + t.Fatal(err) + } + if err := summary.WriteChunkPlan(artifacts.ChunkPlanSummary{}); err != nil { + t.Fatal(err) + } + if err := summary.WriteRunReport(RunReport{RunID: bundle.RunID(), PipelineID: "test"}); err != nil { + t.Fatal(err) + } + if err := summary.WriteWarnings([]contracts.Warning{{ReasonCode: "test"}}); err != nil { + t.Fatal(err) + } + if err := summary.WriteError("failed"); err != nil { + t.Fatal(err) + } + + for _, name := range []string{ + ArtifactInvocationMetadata, + ArtifactEffectiveConfig, + ArtifactResolvedPipeline, + ArtifactResolvedReferences, + ArtifactCheckpointEvents, + ArtifactRunManifest, + ArtifactChunkPlan, + ArtifactRunReport, + ArtifactWarnings, + ArtifactErrorLog, + } { + info, err := os.Stat(filepath.Join(bundle.SummaryRoot(), name)) + if err != nil { + t.Fatalf("summary artifact %q: %v", name, err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("summary artifact %q mode=%#o", name, info.Mode().Perm()) + } + } +} func TestSummaryWriterConfinesArtifacts(t *testing.T) { bundle, err := Allocate(t.TempDir()) if err != nil { @@ -73,3 +150,9 @@ func TestSummaryWriterConfinesArtifacts(t *testing.T) { t.Fatal("accepted backslash") } } + +type testRedactedSummaryPayload struct{} + +func (testRedactedSummaryPayload) RedactedSummaryPayload() any { + return map[string]any{"redacted": true} +} diff --git a/internal/core/fileio/fileio.go b/internal/core/fileio/fileio.go index 47065f1..58c1223 100644 --- a/internal/core/fileio/fileio.go +++ b/internal/core/fileio/fileio.go @@ -46,6 +46,9 @@ func SafePath(root, name string) (string, error) { if rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return "", fmt.Errorf("artifact name %q resolves outside file root", name) } + if err := rejectSymlinkComponents(absRoot, name); err != nil { + return "", err + } return target, nil } @@ -62,16 +65,44 @@ func WriteBytes(root, name string, data []byte, dirMode, fileMode os.FileMode) e if err != nil { return err } - if err := writeAtomic(target, data, dirMode, fileMode); err != nil { + if err := os.MkdirAll(filepath.Dir(target), dirMode); err != nil { + return fmt.Errorf("write artifact %q: %w", name, err) + } + if err := rejectSymlinkComponents(root, name); err != nil { + return err + } + if err := writeAtomic(target, data, fileMode); err != nil { return fmt.Errorf("write artifact %q: %w", name, err) } return nil } -func writeAtomic(target string, data []byte, dirMode, fileMode os.FileMode) error { - if err := os.MkdirAll(filepath.Dir(target), dirMode); err != nil { - return err +func rejectSymlinkComponents(root, name string) error { + absRoot, err := filepath.Abs(root) + if err != nil { + return fmt.Errorf("resolve file root %q: %w", root, err) } + current := absRoot + for _, component := range strings.Split(filepath.FromSlash(name), string(filepath.Separator)) { + if component == "" || component == "." { + continue + } + current = filepath.Join(current, component) + info, err := os.Lstat(current) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("inspect artifact path %q: %w", name, err) + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("artifact path %q must not traverse symbolic links", name) + } + } + return nil +} + +func writeAtomic(target string, data []byte, fileMode os.FileMode) error { temp, err := os.CreateTemp(filepath.Dir(target), "."+filepath.Base(target)+".tmp-*") if err != nil { return err diff --git a/internal/core/fileio/fileio_test.go b/internal/core/fileio/fileio_test.go index 51d8bfa..f9683df 100644 --- a/internal/core/fileio/fileio_test.go +++ b/internal/core/fileio/fileio_test.go @@ -39,3 +39,18 @@ func TestWriteBytesIsAtomicAndUsesRequestedModes(t *testing.T) { } } } + +func TestWriteBytesRejectsSymlinkedComponents(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + if err := os.Symlink(outside, filepath.Join(root, "link")); err != nil { + t.Skipf("symbolic links unavailable: %v", err) + } + + if err := WriteBytes(root, "link/value", []byte("value"), 0o700, 0o600); err == nil { + t.Fatal("WriteBytes accepted a symlinked directory") + } + if _, err := os.Stat(filepath.Join(outside, "value")); !os.IsNotExist(err) { + t.Fatalf("write escaped through symlink: %v", err) + } +} diff --git a/internal/framework/debug/recorder_test.go b/internal/framework/debug/recorder_test.go index 4cc24ce..167df4e 100644 --- a/internal/framework/debug/recorder_test.go +++ b/internal/framework/debug/recorder_test.go @@ -1,9 +1,13 @@ package debug import ( + "fmt" "os" "path/filepath" + "sync" "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" ) func TestFilesystemRecorderWritesWithinTraceRoot(t *testing.T) { @@ -26,3 +30,50 @@ func TestFilesystemRecorderWritesWithinTraceRoot(t *testing.T) { t.Fatal("accepted traversal") } } + +func TestFilesystemRecorderPropagatesWriteFailures(t *testing.T) { + root := filepath.Join(t.TempDir(), "trace") + if err := os.WriteFile(root, []byte("not a directory"), 0o600); err != nil { + t.Fatal(err) + } + recorder, err := NewFilesystemRecorder(root) + if err != nil { + t.Fatal(err) + } + if err := recorder.WriteBytes("attempt/data", []byte("payload")); err == nil { + t.Fatal("WriteBytes concealed a trace-root write failure") + } +} + +func TestSynchronizedFilesystemRecorderSupportsConcurrentWrites(t *testing.T) { + root := t.TempDir() + recorder, err := NewFilesystemRecorder(root) + if err != nil { + t.Fatal(err) + } + recorder = pipeline.SynchronizedDebugRecorder(recorder) + + const writes = 16 + var wg sync.WaitGroup + errs := make(chan error, writes) + for i := 0; i < writes; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + errs <- recorder.WriteBytes(fmt.Sprintf("attempt/%02d/data", i), []byte("payload")) + }() + } + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Fatal(err) + } + } + for i := 0; i < writes; i++ { + if _, err := os.Stat(filepath.Join(root, "attempt", fmt.Sprintf("%02d", i), "data")); err != nil { + t.Fatalf("trace artifact %d: %v", i, err) + } + } +}