From 5d82364e150cc43640848330c8a04e854cc847e1 Mon Sep 17 00:00:00 2001 From: Hessabi Date: Fri, 23 Oct 2026 11:28:09 -0500 Subject: [PATCH 1/2] Don't crash reading stdin when it is not a pipe or file readStdinIfPiped called fs.readFileSync(0) whenever stdin was not a TTY. Under Claude Code's Bash tool (used by the devin-rescue subagent), fd 0 is a non-blocking handle with no data, so the read threw EAGAIN and the task command failed before Devin ran. Now fd 0 is read only when fstat says it is a FIFO or regular file, and EAGAIN/ENXIO are treated as empty input. Dependencies are injectable for unit tests, and an end-to-end test reads from a real non-blocking FIFO with an open, silent writer (tests/fs.test.mjs). --- plugins/devin/scripts/lib/fs.mjs | 25 ++++- tests/fs.test.mjs | 166 +++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 tests/fs.test.mjs diff --git a/plugins/devin/scripts/lib/fs.mjs b/plugins/devin/scripts/lib/fs.mjs index 237eb01..236863e 100644 --- a/plugins/devin/scripts/lib/fs.mjs +++ b/plugins/devin/scripts/lib/fs.mjs @@ -34,9 +34,28 @@ export function isProbablyText(buffer) { return true; } -export function readStdinIfPiped() { - if (process.stdin.isTTY) { +export function readStdinIfPiped(deps = {}) { + const { fstatImpl = fs.fstatSync, readImpl = fs.readFileSync, isTTY = process.stdin.isTTY } = deps; + if (isTTY) { return ""; } - return fs.readFileSync(0, "utf8"); + try { + const stats = fstatImpl(0); + if (!stats.isFIFO() && !stats.isFile()) { + return ""; + } + } catch (e) { + if (e.code === "EAGAIN" || e.code === "ENXIO") { + return ""; + } + throw e; + } + try { + return readImpl(0, "utf8"); + } catch (e) { + if (e.code === "EAGAIN" || e.code === "ENXIO") { + return ""; + } + throw e; + } } diff --git a/tests/fs.test.mjs b/tests/fs.test.mjs new file mode 100644 index 0000000..703881a --- /dev/null +++ b/tests/fs.test.mjs @@ -0,0 +1,166 @@ +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { readStdinIfPiped } from "../plugins/devin/scripts/lib/fs.mjs"; + +const FS_MODULE = fileURLToPath(new URL("../plugins/devin/scripts/lib/fs.mjs", import.meta.url)); + +test("readStdinIfPiped returns empty string when isTTY is true", () => { + const result = readStdinIfPiped({ isTTY: true }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped reads content from FIFO", () => { + const mockStats = { + isFIFO: () => true, + isFile: () => false + }; + const mockRead = () => "hello from pipe"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: mockRead + }); + assert.equal(result, "hello from pipe"); +}); + +test("readStdinIfPiped reads content from regular file", () => { + const mockStats = { + isFIFO: () => false, + isFile: () => true + }; + const mockRead = () => "hello from file"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: mockRead + }); + assert.equal(result, "hello from file"); +}); + +test("readStdinIfPiped returns empty string for non-FIFO/non-file (socket)", () => { + const mockStats = { + isFIFO: () => false, + isFile: () => false + }; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: () => "should not be called" + }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped returns empty string when fstat throws EAGAIN", () => { + const error = new Error("EAGAIN: resource temporarily unavailable"); + error.code = "EAGAIN"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => { throw error; }, + readImpl: () => "should not be called" + }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped returns empty string when fstat throws ENXIO", () => { + const error = new Error("ENXIO: no such device or address"); + error.code = "ENXIO"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => { throw error; }, + readImpl: () => "should not be called" + }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped returns empty string when read throws EAGAIN", () => { + const mockStats = { + isFIFO: () => true, + isFile: () => false + }; + const error = new Error("EAGAIN: resource temporarily unavailable"); + error.code = "EAGAIN"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: () => { throw error; } + }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped returns empty string when read throws ENXIO", () => { + const mockStats = { + isFIFO: () => true, + isFile: () => false + }; + const error = new Error("ENXIO: no such device or address"); + error.code = "ENXIO"; + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: () => { throw error; } + }); + assert.equal(result, ""); +}); + +test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from fstat", () => { + const error = new Error("EPERM: operation not permitted"); + error.code = "EPERM"; + assert.throws( + () => readStdinIfPiped({ + isTTY: false, + fstatImpl: () => { throw error; }, + readImpl: () => "should not be called" + }), + { code: "EPERM" } + ); +}); + +test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from read", () => { + const mockStats = { + isFIFO: () => true, + isFile: () => false + }; + const error = new Error("EACCES: permission denied"); + error.code = "EACCES"; + assert.throws( + () => readStdinIfPiped({ + isTTY: false, + fstatImpl: () => mockStats, + readImpl: () => { throw error; } + }), + { code: "EACCES" } + ); +}); + +// End-to-end repro without mocks: fd 0 is the read end of a FIFO whose +// O_NONBLOCK flag is set and whose writer is open but silent, which is what +// the Claude Code Bash tool hands to child processes. +test("readStdinIfPiped returns empty string for a non-blocking FIFO with no data", { skip: process.platform === "win32" }, () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "devin-stdin-")); + const fifo = path.join(dir, "stdin"); + let readFd; + let writeFd; + try { + execFileSync("mkfifo", [fifo]); + readFd = fs.openSync(fifo, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); + writeFd = fs.openSync(fifo, fs.constants.O_WRONLY | fs.constants.O_NONBLOCK); + const script = `import(${JSON.stringify(FS_MODULE)}).then((m) => process.stdout.write(JSON.stringify(m.readStdinIfPiped())));`; + const result = spawnSync(process.execPath, ["-e", script], { + stdio: [readFd, "pipe", "pipe"], + encoding: "utf8", + timeout: 10000 + }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout, '""'); + } finally { + if (readFd !== undefined) fs.closeSync(readFd); + if (writeFd !== undefined) fs.closeSync(writeFd); + fs.rmSync(dir, { recursive: true, force: true }); + } +}); From 0e323f58473d3eb4e35d5b58cf91c78a65d9ac81 Mon Sep 17 00:00:00 2001 From: Hessabi Date: Sat, 24 Oct 2026 16:05:03 -0500 Subject: [PATCH 2/2] Keep partial stdin reads and bound input size Read stdin with an incremental readSync loop so bytes already received from a non-blocking FIFO survive EAGAIN, decode once at the end, and throw if input exceeds an 8 MiB cap instead of truncating. Add real FIFO tests for pending data, multi-byte UTF-8 and multiple chunks. --- plugins/devin/scripts/lib/fs.mjs | 51 +++++-- tests/fs.test.mjs | 234 ++++++++++++++++++++----------- 2 files changed, 195 insertions(+), 90 deletions(-) diff --git a/plugins/devin/scripts/lib/fs.mjs b/plugins/devin/scripts/lib/fs.mjs index 236863e..e4267a7 100644 --- a/plugins/devin/scripts/lib/fs.mjs +++ b/plugins/devin/scripts/lib/fs.mjs @@ -34,8 +34,23 @@ export function isProbablyText(buffer) { return true; } +// Prompts are text typed or piped by a person or a script; 8 MiB is far above any +// realistic prompt and still small enough to hold in memory safely. +export const MAX_STDIN_BYTES = 8 * 1024 * 1024; +const STDIN_CHUNK_BYTES = 64 * 1024; + +function isNoDataError(e) { + return e.code === "EAGAIN" || e.code === "ENXIO"; +} + export function readStdinIfPiped(deps = {}) { - const { fstatImpl = fs.fstatSync, readImpl = fs.readFileSync, isTTY = process.stdin.isTTY } = deps; + const { + fstatImpl = fs.fstatSync, + readSyncImpl = fs.readSync, + isTTY = process.stdin.isTTY, + chunkSize = STDIN_CHUNK_BYTES, + maxBytes = MAX_STDIN_BYTES + } = deps; if (isTTY) { return ""; } @@ -45,17 +60,37 @@ export function readStdinIfPiped(deps = {}) { return ""; } } catch (e) { - if (e.code === "EAGAIN" || e.code === "ENXIO") { + if (isNoDataError(e)) { return ""; } throw e; } - try { - return readImpl(0, "utf8"); - } catch (e) { - if (e.code === "EAGAIN" || e.code === "ENXIO") { - return ""; + // Read incrementally so bytes already received survive an EAGAIN from a + // non-blocking FIFO whose writer is still open. + const chunks = []; + let total = 0; + for (;;) { + // Ask for one byte past the cap so overflow is detected without truncating. + const length = Math.min(chunkSize, maxBytes + 1 - total); + const buffer = Buffer.alloc(length); + let bytesRead; + try { + bytesRead = readSyncImpl(0, buffer, 0, length, null); + } catch (e) { + if (isNoDataError(e)) { + break; + } + throw e; } - throw e; + if (bytesRead === 0) { + break; + } + total += bytesRead; + if (total > maxBytes) { + throw new Error(`stdin input exceeds the ${maxBytes} byte limit.`); + } + chunks.push(buffer.subarray(0, bytesRead)); } + // Decode once so multi-byte characters split across chunks stay intact. + return Buffer.concat(chunks).toString("utf8"); } diff --git a/tests/fs.test.mjs b/tests/fs.test.mjs index 703881a..fe4268b 100644 --- a/tests/fs.test.mjs +++ b/tests/fs.test.mjs @@ -6,7 +6,7 @@ import path from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { readStdinIfPiped } from "../plugins/devin/scripts/lib/fs.mjs"; +import { MAX_STDIN_BYTES, readStdinIfPiped } from "../plugins/devin/scripts/lib/fs.mjs"; const FS_MODULE = fileURLToPath(new URL("../plugins/devin/scripts/lib/fs.mjs", import.meta.url)); @@ -15,133 +15,177 @@ test("readStdinIfPiped returns empty string when isTTY is true", () => { assert.equal(result, ""); }); -test("readStdinIfPiped reads content from FIFO", () => { - const mockStats = { - isFIFO: () => true, - isFile: () => false +const FIFO_STATS = { isFIFO: () => true, isFile: () => false }; +const FILE_STATS = { isFIFO: () => false, isFile: () => true }; + +function errnoError(code, message) { + const error = new Error(`${code}: ${message}`); + error.code = code; + return error; +} + +// Fake readSync that serves the given steps in order: a Buffer or string is +// copied into the caller's buffer (at most `length` bytes, the rest is kept for +// the next call), an Error is thrown, and an exhausted script reports EOF. +function scriptedRead(steps) { + const queue = steps.map((step) => (typeof step === "string" ? Buffer.from(step) : step)); + return (fd, buffer, offset, length) => { + assert.equal(fd, 0); + if (queue.length === 0) { + return 0; + } + const step = queue.shift(); + if (step instanceof Error) { + throw step; + } + const n = Math.min(length, step.length); + step.copy(buffer, offset, 0, n); + if (n < step.length) { + queue.unshift(step.subarray(n)); + } + return n; }; - const mockRead = () => "hello from pipe"; +} + +test("readStdinIfPiped reads content from FIFO", () => { const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => mockStats, - readImpl: mockRead + fstatImpl: () => FIFO_STATS, + readSyncImpl: scriptedRead(["hello from pipe"]) }); assert.equal(result, "hello from pipe"); }); test("readStdinIfPiped reads content from regular file", () => { - const mockStats = { - isFIFO: () => false, - isFile: () => true - }; - const mockRead = () => "hello from file"; const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => mockStats, - readImpl: mockRead + fstatImpl: () => FILE_STATS, + readSyncImpl: scriptedRead(["hello from file"]) }); assert.equal(result, "hello from file"); }); test("readStdinIfPiped returns empty string for non-FIFO/non-file (socket)", () => { - const mockStats = { - isFIFO: () => false, - isFile: () => false - }; const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => mockStats, - readImpl: () => "should not be called" + fstatImpl: () => ({ isFIFO: () => false, isFile: () => false }), + readSyncImpl: () => { throw new Error("should not be called"); } }); assert.equal(result, ""); }); -test("readStdinIfPiped returns empty string when fstat throws EAGAIN", () => { - const error = new Error("EAGAIN: resource temporarily unavailable"); - error.code = "EAGAIN"; - const result = readStdinIfPiped({ - isTTY: false, - fstatImpl: () => { throw error; }, - readImpl: () => "should not be called" +for (const code of ["EAGAIN", "ENXIO"]) { + test(`readStdinIfPiped returns empty string when fstat throws ${code}`, () => { + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => { throw errnoError(code, "no data"); }, + readSyncImpl: () => { throw new Error("should not be called"); } + }); + assert.equal(result, ""); }); - assert.equal(result, ""); + + test(`readStdinIfPiped returns empty string when the first read throws ${code}`, () => { + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => FIFO_STATS, + readSyncImpl: scriptedRead([errnoError(code, "no data")]) + }); + assert.equal(result, ""); + }); + + test(`readStdinIfPiped keeps bytes already read when a later read throws ${code}`, () => { + const result = readStdinIfPiped({ + isTTY: false, + fstatImpl: () => FIFO_STATS, + readSyncImpl: scriptedRead(["PROMPT ", "DATA\n", errnoError(code, "no more data yet")]) + }); + assert.equal(result, "PROMPT DATA\n"); + }); +} + +test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from fstat", () => { + assert.throws( + () => readStdinIfPiped({ + isTTY: false, + fstatImpl: () => { throw errnoError("EPERM", "operation not permitted"); }, + readSyncImpl: () => { throw new Error("should not be called"); } + }), + { code: "EPERM" } + ); +}); + +test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from read, even after data", () => { + assert.throws( + () => readStdinIfPiped({ + isTTY: false, + fstatImpl: () => FIFO_STATS, + readSyncImpl: scriptedRead(["partial", errnoError("EACCES", "permission denied")]) + }), + { code: "EACCES" } + ); }); -test("readStdinIfPiped returns empty string when fstat throws ENXIO", () => { - const error = new Error("ENXIO: no such device or address"); - error.code = "ENXIO"; +test("readStdinIfPiped joins multiple chunks and honors chunkSize", () => { + const lengths = []; + const inner = scriptedRead(["0123456789abcdefghij"]); const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => { throw error; }, - readImpl: () => "should not be called" + fstatImpl: () => FIFO_STATS, + chunkSize: 6, + readSyncImpl: (fd, buffer, offset, length, position) => { + lengths.push(length); + return inner(fd, buffer, offset, length, position); + } }); - assert.equal(result, ""); + assert.equal(result, "0123456789abcdefghij"); + assert.ok(lengths.every((length) => length <= 6), `unexpected read sizes: ${lengths}`); + assert.ok(lengths.length >= 4); }); -test("readStdinIfPiped returns empty string when read throws EAGAIN", () => { - const mockStats = { - isFIFO: () => true, - isFile: () => false - }; - const error = new Error("EAGAIN: resource temporarily unavailable"); - error.code = "EAGAIN"; +test("readStdinIfPiped decodes multi-byte UTF-8 split across chunks", () => { + const text = "h\u00e9llo \u2713 \u{1F600}\n"; const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => mockStats, - readImpl: () => { throw error; } + fstatImpl: () => FIFO_STATS, + chunkSize: 1, + readSyncImpl: scriptedRead([text]) }); - assert.equal(result, ""); + assert.equal(result, text); }); -test("readStdinIfPiped returns empty string when read throws ENXIO", () => { - const mockStats = { - isFIFO: () => true, - isFile: () => false - }; - const error = new Error("ENXIO: no such device or address"); - error.code = "ENXIO"; +test("readStdinIfPiped accepts input of exactly maxBytes", () => { const result = readStdinIfPiped({ isTTY: false, - fstatImpl: () => mockStats, - readImpl: () => { throw error; } + fstatImpl: () => FIFO_STATS, + chunkSize: 4, + maxBytes: 10, + readSyncImpl: scriptedRead(["0123456789"]) }); - assert.equal(result, ""); + assert.equal(result, "0123456789"); }); -test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from fstat", () => { - const error = new Error("EPERM: operation not permitted"); - error.code = "EPERM"; +test("readStdinIfPiped throws a clear error when input exceeds maxBytes", () => { assert.throws( () => readStdinIfPiped({ isTTY: false, - fstatImpl: () => { throw error; }, - readImpl: () => "should not be called" + fstatImpl: () => FIFO_STATS, + chunkSize: 4, + maxBytes: 10, + readSyncImpl: scriptedRead(["0123456789X"]) }), - { code: "EPERM" } + /stdin input exceeds the 10 byte limit/ ); }); -test("readStdinIfPiped throws non-EAGAIN/ENXIO errors from read", () => { - const mockStats = { - isFIFO: () => true, - isFile: () => false - }; - const error = new Error("EACCES: permission denied"); - error.code = "EACCES"; - assert.throws( - () => readStdinIfPiped({ - isTTY: false, - fstatImpl: () => mockStats, - readImpl: () => { throw error; } - }), - { code: "EACCES" } - ); +test("MAX_STDIN_BYTES is a positive default cap", () => { + assert.ok(Number.isInteger(MAX_STDIN_BYTES) && MAX_STDIN_BYTES >= 1024 * 1024); }); -// End-to-end repro without mocks: fd 0 is the read end of a FIFO whose -// O_NONBLOCK flag is set and whose writer is open but silent, which is what -// the Claude Code Bash tool hands to child processes. -test("readStdinIfPiped returns empty string for a non-blocking FIFO with no data", { skip: process.platform === "win32" }, () => { +// End-to-end tests without mocks: fd 0 is the read end of a FIFO whose +// O_NONBLOCK flag is set and whose writer stays open, which is what the Claude +// Code Bash tool hands to child processes. `payload` is written before the +// child starts; `options` is the argument passed to readStdinIfPiped. +function readFromOpenFifo(payload, options = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "devin-stdin-")); const fifo = path.join(dir, "stdin"); let readFd; @@ -150,17 +194,43 @@ test("readStdinIfPiped returns empty string for a non-blocking FIFO with no data execFileSync("mkfifo", [fifo]); readFd = fs.openSync(fifo, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); writeFd = fs.openSync(fifo, fs.constants.O_WRONLY | fs.constants.O_NONBLOCK); - const script = `import(${JSON.stringify(FS_MODULE)}).then((m) => process.stdout.write(JSON.stringify(m.readStdinIfPiped())));`; + if (payload !== undefined) { + const bytes = Buffer.from(payload, "utf8"); + assert.equal(fs.writeSync(writeFd, bytes), bytes.length); + } + const script = `import(${JSON.stringify(FS_MODULE)}).then((m) => process.stdout.write(JSON.stringify(m.readStdinIfPiped(${JSON.stringify(options)}))));`; const result = spawnSync(process.execPath, ["-e", script], { stdio: [readFd, "pipe", "pipe"], encoding: "utf8", timeout: 10000 }); assert.equal(result.status, 0, result.stderr); - assert.equal(result.stdout, '""'); + return JSON.parse(result.stdout); } finally { if (readFd !== undefined) fs.closeSync(readFd); if (writeFd !== undefined) fs.closeSync(writeFd); fs.rmSync(dir, { recursive: true, force: true }); } +} + +const skipWin32 = { skip: process.platform === "win32" }; + +test("readStdinIfPiped returns empty string for a non-blocking FIFO with no data", skipWin32, () => { + assert.equal(readFromOpenFifo(undefined), ""); +}); + +test("readStdinIfPiped returns bytes already in a non-blocking FIFO whose writer is still open", skipWin32, () => { + assert.equal(readFromOpenFifo("PROMPT DATA\n"), "PROMPT DATA\n"); +}); + +test("readStdinIfPiped returns multi-byte UTF-8 from an open non-blocking FIFO", skipWin32, () => { + assert.equal(readFromOpenFifo("h\u00e9llo \u2713\n"), "h\u00e9llo \u2713\n"); +}); + +test("readStdinIfPiped reads a payload spanning several chunks from an open non-blocking FIFO", skipWin32, () => { + // 7-byte chunks split the 2- and 3-byte characters at awkward offsets; the + // payload stays far below the OS pipe buffer so the writer never hits EAGAIN. + const payload = `${"line of text \u2713 h\u00e9llo\n".repeat(8)}end\n`; + assert.ok(Buffer.byteLength(payload) > 150 && Buffer.byteLength(payload) < 1024); + assert.equal(readFromOpenFifo(payload, { chunkSize: 7 }), payload); });