Skip to content

Revise non-normative notes section #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 8, 2025
Merged

Revise non-normative notes section #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a152da
First draft of overhaul
benjie May 8, 2025
fa915fd
Grammar
benjie May 8, 2025
c566c90
Improve look of compiled spec
benjie May 8, 2025
ea80c00
Rework text around preflight
benjie May 8, 2025
1b73bc3
Fix capitalisation of WHATWG
benjie May 8, 2025
c7d5770
Add spellings
benjie May 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cspell.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ words:
- parallelization
- structs
- subselection
- mitigations
# Software
- ical
- WebDAV
- Protobuf
- WHATWG
- OWASP
# Deliberate typos
- qeury
- __typena
Expand Down
104 changes: 71 additions & 33 deletions spec/GraphQLOverHTTP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ accepting both media types, the client SHOULD indicate it prefers
For HTTP GET requests, the _GraphQL-over-HTTP request_ parameters MUST be
provided in the query component of the request URL, encoded in the
`application/x-www-form-urlencoded` format as specified by the
[WhatWG URLSearchParams class](https://url.spec.whatwg.org/#interface-urlsearchparams).
[WHATWG URLSearchParams class](https://url.spec.whatwg.org/#interface-urlsearchparams).

The {query} parameter MUST be the string representation of the source text of
the document as specified in
Expand All @@ -291,7 +291,7 @@ The {operationName} parameter, if supplied and not the empty string, represents
the name of the operation to be executed within the {query} as a string.

Note: In the final URL all of these parameters will appear in the query
component of the request URL as URL-encoded values due to the WhatWG
component of the request URL as URL-encoded values due to the WHATWG
URLSearchParams encoding specified above.

Setting the value of the {operationName} parameter to the empty string is
Expand Down Expand Up @@ -757,41 +757,79 @@ etc.

# Non-normative notes

This section of the specification is non-normative, even where the words and
phrases specified in RFC2119 are used.

## Security

In this specification, GET requests are not supported for mutations due to
security concerns. GET requests expose variables to logging mechanisms and
intermediaries due to the URL encoding of parameters, which can lead to
sensitive data being inadvertently logged. Furthermore, GET requests are
considered "simple requests" under CORS (Cross-Origin Resource Sharing), meaning
they bypass preflight checks that add a layer of security.

On the other hand, using `application/json` for request bodies mandates a CORS
preflight request, adding a security layer by ensuring the client has explicit
permission from the server before sending the actual request. This is
particularly important in mitigating cross-site request forgery (CSRF) attacks.

It's important to note that "simple requests" like those using
`application/x-www-form-urlencoded` or `multipart/form-data` do not have the
same CORS behavior, and thus do not undergo the same preflight checks.
Implementers should be aware of the security implications of using these types
of requests. While they can be secured with the right headers enforced by the
server, it is crucial to understand and properly account for the security risks
involved.

To mitigate these risks, it is recommended that servers require a custom header
to ensure requests are not "simple." For instance, a `GraphQL-Require-Preflight`
header can be used to indicate that a preflight check has occurred, providing an
additional layer of security.
This specification focuses solely on the intersection of GraphQL and HTTP.
General concerns of either technology, including security concerns, are out of
scope, except where their interaction introduces additional considerations.

### HTTP

Implementers are expected to have a solid understanding of the security
implications of exposing a service over HTTP, and are responsible for
implementing relevant mitigations and solutions. This specification will not
repeat standard HTTP best practices such as not using `GET` for requests with
side effects, safe logging of requests without revealing sensitive information,
ensuring all connections are encrypted via HTTPS, placing limits on the length
of incoming data, implementing rate limits, authorization and authentication
security, request tracing, intrusion detection, and so on.

### GraphQL

Implementers are further expected to have a solid understanding of the security
implications of running a GraphQL service and are responsible for implementing
relevant mitigations and solutions there. For example, they may: limit the size
and token count of GraphQL documents; ensure document validity; limit the number
of errors a response may return; limit information revealed via errors; enforce
validation and execution timeouts and pagination limits; implement query depth
and complexity limits; implement authentication and authorization; apply rate
limits to critical logic; and so on.

### Exercise caution

Where this specification leaves flexibility for the implementer, the implementer
should be very cautious when exercising this freedom. Implementers must make
themselves aware of and account for the security implications of their choices;
while many alternative choices can be secured, securing them is outside of the
scope of this specification.

For example, this specification allows alternative media types to be used to
encode the request body; however, media types such as `multipart/form-data` or
`application/x-www-form-urlencoded` may result in the request being treated by a
browser as a "simple request", which does not require a "preflight", thereby
opening the server up to Cross-Site Request Forgery (CSRF/XSRF) attacks. The
recommended `application/json` media type requires a "preflight" check when
issued cross-domain. See
[CORS protocol](https://fetch.spec.whatwg.org/#http-cors-protocol) in the WHATWG
Fetch spec for more details on this.
Comment on lines +799 to +807
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that this PR still retains explicit comments regarding the use of multipart/form-data, as this is not evident by the rest of the RFC and is important enough to worth of being explicitly noted, as it was/is a common practice.


Note: One approach used by the community to mitigate CSRF risks is to ensure a
request is not "simple" by requiring a custom header—such as
`GraphQL-Require-Preflight`—is included. The presence of a custom header forces
browsers to enact a "preflight" check, thereby adding an additional layer of
security. (This is not a standard header, and many alternative headers could
serve the same purpose. This is presented merely as an example of a pattern seen
in the community.)
Comment on lines +809 to +815
Copy link
Contributor

Choose a reason for hiding this comment

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

What about strengthening the suggestion of the header name to be used here, to help the community settle on a single header within servers that support multipart/form-data? While the above text is completely accurate, it's written as though GraphQL-Require-Preflight were only a random example, not necessarily the best choice. Giving guidance and suggestions within non-normative specifications does not seem to be contrary to its purpose, and would likely be appreciated by the community, in this case assisting implementations that wish to support multipart/form-data for various reasons (e.g. out-of-band file upload). Mentioning the header that Apollo uses (Apollo-Require-Preflight) may also be beneficial in this paragraph.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not convinced that requiring a preflight is a good security practice; the vast majority of GraphQL APIs IMO should either be closed off entirely (no cross-origin requests, first party only, trusted documents), or they should not support any credentials caching that a preflight would be relevant to (include all credentials in the request, none implicit). The <1% of use cases that fall outside of this should have their own experts who know what to do. Given that, I don't want us to be seen as standardizing one because it looks like we're also giving the approach legitimacy that I'm not sure it warrants. IMO it's a workaround to poor architectural choices or restrictions.


Further extending this example, using `multipart/form-data` may allow large
values to be referenced multiple times in a GraphQL operation, potentially
causing the GraphQL service to process a much larger GraphQL request than the
HTTP request size would suggest.

### Other resources

For more detailed security considerations, please refer to
[RFC 7231](https://tools.ietf.org/html/rfc7231),
[RFC 6454](https://tools.ietf.org/html/rfc6454), and other relevant RFCs.
[RFC 6454](https://tools.ietf.org/html/rfc6454), other relevant RFCs, and other
resources such as [OWASP](https://owasp.org).

## Request format compatibility
## Future compatibility

Supporting formats not described by this specification, such as XML or Protobuf,
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed these examples because I didn't want to imply that we might (or might not) be expanding to these formats in future. Clearer to exclude them IMO.

may have potential conflicts with future versions of this specification as
ongoing development aims to standardize and ensure the security and
interoperability of GraphQL over HTTP. For this reason, it is recommended to
adhere to the officially recognized formats outlined here.
Supporting formats not described by this specification may have potential
conflicts with future versions of this specification as ongoing development aims
to standardize and ensure the security and interoperability of GraphQL over HTTP
whilst accounting for its growing feature set. For this reason, it is
recommended to adhere to the officially recognized formats outlined here.