From d368f77fcd474bcfc238a97006a1112df86f329d Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 14:27:46 +0900 Subject: [PATCH] fix(security): make tar archive preflight fail-closed on unparsed entries --- .../tar-zip-entry-listing.test.ts | 155 ++++++++++++++++++ .../tar-zip-entry-listing.ts | 59 ++++++- 2 files changed, 208 insertions(+), 6 deletions(-) create mode 100644 src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts diff --git a/src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts b/src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts new file mode 100644 index 000000000..18c9c285e --- /dev/null +++ b/src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts @@ -0,0 +1,155 @@ +import { afterEach, describe, expect, it, mock, spyOn } from "bun:test" + +import * as logger from "../logger" +import { parseTarListingOutput } from "./tar-zip-entry-listing" + +function createTarFileLine(fileName: string): string { + return `-rw-r--r-- 1 user group 123 Jan 01 12:34 ${fileName}` +} + +function getWarnedUnparsedLines(logSpy: ReturnType): string[] { + return logSpy.mock.calls.flatMap(([message, data]) => { + if ( + message !== "warning: unparsed tar listing line" || + typeof data !== "object" || + data === null || + !("line" in data) || + typeof data.line !== "string" + ) { + return [] + } + + return [data.line] + }) +} + +function captureThrownError(run: () => void): Error { + try { + run() + } catch (error) { + if (error instanceof Error) { + return error + } + } + + throw new Error("Expected parser to throw") +} + +describe("parseTarListingOutput", () => { + afterEach(() => { + mock.restore() + }) + + describe("#given tar output with a small number of unparsed lines", () => { + it("#when parsing the output #then logs warnings and keeps the parsed entries", () => { + // given + const logSpy = spyOn(logger, "log").mockImplementation(() => {}) + const listedOutput = [ + createTarFileLine("file-1.txt"), + createTarFileLine("file-2.txt"), + createTarFileLine("file-3.txt"), + createTarFileLine("file-4.txt"), + createTarFileLine("file-5.txt"), + createTarFileLine("file-6.txt"), + createTarFileLine("file-7.txt"), + createTarFileLine("file-8.txt"), + createTarFileLine("file-9.txt"), + "unparsed listing line", + ].join("\n") + + // when + const parsedEntries = parseTarListingOutput(listedOutput) + + // then + expect(parsedEntries).toHaveLength(9) + expect(logSpy).toHaveBeenCalledWith("warning: unparsed tar listing line", { + line: "unparsed listing line", + }) + expect(getWarnedUnparsedLines(logSpy)).toContain("unparsed listing line") + }) + }) + + describe("#given tar output with too many unparsed lines by ratio", () => { + it("#when parsing the output #then throws a format drift error", () => { + // given + const logSpy = spyOn(logger, "log").mockImplementation(() => {}) + const listedOutput = [ + createTarFileLine("file-1.txt"), + createTarFileLine("file-2.txt"), + createTarFileLine("file-3.txt"), + createTarFileLine("file-4.txt"), + createTarFileLine("file-5.txt"), + createTarFileLine("file-6.txt"), + createTarFileLine("file-7.txt"), + createTarFileLine("file-8.txt"), + "unparsed listing line 1", + "unparsed listing line 2", + ].join("\n") + + // when + const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput)) + + // then + expect(thrownError.message).toMatch(/format drift detected/i) + expect(getWarnedUnparsedLines(logSpy)).toEqual( + expect.arrayContaining([ + "unparsed listing line 1", + "unparsed listing line 2", + ]) + ) + }) + }) + + describe("#given tar output where every non-empty line is unparsed", () => { + it("#when parsing the output #then rejects the listing instead of returning an empty array", () => { + // given + const logSpy = spyOn(logger, "log").mockImplementation(() => {}) + + // when + const thrownError = captureThrownError(() => + parseTarListingOutput(["unknown format 1", "unknown format 2"].join("\n")) + ) + + // then + expect(thrownError.message).toMatch(/format drift detected/i) + expect(getWarnedUnparsedLines(logSpy)).toEqual( + expect.arrayContaining(["unknown format 1", "unknown format 2"]) + ) + }) + }) + + describe("#given tar output with more than five unparsed lines", () => { + it("#when parsing the output #then rejects the listing even at a ten percent ratio", () => { + // given + const logSpy = spyOn(logger, "log").mockImplementation(() => {}) + const parsedLines = Array.from({ length: 54 }, (_, index) => + createTarFileLine(`file-${index + 1}.txt`) + ) + const listedOutput = [ + ...parsedLines, + "unparsed listing line 1", + "unparsed listing line 2", + "unparsed listing line 3", + "unparsed listing line 4", + "unparsed listing line 5", + "unparsed listing line 6", + ].join("\n") + + // when + const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput)) + + // then + expect(thrownError.message).toMatch(/format drift detected/i) + expect(getWarnedUnparsedLines(logSpy)).toEqual( + expect.arrayContaining([ + "unparsed listing line 1", + "unparsed listing line 2", + "unparsed listing line 3", + "unparsed listing line 4", + "unparsed listing line 5", + "unparsed listing line 6", + ]) + ) + }) + }) +}) diff --git a/src/shared/zip-entry-listing/tar-zip-entry-listing.ts b/src/shared/zip-entry-listing/tar-zip-entry-listing.ts index 05c33ac1b..3aaa63ef6 100644 --- a/src/shared/zip-entry-listing/tar-zip-entry-listing.ts +++ b/src/shared/zip-entry-listing/tar-zip-entry-listing.ts @@ -1,6 +1,10 @@ import { spawn } from "bun" import type { ArchiveEntry } from "../archive-entry-validator" +import { log } from "../logger" + +const MAX_UNPARSED_TAR_LINE_RATIO = 0.1 +const MAX_UNPARSED_TAR_LINE_COUNT = 5 function parseTarListedZipEntry(line: string): ArchiveEntry | null { const match = line.match( @@ -26,6 +30,54 @@ function parseTarListedZipEntry(line: string): ArchiveEntry | null { } } +function validateParsedTarListing( + totalLineCount: number, + unparsedLines: string[] +): void { + if (unparsedLines.length === 0) { + return + } + + const unparsedLineRatio = unparsedLines.length / totalLineCount + if ( + unparsedLineRatio > MAX_UNPARSED_TAR_LINE_RATIO || + unparsedLines.length > MAX_UNPARSED_TAR_LINE_COUNT + ) { + throw new Error( + `zip entry listing failed: tar output format drift detected (${unparsedLines.length}/${totalLineCount} lines unparsed)` + ) + } +} + +export function parseTarListingOutput(stdout: string): ArchiveEntry[] { + const listingLines = stdout + .split(/\r?\n/) + .map(line => line.trim()) + .filter(Boolean) + + if (listingLines.length === 0) { + return [] + } + + const parsedEntries: ArchiveEntry[] = [] + const unparsedLines: string[] = [] + + for (const listingLine of listingLines) { + const parsedEntry = parseTarListedZipEntry(listingLine) + if (parsedEntry === null) { + unparsedLines.push(listingLine) + log("warning: unparsed tar listing line", { line: listingLine }) + continue + } + + parsedEntries.push(parsedEntry) + } + + validateParsedTarListing(listingLines.length, unparsedLines) + + return parsedEntries +} + export async function listZipEntriesWithTar( archivePath: string ): Promise { @@ -44,10 +96,5 @@ export async function listZipEntriesWithTar( throw new Error(`zip entry listing failed (exit ${exitCode}): ${stderr}`) } - return stdout - .split(/\r?\n/) - .map(line => line.trim()) - .filter(Boolean) - .map(line => parseTarListedZipEntry(line)) - .filter((entry): entry is ArchiveEntry => entry !== null) + return parseTarListingOutput(stdout) }