fix: harden engine across casino, economy, concurrency, security, and content

1 files changed, 135 insertions, 0 deletions

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)
+	}
+}