Skip to content

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

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

feat(middleware): inspect WebSocket text messages#2477
pimlock wants to merge 26 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>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock

pimlock commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

gator-agent

Blocked

Head SHA: f88bdc0fa92979cf3bc267bc454b37493fbde299

Gator is blocked because this pull request remains a draft, so the independent code review must wait until it is ready for review. The prior reviewer authentication failure did not produce a valid review disposition, and the earlier draft-blocker disposition was for a different head.

DCO is passing. The required Branch Checks, Helm Lint, and E2E gates are pending. The existing test:e2e label was already present; gator did not apply or change it in this cycle.

Next action: mark the pull request ready for review. Gator will then run one fresh independent review for the current head. I did not approve, merge, push, apply test:e2e, or post /ok to test.

pimlock added 6 commits July 28, 2026 16:21
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
…e-introspection/pimlock

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 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test 63d86ff

@pimlock
pimlock marked this pull request as ready for review July 29, 2026 22:03

@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 for forward WebSocket text-message middleware.
Head SHA: 63d86ffd14168b94b7f3c2c77f65b2eefd021514

Thanks @pimlock. I checked the current non-draft head after your updates addressing the earlier plaintext upgrade, admission, lifecycle, policy-reload, regex, token-grant ordering, denial telemetry, docs, and coverage findings. Those earlier fixes are present, but the fresh independent review found two high-severity resource-boundary gaps and two medium-severity lifecycle/compatibility issues; all four are attached inline.

Review findings:

  • High: the persistent-session limit resets with each replaceable middleware registry generation.
  • High: parsed text assembly is aggregate-bounded only when an active middleware session supplies admission.
  • Medium: disabling a stream drops it without the documented best-effort session-end event.
  • Medium: the gateway TOML field rename rejects existing max_body_bytes configurations.

Docs: The relevant Fern pages and mapped skills were updated, and no navigation change is needed. The statement in architecture/sandbox-limits.md that every parsed text assembly acquires shared work before buffering must be corrected with the resource-boundary fix.

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-middleware/src/lib.rs
Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs
Comment thread crates/openshell-supervisor-middleware/src/websocket.rs Outdated
Comment thread crates/openshell-server/src/config_file.rs
@pimlock pimlock added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Jul 29, 2026
@pimlock
pimlock marked this pull request as draft July 29, 2026 22:30
@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 29, 2026
pimlock added 2 commits July 29, 2026 17:24
…e-introspection/pimlock

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock
pimlock marked this pull request as ready for review July 30, 2026 00:47

@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 for forward WebSocket text-message middleware.
Head SHA: b1ee2ddf5a3caed72ea1bf5752b801054390eab9

Thanks @pimlock. I checked your current-head updates and proposed disposition. The process-lifetime admission preservation, legacy max_body_bytes alias, and bounded best-effort session-end fix are present and coherent. The architecture document now discloses the aggregate assembly gap, but the independent review does not consider deferral safe because untrusted sandbox traffic can still consume unbounded aggregate supervisor memory and impair the sandbox security control.

Review findings:

  • High: policy reloads can race payload assembly or middleware evaluation and allow an old-generation message to reach credentials or upstream.
  • High: parsed text assembly remains aggregate-unbounded whenever no active middleware session supplies admission.
  • Medium: plaintext ws:// preflight occurs only after the upstream TCP connection is established.
  • Medium: pre-upgrade session error paths can report CANCELLATION instead of the typed terminal cause.
  • Medium: HTTP-only attachments can be rejected by WebSocket preflight envelope validation before binding selection.
  • Low: the Fern middleware page overstates aggregate admission coverage.

All six findings are attached inline.

Docs: The relevant Fern pages are present and already navigated. The shared-capacity statement needs correction, and the before-upstream-contact contract must match the plaintext implementation.

Next state: gator:in-review pending author changes. No local tests were run as part of this code-only review. I did not approve, merge, push, apply test:e2e, or post /ok to test.

Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs
Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs
Comment thread crates/openshell-supervisor-network/src/proxy.rs
Comment thread crates/openshell-supervisor-network/src/l7/relay.rs
Comment thread crates/openshell-supervisor-middleware/src/websocket.rs Outdated
Comment thread docs/extensibility/supervisor-middleware.mdx Outdated
@pimlock pimlock added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Jul 30, 2026
pimlock added 2 commits July 29, 2026 19:48
…e-introspection/pimlock

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

@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 for forward WebSocket text-message middleware.
Head SHA: 6055dc5ce9d7c694f70b29b3ed14fd1bc6e104ea

Thanks @pimlock. I checked your current-head updates covering the process-wide assembly budget, typed pre-upgrade finalization, published-doc distinction, plaintext preflight ordering, binding selection, and generation rechecks. The independent review found those six earlier concerns resolved and found no blocking code or security defect.

Review findings:

  • Blocking CI: proxy::tests::plaintext_websocket_preflight_denial_does_not_connect_upstream times out with Elapsed(()) at crates/openshell-supervisor-network/src/proxy.rs:5714 in both amd64 and arm64 Rust jobs. The required Branch Checks are therefore failing and need a code/test fix or a demonstrated infrastructure disposition.
  • Low docs gap: the published Fern page does not state the process-wide assembly limits, reload scope, timeout/fragment bounds, or 1013 overload behavior. The actionable detail is attached inline.

Docs: The relevant Fern pages are present and already navigated, but this operator-visible capacity and close-code behavior remains incomplete.

Next state: gator:in-review pending the CI failure and documentation follow-up. Core E2E is also currently failing in two Podman-rootless jobs. No local tests were run as part of this code-only review. I did not approve, merge, push, apply test:e2e, or post /ok to test.

Comment thread docs/extensibility/supervisor-middleware.mdx Outdated
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

@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 for forward WebSocket text-message middleware.
Head SHA: 2a6fd919be84afe8929264253a391bc4800ae49f

Thanks @pimlock. I checked the current non-draft head after the Linux handler-setup adjustment for the plaintext preflight regression. The test-only change preserves the no-upstream-connect assertion, and the required Branch Checks now pass. The fresh independent review found one high-severity resource-boundary defect, two medium-severity reload close-path defects, and two low-severity published-doc inaccuracies; all five are attached inline.

Review findings:

  • High: unbounded upstream writes can retain every process-wide WebSocket assembly permit indefinitely.
  • Medium: a policy reload after the upstream 101 was forwarded can produce abrupt EOF instead of a typed 1012 close.
  • Medium: a generation check between data-frame header and payload writes can corrupt the subsequent close frame.
  • Low: published Fern docs still omit the assembly budget, fragment/timeout limits, 1013 exhaustion behavior, and process-lifetime/reload scope.
  • Low: published ValidateConfig timeout semantics do not match the implementation.

Docs: The relevant Fern pages and mapped skills were updated, and navigation needs no change, but the two attached documentation issues remain.

Next state: gator:in-review pending author changes. DCO, Branch Checks, and Helm Lint are passing; the required E2E gate is pending. No local tests were run as part of this code-only review. I did not approve, merge, push, apply test:e2e, or post /ok to test.

Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs Outdated
Comment thread crates/openshell-supervisor-network/src/l7/relay.rs
Comment thread crates/openshell-supervisor-network/src/l7/websocket.rs Outdated
Comment thread docs/extensibility/supervisor-middleware.mdx Outdated
Comment thread docs/extensibility/supervisor-middleware.mdx Outdated
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

@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 for forward WebSocket text-message middleware.
Head SHA: 4242919a2918b140b56cad1104feec2c01c59867

Thanks @pimlock. I checked the current-head hardening update for bounded upstream forwarding, typed reload closure after 101, frame-boundary generation checks, published limits, and ValidateConfig timeout semantics. All five prior findings are resolved. The fresh independent review found two remaining protocol-correctness issues; one line-specific finding is attached inline.

Review findings:

  • Medium: parsed middleware sessions can preserve WebSocket extensions that the parsed relay cannot handle. Extension negotiation currently depends only on the L7 endpoint configuration, but a retained middleware session independently forces the parsed relay on REST endpoints without credential rewriting and on plaintext forward-proxy routes without WebSocket L7 configuration. Those paths preserve permessage-deflate while configuring the parsed relay with no decompression, so valid RSV1 text frames are rejected with 1002. Make safe extension negotiation depend on a retained inspecting middleware session and add ws:// and wss:// compressed-message relay coverage.
  • Medium: a stage that returns SKIP is immediately ended as CANCELLATION before the aggregate concurrent-preflight result is known. If another stage denies or fails closed, the skipped stream receives the wrong terminal reason. The actionable detail is attached inline.

Docs: The relevant Fern pages now cover assembly and session limits, fragment and timeout bounds, 1013, process-lifetime reload scope, and the correct ValidateConfig timeout. Navigation already includes the extensibility folder.

Next state: gator:in-review pending author changes. Required Branch Checks and E2E are still pending. No local tests were run as part of this code-only review. I did not approve, merge, push, apply test:e2e, or post /ok to test.

Comment thread crates/openshell-supervisor-middleware/src/websocket.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant