From c94786ded747d46ab8e76f3949f361ea691abd68 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:58:40 +0000 Subject: [PATCH 1/2] fix(eslint-factory): recognize .addListener alias and req.on(\"response\", cb) idiom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit require-http-response-error-listener (the only rule at "error" severity) missed two standard Node.js EventEmitter/HTTP idioms: - .addListener("error", ...) is a documented alias for .on(), but was not recognized as satisfying the response-error-listener check. - req.on("response", cb) — registering the response listener separately from the inline request callback — caused getResponseCallback() to return null, skipping the check entirely for that call site. Both idioms are now detected: isErrorListenerCall() accepts "on"/"once"/"addListener", and a new getResponseEventCallback() helper recognizes the req.on("response", cb)/.once(...)/.addListener(...) idiom (including the chained http.request(...).on("response", cb) form), scoped to requests statically resolved to Node's http/https module. Re-verified against the 4 known live call sites in actions/setup/js (start_mcp_gateway.cjs, mount_mcp_as_cli.cjs, mcp_cli_bridge.cjs, handle_agent_failure.cjs) — all remain true negatives (npm run lint:setup-js still reports the rule at 0 errors). Fixes #59646 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...quire-http-response-error-listener.test.ts | 30 +++++ .../require-http-response-error-listener.ts | 107 ++++++++++++++---- 2 files changed, 112 insertions(+), 25 deletions(-) diff --git a/eslint-factory/src/rules/require-http-response-error-listener.test.ts b/eslint-factory/src/rules/require-http-response-error-listener.test.ts index 7935bb2586b..f5cda9e6123 100644 --- a/eslint-factory/src/rules/require-http-response-error-listener.test.ts +++ b/eslint-factory/src/rules/require-http-response-error-listener.test.ts @@ -98,4 +98,34 @@ describe("require-http-response-error-listener", () => { invalid: [], }); }); + + it("valid: '.addListener(\"error\", ...)' is treated as an 'error' listener", () => { + cjsRuleTester.run("require-http-response-error-listener", requireHttpResponseErrorListenerRule, { + valid: [ + `const http = require("http"); http.request(options, res => { res.on("data", () => {}); res.addListener("error", reject); });`, + `const https = require("https"); https.get(url, res => { res.addListener("error", err => { reject(err); }); res.on("end", () => {}); });`, + ], + invalid: [], + }); + }); + + it("invalid: the 'req.on(\"response\", cb)' idiom is checked for a response 'error' listener", () => { + cjsRuleTester.run("require-http-response-error-listener", requireHttpResponseErrorListenerRule, { + valid: [ + `const http = require("http"); const req = http.request(options); req.on("response", res => { res.on("data", () => {}); res.on("error", reject); }); req.on("error", reject);`, + `const https = require("https"); const req = https.get(url); req.once("response", res => { res.on("error", reject); }); req.on("error", reject);`, + `const http = require("http"); http.request(options).on("response", res => { res.on("error", reject); }).on("error", reject);`, + ], + invalid: [ + { + code: `const http = require("http"); const req = http.request(options); req.on("response", res => { let data = ""; res.on("data", chunk => { data += chunk; }); }); req.on("error", reject);`, + errors: [{ messageId: "missingResponseErrorListener" }], + }, + { + code: `const http = require("http"); http.request(options).on("response", res => { res.resume(); }).on("error", reject);`, + errors: [{ messageId: "missingResponseErrorListener" }], + }, + ], + }); + }); }); diff --git a/eslint-factory/src/rules/require-http-response-error-listener.ts b/eslint-factory/src/rules/require-http-response-error-listener.ts index e3618da0446..a9d4487316c 100644 --- a/eslint-factory/src/rules/require-http-response-error-listener.ts +++ b/eslint-factory/src/rules/require-http-response-error-listener.ts @@ -79,6 +79,9 @@ function isHttpRequestCall(call: TSESTree.CallExpression, sourceCode: TSESLint.S type ResponseCallback = TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; +// Node's EventEmitter treats "on" and "addListener" as synonyms, and "once" for a one-shot variant. +const LISTENER_METHODS = new Set(["on", "once", "addListener"]); + /** Returns the response callback argument of an http request call, when it has a single named response parameter. */ function getResponseCallback(call: TSESTree.CallExpression): ResponseCallback | null { for (const arg of call.arguments) { @@ -90,16 +93,59 @@ function getResponseCallback(call: TSESTree.CallExpression): ResponseCallback | return null; } -/** Returns true when `call` is `.on("error", ...)` / `.once("error", ...)`. */ +/** Returns true when `call` is `.on("error", ...)` / `.once("error", ...)` / `.addListener("error", ...)`. */ function isErrorListenerCall(call: TSESTree.CallExpression, name: string): boolean { const callee = call.callee; if (callee.type !== AST_NODE_TYPES.MemberExpression || callee.computed) return false; if (callee.object.type !== AST_NODE_TYPES.Identifier || callee.object.name !== name) return false; - if (callee.property.type !== AST_NODE_TYPES.Identifier || (callee.property.name !== "on" && callee.property.name !== "once")) return false; + if (callee.property.type !== AST_NODE_TYPES.Identifier || !LISTENER_METHODS.has(callee.property.name)) return false; const firstArg = call.arguments[0]; return firstArg !== undefined && firstArg.type === AST_NODE_TYPES.Literal && firstArg.value === "error"; } +/** + * Returns true when `name` (declared at `scopeNode`) is bound, via a single variable declarator, + * to the direct result of an http/https `request()`/`get()` call — e.g. `const req = http.request(...)`. + */ +function isHttpRequestResultBinding(name: string, scopeNode: TSESTree.Node, sourceCode: TSESLint.SourceCode): boolean { + const variable = resolveVariable(name, scopeNode, sourceCode); + if (!variable || variable.defs.length !== 1) return false; + const def = variable.defs[0]; + if (def.type !== "Variable") return false; + const declarator = def.node as TSESTree.VariableDeclarator; + if (declarator.id.type !== AST_NODE_TYPES.Identifier) return false; + return declarator.init !== null && declarator.init !== undefined && declarator.init.type === AST_NODE_TYPES.CallExpression && isHttpRequestCall(declarator.init, sourceCode); +} + +/** + * Returns the callback attached via the `req.on("response", cb)` idiom (or `.once`/`.addListener` + * variant, including the chained `http.request(...).on("response", cb)` form), when `req` resolves + * to the direct result of an http/https `request()`/`get()` call and `cb` has a single named + * response parameter. Returns null otherwise. + */ +function getResponseEventCallback(call: TSESTree.CallExpression, sourceCode: TSESLint.SourceCode): ResponseCallback | null { + const callee = call.callee; + if (callee.type !== AST_NODE_TYPES.MemberExpression || callee.computed) return null; + if (callee.property.type !== AST_NODE_TYPES.Identifier || !LISTENER_METHODS.has(callee.property.name)) return null; + const firstArg = call.arguments[0]; + if (!firstArg || firstArg.type !== AST_NODE_TYPES.Literal || firstArg.value !== "response") return null; + + const object = callee.object; + let isFromHttpRequest = false; + if (object.type === AST_NODE_TYPES.Identifier) { + isFromHttpRequest = isHttpRequestResultBinding(object.name, object, sourceCode); + } else if (object.type === AST_NODE_TYPES.CallExpression) { + isFromHttpRequest = isHttpRequestCall(object, sourceCode); + } + if (!isFromHttpRequest) return null; + + const cb = call.arguments[1]; + if (!cb || (cb.type !== AST_NODE_TYPES.FunctionExpression && cb.type !== AST_NODE_TYPES.ArrowFunctionExpression)) return null; + const firstParam = cb.params[0]; + if (!firstParam || firstParam.type !== AST_NODE_TYPES.Identifier) return null; + return cb; +} + export const requireHttpResponseErrorListenerRule = createRule({ name: "require-http-response-error-listener", meta: { @@ -109,7 +155,9 @@ export const requireHttpResponseErrorListenerRule = createRule({ "Require an 'error' event listener on the response object passed to http.request()/http.get()/https.request()/https.get() callbacks. " + "Node emits 'error' on the IncomingMessage itself for socket-level failures that occur while the body is streamed " + "(reset connections, decompression failures, aborted sockets); a listener on the request does not catch these, " + - "so an unhandled response 'error' event crashes the action. " + + "so an unhandled response 'error' event crashes the action. Also recognizes the `req.on(\"response\", cb)` idiom " + + "(listening for the response event on the request object returned by request()/get(), rather than an inline callback) " + + "and treats `.addListener(\"error\", ...)` as equivalent to `.on(\"error\", ...)` since it is a documented EventEmitter alias. " + 'Scope: only fires when the http/https module identifier is statically resolved through a `require("http")`-style binding.', }, schema: [], @@ -123,31 +171,40 @@ export const requireHttpResponseErrorListenerRule = createRule({ create(context) { const sourceCode = context.sourceCode; + /** Reports `callback` when its declared response parameter never gets an 'error' listener attached. */ + function checkResponseCallback(callback: ResponseCallback) { + const param = callback.params[0]; + if (!param || param.type !== AST_NODE_TYPES.Identifier) return; + const responseName = param.name; + + const variable = sourceCode.getDeclaredVariables(callback).find(candidate => candidate.name === responseName); + if (!variable) return; + + const hasErrorListener = variable.references.some(ref => { + const id = ref.identifier; + const parent = id.parent; + if (!parent || parent.type !== AST_NODE_TYPES.MemberExpression || parent.object !== id) return false; + const grandparent = parent.parent; + return grandparent !== undefined && grandparent.type === AST_NODE_TYPES.CallExpression && grandparent.callee === parent && isErrorListenerCall(grandparent, responseName); + }); + + if (!hasErrorListener) { + context.report({ node: param, messageId: "missingResponseErrorListener" }); + } + } + return { CallExpression(node: TSESTree.CallExpression) { - if (!isHttpRequestCall(node, sourceCode)) return; - - const callback = getResponseCallback(node); - if (!callback) return; - - const param = callback.params[0]; - if (!param || param.type !== AST_NODE_TYPES.Identifier) return; - const responseName = param.name; - - const variable = sourceCode.getDeclaredVariables(callback).find(candidate => candidate.name === responseName); - if (!variable) return; - - const hasErrorListener = variable.references.some(ref => { - const id = ref.identifier; - const parent = id.parent; - if (!parent || parent.type !== AST_NODE_TYPES.MemberExpression || parent.object !== id) return false; - const grandparent = parent.parent; - return grandparent !== undefined && grandparent.type === AST_NODE_TYPES.CallExpression && grandparent.callee === parent && isErrorListenerCall(grandparent, responseName); - }); - - if (!hasErrorListener) { - context.report({ node: param, messageId: "missingResponseErrorListener" }); + // Inline callback form: http.request(options, res => { ... }) + if (isHttpRequestCall(node, sourceCode)) { + const callback = getResponseCallback(node); + if (callback) checkResponseCallback(callback); } + + // Separate-listener idiom: req.on("response", res => { ... }) / .once(...) / .addListener(...), + // including the chained http.request(...).on("response", cb) form. + const responseCallback = getResponseEventCallback(node, sourceCode); + if (responseCallback) checkResponseCallback(responseCallback); }, }; }, From 314f50cb836e6d1644f65da1c65efcdeb7b9e24e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Sep 2026 10:37:19 +0000 Subject: [PATCH 2/2] fix(eslint-factory): validate all writes to request bindings before treating them as HTTP requests Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...quire-http-response-error-listener.test.ts | 10 ++++++++++ .../require-http-response-error-listener.ts | 20 ++++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/eslint-factory/src/rules/require-http-response-error-listener.test.ts b/eslint-factory/src/rules/require-http-response-error-listener.test.ts index f5cda9e6123..d2c72f77f7d 100644 --- a/eslint-factory/src/rules/require-http-response-error-listener.test.ts +++ b/eslint-factory/src/rules/require-http-response-error-listener.test.ts @@ -109,6 +109,16 @@ describe("require-http-response-error-listener", () => { }); }); + it("valid: request bindings reassigned to an unrelated object are ignored", () => { + cjsRuleTester.run("require-http-response-error-listener", requireHttpResponseErrorListenerRule, { + valid: [ + `const http = require("http"); let req = http.request(options); req = makeClient(); req.on("response", res => { res.resume(); });`, + `const http = require("http"); let req = makeClient(); req = http.request(options); req.on("response", res => { res.resume(); });`, + ], + invalid: [], + }); + }); + it("invalid: the 'req.on(\"response\", cb)' idiom is checked for a response 'error' listener", () => { cjsRuleTester.run("require-http-response-error-listener", requireHttpResponseErrorListenerRule, { valid: [ diff --git a/eslint-factory/src/rules/require-http-response-error-listener.ts b/eslint-factory/src/rules/require-http-response-error-listener.ts index a9d4487316c..f0b2d76da9d 100644 --- a/eslint-factory/src/rules/require-http-response-error-listener.ts +++ b/eslint-factory/src/rules/require-http-response-error-listener.ts @@ -103,9 +103,17 @@ function isErrorListenerCall(call: TSESTree.CallExpression, name: string): boole return firstArg !== undefined && firstArg.type === AST_NODE_TYPES.Literal && firstArg.value === "error"; } +/** Returns true when `node` is a direct `.request(...)` / `.get(...)` call expression. */ +function isHttpRequestResultExpression(node: TSESTree.Node | null | undefined, sourceCode: TSESLint.SourceCode): boolean { + if (!node) return false; + return node.type === AST_NODE_TYPES.CallExpression && isHttpRequestCall(node, sourceCode); +} + /** * Returns true when `name` (declared at `scopeNode`) is bound, via a single variable declarator, - * to the direct result of an http/https `request()`/`get()` call — e.g. `const req = http.request(...)`. + * to the direct result of an http/https `request()`/`get()` call — e.g. `const req = http.request(...)` — + * and every write to the binding keeps it holding such a request, so a reassigned variable pointing at an + * unrelated object is never treated as a Node request. */ function isHttpRequestResultBinding(name: string, scopeNode: TSESTree.Node, sourceCode: TSESLint.SourceCode): boolean { const variable = resolveVariable(name, scopeNode, sourceCode); @@ -114,7 +122,13 @@ function isHttpRequestResultBinding(name: string, scopeNode: TSESTree.Node, sour if (def.type !== "Variable") return false; const declarator = def.node as TSESTree.VariableDeclarator; if (declarator.id.type !== AST_NODE_TYPES.Identifier) return false; - return declarator.init !== null && declarator.init !== undefined && declarator.init.type === AST_NODE_TYPES.CallExpression && isHttpRequestCall(declarator.init, sourceCode); + if (!isHttpRequestResultExpression(declarator.init, sourceCode)) return false; + // Any write other than another http request call means the binding may no longer denote a request. + for (const reference of variable.references) { + if (!reference.isWrite()) continue; + if (!isHttpRequestResultExpression(reference.writeExpr, sourceCode)) return false; + } + return true; } /** @@ -157,7 +171,7 @@ export const requireHttpResponseErrorListenerRule = createRule({ "(reset connections, decompression failures, aborted sockets); a listener on the request does not catch these, " + "so an unhandled response 'error' event crashes the action. Also recognizes the `req.on(\"response\", cb)` idiom " + "(listening for the response event on the request object returned by request()/get(), rather than an inline callback) " + - "and treats `.addListener(\"error\", ...)` as equivalent to `.on(\"error\", ...)` since it is a documented EventEmitter alias. " + + 'and treats `.addListener("error", ...)` as equivalent to `.on("error", ...)` since it is a documented EventEmitter alias. ' + 'Scope: only fires when the http/https module identifier is statically resolved through a `require("http")`-style binding.', }, schema: [],