Harden the backup tree's atomic copy (closes #22)
check / check (push) Successful in 37s

The backup copy now fsyncs its temp file before the rename and the
directory after it, using the download writer's new fsyncPath helper.
Each backup run deletes .quak-backup-*.tmp files whose process is no
longer running, leaving those of a concurrent backup alone. The rename
sites and the README backup layout state that a symlink at the
destination is replaced and the new file takes the temp file's
permissions, and the README names the temp files. Adds tests for a
missing and an unwritable destination directory for downloadFile and
downloadThumbnail.

Model: opus-5-5
This commit is contained in:
2026-09-23 00:38:17 +00:00
parent d545dcd8b1
commit 7102e3443e
6 changed files with 223 additions and 16 deletions
+92 -1
View File
@@ -42,15 +42,44 @@ import {
rmSync,
writeFileSync,
} from "node:fs";
import { spawnSync } from "node:child_process";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { Library } from "../../src/library/index.js";
import type { ContentSource } from "../../src/library/content.js";
import type { CollectionsPage, FilesPage } from "../../src/client.js";
import type { Collection, EnteFile } from "../../src/model/types.js";
// `open` and `rename` are wrapped to record, in order, every fsync and rename,
// so a test can pin the sequence "fsync the temp file, rename, fsync the
// directory" that makes a copied original survive a power cut. `vi.hoisted`
// because `vi.mock` factories run before module-level constants exist.
const fsEvents = vi.hoisted(() => [] as string[]);
vi.mock("node:fs/promises", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs/promises")>();
return {
...actual,
open: async (
...args: Parameters<typeof actual.open>
): Promise<Awaited<ReturnType<typeof actual.open>>> => {
const handle = await actual.open(...args);
const realSync = handle.sync.bind(handle);
handle.sync = async (): Promise<void> => {
fsEvents.push(`sync:${String(args[0])}`);
await realSync();
};
return handle;
},
rename: async (from: string, to: string): Promise<void> => {
fsEvents.push(`rename:${to}`);
await actual.rename(from, to);
},
};
});
const USER_ID = 42;
// Decrypted-byte length each stub original writes, keyed by fileID.
@@ -530,4 +559,66 @@ describe("lib.backup", () => {
expect(readLedger(outDir).files["101"]!.attempts).toBe(1);
lib.close();
});
it("fsyncs a copied original before the rename and its directory after", async () => {
const lib = await openLibrary(stubSource());
const outDir = join(root, "backup");
const originals = join(outDir, "originals");
const dest = join(originals, "100.jpg");
fsEvents.length = 0;
await lib.backup({ downloadDirectory: outDir });
const at = fsEvents.indexOf(`rename:${dest}`);
expect(at).toBeGreaterThan(0);
expect(fsEvents[at - 1]).toMatch(
/^sync:.*\/\.quak-backup-100\.jpg-\d+-[0-9a-z]*\.tmp$/,
);
expect(fsEvents[at + 1]).toBe(`sync:${originals}`);
lib.close();
});
it("removes temp files left by a killed backup but not those of one still running", async () => {
const outDir = join(root, "backup");
const originals = join(outDir, "originals");
mkdirSync(originals, { recursive: true });
// A child that has already exited: its process ID is not running.
const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid;
const leftover = `.quak-backup-100.jpg-${exitedPID}-abc123.tmp`;
// This test's own process stands in for a backup running at the same
// time.
const inProgress = `.quak-backup-101.jpg-${process.pid}-def456.tmp`;
writeFileSync(join(originals, leftover), "partial");
writeFileSync(join(originals, inProgress), "partial");
const lib = await openLibrary(stubSource());
await lib.backup({ downloadDirectory: outDir });
const names = readdirSync(originals);
expect(names).not.toContain(leftover);
expect(names).toContain(inProgress);
lib.close();
});
it("removes leftover temp files in thumbnails/ but not those of a backup still running", async () => {
const outDir = join(root, "backup");
const thumbnails = join(outDir, "thumbnails");
mkdirSync(thumbnails, { recursive: true });
const exitedPID = spawnSync(process.execPath, ["-e", ""]).pid;
const leftover = `.quak-backup-100.jpg-${exitedPID}-abc123.tmp`;
const inProgress = `.quak-backup-101.jpg-${process.pid}-def456.tmp`;
writeFileSync(join(thumbnails, leftover), "partial");
writeFileSync(join(thumbnails, inProgress), "partial");
const lib = await openLibrary(stubSource());
await lib.backup({
downloadDirectory: outDir,
includeThumbnails: true,
});
const names = readdirSync(thumbnails);
expect(names).not.toContain(leftover);
expect(names).toContain(inProgress);
lib.close();
});
});
+46
View File
@@ -48,6 +48,7 @@
*/
import {
chmodSync,
existsSync,
mkdirSync,
readdirSync,
@@ -989,6 +990,51 @@ describe.each(entryPoints)(
expect(readFileSync(outPath)).toEqual(Buffer.from(existing));
expect(readdirSync(dir)).toEqual(["rename-fails.bin"]);
});
it("fails without creating anything when the destination directory does not exist", async () => {
const key = sodium.crypto_secretstream_xchacha20poly1305_keygen();
const { header, ciphertext } = encryptFileBody(
patternBytes(64, 33),
key,
);
const { api, file } = fixtureFor(key, header, ciphertext);
const dir = freshDir();
const outPath = join(dir, "missing", "never.bin");
await expect(download(api, file, outPath)).rejects.toMatchObject({
code: "ENOENT",
});
// The missing directory is not created on the caller's behalf.
expect(readdirSync(dir)).toEqual([]);
});
// Root ignores directory permissions, so this cannot fail as root
// (the Docker test image runs as root).
it.skipIf(process.getuid?.() === 0)(
"fails without creating anything when the destination directory is not writable",
async () => {
const key =
sodium.crypto_secretstream_xchacha20poly1305_keygen();
const { header, ciphertext } = encryptFileBody(
patternBytes(64, 34),
key,
);
const { api, file } = fixtureFor(key, header, ciphertext);
const dir = freshDir();
const outPath = join(dir, "never.bin");
chmodSync(dir, 0o500);
try {
await expect(
download(api, file, outPath),
).rejects.toMatchObject({ code: "EACCES" });
} finally {
chmodSync(dir, 0o700);
}
expect(readdirSync(dir)).toEqual([]);
},
);
},
);