fix(downloads): distinguish reused archive inodes by creation time

This commit is contained in:
4gray committed 2026-09-08 05:00:39 +02:00
1 parent b2216efc61
commit 86f7759719
14 files changed
+133 -58

No files matched your search

+2
View File
@@ -1043,6 +1043,8 @@ The same journal stores transfer-phase descriptor identity before truncation;
Resume checks it at open, and rejected replacements are preserved and detached
so Retry can reserve a fresh path. A synchronous completion-commit boundary
rejects late pause/cancel commands before awaited cleanup and persistence.
Archive ownership includes device, inode and positive creation time to reject
reused inodes after unlink; old proofs without creation time remain untrusted.
Private cleanup captures are journaled before relocation, keeping failed
Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active
failures, promotion and startup share that cleanup; Remove waits for active
+2
View File
@@ -1854,6 +1854,8 @@ The same journal stores transfer-phase descriptor identity before truncation;
Resume checks it at open, and rejected replacements are preserved and detached
so Retry can reserve a fresh path. A synchronous completion-commit boundary
rejects late pause/cancel commands before awaited cleanup and persistence.
Archive ownership includes device, inode and positive creation time to reject
reused inodes after unlink; old proofs without creation time remain untrusted.
Private cleanup captures are journaled before relocation, keeping failed
Remove/Clear/cancel cleanup retryable across restarts without hardlinks. Active
failures, promotion and startup share that cleanup; Remove waits for active
@@ -1,6 +1,9 @@
import { lstatSync, rmdirSync, unlinkSync } from 'node:fs';
import { dirname } from 'node:path';
import type { ArchiveFileIdentity } from './download-catchup-output';
import {
sameArchiveFileIdentity,
type ArchiveFileIdentity,
} from './download-catchup-output';
import type { ArchiveDownloadProof } from './download-catchup-journal';
/** Retry a journaled private capture without ever deleting a replacement. */
@@ -20,11 +23,7 @@ function cleanupCapture(
if (!path) return;
try {
const file = lstatSync(path);
if (
file.isFile() &&
file.dev === identity.dev &&
file.ino === identity.ino
) {
if (file.isFile() && sameArchiveFileIdentity(file, identity)) {
unlinkSync(path);
} else {
console.warn(
@@ -1,6 +1,9 @@
import { link, lstat, mkdtemp, rename, rmdir, unlink } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import type { ArchiveFileIdentity } from './download-catchup-output';
import {
sameArchiveFileIdentity,
type ArchiveFileIdentity,
} from './download-catchup-output';
/** Capture the directory entry atomically before inspecting or removing it. */
export async function cleanupCatchupFile(
@@ -14,11 +17,7 @@ export async function cleanupCatchupFile(
try {
await rename(path, captured);
const stats = await lstat(captured);
if (
stats.isFile() &&
stats.dev === identity.dev &&
stats.ino === identity.ino
) {
if (stats.isFile() && sameArchiveFileIdentity(stats, identity)) {
await unlink(captured);
} else {
// A replacement was captured. Restore without clobbering any newer
@@ -2,22 +2,23 @@ import type { DownloadTask, CompletedPartialProgress } from './download-task';
import { cleanupCatchupFile } from './download-catchup-cleanup';
import { constants, type Stats } from 'node:fs';
import { link, lstat, open } from 'node:fs/promises';
import type { ArchiveFileIdentity } from './download-catchup-output';
import {
archiveFileIdentity,
sameArchiveFileIdentity,
type ArchiveFileIdentity,
} from './download-catchup-output';
import type { ReservedPartialDownloadFile } from './download-file-path';
function sameFile(
stats: ArchiveFileIdentity,
identity: ArchiveFileIdentity
): boolean {
return stats.dev === identity.dev && stats.ino === identity.ino;
}
function verify(
stats: Stats,
identity: ArchiveFileIdentity,
size: number
): void {
if (!stats.isFile() || !sameFile(stats, identity) || stats.size !== size) {
if (
!stats.isFile() ||
!sameArchiveFileIdentity(stats, identity) ||
stats.size !== size
) {
throw new Error('Archive partial changed before promotion');
}
}
@@ -130,7 +131,7 @@ export async function finalizeCatchupPartial(
? cleanup.partial()
: cleanupCatchupFile(reservation.partialPath, identity)
).catch(() => undefined);
return { size, identity: { dev: created.dev, ino: created.ino } };
return { size, identity: archiveFileIdentity(created) };
} catch (error) {
if (created) {
await (
@@ -154,8 +155,7 @@ export async function recoverCatchupCompletion(
try {
const file = await lstat(proof.filePath);
return file.isFile() &&
file.dev === proof.identity.dev &&
file.ino === proof.identity.ino &&
sameArchiveFileIdentity(file, proof.identity) &&
file.size === proof.size
? {
filePath: proof.filePath,
@@ -3,7 +3,11 @@ import { eq, inArray } from 'drizzle-orm';
import { lstatSync } from 'node:fs';
import { isAbsolute } from 'node:path';
import * as schema from '../../database/schema';
import type { ArchiveFileIdentity } from './download-catchup-output';
import {
archiveFileIdentity,
sameArchiveFileIdentity,
type ArchiveFileIdentity,
} from './download-catchup-output';
import type { DownloadsDatabase } from './download-task';
export interface ArchiveFinalizationProof {
@@ -57,16 +61,10 @@ async function writeArchiveProof(
): Promise<void> {
const serialized = JSON.stringify({
...proof,
partialIdentity: {
dev: proof.partialIdentity.dev,
ino: proof.partialIdentity.ino,
},
partialIdentity: archiveFileIdentity(proof.partialIdentity),
...(proof.phase !== 'transfer'
? {
finalIdentity: {
dev: proof.finalIdentity.dev,
ino: proof.finalIdentity.ino,
},
finalIdentity: archiveFileIdentity(proof.finalIdentity),
}
: {}),
});
@@ -121,7 +119,9 @@ function identity(value: unknown): value is ArchiveFileIdentity {
const candidate = value as ArchiveFileIdentity;
return (
Number.isSafeInteger(candidate.dev) &&
Number.isSafeInteger(candidate.ino)
Number.isSafeInteger(candidate.ino) &&
Number.isFinite(candidate.birthtimeMs) &&
candidate.birthtimeMs > 0
);
}
@@ -193,8 +193,7 @@ export function verifiedArchiveSize(
try {
const file = lstatSync(proof.filePath);
return file.isFile() &&
file.dev === proof.finalIdentity.dev &&
file.ino === proof.finalIdentity.ino &&
sameArchiveFileIdentity(file, proof.finalIdentity) &&
file.size === proof.size
? proof.size
: null;
@@ -4,6 +4,29 @@ import { lstat, open } from 'node:fs/promises';
export interface ArchiveFileIdentity {
readonly dev: number;
readonly ino: number;
readonly birthtimeMs: number;
}
/** Inodes can be reused after unlink; creation time identifies the generation. */
export function sameArchiveFileIdentity(
a: ArchiveFileIdentity,
b: ArchiveFileIdentity
): boolean {
return (
a.dev === b.dev &&
a.ino === b.ino &&
Number.isFinite(a.birthtimeMs) &&
a.birthtimeMs > 0 &&
a.birthtimeMs === b.birthtimeMs
);
}
export function archiveFileIdentity(
file: ArchiveFileIdentity
): ArchiveFileIdentity {
if (!Number.isFinite(file.birthtimeMs) || file.birthtimeMs <= 0)
throw new Error('Archive filesystem creation time is unavailable');
return { dev: file.dev, ino: file.ino, birthtimeMs: file.birthtimeMs };
}
export class ArchivePartialReplacedError extends Error {
@@ -36,27 +59,26 @@ export async function openCatchupOutput(
);
try {
const current = await handle.stat();
const identity = archiveFileIdentity(current);
// Descriptor identity also protects platforms without O_NOFOLLOW from
// a link/file replacement between lstat and open. Missing files use wx.
if (
!current.isFile() ||
current.nlink !== 1 ||
(before &&
(current.dev !== before.dev || current.ino !== before.ino)) ||
(before && !sameArchiveFileIdentity(current, before)) ||
(before &&
expectedIdentity &&
(current.dev !== expectedIdentity.dev ||
current.ino !== expectedIdentity.ino))
!sameArchiveFileIdentity(current, expectedIdentity))
) {
throw new ArchivePartialReplacedError();
}
// Keep durable ownership evidence until the actual descriptor passes.
await beforeTruncate?.({ dev: current.dev, ino: current.ino });
await beforeTruncate?.(identity);
await handle.truncate(0);
// All writes use this verified descriptor; never reopen by pathname.
return {
stream: handle.createWriteStream({ autoClose: true }),
identity: { dev: current.dev, ino: current.ino },
identity,
};
} catch (error) {
await handle.close();
@@ -217,8 +217,8 @@ it.each([
version: 1,
filePath: '/tmp/a',
size: -1,
partialIdentity: { dev: 1, ino: 1 },
finalIdentity: { dev: 1, ino: 1 },
partialIdentity: { dev: 1, ino: 1, birthtimeMs: 1 },
finalIdentity: { dev: 1, ino: 1, birthtimeMs: 1 },
}),
])('ignores malformed journal %s', (value) => {
expect(parseArchiveFinalization(value)).toBeUndefined();
@@ -1,3 +1,8 @@
import { openCatchupOutput } from './download-catchup-output';
import {
verifiedArchiveSize,
parseArchiveFinalization,
} from './download-catchup-journal';
import {
mkdtempSync,
lstatSync,
@@ -230,3 +235,46 @@ it.each([false, true])(
else expect(() => lstatSync(filePath)).toThrow();
}
);
it('preserves a replacement when the filesystem reuses its predecessor inode', () => {
proof.partialIdentity = {
...proof.partialIdentity,
birthtimeMs: proof.partialIdentity.birthtimeMs - 1,
};
removeJournaledCatchupPartial(filePath, proof, recordCapture);
expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes');
expect(renameSync).not.toHaveBeenCalled();
});
it('rejects a reused inode before truncating or replacing the journal proof', async () => {
const previous = {
...proof.partialIdentity,
birthtimeMs: proof.partialIdentity.birthtimeMs - 1,
};
const record = jest.fn();
await expect(
openCatchupOutput(filePath + '.part', previous, record)
).rejects.toThrow('changed');
expect(record).not.toHaveBeenCalled();
expect(readFileSync(filePath + '.part', 'utf8')).toBe('owned bytes');
});
it('does not recover a same-size final from a reused inode or a legacy proof without creation time', () => {
writeFileSync(filePath, 'foreign archive');
const final = lstatSync(filePath);
const journal: ArchiveFinalizationProof = {
...proof,
phase: 'finalization',
size: final.size,
finalIdentity: { ...final, birthtimeMs: final.birthtimeMs - 1 },
};
expect(verifiedArchiveSize(filePath, journal)).toBeNull();
expect(
parseArchiveFinalization(
JSON.stringify({
...journal,
finalIdentity: { dev: final.dev, ino: final.ino },
})
)
).toBeUndefined();
});
@@ -17,7 +17,10 @@ import {
type ArchiveDownloadProof,
} from './download-catchup-journal';
import type { DownloadsDatabase } from './download-task';
import type { ArchiveFileIdentity } from './download-catchup-output';
import {
sameArchiveFileIdentity,
type ArchiveFileIdentity,
} from './download-catchup-output';
/** IPC removal stays synchronous after its runtime guard, like VOD cleanup. */
export function removeJournaledCatchupPartial(
@@ -44,7 +47,7 @@ function removeOwnedEntry(
recordCapture: (path: string) => void
): void {
const matches = (file: Stats, identity: ArchiveFileIdentity) =>
file.isFile() && file.dev === identity.dev && file.ino === identity.ino;
file.isFile() && sameArchiveFileIdentity(file, identity);
try {
if (!matches(lstatSync(path), identity)) return;
} catch (error) {
@@ -127,8 +130,7 @@ export async function cleanupStoredCatchupFinal(
proof.phase === 'transfer' ||
proof.filePath !== filePath ||
(createdIdentity &&
(createdIdentity.dev !== proof.finalIdentity.dev ||
createdIdentity.ino !== proof.finalIdentity.ino))
!sameArchiveFileIdentity(createdIdentity, proof.finalIdentity))
) {
// Only the exclusively created empty target can precede final proof;
// no copy bytes are written until its identity has committed.
@@ -1,7 +1,10 @@
import { lstat } from 'node:fs/promises';
import { cleanupStoredCatchupPartial } from './download-catchup-removal';
import { clearArchiveFinalization } from './download-catchup-journal';
import { ArchivePartialReplacedError } from './download-catchup-output';
import {
ArchivePartialReplacedError,
sameArchiveFileIdentity,
} from './download-catchup-output';
import { reserveAvailablePartialDownloadFile } from './download-file-path';
import type { DownloadsDatabase, DownloadTask } from './download-task';
@@ -22,8 +25,7 @@ export async function reserveFreshCatchupTarget(
partial &&
(!expected ||
!partial.isFile() ||
partial.dev !== expected.dev ||
partial.ino !== expected.ino)
!sameArchiveFileIdentity(partial, expected))
) {
throw new ArchivePartialReplacedError();
}
@@ -119,6 +119,7 @@ it.each(['failed', 'canceled'])(
expect.objectContaining({
dev: active.catchupPartialIdentity.dev,
ino: active.catchupPartialIdentity.ino,
birthtimeMs: active.catchupPartialIdentity.birthtimeMs,
})
);
await rename(
@@ -1,3 +1,4 @@
import { sameArchiveFileIdentity } from './download-catchup-output';
import {
cleanupStoredCatchupPartial,
cleanupStoredCatchupFinal,
@@ -37,11 +38,7 @@ function hasReplacedArchivePartial(download: StaleDownload): boolean {
try {
const file = lstatSync(`${download.filePath}.part`);
const expected = download.proof.partialIdentity;
return (
!file.isFile() ||
file.dev !== expected.dev ||
file.ino !== expected.ino
);
return !file.isFile() || !sameArchiveFileIdentity(file, expected);
} catch (error) {
return (error as NodeJS.ErrnoException).code !== 'ENOENT';
}
+4 -2
View File
@@ -92,7 +92,7 @@ cancellation to settle before reading/deleting its row; queued archives are
canceled first, and a concurrent new runtime attempt blocks removal. A settled
archive still marked downloading after a failed status write also retries
journal cleanup before row deletion. Remove/Clear preserve a final file whose
journaled identity and full size prove completed promotion, even when completion
journaled device/inode/creation-time identity and full size prove completed promotion, even when completion
status writes failed and its stored status is stale. Repeat submissions, Retry
and Resume restore such a journal-proven completion in place before any new
transfer or ownership reset, before expiry, provider DNS and new-folder checks
@@ -100,7 +100,9 @@ that apply only to another remote transfer; retained cleanup failures keep their
journal. Remove, Clear completed and missing-file
re-download and repeated programme submissions use journal-backed private
capture for archive partial cleanup;
unknown or replaced entries are preserved. Before capture, a synchronous SQLite
unknown or replaced entries are preserved. Ownership includes a positive file
creation timestamp as well as device/inode, so inode reuse after unlink cannot
bless a new entry; proofs lacking creation time remain untrusted. Before capture, a synchronous SQLite
write records `partialCleanupPath` (or `finalCleanupPath` for failed promotion)
in the existing ownership proof. Active cancellation/failure, promotion and
startup recovery use the same journal-backed cleanup as manual actions. Cancel,