Skip to content

Improve hapi coverage - #22522

Draft
jacknojo wants to merge 3 commits into
mainfrom
jacknojo/enhance_hapi_coverage
Draft

Improve hapi coverage#22522
jacknojo wants to merge 3 commits into
mainfrom
jacknojo/enhance_hapi_coverage

Conversation

@jacknojo

@jacknojo jacknojo commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

The false negative reproduced on this PR as a regression test had four separate breaks in the flow:

  1. Any handler given as part of server.route(config) was ignored when config came from a helper, like server.route(makeConfig(handler)).
  2. Forwarding the whole request.query object was not treated as a source.
  3. The custom endpoint registry's definition.handler.call(...) had no resolved target.
  4. Arguments passed through createCached did not reach the wrapped function:
    function createCached(handler) {
     return (...args) => handler(...args);
    }
    
    const cachedHandler = createCached((request) => {
      use(request.query.id);
    });
    
    cachedHandler(hapiRequest);
    Here the flow of hapiRequest stops at ...args.

In javascript/ql/lib/semmle/javascript/frameworks/Hapi.qll we:

  • Now recognize Hapi route and ext calls independently of whether their handlers can immediately be resolved. We backtrack to find the handler.
  • Now track request.query, request.params, and request.payload across supported data-flow steps and mark fields read from those values as sources, including reads in functions called by the handler. This follows the existing Express modeling pattern.
  • Introduce a Hapi-specific flow step for custom route registries. It connects a function stored in a route definition’s handler property with later calls to that property on the same definition object. Other property names than "handler" are not currently supported.

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

  • CodeQL already recognized createCached(fn) as a forwarding wrapper, including the rest/spread pattern (...args) => fn(...args).
  • It did not use that knowledge when the returned wrapper was later invoked.
  • The new step backtracks an invoked wrapper to its concrete wrapped function and maps each call argument to the corresponding function parameter.

This is a change to the overall JavaScript CodeQL code and I am not sure if this is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant