Skip to content

Multi-operation documents: same-name top-level groups cross-contaminate selections — silent field leakage on ≤6.1.2 #142

Description

@allmonday

Summary

A single GraphQL document with two operations selecting the same top-level group silently cross-contaminates the two selections on nexusx ≤ 6.1.2: the first operation is serialized with the second operation's projection tree, leaking fields it never asked for. On the feat/gql-alias-support branch (specs/023, unreleased) the same query is rejected loudly with a response-key conflict error instead — strictly safer, but still not the correct end state (parsing per operation).

No aliases are required to trigger this — it is independent of #140.

Environment

  • nexusx 6.1.2 (master, 5d03777) — silent field leakage
  • Surface: any consumer of QueryParser.parse_document — entity-first GraphQLHandler.execute and execute_compose_query
  • On branch feat/gql-alias-support (specs/023) the same query raises ResponseKeyConflictError (explicit rejection)

Reproduction

mutation { FixtureUser { get { id } } }        # asks for id only
query    { FixtureUser { get { id email } } }  # asks for id + email

On master, executing this document through QueryExecutor:

parsed = QueryParser().parse_document(parse(doc))
print(list(parsed))                                # ['FixtureUser'] — one entry, the later operation won
print(parsed["FixtureUser"].sub_fields["get"].sub_fields)  # {'id', 'email'} — the QUERY's projection tree

result = await executor.execute_query(doc, ..., parsed, ...)
# data: {"FixtureUser": {"get": [{"id": 1, "email": "a@t"}]}}
#        ^ the mutation asked for { id } only — the response leaks `email`

Two layers of contamination:

  1. Projection tree mismatchparse_document merges all operations' top-level selections into one dict keyed by response key; the later operation's FieldSelection overwrites the earlier one, so the first operation serializes with the second's sub-field set.
  2. Response overwriteexecute_query writes results into data[group_key] across all operations, so only the last operation's result survives.

Side effects are not contained either: the executor loops over all definitions, so mutations in both operations execute.

On the 023 branch

The duplicate-response-key detection added by specs/023 makes the same query fail loudly:

ResponseKeyConflictError: Response key conflict: 'FixtureUser' is selected more
than once at 'top level'. ... field merging is not supported.

Workaround there: alias one of the groups (q: FixtureUser), which distinguishes the response keys.

Root cause

QueryParser.parse_document iterates all definitions and merges every operation's top-level selections into a single flat dict, discarding operation ownership:

result: dict[str, FieldSelection] = {}
for definition in document.definitions:            # all operations
    for selection in definition.selection_set.selections:
        key = alias or operation_name
        if key in result: raise ResponseKeyConflictError(...)   # 023 branch
        result[key] = meta                        # master: silent overwrite
return result

The executors then walk the document per operation but look selections up in that shared dict — the data flow loses the operation dimension between parse and execute.

Impact

  • Trigger: one document, ≥2 operations, same top-level group name (no alias distinguishing them).
  • Real-world exposure is low — mainstream clients send one operation per request — but when triggered on ≤6.1.2 the leakage is completely silent: an operation that declared a minimal projection returns fields another operation declared, including sensitive ones.
  • Security angle: if any operation in the document selects sensitive fields, every earlier operation selecting the same group leaks them in its response.

GraphQL spec note

  • Multi-operation documents are legal; the spec requires executing exactly one operation per request (operationName, or the single operation when unnamed).
  • nexusx's executor instead executes all operations (batch semantics). GraphQLHandler.execute accepts an operation_name parameter but execute_query does not use it for selection. Aligning with the spec would also defuse this bug class.

Suggested fix directions

  1. Parse per operation — return a structure carrying the operation dimension (e.g. list of (operation, selections)); parse_document's return type changes, so all callers follow. Best paired with a major release or a parallel method.
  2. Honor operationName — execute only the selected operation (spec alignment); changes the current batch-execution semantics, needs a decision.
  3. Conservative — keep the 023 rejection, mention the alias workaround in the error message, document the limitation.

1 + 2 together are the spec-correct end state.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions