refactor: clarify archive staging intent

This commit is contained in:
Peter Steinberger
2026-03-10 23:54:12 +00:00
parent 0bac47de51
commit 20237358d9
2 changed files with 18 additions and 23 deletions

View File

@@ -10,6 +10,7 @@ import { extractArchive, resolveArchiveKind, resolvePackedRootDir } from "./arch
let fixtureRoot = ""; let fixtureRoot = "";
let fixtureCount = 0; let fixtureCount = 0;
const directorySymlinkType = process.platform === "win32" ? "junction" : undefined;
async function makeTempDir(prefix = "case") { async function makeTempDir(prefix = "case") {
const dir = path.join(fixtureRoot, `${prefix}-${fixtureCount++}`); const dir = path.join(fixtureRoot, `${prefix}-${fixtureCount++}`);
@@ -48,6 +49,14 @@ async function writePackageArchive(params: {
await tar.c({ cwd: params.workDir, file: params.archivePath }, ["package"]); await tar.c({ cwd: params.workDir, file: params.archivePath }, ["package"]);
} }
async function createDirectorySymlink(targetDir: string, linkPath: string) {
await fs.symlink(targetDir, linkPath, directorySymlinkType);
}
async function expectPathMissing(filePath: string) {
await expect(fs.stat(filePath)).rejects.toMatchObject({ code: "ENOENT" });
}
async function expectExtractedSizeBudgetExceeded(params: { async function expectExtractedSizeBudgetExceeded(params: {
archivePath: string; archivePath: string;
destDir: string; destDir: string;
@@ -119,11 +128,7 @@ describe("archive utils", () => {
content: "hi", content: "hi",
}); });
await fs.rm(extractDir, { recursive: true, force: true }); await fs.rm(extractDir, { recursive: true, force: true });
await fs.symlink( await createDirectorySymlink(realExtractDir, extractDir);
realExtractDir,
extractDir,
process.platform === "win32" ? "junction" : undefined,
);
await expect( await expect(
extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }), extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }),
@@ -131,9 +136,7 @@ describe("archive utils", () => {
code: "destination-symlink", code: "destination-symlink",
} satisfies Partial<ArchiveSecurityError>); } satisfies Partial<ArchiveSecurityError>);
await expect( await expectPathMissing(path.join(realExtractDir, "package", "hello.txt"));
fs.stat(path.join(realExtractDir, "package", "hello.txt")),
).rejects.toMatchObject({ code: "ENOENT" });
}); });
}, },
); );
@@ -154,13 +157,7 @@ describe("archive utils", () => {
await withArchiveCase("zip", async ({ workDir, archivePath, extractDir }) => { await withArchiveCase("zip", async ({ workDir, archivePath, extractDir }) => {
const outsideDir = path.join(workDir, "outside"); const outsideDir = path.join(workDir, "outside");
await fs.mkdir(outsideDir, { recursive: true }); await fs.mkdir(outsideDir, { recursive: true });
// Use 'junction' on Windows — junctions target directories without await createDirectorySymlink(outsideDir, path.join(extractDir, "escape"));
// requiring SeCreateSymbolicLinkPrivilege.
await fs.symlink(
outsideDir,
path.join(extractDir, "escape"),
process.platform === "win32" ? "junction" : undefined,
);
const zip = new JSZip(); const zip = new JSZip();
zip.file("escape/pwn.txt", "owned"); zip.file("escape/pwn.txt", "owned");
@@ -273,11 +270,7 @@ describe("archive utils", () => {
await fs.mkdir(outsideDir, { recursive: true }); await fs.mkdir(outsideDir, { recursive: true });
await fs.mkdir(path.join(archiveRoot, "escape"), { recursive: true }); await fs.mkdir(path.join(archiveRoot, "escape"), { recursive: true });
await fs.writeFile(path.join(archiveRoot, "escape", "pwn.txt"), "owned"); await fs.writeFile(path.join(archiveRoot, "escape", "pwn.txt"), "owned");
await fs.symlink( await createDirectorySymlink(outsideDir, path.join(extractDir, "escape"));
outsideDir,
path.join(extractDir, "escape"),
process.platform === "win32" ? "junction" : undefined,
);
await tar.c({ cwd: archiveRoot, file: archivePath }, ["escape"]); await tar.c({ cwd: archiveRoot, file: archivePath }, ["escape"]);
await expect( await expect(
@@ -286,9 +279,7 @@ describe("archive utils", () => {
code: "destination-symlink-traversal", code: "destination-symlink-traversal",
} satisfies Partial<ArchiveSecurityError>); } satisfies Partial<ArchiveSecurityError>);
await expect(fs.stat(path.join(outsideDir, "pwn.txt"))).rejects.toMatchObject({ await expectPathMissing(path.join(outsideDir, "pwn.txt"));
code: "ENOENT",
});
}); });
}); });

View File

@@ -575,6 +575,10 @@ export async function extractArchive(params: {
stripComponents: params.stripComponents, stripComponents: params.stripComponents,
limits, limits,
}); });
// A canonical cwd is not enough here: tar can still follow
// pre-existing child symlinks in the live destination tree.
// Extract into a private staging dir first, then merge through
// the same safe-open boundary checks used by direct file writes.
await tar.x({ await tar.x({
file: params.archivePath, file: params.archivePath,
cwd: stagingDir, cwd: stagingDir,