mirror of
https://github.com/webadderallorg/Recordly.git
synced 2026-09-25 07:16:02 +00:00
Merge pull request #444 from meiiie/fix/pr410-post-merge-export-hardening
Small follow-up to #410 that handles the final CodeRabbit export-hardening notes after merge. - Treat initial destination `EEXIST` as recoverable so the safe replacement fallback can run on Windows. - Check local-read approval before `realpath`/`stat`, then keep the realpath re-check for symlink protection. - Adds focused regression coverage for the initial `EEXIST` path. Validation: - `npm test -- electron/ipc/register/export.test.ts electron/ipc/export/native-video.test.ts` (`67` tests) - `npx biome check --formatter-enabled=false electron/ipc/register/export.ts electron/ipc/register/export.test.ts` - `npx tsc --noEmit` - `git diff --check` CodeRabbit reported no actionable comments on the latest head.
This commit is contained in:
@@ -40,6 +40,7 @@ async function makeTempDir() {
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
await Promise.allSettled(
|
||||
tempDirs.splice(0).map((dir) => fs.rm(dir, { force: true, recursive: true })),
|
||||
);
|
||||
@@ -59,4 +60,29 @@ describe("moveExportedTempFile", () => {
|
||||
);
|
||||
await expect(fs.access(tempPath)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("falls back when Windows reports the destination already exists during initial rename", async () => {
|
||||
const dir = await makeTempDir();
|
||||
const tempPath = path.join(dir, "export-temp.mp4");
|
||||
const destinationPath = path.join(dir, "export-final.mp4");
|
||||
await fs.writeFile(tempPath, "new-export");
|
||||
await fs.writeFile(destinationPath, "previous-export");
|
||||
|
||||
const originalRename = fs.rename.bind(fs);
|
||||
const renameSpy = vi.spyOn(fs, "rename");
|
||||
renameSpy.mockImplementation(async (from, to) => {
|
||||
if (from === tempPath && to === destinationPath) {
|
||||
const error = new Error("destination exists") as NodeJS.ErrnoException;
|
||||
error.code = "EEXIST";
|
||||
throw error;
|
||||
}
|
||||
|
||||
return originalRename(from, to);
|
||||
});
|
||||
|
||||
await moveExportedTempFile(tempPath, destinationPath);
|
||||
|
||||
await expect(fs.readFile(destinationPath, "utf8")).resolves.toBe("new-export");
|
||||
await expect(fs.access(tempPath)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,7 +58,12 @@ export async function moveExportedTempFile(tempPath: string, destinationPath: st
|
||||
return;
|
||||
} catch (error) {
|
||||
const code = (error as NodeJS.ErrnoException).code;
|
||||
if (code !== "EXDEV" && code !== "EPERM" && code !== "ENOTEMPTY") {
|
||||
if (
|
||||
code !== "EXDEV" &&
|
||||
code !== "EPERM" &&
|
||||
code !== "ENOTEMPTY" &&
|
||||
code !== "EEXIST"
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
// Cross-device or Windows permission quirks — fall back to copy + unlink so
|
||||
@@ -143,6 +148,10 @@ async function resolveAllowedReadableFilePath(
|
||||
}
|
||||
|
||||
const resolvedPath = path.resolve(filePath);
|
||||
if (!isAllowedLocalReadPath(resolvedPath)) {
|
||||
throw new Error(`${label} is not approved for local reads`);
|
||||
}
|
||||
|
||||
const realPath = await fs.realpath(resolvedPath).catch(() => null);
|
||||
if (!realPath) {
|
||||
throw new Error(`${label} does not exist`);
|
||||
|
||||
Reference in New Issue
Block a user