Repository navigation
Conversation
Add EvaluateHttp, a streaming HTTP middleware protocol with BUFFERED and STREAM body modes, next to the released HTTP protocol 1, which keeps its engines unchanged and is deprecated for removal in 0.2.0. - One EvaluateHttp stream per HTTP message and stage, or per uninspectable connection. A preflight selects continue, inspect, or reject; late header mutations apply before the head commits. - A service implements one HTTP protocol, chosen per binding with http_protocol_version, and a protocol 2 service requires the openshell.supervisor-middleware.http-v2 capability. The gateway delivers each service's protocol to supervisors. - Protocol 2 is always fail-closed. The gateway rejects fail_open on protocol 2 services and policies whose overlapping selectors mix protocols; a mixed chain fails closed at runtime. - Protocol 2 middleware decides about uninspectable traffic (tls: skip, h2c, unsupported tunnels, raw TCP, SQL passthrough) at a preflight, and is exempt from the static tls: skip conflict rule. - Request bodies stream to the upstream unless a later step needs the whole body; responses stream with chunked framing or buffer before commit. - Migrate the content guard example to protocol 2, add a Docker e2e suite, and document the protocol. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
pimlock
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
October 9, 2026 01:23
|
🌿 Preview your docs: https://nvidia-preview-pr-4359.docs.buildwithfern.com/openshell |
pimlock
marked this pull request as draft
October 9, 2026 01:31
|
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. |
Replace the http-v2 capability and per-binding protocol fields with new binding operations, and stop delivering the protocol to supervisors. - HTTP_REQUEST_V2 and HTTP_RESPONSE_V2 select HTTP protocol 2. Released gateways and supervisors already reject unknown operation/phase pairs at Describe, so the capability, extension_metadata_with_requirements, and MiddlewareBinding.http_protocol_version are removed. - Drop supported_http_body_modes. The preflight offers every mode the message is eligible for, and the stage picks one or continues or rejects. A protocol 2 binding with a zero payload limit is offered no body modes. - Move the tls: skip conflict rule out of the shared policy validator into the gateway's safety validation, which knows each service's protocol. This removes the delivered SupervisorMiddlewareService protocol field, the process-wide protocol 2 set in openshell-policy, and the supervisor's undescribed-service tracking. An undescribed entry follows on_error, which the gateway requires to be fail_closed for protocol 2 services. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Replace EvaluateHttp with EvaluateHttpRequestV2 and EvaluateHttpResponseV2, one RPC per binding operation, as for HTTP protocol 1 and WebSocket. Both keep the shared HttpEvent and HttpResult messages, so the stage pipeline is unchanged; OpenShell picks the RPC from the preflight subject. Traffic OpenShell cannot inspect stays on the request RPC. Only services that bind HTTP_REQUEST_V2 decide about it, so only they are exempt from the gateway's tls: skip conflict rule; a response-only protocol 2 service keeps the rule and its fail-closed entry denies such traffic at runtime. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Rename "HTTP protocol 1/2" to v1 and v2 HTTP hooks across code, proto comments, docs, skills, and the example. The docs page moves to extensibility/supervisor-middleware/http-hooks-v2, the protocol2 modules become http_v2, HttpProtocol becomes HttpHookVersion, and the mixed-chain reason becomes middleware_hook_versions_mixed. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
This was referenced Oct 9, 2026
Rename the streaming HTTP hook RPCs to EvaluateHttpRequestSession and EvaluateHttpResponseSession, and drop the unreleased HTTP_REQUEST_V2 and HTTP_RESPONSE_V2 operations. A service keeps its HTTP_REQUEST and HTTP_RESPONSE bindings and selects HTTP session hooks for all of them by listing openshell.supervisor-middleware.http-session in its extension required_capabilities. Released gateways and supervisors already reject unmet required capabilities at Describe, so they never call such a service through the v1 RPCs. Registration rejects a manifest that supports the capability without requiring it. Mark the v1 HTTP hook RPCs, the HttpResponsePreReturn service, and the types only they use deprecated in the protobuf contract. Rename the docs page, module, and e2e suite to HTTP session hooks, and update RFC 0009 so capability-gated contracts and removing deprecated elements in a minor release match the release stability policy. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Collaborator
Author
|
/ok to test 7f09efe |
pimlock
marked this pull request as ready for review
October 10, 2026 01:45
pimlock
commented
Oct 10, 2026
pimlock
left a comment
Collaborator
Author
There was a problem hiding this comment.
gator-agent
PR Review Status
The initial code review found three blocking issues in the new HTTP session pipeline: response completion before the terminal verdict, response delivery after policy revocation, and incomplete headers at body-stage begin. The inline comments describe the fixes and deterministic regression cases.
Action required: @pimlock, address the three inline findings and push an updated head for a focused follow-up review.
Blocking findings:
GATOR-7f09efe2-01: Preserve detectable failure until the response middleware accepts the terminal result.GATOR-7f09efe2-02: Check the current policy generation before every response write.GATOR-7f09efe2-03: Include all preflight mutations in each body stage's begin head.
Carried findings: None.
Non-blocking suggestions:
- Split size-expanding content-guard scanner output into chunks within the negotiated limit, including the final tail. A 64 KiB chunk of
aexpanded to[REDACTED]currently exceeds the output chunk limit and fails closed.
Gator metadata
- Validation: Focused implementation of the streaming middleware work in #2431 and #3307; repository-admin author and no duplicate identified.
- Docs: Fern HTTP Session Hooks page, migration guidance, related pages, and navigation updated.
- Checks: Current-head Branch Checks, Helm Lint, Trivy Changes, and DCO pass; no merge conflict.
- E2E:
test:e2eis required for proxy, policy, and credential behavior. The existing E2E jobs were skipped without the label; runtime test dispatch remains after review fixes. GPU and Windows labels are not indicated by this patch. - Head SHA:
7f09efe20318880bd2d884c4aadb4a56f902d48d - Base SHA:
0ccc6b9a053fdaf47700e91c1b8ed858f2afed4f - Merge base SHA:
0ccc6b9a053fdaf47700e91c1b8ed858f2afed4f - Patch ID:
611a1c2a9287a1c7583b112c37822874f207e666 - Gator payload:
11 - Review mode:
initial - Previous reviewed SHA:
none - Review budget exhausted:
no - Maintainer decision required:
no - Next state:
gator:in-review - Validation method: Static code review; no local tests run.
A body stage captured the head right after its own preflight, so its begin event lacked the preflight mutations of later stages even though the delivered head carried them. Keep the final preflight head on the pipeline and begin every stage on it plus earlier stages' late mutations, as the HttpBegin contract states. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
A STREAM response stage can keep emitting output after it consumed the upstream body, and the session response writer checked the policy generation only when it committed the head. Output and the terminating chunk then reached the client under revoked authorization, before the final check. Check the generation before every write instead, and treat the response as committed only once a byte was written. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
With a declared output length, a STREAM stage could emit every byte and then reject at input_end. The relay had already written a complete Content-Length message, so the sandbox saw a successful response, and on uploads the upstream received a complete request it could act on or answer. Hold back what would complete the message until every stage returns its terminal result: the final byte of a declared length, or the whole head of a declared empty body. The chunked terminator already waited. An empty response body that is rejected still gets the canonical 403. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
…imit Redacting a term shorter than the marker makes STREAM output longer than its input, so one output chunk could exceed the max_chunk_bytes the preflight offered and fail the message closed. Split every output chunk within that limit. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Collaborator
Author
|
Addressed the gator review of 7f09efe. The new head is 1a4a2bb, with one commit per finding:
Each regression test fails on 7f09efe and passes now. Verification:
|
|
Label |
pimlock
commented
Oct 10, 2026
pimlock
left a comment
Collaborator
Author
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @pimlock. I checked your fixes for terminal completion, policy revocation, and the body-stage Begin head, including the request-upload completion path and the new regression cases. All three findings are fixed, and their Gator threads are resolved. The focused follow-up also checked the content-guard chunk splitting and found no new blocking issues.
Action required: a maintainer must open the current-head Branch E2E Checks run and select Re-run all jobs now that test:e2e is applied. Its runtime jobs were skipped before the label was set; Gator cannot advance to pipeline monitoring until they actually start.
Blocking findings: None.
Carried findings: None; GATOR-7f09efe2-01, GATOR-7f09efe2-02, and GATOR-7f09efe2-03 are fixed.
Gator metadata
- Validation: Streaming supervisor middleware work in #2431 and #3307; project validity established in the initial review.
- Docs: Fern delivery guarantees and preflight-head ordering match the fixes; existing navigation remains appropriate.
- Checks: Branch Checks are running; Helm Lint and Trivy Changes pass, DCO passes, and the PR has no merge conflict. Maintainer approval is still missing.
- E2E:
test:e2eapplied; current-head runtime jobs were skipped and require Re-run all jobs. GPU and Windows labels are not indicated. - Head SHA:
1a4a2bb694bc60b0bcbc478057ef67594de5f540 - Base SHA:
0ccc6b9a053fdaf47700e91c1b8ed858f2afed4f - Merge base SHA:
0ccc6b9a053fdaf47700e91c1b8ed858f2afed4f - Patch ID:
128cfed5631373ce935945cbce6c4849e2591b9f - Gator payload:
11 - Review mode:
follow_up - Previous reviewed SHA:
7f09efe20318880bd2d884c4aadb4a56f902d48d - Review budget exhausted:
no - Maintainer decision required:
no - Next state:
gator:blocked - Blocked reason:
test_dispatch_required - Validation method: Static independent review; no local tests run. Author-reported passing tests were considered alongside inspected regression code.
10 of 12 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduce HTTP session hooks, streaming HTTP request and response hooks that replace the v1 HTTP hooks. The v1 hooks were too limited: they could only inspect requests whose body fit in a buffer within our payload limit. The session hooks stream the body, so middleware can inspect messages of any size.
EvaluateHttpRequestSessionandEvaluateHttpResponseSession, with one stream per HTTP message. A preflight decides whether to continue, reject, or inspect the body, either buffered or streamed. The request preflight also lets middleware allow or refuse traffic OpenShell cannot inspect, such astls: skiptunnels.HTTP_REQUESTandHTTP_RESPONSEbindings and listsopenshell.supervisor-middleware.http-sessionin its extensionrequired_capabilities. Gateways and supervisors have rejected unmet required capabilities atDescribesince v0.1.0, so older peers refuse the service instead of calling it through the v1 RPCs. No RPC, message, or enum value carries a version.fail_open. The session hooks are always fail-closed, which simplifies the UX, the API, and the code.EvaluateHttpRequest,HttpResponsePreReturn, and the types only they use are markeddeprecatedin the protobuf contract and are up for removal in the next version bump (0.2.0). Migration notes are in the new docs page. Both hook versions work side by side until then.Tip
See interactive walk-through of main parts of this PR.
Related Issue
Part of #2431 and #3307.
Changes
openshell.supervisor-middleware.http-sessioncapability. Registration rejects a manifest that lists it as supported but not required, because older peers would accept that service and call v1 HTTP hooks.extension_protocol::http_session_middleware_metadatabuilds the metadata.on_error: fail_openfor session hook services, exempts session hook request middleware from thetls: skipconflict rule, and rejects policies whose overlapping selectors would mix hook versions in one chain.Testing
mise run pre-commitpassesmise run e2e:middleware-http-sessionsuitebuf breakingagainst v0.1.2 reports no breaking changeChecklist