fix(sandbox): relay WebSocket frames after HTTP 101 Switching Protocols#718
fix(sandbox): relay WebSocket frames after HTTP 101 Switching Protocols#718johntmyers wants to merge 1 commit intomainfrom
Conversation
Detect 101 Switching Protocols in relay_response() and switch to raw bidirectional TCP relay instead of re-entering the HTTP parsing loop. Previously, is_bodiless_response() treated 101 as a generic 1xx informational response, forwarding only the headers and returning to the HTTP parsing loop. After a 101, subsequent bytes are upgraded protocol frames (e.g. WebSocket), not HTTP — causing the relay to block or silently drop all post-upgrade traffic. Changes: - Add RelayOutcome enum (Reusable/Consumed/Upgraded) replacing bool return type across L7Provider::relay trait and all relay functions - Detect 101 before generic 1xx handler in relay_response(), capture overflow bytes, return RelayOutcome::Upgraded - Validate client sent Upgrade + Connection: Upgrade headers before accepting 101 (rejects unsolicited upgrades from non-compliant upstream servers) - Extract shared handle_upgrade() helper used by both relay_rest() and relay_passthrough_with_credentials() - Add l7_decision=allow_upgrade audit log annotation for upgrades - Add unit tests for 101 overflow capture, unsolicited 101 rejection, and client_requested_upgrade header validation - Add integration test: WebSocket echo through L7Provider::relay Fixes: #652
|
tested pr #718 on docker desktop + wsl2 (windows 11, amd64, openshell v0.0.16). clean before/after results confirm the fix works. setup:
before (stock binary, v0.0.16): after (pr #718 fix/websocket-101-upgrade-relay-v2): proxy logs (after): the 101 is correctly detected in note on pr #720 interaction: we also merged depending on merge order:
either way it's a one-liner, just flagging it so it doesn't surprise anyone. |
|
Tested this against a live Slack Socket Mode deployment running OpenClaw inside an OpenShell sandbox. Different setup than @davidpeden3's socat echo test — this exercises the full application path including bidirectional file transfers. Environment
How we testedWe run two sandboxes under the same gateway —
ResultsEverything works:
The working policy (for anyone else testing)slack_websocket:
name: slack_websocket
endpoints:
- host: wss-primary.slack.com
port: 443
# tls: skip not needed with this fix
enforcement: enforce
access: full
- host: wss-backup.slack.com
port: 443
enforcement: enforce
access: full
binaries:
- { path: /usr/bin/node }
- { path: /usr/bin/curl }Bottom lineThis fixes #652 for us. On 0.0.16 the same config fails because Nice work @johntmyers — appreciate the quick turnaround. We didn't test the #720 interaction — only learned about it from @davidpeden3's comment. Our build was this branch only, no other PRs merged in. |
3 participants
Summary
Upgrade+Connection: Upgradeheaders before accepting 101 (rejects unsolicited upgrades from non-compliant upstream servers)boolreturn type withRelayOutcomeenum across theL7Providertrait and all relay functions for cleaner upgrade signalingRelated Issue
Changes
crates/openshell-sandbox/src/l7/provider.rs: AddRelayOutcomeenum (Reusable/Consumed/Upgraded { overflow }). UpdateL7Provider::relaytrait return type fromResult<bool>toResult<RelayOutcome>.crates/openshell-sandbox/src/l7/rest.rs: Detect 101 inrelay_response()before the generic 1xx/bodiless handler. ReturnRelayOutcomedirectly (no intermediate tuple). Addclient_requested_upgrade()validation inrelay_http_request_with_resolver(). Updaterelay_response_to_clientwrapper to returnRelayOutcome.crates/openshell-sandbox/src/l7/relay.rs: Add sharedhandle_upgrade()helper. Updaterelay_rest()andrelay_passthrough_with_credentials()to match onRelayOutcome. Addl7_decision = "allow_upgrade"audit log annotation.crates/openshell-sandbox/tests/websocket_upgrade.rs: Integration test spinning up a real WebSocket echo server, upgrading throughL7Provider::relay, and exchanging a text frame bidirectionally.crates/openshell-sandbox/Cargo.toml: Addtokio-tungsteniteandfuturesdev-dependencies for integration tests.Improvements over #683
(bool, u16, Vec<u8>)tupleRelayOutcomeenum directly fromrelay_response()L7Provider::relaystill returnedboolResult<RelayOutcome>Upgrade+Connection: Upgradeheadershandle_upgrade()helperl7_decision = "allow_upgrade"for observabilityrelay_response_to_clientsilently discarded overflowRelayOutcomeL7Provider::relaySecurity Notes
copy_bidirectional, no further L7 inspection occurs. The initial HTTP request is policy-checked before the upstream ever sees it.Consumed). Prevents a non-compliant upstream from triggering a policy bypass.Testing
mise run pre-commitpasses (format fixed)client_requested_upgradeheader parsing testscargo clippyclean (no new warnings)Checklist