Skip to content

feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy#2245

Open
feloy wants to merge 5 commits into
NVIDIA:mainfrom
feloy:fix-1792-corporate-proxy-podman
Open

feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy#2245
feloy wants to merge 5 commits into
NVIDIA:mainfrom
feloy:fix-1792-corporate-proxy-podman

Conversation

@feloy

@feloy feloy commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add corporate HTTP proxy chaining so sandbox egress traffic can traverse enterprise forward proxies (HTTPS_PROXY/HTTP_PROXY/NO_PROXY)
  • The supervisor's policy proxy evaluates SSRF/allowlist rules as before, then dials the corporate proxy with HTTP CONNECT instead of connecting directly
  • Forward proxy environment variables through Podman driver config into sandbox containers

Related Issue

Part of #1792

Changes

  • openshell-supervisor-network: new upstream_proxy module reads standard proxy env vars, implements CONNECT handshake with optional Basic auth, and respects NO_PROXY bypass rules. dial_upstream() replaces the direct TcpStream::connect at the single post-SSRF connect site.
  • openshell-driver-podman: PodmanComputeConfig gains https_proxy, http_proxy, no_proxy fields with validation (only http:// schemes accepted). Container creation injects these as environment variables.
  • docs/reference/gateway-config.mdx: documents the new proxy config fields.
  • architecture/sandbox.md: updated with corporate proxy data flow.

Testing

  • Unit tests for UpstreamProxyConfig parsing, NO_PROXY matching (wildcards, CIDR, ports, loopback bypass), and CONNECT handshake (success, auth, rejection, timeout)

  • Unit tests for Podman config validation (empty values, unsupported schemes)

  • Existing proxy and SSRF test suites pass — policy evaluation is unchanged

  • mise run pre-commit passes

  • Unit tests added/updated

  • E2E tests added/updated (if applicable)

Checklist

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

@feloy feloy requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 13, 2026 16:06
@copy-pr-bot

copy-pr-bot Bot commented Jul 13, 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.

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This is project-valid work for the enterprise restricted-egress gap in #1792. Maintainer triage confirmed the need for corporate proxy chaining after OpenShell policy enforcement, and the PR includes the relevant Podman configuration plus Fern and architecture documentation.

Head SHA: ca5c041fca5547c2f659cf6d919db0ba8aaf39b7

Review findings:

  • Blocking — bind SSRF validation to the destination actually reached by the corporate proxy (crates/openshell-supervisor-network/src/proxy.rs:2818, upstream_proxy.rs:364). OpenShell validates locally resolved addresses but sends the hostname in CONNECT, allowing proxy-side DNS to resolve to a different private, metadata, or control-plane address. Use a selected validated IP for the CONNECT authority while retaining the hostname for HTTP Host/TLS SNI, or explicitly establish the corporate proxy as the SSRF enforcement boundary.
  • Blocking — keep proxy routing operator-owned (crates/openshell-driver-podman/src/container.rs:362, :394; upstream_proxy.rs:357). Sandbox/template environment currently wins over operator values, so a sandbox creator can choose an arbitrary proxy or set NO_PROXY=*. Use reserved supervisor-only variables written after user environment, and do not let ordinary HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY variants control this boundary.
  • Blocking — fail closed on malformed configured proxies (config.rs:211, upstream_proxy.rs:175, :229). Some invalid values pass Podman validation and are then ignored, silently restoring direct egress. Parse once into a typed configuration and reject startup when configured proxy data is invalid.
  • Blocking — preserve bytes read past the CONNECT response headers (upstream_proxy.rs:373-412). A coalesced 200 response and server-first payload currently drops the tunneled bytes. Consume exactly the header or return a buffered stream that replays over-read data.
  • Required follow-up — implement conventional plain-HTTP forwarding or narrow the advertised scope (proxy.rs:3159, :2818). The current HTTP_PROXY path still uses CONNECT and sends origin-form requests; many forward proxies expect absolute-form requests and deny CONNECT to port 80.
  • Required follow-up — reject control characters in decoded proxy credentials (upstream_proxy.rs:254, :365-369) to prevent HTTP header injection.
  • Required follow-up — apply the documented host-gateway bypass to HTTPS CONNECT paths too (proxy.rs:1102, :4396; architecture/sandbox.md:84).

Tests should cover the SSRF/DNS binding, CONNECT over-read, invalid-config fail-closed behavior, plain HTTP against a standard forward proxy, combined Podman environment precedence, and HTTPS host-gateway bypass. Runtime proxy behavior also requires test:e2e once these review findings are resolved; E2E has not been authorized on this head because author changes are needed first.

Docs: The existing Fern gateway reference is the correct page and no docs/index.yml navigation change is needed. The architecture claim about all host-gateway aliases bypassing the proxy must match the implementation. Kubernetes propagation remains outside this PR and should stay explicit.

@feloy, please push an updated commit addressing the items above. Gator will re-review the new head before starting the E2E gate.

Next state: gator:in-review

@johntmyers johntmyers added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 13, 2026
- Chain sandbox egress through a corporate HTTP proxy so outbound
  traffic from within the sandbox respects the host proxy settings
- Forward sandbox proxy environment variables to the generated Podman
  config so the proxy is applied consistently to Podman-managed workloads

Signed-off-by: Philippe Martin <phmartin@redhat.com>
@feloy feloy force-pushed the fix-1792-corporate-proxy-podman branch from ca5c041 to 7a1f8f3 Compare July 16, 2026 08:00
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This remains project-valid work for the corporate restricted-egress gap in #1792. The current scope covers the supervisor proxy path and Podman operator configuration, with the relevant Fern and architecture documentation present.

Head SHA: 7a1f8f3a38a282362f55c326411b9b2e3adcc4ad

The fresh independent code review found these blocking items on the current head:

  • Bind SSRF and allowed_ips validation to the destination the corporate proxy actually reaches (crates/openshell-supervisor-network/src/proxy.rs:2800-2824, upstream_proxy.rs:322-325). The code validates local DNS results, then sends the hostname in CONNECT and lets proxy-side DNS choose a potentially different private or control-plane address. Use a selected validated IP in the CONNECT authority while retaining the hostname for TLS SNI/HTTP Host, or require an explicit mode that delegates this security boundary to the corporate proxy.
  • Keep upstream routing operator-owned (crates/openshell-driver-podman/src/container.rs:362-404, upstream_proxy.rs:175-208). Sandbox/template environment currently takes precedence and can select an arbitrary proxy, set NO_PROXY=*, or use lowercase/ALL_PROXY variants to bypass operator routing. Use dedicated driver-owned supervisor variables injected at highest priority; arbitrary workload proxy variables must not configure this boundary.
  • Fail closed on malformed configured proxies (config.rs:212-233, upstream_proxy.rs:227-253). Values such as http:// or an invalid port pass driver validation, are ignored at supervisor startup, and restore direct dialing. Fully parse once and abort startup for any configured-but-invalid value.
  • Implement standard plain-HTTP forward-proxy behavior or narrow the scope (proxy.rs:3159-3200, :4396-4408). The current HTTP_PROXY path issues CONNECT and then sends origin-form requests; many enterprise proxies reject CONNECT to port 80 and require absolute-form requests. Use a separate forwarding path with operator proxy credentials, or remove the advertised plain-HTTP support.
  • Preserve tunneled bytes read with the CONNECT response (upstream_proxy.rs:373-412). Any payload coalesced after \r\n\r\n is discarded. Return a buffered stream that replays overflow and cover it with a combined-response/payload test.
  • Apply host-gateway bypass to HTTPS CONNECT as documented (proxy.rs:1102-1108, :4396-4400; architecture/sandbox.md:81-89). Only the plain-HTTP path currently bypasses the corporate proxy for host-gateway aliases.

Also tighten port-qualified NO_PROXY matching instead of broadening it to every port, avoid exposing long-lived proxy URL credentials through container environment/metadata, and add coverage for the security and interoperability boundaries above.

The prior control-character concern is not a header-injection blocker because decoded Basic credentials are Base64-encoded before insertion, though rejecting ASCII controls is still sensible input validation.

Docs: docs/reference/gateway-config.mdx is the correct Fern page; no docs/index.yml change is needed because the existing reference folder is already in navigation.

test:e2e and /ok to test are deferred until these review findings are resolved. @feloy, please push an updated commit; gator will review the new head before starting the pipeline gate.

Next state: gator:in-review

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the relevant Fern and architecture documentation is present.

Head SHA: 9f1b270d576c4fade99a36779661c8990eff1df4

The newest commit resolves the prior proxy-selection ownership finding: reserved OPENSHELL_UPSTREAM_* variables now override or remove sandbox/template values, and conventional workload-controlled proxy variables no longer steer the supervisor boundary.

The fresh independent code review found these remaining blocking items:

  • Bind SSRF and allowed_ips validation to the destination the corporate proxy actually reaches (crates/openshell-supervisor-network/src/proxy.rs:2814-2827, upstream_proxy.rs:374-379). The proxied path discards the locally validated addresses and sends the hostname in CONNECT, so proxy-side or split-horizon DNS can reach a different private or control-plane address. Use a selected validated IP for the CONNECT authority while retaining the hostname for TLS SNI/application Host, and add split-horizon regression coverage.
  • Fail closed on every configured-but-invalid proxy value (crates/openshell-driver-podman/src/config.rs:217-238, upstream_proxy.rs:194-205). Values such as http:// still pass driver validation and are ignored by supervisor parsing, silently restoring direct egress. Fully parse configuration at the boundary and abort startup for malformed authority, port, IPv6, userinfo, or mixed valid/invalid scheme configuration.
  • Keep proxy credentials out of the workload and container metadata (crates/openshell-driver-podman/src/container.rs:402-419, crates/openshell-supervisor-process/src/process.rs:73-90). The reserved proxy URLs are inherited by the initial agent because the child-environment denylist does not strip the new names; embedded credentials are also visible in Podman metadata. Strip all reserved variables from child processes and reject URL userinfo or deliver credentials through a protected file/secret channel.
  • Implement conventional plain-HTTP forwarding or narrow the feature scope (crates/openshell-supervisor-network/src/proxy.rs:3364-3369, :4404-4411). The current HTTP path uses CONNECT and then origin-form requests; standard forward proxies commonly require absolute-form requests and may deny CONNECT to port 80.
  • Preserve tunneled bytes read with the CONNECT response (crates/openshell-supervisor-network/src/upstream_proxy.rs:388-427). Bytes coalesced after \r\n\r\n are discarded; replay the overflow through a buffered stream and test a combined response/payload write.
  • Apply host-gateway bypass consistently to HTTPS CONNECT (crates/openshell-supervisor-network/src/proxy.rs:1104-1110, :4399-4403). Only plain HTTP currently forces a direct dial for host-gateway aliases.
  • Honor ports in port-qualified NO_PROXY entries (crates/openshell-supervisor-network/src/upstream_proxy.rs:123-132, :589-594). The current parser strips the port and broadens the bypass to every port and subdomain; carry the destination port through matching and add bracketed IPv6 coverage.
  • Align docs/help and extend trust-boundary tests. crates/openshell-driver-podman/README.md:348-355 and CLI help at src/main.rs:107-117 still describe conventional-variable injection or per-sandbox precedence. Tests must also cover validated-address dialing, fail-closed startup, child-environment stripping, HTTP wire form, HTTPS host-gateway bypass, and CONNECT over-read. The Fern page is already navigated, so no docs/index.yml change is needed.

test:e2e and /ok to test remain deferred because author changes are required before pipeline gating. @feloy, please push an updated commit; gator will review the new head.

Next state: gator:in-review

The operator-configured corporate egress proxy was injected under the
conventional HTTPS_PROXY/HTTP_PROXY/NO_PROXY names as defaults beneath
sandbox spec/template environment, so a sandbox creator could redirect
egress at an arbitrary proxy or disable proxying with NO_PROXY=*.

Route the boundary through reserved, supervisor-only variables
(OPENSHELL_UPSTREAM_HTTPS_PROXY/HTTP_PROXY/NO_PROXY) written in the
Podman driver's required-variable tier. Any sandbox-supplied value under
a reserved name is stripped before the operator value is applied, so the
supervisor never observes a reserved proxy variable the operator did not
set. The supervisor now reads only the reserved names and ignores the
conventional proxy variables the sandbox controls.

Add the reserved proxy variables to the supervisor-only child-environment
denylist so the corporate proxy URL and any embedded credentials are not
inherited by the sandbox workload, which reaches egress through the local
policy proxy and never needs them.

Signed-off-by: Philippe Martin <phmartin@redhat.com>
@feloy feloy force-pushed the fix-1792-corporate-proxy-podman branch from 9f1b270 to 42291f2 Compare July 16, 2026 11:07
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, with the relevant Fern and architecture documentation present.

Head SHA: 42291f2a1b3b1660581dd4fef2f79e3d17e5b7cf

The newest commit resolves the prior child-environment part of the credential finding: reserved OPENSHELL_UPSTREAM_* values are now stripped from workload child processes. The operator-owned routing boundary also remains correctly enforced against sandbox/template overrides.

The fresh independent code review found these remaining blocking items:

  • Bind SSRF and allowed_ips validation to the destination the corporate proxy actually reaches (crates/openshell-supervisor-network/src/proxy.rs:2802-2827, upstream_proxy.rs:337-386). The proxied path still sends the hostname in CONNECT after validating local DNS, allowing proxy-side or split-horizon DNS to reach a different internal or non-allowlisted address. Use a selected validated IP for the CONNECT authority while retaining the hostname for TLS SNI/application Host, or otherwise verifiably bind proxy resolution to the validated destination.
  • Fail closed on every configured-but-invalid proxy value (crates/openshell-driver-podman/src/config.rs:217-238, upstream_proxy.rs:194-205). Values such as http:// or malformed authorities can pass driver validation, be ignored by supervisor parsing, and silently restore direct egress. Reuse a strict typed parser at both boundaries and abort startup for any present-but-invalid value.
  • Keep proxy credentials out of container metadata and generated configuration (crates/openshell-driver-podman/src/container.rs:402-419, tasks/scripts/gateway.sh:311-313, :355-359). URL userinfo is still visible through Podman inspection, and the generated gateway.toml can inherit permissive umask permissions. Use a root-only secret/file channel and mode 0600 for credential-bearing configuration.
  • Implement conventional plain-HTTP forwarding or narrow the feature scope (crates/openshell-supervisor-network/src/proxy.rs:3162-3205, :4399-4412). The HTTP path still uses CONNECT and then sends origin-form requests; common enterprise proxies require absolute-form forwarding and may reject CONNECT to port 80.
  • Preserve tunneled bytes read with the CONNECT response (crates/openshell-supervisor-network/src/upstream_proxy.rs:388-427). Replay bytes received after the response-header terminator and add combined response/payload coverage.
  • Apply host-gateway bypass consistently to HTTPS CONNECT (crates/openshell-supervisor-network/src/proxy.rs:1104-1112, :2814-2827). Plain HTTP bypasses host aliases, while CONNECT still consults the corporate proxy.
  • Honor ports in port-qualified NO_PROXY entries (crates/openshell-supervisor-network/src/upstream_proxy.rs:123-163, :214-225). internal.corp:8443 is still broadened to every port and subdomain.
  • Align docs/help with the implementation and scope. crates/openshell-driver-podman/README.md:348-355 and CLI help still describe conventional variables or per-sandbox precedence. Also either add Kubernetes Secret-backed propagation or explicitly narrow the feature to Podman. The existing Fern page is correct and needs no navigation change.

Please add regression coverage for the SSRF/DNS binding, fail-closed startup, HTTP wire form, HTTPS host-gateway bypass, CONNECT over-read, port-aware NO_PROXY, and credential delivery boundary.

test:e2e and /ok to test remain deferred because author changes are required before pipeline gating. @feloy, please push an updated commit; gator will review the new head.

Next state: gator:in-review

…file

Proxy credentials were embedded inline in the proxy URL, so they were
stored in gateway.toml and exposed in container metadata via
'podman inspect'.

Reject inline 'user:pass@' credentials in https_proxy/http_proxy at
startup (parsed with the url crate rather than hand-splitting), and add a
proxy_auth_file option pointing at a 'user:pass' file. The driver stages
that file as a per-sandbox root-only Podman secret, mounts it at a fixed
path, and exports only the path in the reserved
OPENSHELL_UPSTREAM_PROXY_AUTH_FILE variable, so the credential never
appears in config, environment, or container metadata. Reading the file
fails closed on a missing, empty, or control-character-bearing value.

The supervisor reads the credential from the mounted file and builds the
Proxy-Authorization: Basic header, rejecting control characters, and no
longer derives credentials from URL userinfo. The auth-file path is added
to the child-environment strip list, and the generated gateway.toml is
written owner-only (mode 0600).

Signed-off-by: Philippe Martin <phmartin@redhat.com>
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the relevant Fern and architecture documentation is present.

Head SHA: d74c780b4388877d50b20689f0ea6c1f1c1c48ae

The newest commit resolves the prior credential-storage concern substantially: proxy credentials now use a root-only per-sandbox Podman secret, the reserved auth-file path is stripped from workload children, and generated gateway configuration is mode 0600. Operator-owned proxy routing also remains protected from sandbox/template overrides.

The fresh independent code review found these remaining blocking items:

  • Bind SSRF and allowed_ips validation to the destination the corporate proxy actually reaches (crates/openshell-supervisor-network/src/proxy.rs:1104-1110, :2821-2826; upstream_proxy.rs:366-369, :403-408). The proxied path still sends the hostname after validating local DNS, so split-horizon or proxy-side DNS can reach a different private, metadata, control-plane, or non-allowlisted address. CONNECT to a selected validated IP while retaining the hostname for policy, logging, TLS SNI, and certificate verification, or define an explicit trusted-proxy enforcement contract.
  • Fail closed on every present-but-invalid proxy or auth setting (upstream_proxy.rs:190-224, :281-310, :350-354). Invalid reserved proxy values can be ignored, an unreadable auth file explicitly proceeds without credentials, and malformed credentials silently become unauthenticated. Make any configured-but-invalid URL, auth file, or credential fatal to supervisor startup/readiness and share validation semantics with the driver.
  • Implement conventional plain-HTTP forwarding or narrow the feature scope (proxy.rs:3162-3202, :4404-4411; upstream_proxy.rs:403-415). The current http_proxy path uses CONNECT and then origin-form requests; common enterprise proxies expect absolute-form forwarding and may reject CONNECT to port 80.
  • Preserve tunneled bytes read with the CONNECT response (upstream_proxy.rs:417-456). Bytes received after \r\n\r\n in the same read are discarded. Replay that overflow and cover a combined response/payload write.
  • Apply host-gateway bypass consistently to HTTPS CONNECT (proxy.rs:1104-1110, :4399-4402). Plain HTTP forces a direct dial for host-gateway aliases, but HTTPS still enters generic proxy selection.
  • Honor ports in port-qualified NO_PROXY entries (upstream_proxy.rs:123-163, :249-256). internal.corp:8443 currently bypasses the proxy for every port because the qualifier is discarded.
  • Make the Basic-auth transport boundary explicit or protect it (upstream_proxy.rs:401-415). Credentials are stored safely, but Proxy-Authorization: Basic is sent over the plain http:// connection to the corporate proxy. Support TLS-to-proxy, restrict authenticated use to a protected link, or document this cleartext transport limitation prominently.

Docs: docs/reference/gateway-config.mdx is the correct Fern page and no docs/index.yml change is needed. Please correct the architecture claim that host-gateway aliases always dial directly, avoid implying propagation beyond Podman, and document the CONNECT-only HTTP and Basic-auth transport limitations unless the implementation changes.

Please add focused regressions for validated-IP binding, fatal invalid config/auth handling, absolute-form HTTP forwarding, CONNECT over-read, HTTPS host-gateway bypass, port-aware NO_PROXY, and authenticated proxy lifecycle/E2E behavior.

test:e2e and /ok to test remain deferred because author changes are required before pipeline gating. @feloy, please push an updated commit; gator will review the new head.

Next state: gator:in-review

The reserved OPENSHELL_UPSTREAM_* variables are an operator-owned egress
boundary, but the supervisor treated present-but-invalid values as unset:
an unsupported or malformed proxy URL was ignored with a warning, an
unreadable auth file proceeded without credentials, and a malformed
credential silently became unauthenticated. Any of these could quietly
downgrade the corporate proxy boundary to direct dialing or
unauthenticated proxy access.

Make every configured-but-invalid proxy or auth setting fatal to
supervisor proxy startup, emit an OCSF ConfigStateChange failure event
before refusing, and share URL validation semantics between the Podman
driver and the supervisor through a single validator in
openshell-core (parse_upstream_proxy_url), so a value accepted at
sandbox-create time can never be rejected in-container or vice versa.

Inline user:pass@ URL credentials are now fatal in the supervisor too
(previously warn-and-strip), matching the driver. Unset or empty
variables still mean no proxy; only present-but-invalid values fail.
Error paths never include credential content.

Addresses the fail-closed review item on NVIDIA#2245.

Signed-off-by: Philippe Martin <phmartin@redhat.com>
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the existing Fern gateway reference is the right documentation location.

Head SHA: cd9fd6cd84fc5e9f4726542a3770d2508e66f996

The latest commit materially improves the fail-closed configuration path: non-empty malformed or unsupported proxy URLs, inline credentials, and unreadable or malformed auth-file contents now fail closed through shared URL validation. The operator-owned environment boundary, workload stripping, and Podman secret staging also remain sound.

The fresh independent code review found these remaining blocking items:

  • Bind SSRF and allowed_ips validation to the destination the corporate proxy actually reaches (crates/openshell-supervisor-network/src/proxy.rs:1123, :2833-2845; upstream_proxy.rs:347-389). The supervisor validates locally resolved addresses, then sends the hostname in CONNECT for proxy-side resolution. Split-horizon DNS or rebinding can therefore reach a different private, metadata, control-plane, or non-allowlisted address. CONNECT to a selected validated IP while retaining the hostname for TLS SNI/application Host, or define a verifiable trusted-proxy resolution contract.
  • Implement conventional plain-HTTP forward-proxy behavior or narrow the feature scope (proxy.rs:4423, :4523-4531; upstream_proxy.rs:377-396). http_proxy uses CONNECT and then sends an origin-form request; standard enterprise proxies commonly require absolute-form forwarding and may reject CONNECT to port 80. Add a separate HTTP-forwarding path with trusted proxy auth and representative wire-level coverage.
  • Preserve tunneled bytes read with the CONNECT response (upstream_proxy.rs:398-437). Bytes received after \r\n\r\n in the same read are discarded; replay the overflow and add a combined response/payload regression.
  • Apply host-gateway bypass consistently to HTTPS CONNECT (proxy.rs:1123, :4418-4425; architecture/sandbox.md:89). Plain HTTP bypasses driver-injected host aliases, while HTTPS can still enter corporate proxy selection despite the documented direct-dial invariant.
  • Honor ports in port-qualified NO_PROXY entries (upstream_proxy.rs:129, :266-276, :708-712). internal.corp:8443 is currently broadened to all ports and subdomains; retain the optional port and match it against the requested destination port.
  • Finish the fail-closed credential/config contract (upstream_proxy.rs:211-229, :328-339; crates/openshell-driver-podman/src/driver.rs:132-147). Present-but-whitespace reserved values are treated as unset, and driver/supervisor credential validation still differs. Use one shared credential parser, reject present-but-empty values, and enforce the documented credential form consistently.
  • Protect or clearly document the Basic-auth transport boundary (upstream_proxy.rs:382-396; docs/reference/gateway-config.mdx:365-378). Only plaintext http:// proxy transport is accepted, so Proxy-Authorization: Basic is exposed on that network link. Support authenticated TLS to the proxy or explicitly require a trusted isolated link and document the limitation.

Docs/tests: No docs/index.yml change is needed because the existing Reference folder already includes the page. Update Fern/architecture claims to match the final HTTP, host-gateway, NO_PROXY, fail-closed, and auth-transport behavior. Add focused regressions for proxy/local DNS divergence, absolute-form HTTP forwarding, HTTPS host-gateway bypass, CONNECT over-read, port-qualified NO_PROXY, and the shared config/credential parser.

test:e2e and /ok to test remain deferred because author changes are still required before pipeline gating. @feloy, please push an updated commit; gator will review the new head.

Next state: gator:in-review

… contract

The upstream proxy URL already had a single shared validator, but the
credential did not: the Podman driver rejected only CR/LF/NUL while the
supervisor rejected every control character, so a credential accepted at
sandbox-create time (e.g. one containing a tab) could still be rejected
in-container. Present-but-whitespace reserved OPENSHELL_UPSTREAM_*
values were also silently treated as unset, quietly downgrading the
operator's egress boundary to direct dialing.

Add parse_upstream_proxy_credential to openshell-core as the single
source of truth for the documented user:pass credential form (non-empty
user, no control characters, trimmed) and use it in both the Podman
driver's secret staging and the supervisor's Proxy-Authorization header
construction. Error variants carry no payload so credential content can
never leak into messages.

Make a present-but-empty reserved variable fatal to supervisor proxy
startup instead of meaning "unset"; only fully unset variables disable
the proxy. The driver correspondingly rejects an empty no_proxy at
config time so it can never inject a value the supervisor refuses.

Addresses the remaining fail-closed credential/config review item on

Signed-off-by: Philippe Martin <phmartin@redhat.com>
NVIDIA#2245.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants