Skip to content

Fencing epochs fail open on unreadable or malformed state #55

Description

@vishr

Problem

Both fencing authorities fail open when their durable epoch file is unreadable, empty, truncated, or malformed.

The application lock in internal/engine/lock.go currently does this:

eres, err := e.T.Run(ctx, "cat "+q(e.epochPath())+" 2>/dev/null || echo 0")
prev, _ := strconv.Atoi(strings.TrimSpace(eres.Stdout))
epoch := prev + 1

The backup lock repeats the same behavior in internal/engine/backup_lock.go:

result, err := e.T.Run(ctx, "cat "+q(e.backupEpochPath(service))+" 2>/dev/null || echo 0")
previous, _ := strconv.Atoi(strings.TrimSpace(result.Stdout))
return previous + 1, nil

Both paths therefore turn every read or parse failure into epoch 1. Both also persist epochs with direct echo N > file writes (lock.go and writeBackupFence), so interruption after truncation can create the malformed state that the next acquisition accepts as a first run.

Why this is high priority

Fencing is a published safety guarantee, not an internal optimization. The safety guide says:

A stale process that wakes up after losing its lock cannot act: its epoch no longer matches.

The CLI reference also states that ob resume fences the runner that stopped before continuing.

The fence value is <operation-id> <epoch>. Reusing an epoch is most dangerous when the same operation identity is reclaimed—the normal resume/retry case—because the new runner can recreate the stale runner's exact fence value. That stale runner's next guarded mutation can then pass instead of returning ErrFenced.

The application authority is shared by deploy, bootstrap, resume/abort, destroy, jobs, service/proxy apply, secrets, exec, and backup lifecycle operations. The backup authority separately guards repository, credential, scheduling, and restore mutations for a service.

Required behavior

  • Treat a genuinely absent epoch file as the initial value 0.
  • Refuse an epoch file that exists but cannot be read, is empty, is non-numeric, is negative, or overflows.
  • Persist the next epoch atomically so interruption cannot replace a valid epoch with an empty or partial file.
  • Apply the same helper/contract to application and backup epochs so they cannot drift again.
  • Do not acquire or publish a new lock/fence after epoch validation or persistence fails.

Verification

  • Missing epoch succeeds with epoch 1.
  • Valid epoch N succeeds with N+1.
  • Unreadable, empty, malformed, negative, and overflowing epochs refuse before a new fence is usable.
  • A simulated interrupted write leaves the previous valid epoch readable.
  • Reclaiming the same operation ID always produces a fence value different from the stale runner's.
  • Application and backup lock tests cover the same matrix.

Done when

No reachable error or interrupted-write path can reset either fencing authority to an epoch previously issued, and the documented stale-runner guarantee is true for both application and backup mutations.


Originally split from #50 as item 3; refreshed against current main after the backup subsystem replaced protection_lock.go.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: highHigh-impact work to address before routine backlog items

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions