From ba671a76ed52224f4cf1dd93f477cf280883373a Mon Sep 17 00:00:00 2001
From: vickydotbat <vickydotbat@tutamail.com>
Date: Fri, 31 Jul 2026 13:08:35 +0200
Subject: [PATCH] fix(nwsync): hash through a section reader, pin the encoder
 claim (#76)

Review follow-ups. checkArtifactKey drained the *os.File to EOF, so the
file offset was left at the end; correct only because everything after it
uses ReadAt. Hash a section reader instead, so no later sequential read
can silently see nothing.

The zstd concurrency comment claimed byte-identical output but nothing
asserted it, so add the test. erf.Read now says outright that Data must
not be mutated, since payloads alias one buffer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 internal/erf/erf.go            |  5 +++--
 internal/nwsync/emit.go        |  4 +++-
 internal/nwsync/memory_test.go | 24 ++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/internal/erf/erf.go b/internal/erf/erf.go
index 20ba89d..372878d 100644
--- a/internal/erf/erf.go
+++ b/internal/erf/erf.go
@@ -389,8 +389,9 @@ func ReadPayload(r io.ReaderAt, entry IndexEntry) ([]byte, error) {
 }
 
 // Read materialises a whole archive. Payloads are subslices of the buffer the
-// archive was read into, so nothing is copied twice; callers that only need one
-// resource at a time should use ReadIndex instead.
+// archive was read into, so nothing is copied twice: a caller must not mutate
+// Data. Callers that only need one resource at a time should use ReadIndex
+// instead, which never holds the archive at all.
 func Read(r io.Reader) (Archive, error) {
 	data, err := io.ReadAll(r)
 	if err != nil {
diff --git a/internal/nwsync/emit.go b/internal/nwsync/emit.go
index c2675b3..7574e70 100644
--- a/internal/nwsync/emit.go
+++ b/internal/nwsync/emit.go
@@ -88,7 +88,9 @@ func Emit(options EmitOptions) (EmitResult, error) {
 	if err != nil {
 		return EmitResult{}, fmt.Errorf("read artifact: %w", err)
 	}
-	if err := checkArtifactKey(options.ArtifactKey, artifact); err != nil {
+	// A section reader, not the file itself: hashing must not move the file
+	// offset out from under everything that reads the artifact afterwards.
+	if err := checkArtifactKey(options.ArtifactKey, io.NewSectionReader(artifact, 0, info.Size())); err != nil {
 		return EmitResult{}, err
 	}
 	name := options.As
diff --git a/internal/nwsync/memory_test.go b/internal/nwsync/memory_test.go
index fb8be75..1a94d08 100644
--- a/internal/nwsync/memory_test.go
+++ b/internal/nwsync/memory_test.go
@@ -1,7 +1,9 @@
 package nwsync
 
 import (
+	"bytes"
 	"fmt"
+	"math/rand"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -9,9 +11,31 @@ import (
 	"testing"
 	"time"
 
+	"github.com/klauspost/compress/zstd"
+
 	"git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/erf"
 )
 
+// TestSingleThreadedEncoderMatchesDefault pins the claim the blob encoder's
+// concurrency setting rests on: it saves memory only, and a published blob is
+// the same bytes either way.
+func TestSingleThreadedEncoderMatchesDefault(t *testing.T) {
+	standard, err := zstd.NewWriter(nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer standard.Close()
+
+	body := make([]byte, 4<<20)
+	random := rand.New(rand.NewSource(1))
+	random.Read(body[:len(body)/2])
+	for _, size := range []int{0, 1, 4 << 10, len(body)} {
+		if !bytes.Equal(blobEncoder.EncodeAll(body[:size], nil), standard.EncodeAll(body[:size], nil)) {
+			t.Fatalf("%d bytes compress differently at concurrency 1", size)
+		}
+	}
+}
+
 // resourceSize is one payload in the memory fixtures. Real haks hold a few MB
 // per resource, and peak memory is meant to track that, not the archive.
 const resourceSize = 1 << 20