Skip to content

fix(sandbox): add JSON bodies to proxy 403/502 responses and include port in HTTP log URLs#809

Open
johntmyers wants to merge 1 commit intomainfrom
807-808-network-fixes
Open

fix(sandbox): add JSON bodies to proxy 403/502 responses and include port in HTTP log URLs#809
johntmyers wants to merge 1 commit intomainfrom
807-808-network-fixes

Conversation

@johntmyers
Copy link
Copy Markdown
Collaborator

Summary

Proxy 403/502 error responses were returned with empty bodies, making them invisible to curl -s and indistinguishable from each other. Also, non-default ports were dropped from HTTP activity shorthand log lines.

Related Issue

Closes #807
Closes #808

Changes

  • Add build_json_error_response() helper in proxy.rs that produces JSON bodies with error and detail fields, consistent with the existing L7 deny path (send_deny_response in l7/rest.rs)
  • Update all 9 empty-body 403 sites (CONNECT + forward proxy: OPA denial, L7 denial, SSRF variants) and 1 empty-body 502 site (upstream connect failure) to return JSON bodies
  • Also update the 500 for credential injection failure
  • Fix Url::to_display_string() in openshell-ocsf to include the port when it differs from the scheme default (443/https, 80/http)

Error codes returned:

Code Status When
policy_denied 403 OPA or L7 policy blocks the request
ssrf_denied 403 DNS resolves to blocked IP / allowed_ips check fails
upstream_unreachable 502 TCP connect to upstream failed
credential_injection_failed 500 Unresolved placeholder survived rewriting

Before:

$ curl -s -H "X-Api-Key: $MY_KEY" http://api.example.com:9876/test
(empty output)

After:

{"detail":"GET api.example.com:9876/test not permitted by policy","error":"policy_denied"}

Testing

  • mise run pre-commit passes (excluding pre-existing z3 build env issue)
  • Unit tests added/updated — 3 new tests for build_json_error_response (403, 502, Content-Length correctness), 3 new tests for Url::to_display_string (non-default port, default port, no port), 1 new shorthand formatter test
  • Manual smoke test — created generic provider, attached to sandbox, verified header credential injection works end-to-end, confirmed JSON bodies appear on 403/502
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

…port in HTTP log URLs

L4 CONNECT and forward-proxy denial responses (403) and upstream
failure responses (502) were returned with empty bodies, making them
indistinguishable from each other when using curl -s. Add a
build_json_error_response() helper that produces consistent JSON
bodies with error and detail fields, matching the format already
used by the L7 deny path.

Also fix Url::to_display_string() in openshell-ocsf to include
non-default ports in the shorthand log output. Requests to e.g.
http://host:9876/path were logged as http://host/path, dropping the
port number.

Closes #807, closes #808
@johntmyers johntmyers requested a review from a team as a code owner April 10, 2026 22:17
@johntmyers johntmyers added the area:sandbox Sandbox runtime and isolation work label Apr 10, 2026
@johntmyers johntmyers self-assigned this Apr 10, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Apr 10, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:sandbox Sandbox runtime and isolation work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ocsf): HTTP activity shorthand log drops port from URL fix(proxy): L4 and forward-proxy 403/502 responses should include a body

1 participant