Fix/wss port0 reverse proxy - #693
Closed
HanGYvv wants to merge 7 commits into
Closed
Conversation
added 7 commits
August 9, 2026 11:08
Split the monolithic build workflow into separate ci, debian, docker, and release workflows backed by reusable composite actions, and add a release-notes helper binary.
Add Docker Hub README files for the s6 and classic images, consumed by the docker workflow's description-sync step. Update the main README badges and links to point at the new workflows and repository.
Ported and adapted from lejianwen/forapi: - MUST_LOGIN / JWT login enforcement - RegisterPk / OnlineRequest handling - WebSocket peer forwarding Adjusted for the current codebase and stricter JWT secret enforcement.
hbb_common now pulls in native-tls -> openssl-sys on all platforms, but the cross musl images have no system OpenSSL, so every Linux target (amd64/arm64v8/armv7/i386) failed to compile in the release/docker/debian workflows. Build OpenSSL from source (vendored) on non-macOS/Windows targets, both as a normal dependency and as a build-dependency (build.rs also compiles hbb_common for the host).
UDP send errors (e.g. EINVAL when the destination port is 0, hit by the self-test heartbeat) only drop one datagram; the socket is still healthy. Treating them as fatal made the io_loop drop and recreate the socket in a crash loop. Keep recreation for receive-side failures only, and drop UDP packets from source port 0 outright (no legitimate client sends from port 0).
Clients behind a reverse proxy register with an `IP:0` address that has no reachable UDP port, so RequestRelay (and punch retries) sent over UDP were silently dropped with EINVAL and relay could never be established. Route peer messages through the peer's registered WebSocket/TCP sink first via a shared send_to_peer helper, falling back to UDP only when no sink exists.
A register_pk / register_peer over WebSocket behind a reverse proxy arrives with an address rewritten from X-Real-IP to `IP:0` (port 0 is a marker, not a reachable endpoint). Accepting it overwrote the peer's real UDP socket address that punching/relay depend on. Only update the stored address when the new one is non-zero, or when no real address is held yet.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughChangesRelease automation
Rendezvous authentication and peer delivery
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RendezvousServer
participant JWT
participant Peer
participant Transport
Client->>RendezvousServer: send authenticated peer request
RendezvousServer->>JWT: verify token
JWT-->>RendezvousServer: return claims
RendezvousServer->>Peer: resolve peer state
RendezvousServer->>Transport: send through WebSocket or TCP sink
Transport-->>Client: forward peer message
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
Author
|
Sorry, I submitted it to the wrong place. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| shell: bash | ||
| run: | | ||
| if [ "${GITHUB_EVENT_NAME}" = "workflow_dispatch" ]; then | ||
| tag="${{ inputs.version }}" |
There was a problem hiding this comment.
When a manual run supplies shell syntax in inputs.version, GitHub inserts it directly into the Bash assignment, so it executes before the subsequent regex validation and can run commands with the release workflow's write-capable token. Pass the value through an environment variable or positional argument instead; the same unsafe interpolation occurs in the Debian and Docker workflows. How this was verified: The input expression is embedded in the assignment on line 38, while validation begins only on line 43.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 38
Comment:
**Version input executes as shell**
When a manual run supplies shell syntax in `inputs.version`, GitHub inserts it directly into the Bash assignment, so it executes before the subsequent regex validation and can run commands with the release workflow's write-capable token. Pass the value through an environment variable or positional argument instead; the same unsafe interpolation occurs in the Debian and Docker workflows. **How this was verified:** The input expression is embedded in the assignment on line 38, while validation begins only on line 43.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+32
to
+35
| DOCKER_IMAGE: hangyvv/rustdesk-server-s6 | ||
| DOCKER_IMAGE_CLASSIC: hangyvv/rustdesk-server | ||
| GHCR_IMAGE: ghcr.io/hangyvv/rustdesk-server-s6 | ||
| GHCR_IMAGE_CLASSIC: ghcr.io/hangyvv/rustdesk-server |
There was a problem hiding this comment.
On every tagged or manually dispatched Docker release, these global destinations route all architecture images and manifests to the contributor's hangyvv namespaces instead of RustDesk's release channels, so the project's Docker Hub and GHCR images do not receive the release.
Suggested change
| DOCKER_IMAGE: hangyvv/rustdesk-server-s6 | |
| DOCKER_IMAGE_CLASSIC: hangyvv/rustdesk-server | |
| GHCR_IMAGE: ghcr.io/hangyvv/rustdesk-server-s6 | |
| GHCR_IMAGE_CLASSIC: ghcr.io/hangyvv/rustdesk-server | |
| DOCKER_IMAGE: rustdesk/rustdesk-server-s6 | |
| DOCKER_IMAGE_CLASSIC: rustdesk/rustdesk-server | |
| GHCR_IMAGE: ghcr.io/rustdesk/rustdesk-server-s6 | |
| GHCR_IMAGE_CLASSIC: ghcr.io/rustdesk/rustdesk-server |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/docker.yaml
Line: 32-35
Comment:
**Releases target personal registries**
On every tagged or manually dispatched Docker release, these global destinations route all architecture images and manifests to the contributor's `hangyvv` namespaces instead of RustDesk's release channels, so the project's Docker Hub and GHCR images do not receive the release.
```suggestion
DOCKER_IMAGE: rustdesk/rustdesk-server-s6
DOCKER_IMAGE_CLASSIC: rustdesk/rustdesk-server
GHCR_IMAGE: ghcr.io/rustdesk/rustdesk-server-s6
GHCR_IMAGE_CLASSIC: ghcr.io/rustdesk/rustdesk-server
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Greptile Summary
The PR extends rendezvous registration and routing for WebSocket clients behind reverse proxies, adds optional JWT login enforcement, and substantially restructures CI and release automation.
Confidence Score: 2/5
The PR is not safe to merge until the release-input command injection and incorrect personal container-registry destinations are fixed.
Manual version values are parsed as Bash code in privileged release workflows, and every Docker release is directed to the contributor's personal Docker Hub and GHCR namespaces rather than the project's release channels.
Files Needing Attention: .github/workflows/release.yaml, .github/workflows/debian.yaml, .github/workflows/docker.yaml
Security Review
The manual release workflows interpolate the supplied version directly into Bash source before validation, allowing shell syntax in that input to execute in privileged workflow jobs. How this was verified: The expression is embedded at the assignment line and the regex check occurs only in subsequent shell statements.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Client[Client] -->|UDP/TCP registration| HBBS[HBBS rendezvous server] Proxy[WSS reverse proxy] -->|WebSocket registration<br/>forwarded IP, port 0| HBBS HBBS --> PeerMap[Peer map] HBBS --> UDP[UDP peer delivery] HBBS --> WS[WebSocket sink map] Requester[Connection requester] -->|PunchHoleRequest + optional JWT| HBBS HBBS -->|Route request| UDP HBBS -->|Route request| WSPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(hbbs): don't clobber real peer addr ..." | Re-trigger Greptile