Skip to content

JS: Recognize Fastify per-route rate limiting#21700

Open
hvitved wants to merge 3 commits intogithub:mainfrom
hvitved:js/fastify-per-route-rate-limiting
Open

JS: Recognize Fastify per-route rate limiting#21700
hvitved wants to merge 3 commits intogithub:mainfrom
hvitved:js/fastify-per-route-rate-limiting

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Apr 13, 2026

Authored with help from Copilot CLI.

@github-actions github-actions bot added the JS label Apr 13, 2026
@hvitved hvitved force-pushed the js/fastify-per-route-rate-limiting branch from 406c235 to 7a48409 Compare April 13, 2026 09:31
@hvitved hvitved marked this pull request as ready for review April 13, 2026 10:23
@hvitved hvitved requested a review from a team as a code owner April 13, 2026 10:23
Copilot AI review requested due to automatic review settings April 13, 2026 10:23
@hvitved hvitved requested a review from asgerf April 13, 2026 10:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the js/missing-rate-limiting query’s modeling and tests to recognize Fastify per-route rate limiting configurations.

Changes:

  • Add modeling for Fastify per-route rate limiting via route options (config.rateLimit and rateLimit).
  • Extend the CWE-770 MissingRateLimit query test to include per-route-limited and non-limited Fastify routes.
  • Add a change note documenting the analysis improvement.
Show a summary per file
File Description
javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js Adds Fastify per-route rate limiting test cases and a new expected alert for an un-limited route.
javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected Updates expected results to include the new un-limited Fastify route alert.
javascript/ql/src/change-notes/2026-04-13-fastify-per-route-rate-limit.md Documents the query improvement in change notes.
javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll Introduces a new model class to treat per-route Fastify rate limit options as rate limiting.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 2

Comment on lines +196 to +197
* An options object with a `rateLimit` config passed to a Fastify shorthand route method,
* such as `fastify.post('/path', { config: { rateLimit: { ... } } }, handler)`.
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment describes only the config.rateLimit form, but the implementation also treats a top-level rateLimit property in the options object as per-route rate limiting. Consider updating the doc comment to mention both supported shapes so the documentation matches the behavior.

Suggested change
* An options object with a `rateLimit` config passed to a Fastify shorthand route method,
* such as `fastify.post('/path', { config: { rateLimit: { ... } } }, handler)`.
* An options object passed to a Fastify shorthand route method that enables per-route
* rate limiting, either via `config.rateLimit` or via a top-level `rateLimit` property,
* such as `fastify.post('/path', { config: { rateLimit: { ... } } }, handler)` or
* `fastify.post('/path', { rateLimit: { ... } }, handler)`.

Copilot uses AI. Check for mistakes.
fastifyApp.register(require('fastify-rate-limit'));
fastifyApp.get('/bar', expensiveHandler1);

// Fastify per-route rate limiting via config.rateLimit
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section header mentions only config.rateLimit, but the following test cases also cover the { rateLimit: ... } options shape. Consider adjusting the comment to reflect both forms to avoid confusion when updating/reading the tests later.

Suggested change
// Fastify per-route rate limiting via config.rateLimit
// Fastify per-route rate limiting via config.rateLimit or direct { rateLimit: ... } options

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if DCA doesn't complain

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.

4 participants