From 23359dc72706b7d9f32dcd428f22f5e4fdbfc4b7 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Tue, 21 Oct 2025 11:07:33 +0100 Subject: [PATCH] tka: don't try to read AUMs which are partway through being written Fixes https://github.com/tailscale/tailscale/issues/17600 Signed-off-by: Alex Chan --- tka/tailchonk.go | 10 +++++++++- tka/tailchonk_test.go | 44 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/tka/tailchonk.go b/tka/tailchonk.go index d01c5826e..7750b0622 100644 --- a/tka/tailchonk.go +++ b/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 { diff --git a/tka/tailchonk_test.go b/tka/tailchonk_test.go index 086865980..1a6bad459 100644 --- a/tka/tailchonk_test.go +++ b/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