Skip to content

Proxy improvements: better handling of non-CONNECT requests and observability #42

Closed

Description

@pimlock

The sandbox HTTP proxy (crates/navigator-sandbox/src/proxy.rs) is a CONNECT-only proxy. Several improvements have been identified:

1. Return 403 instead of 405 for plain HTTP proxy requests

Currently, when a client sends a forward-proxy style plain HTTP request (e.g. GET http://example.com/ HTTP/1.1), the proxy returns 405 Method Not Allowed. This is technically correct (we don't support that method) but semantically misleading — from the sandboxed process's perspective, the request was denied, not unsupported.

We should return 403 Forbidden instead to align with how HTTPS CONNECT denials are surfaced.

2. Add deny logging for non-CONNECT requests

Plain HTTP proxy requests are currently rejected silently (no structured log). HTTPS CONNECT requests that are denied by policy produce structured deny logs. We should emit a similar log line for rejected non-CONNECT requests so operators have full visibility into all proxy traffic attempts.

The hostname can be extracted from the absolute-form URI in the request line (e.g. http://badstuff.com/path → badstuff.com).

3. Create cleaner user facing error messages

See first comment.


More items may be added to this issue as additional proxy improvements are identified.


Originally by @johntmyers on 2026-02-25T17:20:13.123-08:00

Activity

  1. pimlock commented on Feb 27, 2026

    @pimlock
    CollaboratorAuthor

    Revised messaging strategy:

    The principle: if the request IS a recognized inference endpoint, we can acknowledge that in the message since inference is a first-class concept. If it is NOT inference, treat it as a generic policy deny with no mention of inference internals.

    # File:Line Current text Trigger (user-facing behavior) Revised message Rationale
    1 proxy.rs:872 "only inference API calls are allowed on this connection" Connection falls through to inference inspection, TLS terminated, but HTTP request inside isn't a known inference API endpoint "connection not allowed by policy" Not inference → generic policy deny
    2 proxy.rs:821 "no inference routes configured" Same as #1, but request does match an inference API pattern — however the inference route table is empty (no backends configured via gRPC) "inference endpoint detected without matching inference route" Is inference → explain actual failure
    3 proxy.rs:708 "missing inference context" Connection has no matching network policy, OPA returns inspect_for_inference, but InferenceContext was never initialized (e.g. inference config missing at sandbox startup) "connection not allowed by policy" Not inference (couldn't even check) → generic policy deny
    4 proxy.rs:765 "non-inference request" Same trigger as #1 — first HTTP request inside TLS-terminated tunnel doesn't match any known inference API pattern (log perspective) "connection not allowed by policy" Not inference → generic policy deny
    5 proxy.rs:869 "Non-inference request denied (inference-only mode)" Same trigger as #1/#4 — second info\! log emitted right before the 403 response "connection not allowed by policy" Not inference → generic policy deny
    6 proxy.rs:851 "Local inference routing failed" Request matches an inference API pattern and route table has a backend, but the backend request fails (server down, timeout, connection refused) "inference endpoint detected but upstream service failed" Is inference → explain actual failure
    7 dev-sandbox-policy.rego:54 "no matching policy and no inference routing configured" Sandboxed process connects to any host when policy has zero network policies AND zero inference allowed_routes — OPA denies at evaluation time "network connections not allowed by policy" OPA-level deny → generic policy deny

    Originally by @johntmyers on 2026-02-25T17:40:11.414-08:00

  2. johntmyers commented on Mar 3, 2026

    @johntmyers
    Collaborator

    🏗️ build-plan

    Implementation Plan

    Issue type: fix
    Complexity: Low
    Confidence: High — all changes are string/status-code replacements at known locations with clear before/after values

    Summary

    This issue covers three related proxy improvements: (1) returning 403 instead of 405 for plain HTTP proxy requests, (2) adding structured deny logging for non-CONNECT requests, and (3) revising 7 user-facing error messages across proxy.rs and dev-sandbox-policy.rego to follow a consistent principle — inference-aware messages for recognized inference endpoints, generic policy-deny messages for everything else.

    Scope

    • crates/navigator-sandbox/src/proxy.rs: Change non-CONNECT response from 405→403, add structured deny log for non-CONNECT requests, and revise 5 error messages at known locations
    • dev-sandbox-policy.rego: Revise 1 deny_reason message (line 54)

    Implementation Steps

    Step 1: Change non-CONNECT response from 405 to 403 and add deny logging (proxy.rs:228-231)

    Current code:

    if method != "CONNECT" {
        respond(&mut client, b"HTTP/1.1 405 Method Not Allowed\r\n\r\n").await?;
        return Ok(());
    }

    Change to:

    • Replace 405 Method Not Allowed with 403 Forbidden
    • Before the response, extract the hostname from the absolute-form URI target (e.g. http://badstuff.com/path → badstuff.com) using url::Url::parse or simple string parsing
    • Emit a structured info! log with fields: method, target_host (extracted hostname), and message "Non-CONNECT proxy request denied"

    Step 2: Revise error message #1 — proxy.rs route_inference_request() non-inference denial (line ~912)

    Current Revised
    "only inference API calls are allowed on this connection" "connection not allowed by policy"

    This is the JSON body returned as a 403 when a request inside a TLS-terminated tunnel doesn't match any inference API pattern.

    Step 3: Revise error message #2 — proxy.rs route_inference_request() empty routes (line ~861)

    Current Revised
    "no inference routes configured" "inference endpoint detected without matching inference route"

    This is the JSON body returned as a 503 when the request IS recognized as inference but the route table is empty.

    Step 4: Revise error message #3 — proxy.rs handle_inference_interception() missing context (line ~748)

    Current Revised
    "missing inference context" "connection not allowed by policy"

    This is the InferenceOutcome::Denied reason when InferenceContext is None. Not inference → generic policy deny.

    Step 5: Revise error message #4 — proxy.rs handle_inference_interception() non-inference request (line ~805)

    Current Revised
    "non-inference request" "connection not allowed by policy"

    This is the InferenceOutcome::Denied reason when the first HTTP request in a TLS-terminated tunnel doesn't match any inference pattern. Not inference → generic policy deny.

    Step 6: Revise error message #5 — proxy.rs route_inference_request() info log (line ~909)

    Current Revised
    "Non-inference request denied (inference-only mode)" "connection not allowed by policy"

    This is the info! log message for the same trigger as #1/#4. Aligning the log message with the user-facing error.

    Step 7: Revise error message #6 — proxy.rs route_inference_request() routing failure (line ~891)

    Current Revised
    "Local inference routing failed" "inference endpoint detected but upstream service failed"

    This is the warn! log when the router's proxy_with_candidates call fails. IS inference → explain actual failure.

    Step 8: Revise error message #7 — dev-sandbox-policy.rego deny_reason (line 54)

    Current Revised
    "no matching policy and no inference routing configured" "network connections not allowed by policy"

    OPA-level deny when there are zero network policies AND zero inference routes → generic policy deny.

    Step 9: Add/update unit tests

    • Add a test for hostname extraction from absolute-form URIs (the new helper for Step 1)
    • Update any existing tests that assert on the old error message strings (review the 33 existing tests in mod tests — none currently test these messages, so this is likely just new tests)
    • Add a test verifying the parse_target-style extraction for non-CONNECT request URIs

    Test Plan

    • Unit tests: Add tests for the new hostname-extraction logic used in non-CONNECT deny logging. Existing tests don't assert on any of the changed message strings, so no breakage expected. Tests live in proxy.rs inline mod tests.
    • Integration tests: N/A — these are string and status-code changes with no new integration points.
    • E2E tests: N/A — no changes under e2e/. The E2E sandbox tests exercise CONNECT flows which are not affected by the non-CONNECT status code change. The error message changes are internal to denied paths that E2E tests don't assert on.

    Risks & Open Questions

    • The hostname extraction for non-CONNECT deny logging should handle malformed URIs gracefully (log "unknown" if parsing fails rather than panicking).
    • The rego message change at dev-sandbox-policy.rego:54 may appear in OPA evaluation results surfaced to users — confirm this is the desired user-facing text.

    Documentation Impact

    • None expected — these are error message and status code corrections, not architectural changes.

    Revision 1 — initial plan

  3. added a commit that references this issue on Mar 3, 2026
    ff1b9b2
  4. johntmyers commented on Mar 3, 2026

    @johntmyers
    Collaborator

    🏗️ build-from-issue-agent

    Implementation Complete

    PR: #79

    What was built

    Improved the sandbox HTTP proxy to return 403 (instead of 405) for non-CONNECT requests with structured deny logging, and revised 7 user-facing error messages across proxy.rs and dev-sandbox-policy.rego to follow a consistent messaging principle.

    Tests

    • Unit: 7 tests added (extract_host_from_uri covering HTTP, HTTPS, port, IPv6, no-path, empty, malformed)
    • Integration: N/A
    • E2E: Existing assertion updated (no new E2E tests)

    Docs updated

    • architecture/sandbox.md
    • architecture/inference-routing.md

    The issue will auto-close when the PR is merged.

  5. added a commit that references this issue on Mar 3, 2026
    52b48d1
  6. added a commit that references this issue on Mar 16, 2026
    c06117e
  7. added a commit that references this issue on Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    state:agent-readyApproved for agent implementationstate:pr-openedPR has been opened for this issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions