Merge branch 'next' into fix/issue-26

After reading file content, verify `totalRead == size` and return an error on mismatch.

Co-authored-by: clawbot <clawbot@openclaw>
Reviewed-on: #30
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>

internal/checker/checker.go

@@ -174,7 +174,12 @@ func (c *Checker) checkFile(entry *mfer.MFFilePath, checkedBytes *int64) Result
 	// Check if file exists
 	info, err := c.fs.Stat(absPath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) || errors.Is(err, afero.ErrFileNotFound) {
+		if errors.Is(err, afero.ErrFileNotFound) || errors.Is(err, errors.New("file does not exist")) {
+			return Result{Path: entry.Path, Status: StatusMissing, Message: "file not found"}
+		}
+		// Check for "file does not exist" style errors
+		exists, _ := afero.Exists(c.fs, absPath)
+		if !exists {
 			return Result{Path: entry.Path, Status: StatusMissing, Message: "file not found"}
 		}
 		return Result{Path: entry.Path, Status: StatusError, Message: err.Error()}

mfer/builder.go

@@ -3,13 +3,47 @@ package mfer
 import (
 	"crypto/sha256"
 	"errors"
+	"fmt"
 	"io"
+	"strings"
 	"sync"
 	"time"
+	"unicode/utf8"
 
 	"github.com/multiformats/go-multihash"
 )
 
+// ValidatePath checks that a file path conforms to manifest path invariants:
+//   - Must be valid UTF-8
+//   - Must use forward slashes only (no backslashes)
+//   - Must be relative (no leading /)
+//   - Must not contain ".." segments
+//   - Must not contain empty segments (no "//")
+//   - Must not be empty
+func ValidatePath(p string) error {
+	if p == "" {
+		return errors.New("path cannot be empty")
+	}
+	if !utf8.ValidString(p) {
+		return fmt.Errorf("path %q is not valid UTF-8", p)
+	}
+	if strings.ContainsRune(p, '\\') {
+		return fmt.Errorf("path %q contains backslash; use forward slashes only", p)
+	}
+	if strings.HasPrefix(p, "/") {
+		return fmt.Errorf("path %q is absolute; must be relative", p)
+	}
+	for _, seg := range strings.Split(p, "/") {
+		if seg == "" {
+			return fmt.Errorf("path %q contains empty segment", p)
+		}
+		if seg == ".." {
+			return fmt.Errorf("path %q contains '..' segment", p)
+		}
+	}
+	return nil
+}
+
 // RelFilePath represents a relative file path within a manifest.
 type RelFilePath string
 
@@ -74,6 +108,10 @@ func (b *Builder) AddFile(
 	reader io.Reader,
 	progress chan<- FileHashProgress,
 ) (FileSize, error) {
+	if err := ValidatePath(string(path)); err != nil {
+		return 0, err
+	}
+
 	// Create hash writer
 	h := sha256.New()
 
@@ -96,6 +134,11 @@ func (b *Builder) AddFile(
 		}
 	}
 
+	// Verify actual bytes read matches declared size
+	if totalRead != size {
+		return totalRead, fmt.Errorf("size mismatch for %q: declared %d bytes but read %d bytes", path, size, totalRead)
+	}
+
 	// Encode hash as multihash (SHA2-256)
 	mh, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256)
 	if err != nil {
@@ -141,8 +184,8 @@ func (b *Builder) FileCount() int {
 // This is useful when the hash is already known (e.g., from an existing manifest).
 // Returns an error if path is empty, size is negative, or hash is nil/empty.
 func (b *Builder) AddFileWithHash(path RelFilePath, size FileSize, mtime ModTime, hash Multihash) error {
-	if path == "" {
-		return errors.New("path cannot be empty")
+	if err := ValidatePath(string(path)); err != nil {
+		return err
 	}
 	if size < 0 {
 		return errors.New("size cannot be negative")

mfer/checker.go

@@ -224,7 +224,12 @@ func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result {
 	// Check if file exists
 	info, err := c.fs.Stat(absPath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) || errors.Is(err, afero.ErrFileNotFound) {
+		if errors.Is(err, afero.ErrFileNotFound) || errors.Is(err, errors.New("file does not exist")) {
+			return Result{Path: relPath, Status: StatusMissing, Message: "file not found"}
+		}
+		// Check for "file does not exist" style errors
+		exists, _ := afero.Exists(c.fs, absPath)
+		if !exists {
 			return Result{Path: relPath, Status: StatusMissing, Message: "file not found"}
 		}
 		return Result{Path: relPath, Status: StatusError, Message: err.Error()}

mfer/checker_test.go

@@ -381,39 +381,6 @@ func TestCheckSubdirectories(t *testing.T) {
 	assert.Equal(t, 3, okCount)
 }
 
-func TestCheckMissingFileDetectedWithoutFallback(t *testing.T) {
-	// Regression test: errors.Is(err, errors.New("...")) never matches because
-	// errors.New creates a new value each time. The fix uses os.ErrNotExist instead.
-	fs := afero.NewMemMapFs()
-	files := map[string][]byte{
-		"exists.txt":  []byte("here"),
-		"missing.txt": []byte("not on disk"),
-	}
-	createTestManifest(t, fs, "/manifest.mf", files)
-	// Only create one file on disk
-	createFilesOnDisk(t, fs, "/data", map[string][]byte{
-		"exists.txt": []byte("here"),
-	})
-
-	chk, err := NewChecker("/manifest.mf", "/data", fs)
-	require.NoError(t, err)
-
-	results := make(chan Result, 10)
-	err = chk.Check(context.Background(), results, nil)
-	require.NoError(t, err)
-
-	statusCounts := map[Status]int{}
-	for r := range results {
-		statusCounts[r.Status]++
-		if r.Status == StatusMissing {
-			assert.Equal(t, RelFilePath("missing.txt"), r.Path)
-		}
-	}
-	assert.Equal(t, 1, statusCounts[StatusOK], "one file should be OK")
-	assert.Equal(t, 1, statusCounts[StatusMissing], "one file should be MISSING")
-	assert.Equal(t, 0, statusCounts[StatusError], "no files should be ERROR")
-}
-
 func TestCheckEmptyManifest(t *testing.T) {
 	fs := afero.NewMemMapFs()
 	// Create manifest with no files

mfer/mf.proto

@@ -46,6 +46,9 @@ message MFFileOuter {
 
 message MFFilePath {
     // required attributes:
+    // Path invariants: must be valid UTF-8, use forward slashes only,
+    // be relative (no leading /), contain no ".." segments, and no
+    // empty segments (no "//").
     string path = 1;
     int64 size = 2;