graphql: Improve GraphQL fingerprinting with specificity scoring#7118
graphql: Improve GraphQL fingerprinting with specificity scoring#7118kingthorin wants to merge 1 commit intozaproxy:mainfrom
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
a8084d8 to
ee1d789
Compare
|
New Issues (3)Checkmarx found the following issues in this Pull Request
Use @Checkmarx to interact with Checkmarx PR Assistant. |
There was a problem hiding this comment.
Pull request overview
Updates the GraphQL add-on’s framework fingerprinter to evaluate fingerprint checks in descending “specificity” order (most unique patterns first), aiming to reduce false positives and improve detection efficiency.
Changes:
- Introduces a
FingerprintCheckrecord with a validated specificity score and uses it to order fingerprint checks. - Refactors fingerprinting flow into
performPatternBasedDetection()+raiseAlertForFramework()with first-match-wins behavior. - Extends unit tests with stricter “no logged WARN/ERROR” assertions and adds a Juniper detection/evidence test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlFingerprinter.java | Adds specificity-scored checks, sorts checks by score, refactors detection/alerting, and hardens Strawberry detection against non-JSON/null responses. |
| addOns/graphql/src/test/java/org/zaproxy/addon/graphql/GraphQlFingerprinterUnitTest.java | Updates invalid-data expectations, introduces helper for asserting no logged errors, and adds a Juniper pattern/evidence test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1ca02f1 to
2a39087
Compare
|
Where's the data/evidence for the claims? |
69803d7 to
3d103a6
Compare
|
First is the removed comment: Logically if the checks can be classified as a range of specific to generic then the order does matter. Further summary/sources: https://gist.github.com/kingthorin/53859ce98cbe73559898dfee355dd6df Let me know if you'd like the gist content added here somewhere/somehow. |
|
It's not a logical conclusion since an error is only generic if there are more than one/many GraphQL engines producing those errors, neither your comment nor the gist provide data/evidence for that (where's the sample of errors from all/some of the engines being checked to claim that they are generic). If you read the quote in the gist it says |
|
Point taken. I'll look at another way to tackle this that's more evidence based. I'll move the small bug fix into another PR, convert this one to draft for now. |
|
To your other point the additional checks were from the same source as the originals. |
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
3d103a6 to
b9803bb
Compare


Overview
Implements specificity-based ordering for GraphQL framework fingerprinting to increase efficiency and potentially reduce false positives by checking specific patterns before generic ones.
This is one of a set of PRs that I'm working on for GraphQL Fingerprinting.
This also fixes a bug whereby the handlers were always reset on instantiation when they should have only been reset if null. So the Tech Detection support had been broken. (I can move that fix to a separate PR if necessary.)Changes
FingerprintCheckrecord: Encapsulates each check with a specificity score (0-100), validated at constructionperformPatternBasedDetection()andraiseAlertForFramework()for claritycheckStrawberryEngine()Specificity Tiers
Scoring Guidelines for Future Contributors
When adding a new fingerprint check, assign a specificity score based on how unique the matched pattern is to that specific framework:
When in doubt, start lower—a false negative (missed detection) is less harmful than a false positive (wrong framework reported).
AI Disclosure
Portions of this contribution were prepared by copilot or augment code.