From 90f0e95ccd5a4524d94955b32fc19f6dc9987da5 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 09:36:56 -0700 Subject: [PATCH] fix(engine): fail closed on invalid epochs Preserve fencing monotonicity by accepting only absent or valid durable epoch state and replacing epoch files atomically. Closes #55 --- cmd/ob/ops_contract_test.go | 3 + internal/engine/backup_lock.go | 17 +- internal/engine/backup_lock_test.go | 10 +- internal/engine/epoch.go | 62 ++++ internal/engine/epoch_test.go | 332 ++++++++++++++++++ internal/engine/lock.go | 6 +- internal/engine/lock_test.go | 15 +- internal/transport/fake.go | 5 + .../docs/explanation/safety-envelope.mdx | 13 + site/src/content/docs/guides/roll-back.mdx | 13 + 10 files changed, 457 insertions(+), 19 deletions(-) create mode 100644 internal/engine/epoch.go create mode 100644 internal/engine/epoch_test.go diff --git a/cmd/ob/ops_contract_test.go b/cmd/ob/ops_contract_test.go index e51d76b2..e9e43b95 100644 --- a/cmd/ob/ops_contract_test.go +++ b/cmd/ob/ops_contract_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/labstack/onebox/internal/app" "github.com/labstack/onebox/internal/transport" ) @@ -179,6 +180,8 @@ services: {postgres: 17} } fake := &transport.Fake{HostName: "example.invalid", Dynamic: func(command string) (transport.Result, bool) { switch { + case strings.HasPrefix(command, ": ob-epoch-probe;"): + return transport.Result{ExitCode: app.ProbeAbsent}, true case strings.Contains(command, "/_host/owner"): return transport.Result{Stdout: "shop\n"}, true case strings.Contains(command, " logs "): diff --git a/internal/engine/backup_lock.go b/internal/engine/backup_lock.go index 30a60d31..0ddadedd 100644 --- a/internal/engine/backup_lock.go +++ b/internal/engine/backup_lock.go @@ -110,7 +110,13 @@ func (e *Engine) AcquireBackupLock(ctx context.Context, service, operationID str return 0, err } if res.ExitCode == 0 { + if e.backupLockVals == nil { + e.backupLockVals = make(map[string]string) + e.backupFenceVals = make(map[string]string) + } + e.backupLockVals[service] = lockValue if err := e.writeBackupFence(ctx, service, operationID, epoch, lockValue); err != nil { + e.ReleaseBackupLock(service) return 0, err } return epoch, nil @@ -159,17 +165,14 @@ func (e *Engine) AcquireBackupLock(ctx context.Context, service, operationID str } func (e *Engine) nextBackupEpoch(ctx context.Context, service string) (int, error) { - result, err := e.T.Run(ctx, "cat "+q(e.backupEpochPath(service))+" 2>/dev/null || echo 0") - if err != nil { - return 0, err - } - previous, _ := strconv.Atoi(strings.TrimSpace(result.Stdout)) - return previous + 1, nil + return e.nextEpoch(ctx, e.backupEpochPath(service)) } func (e *Engine) writeBackupFence(ctx context.Context, service, operationID string, epoch int, lockValue string) error { fenceValue := operationID + " " + strconv.Itoa(epoch) - command := `if [ "$(cat ` + q(e.backupLockPath(service)) + ` 2>/dev/null)" = ` + q(lockValue) + ` ]; then echo ` + strconv.Itoa(epoch) + ` > ` + q(e.backupEpochPath(service)) + ` && echo ` + q(fenceValue) + ` > ` + q(e.backupFencePath(service)) + `; else echo ob-backup-lock-lost >&2; exit 96; fi` + command := `if [ "$(cat ` + q(e.backupLockPath(service)) + ` 2>/dev/null)" = ` + q(lockValue) + ` ]; then ` + + atomicEpochWriteCmd(e.backupEpochPath(service), epoch) + `; echo ` + q(fenceValue) + ` > ` + q(e.backupFencePath(service)) + + `; else echo ob-backup-lock-lost >&2; exit 96; fi` result, err := e.T.Run(ctx, command) if err != nil { return err diff --git a/internal/engine/backup_lock_test.go b/internal/engine/backup_lock_test.go index 532fc2b3..b12d7671 100644 --- a/internal/engine/backup_lock_test.go +++ b/internal/engine/backup_lock_test.go @@ -81,7 +81,7 @@ func TestBackupLockHonorsCancellation(t *testing.T) { } func TestBackupLockReclaimsStaleHolderWithNewFence(t *testing.T) { - holder := `{"owner":"operator","operation_id":"backup-old","service":"database","epoch":4,"ttl_s":10,"acquired_at":"2026-08-07T11:00:00Z"}` + holder := `{"owner":"operator","operation_id":"backup-same","service":"database","epoch":4,"ttl_s":10,"acquired_at":"2026-08-07T11:00:00Z"}` createAttempts := 0 fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { switch { @@ -91,25 +91,25 @@ func TestBackupLockReclaimsStaleHolderWithNewFence(t *testing.T) { return transport.Result{ExitCode: 1}, true } return transport.Result{}, true - case strings.HasPrefix(command, "cat ") && strings.Contains(command, "database.epoch"): + case strings.Contains(command, "cat ") && strings.Contains(command, "database.epoch"): return transport.Result{Stdout: "4\n"}, true case strings.HasPrefix(command, "cat ") && strings.Contains(command, "database.lock"): return transport.Result{Stdout: holder + "\n"}, true case strings.HasPrefix(command, "if [ -L ") && strings.Contains(command, "database.lock"): - return transport.Result{Stdout: "11\n"}, true + return transport.Result{Stdout: "1\n"}, true } return transport.Result{}, false }} engine := backupLockTestEngine(fake) - epoch, err := engine.AcquireBackupLock(context.Background(), "database", "backup-new", 0) + epoch, err := engine.AcquireBackupLock(context.Background(), "database", "backup-same", 0) if err != nil { t.Fatalf("reclaim stale backup lock: %v", err) } if epoch != 5 || createAttempts != 2 { t.Fatalf("reclaimed epoch/attempts = %d/%d, want 5/2", epoch, createAttempts) } - if got := engine.backupFenceVals["database"]; got != "backup-new 5" { + if got := engine.backupFenceVals["database"]; got != "backup-same 5" || got == "backup-same 4" { t.Fatalf("backup fence = %q", got) } } diff --git a/internal/engine/epoch.go b/internal/engine/epoch.go new file mode 100644 index 00000000..7f13add0 --- /dev/null +++ b/internal/engine/epoch.go @@ -0,0 +1,62 @@ +package engine + +import ( + "context" + "fmt" + "path" + "strconv" + "strings" + + "github.com/labstack/onebox/internal/app" +) + +// nextEpoch reads one durable fencing authority. Absence is the only state +// that means zero: an unreadable or malformed value must never reissue an epoch +// a stale runner may still hold. +func (e *Engine) nextEpoch(ctx context.Context, epochPath string) (int, error) { + result, err := e.T.Run(ctx, epochProbeCmd(epochPath)) + if err != nil { + return 0, err + } + switch result.ExitCode { + case 0: + // Parsed below. + case app.ProbeAbsent: + return 1, nil + case app.ProbeUnreadable: + return 0, fmt.Errorf("epoch file %s exists but cannot be read", epochPath) + case app.ProbeNotRegular: + return 0, fmt.Errorf("epoch file %s is not a regular file", epochPath) + case app.ProbeUndetermined: + return 0, fmt.Errorf("epoch file %s cannot be observed because an ancestor directory is not searchable", epochPath) + case app.ProbeStatePathNotDirectory: + return 0, fmt.Errorf("epoch file %s cannot be read because an ancestor path is not a directory", epochPath) + default: + return 0, fmt.Errorf("read epoch file %s failed (exit %d): %s", epochPath, result.ExitCode, strings.TrimSpace(result.Stderr)) + } + + raw := strings.TrimSpace(result.Stdout) + previous, err := strconv.Atoi(raw) + if err != nil || previous < 0 || previous == int(^uint(0)>>1) { + return 0, fmt.Errorf("epoch file %s contains invalid value %q", epochPath, raw) + } + return previous + 1, nil +} + +// epochProbeCmd distinguishes a file that is genuinely absent from one hidden +// by permissions or replaced with another kind of filesystem object. +func epochProbeCmd(epochPath string) string { + p := q(epochPath) + return ": ob-epoch-probe; if [ ! -e " + p + " ] && [ ! -L " + p + " ]; then " + + app.UndeterminedArm(epochPath) + "exit " + strconv.Itoa(app.ProbeAbsent) + "; fi; " + + "if [ ! -f " + p + " ] || [ -L " + p + " ]; then exit " + strconv.Itoa(app.ProbeNotRegular) + "; fi; " + + "if [ ! -r " + p + " ]; then exit " + strconv.Itoa(app.ProbeUnreadable) + "; fi; cat " + p +} + +// atomicEpochWriteCmd writes beside the epoch and renames over it. A killed +// shell can leave a disposable temp file, never a truncated authority. +func atomicEpochWriteCmd(epochPath string, epoch int) string { + template := epochPath + ".tmp.XXXXXX" + return "set -eu; test -d " + q(path.Dir(epochPath)) + "; umask 077; tmp=$(mktemp " + q(template) + "); " + + `trap 'rm -f "$tmp"' 0 1 2 15; printf '%s\n' ` + strconv.Itoa(epoch) + ` > "$tmp"; chmod 600 "$tmp"; mv -f "$tmp" ` + q(epochPath) + `; trap - 0 1 2 15` +} diff --git a/internal/engine/epoch_test.go b/internal/engine/epoch_test.go new file mode 100644 index 00000000..764ac0de --- /dev/null +++ b/internal/engine/epoch_test.go @@ -0,0 +1,332 @@ +package engine + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/labstack/onebox/internal/app" + "github.com/labstack/onebox/internal/transport" +) + +func TestNextEpochFailsClosed(t *testing.T) { + maxInt := int(^uint(0) >> 1) + tests := []struct { + name string + result transport.Result + want int + err bool + }{ + {name: "absent", result: transport.Result{ExitCode: app.ProbeAbsent}, want: 1}, + {name: "valid", result: transport.Result{Stdout: "41\n"}, want: 42}, + {name: "empty", result: transport.Result{}, err: true}, + {name: "malformed", result: transport.Result{Stdout: "not-an-epoch\n"}, err: true}, + {name: "negative", result: transport.Result{Stdout: "-1\n"}, err: true}, + {name: "overflow", result: transport.Result{Stdout: strconv.FormatUint(uint64(maxInt)+1, 10)}, err: true}, + {name: "unreadable", result: transport.Result{ExitCode: app.ProbeUnreadable}, err: true}, + {name: "not-regular", result: transport.Result{ExitCode: app.ProbeNotRegular}, err: true}, + {name: "undetermined", result: transport.Result{ExitCode: app.ProbeUndetermined}, err: true}, + {name: "broken-parent", result: transport.Result{ExitCode: app.ProbeStatePathNotDirectory}, err: true}, + {name: "probe-failure", result: transport.Result{ExitCode: 23, Stderr: "probe failed"}, err: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if command == epochProbeCmd("/state/epoch") { + return test.result, true + } + return transport.Result{}, false + }} + engine := &Engine{T: fake} + got, err := engine.nextEpoch(context.Background(), "/state/epoch") + if test.err { + if err == nil || !strings.Contains(err.Error(), "epoch") { + t.Fatalf("next epoch error = %v", err) + } + return + } + if err != nil || got != test.want { + t.Fatalf("next epoch = %d, %v; want %d", got, err, test.want) + } + }) + } +} + +func TestEpochProbeBehavior(t *testing.T) { + dir := t.TempDir() + run := func(epochPath string) transport.Result { + t.Helper() + result, err := transport.NewLocal().Run(context.Background(), epochProbeCmd(epochPath)) + if err != nil { + t.Fatal(err) + } + return result + } + + epochPath := filepath.Join(dir, "epoch") + if result := run(epochPath); result.ExitCode != app.ProbeAbsent { + t.Fatalf("absent epoch exit = %d, want %d", result.ExitCode, app.ProbeAbsent) + } + if err := os.WriteFile(epochPath, []byte("7\n"), 0o600); err != nil { + t.Fatal(err) + } + if result := run(epochPath); result.ExitCode != 0 || result.Stdout != "7\n" { + t.Fatalf("regular epoch = %#v", result) + } + if err := os.Remove(epochPath); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(epochPath, 0o700); err != nil { + t.Fatal(err) + } + if result := run(epochPath); result.ExitCode != app.ProbeNotRegular { + t.Fatalf("directory epoch exit = %d, want %d", result.ExitCode, app.ProbeNotRegular) + } + if err := os.Remove(epochPath); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, "missing"), epochPath); err != nil { + t.Fatal(err) + } + if result := run(epochPath); result.ExitCode != app.ProbeNotRegular { + t.Fatalf("symlink epoch exit = %d, want %d", result.ExitCode, app.ProbeNotRegular) + } +} + +func TestAtomicEpochWriteReplacesThroughSiblingTemp(t *testing.T) { + dir := t.TempDir() + epochPath := filepath.Join(dir, "epoch") + if err := os.WriteFile(epochPath, []byte("7\n"), 0o600); err != nil { + t.Fatal(err) + } + command := atomicEpochWriteCmd(epochPath, 8) + for _, want := range []string{"umask 077", "mktemp", `trap 'rm -f "$tmp"'`, `> "$tmp"`, "chmod 600", `mv -f "$tmp"`} { + if !strings.Contains(command, want) { + t.Fatalf("atomic epoch command missing %q:\n%s", want, command) + } + } + if strings.Contains(command, "> "+q(epochPath)) { + t.Fatalf("epoch target is truncated directly:\n%s", command) + } + result, err := transport.NewLocal().Run(context.Background(), command) + if err != nil || result.ExitCode != 0 { + t.Fatalf("atomic epoch write: exit=%d err=%v stderr=%s", result.ExitCode, err, result.Stderr) + } + body, err := os.ReadFile(epochPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(body, []byte("8\n")) { + t.Fatalf("epoch body = %q", body) + } + info, err := os.Stat(epochPath) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("epoch mode = %o", info.Mode().Perm()) + } + leftovers, err := filepath.Glob(epochPath + ".tmp.*") + if err != nil { + t.Fatal(err) + } + if len(leftovers) != 0 { + t.Fatalf("epoch temp files remain: %v", leftovers) + } +} + +func TestAtomicEpochWriteFailurePreservesPreviousValue(t *testing.T) { + dir := t.TempDir() + epochPath := filepath.Join(dir, "epoch") + if err := os.WriteFile(epochPath, []byte("7\n"), 0o600); err != nil { + t.Fatal(err) + } + binDir := filepath.Join(dir, "bin") + if err := os.Mkdir(binDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(binDir, "mv"), []byte("#!/bin/sh\nexit 23\n"), 0o700); err != nil { + t.Fatal(err) + } + command := "PATH=" + q(binDir) + ":$PATH; export PATH; " + atomicEpochWriteCmd(epochPath, 8) + result, err := transport.NewLocal().Run(context.Background(), command) + if err != nil { + t.Fatal(err) + } + if result.ExitCode == 0 { + t.Fatal("simulated interrupted rename unexpectedly succeeded") + } + body, err := os.ReadFile(epochPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(body, []byte("7\n")) { + t.Fatalf("failed atomic write changed previous epoch to %q", body) + } +} + +type epochAcquisitionCase struct { + name string + result transport.Result + want int + err bool +} + +func epochAcquisitionCases() []epochAcquisitionCase { + maxInt := int(^uint(0) >> 1) + return []epochAcquisitionCase{ + {name: "absent", result: transport.Result{ExitCode: app.ProbeAbsent}, want: 1}, + {name: "valid", result: transport.Result{Stdout: "41\n"}, want: 42}, + {name: "unreadable", result: transport.Result{ExitCode: app.ProbeUnreadable}, err: true}, + {name: "empty", result: transport.Result{}, err: true}, + {name: "malformed", result: transport.Result{Stdout: "broken\n"}, err: true}, + {name: "negative", result: transport.Result{Stdout: "-1\n"}, err: true}, + {name: "max-value", result: transport.Result{Stdout: strconv.Itoa(maxInt)}, err: true}, + {name: "overflowing", result: transport.Result{Stdout: strconv.FormatUint(uint64(maxInt)+1, 10)}, err: true}, + } +} + +func TestApplicationLockEpochMatrix(t *testing.T) { + for _, test := range epochAcquisitionCases() { + t.Run(test.name, func(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if command == epochProbeCmd("/var/lib/ob/sample/epoch") { + return test.result, true + } + return transport.Result{}, false + }} + engine := lockEngine(t, fake) + got, err := engine.AcquireLock(context.Background(), "same-operation", false) + assertEpochAcquisition(t, fake, got, err, test) + }) + } +} + +func TestBackupLockEpochMatrix(t *testing.T) { + for _, test := range epochAcquisitionCases() { + t.Run(test.name, func(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if command == epochProbeCmd("/var/lib/ob/example/backup/locks/database.epoch") { + return test.result, true + } + return transport.Result{}, false + }} + engine := backupLockTestEngine(fake) + got, err := engine.AcquireBackupLock(context.Background(), "database", "same-operation", 0) + assertEpochAcquisition(t, fake, got, err, test) + }) + } +} + +func assertEpochAcquisition(t *testing.T, fake *transport.Fake, got int, err error, test epochAcquisitionCase) { + t.Helper() + if test.err { + if err == nil { + t.Fatal("invalid epoch was accepted") + } + if strings.Contains(strings.Join(fake.Commands, "\n"), "set -C") { + t.Fatalf("lock was created after epoch validation failed:\n%s", strings.Join(fake.Commands, "\n")) + } + return + } + if err != nil || got != test.want { + t.Fatalf("acquire epoch = %d, %v; want %d", got, err, test.want) + } +} + +func TestApplicationEpochPersistenceFailureReleasesLock(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if strings.Contains(command, "mktemp '/var/lib/ob/sample/epoch.tmp.XXXXXX'") { + return transport.Result{ExitCode: 23, Stderr: "rename interrupted"}, true + } + return transport.Result{}, false + }} + engine := lockEngine(t, fake) + if _, err := engine.AcquireLock(context.Background(), "operation", false); err == nil { + t.Fatal("acquisition succeeded after epoch persistence failed") + } + if engine.lockVal != "" || !strings.Contains(strings.Join(fake.Commands, "\n"), "then rm -f '/var/lib/ob/sample/lock'") { + t.Fatalf("failed acquisition left its lock published:\n%s", strings.Join(fake.Commands, "\n")) + } +} + +func TestBackupEpochPersistenceFailureReleasesLock(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if strings.Contains(command, "mktemp '/var/lib/ob/example/backup/locks/database.epoch.tmp.XXXXXX'") { + return transport.Result{ExitCode: 23, Stderr: "rename interrupted"}, true + } + return transport.Result{}, false + }} + engine := backupLockTestEngine(fake) + if _, err := engine.AcquireBackupLock(context.Background(), "database", "operation", 0); err == nil { + t.Fatal("backup acquisition succeeded after epoch persistence failed") + } + if engine.backupLockVals["database"] != "" || engine.backupFenceVals["database"] != "" || + !strings.Contains(strings.Join(fake.Commands, "\n"), "then rm -f '/var/lib/ob/example/backup/locks/database.lock'") { + t.Fatalf("failed backup acquisition left its lock or fence published:\n%s", strings.Join(fake.Commands, "\n")) + } +} + +func TestRealShellApplicationEpochLifecycle(t *testing.T) { + cfg := testConfig() + cfg.BasePath = t.TempDir() + engine := New(cfg, testProject(t), transport.NewLocal(), Options{Out: &bytes.Buffer{}, Sleep: noSleep}) + ctx := context.Background() + + first, err := engine.AcquireLock(ctx, "same-operation", false) + if err != nil || first != 1 { + t.Fatalf("first acquisition = %d, %v; want 1", first, err) + } + engine.ReleaseLock(ctx) + second, err := engine.AcquireLock(ctx, "same-operation", false) + if err != nil || second != 2 { + t.Fatalf("same-operation reacquisition = %d, %v; want 2", second, err) + } + engine.ReleaseLock(ctx) + + if err := os.WriteFile(engine.epochPath(), nil, 0o600); err != nil { + t.Fatal(err) + } + if _, err := engine.AcquireLock(ctx, "same-operation", false); err == nil { + t.Fatal("real shell accepted an empty application epoch") + } + if _, err := os.Stat(engine.lockPath()); !os.IsNotExist(err) { + t.Fatalf("application lock exists after epoch refusal: %v", err) + } +} + +func TestRealShellBackupEpochLifecycle(t *testing.T) { + cfg := testConfig() + cfg.BasePath = t.TempDir() + engine := New(cfg, testProject(t), transport.NewLocal(), Options{Out: &bytes.Buffer{}, Sleep: noSleep}) + ctx := context.Background() + if _, err := engine.AcquireLock(ctx, "deploy-operation", false); err != nil { + t.Fatal(err) + } + defer engine.ReleaseLock(ctx) + + first, err := engine.AcquireBackupLock(ctx, "database", "same-operation", 0) + if err != nil || first != 1 { + t.Fatalf("first backup acquisition = %d, %v; want 1", first, err) + } + engine.ReleaseBackupLock("database") + second, err := engine.AcquireBackupLock(ctx, "database", "same-operation", 0) + if err != nil || second != 2 { + t.Fatalf("same-operation backup reacquisition = %d, %v; want 2", second, err) + } + engine.ReleaseBackupLock("database") + + if err := os.WriteFile(engine.backupEpochPath("database"), []byte("broken\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := engine.AcquireBackupLock(ctx, "database", "same-operation", 0); err == nil { + t.Fatal("real shell accepted a malformed backup epoch") + } + if _, err := os.Stat(engine.backupLockPath("database")); !os.IsNotExist(err) { + t.Fatalf("backup lock exists after epoch refusal: %v", err) + } +} diff --git a/internal/engine/lock.go b/internal/engine/lock.go index ee4a1b9d..9ea8968a 100644 --- a/internal/engine/lock.go +++ b/internal/engine/lock.go @@ -53,12 +53,10 @@ func (e *Engine) AcquireLock(ctx context.Context, deployID string, force bool) ( // persisted a higher epoch between our attempts; fencing relies on a // strictly increasing epoch, so a value read once before the loop could // be reused and collide. - eres, err := e.T.Run(ctx, "cat "+q(e.epochPath())+" 2>/dev/null || echo 0") + epoch, err := e.nextEpoch(ctx, e.epochPath()) if err != nil { return 0, err } - prev, _ := strconv.Atoi(strings.TrimSpace(eres.Stdout)) - epoch := prev + 1 meta := lockMeta{ Owner: journal.DefaultOperator(), DeployID: deployID, Epoch: epoch, @@ -74,7 +72,7 @@ func (e *Engine) AcquireLock(ctx context.Context, deployID string, force bool) ( } if res.ExitCode == 0 { e.lockVal = string(b) - if res, err := e.T.Run(ctx, "echo "+strconv.Itoa(epoch)+" > "+q(e.epochPath())); err != nil || res.ExitCode != 0 { + if res, err := e.T.Run(ctx, atomicEpochWriteCmd(e.epochPath(), epoch)); err != nil || res.ExitCode != 0 { e.ReleaseLock(ctx) return 0, fmt.Errorf("persist epoch: %v %s", err, res.Stderr) } diff --git a/internal/engine/lock_test.go b/internal/engine/lock_test.go index b0c61c8c..2b89d325 100644 --- a/internal/engine/lock_test.go +++ b/internal/engine/lock_test.go @@ -39,7 +39,9 @@ func TestAcquireLockHappyPath(t *testing.T) { if !strings.Contains(seq, "set -C") || !strings.Contains(seq, "/var/lib/ob/sample/lock") { t.Fatalf("noclobber lock creation missing:\n%s", seq) } - if !strings.Contains(seq, "echo 7 > '/var/lib/ob/sample/epoch'") { + if !strings.Contains(seq, "mktemp '/var/lib/ob/sample/epoch.tmp.XXXXXX'") || + !strings.Contains(seq, "printf '%s\\n' 7") || + !strings.Contains(seq, `mv -f "$tmp" '/var/lib/ob/sample/epoch'`) { t.Fatalf("epoch not persisted:\n%s", seq) } } @@ -130,15 +132,22 @@ func TestAcquireLockSameDeployReclaims(t *testing.T) { if strings.Contains(cmd, "cat '/var/lib/ob/sample/lock'") { return transport.Result{Stdout: `{"owner":"dead@runner","deploy_id":"R9","epoch":6}`}, true } + if strings.Contains(cmd, "cat '/var/lib/ob/sample/epoch'") { + return transport.Result{Stdout: "6\n"}, true + } if strings.Contains(cmd, "date +%s") { return transport.Result{Stdout: "10\n"}, true // fresh — but same deploy } return transport.Result{}, false } e := lockEngine(t, f) - if _, err := e.AcquireLock(context.Background(), "R9", false); err != nil { + epoch, err := e.AcquireLock(context.Background(), "R9", false) + if err != nil { t.Fatalf("same-deploy lock must be reclaimable (resume after crash): %v", err) } + if epoch != 7 { + t.Fatalf("same-deploy reclaim epoch = %d, want 7 so stale fence R9 6 cannot match", epoch) + } } // After breaking a stale lock, the epoch must be read FRESH — a value read once @@ -180,7 +189,7 @@ func TestAcquireLockReReadsEpochAfterBreakingStaleLock(t *testing.T) { if epoch != 7 { t.Fatalf("epoch must derive from the FRESH read (want 7), got %d", epoch) } - if !strings.Contains(strings.Join(f.Commands, "\n"), "echo 7 > '/var/lib/ob/sample/epoch'") { + if !strings.Contains(strings.Join(f.Commands, "\n"), "printf '%s\\n' 7") { t.Fatalf("fresh epoch 7 not persisted:\n%s", strings.Join(f.Commands, "\n")) } } diff --git a/internal/transport/fake.go b/internal/transport/fake.go index 55457b21..510a66cf 100644 --- a/internal/transport/fake.go +++ b/internal/transport/fake.go @@ -115,6 +115,11 @@ func (f *Fake) evalLocked(cmd string) Result { return r.Result } } + // Engine epoch probes default to an absent file on a fresh fake host. Tests + // can override this default through Dynamic or Script. + if strings.HasPrefix(cmd, ": ob-epoch-probe;") { + return Result{ExitCode: 3} + } return Result{ExitCode: 0} } diff --git a/site/src/content/docs/explanation/safety-envelope.mdx b/site/src/content/docs/explanation/safety-envelope.mdx index dbd240a1..c7f17af7 100644 --- a/site/src/content/docs/explanation/safety-envelope.mdx +++ b/site/src/content/docs/explanation/safety-envelope.mdx @@ -23,6 +23,19 @@ interfere serialize rather than race. act: its epoch no longer matches. This is what makes "the CLI lost the connection" survivable rather than dangerous. +The epoch is durable authority, not a disposable counter. Onebox treats only a +genuinely absent epoch file as a first run. If the file exists but is unreadable, +empty, malformed, negative, or too large to increment, the operation stops +before publishing a new lock or fence. The next value is written to a sibling +temporary file and atomically renamed into place, so an interrupted write can +leave debris but cannot truncate the last valid epoch. + +Application operations share one epoch beneath the application's state +directory. Backup mutations add a per-service epoch beneath +`backup/locks/`. Both use the same read and persistence contract; retrying the +same operation identity therefore produces a different fence from the runner it +replaces. + **Append-only journals.** Every operation records ordered steps with deterministic identities. The record of a failed operation survives the operation that replaced it, which is what lets `ob resume` and `ob abort` choose only paths the journal diff --git a/site/src/content/docs/guides/roll-back.mdx b/site/src/content/docs/guides/roll-back.mdx index e5309f1c..7d53201e 100644 --- a/site/src/content/docs/guides/roll-back.mdx +++ b/site/src/content/docs/guides/roll-back.mdx @@ -128,6 +128,19 @@ ob abort --break-lock --output ndjson only. The backup lock has no override and clears on TTL expiry or when the same operation returns. +**An unreadable or invalid fencing epoch** — there is deliberately no override. +The application authority is `//epoch`; each backup service has a +second authority at `//backup/locks/.epoch`. Do not delete +either file or replace it with `0` or `1`: that can recreate a fence still held +by an interrupted runner. + +If the contents are intact, repair only the ownership or permissions that made +the file unreadable. Otherwise preserve the file and host evidence, stop or +isolate every runner that may still hold the old fence, and restore an epoch +known to be greater than every value previously issued. Onebox will refuse to +guess this value because a convenient reset would weaken the stale-runner +guarantee the epoch exists to provide. + ## Recovery onto another host is a different workflow Rolling deployment can avoid interruption while the host is healthy. It cannot