Skip to content

Do not close open connections blindly on policy update: assess whethe… - #5359

Closed
mr-miles wants to merge 1 commit into
NVIDIA:mainfrom
mr-miles:feat/4945-policy-change-closes-everything
Closed

mr-miles wants to merge 1 commit into
NVIDIA:mainfrom
mr-miles:feat/4945-policy-change-closes-everything

Conversation

@mr-miles

@mr-miles mr-miles commented Nov 7, 2026 •

Copy link
Copy Markdown

fix(network): keep connections open across policy reloads that do not affect them

Problem

Every network policy reload, provider attach/detach, and middleware registry change advances one sandbox-wide policy generation. Each relay pins the generation that authorized it and closes as soon as that generation moves (PolicyGenerationGuard::is_stale / wait_until_stale). So approving an endpoint for one host drops every open proxied connection in the sandbox: raw TCP, TLS passthrough, HTTP keep-alive, WebSocket, MCP and so on, even connections to hosts whose rules did not change.

This is reported in #4945 which this PR fixes.

This was deliberate (#2373, "invalidate relays on policy changes") and is documented, but it is coarser than it needs to be. RFC 0005 only says that a reload "can invalidate mappings whose endpoint contract is no longer current".

Change

When the generation moves, a pinned guard now re-checks the connection it protects and closes only if the answer changed.

  • At authorization, the relay captures a ConnectionPolicyContract: the L4 action and matched policy, the ordered endpoint_configs, the matched endpoints (policy name plus full endpoint object), exact_declared_endpoint_host, and the middleware chain selected for the host (plus the registry epoch when that chain is non-empty).
  • A PolicyRevalidator attached to the guard re-evaluates the connection's original OPA input against the live engine when the generation moves. Authorization and middleware selection run under one engine lock. If the contract is equal, the guard re-pins to the new generation and the relay continues. Otherwise the guard becomes permanently stale and the relay closes exactly as before.
  • Guard clones share one pin (Arc<AtomicU64>), so every clone a relay holds sees a re-pin.
  • It is wired in through the four pinning helpers in proxy/relay.rs (pin_policy_generation, pin_l7_evaluator, prepare_http_relay, prepare_raw_relay), so it covers raw TCP, TLS passthrough, transparent TCP, CONNECT HTTP (passthrough and inspected), upgrades, and forward HTTP. Existing call sites go through is_stale() and needed no change. The one exception is the raw generation comparison in the HTTP passthrough middleware path, which now lets the guard re-pin first.

Why a re-pinned tunnel can keep its cloned evaluator

Per-request Rego rules (allow_request, deny_request, request_deny_reason, allowed_token_grant_owners and their helpers) only read endpoints of policies whose binaries admit the caller and whose endpoints match the connection's host and port. That is exactly matched_endpoints. An unchanged contract therefore implies unchanged per-request decisions, and TunnelPolicyEngine keeps its clone.

Deliberately conservative

  • policy_hash is excluded from the comparison. It is the hash of the complete canonical policy, stamped into every endpoint during proto conversion, so it changes on every reload. It only binds endpoint-status observations to an inventory. The endpoint index is also excluded, so inserting an unrelated endpoint in the same policy does not close connections.
  • Fail-closed quarantine always closes, even if the previous policy is restored before a relay notices (last_fail_closed_generation).
  • Middleware registry swaps close connections that run middleware, because tunnels hold a clone of the runner (middleware_registry_epoch).
  • Provider credentials: relays that can inject credentials (HTTP-family) also pin the provider credential installation (revision plus installation id) and close when it changes. Requests already resolve credentials live, but an upstream session such as an upgraded WebSocket may have been authenticated with the previous credential. Raw TCP and TLS passthrough relays never inject credentials and do not pin it.
  • Setup is unchanged: a reload between authorization and relay setup still refuses the relay. Revalidation applies only to established relays.
  • Revalidation is invalidated permanently on the first mismatch or evaluation error, and gives up (stale) after four back-to-back reloads in a single check.

Known limitations and follow-ups

  • Status reporting: a re-pinned MCP tunnel keeps its original policy_hash, so the new inventory rejects its endpoint-status observations until the client reconnects. Authorization is unaffected. A follow-up could refresh the observation binding on re-pin.
  • Credential pin scope: the pin covers the whole provider installation, so attaching or detaching any provider still closes HTTP-family connections. Narrowing it to the providers bound to the connection's endpoints needs per-endpoint credential fingerprints.
  • Rego coupling: if per-request rules start reading policy data outside the matched endpoints, the contract must grow to match. The module docs in opa/revalidation.rs state this invariant.
  • Cost: each open connection does one OPA evaluation per relevant generation change, serialized on the engine lock (microseconds each). Connections with no traffic still re-check when woken by the generation watch.

Files

File Change
crates/openshell-supervisor-network/src/opa/revalidation.rs New: ConnectionPolicyContract, AuthorizedConnection, PolicyRevalidator
crates/openshell-supervisor-network/src/opa.rs Re-pinnable PolicyGenerationGuard, TunnelPolicyEngine::with_revalidator, connection_policy_contract, middleware_selection_at, authorize_egress_locked, quarantine generation and registry epoch tracking, tests
crates/openshell-supervisor-network/src/proxy/relay.rs Helpers capture a revalidator from the EgressDecision and pin credentials for HTTP relays, tests
crates/openshell-supervisor-network/src/proxy/egress.rs EgressDecision::policy_input()
crates/openshell-supervisor-network/src/proxy.rs Call sites pass the decision and, for forward HTTP, credentials
crates/openshell-supervisor-network/src/l7/relay.rs Passthrough middleware check lets the guard re-pin before comparing generations
crates/openshell-supervisor-middleware/src/lib.rs ChainEntry: PartialEq
docs/how-it-works/policies/manage-policies.mdx, network-rules.mdx Describe which changes close a connection

Tests

New engine tests in opa.rs:

  • an unrelated endpoint added: the connection survives;
  • endpoints reordered within the connection's own policy: it survives;
  • its endpoint or L7 rules changed: it closes and stays closed when the policy is restored;
  • its binary losing access, and a new overlapping policy: it closes;
  • quarantine: always closes;
  • clones share one pin;
  • the async wait ignores unrelated reloads;
  • policy_hash restamping is not a contract change;
  • denials capture no revalidator.

New relay tests in proxy/relay.rs:

  • bytes keep flowing through relay_tcp across an unrelated reload, and the relay closes when its endpoint is revoked;
  • an HTTP relay survives an unrelated reload but closes on a provider credential change;
  • a reload between authorization and pin still refuses the relay.

Manual check: open a long-lived connection (for example a WebSocket or curl to a slow endpoint) through the sandbox proxy, approve an endpoint for a different host, and confirm that the connection survives, the log shows "Policy change does not affect established connection; re-pinned", and the connection closes when its own host's rules change.

…r there is a change and leave unaffected connections untouched
@copy-pr-bot

copy-pr-bot Bot commented Nov 7, 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

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @mr-miles.

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 commented Nov 7, 2026 •

Copy link
Copy Markdown

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

@github-actions github-actions Bot closed this Nov 7, 2026
@mr-miles

mr-miles commented Nov 7, 2026

Copy link
Copy Markdown
Author

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

@mr-miles

mr-miles commented Nov 7, 2026

Copy link
Copy Markdown
Author

/recheck

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