sec: Integrate Ariadne's query cost validator #509
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found Additional details and impacted files@@ Coverage Diff @@
## main #509 +/- ##
==========================================
+ Coverage 91.45% 91.46% +0.01%
==========================================
Files 599 599
Lines 16211 16235 +24
==========================================
+ Hits 14825 14849 +24
Misses 1386 1386
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #509 +/- ##
==========================================
+ Coverage 91.45% 91.46% +0.01%
==========================================
Files 599 599
Lines 16211 16235 +24
==========================================
+ Hits 14825 14849 +24
Misses 1386 1386
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found Additional details and impacted files@@ Coverage Diff @@
## main #509 +/- ##
=======================================
+ Coverage 95.76 95.77 +0.01
=======================================
Files 774 774
Lines 17067 17091 +24
=======================================
+ Hits 16344 16368 +24
Misses 723 723
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| maximum_cost=costs.get("maximumAvailable"), | ||
| ), | ||
| ) | ||
| return HttpResponseBadRequest( |
There was a problem hiding this comment.
we don't want to expose the max cost to the client
| ) -> Optional[Collection]: | ||
| return [ | ||
| cost_validator( | ||
| maximum_cost=settings.GRAPHQL_QUERY_COST_THRESHOLD, |
There was a problem hiding this comment.
ingest our env variable when deriving the max cost
Purpose/Motivation
Our goal with this PR is to mitigate the possibility of a DoS attack with users making calls to our GQL API. We aim to tackle this by using Ariadne's built in query cost validator, assigning a default cost of 1 to each field being requested, a multiplier of x amount (between 3 and 25) * number of edge resources requested (e.g. first:20 repos would add 20 * 25 to the cost, not 20 * number of fields requested for repo, but that is something we can update later on if we want additional granularity in the future)
This first pass PR is meant to lay the infrastructure and initial guidelines that I hope we can tinker and update as we better understand what cost "makes sense" for us long term. For now, I have set the maximum cost to the value mentioned on slack. This seems to be reasonable since we're fetching at most "first: 20" in gazebo, and the "lengthiest" query looks to have a size of ~40 fields. Refering to:
src/services/user/useUser.jsDev Notes:
Below is the thought process that got me to this point and why I chose to add the query complexity logic server side, and using ariadne.
Initially started search looking at what we are currently using for sending HTTP requests on the frontend. GraphQL api looks to be a wrapper function around the JS native Fetch API. Didn’t want to rip out existing infrastructure and put in something like Apollo Client, though that does look to have native capabilities around validating query complexity or looking at query depth, etc. Fetch API also doesn’t have much “built in,” but didn’t rule out the possibility of making creating some simple middleware for doing some type of complexity analysis natively. But there’s probably still a much lower friction way to do this, so started looking at ways we can maybe accomplish something similar server side at the resolver level.
It looks like Ariadne has some query cost validator built in: https://ariadnegraphql.org/docs/0.19/query-validators
Helpful resources / Further Reading:
Links to relevant tickets
Closes https://github.com/codecov/internal-issues/issues/385
What does this PR do?
Screenshots from Testing
Query cost massively ballooning when I set query cost to be 20 on the multiplier.
Query cost prior to setting 20 on multiplier
Fuzzy find for "first" in Gazebo
UPDATE FROM 4/18/2024
Testing out nesting pulls on the ReposForOwner query
Without nesting pulls on the ReposForOwner query (Original)
Query for Reference:
query ReposForOwner( $filters: RepositorySetFilters! $owner: String! $ordering: RepositoryOrdering! $direction: OrderingDirection! $after: String $first: Int ) { owner(username: $owner) { repositories( filters: $filters ordering: $ordering orderingDirection: $direction first: $first after: $after ) { edges { node { name active activated private coverage updatedAt latestCommitAt lines author { username } coverageEnabled bundleAnalysisEnabled repositoryConfig { indicationRange { upperRange lowerRange } } pulls( orderingDirection: $direction first: 20 after: $after ){ edges { node { title } } } } } pageInfo { hasNextPage endCursor } } } }Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.