Refuse a file whose size changed between freshen's scan and hash (closes #207)

freshen took a changed or new file's size from the scan and its hash
from a later read, without comparing the bytes read with that size. A
file that grew or shrank in between got an entry no content can match,
so check failed it with nothing saying why. processEntry now fails with
a size mismatch naming the file, as gen does, so freshen exits 1 and
leaves the manifest as it was.

Judgement call: the error wraps the package's existing errSizeMismatch
from fetch; its comment now covers both uses.

Model: opus-5-5

internal/cli/errmsg_test.go

@@ -473,6 +473,28 @@ func TestFreshenReadErrorMessage(t *testing.T) {
 		"read "+filepath.Join(root, "sub")+": is a directory")
 }
 
+// TestFreshenSizeChangedMessage has freshen hash a file that has grown
+// since the scan recorded its size.
+func TestFreshenSizeChangedMessage(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	writeTestFile(t, fs, "/d/log.txt", "hello")
+
+	hasher := &freshenHasher{
+		fs:      fs,
+		absBase: "/d",
+		builder: mfer.NewBuilder(),
+	}
+
+	err := hasher.processEntry(
+		&freshenEntry{path: "log.txt", size: 3, needsHash: true})
+	require.ErrorIs(t, err, errSizeMismatch)
+	assert.EqualError(t, err, `size mismatch for "log.txt": `+
+		`3 bytes when scanned but 5 bytes when read; `+
+		`the file changed while freshen ran`)
+}
+
 func TestURLRequiredMessage(t *testing.T) {
 	t.Parallel()
 

internal/cli/fetch.go

@@ -86,7 +86,8 @@ var (
 	// symlink that already exists in the target directory.
 	errSymlinkInPath = errors.New("symlink in path not allowed")
 	// errSizeMismatch indicates a downloaded file with an unexpected
-	// size.
+	// size, or a file freshen read at a size other than the one its scan
+	// recorded.
 	errSizeMismatch = errors.New("size mismatch")
 	// errHashMismatch indicates a downloaded file whose hash matches no
 	// manifest hash.

internal/cli/freshen.go

@@ -311,6 +311,16 @@ func (h *freshenHasher) processEntry(e *freshenEntry) error {
 		return err
 	}
 
+	// The scan recorded the size; a file that grew or shrank since would
+	// get an entry whose size and hash describe different contents.
+	if bytesRead != e.size {
+		return fmt.Errorf(
+			"%w for %q: %d bytes when scanned but %d bytes when read; "+
+				"the file changed while freshen ran",
+			errSizeMismatch, e.path, e.size, bytesRead,
+		)
+	}
+
 	h.hashedBytes += bytesRead
 	h.hashedFiles++
 

internal/cli/freshen_test.go

@@ -243,6 +243,58 @@ func TestFreshenWithChanges(t *testing.T) {
 	}
 }
 
+// growingFileFs appends to one file each time it is opened, as a program
+// still writing to the file would between freshen's scan and its hash.
+type growingFileFs struct {
+	afero.Fs
+
+	path string
+}
+
+//nolint:ireturn // Open must return afero.File to satisfy afero.Fs.
+func (f growingFileFs) Open(name string) (afero.File, error) {
+	if name == f.path {
+		file, err := f.OpenFile(name, os.O_APPEND|os.O_WRONLY, 0)
+		if err != nil {
+			return nil, err
+		}
+
+		_, err = file.WriteString(" world")
+		_ = file.Close()
+
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return f.Fs.Open(name)
+}
+
+// TestFreshenFileChangedSinceScan has a new file grow after freshen's scan
+// recorded its size and before freshen hashes it: freshen must fail and
+// leave the manifest as it was.
+func TestFreshenFileChangedSinceScan(t *testing.T) {
+	t.Parallel()
+
+	osFs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, osFs,
+		map[string]string{testFileTxt: "content"})
+	grown := filepath.Join(root, "log.txt")
+	writeTestFile(t, osFs, grown, "hello")
+
+	before, err := afero.ReadFile(osFs, manifestPath)
+	require.NoError(t, err)
+
+	opts := testOpts([]string{
+		testApp, cmdFreshen, "-q", testFlagBase, root, manifestPath,
+	}, growingFileFs{Fs: osFs, path: grown})
+	assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
+
+	after, err := afero.ReadFile(osFs, manifestPath)
+	require.NoError(t, err)
+	assert.Equal(t, before, after, "manifest changed")
+}
+
 // TestFreshenLeavesManifestOutOfListing freshens a manifest kept in a
 // subdirectory of the tree it lists: the manifest is not listed, while an
 // ordinary file of the same name at the top of the tree is. The manifest