From 8a8bf552f4505d0ee85f6cfaa85d05616f7ebb39 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Wed, 15 Oct 2025 14:23:24 -0700 Subject: [PATCH] Fix: Receive data in security scanner via stdin not args --- src/async/posix_event_loop.zig | 6 ++ src/install/PackageManager/scanner-entry.ts | 21 ++++- .../PackageManager/security_scanner.zig | 63 +++++++++---- .../bun-install-security-provider.test.ts | 93 +++++++++++++++++-- test/cli/install/dummy.registry.ts | 10 +- 5 files changed, 165 insertions(+), 28 deletions(-) diff --git a/src/async/posix_event_loop.zig b/src/async/posix_event_loop.zig index b503feac19..e7762ccf19 100644 --- a/src/async/posix_event_loop.zig +++ b/src/async/posix_event_loop.zig @@ -149,6 +149,7 @@ pub const FilePoll = struct { const Subprocess = jsc.Subprocess; const StaticPipeWriter = Subprocess.StaticPipeWriter.Poll; const ShellStaticPipeWriter = bun.shell.ShellSubprocess.StaticPipeWriter.Poll; + const SecurityScanSubprocessStaticPipeWriter = bun.install.SecurityScanSubprocess.StaticPipeWriter.Poll; const FileSink = jsc.WebCore.FileSink.Poll; const DNSResolver = bun.api.dns.Resolver; const GetAddrInfoRequest = bun.api.dns.GetAddrInfoRequest; @@ -170,6 +171,7 @@ pub const FilePoll = struct { StaticPipeWriter, ShellStaticPipeWriter, + SecurityScanSubprocessStaticPipeWriter, // ShellBufferedWriter, @@ -371,6 +373,10 @@ pub const FilePoll = struct { var handler: *StaticPipeWriter = ptr.as(StaticPipeWriter); handler.onPoll(size_or_offset, poll.flags.contains(.hup)); }, + @field(Owner.Tag, @typeName(SecurityScanSubprocessStaticPipeWriter)) => { + var handler: *SecurityScanSubprocessStaticPipeWriter = ptr.as(SecurityScanSubprocessStaticPipeWriter); + handler.onPoll(size_or_offset, poll.flags.contains(.hup)); + }, @field(Owner.Tag, @typeName(FileSink)) => { var handler: *FileSink = ptr.as(FileSink); handler.onPoll(size_or_offset, poll.flags.contains(.hup)); diff --git a/src/install/PackageManager/scanner-entry.ts b/src/install/PackageManager/scanner-entry.ts index 79264c45ca..c187c5b6b3 100644 --- a/src/install/PackageManager/scanner-entry.ts +++ b/src/install/PackageManager/scanner-entry.ts @@ -1,9 +1,28 @@ import fs from "node:fs"; const scannerModuleName = "__SCANNER_MODULE__"; -const packages = __PACKAGES_JSON__; const suppressError = __SUPPRESS_ERROR__; +let packagesJson: string = ""; +try { + const stdinBuffer = await Bun.stdin.text(); + packagesJson = stdinBuffer; +} catch (error) { + console.error("Failed to read packages from stdin:", error); + process.exit(1); +} + +let packages: Bun.Security.Package[]; +try { + packages = JSON.parse(packagesJson); + if (!Array.isArray(packages)) { + throw new Error("Expected packages to be an array"); + } +} catch (error) { + console.error("Failed to parse packages JSON from stdin:", error); + process.exit(1); +} + type IPCMessage = | { type: "result"; advisories: Bun.Security.Advisory[] } | { type: "error"; code: "MODULE_NOT_FOUND"; module: string } diff --git a/src/install/PackageManager/security_scanner.zig b/src/install/PackageManager/security_scanner.zig index f1ad798378..8dbfd541d9 100644 --- a/src/install/PackageManager/security_scanner.zig +++ b/src/install/PackageManager/security_scanner.zig @@ -639,17 +639,6 @@ fn attemptSecurityScanWithRetry(manager: *PackageManager, security_scanner: []co temp_source = code.items; } - const packages_placeholder = "__PACKAGES_JSON__"; - if (std.mem.indexOf(u8, temp_source, packages_placeholder)) |index| { - var new_code = std.ArrayList(u8).init(manager.allocator); - try new_code.appendSlice(temp_source[0..index]); - try new_code.appendSlice(json_data); - try new_code.appendSlice(temp_source[index + packages_placeholder.len ..]); - code.deinit(); - code = new_code; - temp_source = code.items; - } - const suppress_placeholder = "__SUPPRESS_ERROR__"; if (std.mem.indexOf(u8, temp_source, suppress_placeholder)) |index| { var new_code = std.ArrayList(u8).init(manager.allocator); @@ -702,16 +691,18 @@ pub const SecurityScanSubprocess = struct { has_received_ipc: bool = false, exit_status: ?bun.spawn.Status = null, remaining_fds: i8 = 0, + stdin_writer: ?*StaticPipeWriter = null, pub const new = bun.TrivialNew(@This()); + pub const StaticPipeWriter = jsc.Subprocess.NewStaticPipeWriter(@This()); pub fn spawn(this: *SecurityScanSubprocess) !void { this.ipc_data = std.ArrayList(u8).init(this.manager.allocator); this.stderr_data = std.ArrayList(u8).init(this.manager.allocator); this.ipc_reader.setParent(this); - const pipe_result = bun.sys.pipe(); - const pipe_fds = switch (pipe_result) { + const ipc_pipe_result = bun.sys.pipe(); + const ipc_pipe_fds = switch (ipc_pipe_result) { .err => { return error.IPCPipeFailed; }, @@ -734,12 +725,22 @@ pub const SecurityScanSubprocess = struct { const spawn_cwd = FileSystem.instance.top_level_dir; + var stdin_stdio = bun.spawn.Stdio{ .pipe = {} }; + + const stdin_opt = switch (stdin_stdio.asSpawnOption(0)) { + .result => |opt| opt, + .err => |e| { + Output.errGeneric("Failed to create stdin pipe: {any}", .{e}); + return error.StdinPipeFailed; + }, + }; + const spawn_options = bun.spawn.SpawnOptions{ .stdout = .inherit, .stderr = .inherit, - .stdin = .inherit, + .stdin = stdin_opt, .cwd = spawn_cwd, - .extra_fds = &.{.{ .pipe = pipe_fds[1] }}, + .extra_fds = &.{.{ .pipe = ipc_pipe_fds[1] }}, .windows = if (Environment.isWindows) .{ .loop = jsc.EventLoopHandle.init(&this.manager.event_loop), }, @@ -747,22 +748,38 @@ pub const SecurityScanSubprocess = struct { var spawned = try (try bun.spawn.spawnProcess(&spawn_options, @ptrCast(&argv), @ptrCast(std.os.environ.ptr))).unwrap(); - pipe_fds[1].close(); + ipc_pipe_fds[1].close(); - if (comptime bun.Environment.isPosix) { - _ = bun.sys.setNonblocking(pipe_fds[0]); + if (comptime bun.Environment.isPosix) { + _ = bun.sys.setNonblocking(ipc_pipe_fds[0]); } this.remaining_fds = 1; this.ipc_reader.flags.nonblocking = true; if (comptime bun.Environment.isPosix) { this.ipc_reader.flags.socket = false; } - try this.ipc_reader.start(pipe_fds[0], true).unwrap(); + try this.ipc_reader.start(ipc_pipe_fds[0], true).unwrap(); var process = spawned.toProcess(&this.manager.event_loop, false); this.process = process; process.setExitHandler(this); + + const json_data_copy = try this.manager.allocator.dupe(u8, this.json_data); + const stdin_source = jsc.Subprocess.Source{ + .blob = jsc.WebCore.Blob.Any.fromOwnedSlice(this.manager.allocator, json_data_copy), + }; + + this.stdin_writer = StaticPipeWriter.create(&this.manager.event_loop, this, spawned.stdin, stdin_source); + + switch (this.stdin_writer.?.start()) { + .err => |err| { + Output.errGeneric("Failed to start stdin writer: {}", .{err}); + return error.StdinWriterFailed; + }, + .result => {}, + } + switch (process.watchOrReap()) { .err => { return error.ProcessWatchFailed; @@ -775,6 +792,14 @@ pub const SecurityScanSubprocess = struct { return this.has_process_exited and this.remaining_fds == 0; } + pub fn onCloseIO(this: *SecurityScanSubprocess, _: jsc.Subprocess.StdioKind) void { + if (this.stdin_writer) |writer| { + writer.source.detach(); + writer.deref(); + this.stdin_writer = null; + } + } + pub fn eventLoop(this: *const SecurityScanSubprocess) *jsc.AnyEventLoop { return &this.manager.event_loop; } diff --git a/test/cli/install/bun-install-security-provider.test.ts b/test/cli/install/bun-install-security-provider.test.ts index 1af1180500..41d1de37f6 100644 --- a/test/cli/install/bun-install-security-provider.test.ts +++ b/test/cli/install/bun-install-security-provider.test.ts @@ -28,13 +28,15 @@ function test( bunfigScanner?: string | false; packages?: string[]; scannerFile?: string; + packageJson?: object; + customRegistry?: (urls: string[]) => any; }, ) { it( name, async () => { const urls: string[] = []; - setHandler(dummyRegistry(urls)); + setHandler(options.customRegistry ? options.customRegistry(urls) : dummyRegistry(urls)); const scannerPath = options.scannerFile || "./scanner.ts"; if (typeof options.scanner === "string") { @@ -53,11 +55,14 @@ function test( await write("./bunfig.toml", `${bunfig}\n[install.security]\nscanner = "${scannerPath}"`); } - await write("package.json", { - name: "my-app", - version: "1.0.0", - dependencies: {}, - }); + await write( + "package.json", + options.packageJson ?? { + name: "my-app", + version: "1.0.0", + dependencies: {}, + }, + ); const expectedExitCode = options.expectedExitCode ?? (options.fails ? 1 : 0); const packages = options.packages ?? ["bar"]; @@ -679,3 +684,79 @@ describe("Package Resolution", () => { }, }); }); + +describe("Large Payload via stdin", () => { + let tgzTempDir: string; + + beforeAll(async () => { + const { tmpdir } = await import("node:os"); + const { mkdtemp } = await import("node:fs/promises"); + const { join } = await import("node:path"); + tgzTempDir = await mkdtemp(join(tmpdir(), "bun-test-tgz-")); + + // Copy bar-0.0.2.tgz and create test-pkg-* copies in temp dir + const testDir = import.meta.dir; + const barTarball = `${testDir}/bar-0.0.2.tgz`; + const barContent = Bun.file(barTarball); + + // Create 10,000 packages to generate ~1.25MB of JSON + // (each entry is ~125 bytes, so 10k * 125 = 1.25MB) + for (let i = 0; i < 10000; i++) { + const targetPath = `${tgzTempDir}/test-pkg-${i}-0.0.2.tgz`; + await Bun.write(targetPath, barContent); + } + }); + + afterAll(async () => { + const { rm } = await import("node:fs/promises"); + try { + await rm(tgzTempDir, { recursive: true, force: true }); + } catch (e) {} + }); + + test("handles JSON data larger than max arg length (>1MB)", { + testTimeout: 60_000, + scanner: async ({ packages }) => { + const jsonSize = JSON.stringify(packages).length; + console.log(`Received JSON payload of ${jsonSize} bytes from ${packages.length} packages via stdin`); + + if (jsonSize < 1024 * 1024) { + throw new Error(`Expected JSON payload to exceed 1MB, got ${jsonSize} bytes`); + } + + if (packages.length === 0) { + throw new Error("Expected to receive packages via stdin"); + } + + return []; + }, + + packageJson: (() => { + const dependencies: Record = {}; + + for (let i = 0; i < 10000; i++) { + dependencies[`test-pkg-${i}`] = "0.0.2"; + } + return { + name: "my-app", + version: "1.0.0", + dependencies, + }; + })(), + packages: [], + customRegistry: urls => dummyRegistry(urls, { "0.0.2": {} }, 0, tgzTempDir), + expectedExitCode: 0, + expect: ({ out, err }) => { + expect(out).toContain("Received JSON payload"); + expect(out).toContain("via stdin"); + expect(out).toContain("packages"); + + const match = out.match(/Received JSON payload of (\d+) bytes/); + if (match) { + const bytes = parseInt(match[1], 10); + expect(bytes).toBeGreaterThan(1024 * 1024); // >1MB + } + expect(err).not.toContain("panic"); + }, + }); +}); diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts index 7693768ad2..3291f4a2b0 100644 --- a/test/cli/install/dummy.registry.ts +++ b/test/cli/install/dummy.registry.ts @@ -36,7 +36,12 @@ export function read(path: string) { return Bun.file(join(package_dir, path)); } -export function dummyRegistry(urls: string[], info: any = { "0.0.2": {} }, numberOfTimesTo500PerURL = 0) { +export function dummyRegistry( + urls: string[], + info: any = { "0.0.2": {} }, + numberOfTimesTo500PerURL = 0, + tgzDir?: string, +) { let retryCountsByURL = new Map(); const _handler: Handler = async request => { urls.push(request.url); @@ -57,7 +62,8 @@ export function dummyRegistry(urls: string[], info: any = { "0.0.2": {} }, numbe expect(request.method).toBe("GET"); if (url.endsWith(".tgz")) { - return new Response(file(join(import.meta.dir, basename(url).toLowerCase())), { status }); + const tgzPath = join(tgzDir ?? import.meta.dir, basename(url).toLowerCase()); + return new Response(file(tgzPath), { status }); } expect(request.headers.get("accept")).toBe( "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*",