Skip to content

fix(ingress): forward all headers in WebSocket proxy#1058

Open
Pangjiping wants to merge 3 commits into
mainfrom
fix/ingress-ws-proxy-forward-headers-1050
Open

fix(ingress): forward all headers in WebSocket proxy#1058
Pangjiping wants to merge 3 commits into
mainfrom
fix/ingress-ws-proxy-forward-headers-1050

Conversation

@Pangjiping

Copy link
Copy Markdown
Collaborator

Summary

  • Replace hardcoded header whitelist (Origin, Sec-WebSocket-Protocol, Cookie) with blacklist approach in WebSocket proxy, forwarding all headers except hop-by-hop (RFC 7230 §6.1) and dialer-managed WebSocket handshake headers
  • Extract hop-by-hop and WebSocket handshake header constants into header.go
  • Fixes X-EXECD-ACCESS-TOKEN being silently dropped, causing execd 401 on PTY WebSocket connections

Closes #1050

Test plan

  • go build ./... passes
  • go test ./pkg/proxy/... passes
  • Manual: create sandbox with EXECD_ACCESS_TOKEN, connect PTY WebSocket through ingress URI mode, verify upgrade succeeds

🤖 Generated with Claude Code

…oded whitelist

The WebSocket proxy only forwarded Origin, Sec-WebSocket-Protocol, and
Cookie headers to the backend, silently dropping all others. This caused
execd to reject PTY WebSocket connections with 401 because the
X-EXECD-ACCESS-TOKEN header never reached it.

Replace the whitelist with a blacklist that skips only hop-by-hop headers
(RFC 7230 §6.1) and WebSocket handshake headers managed by the dialer,
matching httputil.ReverseProxy behavior on the HTTP path.

Closes #1050

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e44f00d0c9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/ingress/pkg/proxy/websocket.go
Parse Connection header values and skip any header listed as a
connection token, in addition to the fixed hop-by-hop set.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12ac3d1907

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread components/ingress/pkg/proxy/websocket.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working component/ingress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ingress WebSocket proxy drops X-EXECD-ACCESS-TOKEN header causing execd 401 / bad handshake

3 participants