Skip to content

fix(graphql-server): exempt schema introspection from the depth/cost gate - #1817

Merged
pyramation merged 1 commit into
mainfrom
fix/introspection-depth-exempt
Sep 8, 2026
Merged

fix(graphql-server): exempt schema introspection from the depth/cost gate#1817
pyramation merged 1 commit into
mainfrom
fix/introspection-depth-exempt

Conversation

@pyramation

Copy link
Copy Markdown
Contributor

Summary

Fixes constructive-hub propagate-to-dashboard (failing every run since Sep 1, e.g. https://github.com/constructive-io/constructive-hub/actions/runs/34269003613): apps/admin codegen dies with Failed to fetch schema: The query is nested too deeply.

Since #1753 the document gate charges introspection against the tenant's max_query_depth. The codegen's SCHEMA_INTROSPECTION_QUERY (and graphql-js's getIntrospectionQuery) nests __schema → types → fields → args → type → ofType×7 = depth 13, over the default of 12 — so any protected endpoint refuses introspection.

Change in walkSelectionSet:

 if (name === '__schema' || name === '__type') {
   if (!protection.enableIntrospection) reject(INTROSPECTION_DISABLED);
+  continue;   // introspection selections are not walked for depth/cost
 }

Why this rather than raising the default: introspection already has its own switch (enable_introspection), its types carry no connections so cost is always 0, and its shape is fixed by the spec rather than by the client — the depth budget exists to bound data queries, and a tenant lowering it for their API should not silently lose introspection. Raising the default would only move the cliff.

Sibling fields in the same operation are still measured (test added).

Link to Devin session: https://app.devin.ai/sessions/1c0e1e05988944c2995bd82bb7e0c215
Open in Devin Desktop: https://app.devin.ai/desktop/session/1c0e1e05988944c2995bd82bb7e0c215?variant=devin
Requested by: @pyramation

…ost gate

The standard introspection document nests __schema > types > fields > args >
type > ofType x7 (depth 13), which the default max_query_depth of 12 rejects,
so every codegen run against a protected endpoint failed with QUERY_TOO_DEEP.
Introspection is governed by enable_introspection alone; its selections carry
no connections and are not walked.
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review complete. 🟡 1 medium

💬 Inline comments (1)


This change tightens the document gate's handling of introspection. The walker in document-gate.ts now continues past __schema/__type fields after the enableIntrospection check, so their selection subtrees are never walked and therefore never charged depth or cost; a doc comment explains that the standard introspection document's fixed ofType chain would otherwise exceed a sensible tenant depth budget. The test file adds coverage asserting the standard introspection document reports { depth: 1, cost: 0 } while a mixed operation that also introspects is still depth-checked.

Files Change
graphql/server/src/protection/document-gate.ts Skips walking __schema/__type selection subtrees after the introspection gate, exempting them from depth/cost accounting.
graphql/server/src/protection/__tests__/document-gate.test.ts Adds tests for introspection depth exemption and for still measuring the rest of a mixed operation.

Reviewed commit: 6c737cb

@tenki-reviewer tenki-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #1817 refines the GraphQL document protection gate so schema introspection (__schema/__type) selections are skipped entirely, exempting them from depth and cost budgets while keeping the enableIntrospection switch authoritative.

Key findings

if (!walk.protection.enableIntrospection) {
reject(errors.INTROSPECTION_DISABLED());
}
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 security · medium

Introspection continue bypasses the depth budget

At graphql/server/src/protection/document-gate.ts:129, the new continue skips the entire __schema/__type selection subtree, so it is never charged depth or cost. Because the introspection schema is recursive (__Type.fields__Field.type__Type), a client can send an arbitrarily deep introspection document that the gate reports as { depth: 1, cost: 0 }, defeating the maxQueryDepth DoS protection; only the statement timeout still bounds it.

📋 Prompt for AI Agents

In graphql/server/src/protection/document-gate.ts around line 129, the continue after the __schema/__type introspection check skips the entire selection subtree, so a recursive introspection document (__schema { types { fields { type { fields { type { ... } } } } } }) bypasses maxQueryDepth and is reported as depth 1. Replace the bare continue with logic that still walks the introspection selection set but caps the depth contributed by introspection recursion (for example, recurse into the __schema/__type selection set with a fixed depth allowance independent of maxQueryDepth, or track an introspection-specific depth counter and reject when it exceeds a bounded constant). Keep exempting introspection from cost, but do not let it escape depth bounding entirely.

@pyramation
pyramation merged commit 3af7609 into main Sep 8, 2026
21 checks passed
@pyramation
pyramation deleted the fix/introspection-depth-exempt branch September 8, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant