Skip to content

fix: stale repo locks, orphaned sandboxes, leaked username, and a nonsense recovery time - #8

Merged
dabelle merged 2 commits into
mainfrom
fix/lock-orphans-and-sanity
Sep 19, 2026
Merged

dabelle merged 2 commits into
mainfrom
fix/lock-orphans-and-sanity

Conversation

@dabelle

@dabelle dabelle commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

The last four defects from the verification sweep. Three of them share a shape: the agent leaving a mess somewhere it does not own.

Stale repository locks

exec.CommandContext SIGKILLs on cancellation, so Ctrl-C during a restore killed restic before it released its lock. The user's next restic forget or prune then failed with repository is already locked by PID …. A tool that sells itself on being read-only against your repository should not break your retention job.

Fix is cmd.Cancel sending an interrupt plus a WaitDelay grace period, so restic unwinds the way it would under a real Ctrl-C. Measured by interrupting inside the restore window, five trials each:

before: LOCKED LOCKED LOCKED clean LOCKED
after:  clean  clean  clean  clean clean

(The one clean in the before row is the interrupt landing outside the lock window, not the bug being absent.)

Orphaned sandboxes

Destroy is deferred and survives panics, failures and signals, but nothing survives SIGKILL, an OOM kill or a power cut. Those left a complete copy of the user's restored data on disk with no owner, and no later run ever reclaimed it. Disk leak and privacy problem on the same facts.

New now sweeps abandoned sandboxes first. Deliberately conservative, since it deletes directories: only the restorable- prefix this package creates, only directly under the configured base dir, and only past an age no legitimate restore reaches, so a concurrent agent's live sandbox is never a candidate. Failures are ignored, because housekeeping must never be why a verification run cannot start. Tests cover all four cases including the concurrent-run and foreign-directory ones.

The account name in the Docker socket path

Docker client errors embed the daemon endpoint, which on a desktop install spells out the operator's username, and it travelled to the control plane. Matching on the docker.sock suffix keeps the scrub from touching any other URL.

A restore longer than the run containing it

The schema refined finished_at >= started_at but had no equivalent for the restore being a phase inside the run. A one-second run claiming a ten-minute restore was accepted, and the dashboard rendered it verbatim as 1s · restore 10m · verify 0s under "verified recovery time". That number is the product's headline claim, so it should be the hardest field to put nonsense into.

Verification

Full Go suite, -race across all 7 packages, golangci-lint clean. Web lint, tsc --noEmit, 52 tests (up from 48), build. All three agent e2e suites pass with the reaper now running on every sandbox creation.

thetechnologist1911 added 2 commits September 19, 2026 09:46
Three defects that all share a shape: the agent leaving a mess somewhere it
does not own.

**Stale repository locks.** exec.CommandContext SIGKILLs on cancellation, so
Ctrl-C during a restore killed restic before it could release its lock. The
user's next `restic forget` or `prune` then failed with "repository is already
locked by PID ...". A tool that sells itself on being read-only against your
repository should not break your retention job. Interrupt instead, with a
WaitDelay grace period, so restic unwinds the way it would under a real Ctrl-C.

Measured, interrupting inside the restore window, five trials each:

  before: LOCKED LOCKED LOCKED clean LOCKED
  after:  clean  clean  clean  clean clean

**Orphaned sandboxes.** Destroy is deferred and survives panics, failures and
signals, but nothing survives SIGKILL, an OOM kill or a power cut. Those left a
complete copy of the user's restored data on disk with no owner, and no later
run ever reclaimed it: a disk leak and a privacy problem on the same facts.
New now sweeps abandoned sandboxes first. Deliberately conservative, since it
deletes directories: only the prefix this package creates, only directly under
the configured base dir, and only past an age no legitimate restore reaches,
so a concurrent agent's live sandbox is never a candidate. Failures are
ignored, because housekeeping must never stop a verification run.

**The account name in the Docker socket path.** Docker client errors embed the
daemon endpoint, which on a desktop install spells out the operator's username,
and that travelled to the control plane. Scrub now masks it. Matching on the
docker.sock suffix keeps it from touching any other URL.
The schema refined finished_at >= started_at but had no equivalent for the
restore being a phase inside the run. A one-second run claiming a ten-minute
restore was accepted, and the dashboard rendered it verbatim as "1s · restore
10m · verify 0s" under the heading "verified recovery time".

That number is the product's headline claim, so it should be the hardest field
to put nonsense into, not the easiest. One second of slack covers clock
granularity between the two measurements; anything beyond that is a bug in an
agent or a forged submission, and neither should reach the dashboard.
@dabelle
dabelle merged commit 1eed86c into main Sep 19, 2026
4 checks passed
@dabelle
dabelle deleted the fix/lock-orphans-and-sanity branch September 19, 2026 13:51
dabelle pushed a commit that referenced this pull request Sep 19, 2026
fix: stale repo locks, orphaned sandboxes, leaked username, and a nonsense recovery time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant