Skip to content

Commit 2b19c84

Browse files
dtorres-fgfclaude
andcommitted
JS: recognize Fastify servers reached through chainable server methods
`Fastify::server()` tracks the server from the `fastify()` invocation, but did not step through methods that configure the instance and return it, such as `withTypeProvider()` and the `set*` family. On a server built as `fastify().withTypeProvider<T>()` the instance was therefore not recognized as a server at all, so neither the plugins registered on it nor the routes declared on it were attributed to it. The effect ran in both directions. Routes on such an instance could be missed entirely, and where they were still reported through another model, a globally registered plugin such as `@fastify/rate-limit` was not seen as guarding them, which produced false positives in `js/missing-rate-limiting`. Route-registering methods (`register`, `addHook`, and the shorthand route methods) also return the server, but they are deliberately left out: they already have a meaning in the routing model, so including them would change the shape of the routing tree rather than only how a server reference is resolved. The step is added inside the type-tracked predicate rather than to the public one, so a chained instance is still resolved when it crosses a function boundary, for example when it is returned from a factory function. A test covers that case, and it fails if the step is placed in the public predicate instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent eb3ddb8 commit 2b19c84

6 files changed

Lines changed: 89 additions & 0 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Fastify servers reached through a chainable configuration method, such as `fastify().withTypeProvider<T>()` or `fastify().setValidatorCompiler(...)`, are now recognized as the same server instance. Routes registered on such an instance are now attributed to their server, which may add results for queries such as `js/missing-rate-limiting` where routes were previously not recognized at all, and remove false positives where a globally registered plugin guards them.

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,25 @@ module Fastify {
2121
StandardServerDefinition() { this = DataFlow::moduleImport("fastify").getAnInvocation() }
2222
}
2323

24+
/**
25+
* Gets the name of a chainable Fastify server method, that is, a configuration or
26+
* lifecycle method that returns the same server instance it was called on, so that a
27+
* call to it still refers to that server.
28+
*
29+
* Route-registering methods such as `register`, `addHook`, and the shorthand route
30+
* methods return the server as well, but they are deliberately excluded here because
31+
* they already have a meaning in the routing model for Fastify.
32+
*/
33+
private string chainableServerMethodName() {
34+
result =
35+
[
36+
"withTypeProvider", "addSchema", "addHttpMethod", "decorate", "decorateRequest",
37+
"decorateReply", "setValidatorCompiler", "setSerializerCompiler", "setSchemaController",
38+
"setReplySerializer", "setSchemaErrorFormatter", "setErrorHandler", "setNotFoundHandler",
39+
"setGenReqId", "setChildLoggerFactory", "after", "ready"
40+
]
41+
}
42+
2443
/** Gets a data flow node referring to a fastify server. */
2544
private DataFlow::SourceNode server(DataFlow::SourceNode creation, DataFlow::TypeTracker t) {
2645
t.start() and
@@ -31,6 +50,11 @@ module Fastify {
3150
t.start() and
3251
result = pluginCallback(creation).(DataFlow::FunctionNode).getParameter(0)
3352
or
53+
// server.withTypeProvider<T>(), server.setValidatorCompiler(...), and friends return
54+
// the server itself, so the result of such a call still refers to it.
55+
t.start() and
56+
result = server(creation).getAMethodCall(chainableServerMethodName())
57+
or
3458
exists(DataFlow::TypeTracker t2 | result = server(creation, t2).track(t2, t))
3559
}
3660

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,13 @@ fastifyWithObjects4.post(
9090
request.params;
9191
}
9292
);
93+
94+
// the server is reached through a chainable configuration method, which returns the
95+
// same instance
96+
var fastifyChained = require("fastify")().withTypeProvider();
97+
fastifyChained.get(
98+
"/",
99+
/* handler */ (request, reply) => {
100+
reply.send({ hello: "world" }); // response
101+
}
102+
);

javascript/ql/test/library-tests/frameworks/fastify/tests.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ test_RouteSetup
77
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) |
88
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) |
99
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) |
10+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) |
1011
test_HeaderAccess
1112
| src/fastify.js:39:5:39:24 | request.headers.name | name |
1213
test_RouteHandler
@@ -25,6 +26,7 @@ test_RouteHandler
2526
| src/fastify.js:65:17:69:3 | functio ... ms;\\n } | src/fastify.js:61:27:61:46 | require("fastify")() |
2627
| src/fastify.js:76:17:80:3 | functio ... ms;\\n } | src/fastify.js:72:27:72:46 | require("fastify")() |
2728
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:83:27:83:46 | require("fastify")() |
29+
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:96:22:96:41 | require("fastify")() |
2830
test_HeaderDefinition
2931
| src/fastify.js:42:5:42:33 | reply.h ... value") | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
3032
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
@@ -34,6 +36,7 @@ test_ServerDefinition
3436
| src/fastify.js:61:27:61:46 | require("fastify")() |
3537
| src/fastify.js:72:27:72:46 | require("fastify")() |
3638
| src/fastify.js:83:27:83:46 | require("fastify")() |
39+
| src/fastify.js:96:22:96:41 | require("fastify")() |
3740
test_RedirectInvocation
3841
| src/fastify.js:44:5:44:29 | reply.r ... e, url) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
3942
test_RequestInputAccess
@@ -57,6 +60,7 @@ test_ResponseSendArgument
5760
| src/fastify.js:6:12:6:29 | { hello: "world" } | src/fastify.js:5:17:7:3 | async ( ... nse\\n } |
5861
| src/fastify.js:27:16:27:33 | { hello: "world" } | src/fastify.js:26:17:28:3 | (reques ... nse\\n } |
5962
| src/fastify.js:45:16:45:22 | payload | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
63+
| src/fastify.js:100:16:100:33 | { hello: "world" } | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
6064
test_RouteSetup_getServer
6165
| src/fastify.js:3:1:8:1 | fastify ... e\\n }\\n) | src/fastify.js:1:15:1:34 | require("fastify")() |
6266
| src/fastify.js:10:1:21:2 | fastify ... > {}\\n}) | src/fastify.js:1:15:1:34 | require("fastify")() |
@@ -66,6 +70,7 @@ test_RouteSetup_getServer
6670
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:61:27:61:46 | require("fastify")() |
6771
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:72:27:72:46 | require("fastify")() |
6872
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:83:27:83:46 | require("fastify")() |
73+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:96:22:96:41 | require("fastify")() |
6974
test_HeaderDefinition_defines
7075
| src/fastify.js:42:5:42:33 | reply.h ... value") | name | value |
7176
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name | value |
@@ -85,6 +90,7 @@ test_RouteSetup_getARouteHandler
8590
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:65:17:69:3 | functio ... ms;\\n } |
8691
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:76:17:80:3 | functio ... ms;\\n } |
8792
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:87:17:91:3 | functio ... ms;\\n } |
93+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
8894
test_RouteHandler_getARequestExpr
8995
| src/fastify.js:5:17:7:3 | async ( ... nse\\n } | src/fastify.js:5:24:5:30 | request |
9096
| src/fastify.js:13:28:13:55 | (reques ... ) => {} | src/fastify.js:13:29:13:35 | request |
@@ -122,6 +128,7 @@ test_RouteHandler_getARequestExpr
122128
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:88:5:88:11 | request |
123129
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:89:5:89:11 | request |
124130
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:90:5:90:11 | request |
131+
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:99:18:99:24 | request |
125132
test_HeaderDefinition_getAHeaderName
126133
| src/fastify.js:42:5:42:33 | reply.h ... value") | name |
127134
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name |

javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@
1111
| tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
1212
| tst.js:111:28:111:44 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
1313
| tst.js:116:39:116:55 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
14+
| tst.js:130:35:130:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
15+
| tst.js:160:35:160:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |

javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,45 @@ const fastifyApp3 = require('fastify')();
116116
fastifyApp3.get('/before-rate-limit', expensiveHandler1); // $ Alert
117117
fastifyApp3.register(require('@fastify/rate-limit'));
118118
fastifyApp3.get('/after-rate-limit', expensiveHandler1);
119+
120+
// the server instance is reached through a chainable configuration method, which
121+
// returns the same instance
122+
const fastifyApp4 = require('fastify')().withTypeProvider();
123+
124+
fastifyApp4.register(require('@fastify/rate-limit'));
125+
fastifyApp4.get('/after-rate-limit', expensiveHandler1);
126+
127+
// same, but with no rate limiter registered at all, so the route is genuinely unguarded
128+
const fastifyApp5 = require('fastify')().withTypeProvider();
129+
130+
fastifyApp5.get('/no-rate-limit', expensiveHandler1); // $ Alert
131+
132+
// several configuration methods chained together
133+
const fastifyApp6 = require('fastify')()
134+
.withTypeProvider()
135+
.setValidatorCompiler(compiler)
136+
.decorate('answer', 42);
137+
138+
fastifyApp6.register(require('@fastify/rate-limit'));
139+
fastifyApp6.get('/after-rate-limit', expensiveHandler1);
140+
141+
// the chained instance is returned from a factory function, so reaching it requires
142+
// tracking the value across the call rather than only through local references
143+
function makeFastifyApp() {
144+
return require('fastify')().withTypeProvider();
145+
}
146+
147+
const fastifyApp7 = makeFastifyApp();
148+
149+
fastifyApp7.register(require('@fastify/rate-limit'));
150+
fastifyApp7.get('/after-rate-limit', expensiveHandler1);
151+
152+
// same, from a separate factory so that the server above does not share its creation
153+
// site, and no rate limiter is registered on it
154+
function makeUnguardedFastifyApp() {
155+
return require('fastify')().withTypeProvider();
156+
}
157+
158+
const fastifyApp8 = makeUnguardedFastifyApp();
159+
160+
fastifyApp8.get('/no-rate-limit', expensiveHandler1); // $ Alert

0 commit comments

Comments
 (0)