diff --git a/docs-website/docs.json b/docs-website/docs.json index b9a4c109ee..db5adf2941 100644 --- a/docs-website/docs.json +++ b/docs-website/docs.json @@ -234,7 +234,8 @@ "router/security/tls", "router/security/config-validation-and-signing", "router/security/hardening-guide", - "router/security/cost-control" + "router/security/cost-control", + "router/security/disallow-inline-arguments" ] }, { diff --git a/docs-website/router/configuration.mdx b/docs-website/router/configuration.mdx index 593cb26c83..8dc1df5495 100644 --- a/docs-website/router/configuration.mdx +++ b/docs-website/router/configuration.mdx @@ -2286,6 +2286,12 @@ The configuration for the security. The security is used to configure the securi | | parser_limits.approximate_depth_limit | | The approximate cumulative depth limit of a query, including fragments. Set to 0 to disable. | 200 | | | parser_limits.total_fields_limit | | The total number of fields the parser will allow. Set to 0 to disable. | 3500 | | SECURITY_OPERATION_NAME_LENGTH_LIMIT | operation_name_length_limit | | The maximum allowed length for the operation name. Set to 0 to disable. | 512 | +| SECURITY_DISALLOW_INLINE_ARGUMENTS | disallow_inline_arguments | | Detect or reject operations that hardcode argument values instead of using variables. See [Disallow Inline Arguments](/router/security/disallow-inline-arguments). | | +| SECURITY_DISALLOW_INLINE_ARGUMENTS_MODE | disallow_inline_arguments.mode | | `off` disables the check; `warn` logs and annotates the response; `enforce` rejects the operation. | off | +| SECURITY_DISALLOW_INLINE_ARGUMENTS_ENFORCE_HTTP_STATUS_CODE | disallow_inline_arguments.enforce_http_status_code | | The HTTP status code returned in enforce mode. Use 200 for strict GraphQL-spec compliance. | 400 | +| SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_CODE | disallow_inline_arguments.error_code | | The GraphQL error `extensions.code` emitted on rejection or annotation. | INLINE_ARGUMENT_VALUES_NOT_ALLOWED | +| SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_MESSAGE | disallow_inline_arguments.error_message | | The human-readable error or hint message. | Inline argument values are not allowed. Use variables instead. | +| SECURITY_DISALLOW_INLINE_ARGUMENTS_INCLUDE_PERSISTED_OPERATIONS | disallow_inline_arguments.include_persisted_operations | | When true, the policy also applies to persisted operations. By default they are exempt. | false | ### Example YAML Configuration @@ -2328,6 +2334,12 @@ security: approximate_depth_limit: 300 total_fields_limit: 700 operation_name_length_limit: 1024 + disallow_inline_arguments: + mode: warn # off | warn | enforce + enforce_http_status_code: 400 + error_code: "INLINE_ARGUMENT_VALUES_NOT_ALLOWED" + error_message: "Inline argument values are not allowed. Use variables instead." + include_persisted_operations: false ``` diff --git a/docs-website/router/security/disallow-inline-arguments.mdx b/docs-website/router/security/disallow-inline-arguments.mdx new file mode 100644 index 0000000000..7485c59e85 --- /dev/null +++ b/docs-website/router/security/disallow-inline-arguments.mdx @@ -0,0 +1,139 @@ +--- +title: "Disallow Inline Arguments" +description: "Detect or reject GraphQL operations that hardcode argument values instead of using variables" +icon: brackets-curly +--- + +## Overview + +Inline argument values embed request data directly in the query document: + +```graphql +query { + employee(id: 1) { + id + } +} +``` + +The same operation written with variables keeps the document stable across requests: + +```graphql +query ($id: Int!) { + employee(id: $id) { + id + } +} +``` + +Hardcoded values cause practical problems at scale. Every distinct value produces a distinct document, which defeats operation-level caching and inflates normalization and plan cache cardinality. Inline values also leak request data such as user IDs into query documents, where they end up in logs and traces. Operations with inline values cannot be registered as persisted operations without one registration per value. + +The `security.disallow_inline_arguments` policy detects inline values in field arguments and directive arguments. Depending on the configured mode, the router logs them, annotates the response, or rejects the operation. Detection runs on the normalized operation, before inline values are extracted into variables, so it checks exactly what the router will execute: inline values in a non-executed sibling operation of a multi-operation document, in an unused fragment, or in a statically resolved `@skip`/`@include` directive are not flagged, because normalization removes them from the executed operation. + +Values supplied through variables are compliant. This includes variable-definition default values and variable usage in directives such as `@include(if: $flag)`. A literal directive argument on a directive that survives normalization is flagged; `@include(if: true)` and `@skip(if: false)` are resolved away by normalization and therefore compliant. + +## Modes + +| Mode | Behavior | +| --------- | ----------------------------------------------------------------------------------------------------------- | +| `off` | The check is disabled. This is the default. | +| `warn` | The operation executes normally. The router logs a warning and annotates the response with an extensions hint. | +| `enforce` | The router rejects the operation before execution. | + +The recommended migration path is to run `warn` mode first, identify offending clients from the logs, migrate them to variables, and then switch to `enforce`. + +## Configuration + +```yaml +security: + disallow_inline_arguments: + mode: warn # off | warn | enforce + enforce_http_status_code: 400 + error_code: "INLINE_ARGUMENT_VALUES_NOT_ALLOWED" + error_message: "Inline argument values are not allowed. Use variables instead." + include_persisted_operations: false +``` + +| Option | Default | Description | +| ------------------------------ | ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | +| `mode` | `off` | Controls the policy behavior. One of `off`, `warn`, `enforce`. | +| `enforce_http_status_code` | `400` | The HTTP status code returned in enforce mode. Use `200` for strict GraphQL-spec compliance. | +| `error_code` | `INLINE_ARGUMENT_VALUES_NOT_ALLOWED` | The `extensions.code` value emitted on rejection or annotation. | +| `error_message` | `Inline argument values are not allowed. Use variables instead.` | The human-readable error or hint message. | +| `include_persisted_operations` | `false` | When `true`, the policy also applies to persisted operations. | + +Each option has an environment variable equivalent: `SECURITY_DISALLOW_INLINE_ARGUMENTS_MODE`, `SECURITY_DISALLOW_INLINE_ARGUMENTS_ENFORCE_HTTP_STATUS_CODE`, `SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_CODE`, `SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_MESSAGE`, and `SECURITY_DISALLOW_INLINE_ARGUMENTS_INCLUDE_PERSISTED_OPERATIONS`. + +## Warn Mode + +In warn mode the operation executes normally and the response gains an `extensions.inlineArguments` annotation listing every offending argument by name and value kind. For the request `{ employee(id: 1) { id } }` the response is: + +```json +{ + "data": { + "employee": { + "id": 1 + } + }, + "extensions": { + "inlineArguments": { + "code": "INLINE_ARGUMENT_VALUES_NOT_ALLOWED", + "message": "Inline argument values are not allowed. Use variables instead.", + "arguments": [ + { "argument": "id", "valueKind": "Int" } + ] + } + } +} +``` + +The router also emits a structured warning log, `inline arguments found in operation`, with the argument count, argument names, operation name, and the client name and version. Use these fields to build a per-client breakdown of inline argument usage before switching to enforce mode. + + + Subscription responses stream and cannot carry a response annotation. For + subscriptions and for operations sent over WebSocket connections, warn mode + logs only. Queries using `@defer` receive the annotation in the `extensions` + of the initial multipart payload. + + +## Enforce Mode + +In enforce mode the router rejects the operation with the configured HTTP status code during validation, before planning and execution. No subgraph requests are made. A schema-invalid operation is rejected with its schema-validation error first; this policy applies to valid operations. The same request is rejected with: + +```json +{ + "errors": [ + { + "message": "Inline argument values are not allowed. Use variables instead.", + "extensions": { + "code": "INLINE_ARGUMENT_VALUES_NOT_ALLOWED", + "inlineArguments": { + "code": "INLINE_ARGUMENT_VALUES_NOT_ALLOWED", + "message": "Inline argument values are not allowed. Use variables instead.", + "arguments": [ + { "argument": "id", "valueKind": "Int" } + ] + } + } + } + ] +} +``` + +The top-level `extensions.code` supports client and APM error classification. The nested `inlineArguments` object carries the full detail for debugging. + +## Transports + +The policy applies to HTTP requests and WebSocket operations. + +Over WebSocket connections, enforce mode rejects the operation with a terminal `error` message carrying the same `extensions` payload as the HTTP response. The `enforce_http_status_code` option does not apply because WebSocket errors travel as protocol messages. + +## Persisted Operations + +Registered persisted operations are exempt by default. They are stored server-side and their contents are intentional. Set `include_persisted_operations: true` to apply the policy to them as well. + +Automatic persisted queries (APQ) are never exempt. Their hashes are computed by the client, so they carry no operator intent and are checked like ad-hoc operations. + +## Telemetry + +When inline arguments are detected on an HTTP request, the router sets the `wg.operation.inline_arguments.count` attribute on the router root span, in both warn and enforce mode. Combine it with the client attributes on the same span to track which clients still send inline arguments. diff --git a/router-tests/security/disallow_inline_arguments_test.go b/router-tests/security/disallow_inline_arguments_test.go new file mode 100644 index 0000000000..bf2e5264dc --- /dev/null +++ b/router-tests/security/disallow_inline_arguments_test.go @@ -0,0 +1,729 @@ +package integration + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/wundergraph/cosmo/router-tests/testenv" + "github.com/wundergraph/cosmo/router/core" + "github.com/wundergraph/cosmo/router/pkg/config" + "github.com/wundergraph/cosmo/router/pkg/otel" + "github.com/wundergraph/cosmo/router/pkg/trace/tracetest" + sdktracetest "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.uber.org/zap/zapcore" +) + +// requireInlineArgumentsLogEntry asserts the single Warn log emitted for a +// FindEmployee operation with one inline `id` argument, sent by test-client/1.2.3. +func requireInlineArgumentsLogEntry(t *testing.T, xEnv *testenv.Environment) { + t.Helper() + logs := xEnv.Observer().FilterMessage("inline arguments found in operation") + require.Equal(t, 1, logs.Len()) + entry := logs.All()[0] + require.Equal(t, zapcore.WarnLevel, entry.Level) + cm := entry.ContextMap() + require.Equal(t, int64(1), cm["count"]) + require.Equal(t, []interface{}{"id"}, cm["arguments"]) + require.Equal(t, "FindEmployee", cm["operation_name"]) + require.Equal(t, "test-client", cm["client_name"]) + require.Equal(t, "1.2.3", cm["client_version"]) +} + +// deferMultipartParts splits a multipart/mixed @defer body on the --graphql +// boundary and returns the raw JSON bytes of each part. +func deferMultipartParts(body []byte) [][]byte { + var result [][]byte + for _, part := range bytes.Split(body, []byte("--graphql")) { + if bytes.HasPrefix(part, []byte("--")) { + continue + } + _, jsonBody, found := bytes.Cut(part, []byte("\r\n\r\n")) + if !found { + continue + } + jsonBody = bytes.TrimSpace(jsonBody) + if len(jsonBody) == 0 { + continue + } + result = append(result, jsonBody) + } + return result +} + +// inlineArgumentsSpanCounts returns the wg.operation.inline_arguments.count value +// of every exported span that carries the attribute. +func inlineArgumentsSpanCounts(exporter *sdktracetest.InMemoryExporter) []int64 { + var counts []int64 + for _, span := range exporter.GetSpans().Snapshots() { + for _, attr := range span.Attributes() { + if attr.Key == otel.WgOperationInlineArgumentsCount { + counts = append(counts, attr.Value.AsInt64()) + } + } + } + return counts +} + +// withInlineArgumentsMode returns a testenv security-configuration modifier that +// enables the disallow-inline-arguments policy in the given mode. +func withInlineArgumentsMode(mode config.DisallowInlineArgumentsMode) func(*config.SecurityConfiguration) { + return func(s *config.SecurityConfiguration) { + s.DisallowInlineArguments = config.DisallowInlineArguments{Mode: mode} + } +} + +func TestDisallowInlineArguments(t *testing.T) { + t.Parallel() + + t.Run("off by default, inline query succeeds normally", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"employee":{"id":1}}}`, res.Body) + }) + }) + + t.Run("enforce mode rejects inline field argument", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.JSONEq(t, `{"errors":[{"message":"Inline argument values are not allowed. Use variables instead.","extensions":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"}]}}}]}`, res.Body) + }) + }) + + t.Run("enforce mode allows compliant operation", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query GetEmployee($id: Int!) { employee(id: $id) { id } }`, + Variables: json.RawMessage(`{"id":1}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Contains(t, res.Body, `"data"`) + require.NotContains(t, res.Body, `"errors"`) + }) + }) + + // Normalization resolves @skip/@include directives with a static condition, + // so their inline boolean is never reported. + t.Run("enforce mode allows statically resolved include directive", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query($id: Int!) { employee(id: $id) @include(if: true) { id } }`, + Variables: json.RawMessage(`{"id":1}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"employee":{"id":1}}}`, res.Body) + }) + }) + + t.Run("enforce mode ignores non-executed sibling operation", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query A($id: Int!) { employee(id: $id) { id } } query B { employee(id: 1) { id } }`, + OperationName: []byte(`"A"`), + Variables: json.RawMessage(`{"id":1}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"employee":{"id":1}}}`, res.Body) + }) + }) + + t.Run("enforce mode rejects inline argument inside used fragment", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query { ...F } fragment F on Query { employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.JSONEq(t, `{"errors":[{"message":"Inline argument values are not allowed. Use variables instead.","extensions":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"}]}}}]}`, res.Body) + }) + }) + + // Introspection-field arguments are ordinary arguments; they are not exempt. + t.Run("enforce mode rejects inline introspection argument", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ __type(name: "Employee") { name } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Contains(t, res.Body, `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + }) + }) + + // Batch entries are processed independently: one offending entry is rejected + // without affecting the compliant entries in the same batch. + t.Run("enforce mode rejects only the offending batch entry", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + BatchingConfig: config.BatchingConfig{ + Enabled: true, + MaxConcurrency: 10, + MaxEntriesPerBatch: 100, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLBatchedRequestRequest([]testenv.GraphQLRequest{ + { + Query: `query GetEmployee($id: Int!) { employee(id: $id) { id } }`, + Variables: json.RawMessage(`{"id":1}`), + }, + { + Query: `{ employee(id: 1) { id } }`, + }, + }, nil) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + + var entries []json.RawMessage + require.NoError(t, json.Unmarshal([]byte(res.Body), &entries)) + require.Len(t, entries, 2) + require.Equal(t, `{"data":{"employee":{"id":1}}}`, string(entries[0])) + require.Contains(t, string(entries[1]), `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + }) + }) + + t.Run("enforce mode custom status code and message", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(s *config.SecurityConfiguration) { + s.DisallowInlineArguments = config.DisallowInlineArguments{ + Mode: config.DisallowInlineArgumentsModeEnforce, + EnforceHTTPStatusCode: http.StatusUnprocessableEntity, + ErrorCode: "VARIABLES_REQUIRED", + ErrorMessage: "Please use variables.", + } + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusUnprocessableEntity, res.Response.StatusCode) + require.JSONEq(t, `{"errors":[{"message":"Please use variables.","extensions":{"code":"VARIABLES_REQUIRED","inlineArguments":{"code":"VARIABLES_REQUIRED","message":"Please use variables.","arguments":[{"argument":"id","valueKind":"Int"}]}}}]}`, res.Body) + }) + }) + + t.Run("warn mode returns success with extensions annotation and logs a warning", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + LogObservation: testenv.LogObservationConfig{ + Enabled: true, + LogLevel: zapcore.WarnLevel, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "test-client") + header.Add("graphql-client-version", "1.2.3") + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query FindEmployee { employee(id: 1) { id } }`, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"employee":{"id":1}},"extensions":{"inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"}]}}}`, res.Body) + + requireInlineArgumentsLogEntry(t, xEnv) + }) + }) + + // Schema-invalid operations never execute, so the check is skipped for them: + // its warn log and span count would only skew the migration telemetry that + // operators use to find non-compliant clients. + t.Run("warn mode does not log for schema-invalid operations", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + LogObservation: testenv.LogObservationConfig{ + Enabled: true, + LogLevel: zapcore.WarnLevel, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ nonExistentField(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Contains(t, res.Body, `"errors"`) + require.NotContains(t, res.Body, `"inlineArguments"`) + require.Equal(t, 0, xEnv.Observer().FilterMessage("inline arguments found in operation").Len()) + }) + }) + + // Pins the cache-consistency property documented on detectInlineArguments. + t.Run("warn mode reports identically across normalization cache miss and hit", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + query := `query { ...F } fragment F on Query { employee(id: 1) { id } }` + want := `{"data":{"employee":{"id":1}},"extensions":{"inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"}]}}}` + for range 2 { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{Query: query}) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, want, res.Body) + } + }) + }) + + t.Run("enforce mode logs a warning with client details", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + LogObservation: testenv.LogObservationConfig{ + Enabled: true, + LogLevel: zapcore.WarnLevel, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "test-client") + header.Add("graphql-client-version", "1.2.3") + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query FindEmployee { employee(id: 1) { id } }`, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + + requireInlineArgumentsLogEntry(t, xEnv) + }) + }) + + // Enforce-mode rejections must attribute their error code like other + // pre-execution rejections, so operators can dashboard the warn->enforce + // migration by error code in access logs and metrics. + t.Run("enforce mode rejection records the error code in access logs", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + AccessLogFields: []config.CustomAttribute{ + { + Key: "error_codes", + ValueFrom: &config.CustomDynamicAttribute{ + ContextField: core.ContextFieldGraphQLErrorCodes, + }, + }, + }, + LogObservation: testenv.LogObservationConfig{ + Enabled: true, + LogLevel: zapcore.InfoLevel, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + + requestLog := xEnv.Observer().FilterMessage("/graphql").All() + require.Len(t, requestLog, 1) + require.Equal(t, []interface{}{"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"}, requestLog[0].ContextMap()["error_codes"]) + }) + }) + + t.Run("inline arguments count is set on the router span", func(t *testing.T) { + t.Parallel() + exporter := tracetest.NewInMemoryExporter(t) + testenv.Run(t, &testenv.Config{ + TraceExporter: exporter, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, []int64{1}, inlineArgumentsSpanCounts(exporter)) + }) + }) + + t.Run("inline arguments count is set on the router span on rejection", func(t *testing.T) { + t.Parallel() + exporter := tracetest.NewInMemoryExporter(t) + testenv.Run(t, &testenv.Config{ + TraceExporter: exporter, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `{ employee(id: 1) { id } }`, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, []int64{1}, inlineArgumentsSpanCounts(exporter)) + }) + }) + + t.Run("inline arguments count is absent for compliant operations", func(t *testing.T) { + t.Parallel() + exporter := tracetest.NewInMemoryExporter(t) + testenv.Run(t, &testenv.Config{ + TraceExporter: exporter, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query GetEmployee($id: Int!) { employee(id: $id) { id } }`, + Variables: json.RawMessage(`{"id":1}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.NotEmpty(t, exporter.GetSpans().Snapshots()) + require.Empty(t, inlineArgumentsSpanCounts(exporter)) + }) + }) + + // The stored persisted operation 4000...0 is + // `query MyQuery($yes: Boolean! = true) { employee(id: 1) { details { forename surname @include(if: $yes) } } }` + // which carries exactly one inline argument (id: 1); the variable-definition + // default and the @include(if: $yes) variable are compliant. + t.Run("enforce mode exempts persisted operations by default", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "my-client") + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + OperationName: []byte(`"MyQuery"`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "4000000000000000000000000000000000000000000000000000000000000000"}}`), + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"employee":{"details":{"forename":"Jens","surname":"Neuse"}}}}`, res.Body) + }) + }) + + // APQ hashes are computed by the client, so an APQ operation carries no operator + // intent: the persisted-operation exemption must not apply, or any client could + // bypass enforce mode by attaching a persistedQuery extension to its request. + t.Run("enforce mode applies to automatic persisted queries", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ApqConfig: config.AutomaticPersistedQueriesConfig{ + Enabled: true, + Cache: config.AutomaticPersistedQueriesCacheConfig{Size: 1024 * 1024}, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + query := `{ employee(id: 1) { id } }` + hash := sha256.Sum256([]byte(query)) + extensions := []byte(fmt.Sprintf(`{"persistedQuery": {"version": 1, "sha256Hash": "%s"}}`, hex.EncodeToString(hash[:]))) + header := make(http.Header) + header.Add("graphql-client-name", "my-client") + + // The APQ registration request carries the query body and its client-computed hash. + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: query, + Extensions: extensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Contains(t, res.Body, `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + + // A hash-only request served from the APQ cache is rejected as well. + res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Extensions: extensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Contains(t, res.Body, `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + }) + }) + + // A REGISTERED persisted operation sent APQ-style (query body + sha256 hash, + // the shape Apollo clients use to seed APQ) must classify as registered, not + // APQ: the exemption applies, and the cached entry must not poison subsequent + // hash-only requests for the same operation. + t.Run("enforce mode exempts registered operation sent as body plus hash", func(t *testing.T) { + t.Parallel() + + // The stored operation 49a2f7dd... for my-client; the body matches the + // fixture byte-for-byte so its sha256 equals the registered hash and the + // prehandler's hash-body match check passes. + const registeredOpBody = "mutation updateEmployeeTag {\n updateEmployeeTag(id: 10, tag: \"dd\") {\n id\n }\n}" + registeredOpExtensions := []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "49a2f7dd56b06f620c7d040dd9d562a1c16eadf7c149be5decdd62cfc92e1b12"}}`) + + apqConfig := config.AutomaticPersistedQueriesConfig{ + Enabled: true, + Cache: config.AutomaticPersistedQueriesCacheConfig{Size: 1024 * 1024}, + } + + t.Run("body plus hash is exempt and does not poison hash-only requests", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ApqConfig: apqConfig, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "my-client") + + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: registeredOpBody, + Extensions: registeredOpExtensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"updateEmployeeTag":{"id":10}}}`, res.Body) + + // The body+hash request above must not have cached the operation as + // APQ; a hash-only follow-up stays exempt. + res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Extensions: registeredOpExtensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"updateEmployeeTag":{"id":10}}}`, res.Body) + }) + }) + + t.Run("hash-only first then body plus hash are both exempt", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ApqConfig: apqConfig, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "my-client") + + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Extensions: registeredOpExtensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"updateEmployeeTag":{"id":10}}}`, res.Body) + + res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: registeredOpBody, + Extensions: registeredOpExtensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.Equal(t, `{"data":{"updateEmployeeTag":{"id":10}}}`, res.Body) + }) + }) + + // Registered operations are per-client: the same body+hash from a client + // the operation is not registered for classifies as APQ and stays subject + // to the policy. + t.Run("body plus hash from another client is not exempt", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ApqConfig: apqConfig, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "other-client") + + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: registeredOpBody, + Extensions: registeredOpExtensions, + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Contains(t, res.Body, `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + }) + }) + }) + + t.Run("enforce mode rejects persisted operations when included", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(s *config.SecurityConfiguration) { + s.DisallowInlineArguments = config.DisallowInlineArguments{ + Mode: config.DisallowInlineArgumentsModeEnforce, + IncludePersistedOperations: true, + } + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + header := make(http.Header) + header.Add("graphql-client-name", "my-client") + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + OperationName: []byte(`"MyQuery"`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "4000000000000000000000000000000000000000000000000000000000000000"}}`), + Header: header, + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.JSONEq(t, `{"errors":[{"message":"Inline argument values are not allowed. Use variables instead.","extensions":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"}]}}}]}`, res.Body) + }) + }) + + t.Run("enforce mode rejects inline arguments over websocket", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + conn := xEnv.InitGraphQLWebSocketConnection(nil, nil, nil) + err := testenv.WSWriteJSON(t, conn, testenv.WebSocketMessage{ + ID: "1", + Type: "subscribe", + Payload: []byte(`{"query":"subscription($i: Int!) { countEmp(max: 5, intervalMilliseconds: $i) }","variables":{"i":500}}`), + }) + require.NoError(t, err) + var res testenv.WebSocketMessage + err = testenv.WSReadJSON(t, conn, &res) + require.NoError(t, err) + require.Equal(t, "error", res.Type) + require.Equal(t, "1", res.ID) + require.JSONEq(t, `[{"message":"Inline argument values are not allowed. Use variables instead.","extensions":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","inlineArguments":{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"max","valueKind":"Int"}]}}}]`, string(res.Payload)) + }) + }) + + t.Run("warn mode logs a warning over websocket", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + LogObservation: testenv.LogObservationConfig{ + Enabled: true, + LogLevel: zapcore.WarnLevel, + }, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + conn := xEnv.InitGraphQLWebSocketConnection(nil, nil, nil) + err := testenv.WSWriteJSON(t, conn, testenv.WebSocketMessage{ + ID: "1", + Type: "subscribe", + Payload: []byte(`{"query":"subscription { countEmp(max: 1, intervalMilliseconds: 100) }"}`), + }) + require.NoError(t, err) + var res testenv.WebSocketMessage + err = testenv.WSReadJSON(t, conn, &res) + require.NoError(t, err) + require.Equal(t, "next", res.Type) + require.Equal(t, "1", res.ID) + + logs := xEnv.Observer().FilterMessage("inline arguments found in operation") + require.Equal(t, 1, logs.Len()) + cm := logs.All()[0].ContextMap() + require.Equal(t, int64(2), cm["count"]) + require.Equal(t, []interface{}{"max", "intervalMilliseconds"}, cm["arguments"]) + }) + }) + + // Deferred queries stream a multipart response; the annotation belongs in the + // initial part's extensions, mirroring the single-body warn-mode behavior. + t.Run("warn mode annotates the initial part of a deferred response", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + NoRetryClient: true, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + payload, err := json.Marshal(map[string]any{ + "query": `query { employee(id: 1) { id ... @defer { isAvailable } } }`, + }) + require.NoError(t, err) + + req := xEnv.MakeGraphQLDeferRequest(http.MethodPost, bytes.NewReader(payload)) + res, err := xEnv.RouterClient.Do(req) + require.NoError(t, err) + defer func() { require.NoError(t, res.Body.Close()) }() + + require.Equal(t, http.StatusOK, res.StatusCode) + require.True(t, strings.HasPrefix(res.Header.Get("Content-Type"), "multipart/mixed"), + "expected multipart/mixed, got %q", res.Header.Get("Content-Type")) + + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + parts := deferMultipartParts(body) + require.NotEmpty(t, parts) + + // The argument appears twice: detection runs on the normalized document + // (see detectInlineArguments), and defer normalization duplicates the + // deferred field's parent path -- including its arguments -- into the + // hoisted deferred selection. + annotation := `{"code":"INLINE_ARGUMENT_VALUES_NOT_ALLOWED","message":"Inline argument values are not allowed. Use variables instead.","arguments":[{"argument":"id","valueKind":"Int"},{"argument":"id","valueKind":"Int"}]}` + var initial struct { + Extensions struct { + InlineArguments json.RawMessage `json:"inlineArguments"` + } `json:"extensions"` + } + require.NoError(t, json.Unmarshal(parts[0], &initial)) + require.JSONEq(t, annotation, string(initial.Extensions.InlineArguments)) + + for _, part := range parts[1:] { + require.NotContains(t, string(part), `"inlineArguments"`) + } + }) + }) + + t.Run("enforce mode rejects deferred query with inline arguments", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + NoRetryClient: true, + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeEnforce), + }, func(t *testing.T, xEnv *testenv.Environment) { + payload, err := json.Marshal(map[string]any{ + "query": `query { employee(id: 1) { id ... @defer { isAvailable } } }`, + }) + require.NoError(t, err) + + req := xEnv.MakeGraphQLDeferRequest(http.MethodPost, bytes.NewReader(payload)) + res, err := xEnv.RouterClient.Do(req) + require.NoError(t, err) + defer func() { require.NoError(t, res.Body.Close()) }() + + require.Equal(t, http.StatusBadRequest, res.StatusCode) + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Contains(t, string(body), `"INLINE_ARGUMENT_VALUES_NOT_ALLOWED"`) + }) + }) + + t.Run("warn mode compliant operation has no annotation", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: withInlineArgumentsMode(config.DisallowInlineArgumentsModeWarn), + }, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: `query GetEmployee($id: Int!) { employee(id: $id) { id } }`, + Variables: json.RawMessage(`{"id":1}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.Response.StatusCode) + require.NotContains(t, res.Body, `"inlineArguments"`) + }) + }) +} diff --git a/router/core/context.go b/router/core/context.go index 5bdf0c52c4..99a1ee9961 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -656,6 +656,8 @@ type operationContext struct { validationTime time.Duration planningTime time.Duration normalizationTime time.Duration + + inlineArgumentsAnnotation []byte } func (o *operationContext) Variables() *astjson.Value { diff --git a/router/core/defer_response_writer.go b/router/core/defer_response_writer.go index 262e3f834f..75847ace50 100644 --- a/router/core/defer_response_writer.go +++ b/router/core/defer_response_writer.go @@ -8,7 +8,9 @@ import ( "net/http" "strings" + "github.com/buger/jsonparser" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" + "go.uber.org/zap" ) type HttpDeferWriter struct { @@ -16,6 +18,13 @@ type HttpDeferWriter struct { writer io.Writer flusher http.Flusher buf *bytes.Buffer + logger *zap.Logger + + // inlineArgumentsAnnotation is injected into the extensions of the initial + // part only (Flush clears it after the first part): per the incremental-delivery + // spec the initial response is where clients look for extensions, matching the + // non-deferred warn-mode behavior. + inlineArgumentsAnnotation []byte } var _ resolve.DeferResponseWriter = (*HttpDeferWriter)(nil) @@ -65,6 +74,20 @@ func (f *HttpDeferWriter) Flush() (err error) { // Write into f.buf, which can't happen before we finish writing it out here. // resp sometimes ends with newlines, trim them so the trailer attaches cleanly. resp := bytes.TrimRight(f.buf.Bytes(), "\n") + + if len(f.inlineArgumentsAnnotation) > 0 { + // jsonparser.Set returns a new slice, so resp no longer aliases the buffer. + body, setErr := jsonparser.Set(resp, f.inlineArgumentsAnnotation, "extensions", "inlineArguments") + if setErr != nil { + // Degrade to the unannotated payload; the warn log already covers the operation. + if f.logger != nil { + f.logger.Error("failed to inject inlineArguments annotation into deferred response", zap.Error(setErr)) + } + } else { + resp = body + } + f.inlineArgumentsAnnotation = nil + } f.buf.Reset() // Write the part directly to the underlying writer rather than assembling a @@ -87,7 +110,9 @@ func (f *HttpDeferWriter) Flush() (err error) { return nil } -func GetDeferResponseWriter(ctx *resolve.Context, _ *http.Request, w http.ResponseWriter) (*resolve.Context, resolve.DeferResponseWriter, bool) { +// GetDeferResponseWriter returns a writer for multipart @defer responses. A non-empty +// inlineArgumentsAnnotation is injected into the initial part's extensions (warn mode). +func GetDeferResponseWriter(ctx *resolve.Context, _ *http.Request, w http.ResponseWriter, inlineArgumentsAnnotation []byte, logger *zap.Logger) (*resolve.Context, resolve.DeferResponseWriter, bool) { flusher, ok := w.(http.Flusher) if !ok { return ctx, nil, false @@ -103,9 +128,11 @@ func GetDeferResponseWriter(ctx *resolve.Context, _ *http.Request, w http.Respon w.Header().Set("X-Accel-Buffering", "no") flushWriter := &HttpDeferWriter{ - writer: w, - flusher: flusher, - buf: &bytes.Buffer{}, + writer: w, + flusher: flusher, + buf: &bytes.Buffer{}, + inlineArgumentsAnnotation: inlineArgumentsAnnotation, + logger: logger, } flushWriter.ctx = ctx.Context() diff --git a/router/core/errors.go b/router/core/errors.go index 8b11963c03..a3112bcd0f 100644 --- a/router/core/errors.go +++ b/router/core/errors.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "io" "net" "net/http" @@ -47,20 +48,22 @@ type ( } Extensions struct { - RateLimit json.RawMessage `json:"rateLimit,omitempty"` - Authorization json.RawMessage `json:"authorization,omitempty"` - Trace json.RawMessage `json:"trace,omitempty"` - StatusCode int `json:"statusCode,omitempty"` - Code string `json:"code,omitempty"` + RateLimit json.RawMessage `json:"rateLimit,omitempty"` + Authorization json.RawMessage `json:"authorization,omitempty"` + Trace json.RawMessage `json:"trace,omitempty"` + InlineArguments json.RawMessage `json:"inlineArguments,omitempty"` + StatusCode int `json:"statusCode,omitempty"` + Code string `json:"code,omitempty"` } ) const ( - ExtCodeErrPersistedQueryNotFound = "PERSISTED_QUERY_NOT_FOUND" - ExtCodeErrErrorRequestCanceled = "REQUEST_CANCELED" - ExtCodeErrBatchSizeExceeded = "BATCH_LIMIT_EXCEEDED" - ExtCodeErrBatchSubscriptionsUnsupported = "BATCHING_SUBSCRIPTION_UNSUPPORTED" - ExtCodeErrDeferMultipartNotAccepted = "DEFER_BAD_HEADER" + ExtCodeErrPersistedQueryNotFound = "PERSISTED_QUERY_NOT_FOUND" + ExtCodeErrErrorRequestCanceled = "REQUEST_CANCELED" + ExtCodeErrBatchSizeExceeded = "BATCH_LIMIT_EXCEEDED" + ExtCodeErrBatchSubscriptionsUnsupported = "BATCHING_SUBSCRIPTION_UNSUPPORTED" + ExtCodeErrInlineArgumentValuesNotAllowed = "INLINE_ARGUMENT_VALUES_NOT_ALLOWED" + ExtCodeErrDeferMultipartNotAccepted = "DEFER_BAD_HEADER" ) // isTerminalSubscriptionError reports whether the given error, when surfaced @@ -219,112 +222,115 @@ func propagateSubgraphErrors(ctx *resolve.Context) { } } -// writeRequestErrorsParams contains parameters for writing request errors to the response. -type writeRequestErrorsParams struct { - request *http.Request - writer http.ResponseWriter - statusCode int - requestErrors graphqlerrors.RequestErrors - logger *zap.Logger - headerPropagation *HeaderPropagation -} - -// writeRequestErrors writes the given request errors to the http.ResponseWriter. -// It accepts a graphqlerrors.RequestErrors object and writes it to the response based on the GraphQL spec. -func writeRequestErrors(params writeRequestErrorsParams) { - if params.requestErrors == nil { - return - } - - params.writer.Header().Set("Cache-Control", "no-store, no-cache, must-revalidate") +// writeRawErrorBody routes a pre-built JSON error body ({"errors":[...]}) through the correct +// transport framing (SSE, multipart, or plain JSON). All pre-execution error responses funnel +// through here; use it directly when the error payload cannot be expressed as +// graphqlerrors.RequestErrors (e.g. when the extensions object has a custom nested shape). +func writeRawErrorBody(r *http.Request, w http.ResponseWriter, statusCode int, body []byte, logger *zap.Logger, headerPropagation *HeaderPropagation) { + w.Header().Set("Cache-Control", "no-store, no-cache, must-revalidate") // According to the tests requestContext can be nil (when called from module WriteResponseError) // As such we have coded this condition defensively to be safe - requestContext := getRequestContext(params.request.Context()) + requestContext := getRequestContext(r.Context()) isSubscription := requestContext != nil && requestContext.operation != nil && requestContext.operation.opType == "subscription" // We only want to apply header propagation for non-subscription operations // In certain cases the requestContext can be nil, e.g.:- when called from the batch handler - if params.headerPropagation != nil && requestContext != nil && !isSubscription { - err := params.headerPropagation.ApplyRouterResponseHeaderRules(params.writer, requestContext) - if err != nil && params.logger != nil { - params.logger.Error("Failed to apply router response header rules on error cases", zap.Error(err)) + if headerPropagation != nil && requestContext != nil && !isSubscription { + if err := headerPropagation.ApplyRouterResponseHeaderRules(w, requestContext); err != nil && logger != nil { + logger.Error("Failed to apply router response header rules on error cases", zap.Error(err)) } } - wgRequestParams := NegotiateSubscriptionParams(params.request, !isSubscription) + wgRequestParams := NegotiateSubscriptionParams(r, !isSubscription) - // Is subscription if wgRequestParams.UseSse || wgRequestParams.UseMultipart { - setSubscriptionHeaders(wgRequestParams, params.request, params.writer) + setSubscriptionHeaders(wgRequestParams, r, w) + } else { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + } + if statusCode != 0 { + w.WriteHeader(statusCode) + } - if params.statusCode != 0 { - params.writer.WriteHeader(params.statusCode) + switch { + case wgRequestParams.UseSse: + if err := writeChunks(w, []byte(GetWriterPrefix(true, false, false)), body, []byte("\n\n")); err != nil { + logWriteResponseError(logger, err) } - - if wgRequestParams.UseSse { - _, err := params.writer.Write([]byte("event: next\ndata: ")) - if err != nil { - if params.logger != nil { - if rErrors.IsBrokenPipe(err) { - params.logger.Warn("Broken pipe, error writing response", zap.Error(err)) - return - } - params.logger.Error("Error writing response", zap.Error(err)) - } - return - } - } else if wgRequestParams.UseMultipart { - // Handle multipart error response - if err := writeMultipartError(params.writer, params.requestErrors, isSubscription); err != nil { - if params.logger != nil { - params.logger.Error("error writing multipart response", zap.Error(err)) - } - } - return + case wgRequestParams.UseMultipart: + if err := writeMultipartError(w, body, isSubscription); err != nil && logger != nil { + logger.Error("error writing multipart response", zap.Error(err)) } - } else { - // Regular request - params.writer.Header().Set("Content-Type", "application/json; charset=utf-8") - if params.statusCode != 0 { - params.writer.WriteHeader(params.statusCode) + default: + if _, err := w.Write(body); err != nil { + logWriteResponseError(logger, err) } } +} - if _, err := params.requestErrors.WriteResponse(params.writer); err != nil { - if params.logger != nil { - if rErrors.IsBrokenPipe(err) { - params.logger.Warn("Broken pipe, error writing response", zap.Error(err)) - return - } - params.logger.Error("Error writing response", zap.Error(err)) +// writeChunks writes each chunk in order, stopping at the first write error. +func writeChunks(w io.Writer, chunks ...[]byte) error { + for _, chunk := range chunks { + if _, err := w.Write(chunk); err != nil { + return err } } + return nil } -// writeMultipartError writes the error response in a multipart format with proper boundaries and headers. -func writeMultipartError( - w http.ResponseWriter, - requestErrors graphqlerrors.RequestErrors, - isSubscription bool, -) error { - // Start with the multipart boundary - prefix := GetWriterPrefix(false, true, true) - if _, err := w.Write([]byte(prefix)); err != nil { - return err +func logWriteResponseError(logger *zap.Logger, err error) { + if logger == nil { + return } + if rErrors.IsBrokenPipe(err) { + logger.Warn("Broken pipe, error writing response", zap.Error(err)) + return + } + logger.Error("Error writing response", zap.Error(err)) +} - // Write the actual error payload - response := graphqlerrors.Response{ - Errors: requestErrors, +// writeRequestErrorsParams contains parameters for writing request errors to the response. +type writeRequestErrorsParams struct { + request *http.Request + writer http.ResponseWriter + statusCode int + requestErrors graphqlerrors.RequestErrors + logger *zap.Logger + headerPropagation *HeaderPropagation +} + +// writeRequestErrors writes the given request errors to the http.ResponseWriter. +// It accepts a graphqlerrors.RequestErrors object and writes it to the response based on the GraphQL spec. +func writeRequestErrors(params writeRequestErrorsParams) { + if params.requestErrors == nil { + return } - responseBytes, err := response.Marshal() + response := graphqlerrors.Response{ + Errors: params.requestErrors, + } + body, err := response.Marshal() if err != nil { + if params.logger != nil { + params.logger.Error("Error marshalling request errors", zap.Error(err)) + } + params.writer.WriteHeader(http.StatusInternalServerError) + return + } + + writeRawErrorBody(params.request, params.writer, params.statusCode, body, params.logger, params.headerPropagation) +} + +// writeMultipartError writes the error response in a multipart format with proper boundaries and headers. +func writeMultipartError(w http.ResponseWriter, body []byte, isSubscription bool) error { + // Start with the multipart boundary + prefix := GetWriterPrefix(false, true, true) + if _, err := w.Write([]byte(prefix)); err != nil { return err } - resp, err := wrapMultipartMessage(responseBytes, isSubscription) + resp, err := wrapMultipartMessage(body, isSubscription) if err != nil { return err } @@ -334,7 +340,7 @@ func writeMultipartError( // multipart chunks correctly. With this fix here (and in a few other places) the clients are now working. resp = append(resp, []byte("\r\n--graphql--")...) - if _, err := w.Write([]byte(resp)); err != nil { + if _, err := w.Write(resp); err != nil { return err } @@ -365,7 +371,10 @@ func writeOperationError(r *http.Request, w http.ResponseWriter, requestLogger * var reportErr ReportError var httpErr HttpError var poNotFoundErr *persistedoperation.PersistentOperationNotFoundError + var inlineArgsErr *inlineArgumentsError switch { + case errors.As(err, &inlineArgsErr): + writeInlineArgumentsError(r, w, inlineArgsErr, requestLogger, propagation) case errors.Is(err, context.Canceled): newErr := NewHttpGraphqlError("request canceled", ExtCodeErrErrorRequestCanceled, http.StatusOK) writeRequestErrors(writeRequestErrorsParams{ diff --git a/router/core/graph_server.go b/router/core/graph_server.go index fa6cd1408f..e148808841 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1545,6 +1545,11 @@ func (s *graphServer) buildGraphMux( return nil, pubSubStartupErr } + inlineArgumentsChecker, err := NewInlineArgumentsChecker(s.securityConfiguration.DisallowInlineArguments) + if err != nil { + return nil, fmt.Errorf("failed to create inline arguments checker: %w", err) + } + operationProcessor := NewOperationProcessor(OperationProcessorOptions{ Executor: executor, MaxOperationSizeInBytes: int64(s.routerTrafficConfig.MaxRequestBodyBytes), @@ -1569,9 +1574,11 @@ func (s *graphServer) buildGraphMux( ApolloRouterCompatibilityFlags: s.apolloRouterCompatibilityFlags, DisableExposingVariablesContentOnValidationError: s.engineExecutionConfiguration.DisableExposingVariablesContentOnValidationError, RelaxSubgraphOperationFieldSelectionMergingNullability: s.engineExecutionConfiguration.RelaxSubgraphOperationFieldSelectionMergingNullability, - EnableDefer: s.engineExecutionConfiguration.EnableDefer, - ComplexityLimits: s.securityConfiguration.ComplexityLimits, - CostControl: s.securityConfiguration.CostControl, + EnableDefer: s.engineExecutionConfiguration.EnableDefer, + ComplexityLimits: s.securityConfiguration.ComplexityLimits, + CostControl: s.securityConfiguration.CostControl, + InlineArgumentsChecker: inlineArgumentsChecker, + Logger: s.logger, }) if opts.ReloadPersistentState.inMemoryPlanCacheFallback.IsEnabled() { diff --git a/router/core/graphql_handler.go b/router/core/graphql_handler.go index aaa594f5c8..d1627fc983 100644 --- a/router/core/graphql_handler.go +++ b/router/core/graphql_handler.go @@ -10,12 +10,14 @@ import ( "net/http" "strconv" "strings" + "sync" + + "github.com/buger/jsonparser" otelmetric "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" - rErrors "github.com/wundergraph/cosmo/router/internal/errors" "github.com/wundergraph/cosmo/router/pkg/config" rmetric "github.com/wundergraph/cosmo/router/pkg/metric" rotel "github.com/wundergraph/cosmo/router/pkg/otel" @@ -35,6 +37,16 @@ var ( errOperationPlanUnsupported = errors.New("unsupported operation plan") ) +// inlineArgsCaptureBufPool holds buffers used to capture the resolved response body +// when the warn-mode inlineArguments extension needs to be injected before writing. +var inlineArgsCaptureBufPool = sync.Pool{ + New: func() any { return &bytes.Buffer{} }, +} + +// inlineArgsCaptureBufMaxRetainedCap bounds the capacity of buffers returned to the +// pool so that a single very large response does not pin its memory indefinitely. +const inlineArgsCaptureBufMaxRetainedCap = 1 << 20 + const ( ExecutionPlanCacheHeader = "X-WG-Execution-Plan-Cache" PersistedOperationCacheHeader = "X-WG-Persisted-Operation-Cache" @@ -248,7 +260,21 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } - info, err := h.executor.Resolver.ArenaResolveGraphQLResponse(resolveCtx, p.Response, hpw) + // Capture the response body when warn-mode inline args were detected so we can inject the extension. + resolveWriter := io.Writer(hpw) + var captureBuf *bytes.Buffer + if len(reqCtx.operation.inlineArgumentsAnnotation) > 0 { + captureBuf = inlineArgsCaptureBufPool.Get().(*bytes.Buffer) + captureBuf.Reset() + defer func() { + if captureBuf.Cap() <= inlineArgsCaptureBufMaxRetainedCap { + inlineArgsCaptureBufPool.Put(captureBuf) + } + }() + resolveWriter = captureBuf + } + + info, err := h.executor.Resolver.ArenaResolveGraphQLResponse(resolveCtx, p.Response, resolveWriter) reqCtx.dataSourceNames = getSubgraphNames(p.Response.DataSources) if err != nil { trackFinalResponseError(resolveCtx.Context(), err) @@ -256,6 +282,19 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + if captureBuf != nil { + body, setErr := jsonparser.Set(captureBuf.Bytes(), reqCtx.operation.inlineArgumentsAnnotation, "extensions", "inlineArguments") + if setErr != nil { + reqCtx.logger.Error("failed to inject inlineArguments annotation into response", zap.Error(setErr)) + body = captureBuf.Bytes() + } + if _, wErr := hpw.Write(body); wErr != nil { + trackFinalResponseError(resolveCtx.Context(), wErr) + logWriteResponseError(reqCtx.logger, wErr) + return + } + } + // Compute actual cost for metrics/telemetry if not already set by the header callback if !reqCtx.operation.costActualSet && resolveCtx.TypeNameStats != nil && reqCtx.operation.preparedPlan != nil && reqCtx.operation.preparedPlan.preparedPlan != nil { @@ -347,7 +386,7 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.setDebugCacheHeaders(w, reqCtx.operation) defer propagateSubgraphErrors(resolveCtx) - resolveCtx, writer, ok = GetDeferResponseWriter(resolveCtx, r, w) + resolveCtx, writer, ok = GetDeferResponseWriter(resolveCtx, r, w, reqCtx.operation.inlineArgumentsAnnotation, reqCtx.logger) if !ok { reqCtx.logger.Error("unable to get defer response writer", zap.Error(errCouldNotFlushResponse)) trackFinalResponseError(r.Context(), errCouldNotFlushResponse) @@ -624,13 +663,8 @@ func (h *GraphQLHandler) writeError(ctx *resolve.Context, err error, res *resolv return } - err = json.NewEncoder(w).Encode(response) - if err != nil { - if rErrors.IsBrokenPipe(err) { - requestLogger.Warn("Broken pipe, unable to write error response", zap.Error(err)) - } else { - requestLogger.Error("Unable to write error response", zap.Error(err)) - } + if encErr := json.NewEncoder(w).Encode(response); encErr != nil { + logWriteResponseError(requestLogger, encErr) } } diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index 477d6000f5..477fa2b517 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -720,6 +720,12 @@ func (h *PreHandler) handleOperation(req *http.Request, httpOperation *httpOpera engineParseSpan.End() } + // Set name and type as soon as parsing succeeds so that error paths below + // see the correct operation type when negotiating the response transport + // (SSE/multipart vs. plain JSON). + requestContext.operation.name = operationKit.parsedOperation.Request.OperationName + requestContext.operation.opType = operationKit.parsedOperation.Type + if h.accessController != nil { // Based on the authentication result, the introspection config, // and wether this is an introspection query, @@ -751,9 +757,6 @@ func (h *PreHandler) handleOperation(req *http.Request, httpOperation *httpOpera } } - requestContext.operation.name = operationKit.parsedOperation.Request.OperationName - requestContext.operation.opType = operationKit.parsedOperation.Type - requestContext.expressionContext.Request.Operation.Name = requestContext.operation.name requestContext.expressionContext.Request.Operation.Type = requestContext.operation.opType @@ -863,6 +866,22 @@ func (h *PreHandler) handleOperation(req *http.Request, httpOperation *httpOpera // that the normalization span reflects only normalization. operationValidationCacheHit, operationValidationErr := operationKit.ValidateOperation() + // Check for inline argument values; the enforce-mode error is held and + // surfaced during validation, after the schema-validation error. Schema-invalid + // operations are skipped: they never execute, so walking them would only skew + // the warn log and span count that operators use to track migration progress. + var inlineArgumentsErr *inlineArgumentsError + if operationValidationErr == nil { + var result InlineArgumentsResult + result, inlineArgumentsErr = operationKit.CheckInlineArguments(requestContext.operation.clientInfo, requestContext.logger) + if result.Count > 0 { + // Set directly on the router span so operators can build a per-client + // breakdown of inline argument usage before enforcing. + httpOperation.routerSpan.SetAttributes(otel.WgOperationInlineArgumentsCount.Int(result.Count)) + } + requestContext.operation.inlineArgumentsAnnotation = result.Annotation + } + /** * Normalize the variables */ @@ -1076,6 +1095,10 @@ func (h *PreHandler) handleOperation(req *http.Request, httpOperation *httpOpera // coerced variable values). Its error is surfaced here so it appears on the // validation span rather than the normalization span. err = operationValidationErr + if err == nil && inlineArgumentsErr != nil { + // A schema-invalid operation gets the more fundamental error first. + err = inlineArgumentsErr + } if err == nil { err = operationKit.ValidateOperationVariables(requestContext.operation.executionOptions.SkipLoader, requestContext.operation.remapVariables, h.apolloCompatibilityFlags) } @@ -1245,11 +1268,12 @@ func (h *PreHandler) getErrorCodes(err error) []string { } } - // If "skipLoader" was passed as false to the Validate function, an httpGraphqlError with - // an extension code could be returned - var httpGqlError *httpGraphqlError - if errors.As(err, &httpGqlError) { - extensionCode := httpGqlError.ExtensionCode() + // If "skipLoader" was passed as false to the Validate function, an HttpError with + // an extension code could be returned; policy errors like inlineArgumentsError + // carry their code the same way. + var httpErr HttpError + if errors.As(err, &httpErr) { + extensionCode := httpErr.ExtensionCode() if extensionCode != "" { errorCodes = append(errorCodes, extensionCode) } diff --git a/router/core/inline_arguments.go b/router/core/inline_arguments.go new file mode 100644 index 0000000000..cebc80dab8 --- /dev/null +++ b/router/core/inline_arguments.go @@ -0,0 +1,283 @@ +package core + +import ( + "encoding/json" + "fmt" + "net/http" + + "github.com/wundergraph/cosmo/router/pkg/config" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" + "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" + "go.uber.org/zap" +) + +// InlineArgument identifies an offending argument by name and value kind. Positions +// are omitted: the normalized document they would refer to is re-parsed on a cache +// hit, so they would not reliably map to the text the client submitted. +type InlineArgument struct { + Name string `json:"argument"` + ValueKind string `json:"valueKind"` +} + +type InlineArgumentsChecker struct { + mode config.DisallowInlineArgumentsMode + enforceHTTPStatusCode int + errorCode string + errorMessage string + includePersistedOperations bool +} + +func NewInlineArgumentsChecker(cfg config.DisallowInlineArguments) (*InlineArgumentsChecker, error) { + switch cfg.Mode { + case config.DisallowInlineArgumentsModeOff, "": + return nil, nil + case config.DisallowInlineArgumentsModeWarn, config.DisallowInlineArgumentsModeEnforce: + default: + // Environment variables bypass the config JSON-schema enum validation, which only + // runs against the YAML bytes; without this check an unrecognized mode would fall + // through to warn behavior while the operator believes enforce is active. + return nil, fmt.Errorf("invalid security.disallow_inline_arguments.mode %q, expected one of %q, %q, %q", + cfg.Mode, config.DisallowInlineArgumentsModeOff, config.DisallowInlineArgumentsModeWarn, config.DisallowInlineArgumentsModeEnforce) + } + errorCode := cfg.ErrorCode + if errorCode == "" { + errorCode = ExtCodeErrInlineArgumentValuesNotAllowed + } + errorMessage := cfg.ErrorMessage + if errorMessage == "" { + errorMessage = "Inline argument values are not allowed. Use variables instead." + } + statusCode := cfg.EnforceHTTPStatusCode + if statusCode == 0 { + statusCode = http.StatusBadRequest + } + // Mirrors the JSON-schema bounds, which env vars bypass just like the mode enum + // above; without this check an out-of-range code would panic in net/http's + // WriteHeader on every enforce-mode rejection. + if statusCode < 200 || statusCode > 599 { + return nil, fmt.Errorf("invalid security.disallow_inline_arguments.enforce_http_status_code %d, expected a value between 200 and 599", statusCode) + } + return &InlineArgumentsChecker{ + mode: cfg.Mode, + enforceHTTPStatusCode: statusCode, + errorCode: errorCode, + errorMessage: errorMessage, + includePersistedOperations: cfg.IncludePersistedOperations, + }, nil +} + +// needsRegisteredOperationClassification reports whether the policy consumes the +// registered/APQ distinction on persisted operations. False when the policy is off +// (nil checker) or when registered operations are checked like everything else, +// letting the persisted operation fetch skip the registered-storage lookup for +// self-contained body+hash requests. +func (c *InlineArgumentsChecker) needsRegisteredOperationClassification() bool { + return c != nil && !c.includePersistedOperations +} + +// InlineArgumentsResult reports the outcome of a Check. +// Count is the number of inline arguments found (also when the operation is rejected), +// Annotation is the pre-built extensions.inlineArguments JSON in warn mode. +type InlineArgumentsResult struct { + Count int + Annotation []byte +} + +// CheckInlineArguments runs the disallow-inline-arguments policy check on the kit's +// current document, after NormalizeOperation and before NormalizeVariables (see detectInlineArguments). +// It is a no-op when the feature is disabled (no checker configured on the processor). +func (o *OperationKit) CheckInlineArguments(clientInfo *ClientInfo, logger *zap.Logger) (InlineArgumentsResult, *inlineArgumentsError) { + checker := o.operationProcessor.inlineArgumentsChecker + if checker == nil { + return InlineArgumentsResult{}, nil + } + return checker.Check(o.parsedOperation, o.kit.doc, o.operationProcessor.executor.ClientSchema, clientInfo, logger) +} + +// Check walks the operation document for non-variable argument values. +// Warn mode returns a pre-built annotation JSON for the extensions.inlineArguments response field. +// Enforce mode returns an error for immediate rejection. +func (c *InlineArgumentsChecker) Check(op *ParsedOperation, doc, definition *ast.Document, clientInfo *ClientInfo, logger *zap.Logger) (InlineArgumentsResult, *inlineArgumentsError) { + // Only operations whose content was registered in persisted operation storage are + // exempt. IsPersistedOperation would be too broad: it is set on mere presence of a + // persistedQuery hash, including APQ operations, whose hashes are client-computed + // and therefore carry no operator intent. + if op.IsRegisteredPersistedOperation && !c.includePersistedOperations { + return InlineArgumentsResult{}, nil + } + + args := detectInlineArguments(doc, definition) + if len(args) == 0 { + return InlineArgumentsResult{}, nil + } + result := InlineArgumentsResult{Count: len(args)} + + if ce := logger.Check(zap.WarnLevel, "inline arguments found in operation"); ce != nil { + names := make([]string, len(args)) + for i, arg := range args { + names[i] = arg.Name + } + fields := []zap.Field{ + zap.Int("count", len(args)), + zap.Strings("arguments", names), + zap.String("operation_name", op.Request.OperationName), + } + if clientInfo != nil { + fields = append(fields, + zap.String("client_name", clientInfo.Name), + zap.String("client_version", clientInfo.Version), + ) + } + ce.Write(fields...) + } + + if c.mode == config.DisallowInlineArgumentsModeEnforce { + return result, &inlineArgumentsError{ + message: c.errorMessage, + code: c.errorCode, + statusCode: c.enforceHTTPStatusCode, + arguments: args, + } + } + + // Subscriptions stream their responses, so there is no single response body + // to annotate; the warn log and span attribute still cover them. + if op.Type == "subscription" { + return result, nil + } + + result.Annotation = marshalInlineArgumentsExtension(c.errorCode, c.errorMessage, args, logger) + return result, nil +} + +// marshalInlineArgumentsExtension builds the extensions.inlineArguments payload shared +// by the warn-mode annotation and both enforce-mode error responses, or nil when +// marshalling fails (logged; the surrounding response is still delivered). +func marshalInlineArgumentsExtension(code, message string, args []InlineArgument, logger *zap.Logger) json.RawMessage { + payload, err := json.Marshal(inlineArgumentsExtension{ + Code: code, + Message: message, + Arguments: args, + }) + if err != nil { + if logger != nil { + logger.Error("failed to marshal inlineArguments extension", zap.Error(err)) + } + return nil + } + return payload +} + +// inlineArgumentsExtension is the extensions.inlineArguments payload, shared by the +// warn-mode response annotation and the enforce-mode error response. +type inlineArgumentsExtension struct { + Code string `json:"code"` + Message string `json:"message"` + Arguments []InlineArgument `json:"arguments"` +} + +// detectInlineArguments walks the operation document and collects arguments with +// non-variable values. It expects the normalized, pre-extraction document (after +// NormalizeOperation, before NormalizeVariables): non-executed sibling operations, +// unused fragments and static @skip/@include are already pruned, while inline +// literals are not yet extracted into variables. It must walk rather than scan the +// flat doc.Arguments slice, which keeps pruned nodes orphaned on a normalization +// cache miss but is re-parsed clean on a hit. +func detectInlineArguments(doc, definition *ast.Document) []InlineArgument { + walker := astvisitor.WalkerFromPool() + defer walker.Release() + visitor := &inlineArgumentsVisitor{operation: doc} + walker.RegisterEnterArgumentVisitor(visitor) + // Walk errors can only stem from a schema-invalid operation, whose + // schema-validation error takes precedence over this policy check anyway. + walker.Walk(doc, definition, &operationreport.Report{}) + return visitor.args +} + +type inlineArgumentsVisitor struct { + operation *ast.Document + args []InlineArgument +} + +func (v *inlineArgumentsVisitor) EnterArgument(ref int) { + arg := v.operation.Arguments[ref] + if arg.Value.Kind == ast.ValueKindVariable { + return + } + v.args = append(v.args, InlineArgument{ + Name: v.operation.ArgumentNameString(ref), + ValueKind: valueKindName(arg.Value.Kind), + }) +} + +func valueKindName(k ast.ValueKind) string { + switch k { + case ast.ValueKindString: + return "String" + case ast.ValueKindBoolean: + return "Boolean" + case ast.ValueKindInteger: + return "Int" + case ast.ValueKindFloat: + return "Float" + case ast.ValueKindNull: + return "Null" + case ast.ValueKindList: + return "List" + case ast.ValueKindObject: + return "Object" + case ast.ValueKindEnum: + return "Enum" + default: + return "Unknown" + } +} + +type inlineArgumentsError struct { + message string + code string + statusCode int + arguments []InlineArgument +} + +// inlineArgumentsError satisfies HttpError so the shared error-dispatch paths +// (getErrorCodes, transport error writers) pick up its code and status without +// type-specific branches; only the nested extensions rendering below is bespoke. +var _ HttpError = (*inlineArgumentsError)(nil) + +func (e *inlineArgumentsError) Error() string { return e.message } +func (e *inlineArgumentsError) Message() string { return e.message } +func (e *inlineArgumentsError) ExtensionCode() string { return e.code } +func (e *inlineArgumentsError) StatusCode() int { return e.statusCode } + +// graphqlError returns the error in response shape, shared by the HTTP and +// WebSocket transports: the extensions carry both the flat code that clients +// and APM tooling use for classification and the full details under +// inlineArguments (nil when marshalling fails; code and message still reach +// the client). +func (e *inlineArgumentsError) graphqlError(logger *zap.Logger) graphqlError { + return graphqlError{ + Message: e.message, + Extensions: &Extensions{ + Code: e.code, + InlineArguments: marshalInlineArgumentsExtension(e.code, e.message, e.arguments, logger), + }, + } +} + +func writeInlineArgumentsError(r *http.Request, w http.ResponseWriter, e *inlineArgumentsError, logger *zap.Logger, headerPropagation *HeaderPropagation) { + body, err := json.Marshal(struct { + Errors []graphqlError `json:"errors"` + }{ + Errors: []graphqlError{e.graphqlError(logger)}, + }) + if err != nil { + if logger != nil { + logger.Error("failed to marshal inline arguments error response", zap.Error(err)) + } + w.WriteHeader(http.StatusInternalServerError) + return + } + writeRawErrorBody(r, w, e.statusCode, body, logger, headerPropagation) +} diff --git a/router/core/inline_arguments_test.go b/router/core/inline_arguments_test.go new file mode 100644 index 0000000000..986eba62a9 --- /dev/null +++ b/router/core/inline_arguments_test.go @@ -0,0 +1,257 @@ +package core + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "github.com/wundergraph/cosmo/router/pkg/config" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" + "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" + "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" +) + +const inlineArgumentsTestSchema = ` +type Query { + employee(id: ID, active: Boolean, role: String): Employee + employees(order: Order, ids: [Int], filter: Filter): [Employee] + score(min: Float): Employee + field(input: Filter): String +} + +type Employee { + id: ID + posts(first: Int): [Employee] +} + +enum Order { + ASC + DESC +} + +input Filter { + active: Boolean +} +` + +func parseTestSchema(t *testing.T) *ast.Document { + t.Helper() + definition, report := astparser.ParseGraphqlDocumentString(inlineArgumentsTestSchema) + require.False(t, report.HasErrors(), "schema parse error: %s", report.Error()) + require.NoError(t, asttransform.MergeDefinitionWithBaseSchema(&definition)) + return &definition +} + +func parseQuery(t *testing.T, query string) *ast.Document { + t.Helper() + doc := ast.NewDocument() + doc.Input.ResetInputBytes([]byte(query)) + report := &operationreport.Report{} + astparser.NewParser().Parse(doc, report) + require.False(t, report.HasErrors(), "parse error: %s", report.Error()) + return doc +} + +// TestDetectInlineArguments exercises the detection walk in isolation on parsed +// documents. In the request pipeline the walk runs on the normalized document +// (see detectInlineArguments); the pruning behavior that implies is covered by +// the integration tests in router-tests/security/disallow_inline_arguments_test.go. +func TestDetectInlineArguments(t *testing.T) { + t.Parallel() + + definition := parseTestSchema(t) + + tests := []struct { + name string + query string + want []InlineArgument + }{ + { + name: "no arguments", + query: `query { employee { id } }`, + want: nil, + }, + { + name: "variable only", + query: `query($id: ID!) { employee(id: $id) { id } }`, + want: nil, + }, + { + name: "inline string", + query: `query { employee(id: "1") { id } }`, + want: []InlineArgument{ + {Name: "id", ValueKind: "String"}, + }, + }, + { + name: "inline integer", + query: `query { employee(id: 1) { id } }`, + want: []InlineArgument{ + {Name: "id", ValueKind: "Int"}, + }, + }, + { + name: "inline float", + query: `query { score(min: 1.5) { id } }`, + want: []InlineArgument{ + {Name: "min", ValueKind: "Float"}, + }, + }, + { + name: "inline boolean", + query: `query { employee(active: true) { id } }`, + want: []InlineArgument{ + {Name: "active", ValueKind: "Boolean"}, + }, + }, + { + name: "inline enum", + query: `query { employees(order: ASC) { id } }`, + want: []InlineArgument{ + {Name: "order", ValueKind: "Enum"}, + }, + }, + { + name: "inline null", + query: `query { employee(id: null) { id } }`, + want: []InlineArgument{ + {Name: "id", ValueKind: "Null"}, + }, + }, + { + name: "inline list", + query: `query { employees(ids: [1, 2]) { id } }`, + want: []InlineArgument{ + {Name: "ids", ValueKind: "List"}, + }, + }, + { + name: "inline object", + query: `query { employees(filter: {active: true}) { id } }`, + want: []InlineArgument{ + {Name: "filter", ValueKind: "Object"}, + }, + }, + { + name: "inline empty object", + query: `query { field(input: {}) }`, + want: []InlineArgument{ + {Name: "input", ValueKind: "Object"}, + }, + }, + { + name: "mixed variable and literal", + query: `query($id: ID!) { employee(id: $id, role: "admin") { id } }`, + want: []InlineArgument{ + {Name: "role", ValueKind: "String"}, + }, + }, + { + name: "directive inline argument", + query: `query($id: ID!) { employee(id: $id) @include(if: true) { id } }`, + want: []InlineArgument{ + {Name: "if", ValueKind: "Boolean"}, + }, + }, + { + name: "nested field argument", + query: `query($id: ID!) { employee(id: $id) { posts(first: 10) { id } } }`, + want: []InlineArgument{ + {Name: "first", ValueKind: "Int"}, + }, + }, + { + name: "introspection field argument", + query: `query { __type(name: "User") { name } }`, + want: []InlineArgument{ + {Name: "name", ValueKind: "String"}, + }, + }, + { + name: "inline arg inside spread fragment", + query: `query { ...F } fragment F on Query { employee(id: "1") { id } }`, + want: []InlineArgument{ + {Name: "id", ValueKind: "String"}, + }, + }, + { + name: "variable definition default not detected", + query: `query($x: ID = "5") { employee(id: $x) { id } }`, + want: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc := parseQuery(t, tc.query) + got := detectInlineArguments(doc, definition) + require.Equal(t, tc.want, got) + }) + } +} + +func TestNewInlineArgumentsChecker(t *testing.T) { + t.Parallel() + + t.Run("off and empty mode disable the checker", func(t *testing.T) { + t.Parallel() + for _, mode := range []config.DisallowInlineArgumentsMode{config.DisallowInlineArgumentsModeOff, ""} { + checker, err := NewInlineArgumentsChecker(config.DisallowInlineArguments{Mode: mode}) + require.NoError(t, err) + require.Nil(t, checker) + } + }) + + t.Run("warn and enforce create a checker", func(t *testing.T) { + t.Parallel() + for _, mode := range []config.DisallowInlineArgumentsMode{config.DisallowInlineArgumentsModeWarn, config.DisallowInlineArgumentsModeEnforce} { + checker, err := NewInlineArgumentsChecker(config.DisallowInlineArguments{Mode: mode}) + require.NoError(t, err) + require.NotNil(t, checker) + } + }) + + // Environment variables bypass the JSON-schema enum validation, which only runs + // against the YAML config bytes. An unrecognized mode must fail startup instead + // of silently behaving like warn while the operator believes enforce is active. + t.Run("unrecognized mode fails instead of degrading to warn", func(t *testing.T) { + t.Parallel() + for _, mode := range []config.DisallowInlineArgumentsMode{"ENFORCE", "Enforce", "enfoce", "on"} { + checker, err := NewInlineArgumentsChecker(config.DisallowInlineArguments{Mode: mode}) + require.Error(t, err) + require.ErrorContains(t, err, string(mode)) + require.Nil(t, checker) + } + }) + + // Same env-var bypass as the mode check: the JSON schema bounds the status code + // to 200-599, but env-derived values skip it, and net/http panics in WriteHeader + // for codes outside 100-999. Out-of-range codes must fail startup instead of + // panicking on every enforce-mode rejection. + t.Run("out-of-range enforce status code fails startup", func(t *testing.T) { + t.Parallel() + for _, statusCode := range []int{99, 199, 600, 1000, -1} { + checker, err := NewInlineArgumentsChecker(config.DisallowInlineArguments{ + Mode: config.DisallowInlineArgumentsModeEnforce, + EnforceHTTPStatusCode: statusCode, + }) + require.Error(t, err) + require.ErrorContains(t, err, fmt.Sprintf("%d", statusCode)) + require.Nil(t, checker) + } + }) + + t.Run("in-range enforce status codes are accepted", func(t *testing.T) { + t.Parallel() + for _, statusCode := range []int{200, 400, 599} { + checker, err := NewInlineArgumentsChecker(config.DisallowInlineArguments{ + Mode: config.DisallowInlineArgumentsModeEnforce, + EnforceHTTPStatusCode: statusCode, + }) + require.NoError(t, err) + require.NotNil(t, checker) + } + }) +} diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index 437ff8604e..403cc7c3f2 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -39,6 +39,7 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/middleware/operation_complexity" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" "github.com/wundergraph/graphql-go-tools/v2/pkg/variablesvalidation" + "go.uber.org/zap" ) var ( @@ -75,6 +76,14 @@ type ParsedOperation struct { IsPersistedOperation bool PersistedOperationCacheHit bool + // IsRegisteredPersistedOperation is true when the operation content was loaded from + // persisted operation storage (or its normalization cache) and was registered there + // ahead of time, as opposed to stored via automatic persisted queries. Unlike + // IsPersistedOperation it is never set on mere presence of a persistedQuery hash, + // so policies that exempt persisted operations cannot be bypassed by a client + // attaching a self-computed hash to the request. + IsRegisteredPersistedOperation bool + // NormalizationCacheHit is set to true if the request is a non-persisted operation, // and the normalized operation was loaded from cache. NormalizationCacheHit bool @@ -131,6 +140,8 @@ type OperationProcessorOptions struct { ParserTokenizerLimits astparser.TokenizerLimits OperationNameLengthLimit int EnableDefer bool + InlineArgumentsChecker *InlineArgumentsChecker + Logger *zap.Logger } // OperationProcessor provides shared resources to the parseKit and OperationKit. @@ -148,6 +159,8 @@ type OperationProcessor struct { costControl *config.CostControl parserTokenizerLimits astparser.TokenizerLimits operationNameLengthLimit int + inlineArgumentsChecker *InlineArgumentsChecker + logger *zap.Logger } // parseKit is a helper struct to parse, normalize and validate operations @@ -455,47 +468,66 @@ func (o *OperationKit) FetchPersistedOperation(ctx context.Context, clientInfo * return true, false, nil } - // If APQ is enabled and the query body is in the request, short-circuit - if o.parsedOperation.Request.Query != "" && o.operationProcessor.persistedOperationClient.APQEnabled() { - isAPQ = true - - // If the operation was fetched with APQ, save it again to renew the TTL - err := o.operationProcessor.persistedOperationClient.SaveOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, o.parsedOperation.Request.Query) - if err != nil { + // If APQ is enabled and the query body is in the request, the request is + // self-contained and only needs an APQ save. Short-circuit unless a policy + // consumes the registered/APQ distinction: the sha may belong to a registered + // persisted operation (the prehandler has already verified sha256(body) == + // hash), and classifying it as APQ would both skip the registered-operation + // exemptions and poison the normalization cache entry for subsequent hash-only + // requests. When nothing consumes the distinction, the (potentially remote) + // registered lookup would be wasted work on every body+hash request. + if o.parsedOperation.Request.Query != "" && + o.operationProcessor.persistedOperationClient.APQEnabled() && + !o.operationProcessor.inlineArgumentsChecker.needsRegisteredOperationClassification() { + // Save the operation again to renew the APQ TTL + if err = o.operationProcessor.persistedOperationClient.SaveOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, o.parsedOperation.Request.Query); err != nil { return false, true, err } - } else { - var persistedOperationData []byte - var err error + return false, true, nil + } - persistedOperationData, isAPQ, err = o.operationProcessor.persistedOperationClient.PersistedOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash) - if err != nil { - return false, isAPQ, err - } + var persistedOperationData []byte - if isAPQ && persistedOperationData == nil && o.parsedOperation.Request.Query == "" { - // If the client has APQ enabled, throw an error if the operation wasn't attached to the request - return false, isAPQ, &persistedoperation.PersistentOperationNotFoundError{ - ClientName: clientInfo.Name, - Sha256Hash: o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, + persistedOperationData, isAPQ, err = o.operationProcessor.persistedOperationClient.PersistedOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash) + if err != nil { + if o.parsedOperation.Request.Query != "" && o.operationProcessor.persistedOperationClient.APQEnabled() { + // A body+hash request is self-contained and never needed the registered + // storage before this lookup existed; a provider outage must not fail it. + // Fall back to the APQ classification and continue with the client body. + if logger := o.operationProcessor.logger; logger != nil { + logger.Warn("failed to look up persisted operation for a request carrying its own query body, classifying as APQ", + zap.String("sha256_hash", o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash), + zap.Error(err)) } + persistedOperationData, isAPQ, err = nil, true, nil + } else { + return false, isAPQ, err } + } - // it's important to make a copy of the persisted operation data, because it's used in the cache - // we might modify it later, so we don't want to modify the cached data - if persistedOperationData != nil { - o.parsedOperation.Request.Query = string(persistedOperationData) - // when we have successfully loaded the operation content from the storage, - // but it was passed via body instead of hash, we need to mark operation as persisted - // to populate persisted operation cache - o.parsedOperation.IsPersistedOperation = true + if isAPQ && persistedOperationData == nil && o.parsedOperation.Request.Query == "" { + // If the client has APQ enabled, throw an error if the operation wasn't attached to the request + return false, isAPQ, &persistedoperation.PersistentOperationNotFoundError{ + ClientName: clientInfo.Name, + Sha256Hash: o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, } + } - // If the operation was fetched with APQ, save it again to renew the TTL - if isAPQ { - if err = o.operationProcessor.persistedOperationClient.SaveOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, o.parsedOperation.Request.Query); err != nil { - return false, true, err - } + // it's important to make a copy of the persisted operation data, because it's used in the cache + // we might modify it later, so we don't want to modify the cached data + if persistedOperationData != nil { + o.parsedOperation.Request.Query = string(persistedOperationData) + // when we have successfully loaded the operation content from the storage, + // but it was passed via body instead of hash, we need to mark operation as persisted + // to populate persisted operation cache + o.parsedOperation.IsPersistedOperation = true + o.parsedOperation.IsRegisteredPersistedOperation = !isAPQ + } + + // If the operation was fetched with APQ, save it again to renew the TTL + if isAPQ { + if err = o.operationProcessor.persistedOperationClient.SaveOperation(ctx, clientInfo.Name, o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash, o.parsedOperation.Request.Query); err != nil { + return false, true, err } } @@ -772,6 +804,10 @@ type NormalizationCacheEntry struct { normalizedRepresentation string operationType string operationDefinitionRef int + // isApq records whether the persisted operation was stored via automatic persisted + // queries, so cache hits keep the registered/APQ distinction. Unused entries of the + // non-persisted normalization cache leave it false. + isApq bool } type VariablesNormalizationCacheEntry struct { @@ -1157,6 +1193,7 @@ func (o *OperationKit) handleFoundPersistedOperationEntry(entry NormalizationCac // otherwise in case it was already cached we will try to normalize an empty document // as we skip parse for the cached persisted operations o.parsedOperation.IsPersistedOperation = true + o.parsedOperation.IsRegisteredPersistedOperation = !entry.isApq o.parsedOperation.NormalizationCacheHit = true o.parsedOperation.NormalizedRepresentation = entry.normalizedRepresentation o.parsedOperation.Type = entry.operationType @@ -1214,6 +1251,7 @@ func (o *OperationKit) savePersistedOperationToCache(clientName string, isApq bo normalizedRepresentation: o.parsedOperation.NormalizedRepresentation, operationType: o.parsedOperation.Type, operationDefinitionRef: o.operationDefinitionRef, + isApq: isApq, } if isApq { @@ -1598,6 +1636,8 @@ func NewOperationProcessor(opts OperationProcessorOptions) *OperationProcessor { operationNameLengthLimit: opts.OperationNameLengthLimit, complexityLimits: opts.ComplexityLimits, costControl: opts.CostControl, + inlineArgumentsChecker: opts.InlineArgumentsChecker, + logger: opts.Logger, parseKitOptions: &parseKitOptions{ enableDefer: opts.EnableDefer, apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, diff --git a/router/core/websocket.go b/router/core/websocket.go index 59f900982e..fc7f73f83b 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -864,7 +864,10 @@ func (h *WebSocketConnectionHandler) writeErrorMessage(operationID string, err e var gqlErr graphqlError var poNotFoundErr *persistedoperation.PersistentOperationNotFoundError + var inlineArgsErr *inlineArgumentsError switch { + case errors.As(err, &inlineArgsErr): + gqlErr = inlineArgsErr.graphqlError(h.logger) case errors.As(err, &poNotFoundErr): // We follow the same pattern of not mentioning the sha256hash // in the normal http requests for the same case @@ -980,6 +983,16 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi // The error is surfaced later, during validation, so normalization timing stays accurate. _, operationValidationErr := operationKit.ValidateOperation() + // Check for inline argument values. Warn-mode annotations are dropped here: + // streamed responses have no single body to annotate; the warn log emitted + // by the check still covers this transport. Schema-invalid operations are + // skipped: they never execute, so walking them would only skew the warn log. + if operationValidationErr == nil { + if _, inlineArgumentsErr := operationKit.CheckInlineArguments(h.clientInfo, h.logger); inlineArgumentsErr != nil { + operationValidationErr = inlineArgumentsErr + } + } + cached, _, err := operationKit.NormalizeVariables() if err != nil { opContext.normalizationTime = time.Since(startNormalization) diff --git a/router/internal/persistedoperation/client.go b/router/internal/persistedoperation/client.go index 648373123c..79af4b7b5e 100644 --- a/router/internal/persistedoperation/client.go +++ b/router/internal/persistedoperation/client.go @@ -65,22 +65,32 @@ func NewClient(opts *Options) (*Client, error) { } func (c *Client) PersistedOperation(ctx context.Context, clientName string, sha256Hash string) ([]byte, bool, error) { - if c.APQEnabled() { - resp, apqErr := c.apqClient.PersistedOperation(ctx, clientName, sha256Hash) - if len(resp) > 0 || apqErr != nil { - return resp, true, apqErr - } - } - + // The cheap local registered sources are consulted before the APQ store: a + // registered sha that was also seeded into the APQ store (e.g. sent as + // body+hash by an APQ client) must classify as registered, not APQ — + // registration carries operator intent that policies like + // disallow_inline_arguments rely on. if data := c.cache.Get(clientName, sha256Hash); data != nil { return data, false, nil } - // PQL manifest check (local, no network) - if c.pqlStore != nil && c.pqlStore.IsLoaded() { + // PQL manifest hit (local, no network); its authoritative miss is handled + // below, after the APQ store had its chance to resolve the hash. + manifestLoaded := c.pqlStore != nil && c.pqlStore.IsLoaded() + if manifestLoaded { if body, found := c.pqlStore.LookupByHash(sha256Hash); found { return body, false, nil } + } + + if c.APQEnabled() { + resp, apqErr := c.apqClient.PersistedOperation(ctx, clientName, sha256Hash) + if len(resp) > 0 || apqErr != nil { + return resp, true, apqErr + } + } + + if manifestLoaded { // Manifest is authoritative — operation not found if c.APQEnabled() { return nil, true, nil diff --git a/router/internal/persistedoperation/client_test.go b/router/internal/persistedoperation/client_test.go new file mode 100644 index 0000000000..14d9ed2be9 --- /dev/null +++ b/router/internal/persistedoperation/client_test.go @@ -0,0 +1,108 @@ +package persistedoperation + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +type fakeStorageClient struct { + operations map[string][]byte + calls int +} + +func (f *fakeStorageClient) PersistedOperation(_ context.Context, clientName string, sha256Hash string) ([]byte, error) { + f.calls++ + if data, ok := f.operations[sha256Hash]; ok { + return data, nil + } + return nil, &PersistentOperationNotFoundError{ClientName: clientName, Sha256Hash: sha256Hash} +} + +func (f *fakeStorageClient) Close() {} + +type fakeApqClient struct { + operations map[string][]byte +} + +func (f *fakeApqClient) Enabled() bool { return true } +func (f *fakeApqClient) IsDistributed() bool { return false } + +func (f *fakeApqClient) PersistedOperation(_ context.Context, _ string, sha256Hash string) ([]byte, error) { + return f.operations[sha256Hash], nil +} + +func (f *fakeApqClient) SaveOperation(_ context.Context, _, sha256Hash string, operationBody []byte) error { + f.operations[sha256Hash] = operationBody + return nil +} + +func (f *fakeApqClient) Close() {} + +func TestPersistedOperationRegisteredSourcesWinOverAPQStore(t *testing.T) { + t.Parallel() + + const ( + clientName = "my-client" + sha = "49a2f7dd56b06f620c7d040dd9d562a1c16eadf7c149be5decdd62cfc92e1b12" + body = `mutation updateEmployeeTag { updateEmployeeTag(id: 10, tag: "dd") { id } }` + ) + + newTestClient := func(t *testing.T, provider StorageClient, apqOps map[string][]byte) *Client { + t.Helper() + client, err := NewClient(&Options{ + CacheSize: 1024 * 1024, + ProviderClient: provider, + ApqClient: &fakeApqClient{operations: apqOps}, + }) + require.NoError(t, err) + t.Cleanup(client.Close) + return client + } + + // A registered sha that was also seeded into the APQ store (e.g. by a client + // sending body+hash) must classify as registered, not APQ. + t.Run("local operations cache wins over APQ store", func(t *testing.T) { + t.Parallel() + client := newTestClient(t, &fakeStorageClient{}, map[string][]byte{sha: []byte(body)}) + client.cache.Set(clientName, sha, []byte(body), 0) + client.cache.Cache.Wait() + + data, isAPQ, err := client.PersistedOperation(context.Background(), clientName, sha) + require.NoError(t, err) + require.False(t, isAPQ) + require.Equal(t, body, string(data)) + }) + + t.Run("APQ store resolves when registered sources miss", func(t *testing.T) { + t.Parallel() + client := newTestClient(t, &fakeStorageClient{}, map[string][]byte{sha: []byte(body)}) + + data, isAPQ, err := client.PersistedOperation(context.Background(), clientName, sha) + require.NoError(t, err) + require.True(t, isAPQ) + require.Equal(t, body, string(data)) + }) + + t.Run("provider hit classifies as registered", func(t *testing.T) { + t.Parallel() + provider := &fakeStorageClient{operations: map[string][]byte{sha: []byte(body)}} + client := newTestClient(t, provider, map[string][]byte{}) + + data, isAPQ, err := client.PersistedOperation(context.Background(), clientName, sha) + require.NoError(t, err) + require.False(t, isAPQ) + require.Equal(t, body, string(data)) + }) + + t.Run("unknown sha with APQ enabled returns no error", func(t *testing.T) { + t.Parallel() + client := newTestClient(t, &fakeStorageClient{}, map[string][]byte{}) + + data, isAPQ, err := client.PersistedOperation(context.Background(), clientName, sha) + require.NoError(t, err) + require.True(t, isAPQ) + require.Nil(t, data) + }) +} diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index a53fd35f22..98367498ad 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -514,6 +514,7 @@ type SecurityConfiguration struct { DepthLimit *QueryDepthConfiguration `yaml:"depth_limit"` ParserLimits ParserLimitsConfiguration `yaml:"parser_limits"` OperationNameLengthLimit int `yaml:"operation_name_length_limit" envDefault:"512" env:"SECURITY_OPERATION_NAME_LENGTH_LIMIT"` // 0 is disabled + DisallowInlineArguments DisallowInlineArguments `yaml:"disallow_inline_arguments,omitempty" envPrefix:"SECURITY_DISALLOW_INLINE_ARGUMENTS_"` } type ParserLimitsConfiguration struct { @@ -603,6 +604,43 @@ func (c *ComplexityLimit) ApplyLimit(isPersistent bool) bool { return c.Enabled && (!isPersistent || !c.IgnorePersistedOperations) } +// DisallowInlineArgumentsMode controls whether inline argument values are allowed. +type DisallowInlineArgumentsMode string + +const ( + DisallowInlineArgumentsModeOff DisallowInlineArgumentsMode = "off" + DisallowInlineArgumentsModeWarn DisallowInlineArgumentsMode = "warn" + DisallowInlineArgumentsModeEnforce DisallowInlineArgumentsMode = "enforce" +) + +// DisallowInlineArguments configures a policy that detects and optionally rejects +// GraphQL operations that carry hardcoded inline argument values instead of variables. +// The policy covers both field arguments and directive arguments; detection runs on the +// normalized operation, so literals that normalization resolves away (e.g. a static +// @include(if: true)) are not reported. +type DisallowInlineArguments struct { + // Mode controls the policy behavior: + // - "off" (default): the scan is never run; no overhead. + // - "warn": inline arguments are detected and logged; the operation succeeds with an extensions annotation. + // - "enforce": inline arguments cause the operation to be rejected during validation, before variable extraction and execution. + Mode DisallowInlineArgumentsMode `yaml:"mode,omitempty" json:"mode,omitempty" envDefault:"off" env:"MODE"` + + // EnforceHTTPStatusCode is the HTTP status returned in enforce mode. + // Defaults to 400. Use 200 for strict GraphQL-spec compliance. + EnforceHTTPStatusCode int `yaml:"enforce_http_status_code,omitempty" json:"enforce_http_status_code,omitempty" envDefault:"400" env:"ENFORCE_HTTP_STATUS_CODE"` + + // ErrorCode is the GraphQL error extensions.code emitted on rejection or annotation. + ErrorCode string `yaml:"error_code,omitempty" json:"error_code,omitempty" envDefault:"INLINE_ARGUMENT_VALUES_NOT_ALLOWED" env:"ERROR_CODE"` + + // ErrorMessage is the human-readable error/hint message. + ErrorMessage string `yaml:"error_message,omitempty" json:"error_message,omitempty" envDefault:"Inline argument values are not allowed. Use variables instead." env:"ERROR_MESSAGE"` + + // IncludePersistedOperations, when true, applies the policy to registered persisted operations as well. + // By default (false) they are exempt because they are stored server-side and intentional. + // Automatic persisted queries are never exempt: their hashes are client-computed and carry no operator intent. + IncludePersistedOperations bool `yaml:"include_persisted_operations,omitempty" json:"include_persisted_operations,omitempty" envDefault:"false" env:"INCLUDE_PERSISTED_OPERATIONS"` +} + type OverrideRoutingURLConfiguration struct { Subgraphs map[string]string `yaml:"subgraphs"` } diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index 7f7cf2b897..a111e8266a 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -3593,6 +3593,41 @@ "default": "512", "minimum": 0 }, + "disallow_inline_arguments": { + "type": "object", + "description": "A policy that detects and optionally rejects GraphQL operations that carry hardcoded inline argument values instead of variables. Covers field arguments and directive arguments.", + "additionalProperties": false, + "properties": { + "mode": { + "type": "string", + "enum": ["off", "warn", "enforce"], + "default": "off", + "description": "Controls the policy behavior: 'off' disables the check entirely; 'warn' detects and logs inline arguments and annotates the successful response with an extensions hint; 'enforce' rejects operations with inline arguments before normalization." + }, + "enforce_http_status_code": { + "type": "integer", + "default": 400, + "minimum": 200, + "maximum": 599, + "description": "The HTTP status code returned in enforce mode. Use 200 for strict GraphQL-spec compliance." + }, + "error_code": { + "type": "string", + "default": "INLINE_ARGUMENT_VALUES_NOT_ALLOWED", + "description": "The GraphQL error extensions.code emitted on rejection or annotation." + }, + "error_message": { + "type": "string", + "default": "Inline argument values are not allowed. Use variables instead.", + "description": "The human-readable error / hint message." + }, + "include_persisted_operations": { + "type": "boolean", + "default": false, + "description": "When true, the policy also applies to persisted operations. By default they are exempt because they are stored server-side and intentional." + } + } + }, "depth_limit": { "type": "object", "description": "DEPRECATED (move to complexity_limits.depth): The configuration for adding a max depth limit for query (how many nested levels you can have in a query).", diff --git a/router/pkg/config/config_test.go b/router/pkg/config/config_test.go index bc047d8fba..52a9b4d9fc 100644 --- a/router/pkg/config/config_test.go +++ b/router/pkg/config/config_test.go @@ -105,6 +105,28 @@ version: "1" require.Equal(t, time.Second*20, cfg.Config.WatchConfig.StartupDelay.Maximum) } +func TestLoadDisallowInlineArgumentsFromEnvars(t *testing.T) { + t.Setenv("SECURITY_DISALLOW_INLINE_ARGUMENTS_MODE", "enforce") + t.Setenv("SECURITY_DISALLOW_INLINE_ARGUMENTS_ENFORCE_HTTP_STATUS_CODE", "200") + t.Setenv("SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_CODE", "CUSTOM_CODE") + t.Setenv("SECURITY_DISALLOW_INLINE_ARGUMENTS_ERROR_MESSAGE", "Custom message") + t.Setenv("SECURITY_DISALLOW_INLINE_ARGUMENTS_INCLUDE_PERSISTED_OPERATIONS", "true") + + f := createTempFileFromFixture(t, ` +version: "1" +`) + + cfg, err := LoadConfig([]string{f}) + + require.NoError(t, err) + + require.Equal(t, DisallowInlineArgumentsModeEnforce, cfg.Config.SecurityConfiguration.DisallowInlineArguments.Mode) + require.Equal(t, 200, cfg.Config.SecurityConfiguration.DisallowInlineArguments.EnforceHTTPStatusCode) + require.Equal(t, "CUSTOM_CODE", cfg.Config.SecurityConfiguration.DisallowInlineArguments.ErrorCode) + require.Equal(t, "Custom message", cfg.Config.SecurityConfiguration.DisallowInlineArguments.ErrorMessage) + require.True(t, cfg.Config.SecurityConfiguration.DisallowInlineArguments.IncludePersistedOperations) +} + func TestConfigHasPrecedence(t *testing.T) { t.Setenv("POLL_INTERVAL", "22s") diff --git a/router/pkg/config/fixtures/full.yaml b/router/pkg/config/fixtures/full.yaml index 2847a043a2..377f3f20c5 100644 --- a/router/pkg/config/fixtures/full.yaml +++ b/router/pkg/config/fixtures/full.yaml @@ -525,6 +525,12 @@ security: estimated_list_size: 10 expose_headers: false operation_name_length_limit: 2000 + disallow_inline_arguments: + mode: enforce + enforce_http_status_code: 422 + error_code: VARIABLES_REQUIRED + error_message: 'Please use variables.' + include_persisted_operations: true persisted_operations: safelist: enabled: true diff --git a/router/pkg/config/testdata/config_defaults.json b/router/pkg/config/testdata/config_defaults.json index 903baa297d..6f0986c29a 100644 --- a/router/pkg/config/testdata/config_defaults.json +++ b/router/pkg/config/testdata/config_defaults.json @@ -439,7 +439,13 @@ "ApproximateDepthLimit": 200, "TotalFieldsLimit": 3500 }, - "OperationNameLengthLimit": 512 + "OperationNameLengthLimit": 512, + "DisallowInlineArguments": { + "mode": "off", + "enforce_http_status_code": 400, + "error_code": "INLINE_ARGUMENT_VALUES_NOT_ALLOWED", + "error_message": "Inline argument values are not allowed. Use variables instead." + } }, "EngineExecutionConfiguration": { "Debug": { diff --git a/router/pkg/config/testdata/config_full.json b/router/pkg/config/testdata/config_full.json index 727d2d7230..50a0bc00f7 100644 --- a/router/pkg/config/testdata/config_full.json +++ b/router/pkg/config/testdata/config_full.json @@ -891,7 +891,14 @@ "ApproximateDepthLimit": 200, "TotalFieldsLimit": 3500 }, - "OperationNameLengthLimit": 2000 + "OperationNameLengthLimit": 2000, + "DisallowInlineArguments": { + "mode": "enforce", + "enforce_http_status_code": 422, + "error_code": "VARIABLES_REQUIRED", + "error_message": "Please use variables.", + "include_persisted_operations": true + } }, "EngineExecutionConfiguration": { "Debug": { diff --git a/router/pkg/otel/attributes.go b/router/pkg/otel/attributes.go index 07e0deaab2..a4519e502b 100644 --- a/router/pkg/otel/attributes.go +++ b/router/pkg/otel/attributes.go @@ -14,14 +14,18 @@ const ( WgOperationHash = attribute.Key("wg.operation.hash") WgOperationVariables = attribute.Key("wg.operation.variables") WgOperationProtocol = attribute.Key("wg.operation.protocol") - WgComponentName = attribute.Key("wg.component.name") - WgClientName = attribute.Key("wg.client.name") - WgClientVersion = attribute.Key("wg.client.version") - WgRouterVersion = attribute.Key("wg.router.version") - WgRouterConfigVersion = attribute.Key("wg.router.config.version") - WgFederatedGraphID = attribute.Key("wg.federated_graph.id") - WgSubgraphID = attribute.Key("wg.subgraph.id") - WgSubgraphName = attribute.Key("wg.subgraph.name") + // WgOperationInlineArgumentsCount is the number of inline (non-variable) argument values + // found by the disallow_inline_arguments security policy; only set when the policy is active + // and at least one inline argument was found. + WgOperationInlineArgumentsCount = attribute.Key("wg.operation.inline_arguments.count") + WgComponentName = attribute.Key("wg.component.name") + WgClientName = attribute.Key("wg.client.name") + WgClientVersion = attribute.Key("wg.client.version") + WgRouterVersion = attribute.Key("wg.router.version") + WgRouterConfigVersion = attribute.Key("wg.router.config.version") + WgFederatedGraphID = attribute.Key("wg.federated_graph.id") + WgSubgraphID = attribute.Key("wg.subgraph.id") + WgSubgraphName = attribute.Key("wg.subgraph.name") // WgRequestError is only used to annotate the request count metric to easily identify errored and non-errored requests // with the same metric. This has simplified the query for the error and request count metric in Cloud. WgRequestError = attribute.Key("wg.request.error")