Skip to content

refactor(middleware)!: adopt two-mode HTTP body protocol - #3450

Open
pimlock wants to merge 8 commits into
mainfrom
feat/2431-streaming-request-middleware/pimlock
Open

pimlock wants to merge 8 commits into
mainfrom
feat/2431-streaming-request-middleware/pimlock

Conversation

@pimlock

@pimlock pimlock commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Replace the WIP HTTP middleware body protocol with the approved two-mode, fail-closed contract. Request middleware now selects bounded in-memory BUFFERED processing or independent duplex STREAM processing, while response middleware initially offers BUFFERED only. A binding can also advertise no body modes for preflight-only header mutation or denial; on Continue, the body and framing pass through that stage unchanged. OpenShell does not retain STREAM recovery copies or spool middleware bodies to disk.

This is the base protocol/runtime PR in a three-PR stack:

  1. This PR: HTTP middleware hook, protocol, runtime, content-guard migration, and documentation.
  2. Follow-up stacked on this PR: move AWS SigV4 into a trusted built-in middleware.
  3. Follow-up stacked on the SigV4 PR: add Git signing as a separate built-in middleware.

The existing inline proxy SigV4 implementation remains in this base PR. Git signing is intentionally excluded.

Related Issue

Part of #2431

Closes #3307

Follow-up to #2426 and #3074.

Changes

  • Add phase-specific EvaluateHttp(stream HttpEvent) returns (stream HttpResult) services with explicit HTTP protocol-version and body-mode capability negotiation.
  • Reduce HTTP body handling to BUFFERED and STREAM; remove ownership acknowledgements, sequence/ACK/replay state, skip semantics, and proxy-managed middleware spooling.
  • Allow an empty HTTP body-mode capability list for preflight-only middleware that mutates headers or denies a request/response without receiving body events; such bindings may advertise a zero payload limit.
  • Run STREAM input and output as independent bounded pumps so middleware can emit early output or delay OutputStart until it has consumed the full request.
  • Keep BUFFERED bodies bounded in RAM with explicit unchanged/replacement results.
  • Make HTTP middleware failures fail closed, including over-capacity bodies and external transport/protocol errors; retain WebSocket-only fail-open behavior.
  • Preserve protected-header handling, body-aware protocol inspection, response eligibility rules, trailer/framing validation, cancellation, deadlines, and policy-generation checks.
  • Offer BUFFERED-only response processing in this rollout and apply the response whole-body deadline across both collection and middleware completion.
  • Remove the unused public POST_CREDENTIALS phase and reserve its enum value for wire safety.
  • Migrate the content-guard example and update architecture, RFC, published documentation, gateway-config reference, and related public skills.

Review hardening in 284a8965f:

  • Restore one aggregate 30-second deadline for each HTTP middleware chain operation while retaining the separate request-body lifetime.
  • Bound complete-body collection from STREAM middleware and normalize output chunks for the next stage's advertised unit limit.
  • Enforce each response stage's input cap after earlier body replacements.
  • Preserve normalized denial diagnostics, findings, metadata, prior-stage reports, and denial/block invocations on failures.
  • Reliably send and drain terminal SessionEnd events, including NORMAL, MIDDLEWARE_DENIAL, MIDDLEWARE_FAILURE, and cancellation paths.
  • Cancel failed relay pumps promptly instead of retaining channel endpoints until the request-body deadline.
  • Return canonical HTTP failures for pre-commit STREAM denial and incompatible SigV4/body-middleware combinations instead of closing the client connection without a response.
  • Honor STREAM output_body_bytes with Content-Length framing and keep HTTP/1.0 requests on a fully collected path.
  • Make the content-guard example continue without body inspection when BUFFERED mode is unavailable and cap its requested size to the platform offer.
  • Correct protocol, RFC, architecture, user documentation, and dependency-placement drift found during review.

The pre-existing SigV4 behavior for bodyless requests without x-amz-content-sha256 remains intentionally deferred to the stacked SigV4 middleware PR, where signing behavior is owned.

Design artifacts

Testing

  • mise run pre-commit
  • mise run test
  • mise run ci
  • cargo test -p openshell-supervisor-middleware (34 passed)
  • cargo test -p openshell-supervisor-network (1,326 passed, 2 ignored; LocalStack tests remain opt-in)
  • Content-guard example suite (17 passed)
  • Focused live tests for delayed whole-body STREAM framing and terminal delivery, pre-OutputStart denial, and SigV4/body-middleware rejection before reading the request body
  • mise run e2e exercised the relevant proxy paths; its aggregate run later hit shared Docker runtime contention in provider_readiness, which passed when rerun in isolation with OPENSHELL_E2E_DOCKER_TEST=provider_readiness mise run e2e:rust

One initial mise run test attempt hit the existing timing-sensitive plaintext MCP proxy test under maximum parallel load. The exact test passed immediately in isolation and then passed in the clean full mise run test and mise run ci reruns.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture and user-facing documentation updated
  • Public protobuf compatibility and mixed-version failure behavior reviewed
  • Related skills reviewed and synchronized

Replace the unary HTTP request evaluator with a bidirectional streaming protocol and move SigV4 signing into the built-in middleware stage.

BREAKING CHANGE: replace the unary HTTP request middleware RPC with the streaming HttpRequestPreCredentials Evaluate contract.

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

Copy link
Copy Markdown

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In auto mode, a bodyless request with no x-amz-content-sha256 falls through to UNSIGNED-PAYLOAD. That's not the normal SigV4 empty-body hash and is service-specific, so a GET/HEAD to a non-S3 AWS service can be re-signed with payload semantics the service rejects. Could BodyFraming::None sign Bytes(&[]) unless the client explicitly requested an unsigned payload, and add a non-S3 empty-body case?

@pimlock
pimlock marked this pull request as draft September 18, 2026 16:48
@copy-pr-bot

copy-pr-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock pimlock changed the title feat(middleware)!: add streaming request hooks refactor(middleware)!: adopt two-mode HTTP body protocol Sep 19, 2026
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
@pimlock

This comment has been minimized.

@pimlock
pimlock marked this pull request as ready for review September 19, 2026 06:51
pimlock

This comment was marked as outdated.

@pimlock pimlock added the gator:in-review Gator is reviewing or awaiting PR review feedback label Sep 21, 2026
…equest-middleware/pimlock

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

This comment was marked as 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

Thanks @pimlock. I checked the four fixes claimed for 284a8965f against the author-only delta. The patch materially improves each area, but each original runtime-contract obligation still has a reachable residual case, so none of the four Gator threads can be resolved yet.

Action required: A maintainer must decide whether these four residual obligations remain required or explicitly waive the identified behavior. If they remain required, @pimlock should address the cases below before another review.

Blocking findings:

  • No new blocking findings

Carried findings:

  • GATOR-96ff9454-01: the complete-body compatibility loop still creates a fresh chain deadline for each entry instead of sharing one aggregate 30-second deadline.
  • GATOR-96ff9454-02: complete-body STREAM collection now enforces the cumulative cap, but preflight still advertises max_total_output_bytes as unbounded.
  • GATOR-96ff9454-03: malformed response-stage diagnostics, trailer mutations, and similar post-exchange validation failures can still discard earlier reports and the failing invocation.
  • GATOR-96ff9454-04: response-body failure exits still fall through Drop and report Cancellation rather than MiddlewareFailure; terminal send and drain also use separate timeout budgets.
Gator metadata
  • Validation: Maintainer-authored work implementing linked issue #3307 and the approved middleware protocol direction
  • Docs: Fern middleware and gateway documentation are updated
  • Checks: Current-head branch checks are still running; Helm, Trivy, E2E, and GPU E2E required statuses are green
  • E2E: Required for middleware and network-proxy behavior; current gate statuses are green, but review blockers remain
  • Head SHA: 284a8965f6ffdcc07d4f2bf1f0747eeb22394649
  • Base SHA: 484f0768fc6a0d93e0a2be295c1679aed24e18a9
  • Merge base SHA: 484f0768fc6a0d93e0a2be295c1679aed24e18a9
  • Patch ID: 910d88d908d4120d9825f7e0ce478bd42b0911f1
  • Gator payload: 9
  • Review mode: follow_up
  • Previous reviewed SHA: 4b19df7873dfa258f332fa08dea2838041321a5d
  • Review budget exhausted: yes — this is the third finding-bearing round
  • Maintainer decision required: yes — four concrete carried obligations remain unresolved
  • Next state: gator:blocked
  • Blocked reason: review_convergence_decision_required

@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 Sep 21, 2026
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>

This branch has not been deployed

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

Labels

gator:blocked Gator is blocked by process or repository gates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(middleware)!: replace unary HTTP request evaluation with a streaming hook

2 participants