From 9eb564116a1b036d6359d832ada107e4814b7ef7 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:52:43 +0200 Subject: [PATCH] fix(merge-recovery): canonicalize 8.3 short paths in artifact containment recoverInterruptedMergeArtifacts resolved the artifact root through fs.realpathSync.native but compared candidate artifact paths with path.resolve only. On systems whose temporary or download directories surface as Windows 8.3 short paths the queue-persisted artifact paths never matched the canonical long root, so crash artifacts were treated as outside the owned directory, nothing was removed and interrupted merge items were incorrectly blocked. isInside now canonicalizes both sides through the deepest existing path segment, keeping planned but not yet created paths comparable, and a Windows ShortPath regression test covers the recovery flow end to end. --- src/main/domain/merge-recovery.test.ts | 39 ++++++++++++++++++++++++++ src/main/domain/merge-recovery.ts | 18 +++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/main/domain/merge-recovery.test.ts b/src/main/domain/merge-recovery.test.ts index 8931244..a0169c2 100644 --- a/src/main/domain/merge-recovery.test.ts +++ b/src/main/domain/merge-recovery.test.ts @@ -1,3 +1,4 @@ +import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -5,6 +6,18 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import type { QueueItem } from '../../types'; import { getInterruptedMergeItemIds, recoverInterruptedMergeArtifacts } from './merge-recovery'; +function getWindowsShortPath(targetPath: string): string { + const command = `for %I in ("${targetPath}") do @echo %~sI`; + const result = spawnSync(process.env.ComSpec || 'cmd.exe', ['/d', '/c', command], { + encoding: 'utf8', + windowsHide: true, + windowsVerbatimArguments: true + }); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error(result.stderr.trim() || `ShortPath lookup exited with ${result.status}`); + return result.stdout.trim(); +} + let directory: string; beforeEach(() => { @@ -69,6 +82,32 @@ describe('recoverInterruptedMergeArtifacts', () => { expect(result.queue[0].artifactRoot).toBe(fs.realpathSync.native(directory)); }); + it.skipIf(process.platform !== 'win32')('removes crash artifacts referenced through a Windows 8.3 short path root', (context) => { + const shortDirectory = getWindowsShortPath(directory); + if (!shortDirectory || shortDirectory.toLowerCase() === fs.realpathSync.native(directory).toLowerCase()) { + context.skip(); + return; + } + const jobDirectory = path.join(shortDirectory, 'alice'); + fs.mkdirSync(jobDirectory); + const first = path.join(jobDirectory, 'merge_tmp_0_100.mp4'); + const merged = path.join(jobDirectory, '.merge_output_300_123.mp4'); + fs.writeFileSync(first, 'partial-a'); + fs.writeFileSync(merged, 'partial-merge'); + const item = queueItem(); + item.mergeGroup!.downloadedFiles = { 0: first }; + item.mergeGroup!.mergedFile = merged; + + const result = recoverInterruptedMergeArtifacts([item], shortDirectory, new Set([item.id])); + + expect(result.failedFiles).toEqual([]); + expect(result.removedFiles.sort()).toEqual([first, merged].sort()); + expect(fs.existsSync(first)).toBe(false); + expect(fs.existsSync(merged)).toBe(false); + expect(result.queue[0]).toMatchObject({ status: 'pending', progress: 0 }); + expect(result.queue[0].artifactRoot).toBe(fs.realpathSync.native(directory)); + }); + it('uses persisted artifact provenance after the configured download root changes', () => { const previousRoot = path.join(directory, 'previous'); const currentRoot = path.join(directory, 'current'); diff --git a/src/main/domain/merge-recovery.ts b/src/main/domain/merge-recovery.ts index a3426e8..a044d15 100644 --- a/src/main/domain/merge-recovery.ts +++ b/src/main/domain/merge-recovery.ts @@ -17,8 +17,24 @@ export interface MergeArtifactRootResolution { migrated: boolean; } +function canonicalizeExistingPrefix(candidatePath: string): string { + let existingPath = path.resolve(candidatePath); + const missingSegments: string[] = []; + for (;;) { + try { + return path.join(fs.realpathSync.native(existingPath), ...missingSegments.reverse()); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') return path.resolve(candidatePath); + const parentPath = path.dirname(existingPath); + if (parentPath === existingPath) return path.resolve(candidatePath); + missingSegments.push(path.basename(existingPath)); + existingPath = parentPath; + } + } +} + function isInside(root: string, candidate: string): boolean { - const relative = path.relative(path.resolve(root), path.resolve(candidate)); + const relative = path.relative(canonicalizeExistingPrefix(root), canonicalizeExistingPrefix(candidate)); return relative.length > 0 && !relative.startsWith('..') && !path.isAbsolute(relative); }