fix(sandbox): rewrite credential placeholders in websocket text frames#1286
fix(sandbox): rewrite credential placeholders in websocket text frames#1286ericksoa wants to merge 12 commits intoNVIDIA:mainfrom
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
All contributors have signed the DCO ✍️ ✅ |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Comparative implementation audit: WebSocket credential rewriteThis is a behavior-level comparison for PR memory. I compared the branch against permissively licensed, widely used WebSocket implementations and did not copy code from any of them. Comparison setAll comparison targets are permissively licensed enough for behavioral review and architectural comparison:
Scope differenceOpenShell is not a general WebSocket endpoint library. The PR implements a security relay for a narrow problem: keep provider secrets out of sandbox-owned environment, argv, config, logs, and generated payloads, then resolve placeholders only at the relay boundary for opted-in client-to-server text frames. That means several mature-library features are intentionally not goals here: application APIs, broadcast helpers, endpoint origin policy, generic server/client ownership, and full bidirectional message inspection. The relevant comparison is protocol safety plus operational confidence around the relay path. Where OpenShell is stronger or more specialized
Where mature implementations are ahead
Plan to close the differences
Current conclusionThe branch is defensible for the issue-specific threat model and stronger than generic libraries on the no-secret relay invariant. It is weaker than mature WebSocket stacks on external conformance proof, structured negotiation parsing, subprotocol validation, close-handshake polish, and performance/fuzz history. The most valuable next step is not broadening features; it is adding external conformance and parser-hardening evidence while keeping the accepted protocol surface intentionally small. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Follow-up hardening passThis pass turns the portable lessons from mature WebSocket stacks into OpenShell-specific behavior, without copying implementation code: parse negotiation as structured protocol tokens, validate server-selected options against the original client offer, canonicalize equivalent safe extension forms, and fail closed when the rewrite path sees parameters it does not explicitly support. Concretely, This matters for credential rewriting because the proxy becomes a protocol participant after it mutates an upgraded flow. Loose negotiation can let the upstream select compression state or subprotocol behavior the relay did not actually offer or understand, which creates room for state confusion, fail-open compression handling, or secret-bearing payloads moving through a path that no longer has the invariants the rewriter depends on. The new behavior keeps the supported surface narrow and deterministic: either the relay can validate and safely process the negotiated shape, or the handshake fails before forwarding the 101 to the client. Remaining conformance work is still useful, especially an Autobahn-style relay harness and broader fuzz/property coverage for handshake parsing and frame sequencing. Those should come after this focused spec pass so future parser or engine swaps have a stable regression matrix to preserve. |
|
I have read the DCO document and I hereby sign the DCO. |
|
I have read the Contributor Agreement including DCO and I hereby sign the Contributor Agreement and DCO |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Focused conformance matrix addedI added
This is not a replacement for Autobahn. It is the closest low-cost regression layer for this PR because it covers the specific OpenShell behavior boundary: once the proxy becomes an active WebSocket participant for credential rewriting, the handshake assumptions, compression negotiation, and post-upgrade frame relay need to be tested together rather than only as isolated parser/frame unit tests. Full Autobahn remains useful as a follow-up CI or manual conformance job, but it would add Docker/runtime/tooling weight beyond this focused PR pass. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Progress update: policy configuration now treats WebSocket as a first-class inspected protocol in the incremental update flow. What changed:
Why it matters: Validation:
|
…dential-rewrite Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Progress update: WebSocket credential replacement is now configurable from the incremental policy workflow and covered through the route-selected relay path. What changed:
Why it matters: Validation:
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Follow-up update after the policy CLI shape review:
openshell policy update demo \
--add-endpoint realtime.example.com:443:read-write:websocket:enforce:websocket-credential-rewrite \
--binary /usr/bin/nodeRationale: Message-content policy would buy a different class of control than this PR currently implements: it would let OpenShell authorize WebSocket messages by application intent inside the text payload, such as event type, channel, action, model/tool name, or service-specific JSON shape, instead of only by host/port, upgrade path, and Validation after this update:
All passed locally. |
Summary
Fixes #872 by adding an opt-in WebSocket credential placeholder rewrite path for REST/L7 endpoints after an allowed HTTP
101 Switching Protocolsupgrade.The implementation preserves the placeholder/no-secret-in-sandbox model: provider secrets remain out of sandbox env, argv, config, logs, and sandbox-generated payloads until OpenShell reaches the relay boundary. This complements, but does not replace, #984 selective credential passthrough.
This is scoped to client-to-server WebSocket text-frame credential rewrite. It is not generic post-upgrade L7 inspection.
Related Issue
Fixes #872
Changes
websocket_credential_rewriteas an opt-in endpoint policy field, defaulting tofalse.101headers, including status,Upgrade,Connection,Sec-WebSocket-Accept, and safe extension negotiation.openshell:resolve:env:*placeholders.Security
protocol: restendpoints.Testing
mise run pre-commitpassesAdditional focused validation run locally:
cargo fmtcargo test -p openshell-sandbox websocketcargo test -p openshell-sandbox secretscargo test -p openshell-policycargo clippy -p openshell-sandbox -p openshell-policy --all-targets -- -D warningsmise run docsmise run ciAutobahn was not run because the repo does not include an Autobahn harness; the PR adds focused RFC 6455 and RFC 7692 regression tests instead.
Checklist