Skip to content

fix(http): enforce bounded WebSocket request handling - #315

Draft
muharshad0311-source wants to merge 2 commits into
agentclientprotocol:mainfrom
muharshad0311-source:fix/10751-websocket-limits-v1-impl
Draft

fix(http): enforce bounded WebSocket request handling#315
muharshad0311-source wants to merge 2 commits into
agentclientprotocol:mainfrom
muharshad0311-source:fix/10751-websocket-limits-v1-impl

Conversation

@muharshad0311-source

@muharshad0311-source muharshad0311-source commented Aug 9, 2026

Copy link
Copy Markdown

Summary

  • add an opt-in WebSocketLimits builder to the ACP HTTP server
  • enforce finite frame and message limits during the Axum WebSocket upgrade
  • map typed Tungstenite MessageTooLong failures to WebSocket close code 1009
  • reject oversized single and batch JSON-RPC requests at a lower soft limit with correlated errors while keeping the socket usable
  • bound generated soft-limit responses by the configured hard message limit
  • replace payload-bearing WebSocket and protocol-actor traces with bounded metadata, and remove malformed wire text from client-visible errors

This provides a generic transport boundary for downstream servers such as aaif-goose/goose (issue #10751), where one physical WebSocket can carry multiple ACP sessions.

Behavior

  • Requests at or below the soft limit continue through the existing ACP path.
  • A valid request above the soft limit receives the same request ID with error code -32600 and structured payload_too_large size metadata.
  • Explicit JSON-RPC id: null remains distinguishable from a notification and receives a correlated null-ID error response.
  • An oversized batch is rejected as one frame and receives one batch-shaped error array containing every ID-bearing request in source order. Invalid non-response entries receive id: null; notifications and response-shaped entries do not receive pseudo-responses.
  • The rejected single request or batch is not forwarded, and another session can continue on the same WebSocket.
  • A frame or message above the hard transport limit receives close code 1009 (Message Too Big). The physical socket and all sessions on it are then cleaned up.
  • If a generated batch error cannot fit within the hard message limit, serialization stops at that limit and the socket closes with 1009 instead of allocating an unbounded response.
  • Dense invalid batches are streamed directly into the bounded response writer. Once that writer overflows, the preflight only consumes borrowed RawValue entries to validate the remaining JSON; it does not collect request IDs or expand input into an unbounded side structure.

Safety and compatibility

  • Limits are opt-in, so existing AcpHttpServer behavior remains unchanged unless the builder is called.
  • ServerOptions is unchanged, preserving downstream struct literals.
  • The soft-limit preflight borrows the Axum text buffer and streams RawValue batch entries through a narrow envelope visitor and bounded writer. It examines only JSON-RPC envelope fields and does not materialize params, prompt text, or base64 data before rejecting an oversized request.
  • WebSocket transport traces record byte counts instead of serialized JSON. Generic inbound/outgoing protocol, role, typed-handler, and session traces on this path record only event/method/type metadata rather than Dispatch, OutgoingMessage, typed requests, session updates, or params. Soft-limit logs contain only connection/category/count/size metadata; client-controlled request IDs are not logged by the exercised core or WebSocket path.
  • Standalone malformed WebSocket input retains raw text only for internal response-shape classification, preventing response-to-response errors. Client-visible parse-error error.data is removed. Typed JSON conversion failures retain only a fixed phase marker—never the original JSON or Serde error text—so structurally valid but typed-invalid prompts cannot echo prompt or base64 content into logs or JSON-RPC errors.
  • Cross-target regression tests verify that accepted prompt text, base64 content, malformed raw text, typed-invalid prompt content, typed callback response content, and text session updates do not enter core or HTTP traces. A real actor assertion covers string request IDs across all captured core and WebSocket logs. Custom failing serializers verify that conversion errors do not echo serializer text. Typed response traces record only response type metadata, while JSON-RPC responses still carry the request ID as required by the protocol.
  • Soft-limit responses use the existing WebSocket outbound mailbox, preserving already-queued output order.
  • Hard-limit detection uses the typed Tungstenite 0.29 error and does not match formatted error strings.
  • Axum is constrained to =0.8.9, and the direct Tungstenite dependency is pinned to exactly 0.29.0, so the typed downcast cannot silently split across crate identities. In the independently resolved server feature stack, the direct dependency and Axum both use Tungstenite 0.29. The separate client feature may resolve Tungstenite 0.30 through async-tungstenite; that type is not used by the Axum server error downcast.

Tests

cargo test -p agent-client-protocol-http --all-features
101 passed; 0 failed

cargo test -p agent-client-protocol --all-features
all unit, integration, and doctests passed

cargo clippy -p agent-client-protocol-http --all-features --all-targets -- -D warnings
passed

cargo clippy -p agent-client-protocol --all-features --all-targets -- -D warnings
passed

cargo fmt --all -- --check
passed

git diff --cached --check
passed

cargo package -p agent-client-protocol-http --allow-dirty --offline
packaged and verified 13 files (435.7 KiB, 70.6 KiB compressed)

cargo package -p agent-client-protocol --allow-dirty --offline
packaged and verified 81 files (1.4 MiB, 248.3 KiB compressed)

cargo tree --manifest-path <packaged-crate>/Cargo.toml --no-default-features --features server -i tungstenite@0.29.0 --offline
direct dependency and Axum both resolve to Tungstenite 0.29.0

cargo test --workspace --all-features --exclude agent-client-protocol-conductor
passed, including 101 agent-client-protocol-http tests and workspace doctests

The regression matrix covers below/exact/above-soft requests, numeric/null/string request IDs, single and mixed batches, dense invalid batch entries, notification/response filtering, malformed response-shaped input, valid and typed-invalid text-and-image prompts through a real protocol actor, a typed callback response and session update carrying sensitive content, serializer-failure redaction, same-socket continuation by another session, outbound ordering, bounded generated responses, cross-target accepted/rejected/malformed log redaction, frame and message hard limits, close 1009, malformed JSON continuation, non-capacity errors, and connection cleanup.

The unexcluded repository-wide command reaches an unchanged conductor integration test and then fails in test_nested_conductor_with_external_arrow_proxies with Incoming transport closed. The conductor dependency tree contains no agent-client-protocol-http; excluding that package makes the remainder of the workspace green. This PR does not modify conductor code.

Maintainer feedback requested

  • Is -32600 the preferred JSON-RPC error code for a syntactically valid request rejected by this transport policy?
  • Is the generic opt-in builder the preferred API surface for downstream servers?
  • Is the exact Axum 0.8.9 constraint an acceptable compatibility tradeoff for preserving typed hard-limit detection, or would maintainers prefer a stable typed boundary upstream in Axum before relaxing it?
  • Is replacing payload-bearing traces across the generic protocol actor/role/handler/session path with event, method, and type metadata acceptable as part of the transport privacy boundary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant