Skip to content

feat(supervisor): let operators require specific middleware bindings - #4678

Open
freeqaz-openai wants to merge 3 commits into
NVIDIA:mainfrom
freeqaz-openai:feat/2282-supervisor-middleware-binding-policy/freeqaz-openai
Open

freeqaz-openai wants to merge 3 commits into
NVIDIA:mainfrom
freeqaz-openai:feat/2282-supervisor-middleware-binding-policy/freeqaz-openai

Conversation

@freeqaz-openai

Copy link
Copy Markdown

Summary

Let an operator require a specific supervisor middleware binding, such as HTTP request inspection before credentials are attached. Today a service can advertise only a response hook, and OpenShell will correctly skip it on requests. That behavior is useful for response middleware, but it cannot express “this service must participate in request inspection.”

This is an opt-in registration contract. The sandbox policy must still attach the middleware and use on_error: fail_closed where a failed call should deny a request. Upgrade supervisors before requiring the policy; older supervisors do not enforce it, and an unsuccessful configuration update does not retroactively replace the last working configuration.

Related issue

Closes #2282. Draft for design review; the issue does not currently have state:accepted. Please leave the normal issue-acceptance and review checks in place.

Changes

  • dynamic (default): preserve existing manifest-driven behavior, including response-only services.
  • allowlist: every configured binding must be advertised; ignore other advertised bindings.
  • exact: the configured and advertised binding sets must match.

The gateway checks the manifest, carries the operator's policy in sandbox configuration, and the supervisor checks its own Describe response before installing the registry. No credential, sender, transport, or per-request wire protocol changes.

Testing

  • A real loopback gRPC service advertised the required request hook to the gateway, then only its response hook to the supervisor. The supervisor refused that registration. Dynamic response-only operation still passed.
  • Final source: 575 core and 127 middleware tests passed; compiled a downstream Rust consumer importing enum variants through both the existing and new public names; workspace formatting and diff checks passed.
  • Before that final Rust public-name compatibility adjustment: 169 targeted gateway, supervisor and networking tests passed (one existing full-duplex test ignored); strict Clippy, protobuf lint and compatibility against the base, generated Go binding comparison, and Go SDK tests passed.
  • Full upstream CI and an installed sandbox run are not claimed. Per-binding payload and timeout overrides are outside this patch.

Let operators require and limit the operation and phase pairs that an
external middleware may expose. Validate the same binding policy at
gateway startup and when a supervisor discovers the service, and keep
only authorized bindings in the registry used for dispatch.

Preserve dynamic discovery and the existing wire configuration when no
binding policy is set. Document that older supervisors must be upgraded
before relying on binding restrictions.

Signed-off-by: Free Wortley <freeqaz@openai.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @freeqaz-openai.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions github-actions Bot closed this Oct 17, 2026
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@mrunalp mrunalp reopened this Oct 17, 2026
@freeqaz-openai

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

@freeqaz-openai
freeqaz-openai marked this pull request as ready for review October 21, 2026 17:21
@mrunalp

mrunalp commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

/ok to test c94bc45

@mrunalp mrunalp added the test:e2e Requires end-to-end coverage label Oct 21, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for c94bc45. 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, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@mrunalp

mrunalp commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

The approach makes sense: require the inspection hook at registration, and recheck it against the supervisor’s own Describe response.

Please also update skills/generate-sandbox-policy/SKILL.md and skills/debug-openshell-cluster/references/supervisor-middleware.md. They currently tell agents to check whether the service advertises the desired hook. With this change, they also need to check whether the operator’s binding policy authorizes it.

For example, a service could advertise both request and response hooks while its allowlist enables only the response hook. Following the existing guidance could lead an agent to configure a policy expecting request inspection, but requests would skip that middleware even with
on_error: fail_closed.

@github-actions

Copy link
Copy Markdown

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. rust-sdk — No explicit risk acceptance found

raw::proto::SupervisorMiddlewareService adds binding_policy and bindings, so existing exhaustive Rust struct literals no longer compile.

  • Impact: Rust SDK consumers constructing this generated message with all previous fields must add the two new fields or use ..Default::default().
  • Action: Preserve the existing raw Rust struct in v0.1.5, or defer the change to a minor train with guidance to add both fields or ..Default::default().
  • PR: feat(supervisor): let operators require specific middleware bindings #4678
Review details
  • Execution: complete
  • Candidate: PR #4678 (ec17754b1d5670e3e81bf2e2a86b70ec41e9b273)
  • Baseline: 55bf786453395f3fe1858a187152537b457de77d (55bf786453395f3fe1858a187152537b457de77d)
  • Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

…thorizes

Signed-off-by: Free Wortley <freeqaz@openai.com>
@mrunalp

mrunalp commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 0411584

Restore the comment separator emitted by the pinned generator. This keeps the checked-in binding byte-for-byte reproducible without changing the protobuf schema or Go behavior.

Signed-off-by: Free Wortley <freeqaz@openai.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

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add operator-controlled binding policies to supervisor middleware

3 participants