From 9666676766bf21364f4fc4d0d1d336e72f2a91b4 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Tue, 6 Oct 2026 12:57:23 -0400 Subject: refactor: split casino.go, dedup helpers, remove audit docs --- internal/persistence/txn.go | 34 ++++++++++++ internal/persistence/txn_containment_test.go | 79 ++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 internal/persistence/txn_containment_test.go (limited to 'internal/persistence') diff --git a/internal/persistence/txn.go b/internal/persistence/txn.go index 1babfc5..df2b2e4 100644 --- a/internal/persistence/txn.go +++ b/internal/persistence/txn.go @@ -226,6 +226,9 @@ func (t *Transaction) Delete(path string) error { // Move relocates a file (rename semantics). If newPath already exists its // before-image is captured in a separate move-target record for rollback. func (t *Transaction) Move(oldPath, newPath string) error { + if oldPath == newPath { + return fmt.Errorf("move source and destination are the same: %s", oldPath) + } src, err := t.getOp(oldPath) if err != nil { return err @@ -240,6 +243,12 @@ func (t *Transaction) Move(oldPath, newPath string) error { if src.Kind == kindReplace || src.Kind == kindDelete { return fmt.Errorf("conflicting operation on %s: already replaced or deleted", oldPath) } + // Moving a path that already received a Replace as a move destination + // would drop that after-image: the kindMove branch of apply never writes + // op.New. Refuse rather than silently lose the write. + if src.Kind == kindMoveTarget && src.NewExists { + return fmt.Errorf("conflicting operation on %s: already replaced as a move destination", oldPath) + } if dst.Kind != kindMoveTarget && dst.Kind != "" { return fmt.Errorf("conflicting operation on %s", newPath) } @@ -366,6 +375,8 @@ func (t *Transaction) apply() error { return fmt.Errorf("write %s: %w", op.Path, err) } } + default: + return fmt.Errorf("unknown op kind %q for %s", op.Kind, op.Path) } } return nil @@ -449,9 +460,16 @@ func Recover(journalDir string) error { if entry.Version != 0 && entry.Version != journalVersion { return fmt.Errorf("journal %s: unsupported version %d", path, entry.Version) } + root := filepath.Dir(journalDir) t := &Transaction{ops: make(map[string]*fileOp), journalDir: journalDir} for i := range entry.Ops { op := entry.Ops[i] + if !withinDir(root, op.Path) { + return fmt.Errorf("recover %s: op path %q escapes data root %s", path, op.Path, root) + } + if op.MoveTo != "" && !withinDir(root, op.MoveTo) { + return fmt.Errorf("recover %s: move target %q escapes data root %s", path, op.MoveTo, root) + } t.ops[op.Path] = &op t.order = append(t.order, op.Path) } @@ -475,6 +493,22 @@ func hasPrefix(s, prefix string) bool { return len(s) >= len(prefix) && s[:len(prefix)] == prefix } +// withinDir reports whether path is lexically contained in root. Journal +// recovery replays before-images at the paths recorded in the journal, so a +// journal whose ops escape the data root (hand-edited on disk, or written by +// a future/attacker version) must be refused instead of allowed to write or +// delete arbitrary files as the service user. +func withinDir(root, path string) bool { + if path == "" { + return false + } + rel, err := filepath.Rel(root, path) + if err != nil || filepath.IsAbs(rel) { + return false + } + return rel != ".." && !hasPrefix(rel, ".."+string(filepath.Separator)) +} + // ListJournalFiles is a debugging helper: returns sorted journal file names. func ListJournalFiles(journalDir string) ([]string, error) { entries, err := os.ReadDir(journalDir) diff --git a/internal/persistence/txn_containment_test.go b/internal/persistence/txn_containment_test.go new file mode 100644 index 0000000..cfcd4c2 --- /dev/null +++ b/internal/persistence/txn_containment_test.go @@ -0,0 +1,79 @@ +package persistence + +import ( + "os" + "path/filepath" + "testing" +) + +func TestTxnMoveRejectsSamePath(t *testing.T) { + dir := t.TempDir() + txn := NewTransaction(filepath.Join(dir, ".journal")) + p := filepath.Join(dir, "a.yaml") + if err := txn.Move(p, p); err == nil { + t.Fatal("expected Move(path, path) to be rejected") + } +} + +// A path that already received a Replace as a move destination carries an +// after-image in its move-target record; a later Move would silently drop it. +func TestTxnMoveRejectsReplacedMoveDestination(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "a.yaml") + b := filepath.Join(dir, "b.yaml") + writeTestFile(t, a, "A") + writeTestFile(t, b, "B") + txn := NewTransaction(filepath.Join(dir, ".journal")) + if err := txn.Move(a, b); err != nil { + t.Fatal(err) + } + if err := txn.Replace(b, []byte("X"), 0644); err != nil { + t.Fatal(err) + } + if err := txn.Move(b, filepath.Join(dir, "c.yaml")); err == nil { + t.Fatal("expected Move over a replaced move destination to be rejected") + } +} + +// A journal on disk is untrusted input: recovery must not replay a before-image +// at a path outside the journal's data root. +func TestRecoverRejectsEscapingOpPath(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + escape := filepath.Join(outside, "secret.yaml") + body := `{"version":1,"ops":[{"path":"` + escape + `","kind":"delete","old_exists":false}]}` + evil := filepath.Join(journalDir, "txn-evil.json") + if err := os.WriteFile(evil, []byte(body), 0600); err != nil { + t.Fatal(err) + } + if err := Recover(journalDir); err == nil { + t.Fatal("expected Recover to reject an escaping op path") + } + // Fail loud and keep the journal rather than silently touching the path. + if _, err := os.Stat(evil); err != nil { + t.Fatalf("journal should be retained on rejection: %v", err) + } + if _, err := os.Stat(escape); !os.IsNotExist(err) { + t.Fatalf("escaping path must not be touched: %v", err) + } +} + +func TestRecoverRejectsEscapingMoveTarget(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + journalDir := filepath.Join(dir, ".journal") + if err := os.MkdirAll(journalDir, 0700); err != nil { + t.Fatal(err) + } + body := `{"version":1,"ops":[{"path":"` + filepath.Join(dir, "a.yaml") + `","kind":"move","move_to":"` + filepath.Join(outside, "b.yaml") + `"}]}` + if err := os.WriteFile(filepath.Join(journalDir, "txn-evil.json"), []byte(body), 0600); err != nil { + t.Fatal(err) + } + if err := Recover(journalDir); err == nil { + t.Fatal("expected Recover to reject an escaping move target") + } +} -- cgit v1.2.3