From 4c3d64fcd801c5f731e33db7f66ff822c39cbb70 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Mon, 2 Nov 2026 20:21:34 -0800 Subject: [PATCH] fix(audit): split Go flags as Go does and read external linker flags Go splits a flag value with quoted.Split: a field that starts with a quote runs to the matching quote, and any other quote is a literal character. The reader stripped every quote, so CGO_CFLAGS -I"/home/O'Neil/include" reported /home/ONeil/include, while cc was given the directory "/home/O'Neil/include", quotes included. -ldflags kept -extldflags values with the Go linker flags, where only -L counts, so -extldflags=-Lprivate-sdk reported no path. The values are now read as the C compiler driver reads them, including -Wl, and -Xlinker forwarding. A package member named payload.json or payload.js.map that holds an ASAR took the text or map route, which checked only native signatures, so the nested archive was scanned as text and coverage read complete. The route's own read now recognizes the ASAR signature as well, and the nested package leaves coverage unknown. --- .../ReleaseExposureService.test.ts | 31 +++++++- .../releaseExposure/ReleaseExposureService.ts | 72 +++++++++++++------ src/artifacts/ReleaseExposureReader.test.ts | 17 +++++ src/artifacts/ReleaseExposureReader.ts | 48 +++++++++---- 4 files changed, 130 insertions(+), 38 deletions(-) diff --git a/src/application/releaseExposure/ReleaseExposureService.test.ts b/src/application/releaseExposure/ReleaseExposureService.test.ts index 8d86660707..a08291494d 100644 --- a/src/application/releaseExposure/ReleaseExposureService.test.ts +++ b/src/application/releaseExposure/ReleaseExposureService.test.ts @@ -1501,7 +1501,6 @@ describe("ReleaseExposureService Go relative flag paths", () => { `-X 'main.usage=see -L level'`, `-X '-L level'`, `-X main.usage=-L level`, - `-X='main.usage=see -L level'`, `--X 'main.usage=-L level'`, `all=-X 'main.usage=-L level'`, `"-X main.usage=-L level"`, @@ -1511,6 +1510,34 @@ describe("ReleaseExposureService Go relative flag paths", () => { expect(check?.status).toBe("not_observed"); }); + it.each([ + ["-extldflags=-Lprivate-sdk -s -w", "private-sdk"], + ["-extldflags -Lprivate-sdk -s -w", "private-sdk"], + [`-extldflags "-L private-sdk -static"`, "private-sdk"], + [`all=-extldflags '-Wl,-L,private-sdk'`, "private-sdk"], + [`-extldflags "-Xlinker -L -Xlinker private-sdk"`, "private-sdk"], + ])( + "reads the external linker flags in %s as the compiler driver does", + async (value, path) => { + const report = await goReport(goModule([{ key: "-ldflags", value }])); + const check = report.checks.find((item) => item.id === "go.build_paths"); + expect(check?.status).toBe("exposed"); + expect(check?.observation.samples).toEqual([path]); + }, + ); + + it.each([ + ["CGO_CFLAGS", `-I"/home/O'Neil/include"`, `"/home/O'Neil/include"`], + ["CGO_CFLAGS", `"-I/home/O'Neil/include"`, "/home/O'Neil/include"], + ["CGO_CFLAGS", `'-I/home/O"Neil/include'`, `/home/O"Neil/include`], + ["-ldflags", `-X='main.usage=see -L level'`, "level'"], + ])("keeps the quotes Go passes on in %s %s", async (key, value, path) => { + const report = await goReport(goModule([{ key, value }])); + const check = report.checks.find((item) => item.id === "go.build_paths"); + expect(check?.status).toBe("exposed"); + expect(check?.observation.samples).toEqual([path]); + }); + it.each([ [`"-isystem /opt/my sdk/include"`, "/opt/my sdk/include"], [`"-I private sdk/include"`, "private sdk/include"], @@ -1535,7 +1562,7 @@ describe("ReleaseExposureService Go paths", () => { key: "CGO_CXXFLAGS", value: "-isystem/opt/isystem -iquote /opt/quote", }, - { key: "CGO_LDFLAGS", value: `-L'/opt/my libs' -T/opt/link.ld` }, + { key: "CGO_LDFLAGS", value: `"-L/opt/my libs" -T/opt/link.ld` }, { key: "-ldflags", value: `-extldflags "-L/srv/ext lib -static"` }, { key: "-gcflags", value: "all=-trimpath=/src/a=/b" }, { key: "-pgo", value: "/srv/default.pgo" }, diff --git a/src/application/releaseExposure/ReleaseExposureService.ts b/src/application/releaseExposure/ReleaseExposureService.ts index 154b6da77c..6e88d94f6a 100644 --- a/src/application/releaseExposure/ReleaseExposureService.ts +++ b/src/application/releaseExposure/ReleaseExposureService.ts @@ -814,8 +814,12 @@ const filesystemPath = (value: string | null | undefined): string | null => { /** Go build settings whose values are flags or a build input path. */ const GO_FLAG_SETTING = /^(?:CGO_[A-Z]+FLAGS|-asmflags|-gccgoflags|-gcflags|-ldflags|-pgo)$/u; -/** One whitespace-separated field. Quotes keep their spaces inside it. */ -const GO_FLAG_FIELD = /(?:[^\s"']|"[^"]*"|'[^']*')+/gu; +/** + * One field as Go's `quoted.Split` reads a flag value: a field that begins with + * `"` or `'` runs to the matching quote, which it drops; any other field runs to + * whitespace and keeps every quote in it as a literal character. + */ +const GO_FLAG_FIELD = /"([^"]*)"|'([^']*)'|[^\s"'][^\s]*/gu; /** A path after a flag prefix, whitespace, `=`, or `,`, as in `-isystem/opt/x`. */ const GO_FLAG_PATH = /(?:^|[\s=,])(?:-(?:idirafter|imacros|include|iprefix|iquote|isysroot|isystem|[BFILT]))?((?:\.{1,2}[\\/]|\/|[A-Za-z]:[\\/]|\\{2}[^\\\s=,]+\\)[^=,]+)/gu; @@ -852,11 +856,16 @@ const GO_TOOL_PATH_FLAGS = new Map([ const GO_PACKAGE_SELECTOR = /^[^\s=-][^\s=]*=(?=-)/u; /** `-X name=value` sets a string, which names no option. */ const GO_LINKER_STRING_FLAG = /^-{1,2}X(?:[=\s]|$)/u; +/** + * `go tool link -extldflags` passes its value, split like any Go flag value, to + * the external linker, which Go runs through the C compiler driver. + */ +const GO_EXTERNAL_LINKER_FLAGS = /^-{1,2}extldflags(?:=(.*))?$/su; -/** Fields in one value, without their quotes. */ +/** Fields in one value, without their delimiting quotes. */ const goFields = (value: string): string[] => - [...value.matchAll(GO_FLAG_FIELD)].map(([field]) => - field.replace(/["']/gu, ""), + [...value.matchAll(GO_FLAG_FIELD)].map( + ([field, double, single]) => double ?? single ?? field, ); /** A field's flags, with a quoted path kept whole up to the next flag. */ @@ -866,23 +875,36 @@ const splitGoField = (field: string): string[] => field.split(/\s+(?=-)/u); const goFlags = (value: string): string[] => goFields(value).flatMap(splitGoField); -/** One Go tool's flags, each `-X` kept apart from the string it sets. */ -const goToolFlags = (value: string): string[] => { +/** + * One Go tool's flags, each `-X` kept apart from the string it sets, and the + * values given to `-extldflags` kept apart for the external linker. + */ +const goToolFlags = ( + value: string, +): { flags: string[]; external: string[] } => { const flags: string[] = []; - let setsString = false; + const external: string[] = []; + let argument: "string" | "external" | null = null; for (const [index, field] of goFields(value).entries()) { - if (setsString) { - setsString = false; + if (argument !== null) { + if (argument === "external") external.push(field); + argument = null; continue; } const flag = index === 0 ? field.replace(GO_PACKAGE_SELECTOR, "") : field; if (GO_LINKER_STRING_FLAG.test(flag)) { - setsString = /^-{1,2}X$/u.test(flag); + if (/^-{1,2}X$/u.test(flag)) argument = "string"; + continue; + } + const externalFlags = GO_EXTERNAL_LINKER_FLAGS.exec(flag); + if (externalFlags !== null) { + if (externalFlags[1] === undefined) argument = "external"; + else external.push(externalFlags[1]); continue; } flags.push(...splitGoField(flag)); } - return flags; + return { flags, external }; }; const goFlagPaths = (value: string): string[] => @@ -924,19 +946,25 @@ const splitForwardedLinkerFlags = ( return { compiler, linker }; }; +/** Relative paths in flags a C compiler driver reads. */ +const compilerRelativePaths = (value: string): string[] => { + const { compiler, linker } = splitForwardedLinkerFlags(goFlags(value)); + return [ + ...goRelativeFlagPaths(compiler, GO_COMPILER_PATH_FLAG), + ...goRelativeFlagPaths(linker, LINKER_PATH_FLAG), + ]; +}; + /** Relative paths in the settings that take a path flag. */ const goSettingRelativePaths = (key: string, value: string): string[] => { - if (GO_COMPILER_SETTING.test(key)) { - const { compiler, linker } = splitForwardedLinkerFlags(goFlags(value)); - return [ - ...goRelativeFlagPaths(compiler, GO_COMPILER_PATH_FLAG), - ...goRelativeFlagPaths(linker, LINKER_PATH_FLAG), - ]; - } + if (GO_COMPILER_SETTING.test(key)) return compilerRelativePaths(value); const pathFlag = GO_TOOL_PATH_FLAGS.get(key); - return pathFlag === undefined - ? [] - : goRelativeFlagPaths(goToolFlags(value), pathFlag); + if (pathFlag === undefined) return []; + const { flags, external } = goToolFlags(value); + return [ + ...goRelativeFlagPaths(flags, pathFlag), + ...external.flatMap(compilerRelativePaths), + ]; }; const goPaths = ( diff --git a/src/artifacts/ReleaseExposureReader.test.ts b/src/artifacts/ReleaseExposureReader.test.ts index 6482d5236f..32a3c5158b 100644 --- a/src/artifacts/ReleaseExposureReader.test.ts +++ b/src/artifacts/ReleaseExposureReader.test.ts @@ -2266,6 +2266,23 @@ describe("ReleaseExposureReader unexpanded package PE reads", () => { ]); }); + it.each(["payload.json", "payload.js.map"])( + "keeps a nested ASAR named %s unknown", + async (name) => { + const inner = asarArchive([ + { name: "app.js.map", bytes: embeddedSourceMap() }, + ]); + const root = await bundled([{ path: name, bytes: inner }]); + const scanned = await inspect(root); + expect(scanned.source_maps.embedded).toBe(0); + expect(scanned.coverage).toBe("unknown"); + expect(sourceMapSourcesStatus(scanned)).toBe("unknown"); + expect(scanned.limitations).toContain( + `Nested package ${join(root, "app.dll")}/${name} was not expanded.`, + ); + }, + ); + it("retains each compressed duplicate entry across PE reads", async () => { const root = await createTestTempDirectory("rea-release-duplicate-pe-"); const paths = ["C:\\build\\first.pdb", "C:\\build\\second.pdb"]; diff --git a/src/artifacts/ReleaseExposureReader.ts b/src/artifacts/ReleaseExposureReader.ts index 95fbe9bfab..2c1bc8f433 100644 --- a/src/artifacts/ReleaseExposureReader.ts +++ b/src/artifacts/ReleaseExposureReader.ts @@ -1015,10 +1015,18 @@ const examineMapMember = async ( ? member : { ...member, logical: `${member.logical}.js.map` }; const read = await readMap(state, declared); - const native = - read === null ? null : packagedNativeOfRead(member, read.loaded); - if (native !== null) - return examinePackagedBinary(state, { ...member, format: native }, context); + const packaged = + read === null ? null : packagedFormatOfRead(member, read.loaded); + if (packaged === "asar") { + noteNestedPackage(state, "asar", member.location); + return true; + } + if (packaged !== null) + return examinePackagedBinary( + state, + { ...member, format: packaged }, + context, + ); if (read !== null) settleMapRead(state, declared, read); const loaded = read?.loaded ?? null; if (loaded?.kind === "bytes") @@ -1230,10 +1238,20 @@ const examineTextMember = async ( const loaded = await member.read( Math.min(MAX_RELEASE_EXPOSURE_TEXT_MEMBER_BYTES, remaining), ); - const native = packagedNativeOfRead({ ...member, kind: listedKind }, loaded); - if (native !== null) { + const packaged = packagedFormatOfRead( + { ...member, kind: listedKind }, + loaded, + ); + if (packaged !== null) { state.textBytes += state.memberBytesRead - bytesReadBefore; - return examinePackagedBinary(state, { ...member, format: native }, context); + if (packaged !== "asar") + return examinePackagedBinary( + state, + { ...member, format: packaged }, + context, + ); + noteNestedPackage(state, "asar", member.location); + return true; } if (loaded.kind === "bytes") { const text = decodeMemberText(loaded.bytes); @@ -1274,23 +1292,25 @@ const hasNodeShebang = (prefix: Buffer): boolean => { /** * A name is not a format: a package member called `payload.json` or - * `payload.js.map` can be a PE, and a script or map declaration names whatever - * is at its target. The route's own read decides, so real text and maps cost no - * further read. A kind the owner stated is kept; a promoted member passes the - * kind it was listed with. + * `payload.js.map` can be a PE or an ASAR, and a script or map declaration + * names whatever is at its target. The route's own read decides, so real text + * and maps cost no further read. A kind the owner stated is kept; a promoted + * member passes the kind it was listed with. */ -const packagedNativeOfRead = ( +const packagedFormatOfRead = ( member: MemberView, loaded: MemberRead, -): NativeFormat | null => { +): NativeFormat | "asar" | null => { if (member.packageFormat === null || member.kind !== "file") return null; if (loaded.kind === "unread") return null; const header = loaded.kind === "bytes" ? loaded.bytes : loaded.prefix; if (header === null) return null; - return sniffNativeFormat( + const native = sniffNativeFormat( header, loaded.kind === "bytes" ? header.length : member.declaredSize, ); + if (native !== null) return native; + return holdsAsarHeader(header) ? "asar" : null; }; /**