fix(proxy): return 403 for non-CONNECT requests, add deny logging, and revise error messages#79
Merged
johntmyers merged 1 commit intomainfrom Mar 3, 2026
Conversation
…d revise error messages Closes #42 - Change non-CONNECT proxy response from 405 to 403 to align with how CONNECT denials are surfaced - Add structured deny logging for non-CONNECT requests with hostname extraction from absolute-form URIs - Revise 7 user-facing error messages across proxy.rs and dev-sandbox-policy.rego to follow consistent principle: generic policy-deny messages for non-inference requests, descriptive messages for recognized inference endpoints - Update E2E test assertion to match new error message - Update architecture docs to reflect new behavior
drew
approved these changes
Mar 3, 2026
drew
pushed a commit
that referenced
this pull request
Mar 16, 2026
…) (!52) > **🏗️ build-from-issue-agent** Closes #79 ## Summary Replaces the gateway-proxied inference model (`ProxyInference` RPC) with sandbox-local execution. The sandbox now resolves routes from either a standalone YAML route file or a cluster bundle fetched via the new `GetSandboxInferenceBundle` RPC, then forwards requests directly to inference backends using `navigator-router`. ### Benefits of removing the gRPC hop Moving inference execution from the gateway to the sandbox eliminates the gRPC round-trip for every inference request: - **No payload size limits**: Direct HTTP forwarding replaces protobuf-over-gRPC serialization. - **Streaming-ready**: Preserves connection semantics for future SSE support (e.g., `stream: true`). - **Lower latency**: Direct sandbox → backend path instead of sandbox → gateway → backend round-trip. - **Reduced gateway load**: Gateway only serves the control-plane bundle delivery RPC. - **Simpler error model**: HTTP status codes flow through directly without gRPC wrapping. ## Changes Made - **Proto**: Removed `ProxyInference` RPC. Added `GetSandboxInferenceBundle` RPC for route bundle delivery. - **Gateway**: Replaced `proxy_inference` with `get_sandbox_inference_bundle`. Removed `Router` and `navigator-router` dependency — gateway is now control-plane only for inference. - **Router**: Switched config file format from TOML to YAML. Added `Debug` impl that redacts API keys. - **Sandbox**: New `InferenceContext` with local `Router` + route cache. Routes loaded from `--inference-routes` file (standalone) or cluster bundle via gRPC, with 30s background refresh. New `--inference-routes` / `NAVIGATOR_INFERENCE_ROUTES` CLI arg. - **Dev sandbox**: Added `inference-routes.yaml` at repo root with default NVIDIA NIM route. `mise run sandbox` now mounts it automatically and supports `-e` flag to forward env vars (e.g., `mise run sandbox`). - **Example**: Added standalone example (`examples/inference/routes.yaml`) and rewrote README to cover both standalone and cluster workflows. - **E2E tests**: Updated existing test names/docs. Added tests for Anthropic messages protocol and multi-route policy filtering. ## Tests Added - **Unit (21 new):** `router_error_to_http` (6 variants), `load_from_file` YAML round-trip (5), `resolve_sandbox_inference_bundle` gRPC handler (6), `build_inference_context` route loading (4) - **E2E (2 new):** Anthropic messages protocol routing, route filtering by `allowed_routes` - **Total:** 188 unit tests passing across 3 crates ## Documentation Updated - `architecture/gateway.md`: Removed router, updated Inference Service - `architecture/sandbox.md`: Updated orchestration flow, InferenceContext, route loading - `architecture/inference-routing.md`: Complete rewrite for sandbox-local architecture ## Verification - [x] All unit tests passing (188 across 3 crates) - [x] Pre-commit Rust checks passing (fmt, clippy, tests) - [x] Architecture documentation updated - [x] E2E tests updated with new test cases - [x] Standalone example and documentation added - [x] Default inference-routes.yaml with env passthrough for mise run sandbox
drew
pushed a commit
that referenced
this pull request
Mar 16, 2026
…d revise error messages (#79) Closes #42 - Change non-CONNECT proxy response from 405 to 403 to align with how CONNECT denials are surfaced - Add structured deny logging for non-CONNECT requests with hostname extraction from absolute-form URIs - Revise 7 user-facing error messages across proxy.rs and dev-sandbox-policy.rego to follow consistent principle: generic policy-deny messages for non-inference requests, descriptive messages for recognized inference endpoints - Update E2E test assertion to match new error message - Update architecture docs to reflect new behavior Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
drew
pushed a commit
that referenced
this pull request
Mar 16, 2026
…) (!52) > **🏗️ build-from-issue-agent** Closes #79 ## Summary Replaces the gateway-proxied inference model (`ProxyInference` RPC) with sandbox-local execution. The sandbox now resolves routes from either a standalone YAML route file or a cluster bundle fetched via the new `GetSandboxInferenceBundle` RPC, then forwards requests directly to inference backends using `navigator-router`. ### Benefits of removing the gRPC hop Moving inference execution from the gateway to the sandbox eliminates the gRPC round-trip for every inference request: - **No payload size limits**: Direct HTTP forwarding replaces protobuf-over-gRPC serialization. - **Streaming-ready**: Preserves connection semantics for future SSE support (e.g., `stream: true`). - **Lower latency**: Direct sandbox → backend path instead of sandbox → gateway → backend round-trip. - **Reduced gateway load**: Gateway only serves the control-plane bundle delivery RPC. - **Simpler error model**: HTTP status codes flow through directly without gRPC wrapping. ## Changes Made - **Proto**: Removed `ProxyInference` RPC. Added `GetSandboxInferenceBundle` RPC for route bundle delivery. - **Gateway**: Replaced `proxy_inference` with `get_sandbox_inference_bundle`. Removed `Router` and `navigator-router` dependency — gateway is now control-plane only for inference. - **Router**: Switched config file format from TOML to YAML. Added `Debug` impl that redacts API keys. - **Sandbox**: New `InferenceContext` with local `Router` + route cache. Routes loaded from `--inference-routes` file (standalone) or cluster bundle via gRPC, with 30s background refresh. New `--inference-routes` / `NAVIGATOR_INFERENCE_ROUTES` CLI arg. - **Dev sandbox**: Added `inference-routes.yaml` at repo root with default NVIDIA NIM route. `mise run sandbox` now mounts it automatically and supports `-e` flag to forward env vars (e.g., `mise run sandbox`). - **Example**: Added standalone example (`examples/inference/routes.yaml`) and rewrote README to cover both standalone and cluster workflows. - **E2E tests**: Updated existing test names/docs. Added tests for Anthropic messages protocol and multi-route policy filtering. ## Tests Added - **Unit (21 new):** `router_error_to_http` (6 variants), `load_from_file` YAML round-trip (5), `resolve_sandbox_inference_bundle` gRPC handler (6), `build_inference_context` route loading (4) - **E2E (2 new):** Anthropic messages protocol routing, route filtering by `allowed_routes` - **Total:** 188 unit tests passing across 3 crates ## Documentation Updated - `architecture/gateway.md`: Removed router, updated Inference Service - `architecture/sandbox.md`: Updated orchestration flow, InferenceContext, route loading - `architecture/inference-routing.md`: Complete rewrite for sandbox-local architecture ## Verification - [x] All unit tests passing (188 across 3 crates) - [x] Pre-commit Rust checks passing (fmt, clippy, tests) - [x] Architecture documentation updated - [x] E2E tests updated with new test cases - [x] Standalone example and documentation added - [x] Default inference-routes.yaml with env passthrough for mise run sandbox
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
2 participants
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.
Closes #42
Summary
Improves the sandbox HTTP proxy by returning 403 (instead of 405) for non-CONNECT requests, adding structured deny logging for non-CONNECT traffic, and revising 7 user-facing error messages to follow a consistent principle: generic policy-deny messages for non-inference requests, descriptive failure messages for recognized inference endpoints.
Changes Made
crates/navigator-sandbox/src/proxy.rs: Changed non-CONNECT response from 405→403, addedextract_host_from_uri()helper and structuredinfo!log for non-CONNECT denials, revised 5 error messages per the messaging strategy in issue Proxy improvements: better handling of non-CONNECT requests and observability #42's first commentdev-sandbox-policy.rego: Revised OPA deny_reason from "no matching policy and no inference routing configured" to "network connections not allowed by policy"e2e/python/test_inference_routing.py: Updated assertion to match new error message "inference endpoint detected without matching inference route"architecture/sandbox.md: Updated error messages, status codes, and deny reason referencesarchitecture/inference-routing.md: Updated error message strings in step-by-step flow and error tableDeviations from Plan
None — implemented as planned.
Tests Added
extract_host_from_uri()covering HTTP URIs, HTTPS URIs, URIs with ports, IPv6 bracket notation, no-path URIs, empty input, and malformed inputDocumentation Updated
architecture/sandbox.md: Updated error messages, 405→403 status code, and deny reason referencesarchitecture/inference-routing.md: Updated error message strings in flow descriptions and error tablesVerification