From 86f7759719c4ebc8e4e0689a6423b308a63b756c Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 8 Sep 2026 05:00:39 +0200 Subject: [PATCH] fix(downloads): distinguish reused archive inodes by creation time --- AGENTS.md | 2 + CLAUDE.md | 2 + .../database/download-catchup-capture.ts | 11 ++--- .../database/download-catchup-cleanup.ts | 11 ++--- .../database/download-catchup-finalize.ts | 24 +++++----- .../database/download-catchup-journal.ts | 23 +++++---- .../database/download-catchup-output.ts | 34 ++++++++++--- .../download-catchup-recovery.spec.ts | 4 +- .../database/download-catchup-removal.spec.ts | 48 +++++++++++++++++++ .../database/download-catchup-removal.ts | 10 ++-- .../database/download-catchup-reservation.ts | 8 ++-- .../database/download-catchup-runtime.spec.ts | 1 + .../app/events/database/download-recovery.ts | 7 +-- docs/architecture/download-manager.md | 6 ++- 14 files changed, 133 insertions(+), 58 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8b0118705..dfe8e4dd4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/CLAUDE.md b/CLAUDE.md index 6293742cc..f8a19125a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 diff --git a/apps/electron-backend/src/app/events/database/download-catchup-capture.ts b/apps/electron-backend/src/app/events/database/download-catchup-capture.ts index 7b26c72c6..39cb8748d 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-capture.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-capture.ts @@ -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( diff --git a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts index f22cfe8b7..9df6aa236 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-cleanup.ts @@ -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 diff --git a/apps/electron-backend/src/app/events/database/download-catchup-finalize.ts b/apps/electron-backend/src/app/events/database/download-catchup-finalize.ts index 866bf1ae7..d69ef7173 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-finalize.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-finalize.ts @@ -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, diff --git a/apps/electron-backend/src/app/events/database/download-catchup-journal.ts b/apps/electron-backend/src/app/events/database/download-catchup-journal.ts index 7aeff0768..5272bd875 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-journal.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-journal.ts @@ -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 { 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; diff --git a/apps/electron-backend/src/app/events/database/download-catchup-output.ts b/apps/electron-backend/src/app/events/database/download-catchup-output.ts index e810b5165..edd0ecf39 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-output.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-output.ts @@ -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(); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts index 382612c95..458dabcd9 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-recovery.spec.ts @@ -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(); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts index 1dbb215ea..9f88a79ea 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-removal.spec.ts @@ -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(); +}); diff --git a/apps/electron-backend/src/app/events/database/download-catchup-removal.ts b/apps/electron-backend/src/app/events/database/download-catchup-removal.ts index 4c5a866bd..251042906 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-removal.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-removal.ts @@ -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. diff --git a/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts b/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts index 27de65b31..72df40b67 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-reservation.ts @@ -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(); } diff --git a/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts b/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts index 829f0f521..a47101fba 100644 --- a/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts +++ b/apps/electron-backend/src/app/events/database/download-catchup-runtime.spec.ts @@ -119,6 +119,7 @@ it.each(['failed', 'canceled'])( expect.objectContaining({ dev: active.catchupPartialIdentity.dev, ino: active.catchupPartialIdentity.ino, + birthtimeMs: active.catchupPartialIdentity.birthtimeMs, }) ); await rename( diff --git a/apps/electron-backend/src/app/events/database/download-recovery.ts b/apps/electron-backend/src/app/events/database/download-recovery.ts index a0ab9be4b..258971072 100644 --- a/apps/electron-backend/src/app/events/database/download-recovery.ts +++ b/apps/electron-backend/src/app/events/database/download-recovery.ts @@ -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'; } diff --git a/docs/architecture/download-manager.md b/docs/architecture/download-manager.md index eaef239a9..f05efb3bb 100644 --- a/docs/architecture/download-manager.md +++ b/docs/architecture/download-manager.md @@ -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,