Skip to content

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

Open
jbbqqf wants to merge 1 commit into
mirumee:mainfrom
jbbqqf:fix/1319-cost-validator-variables
Open

fix: don't reject queries using variables when cost_validator has none (#1319)#1325
jbbqqf wants to merge 1 commit into
mirumee:mainfrom
jbbqqf:fix/1319-cost-validator-variables

fix: don't reject queries with variables when cost_validator has no v…

15d4051
Select commit
Loading
Failed to load commit list.
prelint / Prelint: Decision Review succeeded May 26, 2026 in 2m 48s

Decision Review

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.