Fixed issue where streams would not be tracked via chainable methods

Napalys · Napalys · commit ef1bde554a1e · 2025-05-21T11:40:35.000+02:00
diff --git a/javascript/ql/src/Quality/UnhandledStreamPipe.ql b/javascript/ql/src/Quality/UnhandledStreamPipe.ql
@@ -30,31 +30,40 @@ class PipeCall extends DataFlow::MethodCallNode {
  */
 string getEventHandlerMethodName() { result = ["on", "once", "addListener"] }
 
+/**
+ * Gets the method names that are chainable on Node.js streams.
+ */
+string getChainableStreamMethodName() {
+  result =
+    [
+      "setEncoding", "pause", "resume", "unpipe", "destroy", "cork", "uncork", "setDefaultEncoding",
+      "off", "removeListener", getEventHandlerMethodName()
+    ]
+}
+
 /**
  * A call to register an event handler on a Node.js stream.
  * This includes methods like `on`, `once`, and `addListener`.
  */
 class StreamEventRegistration extends DataFlow::MethodCallNode {
   StreamEventRegistration() { this.getMethodName() = getEventHandlerMethodName() }
-
-  /** Gets the stream (receiver of the event handler). */
-  DataFlow::Node getStream() { result = this.getReceiver() }
 }
 
 /**
  * Models flow relationships between streams and related operations.
  * Connects destination streams to their corresponding pipe call nodes.
- * Connects streams to their event handler registration nodes.
+ * Connects streams to their chainable methods.
  */
 predicate streamFlowStep(DataFlow::Node streamNode, DataFlow::Node relatedNode) {
   exists(PipeCall pipe |
     streamNode = pipe.getDestinationStream() and
     relatedNode = pipe
   )
   or
-  exists(StreamEventRegistration handler |
-    streamNode = handler.getStream() and
-    relatedNode = handler
+  exists(DataFlow::MethodCallNode chainable |
+    chainable.getMethodName() = getChainableStreamMethodName() and
+    streamNode = chainable.getReceiver() and
+    relatedNode = chainable
   )
 }
 
diff --git a/javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected b/javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected
@@ -8,7 +8,6 @@
 | test.js:109:26:109:37 | s.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:116:5:116:21 | stream.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:125:5:125:26 | getStre ... e(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
-| test.js:139:5:139:87 | stream. ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:143:5:143:62 | stream. ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:147:5:147:28 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
 | test.js:151:20:151:43 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
diff --git a/javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.js b/javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.js
@@ -136,7 +136,7 @@ function test() {
   }
   { // Long chained pipe with error handler
     const stream = getStream();
-    stream.pause().on('error', handleError).setEncoding('utf8').resume().pipe(writable); // $SPURIOUS:Alert
+    stream.pause().on('error', handleError).setEncoding('utf8').resume().pipe(writable);
   }
   { // Long chained pipe without error handler
     const stream = getStream();

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ function test() {`
`136`	`136`	`}`
`137`	`137`	`{ // Long chained pipe with error handler`
`138`	`138`	`const stream = getStream();`
`139`		`- stream.pause().on('error', handleError).setEncoding('utf8').resume().pipe(writable); // $SPURIOUS:Alert`
	`139`	`+ stream.pause().on('error', handleError).setEncoding('utf8').resume().pipe(writable);`
`140`	`140`	`}`
`141`	`141`	`{ // Long chained pipe without error handler`
`142`	`142`	`const stream = getStream();`