Skip to content

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

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

feat(middleware): inspect WebSocket text messages#2477
pimlock wants to merge 24 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

/ok to test ce9d4cb

@pimlock pimlock added the test:e2e Requires end-to-end coverage label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for ce9d4cb. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

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
))
.into());
}
let admission = if let Some(session) = options.middleware_session.as_mut() {

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

High (CWE-770): Assembly admission is acquired only when a middleware session exists, but parsed WebSocket text is also buffered for native policy, credential rewriting, compression, and sessions whose fail-open middleware stages were disabled. An attacker can open many such connections and pin up to 4 MiB per in-progress message for the assembly timeout, leaving aggregate supervisor memory unbounded. Acquire a process-wide assembly permit before buffering every parsed text message, independent of middleware presence, and hold it through inspection/rewrite. Cover non-middleware and post-disable parsed relays in the saturation tests.

@pimlock pimlock Jul 30, 2026

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.

Proposed disposition: defer aggregate admission for every parsed WebSocket text assembly to a focused hardening follow-up rather than expand PR1 further.

The current per-connection protections remain: a 4 MiB complete and decompressed message ceiling, 4,096 fragment ceiling, 30 second assembly idle timeout, and 2 minute absolute assembly timeout. Active middleware sessions also acquire shared work admission before buffering. Commit b1ee2ddf corrects architecture/sandbox-limits.md so it no longer claims that every parsed relay path has aggregate admission.

The rationale is:

  • The aggregate gap predates this PR. The base implementation already buffered up to 1 MiB per parsed connection without process-wide assembly admission. This PR raises the shared payload ceiling to 4 MiB but also adds the idle and total assembly deadlines.
  • The intended deployment is primarily one agent process or tightly related process tree per sandbox. Exhaustion by a rogue client is therefore mainly sandbox self-denial of service: it can terminate its own supervisor and workload, rather than violating the functional correctness of another sandbox.
  • There can still be host pressure if the runtime does not enforce sandbox memory, process, socket, and file-descriptor limits. Those runtime and node boundaries remain the multi-sandbox protection layer.
  • External middleware must enforce its own tenant and global limits regardless of any per-sandbox cap. Even a 32-session sandbox limit does not protect a service receiving traffic from many thousands of sandboxes.
  • A complete fix belongs to the network supervisor, independent of middleware presence and registry generation. It needs an explicit choice between concurrent-assembly and byte-weighted admission, bounded wait versus immediate shedding, terminal close behavior, telemetry, operator narrowing, and fairness. Coverage must include native policy, credential rewriting, compression, and post-disable fail-open relays.

Given the current single-workload sandbox model and existing per-message and time bounds, I consider this defense-in-depth hardening rather than a prerequisite for the forward text middleware contract. If sandbox runtime resource limits are not reliably enforced, this should be promoted because it then becomes indirect host protection rather than only sandbox self-denial of service.

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.

miette!("websocket middleware message missing work admission"),
)
})?;
let outcome = session

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

High (CWE-367): Generation is checked only after reading the frame header. A reload while admission, payload assembly, or this middleware RPC is awaiting can still let the old-generation message undergo credential resolution and reach upstream. Recheck the guard after admission/assembly and after middleware evaluation, immediately before credential resolution or upstream writes; terminate with close 1012 and POLICY_RELOAD. Add regressions that reload during a partial single-frame payload and a delayed middleware response, asserting no stale message or credential reaches upstream.

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.

Fixed in 6055dc5. The parsed relay now revalidates generation after assembly admission, middleware admission, every payload assembly, native inspection, middleware evaluation, credential rewriting, and between frame-header and payload writes. Upgrade handoff also checks generation before and after session start. Regression coverage includes reload during a partial single-frame payload, fragmented input, and a delayed middleware response carrying a credential placeholder.

))
.into());
}
let admission = if let Some(session) = options.middleware_session.as_mut() {

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

High (CWE-770 / OWASP LLM06): Aggregate admission is conditional on an active middleware session. Native-policy, credential-rewrite, compression-only, no-middleware, and fully disabled fail-open relays can each buffer 4 MiB across an unbounded number of concurrent connections. Per-message, fragment, and timeout bounds do not provide an aggregate memory bound. Add a process-wide network-supervisor assembly budget, preserved across reloads, acquired before payload buffering with bounded wait/load shedding. Test many concurrent incomplete messages without active middleware. I do not consider deferral safe: the sandbox workload is untrusted, and cgroups limit host impact but do not preserve availability of the sandbox security control.

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.

Implemented in 6055dc5. This is a network-owned process-lifetime budget, independent of middleware: 32 active text assemblies plus 64 unbuffered waiters, shared by old and new policy generations and used by every parsed relay. Admission happens before payload allocation/read and the permit is retained through decompression, inspection, middleware, rewriting, and forwarding. Queue exhaustion closes with 1013 and emits payload-free telemetry. Tests cover saturation without middleware, bounded waiting/load shedding, recovery, and reload sharing.

};
}

let mut middleware_session = if let Some(chain) = websocket_chain.as_deref() {

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

Medium (CWE-863): Plaintext ws:// middleware preflight occurs after TcpStream::connect at line 4848. A preflight DENY therefore remains externally observable by the destination, contrary to the protobuf contract that preflight occurs before upstream contact. Move the direct dial after HTTP admission and WebSocket preflight. Add a listener regression asserting that a denied upgrade causes no upstream accept.

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.

Fixed in 6055dc5. The plaintext forward path now dials upstream only after authorization, HTTP middleware, WebSocket preflight, token-grant resolution, credential rewriting, and generation validation, then validates generation again after the awaited connect. A Linux handler regression verifies that a denied WebSocket preflight never reaches the upstream listener.

.await?;
return Ok(());
}
preflight.session

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

Medium: After retaining the preflight session, later guarded relay/setup operations use ? or early returns. Those paths drop WebSocketSession, whose Drop implementation reports CANCELLATION; a generation change should report POLICY_RELOAD, while upstream rejection should use its typed reason. Introduce reason-aware finalization around the pre-upgrade session. Cover reload after successful preflight but before the upgrade write. The plaintext path around proxy.rs:5070 has the same classification problem.

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.

Fixed in 6055dc5. Pre-upgrade sessions now use reason-aware finalization: stale generations report POLICY_RELOAD, ordinary relay/connect rejection reports UPSTREAM_REJECTED, and local setup cancellation is explicit. Generation is checked around session start as well as before the upgrade handoff. Regression coverage verifies that reload after successful preflight ends the opened stream with POLICY_RELOAD rather than the Drop fallback.

if entries.is_empty() {
return Ok(empty_preflight_result());
}
validate_preflight_input(&input)?;

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

Medium: Preflight input limits are validated before operation binding selection. Consequently, a valid upgrade with more than 32 offered subprotocols is rejected with 502 even when every matched attachment is HTTP-only and should produce binding_not_selected coverage while allowing the connection. Describe/select the WebSocket chain first and validate the preflight envelope only when a WebSocket stage participates. Add an HTTP-only attachment regression with 33 valid subprotocols.

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.

Fixed in 6055dc5. WebSocket bindings are now described and selected before the preflight envelope is validated. If no WebSocket stage participates, OpenShell returns coverage-only success without applying limits to an envelope it will never send. The regression uses an HTTP-only fail-closed attachment with 33 requested subprotocols and verifies allowed/no-session binding_not_selected coverage.

3. Complete client-to-upstream text messages in sequence order. OpenShell reassembles fragmented messages and decompresses negotiated `permessage-deflate` messages before evaluation.
4. A best-effort session-end event when the stage stream remains writable. OpenShell attempts at most one terminal event for each opened stream, including streams opened during a preflight that rejects the upgrade before session start.

Allowed replacements are re-framed, re-compressed when required, and forwarded. Binary messages, control frames, and upstream-to-client traffic remain uninspected. Binary messages pass through under both `on_error` modes. For each active selected stage, OpenShell emits an informational `unsupported_message_type` coverage event and advances the session-global sequence; the next text message can therefore reach the stage with a valid sequence gap. OpenShell reserves shared middleware capacity before buffering HTTP bodies or WebSocket text messages; at most 32 evaluations run and 64 additional unbuffered callers wait for capacity. When both bounds are full, OpenShell sheds an HTTP request with `503 Service Unavailable` before reading its body. Persistent middleware streams use a separate process-wide budget of 32 sessions. WebSocket session admission does not wait: if the budget is full, OpenShell applies each selected config's `on_error` behavior before opening a stream.

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

Low: This says shared middleware capacity is reserved before buffering WebSocket text messages generally. The implementation only does so for active middleware sessions; architecture/sandbox-limits.md correctly documents the uncovered parsed paths. Qualify this statement or implement aggregate admission for every parsed text assembly.

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.

Fixed in 6055dc5. The documentation now distinguishes the network-owned assembly budget used by every parsed WebSocket text message from the additional middleware work budget used only by active middleware sessions. The sandbox limits inventory also documents the 32 active/64 waiter assembly bound, 1013 load shedding, reload-stable ownership, and payload-free exhaustion telemetry.

@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.

3. Complete client-to-upstream text messages in sequence order. OpenShell reassembles fragmented messages and decompresses negotiated `permessage-deflate` messages before evaluation.
4. A best-effort session-end event when the stage stream remains writable. OpenShell attempts at most one terminal event for each opened stream, including streams opened during a preflight that rejects the upgrade before session start.

Allowed replacements are re-framed, re-compressed when required, and forwarded. Binary messages, control frames, and upstream-to-client traffic remain uninspected. Binary messages pass through under both `on_error` modes. For each active selected stage, OpenShell emits an informational `unsupported_message_type` coverage event and advances the session-global sequence; the next text message can therefore reach the stage with a valid sequence gap. The network supervisor reserves process-wide assembly capacity before buffering every parsed WebSocket text message. Active middleware sessions additionally reserve shared middleware capacity before buffering WebSocket text, and HTTP middleware reserves the same capacity before buffering request bodies; at most 32 evaluations run and 64 additional unbuffered callers wait for capacity. When both middleware bounds are full, OpenShell sheds an HTTP request with `503 Service Unavailable` before reading its body. Persistent middleware streams use a separate process-wide budget of 32 sessions. WebSocket session admission does not wait: if the budget is full, OpenShell applies each selected config's `on_error` behavior before opening a stream.

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

Low — operator documentation: The published docs distinguish the network-owned assembly budget from middleware admission, but omit its operator-visible limits and failure behavior. The implementation permits 32 active assemblies and 64 unbuffered waiters, closes overflow with 1013, limits messages to 4,096 fragments with 30-second idle and 120-second total deadlines, and preserves the budget across reloads. Those details currently appear only under architecture/, while the Fern close-code list also omits 1013. Please document them here or link to a published Fern reference.

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