fix(iast): isolate the mongodb nosql-analysis marker per request#9076
Merged
Conversation
The NoSQL injection analyzer marks the async context with `nosqlAnalyzed` when a mongoose query starts so the nested mongodb driver call skips re-analysis, then restores the previous store on `filter:finish`. When a request ends before that restore runs — a handler that fires its query without awaiting it — the marker is left in the async context and the next request inherits it, so its query is never analyzed and the injection goes unreported. Store the in-progress IAST context as the marker and compare it by reference, so a marker stranded by a previous request no longer matches the current one. Surfaced as a flaky `AppSec / mongoose` job that reported the vulnerability only on the first request of each version block.
Contributor
Overall package sizeSelf size: 6.42 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9076 +/- ##
==========================================
+ Coverage 93.69% 93.72% +0.03%
==========================================
Files 889 891 +2
Lines 50856 51163 +307
Branches 11830 11900 +70
==========================================
+ Hits 47647 47952 +305
- Misses 3209 3211 +2 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:
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: b92b1a8 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-29 15:19:56 Comparing candidate commit b92b1a8 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2259 metrics, 27 unstable metrics.
|
BridgeAR
marked this pull request as ready for review
June 26, 2026 13:42
![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: e794e8dbda
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The NoSQL injection analyzer marks the async context so the nested mongodb driver call skips re-analysis of a filter the mongoose/mquery layer already analyzed. Marking with `enterWith` leaked the marker past the query: a request that ended before the query finished stranded it for the next request, and two concurrent queries of the same request saw each other's marker, leaving one query unanalyzed and its injection unreported. Binding the marker on the query-build channel instead fixed the leak but lost the marker on deferred queries. A mongoose query builds, executes, and reaches the driver in three separate async steps; `find().then()`/`.exec()` run the driver a turn after the synchronous build, outside the build's `runStores` scope, so the driver re-analyzed the same filter and reported the injection twice. Bind the marker around the execution channel the instrumentation now publishes across `exec` (and the synchronous-callback path), so it covers the whole async scope that reaches the driver and `runStores` restores the parent on its own. The marker stays isolated per query and per request by construction, and the deferred driver call inherits it instead of re-analyzing.
BridgeAR
force-pushed
the
BridgeAR/2026-06-26-flaky-iast-mongoose
branch
from
June 26, 2026 16:19
e794e8d to
78eaefe
Compare
The skip stood in for APPSEC-66705: lifting the mongodb test floor to 7.2.0 dropped Node 18 and surfaced lost `req.query` taint on the Node 20 + Express 5 lane, where the mongoose filter reaches the driver a turn after the synchronous build. The per-query marker scoping in this PR restores the marker across that deferred exec, so the suite detects the injection again on that combination.
The suite was skipped under APPSEC-62431 because the mquery driver call
re-analyzed a filter the mquery layer had already analyzed, reporting each
injection N+1 times. The per-query marker scoping in this PR binds the marker
across the mquery execution channel, so the deferred driver call inherits it
instead of re-analyzing, and every case now reports the exact occurrence count.
Drive-by fix:
* Drop the dead `|| {}` fallback in the mquery start bind. The tracing-start
channel always publishes a context object, matching the other onStart
subscribers that destructure it unguarded.
The mongodb >=7 driver resolves the Web Crypto API off the `crypto` global, which Node.js 18 does not expose by default. On the AppSec mongodb-core CI job's oldest-maintenance-lts run, the re-enabled mquery suite's `before` hook threw `crypto is not defined` from `insertMany`, failing the matrix combination the previous skip never exercised. Guard the suite the same way the sibling express-mongo-sanitize suite already does, so it runs everywhere except mongodb >=7 on Node < 20.
Merged
Merged
rochdev
pushed a commit
that referenced
this pull request
Jul 2, 2026
The NoSQL injection analyzer used `enterWith` to mark the async context, which leaked the marker past the query. A request that ended before its query finished stranded the marker for the next request, so that request's injection went unreported. Two concurrent queries within the same request also saw each other's marker, leaving one unanalyzed. Binding the marker on the query-build channel fixed the leak but lost it on deferred queries. A mongoose query builds, executes, and reaches the driver in three separate async steps. `find().then()`/`.exec()` runs the driver a turn after the synchronous build, outside the build's `runStores` scope, so the driver re-analyzed the same filter and reported the injection twice. Binding the marker around the execution channel instead covers the full async scope that reaches the driver, and `runStores` restores the parent on its own. This re-enables the mongoose nosqli suite on Node 20 + Express 5 (skipped for APPSEC-66705) and the mquery nosqli integration suite (skipped for APPSEC-62431, where the unscoped marker caused each injection to be reported N+1 times). The mquery suite skips mongodb >=7 on Node < 20: that driver reads Web Crypto off the `crypto` global, which Node 18 does not expose by default.
rochdev
pushed a commit
that referenced
this pull request
Jul 2, 2026
The NoSQL injection analyzer used `enterWith` to mark the async context, which leaked the marker past the query. A request that ended before its query finished stranded the marker for the next request, so that request's injection went unreported. Two concurrent queries within the same request also saw each other's marker, leaving one unanalyzed. Binding the marker on the query-build channel fixed the leak but lost it on deferred queries. A mongoose query builds, executes, and reaches the driver in three separate async steps. `find().then()`/`.exec()` runs the driver a turn after the synchronous build, outside the build's `runStores` scope, so the driver re-analyzed the same filter and reported the injection twice. Binding the marker around the execution channel instead covers the full async scope that reaches the driver, and `runStores` restores the parent on its own. This re-enables the mongoose nosqli suite on Node 20 + Express 5 (skipped for APPSEC-66705) and the mquery nosqli integration suite (skipped for APPSEC-62431, where the unscoped marker caused each injection to be reported N+1 times). The mquery suite skips mongodb >=7 on Node < 20: that driver reads Web Crypto off the `crypto` global, which Node 18 does not expose by default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The NoSQL injection analyzer marks the async context with
nosqlAnalyzedwhen amongoose query starts so the nested mongodb driver call skips re-analysis, then
restores the previous store on
filter:finish. When a request ends before thatrestore runs — a handler that fires its query without awaiting it — the marker is
left in the async context and the next request inherits it, so its query is never
analyzed and the injection goes unreported.
The marker now stores the in-progress IAST context and is compared by reference,
so a marker stranded by a previous request no longer matches the current one.
This surfaced as a flaky
AppSec / mongoosejob that reported the vulnerabilityonly on the first request of each version block and timed out on the rest,
recovering on re-run.
Test plan
PLUGINS=mongoose SERVICES=mongo yarn test:appsec:plugins:ci— every request reports the vulnerability, not just the first per version block.nosql-injection-mongodb-analyzer.spec.jspin both sides (analysis runs when a previous request left a marker; the nested driver call is still skipped within the same request); they fail on master and pass with the fix.