962bc0c refactor(snapshot): tighten repository validation · vev

M internal/adapters/snapshot/atomic.go +2 -2


@@ -72,7 +72,7 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {
72	72		created = true
73	73		}
74	74		parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory")
75		-	if mkdirErr != nil && mkdirErr != syscall.EEXIST {
	75	+	if mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) {
76	76		err = mkdirErr
77	77		joinCloseError(&err, "close snapshot parent directory", parentCloseErr)
78	78		return err

@@ -326,7 +326,7 @@ func (r *Repository) remove(path string) (err error) {
326	326		defer func() {
327	327		joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))
328	328		}()
329		-	if err := syscall.Unlinkat(fd, name); err != syscall.EISDIR {
	329	+	if err := syscall.Unlinkat(fd, name); !errors.Is(err, syscall.EISDIR) {
330	330		return err
331	331		}
332	332		namePtr, err := syscall.BytePtrFromString(name)


@@ -72,7 +72,7 @@ func (r *Repository) ensurePrivateDirectoryPhase(dir, phase string) error {
72		created = true	72		created = true
73		}	73		}
74		parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory")	74		parentCloseErr := r.closeDescriptor(parent, "close snapshot parent directory")
75	-	if mkdirErr != nil && mkdirErr != syscall.EEXIST {	75	+	if mkdirErr != nil && !errors.Is(mkdirErr, syscall.EEXIST) {
76		err = mkdirErr	76		err = mkdirErr
77		joinCloseError(&err, "close snapshot parent directory", parentCloseErr)	77		joinCloseError(&err, "close snapshot parent directory", parentCloseErr)
78		return err	78		return err

@@ -326,7 +326,7 @@ func (r *Repository) remove(path string) (err error) {
326		defer func() {	326		defer func() {
327		joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))	327		joinCloseError(&err, "close snapshot parent directory", r.closeDescriptor(fd, "close snapshot parent directory"))
328		}()	328		}()
329	-	if err := syscall.Unlinkat(fd, name); err != syscall.EISDIR {	329	+	if err := syscall.Unlinkat(fd, name); !errors.Is(err, syscall.EISDIR) {
330		return err	330		return err
331		}	331		}
332		namePtr, err := syscall.BytePtrFromString(name)	332		namePtr, err := syscall.BytePtrFromString(name)

M internal/adapters/snapshot/legacy.go +10 -2


@@ -38,6 +38,14 @@ func (r *Repository) openLegacyDirectory(path string) (legacyDirectory, error) {
38	38		return r.openDirectory(path)
39	39		}
40	40
	41	+	// readLegacyImportCandidate reports whether an untrusted root entry can be
	42	+	// safely imported. Legacy scanning skips entries that disappear or fail secure
	43	+	// descriptor validation so one hostile .snap file does not block other imports.
	44	+	func (r *Repository) readLegacyImportCandidate(path string) ([]byte, bool) {
	45	+	data, err := r.readBounded(path)
	46	+	return data, err == nil
	47	+	}
	48	+
41	49		// LoadLegacy reads only pre-incremental root .snap files. It is deliberately
42	50		// isolated from repository reads so incremental directories are never traversed.
43	51		func (r *Repository) LoadLegacy(ctx context.Context) (out []ports.LegacySnapshot, err error) {

@@ -87,8 +95,8 @@ func (r *Repository) LoadLegacy(ctx context.Context) (out []ports.LegacySnapshot
87	95		return nil, fmt.Errorf("legacy snapshot import has too many files (maximum %d)", maxLegacySnapshotFiles)
88	96		}
89	97		path := filepath.Join(r.dir, entry.Name())
90		-	data, dataErr := r.readBounded(path)
91		-	if dataErr != nil {
	98	+	data, importable := r.readLegacyImportCandidate(path)
	99	+	if !importable {
92	100		continue
93	101		}
94	102		if len(data) > maxLegacySnapshotBytes-total {

 	return r.openDirectory(path)
 }
+// readLegacyImportCandidate reports whether an untrusted root entry can be
+// safely imported. Legacy scanning skips entries that disappear or fail secure
+// descriptor validation so one hostile .snap file does not block other imports.
+func (r *Repository) readLegacyImportCandidate(path string) ([]byte, bool) {
+	data, err := r.readBounded(path)
+	return data, err == nil
+}
 // LoadLegacy reads only pre-incremental root .snap files. It is deliberately
 // isolated from repository reads so incremental directories are never traversed.
 func (r *Repository) LoadLegacy(ctx context.Context) (out []ports.LegacySnapshot, err error) {
 				return nil, fmt.Errorf("legacy snapshot import has too many files (maximum %d)", maxLegacySnapshotFiles)
 			}
 			path := filepath.Join(r.dir, entry.Name())
-			data, dataErr := r.readBounded(path)
+			data, importable := r.readLegacyImportCandidate(path)
-			if dataErr != nil {
+			if !importable {
 				continue
 			}
 			if len(data) > maxLegacySnapshotBytes-total {

M internal/adapters/snapshot/maintenance_cursor.go +3 -2


@@ -156,13 +156,14 @@ func (r Repository) readMaintenanceDirent(dir string, limit int, cursor mainte
156	156		}
157	157		record := (*syscall.Dirent)(unsafe.Pointer(&data[0]))
158	158		reclen := int(record.Reclen)
159		-	if reclen <= 0 \|\| reclen > len(data) {
	159	+	nameOffset := int(unsafe.Offsetof(syscall.Dirent{}.Name))
	160	+	if reclen < nameOffset \|\| reclen > len(data) {
160	161		return nil, false, syscall.EIO
161	162		}
162	163		data = data[reclen:]
163	164		// Advance past every raw record, including dot and disappeared entries.
164	165		cursor.offset = record.Off
165		-	nameBytes := unsafe.Slice((*byte)(unsafe.Pointer(&record.Name[0])), reclen-int(unsafe.Offsetof(syscall.Dirent{}.Name)))
	166	+	nameBytes := unsafe.Slice((*byte)(unsafe.Pointer(&record.Name[0])), reclen-nameOffset)
166	167		if end := strings.IndexByte(string(nameBytes), 0); end >= 0 {
167	168		nameBytes = nameBytes[:end]
168	169		}

 			}
 			record := (*syscall.Dirent)(unsafe.Pointer(&data[0]))
 			reclen := int(record.Reclen)
-			if reclen <= 0 || reclen > len(data) {
+			nameOffset := int(unsafe.Offsetof(syscall.Dirent{}.Name))
+			if reclen < nameOffset || reclen > len(data) {
 				return nil, false, syscall.EIO
 			}
 			data = data[reclen:]
 			// Advance past every raw record, including dot and disappeared entries.
 			cursor.offset = record.Off
-			nameBytes := unsafe.Slice((*byte)(unsafe.Pointer(&record.Name[0])), reclen-int(unsafe.Offsetof(syscall.Dirent{}.Name)))
+			nameBytes := unsafe.Slice((*byte)(unsafe.Pointer(&record.Name[0])), reclen-nameOffset)
 			if end := strings.IndexByte(string(nameBytes), 0); end >= 0 {
 				nameBytes = nameBytes[:end]
 			}

M internal/adapters/snapshot/maintenance_gc.go +11 -21


@@ -7,6 +7,7 @@ import (
7	7		"fmt"
8	8		"os"
9	9		"path/filepath"
	10	+	"sort"
10	11		"strings"
11	12
12	13		"github.com/bnema/vev/internal/ports"

@@ -226,20 +227,21 @@ func maintenanceManifestRefs(manifest codec.Manifest) (map[ports.SnapshotDigest]
226	227		return refs, true
227	228		}
228	229
229		-	func retainedManifestQueue(marked map[uint64]manifestMaintenance) []uint64 {
	230	+	// completeGenerationsDescending returns only complete generations in canonical
	231	+	// newest-first order for both manifest and object retention.
	232	+	func completeGenerationsDescending(marked map[uint64]manifestMaintenance) []uint64 {
230	233		complete := make([]uint64, 0, len(marked))
231	234		for generation, item := range marked {
232	235		if item.complete {
233	236		complete = append(complete, generation)
234	237		}
235	238		}
236		-	for i := range complete {
237		-	for j := i + 1; j < len(complete); j++ {
238		-	if complete[j] > complete[i] {
239		-	complete[i], complete[j] = complete[j], complete[i]
240		-	}
241		-	}
242		-	}
	239	+	sort.Slice(complete, func(i, j int) bool { return complete[i] > complete[j] })
	240	+	return complete
	241	+	}
	242	+
	243	+	func retainedManifestQueue(marked map[uint64]manifestMaintenance) []uint64 {
	244	+	complete := completeGenerationsDescending(marked)
243	245		if len(complete) <= 2 {
244	246		return nil
245	247		}

@@ -354,19 +356,7 @@ func retainedReferences(marked map[uint64]manifestMaintenance, conservative bool
354	356		}
355	357		return references
356	358		}
357		-	complete := make([]uint64, 0, len(marked))
358		-	for generation, item := range marked {
359		-	if item.complete {
360		-	complete = append(complete, generation)
361		-	}
362		-	}
363		-	for i := range complete {
364		-	for j := i + 1; j < len(complete); j++ {
365		-	if complete[j] > complete[i] {
366		-	complete[i], complete[j] = complete[j], complete[i]
367		-	}
368		-	}
369		-	}
	359	+	complete := completeGenerationsDescending(marked)
370	360		keep := make(map[uint64]bool, 2)
371	361		for _, generation := range complete {
372	362		if len(keep) == 2 {


@@ -7,6 +7,7 @@ import (
7		"fmt"	7		"fmt"
8		"os"	8		"os"
9		"path/filepath"	9		"path/filepath"
			10	+	"sort"
10		"strings"	11		"strings"
11			12
12		"github.com/bnema/vev/internal/ports"	13		"github.com/bnema/vev/internal/ports"

@@ -226,20 +227,21 @@ func maintenanceManifestRefs(manifest codec.Manifest) (map[ports.SnapshotDigest]
226		return refs, true	227		return refs, true
227		}	228		}
228			229
229	-	func retainedManifestQueue(marked map[uint64]manifestMaintenance) []uint64 {	230	+	// completeGenerationsDescending returns only complete generations in canonical
			231	+	// newest-first order for both manifest and object retention.
			232	+	func completeGenerationsDescending(marked map[uint64]manifestMaintenance) []uint64 {
230		complete := make([]uint64, 0, len(marked))	233		complete := make([]uint64, 0, len(marked))
231		for generation, item := range marked {	234		for generation, item := range marked {
232		if item.complete {	235		if item.complete {
233		complete = append(complete, generation)	236		complete = append(complete, generation)
234		}	237		}
235		}	238		}
236	-	for i := range complete {	239	+	sort.Slice(complete, func(i, j int) bool { return complete[i] > complete[j] })
237	-	for j := i + 1; j < len(complete); j++ {	240	+	return complete
238	-	if complete[j] > complete[i] {	241	+	}
239	-	complete[i], complete[j] = complete[j], complete[i]	242	+
240	-	}	243	+	func retainedManifestQueue(marked map[uint64]manifestMaintenance) []uint64 {
241	-	}	244	+	complete := completeGenerationsDescending(marked)
242	-	}
243		if len(complete) <= 2 {	245		if len(complete) <= 2 {
244		return nil	246		return nil
245		}	247		}

@@ -354,19 +356,7 @@ func retainedReferences(marked map[uint64]manifestMaintenance, conservative bool
354		}	356		}
355		return references	357		return references
356		}	358		}
357	-	complete := make([]uint64, 0, len(marked))	359	+	complete := completeGenerationsDescending(marked)
358	-	for generation, item := range marked {
359	-	if item.complete {
360	-	complete = append(complete, generation)
361	-	}
362	-	}
363	-	for i := range complete {
364	-	for j := i + 1; j < len(complete); j++ {
365	-	if complete[j] > complete[i] {
366	-	complete[i], complete[j] = complete[j], complete[i]
367	-	}
368	-	}
369	-	}
370		keep := make(map[uint64]bool, 2)	360		keep := make(map[uint64]bool, 2)
371		for _, generation := range complete {	361		for _, generation := range complete {
372		if len(keep) == 2 {	362		if len(keep) == 2 {

M internal/adapters/snapshot/maintenance_test.go +27 -2


@@ -3,12 +3,15 @@ package snapshot
3	3		import (
4	4		"context"
5	5		"crypto/sha256"
	6	+	"encoding/binary"
6	7		"errors"
7	8		"fmt"
8	9		"os"
9	10		"path/filepath"
10	11		"strings"
	12	+	"syscall"
11	13		"testing"
	14	+	"unsafe"
12	15
13	16		"github.com/bnema/vev/internal/ports"
14	17		)

@@ -77,15 +80,37 @@ func TestReadMaintenanceDirentPreservesCursorAndCloseFailuresWithoutPaths(t *tes
77	80		}
78	81		}
79	82
	83	+	func TestReadMaintenanceDirentRejectsRecordShorterThanNameOffset(t *testing.T) {
	84	+	repo := NewRepository(privateDir(t))
	85	+	nameOffset := int(unsafe.Offsetof(syscall.Dirent{}.Name))
	86	+	recordData := make([]byte, nameOffset)
	87	+	reclenOffset := unsafe.Offsetof(syscall.Dirent{}.Reclen)
	88	+	binary.NativeEndian.PutUint16(recordData[reclenOffset:], 1)
	89	+
	90	+	repo.hooks.openMaintenanceDirectory = func(string) (maintenanceDirectory, error) {
	91	+	return fakeMaintenanceDirectory{data: recordData}, nil
	92	+	}
	93	+	_, _, err := repo.readMaintenanceDirent("unused", 1, &maintenanceCursor{})
	94	+	if !errors.Is(err, syscall.EIO) {
	95	+	t.Fatalf("readMaintenanceDirent error = %v, want EIO", err)
	96	+	}
	97	+	}
	98	+
80	99		type fakeMaintenanceDirectory struct {
	100	+	data []byte
81	101		seekErr error
82	102		readErr error
83	103		closeErr error
84	104		}
85	105
86	106		func (d fakeMaintenanceDirectory) Seek(int64, int) (int64, error) { return 0, d.seekErr }
87		-	func (d fakeMaintenanceDirectory) ReadDirent([]byte) (int, error) { return 0, d.readErr }
88		-	func (d fakeMaintenanceDirectory) Close() error { return d.closeErr }
	107	+	func (d fakeMaintenanceDirectory) ReadDirent(buffer []byte) (int, error) {
	108	+	if d.readErr != nil {
	109	+	return 0, d.readErr
	110	+	}
	111	+	return copy(buffer, d.data), nil
	112	+	}
	113	+	func (d fakeMaintenanceDirectory) Close() error { return d.closeErr }
89	114
90	115		func TestRepositoryDeleteQuarantinesCanonicalSessionBeforeCleanup(t *testing.T) {
91	116		repo := NewRepository(privateDir(t))

 import (
 	"context"
 	"crypto/sha256"
+	"encoding/binary"
 	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
+	"unsafe"
 	"github.com/bnema/vev/internal/ports"
 )
 	}
 }
+func TestReadMaintenanceDirentRejectsRecordShorterThanNameOffset(t *testing.T) {
+	repo := NewRepository(privateDir(t))
+	nameOffset := int(unsafe.Offsetof(syscall.Dirent{}.Name))
+	recordData := make([]byte, nameOffset)
+	reclenOffset := unsafe.Offsetof(syscall.Dirent{}.Reclen)
+	binary.NativeEndian.PutUint16(recordData[reclenOffset:], 1)
+	repo.hooks.openMaintenanceDirectory = func(string) (maintenanceDirectory, error) {
+		return fakeMaintenanceDirectory{data: recordData}, nil
+	}
+	_, _, err := repo.readMaintenanceDirent("unused", 1, &maintenanceCursor{})
+	if !errors.Is(err, syscall.EIO) {
+		t.Fatalf("readMaintenanceDirent error = %v, want EIO", err)
+	}
+}
 type fakeMaintenanceDirectory struct {
+	data     []byte
 	seekErr  error
 	readErr  error
 	closeErr error
 }
 func (d fakeMaintenanceDirectory) Seek(int64, int) (int64, error) { return 0, d.seekErr }
-func (d fakeMaintenanceDirectory) ReadDirent([]byte) (int, error) { return 0, d.readErr }
+func (d fakeMaintenanceDirectory) ReadDirent(buffer []byte) (int, error) {
-func (d fakeMaintenanceDirectory) Close() error                   { return d.closeErr }
+	if d.readErr != nil {
+		return 0, d.readErr
+	}
+	return copy(buffer, d.data), nil
+}
+func (d fakeMaintenanceDirectory) Close() error { return d.closeErr }
 func TestRepositoryDeleteQuarantinesCanonicalSessionBeforeCleanup(t *testing.T) {
 	repo := NewRepository(privateDir(t))

M internal/adapters/snapshot/path_security_test.go +3 -1


@@ -35,7 +35,9 @@ func TestRepositoryRejectsSymlinkedGenerationAndObjectShards(t *testing.T) {
35	35		t.Fatal("Load succeeded through symlinked repository component")
36	36		}
37	37		// Maintenance must reject the replacement rather than following it.
38		-	_ = repo.Maintain(context.Background())
	38	+	if err := repo.Maintain(context.Background()); err == nil {
	39	+	t.Fatal("Maintain succeeded through symlinked repository component")
	40	+	}
39	41		got, err := os.ReadFile(guard)
40	42		if err != nil \|\| string(got) != "guard" {
41	43		t.Fatalf("outside guard = %q, %v", got, err)

 				t.Fatal("Load succeeded through symlinked repository component")
 			}
 			// Maintenance must reject the replacement rather than following it.
-			_ = repo.Maintain(context.Background())
+			if err := repo.Maintain(context.Background()); err == nil {
+				t.Fatal("Maintain succeeded through symlinked repository component")
+			}
 			got, err := os.ReadFile(guard)
 			if err != nil || string(got) != "guard" {
 				t.Fatalf("outside guard = %q, %v", got, err)

M internal/adapters/snapshot/repository_phase6_test.go +2 -2


@@ -2,6 +2,7 @@ package snapshot
2	2
3	3		import (
4	4		"context"
	5	+	"errors"
5	6		"fmt"
6	7		"os"
7	8		"path/filepath"

@@ -27,7 +28,6 @@ func TestRepositoryPublishDeleteManySessionsRaceFree(t *testing.T) {
27	28
28	29		var wg sync.WaitGroup
29	30		for _, publication := range publications {
30		-	publication := publication
31	31		wg.Add(2)
32	32		go func() {
33	33		defer wg.Done()

@@ -73,7 +73,7 @@ func TestRepositoryBoundsAndEvictsPartialMaintenanceSessionState(t *testing.T) {
73	73
74	74		ctx, cancel := context.WithCancel(context.Background())
75	75		cancel()
76		-	if err := repo.Maintain(ctx); err != context.Canceled {
	76	+	if err := repo.Maintain(ctx); !errors.Is(err, context.Canceled) {
77	77		t.Fatalf("canceled Maintain error = %v, want context canceled", err)
78	78		}
79	79		locks, epochs = repo.sessionStateCounts()

 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	var wg sync.WaitGroup
 	for _, publication := range publications {
-		publication := publication
 		wg.Add(2)
 		go func() {
 			defer wg.Done()
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	if err := repo.Maintain(ctx); err != context.Canceled {
+	if err := repo.Maintain(ctx); !errors.Is(err, context.Canceled) {
 		t.Fatalf("canceled Maintain error = %v, want context canceled", err)
 	}
 	locks, epochs = repo.sessionStateCounts()

M internal/adapters/snapshot/safe_path.go +2 -1


@@ -3,6 +3,7 @@ package snapshot
3	3		import (
4	4		"crypto/rand"
5	5		"encoding/hex"
	6	+	"errors"
6	7		"fmt"
7	8		"os"
8	9		"path/filepath"

@@ -166,7 +167,7 @@ func (r Repository) createTempAt(dir string) (temp os.File, err error) {
166	167		}
167	168		name := ".tmp-" + hex.EncodeToString(random[:])
168	169		tempFD, err := syscall.Openat(int(fd.Fd()), name, syscall.O_WRONLY\|syscall.O_CREAT\|syscall.O_EXCL\|syscall.O_NOFOLLOW\|syscall.O_CLOEXEC, 0o600)
169		-	if err == syscall.EEXIST {
	170	+	if errors.Is(err, syscall.EEXIST) {
170	171		continue
171	172		}
172	173		if err != nil {

 import (
 	"crypto/rand"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 		}
 		name := ".tmp-" + hex.EncodeToString(random[:])
 		tempFD, err := syscall.Openat(int(fd.Fd()), name, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0o600)
-		if err == syscall.EEXIST {
+		if errors.Is(err, syscall.EEXIST) {
 			continue
 		}
 		if err != nil {