Skip to content

Commit def5b56

Browse files
IlyasShabiuuriensimon-id
authored
collect express endpoints (#6271)
Endpoints discovery for express Co-authored-by: Ugaitz Urien <ugaitz.urien@datadoghq.com> Co-authored-by: simon-id <simon.id@datadoghq.com>
1 parent c44d8e5 commit def5b56

File tree

11 files changed

+1063
-38
lines changed

11 files changed

+1063
-38
lines changed

‎integration-tests/appsec/endpoints-collection.spec.js‎

Lines changed: 91 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ describe('Endpoints collection', () => {
1313
before(async function () {
1414
this.timeout(process.platform === 'win32' ? 90000 : 30000)
1515

16-
sandbox = await createSandbox(
17-
['fastify'],
18-
false
19-
)
16+
sandbox = await createSandbox(['express', 'fastify'])
2017

2118
cwd = sandbox.folder
2219
})
@@ -35,34 +32,16 @@ describe('Endpoints collection', () => {
3532
{ method: 'PUT', path: '/users/:id' },
3633
{ method: 'DELETE', path: '/users/:id' },
3734
{ method: 'PATCH', path: '/users/:id/:name' },
38-
{ method: 'OPTIONS', path: '/users/:id?' },
39-
40-
// Route with regex
41-
{ method: 'DELETE', path: '/regex/:hour(^\\d{2})h:minute(^\\d{2})m' },
4235

4336
// Additional methods
4437
{ method: 'TRACE', path: '/trace-test' },
4538
{ method: 'HEAD', path: '/head-test' },
4639

47-
// Custom method
48-
{ method: 'MKCOL', path: '/example/near/:lat-:lng/radius/:r' },
49-
5040
// Using app.route()
5141
{ method: 'POST', path: '/multi-method' },
5242
{ method: 'PUT', path: '/multi-method' },
5343
{ method: 'PATCH', path: '/multi-method' },
5444

55-
// All supported methods route
56-
{ method: 'GET', path: '/all-methods' },
57-
{ method: 'HEAD', path: '/all-methods' },
58-
{ method: 'TRACE', path: '/all-methods' },
59-
{ method: 'DELETE', path: '/all-methods' },
60-
{ method: 'OPTIONS', path: '/all-methods' },
61-
{ method: 'PATCH', path: '/all-methods' },
62-
{ method: 'PUT', path: '/all-methods' },
63-
{ method: 'POST', path: '/all-methods' },
64-
{ method: 'MKCOL', path: '/all-methods' }, // Added with addHttpMethod
65-
6645
// Nested routes with Router
6746
{ method: 'PUT', path: '/v1/nested/:id' },
6847

@@ -73,16 +52,83 @@ describe('Endpoints collection', () => {
7352
{ method: 'HEAD', path: '/api/sub/deep' },
7453
{ method: 'POST', path: '/api/sub/deep/:id' },
7554

76-
// Wildcard routes
77-
{ method: 'GET', path: '/wildcard/*' },
78-
{ method: 'HEAD', path: '/wildcard/*' },
79-
{ method: 'GET', path: '*' },
80-
{ method: 'HEAD', path: '*' },
81-
8255
{ method: 'GET', path: '/later' },
8356
{ method: 'HEAD', path: '/later' },
8457
]
8558

59+
if (framework === 'fastify') {
60+
expectedEndpoints.push(
61+
// Route with regex - not supported in express5
62+
{ method: 'DELETE', path: '/regex/:hour(^\\d{2})h:minute(^\\d{2})m' },
63+
64+
{ method: 'OPTIONS', path: '/users/:id?' },
65+
{ method: 'MKCOL', path: '/example/near/:lat-:lng/radius/:r' }, // Added with addHttpMethod
66+
67+
// All supported methods route
68+
{ method: 'GET', path: '/all-methods' },
69+
{ method: 'HEAD', path: '/all-methods' },
70+
{ method: 'TRACE', path: '/all-methods' },
71+
{ method: 'DELETE', path: '/all-methods' },
72+
{ method: 'OPTIONS', path: '/all-methods' },
73+
{ method: 'PATCH', path: '/all-methods' },
74+
{ method: 'PUT', path: '/all-methods' },
75+
{ method: 'POST', path: '/all-methods' },
76+
{ method: 'MKCOL', path: '/all-methods' }, // Added with addHttpMethod
77+
78+
// Wildcard routes
79+
{ method: 'GET', path: '/wildcard/*' },
80+
{ method: 'HEAD', path: '/wildcard/*' },
81+
{ method: 'GET', path: '*' },
82+
{ method: 'HEAD', path: '*' }
83+
)
84+
}
85+
86+
if (framework === 'express') {
87+
expectedEndpoints.push(
88+
{ method: 'CONNECT', path: '/connect-test' },
89+
{ method: '*', path: '/multi-method' },
90+
{ method: '*', path: '/all-methods' },
91+
{ method: '*', path: '/wildcard/*name' },
92+
{ method: '*', path: '/^\\/login\\/.*$/i' },
93+
{ method: 'OPTIONS', path: '/users/:id' },
94+
{ method: 'PATCH', path: '/^\\/ab(cd)?$/' },
95+
{ method: 'POST', path: '/array-route-one' },
96+
{ method: 'POST', path: '/array-route-two' },
97+
{ method: 'POST', path: '/api/array/array-one' },
98+
{ method: 'POST', path: '/api/array/array-two' },
99+
{ method: 'PUT', path: '/api/regex/^\\/item\\/(\\d+)$/' },
100+
{ method: 'GET', path: '/multi-array' },
101+
{ method: 'HEAD', path: '/multi-array' },
102+
{ method: 'GET', path: '/multi-array/mounted' },
103+
{ method: 'HEAD', path: '/multi-array/mounted' },
104+
{ method: 'GET', path: '/multi-array-alt' },
105+
{ method: 'HEAD', path: '/multi-array-alt' },
106+
{ method: 'GET', path: '/multi-array-alt/mounted' },
107+
{ method: 'HEAD', path: '/multi-array-alt/mounted' },
108+
{ method: 'GET', path: '/^\\/regex-mount(?:\\/|$)/mounted' },
109+
{ method: 'HEAD', path: '/^\\/regex-mount(?:\\/|$)/mounted' },
110+
111+
// Multiple routers without mount path
112+
{ method: 'PUT', path: '/router1' },
113+
{ method: 'PUT', path: '/router2' },
114+
115+
// Nested routers mounted after definitions
116+
{ method: 'GET', path: '/root/path/path2/endpoint' },
117+
{ method: 'HEAD', path: '/root/path/path2/endpoint' },
118+
119+
// Same router mounted at multiple paths
120+
{ method: 'GET', path: '/api/v1/shared-before' },
121+
{ method: 'HEAD', path: '/api/v1/shared-before' },
122+
{ method: 'GET', path: '/api/v2/shared-before' },
123+
{ method: 'HEAD', path: '/api/v2/shared-before' },
124+
125+
{ method: 'GET', path: '/api/v1/shared-after' },
126+
{ method: 'HEAD', path: '/api/v1/shared-after' },
127+
{ method: 'GET', path: '/api/v2/shared-after' },
128+
{ method: 'HEAD', path: '/api/v2/shared-after' }
129+
)
130+
}
131+
86132
return expectedEndpoints
87133
}
88134

@@ -97,6 +143,8 @@ describe('Endpoints collection', () => {
97143
const endpointsFound = []
98144
const isFirstFlags = []
99145

146+
const expectedMessageCount = framework === 'express' ? 7 : 4
147+
100148
const telemetryPromise = agent.assertTelemetryReceived(({ payload }) => {
101149
isFirstFlags.push(Boolean(payload.payload.is_first))
102150

@@ -111,7 +159,7 @@ describe('Endpoints collection', () => {
111159
})
112160
})
113161
}
114-
}, 'app-endpoints', 5_000, 4)
162+
}, 'app-endpoints', 5_000, expectedMessageCount)
115163

116164
proc = await spawnProc(appFile, {
117165
cwd,
@@ -132,6 +180,7 @@ describe('Endpoints collection', () => {
132180
const found = endpointsFound.find(e =>
133181
e.method === expected.method && e.path === expected.path
134182
)
183+
135184
expect(found).to.exist
136185
expect(found.type).to.equal('REST')
137186
expect(found.operation_name).to.equal('http.request')
@@ -140,12 +189,25 @@ describe('Endpoints collection', () => {
140189

141190
// check that no additional endpoints were found
142191
expect(endpointsFound.length).to.equal(expectedEndpoints.length)
192+
193+
// Explicitly verify invalid endpoints are NOT reported
194+
if (framework === 'express') {
195+
const cycleEndpoints = endpointsFound.filter(e => e.path.includes('cycle'))
196+
expect(cycleEndpoints).to.have.lengthOf(0, 'Cycle router endpoints should not be collected')
197+
198+
const invalidEndpoints = endpointsFound.filter(e => e.path.includes('invalid'))
199+
expect(invalidEndpoints).to.have.lengthOf(0, 'Invalid router paths should not be collected')
200+
}
143201
} finally {
144202
proc?.kill()
145203
await agent?.stop()
146204
}
147205
}
148206

207+
it('should send express endpoints via telemetry', async () => {
208+
await runEndpointTest('express')
209+
})
210+
149211
it('should send fastify endpoints via telemetry', async () => {
150212
await runEndpointTest('fastify')
151213
})

‎integration-tests/appsec/endpoints-collection/express.js‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
'use strict'
2+
3+
const tracer = require('dd-trace')
4+
tracer.init({
5+
flushInterval: 0
6+
})
7+
8+
const express = require('express')
9+
const app = express()
10+
11+
// Basic routes
12+
app.get('/users', (_, res) => res.send('ok'))
13+
app.post('/users/', (_, res) => res.send('ok'))
14+
app.put('/users/:id', (_, res) => res.send('ok'))
15+
app.delete('/users/:id', (_, res) => res.send('ok'))
16+
app.patch('/users/:id/:name', (_, res) => res.send('ok'))
17+
app.options('/users/:id', (_, res) => res.send('ok'))
18+
19+
// Additional methods
20+
app.trace('/trace-test', (_, res) => res.send('ok'))
21+
app.head('/head-test', (_, res) => res.send('ok'))
22+
app.connect('/connect-test', (_, res) => res.send('ok'))
23+
24+
// Using app.route()
25+
app.route('/multi-method')
26+
.post((_, res) => res.send('ok'))
27+
.put((_, res) => res.send('ok'))
28+
.patch((_, res) => res.send('ok'))
29+
.all((_, res) => res.send('ok'))
30+
31+
app.route(/^\/ab(cd)?$/)
32+
.patch((_, res) => res.send('ok'))
33+
34+
app.route(['/array-route-one', ['/array-route-two']])
35+
.post((_, res) => res.send('ok'))
36+
37+
// All supported methods route
38+
app.all('/all-methods', async (_, res) => res.send('ok'))
39+
40+
// Wildcard routes via array
41+
app.all(['/wildcard/*name'], (_, res) => res.send('ok'))
42+
43+
// RegExp wildcard route
44+
app.all(/^\/login\/.*$/i, (_, res) => {
45+
res.send('ok')
46+
})
47+
48+
// Nested routes with Router
49+
const apiRouter = express.Router()
50+
app.use('/v1', apiRouter)
51+
apiRouter.put('/nested/:id', (_, res) => res.send('ok'))
52+
53+
// Multiple routers without mount path
54+
const apiRouter1 = express.Router()
55+
const apiRouter2 = express.Router()
56+
57+
apiRouter1.put('/router1', (_, res) => res.send('ok 1'))
58+
apiRouter2.put('/router2', (_, res) => res.send('ok 2'))
59+
60+
app.use(apiRouter1, apiRouter2)
61+
62+
// Test nested routers before app.use
63+
const router1 = express.Router()
64+
const router2 = express.Router()
65+
const router3 = express.Router()
66+
67+
router2.use('/path2', router3)
68+
router3.get('/endpoint', (_, res) => res.send('ok'))
69+
router1.use('/path', router2)
70+
71+
// Array mount path
72+
const arrayMountRouter = express.Router()
73+
arrayMountRouter.get('/', (_, res) => res.send('ok'))
74+
arrayMountRouter.get('/mounted', (_, res) => res.send('ok'))
75+
76+
app.use(['/multi-array', '/multi-array-alt'], arrayMountRouter)
77+
78+
// Regex mount path
79+
const regexMountRouter = express.Router()
80+
regexMountRouter.get('/mounted', (_, res) => res.send('ok'))
81+
82+
app.use(/^\/regex-mount(?:\/|$)/, regexMountRouter)
83+
84+
// Add endpoint during runtime
85+
setTimeout(() => {
86+
app.use('/root', router1)
87+
88+
// Deeply nested routes
89+
const deepRouter = express.Router()
90+
deepRouter.get('/nested', (_, res) => res.send('ok'))
91+
const subRouter = express.Router()
92+
subRouter.get('/deep', (_, res) => res.send('ok'))
93+
subRouter.post('/deep/:id', (_, res) => res.send('ok'))
94+
deepRouter.use('/sub', subRouter)
95+
const arrayRouter = express.Router()
96+
arrayRouter.post(['/array-one', '/array-two'], (_, res) => res.send('ok'))
97+
deepRouter.use('/array', arrayRouter)
98+
const regexRouter = express.Router()
99+
regexRouter.put(/^\/item\/(\d+)$/, (_, res) => res.send('ok'))
100+
deepRouter.use('/regex', regexRouter)
101+
app.use('/api', deepRouter)
102+
app.get('/later', (_, res) => res.send('ok'))
103+
}, 1_000)
104+
105+
// Same router mounted at multiple paths - should report both
106+
const sharedRouter = express.Router()
107+
sharedRouter.get('/shared-before', (_, res) => res.send('ok'))
108+
109+
app.use('/api/v1', sharedRouter)
110+
app.use('/api/v2', sharedRouter)
111+
112+
sharedRouter.get('/shared-after', (_, res) => res.send('ok'))
113+
114+
// Cycle routers - should not be collected
115+
const cycleRouter = express.Router()
116+
cycleRouter.use('/cycle', cycleRouter)
117+
app.use('/cycle', cycleRouter)
118+
119+
// Invalid route path - should not be collected
120+
const invalidRouter = express.Router()
121+
app.use('/invalid', invalidRouter)
122+
invalidRouter.get('nested', (_, res) => res.send('ok'))
123+
124+
const server = app.listen(0, '127.0.0.1', () => {
125+
const port = server.address().port
126+
process.send({ port })
127+
})

0 commit comments

Comments
 (0)