fix(downloads): preserve the last link of captured foreign files

This commit is contained in:
4gray committed 2026-09-08 08:03:38 +02:00
1 parent c5e1c78392
commit 0f0e6ebf86
8 files changed
+87 -41

No files matched your search

+2
View File
@@ -1057,6 +1057,8 @@ reused inodes after unlink; old proofs without creation time remain untrusted.
Fresh reservations atomically commit their row path/name and captured ownership
before the initial HTTP wait;
no preexisting partial is truncated without matching expected ownership.
Captured foreign files retain their recovery copy and journal even after public
restoration, until the user explicitly removes the recovery copy.
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
@@ -1868,6 +1868,8 @@ reused inodes after unlink; old proofs without creation time remain untrusted.
Fresh reservations atomically commit their row path/name and captured ownership
before the initial HTTP wait;
no preexisting partial is truncated without matching expected ownership.
Captured foreign files retain their recovery copy and journal even after public
restoration, until the user explicitly removes the recovery copy.
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
@@ -39,20 +39,7 @@ function cleanupCapture(
}
if (file) {
if (!(file.isFile() && sameArchiveFileIdentity(file, identity))) {
// Restore without clobbering. If a prior restore linked successfully
// but unlink failed, the matching public entry permits cleanup retry.
try {
linkSync(path, publicPath);
} catch (error) {
if (
(error as NodeJS.ErrnoException).code !== 'EEXIST' ||
!sameArchiveFileIdentity(
readArchiveStatsSync(publicPath),
file
)
)
throw error;
}
restoreArchiveReplacement(path, publicPath);
}
unlinkSync(path);
}
@@ -62,3 +49,25 @@ function cleanupCapture(
/* never recursively delete captures */
}
}
/** Never unlink a foreign capture after publishing a raceable public link. */
export function restoreArchiveReplacement(
captured: string,
publicPath: string
): never {
const file = readArchiveStatsSync(captured);
try {
linkSync(captured, publicPath);
} catch (error) {
if (
(error as NodeJS.ErrnoException).code !== 'EEXIST' ||
!sameArchiveFileIdentity(readArchiveStatsSync(publicPath), file)
)
throw error;
}
// A second process can remove the public link at any time. Only an explicit
// user cleanup of this recovery copy may release its durable journal entry.
throw new Error(
`Archive cleanup preserved an unrelated recovery file: ${captured}`
);
}
@@ -61,7 +61,9 @@ it('restores a replacement that arrives before atomic capture', async () => {
await writeFile(from, 'replacement');
await actual.rename(from, to);
});
await cleanupCatchupFile(path, identity);
await expect(cleanupCatchupFile(path, identity)).rejects.toThrow(
'preserved an unrelated recovery file'
);
expect(await readFile(path, 'utf8')).toBe('replacement');
expect(await readFile(join(directory, 'original'), 'utf8')).toBe('archive');
});
@@ -25,7 +25,9 @@ export async function cleanupCatchupFile(
// public entry. If restoration is unavailable, retain it privately.
try {
await link(captured, path);
await unlink(captured);
throw new Error(
`Archive cleanup preserved an unrelated recovery file: ${captured}`
);
} catch (error) {
console.warn(
'[Downloads] Replaced file retained for recovery:',
@@ -73,10 +73,15 @@ it('restores a replacement captured at the cleanup boundary', () => {
writeFileSync(from, 'unrelated bytes');
actual.renameSync(from, to);
});
removeJournaledCatchupPartial(filePath, proof, recordCapture);
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('preserved an unrelated recovery file');
expect(readFileSync(filePath + '.part', 'utf8')).toBe('unrelated bytes');
expect(readFileSync(proof.partialCleanupPath!, 'utf8')).toBe(
'unrelated bytes'
);
});
it('retains a replacement capture until no-clobber restoration succeeds', () => {
it('retains a replacement capture until explicit recovery-copy cleanup', () => {
jest.mocked(renameSync).mockImplementationOnce((from, to) => {
actual.renameSync(from, join(directory, 'original'));
writeFileSync(from, 'foreign bytes');
@@ -113,16 +118,20 @@ it('retains a replacement capture until no-clobber restoration succeeds', () =>
expect(readFileSync(filePath + '.part', 'utf8')).toBe('newer public bytes');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
actual.unlinkSync(filePath + '.part');
jest.mocked(unlinkSync).mockImplementationOnce(() => {
throw new Error('restored capture still locked');
});
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('restored capture still locked');
).toThrow('preserved an unrelated recovery file');
expect(readFileSync(filePath + '.part', 'utf8')).toBe('foreign bytes');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
// The restored public entry may disappear immediately; its recovery copy
// must survive and remain journaled across another cleanup attempt.
actual.unlinkSync(filePath + '.part');
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('preserved an unrelated recovery file');
expect(readFileSync(capture, 'utf8')).toBe('foreign bytes');
actual.unlinkSync(capture); // explicit recovery-copy cleanup by the user
removeJournaledCatchupPartial(filePath, proof, recordCapture);
expect(readFileSync(filePath + '.part', 'utf8')).toBe('foreign bytes');
expect(() => lstatSync(capture)).toThrow();
});
@@ -360,3 +369,31 @@ it.each([false, true])(
expect(renameSync).not.toHaveBeenCalled();
}
);
it.each([false, true])(
'preserves the last foreign link if the restored public entry is immediately removed (retry=%s)',
(retry) => {
if (retry) {
const capture = join(directory, 'foreign-capture');
writeFileSync(capture, 'foreign bytes');
proof.partialCleanupPath = capture;
actual.unlinkSync(filePath + '.part');
} else {
jest.mocked(renameSync).mockImplementationOnce((from, to) => {
actual.renameSync(from, join(directory, 'original'));
writeFileSync(from, 'foreign bytes');
actual.renameSync(from, to);
});
}
jest.mocked(linkSync).mockImplementationOnce((from, to) => {
actual.linkSync(from, to);
actual.unlinkSync(to);
});
expect(() =>
removeJournaledCatchupPartial(filePath, proof, recordCapture)
).toThrow('preserved an unrelated recovery file');
expect(readFileSync(proof.partialCleanupPath!, 'utf8')).toBe(
'foreign bytes'
);
}
);
@@ -2,14 +2,11 @@ import {
readArchiveStatsSync,
type ArchiveFileStats,
} from './download-catchup-stats';
import { cleanupArchiveCapture } from './download-catchup-capture';
import {
mkdtempSync,
renameSync,
linkSync,
unlinkSync,
rmdirSync,
} from 'node:fs';
cleanupArchiveCapture,
restoreArchiveReplacement,
} from './download-catchup-capture';
import { mkdtempSync, renameSync, unlinkSync, rmdirSync } from 'node:fs';
import { dirname, join } from 'node:path';
import {
readArchiveFinalizations,
@@ -63,16 +60,7 @@ function removeOwnedEntry(
if (matches(readArchiveStatsSync(captured), identity)) {
unlinkSync(captured);
} else {
try {
linkSync(captured, path);
unlinkSync(captured);
} catch (error) {
console.warn(
'[Downloads] Replaced file retained for recovery:',
captured
);
throw error;
}
restoreArchiveReplacement(captured, path);
}
} finally {
try {
+5 -1
View File
@@ -117,7 +117,11 @@ final before reserving a destination. A failed
unlink or replacement restoration keeps that durable pointer and blocks journal
deletion/replacement. Later cleanup retries no-clobber restoration of captured
foreign entries; an occupied public path or unsupported hardlinks preserves both
the capture and its journal for recovery. Remove/Clear, Retry/Resume and fresh
the capture and its journal for recovery. Even after a foreign entry is restored,
its private recovery copy is never automatically unlinked: another process could
remove the public link first. The error names the recovery file; after the user
recovers it and explicitly removes that private copy, cleanup may release the
journal. Ordinary owned-file cleanup remains automatic. Remove/Clear, Retry/Resume and fresh
reservations retry identity-verified cleanup, including after restart and on
filesystems without hardlinks. Cleanup remains synchronous after the
runtime guard, so a completion transition cannot interleave with unlink.