Harden debug bundle filesystem collaborators

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}
+}

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

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)
+	}
+}

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)
+		}
+	}
+}