runBackup must not crash on symlink failure, and must repair partial originals #8

New Issue

clawbot · 2026-08-09T03:44:34+02:00

clawbot commented

2026-08-09 03:44:34 +02:00

Problem

The README sells quak backup as resilient: "per-file errors are logged and the run
continues", and the Rationale section calls out the Go CLI crashing on a single failed
download as the reason quak exists. Two paths break that promise.

symlinkSync is unguarded. src/backup.ts:137-140 calls symlinkSync outside the
per-file try/catch. It throws — aborting the entire backup — on a name collision, on a
filesystem without symlink support, or on Windows without the required privilege. The
collection name and file title that make up the link path both come from server-controlled
decrypted metadata, so a single awkwardly named album kills the run.
A partially written original is never repaired. src/backup.ts:91 skips any existing
file with size > 0. A file left behind by a killed or interrupted run is therefore
treated as complete forever. (Atomic writes are handled in the download-layer issue; this
issue covers the pre-existing files that atomicity does not retroactively fix, and the
skip heuristic itself.)

Definition of done

A symlink failure for one file is caught, recorded in BackupResult's error list with the
collection, file ID and the underlying reason, and the run continues to the next file.
runBackup never throws for any per-file or per-collection condition; it reports through
its result value. The process exit code contract documented in the README is unchanged.
The skip heuristic is strengthened so an original is only skipped when it is actually
complete, not merely non-empty. Compare against the size the server reports where one is
available; where it is not, document in a code comment why the chosen check is sufficient.
Tests cover: a symlink that cannot be created does not abort the run and is reported as an
error; a second run over a complete backup still downloads nothing; a truncated
pre-existing original is re-downloaded rather than skipped; a run with a mix of successes
and failures reports both and exits non-zero.
make check green.
TODO.md updated in the same commit.

Depends on

The download atomic-write issue, which this builds on.

## Problem The README sells `quak backup` as resilient: "per-file errors are logged and the run continues", and the Rationale section calls out the Go CLI crashing on a single failed download as the reason quak exists. Two paths break that promise. 1. **`symlinkSync` is unguarded.** `src/backup.ts:137-140` calls `symlinkSync` outside the per-file `try`/`catch`. It throws — aborting the entire backup — on a name collision, on a filesystem without symlink support, or on Windows without the required privilege. The collection name and file title that make up the link path both come from server-controlled decrypted metadata, so a single awkwardly named album kills the run. 2. **A partially written original is never repaired.** `src/backup.ts:91` skips any existing file with `size > 0`. A file left behind by a killed or interrupted run is therefore treated as complete forever. (Atomic writes are handled in the download-layer issue; this issue covers the pre-existing files that atomicity does not retroactively fix, and the skip heuristic itself.) ## Definition of done 1. A symlink failure for one file is caught, recorded in `BackupResult`'s error list with the collection, file ID and the underlying reason, and the run continues to the next file. 2. `runBackup` never throws for any per-file or per-collection condition; it reports through its result value. The process exit code contract documented in the README is unchanged. 3. The skip heuristic is strengthened so an original is only skipped when it is actually complete, not merely non-empty. Compare against the size the server reports where one is available; where it is not, document in a code comment why the chosen check is sufficient. 4. Tests cover: a symlink that cannot be created does not abort the run and is reported as an error; a second run over a complete backup still downloads nothing; a truncated pre-existing original is re-downloaded rather than skipped; a run with a mix of successes and failures reports both and exits non-zero. 5. `make check` green. 6. `TODO.md` updated in the same commit. ## Depends on The download atomic-write issue, which this builds on.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:44:34 +02:00

clawbot self-assigned this 2026-08-09 03:44:34 +02:00

clawbot referenced this issue

2026-08-09 03:47:27 +02:00

Tag v1.0.0 #15

clawbot referenced this issue

2026-08-09 04:12:18 +02:00

Verify secretstream TAG_FINAL and write downloads atomically (closes #1) #20

clawbot referenced this issue

2026-08-09 04:26:52 +02:00

Verify secretstream TAG_FINAL and write downloads atomically (closes #1) #20

clawbot referenced this issue