fix(appsec): support apollo v5 blocking#9007
Conversation
Overall package sizeSelf size: 6.57 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.0 | 117.14 kB | 432.47 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 3b4b681 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-07 08:23:18 Comparing candidate commit 3b4b681 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2246 metrics, 40 unstable metrics.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9007 +/- ##
==========================================
- Coverage 93.59% 93.46% -0.13%
==========================================
Files 900 900
Lines 52679 52939 +260
Branches 12405 12492 +87
==========================================
+ Hits 49306 49481 +175
- Misses 3373 3458 +85 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bdf60609e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| addHook({ name: '@apollo/server', file: 'dist/cjs/utils/HeaderMap.js', versions: ['4'] }, apolloHeaderMapHook) | ||
| addHook( | ||
| { name: '@apollo/server', file: 'dist/cjs/utils/HeaderMap.js', versions: ['>=4'] }, |
There was a problem hiding this comment.
When users enable Apollo Server's allowBatchedHttpRequests, this v5 hook now activates blocking inside runHttpQuery, which Apollo calls once per operation in a batch. The per-operation response returned by wrapExecuteHTTPGraphQLRequest carries the 403 status and block headers, but Apollo's batch combiner only uses each operation's response.body.string and keeps the shared HTTP head, so a blocked operation can be returned as part of a 200 batch response instead of blocking the HTTP request. The v5 path needs to wrap the whole HTTP request or propagate the blocking status/headers back to the shared batch response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is correct, I'll fix it
There was a problem hiding this comment.
I'll set this PR as draft, feel free to undraft it once this comment is addressed.
There was a problem hiding this comment.
I addressed it but amended the commit sorry!
2bdf606 to
77491f0
Compare
| ) | ||
|
|
||
| addHook( | ||
| { name: '@apollo/server', file: 'dist/cjs/runHttpQuery.js', versions: ['>=5.0.0'] }, |
There was a problem hiding this comment.
why isn't this necessary anymore?
There was a problem hiding this comment.
I have the same question. All I can think is that the other now-version-uncapped hook handles this functionality.
There was a problem hiding this comment.
I removed it because it's not needed for instrumentation of v5, executeHTTPGraphQLRequest is the public entry and I switched it to >=4.
The real between v4 and v5 was just that HeaderMap lives in a slightly different module load path. By removing previous v5 instrumentation we are relying on what we did for v4 for blocking
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4600cd0207
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert.strictEqual(payload[1][0].name, 'web.request') | ||
| assert.strictEqual(payload[1][0].meta['appsec.blocked'], 'true') | ||
| assert.strictEqual(payload[1][0].meta['appsec.event'], 'true') |
There was a problem hiding this comment.
The assertion collection here is smaller than the ones in the previous test cases. It'd be good to keep assertions consistent acroos all the cases... or is there a reason to use a smaller set here?
There was a problem hiding this comment.
I did the same as previous test!
* fix(appsec): support apollo v5 blocking
* fix(appsec): support apollo v5 blocking
* fix(appsec): support apollo v5 blocking
* fix(appsec): support apollo v5 blocking
4 participants
What does this PR do?
This PR extends the
HeaderMap.jshook from versions:['4']to['>=4']so it also fires for v5. It also adds an integration test that verifies blocking returns 403 withappsec.blocked: 'true'on Apollo v5.Motivation
The previous PR adding Apollo Server v5 support (#6155) did not cover blocking on v5
Additional Notes
APPSEC-68543