From 726d19c3cc08ec52bbc19c758ef68e1f1588ccb6 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 14:48:44 +0900 Subject: [PATCH] fix(security): make tar parser strictly fail-closed on any unparsed line --- .../tar-zip-entry-listing.test.ts | 63 +++---------------- .../tar-zip-entry-listing.ts | 15 ++--- 2 files changed, 13 insertions(+), 65 deletions(-) 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 index 18c9c285e..106bb29fa 100644 --- a/src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts +++ b/src/shared/zip-entry-listing/tar-zip-entry-listing.test.ts @@ -40,37 +40,27 @@ describe("parseTarListingOutput", () => { 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", () => { + describe("#given tar output with any unparsed lines", () => { + it("#when parsing the output #then throws immediately (fail-closed)", () => { // 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) + const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput)) // then - expect(parsedEntries).toHaveLength(9) - expect(logSpy).toHaveBeenCalledWith("warning: unparsed tar listing line", { - line: "unparsed listing line", - }) + expect(thrownError.message).toMatch(/could not be parsed/i) 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", () => { + describe("#given tar output with multiple unparsed lines", () => { + it("#when parsing the output #then throws with count details", () => { // given const logSpy = spyOn(logger, "log").mockImplementation(() => {}) const listedOutput = [ @@ -90,7 +80,7 @@ describe("parseTarListingOutput", () => { const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput)) // then - expect(thrownError.message).toMatch(/format drift detected/i) + expect(thrownError.message).toMatch(/could not be parsed/i) expect(getWarnedUnparsedLines(logSpy)).toEqual( expect.arrayContaining([ "unparsed listing line 1", @@ -101,7 +91,7 @@ describe("parseTarListingOutput", () => { }) 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", () => { + it("#when parsing the output #then rejects the listing", () => { // given const logSpy = spyOn(logger, "log").mockImplementation(() => {}) @@ -111,45 +101,10 @@ describe("parseTarListingOutput", () => { ) // then - expect(thrownError.message).toMatch(/format drift detected/i) + expect(thrownError.message).toMatch(/could not be parsed/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 3aaa63ef6..10b231905 100644 --- a/src/shared/zip-entry-listing/tar-zip-entry-listing.ts +++ b/src/shared/zip-entry-listing/tar-zip-entry-listing.ts @@ -3,8 +3,7 @@ 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( @@ -38,15 +37,9 @@ function validateParsedTarListing( 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)` - ) - } + throw new Error( + `zip entry listing failed: ${unparsedLines.length}/${totalLineCount} tar listing lines could not be parsed (fail-closed)` + ) } export function parseTarListingOutput(stdout: string): ArchiveEntry[] {