tka: don't try to read AUMs which are partway through being written

Fixes tailscale#17600

Signed-off-by: Alex Chan <alexc@tailscale.com>
(cherry picked from commit 23359dc)

Author: alexwlchan

tka/tailchonk.go

@@ -9,6 +9,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"log"
 	"os"
 	"path/filepath"
 	"slices"
@@ -403,9 +404,16 @@ func (c *FS) scanHashes(eachHashInfo func(*fsHashInfo)) error {
 			return fmt.Errorf("reading prefix dir: %v", err)
 		}
 		for _, file := range files {
+			// Ignore files whose names aren't valid AUM hashes, which may be
+			// temporary files which are partway through being written, or other
+			// files added by the OS (like .DS_Store) which we can ignore.
+			// TODO(alexc): it might be useful to append a suffix like `.aum` to
+			// filenames, so we can more easily distinguish between AUMs and
+			// arbitrary other files.
 			var h AUMHash
 			if err := h.UnmarshalText([]byte(file.Name())); err != nil {
-				return fmt.Errorf("invalid aum file: %s: %w", file.Name(), err)
+				log.Printf("ignoring unexpected non-AUM: %s: %v", file.Name(), err)
+				continue
 			}
 			info, err := c.get(h)
 			if err != nil {

tka/tailchonk_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"os"
 	"path/filepath"
+	"slices"
 	"sync"
 	"testing"
 	"time"
@@ -83,6 +84,49 @@ func TestTailchonkFS_CommitTime(t *testing.T) {
 	}
 }
 
+// If we were interrupted while writing a temporary file, AllAUMs()
+// should ignore it when scanning the AUM directory.
+func TestTailchonkFS_IgnoreTempFile(t *testing.T) {
+	base := t.TempDir()
+	chonk := must.Get(ChonkDir(base))
+	parentHash := randHash(t, 1)
+	aum := AUM{MessageKind: AUMNoOp, PrevAUMHash: parentHash[:]}
+	must.Do(chonk.CommitVerifiedAUMs([]AUM{aum}))
+
+	writeAUMFile := func(filename, contents string) {
+		t.Helper()
+		if err := os.MkdirAll(filepath.Join(base, filename[0:2]), os.ModePerm); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(base, filename[0:2], filename), []byte(contents), 0600); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Check that calling AllAUMs() returns the single committed AUM
+	got, err := chonk.AllAUMs()
+	if err != nil {
+		t.Fatalf("AllAUMs() failed: %v", err)
+	}
+	want := []AUMHash{aum.Hash()}
+	if !slices.Equal(got, want) {
+		t.Fatalf("AllAUMs() is wrong: got %v, want %v", got, want)
+	}
+
+	// Write some temporary files which are named like partially-committed AUMs,
+	// then check that AllAUMs() only returns the single committed AUM.
+	writeAUMFile("AUM1234.tmp", "incomplete AUM\n")
+	writeAUMFile("AUM1234.tmp_123", "second incomplete AUM\n")
+
+	got, err = chonk.AllAUMs()
+	if err != nil {
+		t.Fatalf("AllAUMs() failed: %v", err)
+	}
+	if !slices.Equal(got, want) {
+		t.Fatalf("AllAUMs() is wrong: got %v, want %v", got, want)
+	}
+}
+
 func TestMarkActiveChain(t *testing.T) {
 	type aumTemplate struct {
 		AUM AUM