From 153acb7f644d755c45533b7c5311e3ffa47e8973 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Fri, 18 Sep 2026 05:31:09 -0400 Subject: fix: harden engine across casino, economy, concurrency, security, and content --- .../hardening_txn_commit_durability_test.go | 135 +++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 internal/persistence/hardening_txn_commit_durability_test.go (limited to 'internal/persistence/hardening_txn_commit_durability_test.go') diff --git a/internal/persistence/hardening_txn_commit_durability_test.go b/internal/persistence/hardening_txn_commit_durability_test.go new file mode 100644 index 0000000..f02bc9f --- /dev/null +++ b/internal/persistence/hardening_txn_commit_durability_test.go @@ -0,0 +1,135 @@ +package persistence + +// Commit-point durability regressions: the unlink that completes a +// transaction is the commit point, so it must be retried and made durable +// exactly like the journal write and every applied file. Otherwise a crash +// resurrects the journal and Recover rolls back an operation the caller was +// told succeeded. + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestCommitPointSyncsJournalDirectory: the journal removal is followed by a +// sync of the journal directory. Without it the unlink may not survive a +// crash and the replayed before-images revert committed work (and clobber +// later writes to the same files). +func TestCommitPointSyncsJournalDirectory(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "a.yaml"), "old-a") + + txn := NewTransaction(journalDir) + if err := txn.Replace(filepath.Join(dir, "a.yaml"), []byte("new-a"), 0600); err != nil { + t.Fatal(err) + } + var synced []string + txn.dirSync = func(d string) error { + synced = append(synced, d) + return syncDir(d) + } + if err := txn.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + if len(synced) == 0 { + t.Fatal("commit point removed the journal without syncing the journal directory: a crash can resurrect it and roll back the committed transaction") + } + for _, d := range synced { + if d != journalDir { + t.Fatalf("synced %s, want the journal directory %s", d, journalDir) + } + } + // The commit point holds: the journal is gone and nothing is rolled back. + if err := Recover(journalDir); err != nil { + t.Fatalf("recover after commit: %v", err) + } + if got := readTestFile(t, filepath.Join(dir, "a.yaml")); got != "new-a" { + t.Fatalf("committed write reverted: %q", got) + } +} + +// TestCommitReportsNonDurableCommitPoint: when the directory sync after the +// removal fails, the removal may not survive a crash, so Commit must not +// report a completed transaction: the error names the rollback risk. +func TestCommitReportsNonDurableCommitPoint(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + writeTestFile(t, filepath.Join(dir, "a.yaml"), "old-a") + + txn := NewTransaction(journalDir) + if err := txn.Replace(filepath.Join(dir, "a.yaml"), []byte("new-a"), 0600); err != nil { + t.Fatal(err) + } + txn.dirSync = func(string) error { return errors.New("injected sync failure") } + err := txn.Commit() + if err == nil { + t.Fatal("a non-durable commit point must be reported as a failure") + } + if !strings.Contains(err.Error(), "could not be synced") { + t.Fatalf("error must name the unsynced removal, got: %v", err) + } + // The journal is gone from the visible filesystem but its removal is not + // durable: recovery at the next startup may roll the operation back. + if entries, _ := ListJournalFiles(journalDir); len(entries) != 0 { + t.Fatalf("journal removal did not happen: %v", entries) + } +} + +// TestRemoveJournalRetainsJournalOnPersistentFailure: when the unlink itself +// keeps failing, Commit must return an error that names the retained journal +// and the rollback that will follow, and the journal must stay on disk as the +// durable record. Once the fault is repaired the removal succeeds. +// +// The failure is injected with a journal path that cannot be unlinked (a +// non-empty directory, which unlink refuses regardless of privilege) instead +// of a read-only directory: a chmod-based injection is a no-op for root, so +// under the root-owned test runner the test used to skip and never executed +// the retained-journal branch at all. +func TestRemoveJournalRetainsJournalOnPersistentFailure(t *testing.T) { + dir := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + journalPath := filepath.Join(journalDir, "txn-test.json") + if err := os.Mkdir(journalPath, 0700); err != nil { + t.Fatal(err) + } + blocker := filepath.Join(journalPath, "keep") + if err := os.WriteFile(blocker, []byte("keep"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Remove(journalPath); err == nil || os.IsNotExist(err) { + t.Fatalf("removing a non-empty journal path unexpectedly succeeded: a removal failure cannot be injected") + } + + txn := NewTransaction(journalDir) + err := txn.removeJournalDurable(journalPath) + if err == nil { + t.Fatal("a persistent journal removal failure must be reported, not swallowed") + } + for _, want := range []string{"retained", "rolled back"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error must state that the journal is %q, got: %v", want, err) + } + } + if _, statErr := os.Stat(journalPath); statErr != nil { + t.Fatalf("the retained journal is the durable record and must stay on disk: %v", statErr) + } + + // Repair: the path is unlinkable again, so the next attempt removes the + // journal and succeeds. + if err := os.Remove(blocker); err != nil { + t.Fatal(err) + } + if err := txn.removeJournalDurable(journalPath); err != nil { + t.Fatalf("removal after repair: %v", err) + } + if _, statErr := os.Stat(journalPath); !os.IsNotExist(statErr) { + t.Fatalf("journal not removed after repair: %v", statErr) + } +} -- cgit v1.2.3