From 5208957ce91ca34e8947e84afbf4e0f81a1c33a5 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 10 Oct 2026 19:13:27 -0400 Subject: [PATCH] fix(agent): nothing the VPS sends deletes or alters a pulled backup Anchoring the rotation on the newest directory name let the VPS steer it: one future-dated directory (VPS clock off, or a VPS gone rogue) pushed the window forward and deleted every real backup, with exit 0 and an ok report. Two more inputs from the outbox could do harm: rsync -rt rewrote an already pulled .age whenever the VPS served another version, and an impossible dated name (2026-13-45) failed `date -d` under set -e, stopping every later pass for good. - latest is the newest valid dated directory not after today; the rotation keeps it and the 29 days before it; - a name is dated only if `date` gives it back unchanged (busybox turns 2026-02-30 into 2026-03-02); an invalid or future name is logged, never counted, never removed, and the pass goes on; - --ignore-existing: a pulled file is never rewritten. The VPS writes a .tmp and renames it, and the filter drops the .tmp. Refs maximus/la-compagnie-maximus#220 Co-Authored-By: Claude Opus 5.5 --- __tests__/agent-pg-backup-pull.test.js | 55 ++++++++++++++++++++-- agent/README.md | 19 ++++++-- agent/pg-backup-pull/run-pg-backup-pull.sh | 41 +++++++++++++--- 3 files changed, 101 insertions(+), 14 deletions(-) diff --git a/__tests__/agent-pg-backup-pull.test.js b/__tests__/agent-pg-backup-pull.test.js index 1f00adb..b0fada2 100644 --- a/__tests__/agent-pg-backup-pull.test.js +++ b/__tests__/agent-pg-backup-pull.test.js @@ -129,6 +129,7 @@ describe("pull", () => { expect(argv.indexOf("--include=*.age")).toBeLessThan(argv.indexOf("--exclude=*")); expect(argv).not.toContain("--delete"); expect(argv).toContain("--prune-empty-dirs"); + expect(argv).toContain("--ignore-existing"); expect(argv).toContain("pgbackup@100.86.30.21:"); const ssh = argv.find((a) => a.startsWith("ssh ")); expect(ssh).toContain("-p 2222"); @@ -197,7 +198,7 @@ describe("pull", () => { // dated directory holding no .age file is not created. Skipped // where rsync is absent (the CI image); the argv order test above runs there. test.skipIf(!["/usr/bin/rsync", "/bin/rsync"].some((f) => fs.existsSync(f)))( - "with the real rsync, only .age files cross, inside their dated directory, never an empty one", + "with the real rsync, only .age files cross, inside their dated directory, never an empty one, never over a pulled file", () => { fs.rmSync(path.join(bin, "rsync")); const outbox = path.join(tmp, "outbox"); @@ -207,10 +208,17 @@ describe("pull", () => { fs.writeFileSync(path.join(outbox, "stray.sql"), "clear"); fs.mkdirSync(path.join(outbox, "2026-10-12")); fs.writeFileSync(path.join(outbox, "2026-10-12", "maximus_api.dump"), "clear"); - const { status, output, dest } = run({ extraEnv: { FAKE_OUTBOX: outbox } }); + fs.mkdirSync(path.join(outbox, "2026-10-10")); + fs.writeFileSync(path.join(outbox, "2026-10-10", "globals.sql.age"), "garbage"); + const dest = path.join(tmp, "dest"); + fs.mkdirSync(path.join(dest, "2026-10-10"), { recursive: true }); + fs.writeFileSync(path.join(dest, "2026-10-10", "globals.sql.age"), "good-backup"); + const { status, output } = run({ extraEnv: { FAKE_OUTBOX: outbox } }); expect(status).toBe(0); - expect(fs.readdirSync(dest)).toEqual([TODAY]); + expect(fs.readdirSync(dest).sort()).toEqual(["2026-10-10", TODAY]); expect(fs.readdirSync(path.join(dest, TODAY))).toEqual(["maximus_api.dump.age"]); + // A file already pulled is never rewritten by what the VPS serves later. + expect(fs.readFileSync(path.join(dest, "2026-10-10", "globals.sql.age"), "utf8")).toBe("good-backup"); expect(output).toContain(`latest=${TODAY} files=1`); }, ); @@ -308,6 +316,47 @@ describe("rotation", () => { expect(JSON.parse(read("curl.body"))).toMatchObject({ ok: true, files: 1 }); }); + // Nothing the VPS sends may delete a copy already here. A future-dated + // directory (VPS clock off, or a VPS gone rogue) would, as the newest name, + // push the rotation window forward over every real backup. + test("a future-dated directory from the VPS removes nothing and is not counted", () => { + const dest = path.join(tmp, "dest"); + for (const d of ["2026-09-20", "2026-10-01", "2026-10-10"]) { + fs.mkdirSync(path.join(dest, d), { recursive: true }); + fs.writeFileSync(path.join(dest, d, "a.age"), "x"); + } + fs.writeFileSync( + path.join(bin, "rsync"), + `#!/bin/sh\nfor last; do :; done\nmkdir -p "$last/2099-01-01"\necho z > "$last/2099-01-01/z.age"\n`, + ); + const { status, output } = run({ envFile: writeEnv() }); + expect(status).toBe(0); + expect(fs.readdirSync(dest).sort()).toEqual(["2026-09-20", "2026-10-01", "2026-10-10", "2099-01-01"]); + expect(output).toContain(`warning: ignoring 2099-01-01, after ${TODAY}`); + expect(output).toContain("latest=2026-10-10 files=1"); + expect(output).not.toContain("rotation: removed"); + }); + + // A name of the dated shape that is no date (2026-13-45; busybox would turn + // 2026-02-30 into 2026-03-02) is ignored: the pass goes on, the directory + // is neither counted nor removed. + test("an impossible dated name is ignored, never stops the pass, never removed", () => { + const dest = path.join(tmp, "dest"); + for (const d of ["2026-13-45", "2026-02-30", "2026-01-01"]) { + fs.mkdirSync(path.join(dest, d), { recursive: true }); + fs.writeFileSync(path.join(dest, d, "z.age"), "z"); + } + const { status, output } = run({ envFile: writeEnv() }); + expect(status).toBe(0); + expect(output).toContain("warning: ignoring 2026-13-45, not a valid date"); + expect(output).toContain("warning: ignoring 2026-02-30, not a valid date"); + expect(output).toContain(`latest=${TODAY} files=2`); + expect(output).toContain("rotation: removed 2026-01-01"); + expect(fs.existsSync(path.join(dest, "2026-13-45"))).toBe(true); + expect(fs.existsSync(path.join(dest, "2026-02-30"))).toBe(true); + expect(JSON.parse(read("curl.body"))).toMatchObject({ ok: true, files: 2 }); + }); + // The failure comes from a fake rm, not from file modes: the CI runs as // root, which ignores a read-only directory. test("a directory that cannot be removed fails loudly and reports rotation_failed", () => { diff --git a/agent/README.md b/agent/README.md index 3fcc4b5..e1c9b9d 100644 --- a/agent/README.md +++ b/agent/README.md @@ -436,10 +436,21 @@ producing, or the pass ran before its 03:00 UTC run. A dated directory with no directory's count (partial files in `.rsync-partial/` not counted); freshness is judged on the VPS side by `GET /backups`. -**Rotation is anchored on the newest backup held here, not on today's date.** -It keeps that directory and the 29 days before it. If the VPS stops producing -(dead cron, failing `pg_dump`) or this machine's clock jumps ahead, the last -off-site copy stays put instead of aging out after 30 days. +**Nothing the VPS sends may delete or alter a copy already here.** + +- **No overwrite.** A file already pulled is never rewritten + (`--ignore-existing`): the VPS writes each dump to a `.tmp` and renames it, + so what lands is whole. +- **Directory names are checked.** A name is dated only if `date` gives it + back unchanged: `2026-13-45` or `2026-02-30` is logged and ignored, never + counted, never removed, and the pass goes on. A dated name after today + (VPS clock off, or a VPS gone rogue) is logged and ignored the same way. +- **Rotation is anchored on the newest valid backup not after today.** It + keeps that directory and the 29 days before it. Not on today alone: if the + VPS stops producing (dead cron, failing `pg_dump`) or this machine's clock + jumps ahead, the last off-site copy stays put instead of aging out after + 30 days. Not on the newest name alone: a future-dated directory would push + the window over every real backup. The script sticks to what busybox offers (no `find -printf`, no `date -d "... days"`): the CI runs its tests in `node:22-alpine`. diff --git a/agent/pg-backup-pull/run-pg-backup-pull.sh b/agent/pg-backup-pull/run-pg-backup-pull.sh index 8e06838..5b51abf 100755 --- a/agent/pg-backup-pull/run-pg-backup-pull.sh +++ b/agent/pg-backup-pull/run-pg-backup-pull.sh @@ -114,7 +114,10 @@ report() { # a clear dump here. No --delete: the VPS keeps 7 days, this machine keeps 30. # --prune-empty-dirs: a dated directory with no .age file (encryption failed on # the VPS) is not created here, so it cannot pass for today's backup. -if ! timeout "$RSYNC_TIMEOUT_SECONDS" rsync -rt --prune-empty-dirs --partial-dir=.rsync-partial \ +# --ignore-existing: a file already pulled is never rewritten, whatever the VPS +# serves under its name later. The VPS writes each dump to a .tmp and renames +# it (the .tmp never passes the filter), so what lands here is already whole. +if ! timeout "$RSYNC_TIMEOUT_SECONDS" rsync -rt --prune-empty-dirs --ignore-existing --partial-dir=.rsync-partial \ --include='*/' --include='*.age' --exclude='*' \ -e "ssh -p $SSH_PORT -i $SSH_KEY -o IdentitiesOnly=yes -o UserKnownHostsFile=$KNOWN_HOSTS -o StrictHostKeyChecking=yes -o BatchMode=yes -o ConnectTimeout=20 -o ServerAliveInterval=15 -o ServerAliveCountMax=4" \ "$SOURCE" "$DEST/"; then @@ -124,22 +127,43 @@ if ! timeout "$RSYNC_TIMEOUT_SECONDS" rsync -rt --prune-empty-dirs --partial-dir fi chmod -R go-rwx "$DEST" +# Nothing the VPS sends may delete or alter a copy already here: directory +# names come from the outbox, so they are checked before they steer anything. +# # Plain shell over the glob, no `find -printf` nor `date -d "... days"`: both # are GNU-only, and the CI runs this script under busybox (node:22-alpine). +# +# A name is dated only if `date` gives it back unchanged: GNU date refuses +# 2026-13-45, busybox turns 2026-02-30 into 2026-03-02. Anything else is never +# picked as latest, never removed, and must not stop the pass. is_dated() { case "$1" in - [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]) return 0 ;; + [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]) ;; *) return 1 ;; esac + [ "$(date -u -d "$1" +%F 2>/dev/null)" = "$1" ] } # YYYY-MM-DD as a YYYYMMDD integer: `[ a \< b ]` is not POSIX, and /bin/sh is dash. day_number() { echo "$1" | tr -d -; } +# latest is the newest dated directory not after today: a future name (VPS +# clock off, or a VPS gone rogue) is kept but steers neither the report nor the +# rotation below. latest="" for dir in "$DEST"/*/; do [ -d "$dir" ] || continue name=$(basename "$dir") - is_dated "$name" || continue + # Only names of the dated shape are logged: they hold digits and dashes, so a + # name from the VPS cannot forge a journal line. + case "$name" in [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]) ;; *) continue ;; esac + if ! is_dated "$name"; then + log "warning: ignoring $name, not a valid date" + continue + fi + if [ "$(day_number "$name")" -gt "$(day_number "$TODAY")" ]; then + log "warning: ignoring $name, after $TODAY" + continue + fi if [ -z "$latest" ] || [ "$(day_number "$name")" -gt "$(day_number "$latest")" ]; then latest="$name" fi @@ -161,10 +185,13 @@ if [ -n "$latest" ] && [ "$files" -eq 0 ]; then fi # --- Rotation (fails loudly) -------------------------------------------------- -# Anchored on the newest backup held here, never on today's date: when the VPS -# stops producing (dead cron, failing pg_dump), or this machine's clock jumps -# ahead, a TODAY-based cutoff would delete the last off-site copy. This keeps -# the 30 days before the newest backup, which itself is never removed. +# Anchored on latest, the newest backup held here that is not after today. Not +# on today alone: when the VPS stops producing (dead cron, failing pg_dump), or +# this machine's clock jumps ahead, a today-based cutoff would delete the last +# off-site copy. Not on the newest name alone: a future-dated directory from +# the VPS would push the window forward and delete every real backup. This +# keeps latest and the 29 days before it; a future or invalid name is newer +# than the cutoff or not dated, so it is never removed. # Epoch arithmetic: `date -d YYYY-MM-DD` and `date -d @N` are read by GNU date # and busybox alike. rotation_ok=true