feat(middleware): inspect HTTP responses before return - #3070
Conversation
Remove the request write-prefix restriction and route request and response mutations through one direction-aware atomic applicator.\n\nRefs #2691 Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Add the finalized bidirectional response protocol, separate gRPC service transport, manifest binding, and ordered headers/whole-body/streaming/trailer session runner.\n\nRefs #2691 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>
|
🌿 Preview your docs: https://nvidia-preview-pr-3070.docs.buildwithfern.com/openshell |
drew
left a comment
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
This accepted-issue implementation has four blocking findings in response buffering, HTTP framing, connection lifecycle, and fail-open observability. The Fern documentation covers the new user-facing contract; the stale agent-skill guidance is a non-blocking follow-up under Gator's review calibration.
Action required: @pimlock, please address GATOR-8768b149-01 through GATOR-8768b149-04 and push an updated head for a focused follow-up review.
Blocking findings:
GATOR-8768b149-01: fail-open whole-body failure can retain an unbounded responseGATOR-8768b149-02: middleware can emit chunked framing on an HTTP/1.0 responseGATOR-8768b149-03: middleware reverses established relay connection-lifecycle outcomesGATOR-8768b149-04: fail-open response bypasses omit the required detection finding
Carried findings:
- None
Non-blocking suggestions:
- Update the mapped policy-generation, CLI, and cluster-debugging skills for
HTTP_RESPONSE/PRE_RETURNbefore release so agent guidance matches the new capability.
Gator metadata
- Validation: Project-valid implementation of accepted issue #2691
- Docs: Fern docs and architecture/RFC guidance updated; existing navigation already covers the edited pages
- Checks: Current Branch Checks and Helm Lint statuses are green; required E2E dispatch follows resolution of review blockers
- E2E:
test:e2erequired for network proxy, policy, and response-delivery behavior; not yet dispatched while code feedback is open - Head SHA:
8768b1498a8c4c16e461b1737bed50b61795dd04 - Base SHA:
8ffc6c2a13cd482dea9e2195058309344c274756 - Merge base SHA:
8ffc6c2a13cd482dea9e2195058309344c274756 - Patch ID:
97aabced6b999ee511cd1920996930217f7a2e23 - Gator payload:
7 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| let deadline = Instant::now() + MAX_MIDDLEWARE_CHAIN_TIMEOUT; | ||
| let output = self.process_units_from(0, vec![data], deadline).await?; | ||
| if self.defer_output_until_finish { | ||
| self.deferred_output.extend(output); |
There was a problem hiding this comment.
gator-agent
Critical — GATOR-8768b149-01 · A failed whole-body barrier retains an unbounded response
Summary: With a fail-open whole-body stage on a chunked, close-delimited, or streaming response, crossing the configured limit disables that stage but leaves deferred output enabled. Every later upstream unit is accumulated until EOF, so an unbounded upstream can exhaust supervisor memory despite the documented payload cap.
Fix: When the last whole-body barrier fails open, release retained original bytes and explicitly transition the relay to bounded committed streaming; cover timeout, malformed-result, stream-closure, and overflow exits.
Verify: Configure a 4 KiB fail-open whole-body stage, push 4097 bytes and then repeated 64 KiB units. After the overflow, units must be released with bounded retention instead of accumulating until finish.
Agent context
- Location:
crates/openshell-supervisor-middleware/src/response.rs:215 - Ownership: This PR introduces the whole-body barrier and deferred accumulator.
| let head = serialize_response_head( | ||
| &status_line, | ||
| &preflight.headers, | ||
| ResponseFraming::Chunked, |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-8768b149-02 · HTTP/1.0 responses receive invalid chunked framing
Summary: When response middleware selects headers-only or streaming inspection, OpenShell preserves an upstream HTTP/1.0 status line but unconditionally rewrites the response as Transfer-Encoding: chunked. HTTP/1.0 recipients do not support chunked coding, so clients can expose chunk markers, truncate the body, or wait for close-delimited completion. Whole-body responses with trailers reach the same invalid combination.
Fix: Track the applicable HTTP version and choose compatible downstream framing; never emit chunked transfer coding with an HTTP/1.0 status line.
Verify: Relay HTTP/1.0 200 OK with a five-byte body through headers-only, streaming, and trailer-producing modes. Each result must use valid HTTP/1.0 framing and deliver exactly those body bytes without chunk markers.
Agent context
- Location:
crates/openshell-supervisor-network/src/l7/rest.rs:3530 - Sibling: The trailer path selects chunked framing at
rest.rs:3671.
| } | ||
| client.flush().await.into_diagnostic()?; | ||
| Ok(Some( | ||
| if server_wants_close || matches!(body_length, BodyLength::None) { |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-8768b149-03 · Middleware reverses relay connection-lifecycle semantics
Summary: After a successful middleware response, this branch returns Consumed for every Connection: close response and every response without body framing. The baseline relay deliberately returns Reusable for explicitly framed close responses so the next upstream write observes closure, and for unframed keep-alive responses treated as empty. The new result can tear down the enclosing tunnel, reintroduce long client retry delays, and lose supported reuse.
Fix: Mirror the baseline outcome rules: consume only close-delimited or event-stream unframed bodies; preserve reuse for explicit framing and unframed keep-alive responses.
Verify: With headers-only middleware, relay an explicitly framed Connection: close response and an unframed keep-alive empty response. Both must match their no-middleware Reusable outcomes.
Agent context
- Location:
crates/openshell-supervisor-network/src/l7/rest.rs:3704 - Ownership: This PR adds the separate middleware completion calculation.
| ) -> Vec<openshell_ocsf::OcsfEvent> { | ||
| invocations | ||
| .iter() | ||
| .map(|invocation| { |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-8768b149-04 · Fail-open response bypasses lose the alert signal
Summary: A response stage that fails open after capacity exhaustion, timeout, stream closure, malformed output, or whole-body overflow emits only an HTTP activity record. Existing middleware paths and the published contract also emit a detection finding for reduced enforcement, which is the signal operators use to alert when inspection was bypassed. Successful delivery here can therefore hide degraded response controls.
Fix: Dual-emit a platform-owned detection finding for every failed FailOpen response invocation, including session-capacity exhaustion, using only validated policy, implementation, host, phase, and failure-category fields.
Verify: Exercise preflight failure, runtime timeout, whole-body overflow, and session-capacity exhaustion with fail-open stages. Each case must emit both its HTTP activity and one sanitized detection finding.
Agent context
- Location:
crates/openshell-supervisor-network/src/l7/rest.rs:3733
2 participants
Summary
Add the finalized
HTTP_RESPONSE/PRE_RETURNmiddleware contract and enforce it on HTTP/1.x responses before sandbox delivery. The implementation supports headers-only, bounded whole-body, and normalized lockstep streaming modes while preserving explicit fail-open/fail-closed delivery semantics.Related Issue
Closes #2691
Changes
HttpResponsePreReturn.EvaluateAPI, manifest binding, remote transport, and ordered response session runtime.1xxresponses, repair downstream framing, normalize trailers, bypass upgrades, and distinguish failures before and after commitment.Testing
mise run pre-commitpassesmise run testpassesmise run cipassesmise run e2epasses (Rust, 89 Python scenarios, and MCP conformance)Checklist