Cover bounded reads and cancelled replacements

docs/internal/fileops.md

@@ -9,10 +9,11 @@ and destination-directory policy.
 Ordinary workspace replacement uses standard operating-system path semantics.
 Callers with a root-scoped safety contract own that validation at their boundary.
 
-Remote object acquisition uses a writer supplied by the storage owner. The
-writer receives a `fileops`-owned, already-open sibling temporary file rather
-than a mutable destination path. Callers still own remote object selection,
-validation, conflict handling, and final mode.
+Remote object acquisition is owned by its domain caller. The caller writes a
+sibling temporary file, validates the complete result, and passes that file to
+`InstallSiblingTempFile` only when it is ready to become canonical. Callers own
+remote object selection, validation, conflict handling, and temporary-file
+cleanup.
 
 Directory bundles are not promoted by `internal/fileops`; extraction owns its
 validated Notarius bundle rename.
@@ -48,6 +49,7 @@ Replacement follows the operating system's same-filesystem rename semantics.
 If a platform cannot replace an existing destination, the operation returns an
 error and never removes the old file as an emulation step.
 
-`WriteFileAtomic`, copy helpers, and downloaded temporary-file installation
-retain their compatibility behavior of creating the destination parent with
-the repository's workspace permissions before using this contract.
+`WriteFileAtomic` and the copy helpers create the destination parent with the
+repository's workspace permissions. `InstallSiblingTempFile` requires the
+caller to provide an existing temporary file in the existing destination
+directory.

internal/fileops/fileops_test.go

@@ -3,6 +3,7 @@ package fileops
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
@@ -45,6 +46,33 @@ func TestWriteFileAtomicCleansTempFileOnInstallFailure(t *testing.T) {
 	assertNoMatchingTempFiles(t, root, ".blocked.tmp-")
 }
 
+func TestReplaceFileAtomicCancellationPreservesDestinationAndCleansTemp(t *testing.T) {
+	root := t.TempDir()
+	dst := filepath.Join(root, "value.txt")
+	if err := os.WriteFile(dst, []byte("old"), 0o644); err != nil {
+		t.Fatalf("WriteFile(destination) error = %v", err)
+	}
+	wantErr := errors.New("cancel replacement")
+
+	err := ReplaceFileAtomic(dst, []byte("new"), ReplaceFileOptions{
+		Mode: WorkspaceFileMode,
+		BeforeRename: func() error {
+			return wantErr
+		},
+	})
+	if !errors.Is(err, wantErr) {
+		t.Fatalf("ReplaceFileAtomic() error = %v, want wrapped cancellation", err)
+	}
+	data, err := os.ReadFile(dst)
+	if err != nil {
+		t.Fatalf("ReadFile(destination) error = %v", err)
+	}
+	if string(data) != "old" {
+		t.Fatalf("destination content = %q, want old content", data)
+	}
+	assertNoMatchingTempFiles(t, root, ".value.txt.tmp-")
+}
+
 func TestCopyFileAtomicWithChecksumMatchesDestination(t *testing.T) {
 	root := t.TempDir()
 	src := filepath.Join(root, "source.txt")

internal/fileops/read_bounded.go

@@ -3,6 +3,7 @@ package fileops
 import (
 	"fmt"
 	"io"
+	"math"
 	"os"
 	"strings"
 )
@@ -21,7 +22,11 @@ func ReadFileBounded(path string, limit int64) ([]byte, error) {
 		return nil, fmt.Errorf("open file %q: %w", path, err)
 	}
 	defer func() { _ = file.Close() }()
-	data, err := io.ReadAll(io.LimitReader(file, limit+1))
+	readLimit := limit
+	if limit < math.MaxInt64 {
+		readLimit++
+	}
+	data, err := io.ReadAll(io.LimitReader(file, readLimit))
 	if err != nil {
 		return nil, fmt.Errorf("read file %q: %w", path, err)
 	}

internal/fileops/read_bounded_test.go

@@ -0,0 +1,61 @@
+package fileops
+
+import (
+	"math"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestReadFileBoundedEnforcesLimitWithoutPartialContent(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, "value.txt")
+	if err := os.WriteFile(path, []byte("12345"), 0o644); err != nil {
+		t.Fatalf("WriteFile() error = %v", err)
+	}
+
+	data, err := ReadFileBounded(path, 5)
+	if err != nil {
+		t.Fatalf("ReadFileBounded(exact limit) error = %v", err)
+	}
+	if string(data) != "12345" {
+		t.Fatalf("ReadFileBounded(exact limit) = %q, want complete content", data)
+	}
+
+	data, err = ReadFileBounded(path, 4)
+	if err == nil || !strings.Contains(err.Error(), "4-byte limit") {
+		t.Fatalf("ReadFileBounded(limit plus one) data = %q error = %v, want limit error", data, err)
+	}
+	if data != nil {
+		t.Fatalf("ReadFileBounded(limit plus one) returned partial data %q", data)
+	}
+}
+
+func TestReadFileBoundedHandlesBoundaryLimits(t *testing.T) {
+	root := t.TempDir()
+	emptyPath := filepath.Join(root, "empty.txt")
+	valuePath := filepath.Join(root, "value.txt")
+	if err := os.WriteFile(emptyPath, nil, 0o644); err != nil {
+		t.Fatalf("WriteFile(empty) error = %v", err)
+	}
+	if err := os.WriteFile(valuePath, []byte("value"), 0o644); err != nil {
+		t.Fatalf("WriteFile(value) error = %v", err)
+	}
+
+	if data, err := ReadFileBounded(emptyPath, 0); err != nil || len(data) != 0 {
+		t.Fatalf("ReadFileBounded(empty, zero) data = %q error = %v", data, err)
+	}
+	if data, err := ReadFileBounded(valuePath, 0); err == nil || data != nil {
+		t.Fatalf("ReadFileBounded(value, zero) data = %q error = %v, want limit error", data, err)
+	}
+	if data, err := ReadFileBounded(valuePath, math.MaxInt64); err != nil || string(data) != "value" {
+		t.Fatalf("ReadFileBounded(max limit) data = %q error = %v", data, err)
+	}
+	if _, err := ReadFileBounded(valuePath, -1); err == nil || !strings.Contains(err.Error(), "invalid") {
+		t.Fatalf("ReadFileBounded(negative limit) error = %v, want invalid limit", err)
+	}
+	if _, err := ReadFileBounded(" ", 1); err == nil || !strings.Contains(err.Error(), "path is required") {
+		t.Fatalf("ReadFileBounded(empty path) error = %v, want required path", err)
+	}
+}