Merging to release-5.15.0: [TT-17921] Improve header handling for GraphQL-based APIs (#8602)

[probelabs]

TT-17921 Improve header handling for GraphQL-based APIs (#8602)

Summary

Values that belong to a single request — the round tripper, the resolved
upstream headers, the
upstream response — were being written into, or keyed on, state that
outlives the request: the
shared per-API http.Client, the plan cache, and the upstream
connection-reuse key. Under
concurrency the result was not deterministic: which request's values a
fetch used depended on
timing.

This makes that state request-scoped, and fixes the header handling that
depended on it. Seven
commits, each independently revertable.

internal/graphengine goes from 22 to 76 test functions (273 including
subtests), green under
-race -count=10.

Behavioural detail and reproduction steps are on the ticket.

What changed

scope transports to requests

reverseProxyPreHandlerV1/V2.PreHandle assigned params.RoundTripper
onto the per-API
httpClient.Transport on every request, so concurrent requests
overwrote each other's. The round
tripper and headers config now travel in the request context
(SetGraphQLEngineTransportContextValue), and
GraphQLEngineTransport.RoundTrip builds a
per-request view from them, falling back to the client's own transport.

keep subscriptions alive, isolate proxy-only responses

Three defects, two of them introduced by the commit above:

• WebSocket subscriptions never delivered. The handovers passed
  WithContext(params.OutRequest.Context()), but net/http calls
  w.cancelCtx() as soon as
  ServeHTTP returns — before the hijacked check in conn.serve — and
  the gateway returns as soon
  as the connection is hijacked. The subscription context was cancelled
  before anything was sent.
  subscriptionRequestContext roots the subscription at the long-lived
  engine spec context while
  carrying the opening request's transport values across. EngineV1 passes
  no context: its legacy
  data sources drop it, so inheriting the request's cancellation could
  only break the subscription.
• handleProxyOnly rewrote the outgoing method, turning a
  proxy-only upstream WebSocket
  handshake into a POST, which WebSocket servers reject. Handshakes now
  stay GET.
• GraphQLProxyOnlyContextValues.upstreamResponse was
  unsynchronised. The transport writes it
  on the resolver's trigger goroutine while the engine reads it (~1 in 20
  under -race). Now
  behind a mutex.

cover the request isolation matrix

credential_isolation_test.go: proxy-only, UDG and supergraph × HTTP,
SSE, graphql-ws and
graphql-transport-ws × EngineV2 and EngineV3. Each test drives more
than one request through one
engine and asserts what each upstream connection actually received, with
negative controls
(strip_auth_data, a custom auth header name, a request that sets no
header after one that did),
20-request concurrent bursts, a per-request round tripper test that
shared mutable transport state
cannot pass, and the client WebSocket-upgrade path that the regression
above had slipped through.

move to the fixed graphql-go-tools, cover the cold path

Both module pins move to the library branch; coder/websocket becomes a
direct dependency. Two
races are fixed upstream — v1 Schema.Hash writing s.hash on first
use, and v2
ResolveGraphQLResponse writing Info on the cached plan — so the
cold-start concurrent burst
becomes permanent coverage instead of something a warm-up had to work
around.

resolve dynamic upstream headers before the fetch is keyed

The v1 header modifier merged Tyk's additional headers and stopped,
leaving tykVariableReplacer
injected but never read, so $tyk_context.* in an upstream header
reached the fetch unresolved on
config version 2. That matters for more than the value that arrives
upstream: the library applies
the modifier before it derives the key that groups equivalent
subscriptions onto one upstream
connection (connectionKey in the v1 subscription client,
UniqueRequestID in the v2 resolver).
An unresolved template is the same string for every request, so requests
that should have been
distinct were treated as equivalent and pooled together.

Resolving there is only correct now that the library moved header
modification to the execution
phase. TT-14357 (adca34d46) had removed it because the modifier ran at
plan post-process time and
the result was cached into the plan, and added
variableReplaceRoundTripper as the workaround.
That wrapper is retired here: it ran after the grouping key was derived,
it collapsed multi-value
headers via Get/Set on the way to the wire, and it expanded
variables in header values that did
not come from the API definition. Its one remaining job,
request_headers_rewrite, is resolved
once where the rules are built, from configuration only.

restore the gateway round tripper for EngineV1 fetches

Regression from cf506a971, caught while verifying the above:
TestGraphQL_InternalDataSource/graphql_engine_v1 fails at the branch
tip and passes at the base —
a config version 1 API with a tyk:// data source answers 500. The
transport assignment that
commit removed was also the only thing giving EngineV1 the gateway round
tripper, and only that
round tripper can route tyk://. Since EngineV1's data sources drop the
execution context, the
round tripper is handed over as a mutex-guarded transport-level
fallback, used only when a request
carries none of its own. EngineV2 and EngineV3 stay fully
request-scoped.

That fallback is the one piece of transport state shared between
requests, and it is only sound
because the round tripper the gateway hands over no longer carries
request-specific state — the
wrapper retired in 4d67f9af4 did. The two commits must not be
reverted independently; both
sides carry comments saying so.

forward a consumer header to the upstream once

Two writers put the consumer's headers on the upstream request and
neither knew about the other:
propagateAuthHeaders adds them to the fetch input when
strip_auth_data is off, and
setProxyOnlyHeaders then forwards every consumer header again with
Header.Add. With
use_immutable_headers off nothing removed the first copy, so a header
present in both arrived
duplicated. It went unnoticed because the wrapper retired in 4d67f9af4
had been collapsing
repeated values with Get/Set since before the second writer was
added.

setProxyOnlyHeaders now skips a value the engine already placed there,
compared against a
snapshot taken before forwarding so a consumer that genuinely sent the
same value twice keeps both.
It also hoists the use_immutable_headers delete out of the value loop,
where it removed the value
added on the previous pass and left a multi-value consumer header
holding only its last value.

Behaviour changes worth a reviewer's sign-off

1. An API reload or release now ends that API's subscriptions.
   Previously they ran on
   context.Background() and outlived the reload.
2. No header value is resolved twice on config version 2 or
   3-preview. Values that do not come
   from the API definition are no longer expanded, and a value that
   resolved into another template
   token no longer gets a second pass.
3. An exact overlap between a configured request_headers value and a
   consumer value now reaches
   the upstream once, not twice. Differing values still both go upstream
   — TT-11990 / TT-12190
   semantics are unchanged.
4. Multi-value headers survive to the upstream. Accept-Encoding: gzip, deflate, br used to
   arrive as gzip alone.

Ticket Details

TT-17921

Status	Merge
Summary	Federated supergraph reuses first caller’s JWT across users
and can share HTTP responses via single-flight

Generated at: 2026-08-20 13:03:45

[sentinelone-cnapp-eu1]

SentinelOne CNS Hardcoded Secret Detector
✅ Congratulations, your code is safe

SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes.

[github-actions]

🚨 Jira Linter Failed

Commit: b8cef30
Failed at: 2026-08-24 08:17:56 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

failed to get Jira issue: Jira issue RELEASE-5 not found (HTTP 404). The issue may exist but the API token may lack permission to access it. Verify that the token owner has access to the project and that JL_JIRA_BASEURL (https://api.atlassian.com/ex/jira/c25a3295-62f6-4d5a-8ddd-58122b144a37) is correct

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

---

This comment will be automatically deleted once the linter passes.

[github-actions]

🎯 Recommended Merge Targets

Based on JIRA ticket TT-17921: Federated supergraph reuses first caller’s JWT across users and can share HTTP responses via single-flight

Fix Version: Tyk 5.15.0

Required:

• release-5.15.0 - Exact version branch for Tyk 5.15.0 - specific patch release
• master - Main development branch - ensures fix is in all future releases

Recommended:

• release-5.15 - Minor version branch for 5.15.x releases

Fix Version: Tyk 5.13.2

Required:

• release-5.13.2 - Exact version branch for Tyk 5.13.2 - specific patch release
• release-5.13 - Minor version branch for 5.13.x patches - required for creating Tyk 5.13.2
• master - Main development branch - ensures fix is in all future releases

---

📋 Workflow

1. Merge this PR to master first

2. Cherry-pick to release branches by commenting on the merged PR:

   • /release to release-5.15.0
   • /release to release-5.15
   • /release to release-5.13.2
   • /release to release-5.13

3. Automated backport - The bot will automatically create backport PRs to the specified release branches

[probelabs]

This pull request addresses a critical race condition in the GraphQL gateway where concurrent requests could inadvertently share state, leading to non-deterministic behavior and potential credential leakage. The fix refactors the handling of request-specific data, such as transport configurations and headers, to be scoped to individual requests via context.Context rather than being stored in a shared, mutable http.Client.

Files Changed Analysis

• gateway/: reverse_proxy.go removes the now-obsolete variableReplaceRoundTripper. reverse_proxy_test.go is expanded to verify correct header handling.
• go.mod, go.sum: Updates the graphql-go-tools dependency to a version containing upstream race condition fixes.
• internal/graphengine/: Contains the core logic changes.
  • context.go: Introduces SetGraphQLEngineTransportContextValue to propagate request-scoped state.
  • transport.go: The GraphQLEngineTransport is refactored to be stateless, dynamically configuring itself from the request context. This includes fixes for header duplication and WebSocket handshakes.
  • engine_v{1,2,3}.go: All engine versions are updated to use the new context-based state propagation. A subscriptionRequestContext is added to fix premature WebSocket subscription termination. Engine V1 includes a fallback for tyk:// data sources.
  • Testing: A comprehensive new test suite, credential_isolation_test.go, has been added to cover numerous concurrency and isolation scenarios. Other test files (context_test.go, transport_test.go, etc.) are also significantly expanded.

Architecture & Impact Assessment

• Accomplishment: Eliminates a severe race condition, enhancing the security and reliability of the GraphQL subsystem by isolating request state.
• Key Changes:
  1. Request-Scoped State: Shifted from modifying a shared http.Client.Transport to passing request-specific data via the request's context.
  2. Dynamic Transport: The GraphQLEngineTransport now reads from the context to configure behavior per-request.
  3. Subscription Lifecycle Fix: A new subscriptionRequestContext links the subscription lifecycle to the long-lived API spec context, preventing premature cancellation.
  4. Header Resolution: Dynamic $tyk_context.* variables are now resolved correctly, ensuring unique subscription connection keys.

graph TD
    subgraph "before" ["Before: Mutable Shared State"]
        SharedClient[Shared http.Client] --> SharedTransport("Transport")
        R1[Request 1] -- modifies --> SharedTransport
        R2[Request 2] -- modifies --> SharedTransport
        SharedTransport --|"uses latest state (race condition)"|--> Upstream
    end

    subgraph "after" ["After: Request-Scoped State via Context"]
        SharedClient_A[Shared http.Client] --> WrapperTransport("GraphQLEngineTransport")
        
        subgraph "req1" ["Request 1"]
            R1_A[http.Request] --> C1("Context w/ RT1, Cfg1")
        end
        
        subgraph "req2" ["Request 2"]
            R2_A[http.Request] --> C2("Context w/ RT2, Cfg2")
        end

        R1_A -- passed to --> WrapperTransport
        R2_A -- passed to --> WrapperTransport

        WrapperTransport --|reads C1|--> Fetch1("Fetch for Request 1")
        WrapperTransport --|reads C2|--> Fetch2("Fetch for Request 2")

        Fetch1 --|uses RT1, Cfg1|--> Upstream_A[Upstream]
        Fetch2 --|uses RT2, Cfg2|--> Upstream_A
    end

Loading

• Affected Components: GraphQL Gateway, Authentication Propagation, GraphQL Subscriptions (all engine versions).

Scope Discovery & Context Expansion

The changes are well-contained within the GraphQL subsystem but are fundamental to its security and correctness. The investigation appears exhaustive, demonstrated by the comprehensive new test suite covering interactions between different features and engine versions under concurrent load.

References

• Core Logic: internal/graphengine/transport.go, internal/graphengine/context.go
• Pre-handler Mods: internal/graphengine/graphql_go_tools_v1.go, internal/graphengine/graphql_go_tools_v2.go
• Subscription Fix: internal/graphengine/engine_v2.go, internal/graphengine/engine_v3_reverse_proxy.go
• Isolation Tests: internal/graphengine/credential_isolation_test.go

Metadata

• Review Effort: 5 / 5
• Primary Label: n/a

---

Powered by Visor from Probelabs

Last updated: 2026-08-24T08:21:08.903Z | Triggered by: pr_opened | Commit: b8cef30

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	internal/graphengine/transport.go:50-56	The use of `sync.RWMutex` to protect `fallbackRoundTripper` introduces lock contention on the hot path for every request to a GraphQL EngineV1 API. The `setFallbackRoundTripper` method acquires a write lock and is called on each request for EngineV1 APIs. This serialization can become a bottleneck under high concurrent load. 💡 Suggestion For updating a single pointer value, consider using `atomic.Pointer` (available in Go 1.19+) to achieve a lock-free implementation. This would provide the same thread-safety guarantees without the contention caused by a mutex, improving performance for EngineV1 APIs under concurrent load.

---

Powered by Visor from Probelabs

Last updated: 2026-08-24T08:20:48.533Z | Triggered by: pr_opened | Commit: b8cef30

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.1% Coverage on New Code
1.5% Duplication on New Code

See analysis details on SonarQube Cloud