mirror of
https://github.com/kennethreitz/responder.git
synced 2026-07-21 18:39:29 +00:00
kennethreitz
2acb0f8100
## Summary
A fresh security audit of the areas the earlier review didn't cover
turned this up in the GraphQL extension
(`responder/ext/graphql/__init__.py`).
The view executed whatever operation arrived — including mutations sent
via **GET** query params:
```
GET /graphql?query=mutation{ createUser(name:"eve"){ ok } }
```
`GET` must be safe and idempotent. Running a mutation over `GET`:
- is **CSRF-able** — a plain `<img src="/graphql?query=mutation{...}">`
fires it with the victim's cookies, no token needed;
- is **cacheable / loggable** — proxies and browsers may cache or replay
it.
This is the standard GraphQL-over-HTTP requirement (GET = queries only).
## Fix
On a `GET` request, parse the document, find the operation that would
execute (respecting `operationName`), and if it's a
mutation/subscription, return **`405`** with `Allow: POST` instead of
executing it. Query operations over `GET` are unaffected, and `POST`
mutations work as before. A syntax error falls through to normal
execution so the existing error path still reports it.
## Tests
New `tests/test_graphql_get_csrf.py`: mutation-over-GET → 405,
query-over-GET → 200, mutation-over-POST → 200, and a named-operation
mutation-over-GET → 405. Existing `tests/test_graphql.py` unaffected.
Full suite: **759 passed**, 1 skipped (php not installed); ruff clean;
mypy clean.
## Audit notes (what I looked at but did *not* change)
- **GraphiQL / introspection on by default** — real exposure, but
flipping the defaults is an opinionated breaking change and they're
already documented as "set `False` in production." Left as-is.
- **Static file serving** — confirmed safe; delegates to Starlette's
hardened `StaticFiles` (realpath/commonpath traversal checks).
- **clientgen path-param escaping** — a separate, lower-severity
correctness issue (an untrusted OpenAPI spec with a quote/newline in a
path-param name breaks the generated client's string literals). Worth a
follow-up PR; not bundled here.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
73 lines
2.3 KiB
Python
73 lines
2.3 KiB
Python
# ruff: noqa: E402
|
|
"""GET requests may only run GraphQL query operations, never mutations.
|
|
|
|
Allowing a mutation over GET makes it CSRF-able (a plain ``<img src>`` would
|
|
trigger it with the victim's cookies) and cacheable. Mutations must use POST.
|
|
"""
|
|
|
|
import pytest
|
|
|
|
graphene = pytest.importorskip("graphene")
|
|
|
|
from responder.ext.graphql import GraphQLView
|
|
|
|
|
|
@pytest.fixture
|
|
def mutation_schema():
|
|
class Query(graphene.ObjectType):
|
|
hello = graphene.String()
|
|
|
|
def resolve_hello(self, info):
|
|
return "hi"
|
|
|
|
class CreateUser(graphene.Mutation):
|
|
class Arguments:
|
|
name = graphene.String(required=True)
|
|
|
|
ok = graphene.Boolean()
|
|
|
|
def mutate(self, info, name):
|
|
return CreateUser(ok=True)
|
|
|
|
class Mutation(graphene.ObjectType):
|
|
create_user = CreateUser.Field()
|
|
|
|
return graphene.Schema(query=Query, mutation=Mutation)
|
|
|
|
|
|
def test_mutation_over_get_is_rejected(api, mutation_schema):
|
|
api.add_route("/", GraphQLView(schema=mutation_schema, api=api))
|
|
r = api.requests.get(
|
|
'http://;/?query=mutation { createUser(name: "eve") { ok } }',
|
|
headers={"Accept": "json"},
|
|
)
|
|
assert r.status_code == 405
|
|
assert r.headers["Allow"] == "POST"
|
|
assert "POST" in r.json()["errors"][0]["message"]
|
|
|
|
|
|
def test_query_over_get_still_works(api, mutation_schema):
|
|
api.add_route("/", GraphQLView(schema=mutation_schema, api=api))
|
|
r = api.requests.get("http://;/?query={ hello }", headers={"Accept": "json"})
|
|
assert r.status_code == 200
|
|
assert r.json() == {"data": {"hello": "hi"}}
|
|
|
|
|
|
def test_mutation_over_post_still_works(api, mutation_schema):
|
|
api.add_route("/", GraphQLView(schema=mutation_schema, api=api))
|
|
r = api.requests.post(
|
|
"http://;/", json={"query": 'mutation { createUser(name: "eve") { ok } }'}
|
|
)
|
|
assert r.status_code == 200
|
|
assert r.json() == {"data": {"createUser": {"ok": True}}}
|
|
|
|
|
|
def test_named_mutation_over_get_is_rejected(api, mutation_schema):
|
|
# A document with a named mutation selected by operationName is also blocked.
|
|
api.add_route("/", GraphQLView(schema=mutation_schema, api=api))
|
|
doc = 'mutation M { createUser(name: "eve") { ok } }'
|
|
r = api.requests.get(
|
|
f"http://;/?query={doc}&operationName=M", headers={"Accept": "json"}
|
|
)
|
|
assert r.status_code == 405
|