fix(security): make tar parser strictly fail-closed on any unparsed line
This commit is contained in:
@@ -40,37 +40,27 @@ describe("parseTarListingOutput", () => {
|
|||||||
mock.restore()
|
mock.restore()
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("#given tar output with a small number of unparsed lines", () => {
|
describe("#given tar output with any unparsed lines", () => {
|
||||||
it("#when parsing the output #then logs warnings and keeps the parsed entries", () => {
|
it("#when parsing the output #then throws immediately (fail-closed)", () => {
|
||||||
// given
|
// given
|
||||||
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
||||||
const listedOutput = [
|
const listedOutput = [
|
||||||
createTarFileLine("file-1.txt"),
|
createTarFileLine("file-1.txt"),
|
||||||
createTarFileLine("file-2.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",
|
"unparsed listing line",
|
||||||
].join("\n")
|
].join("\n")
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const parsedEntries = parseTarListingOutput(listedOutput)
|
const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput))
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(parsedEntries).toHaveLength(9)
|
expect(thrownError.message).toMatch(/could not be parsed/i)
|
||||||
expect(logSpy).toHaveBeenCalledWith("warning: unparsed tar listing line", {
|
|
||||||
line: "unparsed listing line",
|
|
||||||
})
|
|
||||||
expect(getWarnedUnparsedLines(logSpy)).toContain("unparsed listing line")
|
expect(getWarnedUnparsedLines(logSpy)).toContain("unparsed listing line")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("#given tar output with too many unparsed lines by ratio", () => {
|
describe("#given tar output with multiple unparsed lines", () => {
|
||||||
it("#when parsing the output #then throws a format drift error", () => {
|
it("#when parsing the output #then throws with count details", () => {
|
||||||
// given
|
// given
|
||||||
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
||||||
const listedOutput = [
|
const listedOutput = [
|
||||||
@@ -90,7 +80,7 @@ describe("parseTarListingOutput", () => {
|
|||||||
const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput))
|
const thrownError = captureThrownError(() => parseTarListingOutput(listedOutput))
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(thrownError.message).toMatch(/format drift detected/i)
|
expect(thrownError.message).toMatch(/could not be parsed/i)
|
||||||
expect(getWarnedUnparsedLines(logSpy)).toEqual(
|
expect(getWarnedUnparsedLines(logSpy)).toEqual(
|
||||||
expect.arrayContaining([
|
expect.arrayContaining([
|
||||||
"unparsed listing line 1",
|
"unparsed listing line 1",
|
||||||
@@ -101,7 +91,7 @@ describe("parseTarListingOutput", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("#given tar output where every non-empty line is unparsed", () => {
|
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
|
// given
|
||||||
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
const logSpy = spyOn(logger, "log").mockImplementation(() => {})
|
||||||
|
|
||||||
@@ -111,45 +101,10 @@ describe("parseTarListingOutput", () => {
|
|||||||
)
|
)
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(thrownError.message).toMatch(/format drift detected/i)
|
expect(thrownError.message).toMatch(/could not be parsed/i)
|
||||||
expect(getWarnedUnparsedLines(logSpy)).toEqual(
|
expect(getWarnedUnparsedLines(logSpy)).toEqual(
|
||||||
expect.arrayContaining(["unknown format 1", "unknown format 2"])
|
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",
|
|
||||||
])
|
|
||||||
)
|
|
||||||
})
|
|
||||||
})
|
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -3,8 +3,7 @@ import { spawn } from "bun"
|
|||||||
import type { ArchiveEntry } from "../archive-entry-validator"
|
import type { ArchiveEntry } from "../archive-entry-validator"
|
||||||
import { log } from "../logger"
|
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 {
|
function parseTarListedZipEntry(line: string): ArchiveEntry | null {
|
||||||
const match = line.match(
|
const match = line.match(
|
||||||
@@ -38,15 +37,9 @@ function validateParsedTarListing(
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
const unparsedLineRatio = unparsedLines.length / totalLineCount
|
throw new Error(
|
||||||
if (
|
`zip entry listing failed: ${unparsedLines.length}/${totalLineCount} tar listing lines could not be parsed (fail-closed)`
|
||||||
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[] {
|
export function parseTarListingOutput(stdout: string): ArchiveEntry[] {
|
||||||
|
|||||||
Reference in New Issue
Block a user