Merge pull request #22522 from github/jacknojo/enhance_hapi_coverage

Improve hapi coverage

Author: jacknojo

javascript/ql/lib/change-notes/2026-09-08-hapi-wrapped-routes.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* Improved Hapi route handler and request input tracking through custom route registration helpers and higher-order function wrappers.

javascript/ql/lib/semmle/javascript/dataflow/internal/FunctionWrapperSteps.qll

@@ -145,3 +145,41 @@ private module Cached {
 }
 
 import Cached
+
+/** Gets a value referring to a forwarding-wrapper around `func`. */
+private DataFlow::SourceNode trackWrappedFunction(
+  DataFlow::FunctionNode func, DataFlow::TypeTracker t
+) {
+  t.start() and
+  functionOneWayForwardingStep(func, result) and
+  // Exclude functions since ordinary call edges can handle that case.
+  // This will typically match calls to a "wrapper factory".
+  not result instanceof DataFlow::FunctionNode
+  or
+  functionOneWayForwardingStep(trackWrappedFunction(func, t.continue()), result)
+  or
+  exists(DataFlow::TypeTracker t2 | result = trackWrappedFunction(func, t2).track(t2, t))
+}
+
+/** Gets a value referring to a forwarding-wrapper around `func`. */
+DataFlow::SourceNode trackWrappedFunction(DataFlow::SourceNode func) {
+  result = trackWrappedFunction(func, DataFlow::TypeTracker::end())
+}
+
+/**
+ * Data flow into a concrete function invoked through a forwarding wrapper.
+ *
+ * Only arguments with a statically known position and a corresponding non-rest parameter are
+ * modeled.
+ */
+private class FunctionWrapperCallStep extends DataFlow::SharedFlowStep {
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(DataFlow::CallNode call, DataFlow::FunctionNode wrapped, int index |
+      call = trackWrappedFunction(wrapped).getACall() and
+      pred = call.getArgument(index) and
+      succ = wrapped.getParameter(index) and
+      // A rest parameter receives an array, not the argument at this index.
+      not succ.(DataFlow::ParameterNode).isRestParameter()
+    )
+  }
+}

javascript/ql/lib/semmle/javascript/frameworks/Hapi.qll

@@ -4,6 +4,7 @@
 
 import javascript
 import semmle.javascript.frameworks.HTTP
+private import semmle.javascript.dataflow.internal.CallGraphs
 
 module Hapi {
   /**
@@ -101,6 +102,19 @@ module Hapi {
     override RequestSource src;
   }
 
+  private DataFlow::SourceNode requestInputRef(
+    RouteHandler rh, string property, DataFlow::TypeTracker t
+  ) {
+    t.start() and
+    result = rh.getARequestSource().ref().getAPropertyRead(property)
+    or
+    exists(DataFlow::TypeTracker t2 | result = requestInputRef(rh, property, t2).track(t2, t))
+  }
+
+  private DataFlow::SourceNode requestInputRef(RouteHandler rh, string property) {
+    result = requestInputRef(rh, property, DataFlow::TypeTracker::end())
+  }
+
   /**
    * An access to a user-controlled Hapi request input.
    */
@@ -115,18 +129,18 @@ module Hapi {
           // `request.rawPayload`
           this.(DataFlow::PropRead).accesses(request, "rawPayload")
           or
-          exists(DataFlow::PropRead payload |
-            // `request.payload.name`
-            payload.accesses(request, "payload") and
-            this.(DataFlow::PropRead).accesses(payload, _)
-          )
+          // `request.payload` is an object, so prefer a property read if possible.
+          if exists(requestInputRef(rh, "payload").getAPropertyRead())
+          then this = requestInputRef(rh, "payload").getAPropertyRead()
+          else this = rh.getARequestSource().ref().getAPropertyRead("payload")
         )
         or
         kind = "parameter" and
-        exists(DataFlow::PropRead query |
-          // `request.query.name`
-          query.accesses(request, ["query", "params"]) and
-          this.(DataFlow::PropRead).accesses(query, _)
+        exists(string property | property = ["query", "params"] |
+          // These are objects, so prefer a property read if possible.
+          if exists(requestInputRef(rh, property).getAPropertyRead())
+          then this = requestInputRef(rh, property).getAPropertyRead()
+          else this = rh.getARequestSource().ref().getAPropertyRead(property)
         )
         or
         exists(DataFlow::PropRead url |
@@ -199,30 +213,10 @@ module Hapi {
    */
   class RouteSetup extends DataFlow::MethodCallNode, Http::Servers::StandardRouteSetup {
     ServerDefinition server;
-    DataFlow::Node handler;
 
     RouteSetup() {
       server.ref().getAMethodCall() = this and
-      (
-        // server.route({ handler: fun })
-        this.getMethodName() = "route" and
-        this.getOptionArgument(0, "handler") = handler
-        or
-        // server.ext('/', fun)
-        this.getMethodName() = "ext" and
-        handler = this.getArgument(1)
-        or
-        // server.route([{ handler(request){}])
-        this.getMethodName() = "route" and
-        handler =
-          this.getArgument(0)
-              .getALocalSource()
-              .(DataFlow::ArrayCreationNode)
-              .getAnElement()
-              .getALocalSource()
-              .getAPropertySource("handler")
-              .getAFunctionValue()
-      )
+      this.getMethodName() = ["route", "ext"]
     }
 
     override DataFlow::SourceNode getARouteHandler() {
@@ -233,11 +227,45 @@ module Hapi {
       t.start() and
       result = this.getRouteHandler().getALocalSource()
       or
-      exists(DataFlow::TypeBackTracker t2 | result = this.getARouteHandler(t2).backtrack(t2, t))
+      this.getMethodName() = "route" and
+      t.isInProp("handler") and
+      result = this.getArgument(0).getALocalSource()
+      or
+      exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ |
+        succ = this.getARouteHandler(t2)
+      |
+        result = succ.backtrack(t2, t)
+        or
+        Http::routeHandlerStep(result, succ) and
+        t = t2
+        or
+        DataFlow::SharedFlowStep::storeStep(result.getALocalUse(), succ,
+          DataFlow::PseudoProperties::arrayElement()) and
+        t = t2.continue()
+      )
     }
 
     pragma[noinline]
-    private DataFlow::Node getRouteHandler() { result = handler }
+    private DataFlow::Node getRouteHandler() {
+      // server.route({ handler: fun })
+      this.getMethodName() = "route" and
+      this.getOptionArgument(0, "handler") = result
+      or
+      // server.ext('/', fun)
+      this.getMethodName() = "ext" and
+      result = this.getArgument(1)
+      or
+      // server.route([{ handler(request){}])
+      this.getMethodName() = "route" and
+      result =
+        this.getArgument(0)
+            .getALocalSource()
+            .(DataFlow::ArrayCreationNode)
+            .getAnElement()
+            .getALocalSource()
+            .getAPropertySource("handler")
+            .getAFunctionValue()
+    }
 
     override DataFlow::Node getServer() { result = server }
   }
@@ -263,6 +291,56 @@ module Hapi {
     }
   }
 
+  private DataFlow::SourceNode routeDefinitionRef(
+    DataFlow::ObjectLiteralNode definition, DataFlow::TypeTracker t
+  ) {
+    t.start() and
+    result = definition
+    or
+    exists(DataFlow::TypeTracker t2 | result = routeDefinitionRef(definition, t2).track(t2, t))
+  }
+
+  private predicate handlerRegistration(
+    DataFlow::FunctionNode handler, DataFlow::ObjectLiteralNode definition
+  ) {
+    exists(
+      DataFlow::CallNode registration, DataFlow::FunctionNode registrar,
+      DataFlow::ParameterNode handlerParameter, DataFlow::SourceNode handlerRef, int index
+    |
+      registration.getACallee() = registrar.getFunction() and
+      handlerParameter = registrar.getParameter(index) and
+      handlerParameter.flowsTo(definition.getAPropertyWrite("handler").getRhs()) and
+      (
+        handlerRef = handler
+        or
+        handlerRef = CallGraph::callgraphStep(handler, DataFlow::TypeTracker::end())
+      ) and
+      handlerRef.flowsTo(registration.getArgument(index))
+    )
+  }
+
+  /** Data flow through handlers stored in route definitions by registration helpers. */
+  private class RegisteredHandlerCallStep extends DataFlow::SharedFlowStep {
+    DataFlow::CallNode call;
+    DataFlow::FunctionNode handler;
+
+    RegisteredHandlerCallStep() {
+      exists(DataFlow::ObjectLiteralNode definition, DataFlow::PropRead handlerRead |
+        handlerRegistration(handler, definition) and
+        handlerRead.getPropertyName() = "handler" and
+        routeDefinitionRef(definition, DataFlow::TypeTracker::end()).flowsTo(handlerRead.getBase()) and
+        call.getCalleeNode() = handlerRead
+      )
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(int index |
+        pred = call.getArgument(index) and
+        succ = handler.getParameter(index)
+      )
+    }
+  }
+
   /**
    * A function that looks like a Hapi route handler and flows to a route setup.
    */

javascript/ql/test/library-tests/frameworks/hapi/WrappedRouteFlow.qll

@@ -0,0 +1,15 @@
+import javascript
+
+module WrappedRouteConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Http::RequestInputAccess }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink = DataFlow::globalVarRef("sink").getACall().getArgument(0)
+  }
+}
+
+module WrappedRouteTaint = TaintTracking::Global<WrappedRouteConfig>;
+
+query predicate test_WrappedRouteFlow(DataFlow::Node source, DataFlow::Node sink) {
+  WrappedRouteTaint::flow(source, sink)
+}

javascript/ql/test/library-tests/frameworks/hapi/src/hapi.js

@@ -19,8 +19,8 @@ server2.ext('onPreResponse', function handler3(request, reply) { // HTTP::RouteH
 
 function handler4(request, reply){
   request.rawPayload;
-  request.payload.foo;
-  request.query.bar;
+  getRequest(request).payload.foo;
+  getRequest(request).query.bar; getRequest(request).params.bar;
   request.url.path;
   request.headers.baz;
   request.state.token;
@@ -34,3 +34,7 @@ function getHandler() {
     return function (req, h){}
 }
 server2.route({handler: getHandler()});
+
+function getRequest(request) {
+  return request;
+}

javascript/ql/test/library-tests/frameworks/hapi/src/wrapped-route-cache.js

@@ -0,0 +1,7 @@
+function createCached(fn) {
+  return async function (...args) {
+    return fn(...args);
+  };
+}
+
+module.exports = { createCached };

javascript/ql/test/library-tests/frameworks/hapi/src/wrapped-route-configurator.js

@@ -0,0 +1,30 @@
+const endpoints = [];
+
+function apiEndpoint(method, path, handler) {
+  endpoints.push({ method, path, handler });
+}
+
+function createRouteConfig(method, path, handler) {
+  const hapiHandler = async function (request, h) {
+    return handler(request.params, request.query, request.payload);
+  };
+
+  return {
+    method,
+    path,
+    handler: hapiHandler,
+  };
+}
+
+function registerEndpoints(server, serviceInstance) {
+  for (const definition of endpoints) {
+    const wrapped = async (params, query, payload) => {
+      return definition.handler.call(serviceInstance, params, query, payload);
+    };
+
+    const config = createRouteConfig(definition.method, definition.path, wrapped);
+    server.route(config);
+  }
+}
+
+module.exports = { apiEndpoint, registerEndpoints };

javascript/ql/test/library-tests/frameworks/hapi/src/wrapped-route-service.js

@@ -0,0 +1,15 @@
+const { createCached } = require("./wrapped-route-cache");
+
+class ReportService {
+  async runQuery(filter) {
+    sink(filter);
+  }
+}
+
+const service = new ReportService();
+
+const getReportCached = createCached(async function (filter) {
+  return service.runQuery(filter);
+});
+
+module.exports = { getReportCached };

javascript/ql/test/library-tests/frameworks/hapi/src/wrapped-route.js

@@ -0,0 +1,22 @@
+const Hapi = require("@hapi/hapi");
+const {
+  apiEndpoint,
+  registerEndpoints,
+} = require("./wrapped-route-configurator");
+const { getReportCached } = require("./wrapped-route-service");
+
+class ReportRoutes {
+  async getReport(params, query) {
+    return getReportCached(query.filter);
+  }
+}
+
+apiEndpoint("GET", "/reports", ReportRoutes.prototype.getReport);
+
+async function main() {
+  const server = Hapi.server();
+  registerEndpoints(server, new ReportRoutes());
+  return server;
+}
+
+main();