fix(downloads): verify durable archive identity at resume open

This commit is contained in:
4gray committed 2026-09-08 01:47:16 +02:00
1 parent 414b15d5c8
commit 76632d2003
7 files changed
+73 -7

No files matched your search

@@ -8,7 +8,9 @@ export interface ArchiveFileIdentity {
/** Open without truncation first: a replaced partial must not damage its target. */
export async function openCatchupOutput(
partialPath: string
partialPath: string,
expectedIdentity?: ArchiveFileIdentity,
beforeTruncate?: () => Promise<void>
): Promise<{ stream: WriteStream; identity: ArchiveFileIdentity }> {
const before = await lstat(partialPath).catch(
(error: NodeJS.ErrnoException) => {
@@ -34,10 +36,16 @@ export async function openCatchupOutput(
!current.isFile() ||
current.nlink !== 1 ||
(before &&
(current.dev !== before.dev || current.ino !== before.ino))
(current.dev !== before.dev || current.ino !== before.ino)) ||
(before &&
expectedIdentity &&
(current.dev !== expectedIdentity.dev ||
current.ino !== expectedIdentity.ino))
) {
throw new Error('Archive partial changed before opening');
}
// Keep durable ownership evidence until the actual descriptor passes.
await beforeTruncate?.();
await handle.truncate(0);
// All writes use this verified descriptor; never reopen by pathname.
return {
@@ -11,10 +11,13 @@ import { join } from 'node:path';
import { getDatabase } from '../../database/connection';
import { transferCatchupToPartialFile } from './download-catchup-transfer';
import { enqueueDownload } from './download-runtime';
import { readArchiveFinalizations } from './download-catchup-journal';
import type { DownloadTask } from './download-task';
jest.mock('./download-catchup-journal', () => ({
recordArchiveFinalization: jest.fn().mockResolvedValue(undefined),
clearArchiveFinalization: jest.fn().mockResolvedValue(undefined),
readArchiveFinalizations: jest.fn().mockResolvedValue(new Map()),
}));
jest.mock('../../database/connection', () => ({ getDatabase: jest.fn() }));
jest.mock('./download-catchup-transfer', () => ({
@@ -61,6 +64,12 @@ it.each(['failed', 'canceled'])(
active.catchupPartialIdentity = await lstat(
reservation.partialPath
);
expect(active.catchupExpectedPartialIdentity).toEqual(
expect.objectContaining({
dev: active.catchupPartialIdentity.dev,
ino: active.catchupPartialIdentity.ino,
})
);
await rename(
reservation.partialPath,
join(directory, 'original')
@@ -74,6 +83,23 @@ it.each(['failed', 'canceled'])(
.spyOn(console, 'error')
.mockImplementation(() => undefined);
try {
task.filePath = join(directory, 'show.ts');
await writeFile(task.filePath + '.part', 'retained archive');
const partial = await lstat(task.filePath + '.part');
jest.mocked(readArchiveFinalizations).mockResolvedValueOnce(
new Map([
[
task.id,
{
version: 1,
filePath: task.filePath,
size: partial.size,
partialIdentity: partial,
finalIdentity: partial,
},
],
])
);
enqueueDownload(task);
await completed;
// Let the queue's finally block retire this task before the next case.
@@ -224,7 +224,7 @@ describe('TS archive transfer', () => {
});
});
it.each(['symlink', 'hardlink'])(
it.each(['symlink', 'hardlink', 'regular file'])(
'refuses a retained archive partial replaced by a %s',
async (kind) => {
const directory = await mkdtemp(join(tmpdir(), 'archive-replaced-'));
@@ -234,7 +234,9 @@ it.each(['symlink', 'hardlink'])(
await writeFile(target, 'keep this file intact');
await (kind === 'symlink'
? symlink(target, path + '.part')
: link(target, path + '.part'));
: kind === 'hardlink'
? link(target, path + '.part')
: writeFile(path + '.part', 'keep this file intact'));
jest.mocked(requestWithValidatedRedirects).mockResolvedValue({
status: 200,
headers: { 'content-type': 'video/mp2t' },
@@ -242,11 +244,14 @@ it.each(['symlink', 'hardlink'])(
} as never);
const task: DownloadTask = {
id: 3,
catchupExpectedPartialIdentity: await stat(target),
url: 'https://host/archive.ts',
fileName: 'archive.ts',
directory,
catchup: metadata,
};
const journalClears = jest.mocked(clearArchiveFinalization).mock
.calls.length;
await expect(
transferCatchupToPartialFile({} as DownloadsDatabase, task, {
path,
@@ -254,6 +259,12 @@ it.each(['symlink', 'hardlink'])(
filename: 'archive.ts',
})
).rejects.toThrow();
expect(
jest.mocked(clearArchiveFinalization).mock.calls.length
).toBe(journalClears);
expect(await readFile(path + '.part', 'utf8')).toBe(
'keep this file intact'
);
expect(await readFile(target, 'utf8')).toBe(
'keep this file intact'
);
@@ -71,7 +71,6 @@ export async function transferCatchupToPartialFile(
let output: Awaited<ReturnType<typeof openCatchupOutput>> | undefined;
let pendingProgress = Promise.resolve();
try {
await clearArchiveFinalization(db, task.id);
const response = await requestWithValidatedRedirects<Readable>(
task.url,
{
@@ -100,7 +99,11 @@ export async function transferCatchupToPartialFile(
Number.isSafeInteger(length) && length > 0 ? length : null;
// Restart safely before measuring space: the retained file is
// reclaimable only after its verified descriptor has been truncated.
output = await openCatchupOutput(reservation.partialPath);
output = await openCatchupOutput(
reservation.partialPath,
task.catchupExpectedPartialIdentity,
() => clearArchiveFinalization(db, task.id)
);
output.stream.on('error', () => undefined);
task.catchupPartialIdentity = output.identity;
const byteLimit = await getArchiveByteLimit(
@@ -1,3 +1,7 @@
import {
clearArchiveFinalization,
readArchiveFinalizations,
} from './download-catchup-journal';
import { cleanupSelectedCatchupPartial } from './download-catchup-cleanup';
import {
persistCancellation,
@@ -196,6 +200,17 @@ async function startDownload(task: DownloadTask): Promise<void> {
return;
}
if (task.catchup) {
if (task.filePath) {
const proof = (
await readArchiveFinalizations(db, [task.id])
).get(task.id);
task.catchupExpectedPartialIdentity = proof?.partialIdentity;
} else {
// A fresh reservation must not inherit a previous attempt's proof.
await clearArchiveFinalization(db, task.id);
}
}
reservation = await reserveTarget(task);
task.fileName = reservation.filename;
task.filePath = reservation.path;
@@ -16,6 +16,7 @@ export interface CompletedPartialProgress extends TransferProgress {
export interface DownloadTask {
catchup?: CatchupDownloadMetadata;
catchupPartialIdentity?: ArchiveFileIdentity;
catchupExpectedPartialIdentity?: ArchiveFileIdentity;
catchupFinalized?: {
filePath: string;
identity: ArchiveFileIdentity;
+3 -1
View File
@@ -67,7 +67,9 @@ journal also makes startup partial cleanup identity-aware. A journaled partial
whose identity changed is preserved and detached from the failed row; Retry
reserves a fresh path instead of adopting or truncating it. The journal remains with
completed archives until they are removed. An explicitly restarted transfer
clears the previous attempt's proof before network requests or partial-file writes. Process-local proof allows immediate
carries the journaled source identity through any destination renaming and checks
it against the opened descriptor. Only then does it clear the previous proof,
before truncation or writes; a rejected replacement keeps its journal for retry. Process-local proof allows immediate
recovery after a transient completion DB error without waiting for a restart.
Fallback copying observes pause/cancel between bounded 64 KiB reads/writes and
before publication completes. Interruption removes only the owned copy and