Skip to content

[TT-17442] Upstream WebSocket connection reuse ignores per-request auth headers#8303

Merged
radkrawczyk merged 3 commits into
masterfrom
TT-17442-upstream-web-socket-connection-reuse-ignores-per-request-auth-headers
Jun 11, 2026
Merged

[TT-17442] Upstream WebSocket connection reuse ignores per-request auth headers#8303
radkrawczyk merged 3 commits into
masterfrom
TT-17442-upstream-web-socket-connection-reuse-ignores-per-request-auth-headers

Conversation

@MaciekMis

Copy link
Copy Markdown
Contributor

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🎯 Recommended Merge Targets

Based on JIRA ticket TT-17442: Upstream WebSocket connection reuse ignores per-request auth headers

Fix Version: Tyk 5.8.15

Required:

  • release-5.8 - Minor version branch for 5.8.x patches - required for creating Tyk 5.8.15
  • master - Main development branch - ensures fix is in all future releases

Fix Version: Tyk 5.13.1

Required:

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

Fix Version: Tyk 5.14.0

⚠️ Warning: Expected release branches not found in repository

Required:

  • master - No matching release branches found. Fix will be included in 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.8
    • /release to release-5.13
  3. Automated backport - The bot will automatically create backport PRs to the specified release branches

@probelabs

probelabs Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

This PR addresses a bug where reused upstream WebSocket connections for GraphQL subscriptions ignored per-request authentication headers. The fix is implemented by updating the github.com/TykTechnologies/graphql-go-tools dependency to a new version that contains the corrected logic.

Files Changed Analysis

The changes are limited to dependency management files:

  • go.mod: The version of github.com/TykTechnologies/graphql-go-tools is updated.
  • go.sum: The checksums for the updated dependency are modified to reflect the new version.

This indicates that the functional fix is entirely contained within the updated dependency, and this PR serves to integrate that fix into the main gateway.

Architecture & Impact Assessment

  • What this PR accomplishes: This PR fixes a security vulnerability in the GraphQL proxy. Previously, when the gateway reused a WebSocket connection for multiple subscription requests, it failed to forward the unique authentication headers of each new request, sending the headers from the initial request instead. This could lead to authentication failures or incorrect authorization for subsequent clients. The fix ensures each request's headers are correctly forwarded.

  • Key technical changes introduced: The only technical change is the version bump of the graphql-go-tools library. The underlying fix within that library ensures that the gateway forwards the correct, per-request headers when communicating with the upstream service over a shared WebSocket connection.

  • Affected system components: The primary component affected is the GraphQL Proxy Engine within the Tyk Gateway, specifically its handling of WebSocket-based GraphQL subscriptions. Any APIs configured to use GraphQL subscriptions with dynamic, per-request authorization (e.g., JWTs, API keys in headers) are impacted.

graph TD
    subgraph Before_Fix ["Behavior Before Fix"]
        Client1[Client 1 w/ Auth Header A] --> GQLProxy(GraphQL Proxy);
        Client2[Client 2 w/ Auth Header B] --> GQLProxy;
        GQLProxy --|1. Establishes WebSocket, sends Auth A|--> Upstream{Upstream GraphQL Service};
        GQLProxy --|2. Reuses WebSocket, sends Auth A again (Incorrect)|--> Upstream;
    end

    subgraph After_Fix ["Behavior After Fix"]
        Client1_fix[Client 1 w/ Auth Header A] --> GQLProxy_fix(GraphQL Proxy);
        Client2_fix[Client 2 w/ Auth Header B] --> GQLProxy_fix;
        GQLProxy_fix --|1. Establishes WebSocket, sends Auth A|--> Upstream_fix{Upstream GraphQL Service};
        GQLProxy_fix --|2. Reuses WebSocket, sends Auth B (Correct)|--> Upstream_fix;
    end

    style GQLProxy fill:#f9f,stroke:#333,stroke-width:2px
    style GQLProxy_fix fill:#9f9,stroke:#333,stroke-width:2px
Loading

Scope Discovery & Context Expansion

While the code change in this repository is minimal, the functional impact is significant for the security and correctness of GraphQL subscriptions. The issue is rooted in how the underlying graphql-go-tools library handles the lifecycle of WebSocket connections and the injection of request metadata.

The fix, contained within the dependency, likely targets the WebSocket transport layer to ensure that an appropriate hook properly injects the latest request's headers into the outgoing message to the upstream service, even when the underlying connection is reused.

Metadata
  • Review Effort: 2 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-06-11T05:21:07.135Z | Triggered by: pr_updated | Commit: 15724dd

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

@probelabs

probelabs Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity Location Issue
🟡 Warning go.mod:29
The version of the `github.com/TykTechnologies/graphql-go-tools` dependency was updated without any explanation in the pull request description. The PR template includes a checklist item 'I explained why this PR updates go.mod in detail with reasoning why it's required', which has been left unchecked. Updating dependencies without justification introduces risk and makes it difficult for reviewers and future maintainers to understand the change's impact.
💡 SuggestionUpdate the pull request description to detail the reasons for this dependency upgrade. Explain what changes are included in the new version (e.g., bug fixes, new features) and why they are necessary for this project. Linking to the relevant changes or release notes in the dependency's repository is also best practice.

Powered by Visor from Probelabs

Last updated: 2026-06-11T05:20:50.564Z | Triggered by: pr_updated | Commit: 15724dd

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

@MaciekMis
MaciekMis requested a review from kofoworola June 10, 2026 09:58
@kofoworola kofoworola added the deps-reviewed Dependency changes reviewed and approved for CI execution label Jun 10, 2026
@radkrawczyk
radkrawczyk self-requested a review June 10, 2026 11:50

@radkrawczyk radkrawczyk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@MaciekMis MaciekMis closed this Jun 10, 2026
@MaciekMis MaciekMis reopened this Jun 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚨 Jira Linter Failed

Commit: 15724dd
Failed at: 2026-06-11 05:19:45 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: failed to fetch Jira issue TT-17442: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

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.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@radkrawczyk
radkrawczyk merged commit d0d255d into master Jun 11, 2026
52 of 55 checks passed
@radkrawczyk
radkrawczyk deleted the TT-17442-upstream-web-socket-connection-reuse-ignores-per-request-auth-headers branch June 11, 2026 07:34
@radkrawczyk

Copy link
Copy Markdown
Contributor

/release to release-5.8

@radkrawczyk

Copy link
Copy Markdown
Contributor

/release to release-5.13

@probelabs

probelabs Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8308

@probelabs

probelabs Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

✅ Cherry-pick successful. A PR was created: #8309

radkrawczyk added a commit that referenced this pull request Jun 11, 2026
…se ignores per-request auth headers (#8303) (#8309)

[TT-17442] Upstream WebSocket connection reuse ignores per-request auth
headers (#8303)

<!-- Provide a general summary of your changes in the Title above -->

## Description

<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

Co-authored-by: Radosław Krawczyk
<[email protected]>

[TT-17442]:
https://tyktech.atlassian.net/browse/TT-17442?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: Maciej Miś <[email protected]>
Co-authored-by: Radosław Krawczyk <[email protected]>
radkrawczyk pushed a commit that referenced this pull request Jun 12, 2026
…e ignores per-request auth headers (#8303) (#8308)

Cherry-pick of `d0d255dcf75c4549e2d5149c60ef90ca381bbe12` from `master`
to `release-5.8` requires manual resolution.

  **Conflicts detected:** 3
   - go.sum
  
  Tips:
- Check out this branch locally and run: `git cherry-pick -x
d0d255d`
- Resolve conflicts (including submodules if any), then push back to
this branch.
  
Original commit:
d0d255d

---------

Co-authored-by: Tyk Bot <[email protected]>
Co-authored-by: Maciej Miś <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps-reviewed Dependency changes reviewed and approved for CI execution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants