Skip to content

feat(middleware): inspect WebSocket text messages - #2477

Open
pimlock wants to merge 29 commits into
mainfrom
2428-websocket-message-introspection/pimlock
Open

feat(middleware): inspect WebSocket text messages#2477
pimlock wants to merge 29 commits into
mainfrom
2428-websocket-message-introspection/pimlock

Conversation

@pimlock

@pimlock pimlock commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Tip

Visual overview of this change: preview, source.
Note: this reads from github without auth, so it may be rate limited, especially when on VPN.

This pull request adds supervisor middleware for WebSocket traffic.

OpenShell asks selected middleware for a session decision before it contacts the upstream service. A middleware stage can inspect the session, skip it, or deny the upgrade. A skipped stage uses no persistent stream and receives no messages for that session.

After the upstream service accepts the upgrade, middleware can inspect complete text messages from the client. It can allow, replace, or deny each message. OpenShell runs middleware before it adds managed credentials.

This first version does not inspect messages from the upstream service, binary messages, or control frames. Binary messages pass without inspection and produce coverage telemetry. Inspection of messages from the upstream service remains future work.

This preflight lifecycle also provides a base for other streaming protocols, such as streaming HTTP.

Related Issue

Part of #2428

Changes

Attachment and preflight

  • Use the existing endpoints.include and endpoints.exclude host selectors to attach middleware.
  • Select only middleware that advertises WEBSOCKET_MESSAGE/PRE_CREDENTIALS.
  • Send a bounded preflight request before upstream contact and credential injection.
  • Support INSPECT, SKIP, and DENY preflight decisions.
  • Treat SKIP as a successful decision. It does not use on_error.
  • Enforce DENY before upstream contact. It applies in both error modes.
  • Keep arbitrary headers, query data, and message bodies out of WebSocket preflight in this version.
  • Let an HTTP-only attachment inspect the upgrade request without joining the WebSocket message chain. Emit binding_not_selected coverage for traffic after the upgrade.

The host selector is less precise than a request selector. Future work can add more selection inputs after we have clear use cases.

Message processing

  • Open one ordered EvaluateWebSocket stream for each inspecting stage.
  • Report the selected subprotocol after the upstream service accepts the upgrade.
  • Reassemble fragmented client text messages before middleware evaluation.
  • Use session-wide sequence numbers. Each stage receives an increasing subset, so gaps are valid.
  • Apply transformations before credential placeholder resolution.
  • Check message policy again after a middleware transformation.
  • Close stale sessions after a policy reload.
  • Support the built-in openshell/regex middleware for HTTP bodies and client WebSocket text messages.

Resource limits

OpenShell records middleware limits in architecture/sandbox-limits.md.

Each sandbox supervisor owns its middleware registry and its local budgets. One sandbox cannot directly consume another sandbox's permits. The limits still protect host memory, CPU time, relay tasks, gRPC streams, channel buffers, and shared middleware services.

Initial limits:

  • 32 active middleware operations.
  • 64 additional unbuffered waiters.
  • 32 persistent middleware sessions.
  • 10 stages in one chain.
  • A 4 MiB platform ceiling for one HTTP body or WebSocket text message.
  • A 30-second input-idle limit and a two-minute total limit for text-message assembly.
  • 4,096 fragments in one logical message.

These are permissive alpha defaults. Production data can change them. Future configuration can let operators reduce selected limits below platform ceilings.

When both work budgets are full, OpenShell returns 503 Service Unavailable before it buffers an HTTP body. WebSocket overload keeps its protocol-specific failure behavior.

Compression

OpenShell does not disable all WebSocket compression.

For parsed modes that manage WebSocket extensions, OpenShell supports permessage-deflate only when client-to-server context takeover is disabled with client_no_context_takeover. OpenShell removes unsupported extension offers.

OpenShell decompresses and recompresses each inspected client message with a fresh DEFLATE context. Each client message is independently decodable. This mode also keeps memory accounting and message replacement bounded.

Server-to-client context takeover can remain enabled because this version does not inspect or replace messages in that direction.

Failure handling and telemetry

  • Apply on_error only after a selected binding fails.
  • Disable a broken fail-open stage for the rest of the session.
  • Release work and session permits when a stage or session ends.
  • Emit safe invocation, failure, coverage, saturation, and lifecycle events.
  • Use typed close behavior for protocol errors and policy or middleware decisions.
  • Use close codes 1002, 1007, 1008, 1009, and 1012 for the defined terminal cases.

Testing

  • mise run pre-commit
  • mise run test
  • Focused WebSocket lifecycle, compression, saturation, and fault tests
  • mise run ci
  • mise run e2e:websocket-conformance
  • mise run e2e:docker

Checklist

  • Uses Conventional Commits
  • Includes DCO sign-off on each commit
  • Updates architecture and user documentation
  • Updates related agent skills

pimlock

This comment was marked as resolved.

@pimlock pimlock added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 25, 2026
@github-actions

Copy link
Copy Markdown

@pimlock

This comment was marked as resolved.

@pimlock pimlock left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This PR remains project-valid because it implements the maintainer-authored, review-ready PR1 plan in #2428.
Head SHA: d81890743a80e74962ab4fdf8a51b22e068f2477

Thanks @pimlock. I checked your update describing the plaintext-upgrade path, bounded admission, typed termination, stream lifetime, fail-open observability, docs, skills, and expanded coverage. The plaintext path, pre-buffer admission, typed close handling, stream-deadline removal, mapped skill updates, and much of the added coverage are present. The independent re-review found three high-severity and four medium-severity issues that remain; they are attached inline.

Review findings:

  • High: incomplete WebSocket messages can pin all shared middleware admission indefinitely.
  • High: middleware-only parsed WebSockets do not observe policy-generation reloads.
  • High: the operator docs claim the built-in regex covers WebSockets, but its manifest and implementation do not.
  • Medium: persistent middleware streams have no independent session bound; fully disabled fail-open sessions still require admission; one path performs token-grant work before preflight; and policy denial is reported as middleware denial.

Docs: Fern pages and all three mapped skills were updated, but the built-in WebSocket coverage claim must be corrected or implemented.

Next state: gator:in-review pending author changes. No local tests were run as part of this code-only review.

Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs Outdated
Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs
Comment thread docs/extensibility/supervisor-middleware.mdx
Comment thread crates/openshell-supervisor-middleware/src/websocket.rs Outdated
Comment thread crates/openshell-supervisor-middleware/src/websocket.rs Outdated
Comment thread crates/openshell-supervisor-network/src/l7/relay.rs
Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs Outdated
pimlock

This comment was marked as duplicate.

@pimlock
pimlock marked this pull request as draft July 27, 2026 21:00
@copy-pr-bot

This comment was marked as resolved.

@pimlock pimlock added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Jul 27, 2026
pimlock added 6 commits July 27, 2026 15:14
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock
pimlock force-pushed the 2428-websocket-message-introspection/pimlock branch from ef26419 to 83121b1 Compare July 27, 2026 22:16
pimlock added 2 commits July 27, 2026 16:11
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock

pimlock commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test dea07e4

pimlock added 4 commits July 28, 2026 10:10
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock pimlock added gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates and removed gator:blocked Gator is blocked by process or repository gates gator:in-review Gator is reviewing or awaiting PR review feedback labels Aug 1, 2026
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock

pimlock commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

gator-agent

Review Convergence Checkpoint

Head SHA: 274567063ea0c5f8703d67e2e0781d764ea6bd73
Base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Merge base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Patch ID: 745c3b8b6d0b42bde9f9cb154de39fda1e9773f7
Gator payload: 2

Four finding-bearing review rounds have completed, so Gator has reached the required human convergence checkpoint.

Thanks @pimlock. I checked the merge-main update and its conflict resolutions in the six affected files. The checkpoint-only independent review found no new Critical defect introduced by that delta.

Root-cause findings:

  • Open legacy obligation: a retained inspecting middleware session can force the parsed WebSocket relay on routes where extension negotiation still preserves permessage-deflate without configuring decompression. A valid RSV1 client text frame can then be rejected with 1002. The prior request was to make extension negotiation depend on the retained inspecting session and cover compressed ws:// and wss:// relay paths.
  • Waived by verified maintainer @pimlock: the already-skipped preflight stage reports stage-local CANCELLATION when another stage later denies or fails closed. @pimlock explicitly deferred this telemetry-attribution refinement to a focused follow-up; the aggregate traffic decision remains fail-closed.
  • Resolved: the earlier admission/resource bounds, reload and session lifecycle, plaintext preflight ordering, typed close behavior, policy-generation checks, forwarding bounds, documentation, and compatibility findings remain resolved.

Scope growth:

  • Remediation expanded into process-lifetime assembly/work/session budgets, reload-generation state, plaintext and TLS WebSocket relay behavior, compression handling, typed lifecycle reporting, and published limit documentation.

Reviewer-quality signals:

  • Ledger: 7 review rounds, 4 finding-bearing rounds, 26 unique legacy inline findings, 0 duplicate finding IDs, and 0 findings repeated across review heads.
  • Current checkpoint review: 0 proposed findings, 0 unchanged-code proposals, 0 duplicate invariants, and 0 waived findings re-raised.

Maintainer action: accept the current scope, split the remaining extension-negotiation obligation into follow-up work, waive it, or explicitly authorize another autonomous review round.

Next state: gator:blocked

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock

pimlock commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Maintainer convergence update

Addressed the remaining compression obligation in 71579c1a.

  • A retained inspecting middleware session now selects safe permessage-deflate negotiation before the upgrade is forwarded, including REST compatibility and plaintext forward-proxy routes.
  • Added end-to-end compressed-message regressions for both wss:// (post-TLS REST relay) and ws:// (plaintext forward proxy). Each sends a valid RSV1 text message through openshell/regex and verifies redacted compressed text reaches upstream instead of a 1002 protocol close.
  • Message-result SKIP remains intentionally unsupported until a concrete use case defines its per-message semantics; the exploratory review thread is resolved.
  • The previously waived mixed-preflight terminal-reason attribution remains deferred. Aggregate admission is still fail-closed, so that deferral does not admit rejected traffic or credentials.

Validation:

  • mise exec -- cargo test -p openshell-supervisor-network middleware_inspects_compressed --no-default-features
  • mise run test
  • mise run pre-commit

I explicitly authorize one final focused Gator verification round on current head 71579c1a51d15afb98371c2aa48f64e79532c334, limited to the compression fix and current-head convergence. If it finds no new Critical or High defect, please conclude the autonomous review and hand the PR to a CODEOWNER for final approval.

@pimlock

pimlock commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

gator-agent

Re-check After Maintainer Update

Thanks @pimlock. I re-evaluated current head 71579c1a51d15afb98371c2aa48f64e79532c334 after your August 4 convergence update authorizing one final focused verification of the compression fix.

Head SHA: 71579c1a51d15afb98371c2aa48f64e79532c334
Base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Merge base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Patch ID: 707470e621a5af6443cdd62dd56e144170797b02
Gator payload: 2
Review mode: human_checkpoint

What I checked: the post-checkpoint author delta, the retained-session extension-negotiation decision in the REST/TLS and plaintext forward-proxy paths, and the compressed wss:// and ws:// message regressions. The focused independent review found no new Critical defect.

Disposition: resolved. A retained inspecting middleware session now selects managed permessage-deflate negotiation, so the parsed relay is configured to decompress and recompress valid compressed client text messages on both supported paths instead of rejecting RSV1 with 1002.

The mixed-preflight terminal-reason attribution remains waived by your verified maintainer disposition and was not re-raised. All other durable Gator findings remain resolved.

Remaining items:

  • No blocking review findings remain.
  • Required Branch Checks, Helm Lint, and E2E checks for this head are still in progress; test:e2e was already present and Gator did not apply or change it.
  • Maintainer approval is still required after the checks pass.

Next state: gator:watch-pipeline

I did not approve, merge, push, apply test:e2e, or post /ok to test.

@pimlock pimlock added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:blocked Gator is blocked by process or repository gates labels Aug 4, 2026
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock pimlock added the area:supervisor Proxy and routing-path work label Aug 5, 2026
@pimlock

pimlock commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

gator-agent

Re-check After Author Update

Thanks @pimlock. I checked head b79de406b71127c8e6042b7d54914607afc5d2af after your August 5 test-stabilization commit for the compressed WebSocket regressions.

Head SHA: b79de406b71127c8e6042b7d54914607afc5d2af
Base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Merge base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Patch ID: a298923d7b02bc498b9d9969f4662ffede18396d
Gator payload: 2
Review mode: human_checkpoint
Previous reviewed SHA: 71579c1a51d15afb98371c2aa48f64e79532c334

What I checked: the author-only two-file test delta, including the bounded scenario orchestration, relay-task cleanup, and the arm64 timeout path that failed on the previous head. The bounded independent checkpoint review found no newly introduced Critical security, data-loss, or correctness defect. No production behavior changed.

Disposition: resolved. All durable Gator findings remain resolved or maintainer-waived, and no blocking review findings remain.

Required Branch Checks and E2E are running for this head. Helm Lint and GPU E2E are green. The existing test:e2e label was already present; Gator did not apply or change it.

Next state: gator:watch-pipeline

I did not approve, merge, push, apply test:e2e, or post /ok to test.

@pimlock pimlock added gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Aug 5, 2026
}

message WebSocketPreflightDecision {
WebSocketPreflightAction action = 1;

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.

long chat w/ agent:

Should an INSPECT preflight decision be able to declare which requested subprotocols the middleware supports? Preflight sees the offered protocols, but the actual selection is learned later through the notification-only session_start. If middleware supports only a subset, it currently cannot distinguish “inspect if upstream selects X” from “inspect regardless of the selection.”
Consider adding something like accepted_subprotocols to WebSocketPreflightDecision. OpenShell could validate the upstream selection before starting message inspection and apply explicitly defined behavior—such as denying or skipping—when the selected protocol is outside that set. This keeps preflight as the single decision point instead of introducing a second decision at session start.

Bascially, do we see the initial preflight req/resp as being protocol-aware?

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.

the callout was that the subprotocol-negotiation happens after this initial decision, which kinda makes a nested state machine

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good question.

I think we can defer this until a concrete middleware needs it. We can still add it later, if we get some signal on it being needed. Adding it now introduces more conditions into an already complex WebSocket handshake and I'm not sure how common this subprotocol negotiation is, how often does it include multiple options, etc. (tried to look for this and it seems like it's not that common).

What could go wrong today?

  • Client offers subprotocols [a, b, c].
  • Middleware only understands b.
  • Middleware sees the offered protocols during preflight, but cannot make INSPECT conditional on the final selection.
  • Upstream selects a.
  • Middleware receives session_start with a, but that event is notification-only.
  • When messages arrive, middleware may fail to parse them and either deny or trigger its configured on_error behavior.

If middleware could declare support for [b], OpenShell could narrow the upstream offer to [b]. If upstream supports b, the connection could proceed successfully.

Possible future flow

  • Client offers [a, b, c].
  • Preflight returns an optional accepted-subprotocol constraint.
  • We intersect all sets (client + all middleware's preflight) and offer that to the server.
  • One gets selected or server rejects the upgrade.

This would be an addition to the preflight decision and we could make it backward compatible (i.e. not a one-way door).

@pimlock pimlock added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:approval-needed Gator completed review; maintainer approval needed labels Aug 5, 2026
Comment thread proto/supervisor_middleware.proto
}

message WebSocketPreflightDecision {
WebSocketPreflightAction action = 1;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good question.

I think we can defer this until a concrete middleware needs it. We can still add it later, if we get some signal on it being needed. Adding it now introduces more conditions into an already complex WebSocket handshake and I'm not sure how common this subprotocol negotiation is, how often does it include multiple options, etc. (tried to look for this and it seems like it's not that common).

What could go wrong today?

  • Client offers subprotocols [a, b, c].
  • Middleware only understands b.
  • Middleware sees the offered protocols during preflight, but cannot make INSPECT conditional on the final selection.
  • Upstream selects a.
  • Middleware receives session_start with a, but that event is notification-only.
  • When messages arrive, middleware may fail to parse them and either deny or trigger its configured on_error behavior.

If middleware could declare support for [b], OpenShell could narrow the upstream offer to [b]. If upstream supports b, the connection could proceed successfully.

Possible future flow

  • Client offers [a, b, c].
  • Preflight returns an optional accepted-subprotocol constraint.
  • We intersect all sets (client + all middleware's preflight) and offer that to the server.
  • One gets selected or server rejects the upgrade.

This would be an addition to the preflight decision and we could make it backward compatible (i.e. not a one-way door).

@pimlock pimlock added gator:approval-needed Gator completed review; maintainer approval needed and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:supervisor Proxy and routing-path work gator:approval-needed Gator completed review; maintainer approval needed test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants