fix: don't reject queries using variables when cost_validator has none (#1319)

[jbbqqf]

Summary

cost_validator() returns a validation rule that takes an optional variables dict, baked into the class at construction time. When the rule is added to a static validation_rules=[cost_validator(...)] list — the natural form most users reach for — it never receives per-request variables. get_argument_values then raises a GraphQLError for any argument supplied via a query variable, and every such query is rejected with a misleading "was not provided a runtime value" message.

This PR makes that failure mode non-fatal: when self.variables is None, the validator falls back to an empty args dict instead of reporting an error, so the query is allowed to run. Variable-driven multipliers are simply not counted in the cost. When the caller does pass an explicit variables dict, behaviour is unchanged — missing entries are still reported as validation errors (the caller has told the validator what is available).

Fixes #1319.

Context

Root cause is in ariadne/validation/query_cost.py, lines 92-98. The try/except around get_argument_values reported any exception via report_error, which surfaces as a client-facing validation error and aborts the query. The function's signature (get_argument_values(field, child_node, variable_values=None)) raises on the first non-null variable it can't resolve, so for the static-rule form this triggers on every query that uses a non-null variable.

The documented workaround — passing validation_rules as a per-request callable — keeps working unchanged; this fix only removes the spurious failure on the static form.

Changes

• ariadne/validation/query_cost.py — in CostValidator.compute_node_cost, branch on self.variables is None: silently fall back to empty field_args (cost is best-effort), otherwise preserve the existing report_error path. Added a # See issue #1319. comment so a reviewer reading the diff cold understands the invariant.
• tests/test_query_cost_validation.py — four new regression tests covering: required scalar variable, required non-null input variable, best-effort cost computation with a missing variable, and explicit variables={} still surfacing errors (guards against the fix loosening behaviour where the caller can actually catch it).

Reproduce BEFORE/AFTER yourself (copy-paste)

# --- one-time setup ---
git clone https://github.com/mirumee/ariadne.git /tmp/repro-1319 && cd /tmp/repro-1319
python -m venv .venv && source .venv/bin/activate
pip install -e . pytest >/dev/null

cat > /tmp/repro_1319.py <<'PY'
from graphql.language import parse
from graphql.validation import validate
from ariadne import make_executable_schema
from ariadne.validation import cost_validator

type_defs = """
    type Query { _dummy: String }
    type Mutation { echo(input: EchoInput!): String }
    input EchoInput { message: String! }
"""
schema = make_executable_schema(type_defs)
query = "mutation Echo($input: EchoInput!) { echo(input: $input) }"
rule = cost_validator(maximum_cost=100, default_complexity=1)
errors = validate(schema, parse(query), [rule])
print("errors:", errors)
PY

# --- BEFORE: origin/main, expect the spurious validation error ---
git checkout origin/main -- ariadne/ 2>/dev/null
python /tmp/repro_1319.py
# Expected: errors: [GraphQLError("Argument 'input' of required type 'EchoInput!' was provided the variable '$input' which was not provided a runtime value...")]

# --- AFTER: this branch, expect no errors ---
git fetch https://github.com/jbbqqf/ariadne.git fix/1319-cost-validator-variables
git checkout FETCH_HEAD -- ariadne/
python /tmp/repro_1319.py
# Expected: errors: []

What I ran locally

$ pytest tests/test_query_cost_validation.py -v
...
tests/test_query_cost_validation.py::test_query_with_required_variable_is_not_rejected_when_variables_not_passed PASSED
tests/test_query_cost_validation.py::test_query_with_required_input_variable_is_not_rejected_when_variables_not_passed PASSED
tests/test_query_cost_validation.py::test_cost_computation_skips_variable_multipliers_when_variables_not_passed PASSED
tests/test_query_cost_validation.py::test_explicit_variables_dict_still_reports_missing_variable_errors PASSED
============================== 36 passed in 0.10s ==============================

$ pytest -q
....................................................................... [100%]
737 passed, 1 skipped in 82.72s

I also confirmed the three new "non-rejection" tests fail on origin/main by stashing only ariadne/validation/query_cost.py (keeping the new tests):

FAILED tests/test_query_cost_validation.py::test_query_with_required_variable_is_not_rejected_when_variables_not_passed
FAILED tests/test_query_cost_validation.py::test_query_with_required_input_variable_is_not_rejected_when_variables_not_passed
FAILED tests/test_query_cost_validation.py::test_cost_computation_skips_variable_multipliers_when_variables_not_passed

Edge cases

#	Scenario	variables=	Query	Before	After
1	Static rule, query uses variable	None (default)	query($v: Int!){ simple(value: $v) }	rejected with "not provided a runtime value"	allowed; cost = default_complexity
2	Static rule, non-null input variable	None	mutation($i: I!){ echo(input: $i) }	rejected	allowed
3	Static rule, no variables in query	None	{ constant }	allowed	allowed (unchanged)
4	Explicit dict, variable present	{"v": 5}	query($v: Int!){ simple(value: $v) }	allowed, multiplier counted	allowed, multiplier counted (unchanged)
5	Explicit dict, variable missing	{}	query($v: Int!){ simple(value: $v) }	rejected	rejected (preserved on purpose — caller said what's available)
6	Cost-map / directive misconfig	any	n/a	reports TypeError/ValueError from compute_cost	reports TypeError/ValueError (unchanged — different except branch)

Risk / blast radius

The change is a one-line conditional in a single except branch in CostValidator.compute_node_cost. The default-arg semantics (variables: dict | None = None) are unchanged, so no callers need to update anything. Cost computation degrades gracefully — a query with variables that would multiply the cost by $value=1000 will now under-count instead of refusing to run, which is the same trade-off the documented callable form makes between releases.

The fix does not touch:

• The (TypeError, ValueError) handlers around compute_cost (real misconfigurations are still reported).
• cost_map validation in enter_operation_definition (schema-level errors are still reported).
• The behaviour when the caller passes any non-None variables dict, including {} (test 5 above).

---

PR drafted with assistance from Claude Code (Anthropic). The change was reviewed manually against ariadne's source. The reproducer block above is the one I used during development; reviewers can paste it verbatim.

Summary by CodeRabbit

• Bug Fixes

  • Cost validator now gracefully handles queries with GraphQL variables when configured without per-request variables, avoiding rejected queries and computing costs by omitting variable-driven multipliers.

• Tests

  • Added regression tests for cost validator behavior with required GraphQL variables.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes a regression where cost_validator rejected all queries using GraphQL variables. The validator now conditionally handles argument-resolution exceptions: when configured without per-request variables, it treats missing variable values as a cost-calculation limitation and falls back to empty arguments; when variables are explicitly provided, the original error reporting is preserved. Four regression tests validate the corrected behavior.

Changes

Variable-aware exception handling in cost validation

Layer / File(s)

Summary

Variable-aware argument resolution with exception handling ariadne/validation/query_cost.py, tests/test_query_cost_validation.py

compute_node_cost now inspects self.variables before reporting exceptions from get_argument_values: when None, exceptions are silently handled and field_args defaults to {}; when provided, exceptions are reported as validation errors. Four regression tests confirm queries with required variables pass when unconfigured, cost computation skips variable multipliers, and explicit variables={} still reports missing value errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A query with variables once would fail,
Now graceful fallback tells the tale—
No variables? Skip the multiplier dance,
Cost computes with a second chance! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately reflects the main fix: preventing rejection of queries that use GraphQL variables when cost_validator lacks per-request variables.
Linked Issues check	✅ Passed	All coding requirements from issue #1319 are met: the fix prevents query rejection when variables are unavailable, preserves error reporting when variables are explicitly passed, and includes comprehensive regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to resolving issue #1319: modifications to cost_validator logic and corresponding regression tests with no unrelated alterations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.30%. Comparing base (1534006) to head (15d4051).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
+ Coverage   97.24%   97.30%   +0.06%     
==========================================
  Files         127      127              
  Lines        9214     9241      +27     
==========================================
+ Hits         8960     8992      +32     
+ Misses        254      249       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_query_cost_validation.py (1)

656-744: ⚡ Quick win

Add return type hints to the new test functions.

The new test defs at Line 656, Line 680, Line 696, and Line 725 should include explicit -> None to match repo typing rules for Python files.

Suggested diff

 def test_query_with_required_variable_is_not_rejected_when_variables_not_passed(
     schema,
-):
+) -> None:
@@
 def test_query_with_required_input_variable_is_not_rejected_when_variables_not_passed(
     schema,
-):
+) -> None:
@@
 def test_cost_computation_skips_variable_multipliers_when_variables_not_passed(
     schema,
-):
+) -> None:
@@
-def test_explicit_variables_dict_still_reports_missing_variable_errors(schema):
+def test_explicit_variables_dict_still_reports_missing_variable_errors(
+    schema,
+) -> None:

As per coding guidelines: "Use Python 3.10+ with type hints throughout".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_query_cost_validation.py` around lines 656 - 744, Add explicit
return type hints "-> None" to the four new test functions so they conform to
repo typing rules: update the definitions of
test_query_with_required_variable_is_not_rejected_when_variables_not_passed,
test_query_with_required_input_variable_is_not_rejected_when_variables_not_passed,
test_cost_computation_skips_variable_multipliers_when_variables_not_passed, and
test_explicit_variables_dict_still_reports_missing_variable_errors to include
"-> None" before the colon.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_query_cost_validation.py`:
- Around line 656-744: Add explicit return type hints "-> None" to the four new
test functions so they conform to repo typing rules: update the definitions of
test_query_with_required_variable_is_not_rejected_when_variables_not_passed,
test_query_with_required_input_variable_is_not_rejected_when_variables_not_passed,
test_cost_computation_skips_variable_multipliers_when_variables_not_passed, and
test_explicit_variables_dict_still_reports_missing_variable_errors to include
"-> None" before the colon.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0df11db9-ff94-412c-bca4-7cce515cf5ef

📥 Commits

Reviewing files that changed from the base of the PR and between cb2efb5 and 15d4051.

📒 Files selected for processing (2)

• ariadne/validation/query_cost.py
• tests/test_query_cost_validation.py

[prelint]

Decision review: Cost validator silently accepts queries when variable costs cannot be resolved

Product decisions in this change

1. When the cost validator has no access to variable values, queries that use variables for cost-multiplying arguments are allowed through rather than rejected. The validator computes a partial cost — counting only the parts it can see — and approves the query.

2. "No variables were configured" and "an empty variable set was explicitly configured" are treated differently. Passing nothing produces silent best-effort validation; passing an explicit empty collection still rejects queries that reference missing variables.

3. There is no signal to the developer that cost protection is incomplete. When the validator falls back to ignoring variable-driven costs, it does so without a warning, log, or error. The developer's configuration looks correct and queries pass.

4. The natural, common form of cost validation (a static rule list) is now defined as a best-effort tool, not a guarantee. This is an implicit downgrade of what the feature promises.

---

Assessment

1. Allowing queries through when variable costs cannot be resolved

This is the core decision and the most consequential one. The cost validator exists as a protection mechanism — to prevent users from sending queries that would be too expensive to execute. Silently allowing queries through when cost cannot be fully computed means the protection can be bypassed by any attacker who routes cost-multiplying arguments through query variables rather than inline literals. A query like items(limit: $limit) with $limit = 10000 would pass the validator with a cost of 1.

The current behavior (rejecting queries) is clearly wrong — it breaks APIs that are misconfigured in a completely innocent way. But the proposed behavior (silently passing queries with incomplete cost estimates) is "quietly wrong" in a way that feels safe to a developer while offering reduced protection.

The third option — making the degradation visible — is absent from this PR.

Option	What it gives users	What it costs	Effort to change later
Current (reject query)	Loud signal of misconfiguration	Breaks APIs for innocent queries using variables	No change needed; just fix the root cause
This PR (silent fallback)	Queries work; cost counted where possible	Variable-driven costs unprotected with no warning; false sense of security	Changing later requires semantic versioning if behavior is documented
Emit a developer warning + allow query	Queries work; developer learns protection is partial	Slightly noisier logs; still no fix for the security gap	Same as this PR
Reject with a configuration-guidance message	Loud signal pointing toward correct configuration	Still breaks queries until developer fixes config	Same effort as this PR

The silent fallback is better than spurious rejection for developers who never intended to use variable-driven cost multipliers (i.e., most users). But for developers who specifically want to cap query cost by limiting variable values, this fix removes protection while looking like it resolves the problem.

Agree with direction, disagree with the absence of any developer signal.

---

2. None vs. explicit empty collection as different contracts

This distinction is subtle and creates a trap. A developer who reads that the default is "no variables" and then decides to be explicit — writing cost_validator(maximum_cost=100, variables={}) — now has a validator that rejects all queries using variables. This is the opposite of what most developers would expect from "making the default explicit." The behavior difference is documented only in a code comment and a test docstring, not in any user-facing API documentation.

The distinction does have a reasonable rationale (if you pass a dict, you're asserting it represents the available variables), but it is a footgun that the PR does not address.

Disagree with leaving this undocumented. The API contract should be visible at the point of configuration, not discoverable only through failure.

---

3. No signal when cost protection is incomplete

This is the decision the PR most clearly does not make — it is an absence of a decision. When variable-driven cost multipliers are skipped, the developer receives no indication. Queries pass, costs are computed, extensions report a cost number. Everything looks correct.

For a security feature, silent degradation is a meaningful product decision: it means developers who audit their configuration (by looking at query results) will see costs being reported and assume the numbers are complete. A log-level warning, a note in the extensions payload (e.g., "variableCostsSkipped": true), or any other signal would close this gap without changing the allow/reject decision.

Disagree with silent degradation for a security mechanism. A visible signal should accompany the fallback.

---

4. The static rule form is now implicitly a best-effort tool

This is consistent with prior art: many GraphQL cost-validation libraries distinguish between "static analysis" (approximate, no runtime values) and "runtime validation" (exact, with variables). Making the static form approximate is the right product direction. The problem is that this distinction is not surfaced anywhere a developer would find it — the API signature, the documentation, or the result extensions.

Agree with the direction; the framing needs to be explicit in documentation.

---

Open questions

• What proportion of real-world cost attacks rely on variable-driven multipliers (e.g., limit, first, count arguments) versus pure field-complexity accumulation? If variable multipliers are the primary risk vector, the silent fallback removes the most important protection.
• Does Ariadne have a convention for developer-facing warnings (log output, schema validation warnings, deprecation notices)? If yes, this PR should use it.
• Is there a path to make the static rule form capture per-request variables without requiring the callable form? (Many GraphQL server frameworks allow middleware to attach data to the validation context.)
• Should the cost extensions payload indicate whether the reported cost is exact or approximate? This would let monitoring tooling detect under-reported costs.

---

Recommendation

Ship with changes

The fix is correct in direction — spurious query rejection is worse than allowing queries through — but shipping a security feature that silently under-counts without any developer signal is a meaningful product risk. Before merging, add at minimum: (a) a developer-facing warning or extensions flag when variable costs are skipped, and (b) documentation that names the two modes (static/approximate vs. per-request/exact) and explains when each is appropriate. Without these, developers will configure the static form expecting full protection and have no way to discover the gap.

[prelint]

Caution

The cost-validator is a security control (DoS / query-complexity limiting). Silently falling back to field_args = {} when self.variables is None means any variable-driven multiplier is simply not...

^{ariadne/validation/query_cost.py:107}

1 finding(s) posted as inline comments.

Warning

docs/06-Extensions/03-query-validators.md (line 131) [doc_conflict]: The section "Exposing query variables to cost_validator" (lines 129–152) now describes deprecated behaviour as current: the stated invariant ("will raise an error") is no longer true, and the callable-workaround example exists solely to avoid that error. After this PR the section is misleading and the recommended workaround is no longer necessary for the common case — the docs must be updated to reflect the new semantics and any remaining reason to prefer the callable form (accurate cost accounting when multipliers are variable-driven).

[prelint]

Caution

The cost-validator is a security control (DoS / query-complexity limiting). Silently falling back to field_args = {} when self.variables is None means any variable-driven multiplier is simply not counted — a client can send $count=10000 and the validator computes cost as if the argument were absent. The documented design (see docs/06-Extensions/03-query-validators.md lines 131–152) deliberately surfaces this as an error and provides a callable pattern so callers explicitly forward per-request variables; degrading silently instead removes the incentive to apply that pattern and allows variable-based cost inflation to bypass the limit undetected.