Add upstream proxy by eraow · Pull Request #104 · coder/httpjail

eraow · 2026-07-13T13:18:20Z

Note: This PR was generated with AI assistance.

Summary

Adds support for routing httpjail's outbound requests through an upstream proxy using the standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables.

Motivation

httpjail currently connects directly to destination hosts. This does not work in corporate environments where internet access must pass through an HTTP proxy.

Behavior

Uses HTTP_PROXY for HTTP destinations and HTTPS_PROXY for HTTPS destinations.
Accepts the lowercase http_proxy, https_proxy, and no_proxy variants when their uppercase equivalents are unset.
Uses NO_PROXY to contact matching destinations directly.
- Supports domains and subdomains.
- Supports exact IPv4 and IPv6 addresses.
- Supports IPv4 and IPv6 CIDRs.
- A value of * disables upstream proxying.
Supports http://host:port and bare host:port proxy addresses.
Supports Basic authentication through proxy URL user information.
Reaching the proxy itself over TLS (https://proxy) is not supported. HTTPS destinations still work through an ordinary HTTP proxy using CONNECT.
Sends HTTPS traffic through a CONNECT tunnel and forwards plain HTTP requests in absolute-form.
Sends proxy credentials only to the upstream proxy, never to a destination reached directly through NO_PROXY or inside an established HTTPS tunnel.
Reads CONNECT responses in bounded chunks and preserves tunnel bytes received after the response headers.
Bounds proxy TCP connection and CONNECT exchange setup without imposing a timeout on established connections such as WebSocket or gRPC.
Keeps the upstream client and its configuration scoped to each ProxyServer.
Removes the parent's proxy environment from jailed processes. Inheriting the parent's NO_PROXY would allow the child process to bypass httpjail's rule evaluation.

Proxy configuration is environment-only; no new command-line option is introduced. Behavior is unchanged when no upstream proxy is configured.

Usage

Route HTTPS requests through a corporate proxy:

HTTPS_PROXY=http://proxy.example.com:8080 \
  httpjail --js-file rules.js -- curl https://github.com

Route HTTP and HTTPS requests through the same proxy:

HTTP_PROXY=http://proxy.example.com:8080 \
HTTPS_PROXY=http://proxy.example.com:8080 \
  httpjail --js-file rules.js -- ./my-app

Configure proxy credentials:

HTTPS_PROXY=http://user:pass@proxy.example.com:8080 \
  httpjail --js-file rules.js -- curl https://github.com

Reach internal destinations directly:

HTTPS_PROXY=http://proxy.example.com:8080 \
NO_PROXY=api.internal.example,10.0.0.0/8 \
  httpjail --js-file rules.js -- ./my-app

A request bypassed by NO_PROXY is still evaluated by httpjail's rules.

Manual verification

The following is a sanitized transcript from manual testing. Proxy and internal
service host names have been replaced with reserved example domains.

Given rules allowing only github.com:

const DOMAINS = [
  "github.com",
];

(function () {
  const h = (r.host || "").toLowerCase();
  for (const d of DOMAINS) {
    if (d[0] === ".") {
      if (h === d.slice(1) || h.endsWith(d)) return true;
    } else if (h === d) {
      return true;
    }
  }
  return false;
})();

Allowed request

$ sudo env -u SUDO_UID -u SUDO_GID \
    HTTPS_PROXY=http://proxy.example.com:8080 \
    target/debug/httpjail -vv \
    --js-file rules.js \
    --request-log jail.log \
    curl -s -o /dev/null -w "%{http_code}\n" https://github.com
...
DEBUG httpjail: Routing httpjail upstream requests through the proxy environment
DEBUG httpjail::proxy: Upstream client initialized to route through the upstream proxy
...
200

Blocked request

$ sudo env -u SUDO_UID -u SUDO_GID \
    HTTPS_PROXY=http://proxy.example.com:8080 \
    target/debug/httpjail -vv \
    --js-file rules.js \
    --request-log jail.log \
    curl -s -o /dev/null -w "%{http_code}\n" https://example.com
...
403

`NO_PROXY` bypass

$ HTTPS_PROXY=http://proxy.example.com:8080 \
    NO_PROXY=api.internal.example \
    ./target/debug/httpjail --weak -vv --js "true" -- \
    curl -s -o /dev/null -w "%{http_code}\n" https://api.internal.example
...
DEBUG httpjail::upstream: Bypassing upstream proxy for api.internal.example
...
302

The 302 is the internal service's normal redirect response. Together with the
debug line, it confirms that httpjail contacted the service directly instead of
sending the request to the configured upstream proxy.

The jailed process does not inherit the parent's NO_PROXY value:

$ NO_PROXY=api.internal.example ./target/debug/httpjail --weak --js "true" -- \
    env | grep -i no_proxy
NO_PROXY=localhost,127.0.0.1,::1
no_proxy=localhost,127.0.0.1,::1

Test results

The following commands were run against the final branch and passed:

cargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings
cargo test --lib -- --test-threads=1
cargo test --test weak_integration
cargo test --test weak_integration_max_tx_bytes

Results:

71 unit tests passed.
14 weak-mode integration tests passed.
2 max_tx_bytes integration tests passed.

ammario · 2026-07-20T17:23:55Z

+By default httpjail contacts destination servers directly. When httpjail itself
+runs in an environment that has no direct internet access — for example behind a
+corporate proxy — you can route httpjail's own outbound requests through an
+upstream proxy with `--upstream-proxy` (or the `HTTPJAIL_UPSTREAM_PROXY`


Why shouldn't httpjail itself respect the standard HTTP_PROXY variables? I think it's clear on its face it wouldn't pass that down to children (as that would invalidate the whole point of the jail).

Agreed. For the first release, I’ll remove --upstream-proxy and HTTPJAIL_UPSTREAM_PROXY and use the standard HTTP_PROXY / HTTPS_PROXY environment variables for httpjail’s own egress.
Adding a dedicated CLI option and httpjail-specific env var creates an extra configuration path before we have a concrete need for it. If we later need an explicit per-invocation override, we can add it in a follow-up change.

Done in 701d116.

@ammario following up on 701d116, which drops --upstream-proxy / HTTPJAIL_UPSTREAM_PROXY in favor of the standard HTTP_PROXY / HTTPS_PROXY for httpjail's own egress.

One question: did you intend for the dedicated flag and env var to be removed entirely, or kept as an explicit override alongside HTTP_PROXY? I went with full removal, but it's easy to bring back either way. Let me know how this looks.

Remove the upstream proxy CLI option and httpjail-specific environment variable, and resolve httpjail's own upstream proxy from HTTP_PROXY and HTTPS_PROXY instead. Also keep the new upstream proxy initialization logs at debug level so normal CLI output is not affected.

ammar-agent · 2026-08-03T16:25:33Z

I reviewed this against the latest main (f66518b). The core approach—a Hyper connector that preserves streaming, pooling, and unbounded established connections—is reasonable, so I don't think the raw line count alone warrants a rewrite. I do think a few changes are needed before merging:

NO_PROXY / no_proxy is not honored. With HTTP_PROXY or HTTPS_PROXY set, every destination of that scheme is proxied, including internal hosts that a standard corporate proxy environment expects to connect to directly. Please either implement bypass matching or explicitly narrow the documented semantics.
Proxy configuration is silently process-global through HTTPS_CLIENT.get_or_init. If one ProxyServer initializes the client, a later call to the public new_with_upstream_proxies constructor can ignore its proxy argument. Keeping the client on ProxyServer / ProxyContext would make the configuration honest and remove hidden global state.
The most intricate path lacks an end-to-end test: HTTPS destination through an upstream CONNECT proxy, followed by destination TLS and an HTTP response. The existing tests separately cover plain HTTP proxying and the CONNECT exchange.
read_connect_status performs one async read per byte. That avoids swallowing tunnel payload, but is contrary to the project's minimal-latency goal. Please use buffered reads while preserving any bytes read beyond the CONNECT headers.

For complexity cleanup, the manual RunArgs Debug implementation appears to be residue from the removed dedicated proxy option and can return to a derive. Test-only/public helpers such as ProxyConnector::new and UpstreamProxies::all could also be narrowed. If TLS-to-proxy (https://proxy) is not a demonstrated requirement for the first release, deferring it would remove a meaningful portion of the stream-erasure and second-TLS-config machinery while still supporting HTTPS destinations through an ordinary HTTP proxy.

Local verification: formatting and cargo clippy --all-targets -- -D warnings pass; all unit tests pass. The full suite encountered the unchanged parallel fixed-port collision in weak_integration_max_tx_bytes.

— Codex, AI review agent

The manual Debug implementation was needed while RunArgs carried a dedicated upstream proxy option whose value could contain credentials. That option was removed in 701d116 in favor of the standard HTTP_PROXY / HTTPS_PROXY environment variables, so the implementation now enumerates every field in declaration order and produces output identical to the derive.

Reaching the upstream proxy itself over TLS (an `https://` proxy URL) was implemented but never a demonstrated requirement, and it was the only reason the connector had to erase the stream type behind `Box<dyn>` and carry a second rustls ClientConfig. HTTPS destinations are unaffected: they are still tunneled through a plain HTTP proxy with CONNECT, which is the standard corporate proxy configuration. `https://` proxy URLs are now rejected with an error naming the limitation rather than silently treated as plain HTTP, and ProxyStream holds a concrete TcpStream so established tunnels no longer pay for dynamic dispatch on every read and write.

The upstream client lived in a process-global OnceLock, so its configuration was decided by whichever ProxyServer was constructed first. A later call to new_with_upstream_proxies silently discarded its upstream_proxies argument, and get_client() could install a native-roots fallback client that no proxy configuration could ever replace. UpstreamClient::new now builds a client per server and ProxyContext carries it to the handlers, so what a caller passes is what its requests use. The public init_client_with_ca and get_client are gone along with the static, and proxy_request / proxy_https_request take the client from the context they already receive. This also removes the pre-initialization fallback path entirely: a context cannot exist without a client, so the case the warning covered is now unrepresentable.

Uri::host() returns an IPv6 literal with the square brackets that URI syntax requires around it, so `https://[::1]/` yields `[::1]`. host_port_authority() then saw a colon in the host and bracketed it a second time, making the connector emit `CONNECT [[::1]]:443` with a matching Host header. The authority is malformed, so an upstream proxy rejects it and IPv6 literal HTTPS destinations are unreachable. d08e61c fixed the same class of problem for the proxy's own address, which is parsed by the url crate and therefore arrives unbracketed; the destination side comes from Uri::host() and was left doubled. The existing test passed the bare "::1" directly and so did not exercise the Uri path. uri_host() now strips the brackets Uri::host() keeps, leaving host_port_authority() responsible for adding them back in authority position. The test builds its host from a Uri the way the connector does.

With HTTP_PROXY or HTTPS_PROXY set, every destination of that scheme was sent to the upstream proxy, including the internal hosts a corporate proxy environment expects to be reached directly. Such a proxy refuses those CONNECTs, so httpjail did not work in the environment the feature exists for. NO_PROXY now selects destinations to contact directly, following curl 8.14.1: domain entries match the domain and its subdomains at a label boundary, one leading and one trailing dot are ignored on both sides, a list of exactly `*` disables proxying, and IP or CIDR entries apply to destinations written as an IP literal. Domain entries and address entries never cross-match, as the destination decides which kind can apply. Entries are parsed once at startup so the request path only compares. Deliberate divergences, all documented: whitespace separates entries instead of truncating the list, `/0` covers its address family, a mistyped CIDR is a startup error rather than silently ignored, non-byte-aligned IPv6 prefixes match correctly (curl fixed the same defect in 8.17.0), and the uppercase spelling keeps precedence for consistency with the other proxy variables. Two properties the implementation is shaped around: Proxy-Authorization is now decided by http_auth_for_uri(), which is the only path the request builder uses. Attaching the header from the caller would have leaked the proxy's credentials to any bypassed internal host, and to the origin server itself for HTTPS destinations, whose request travels inside the CONNECT tunnel. from_env() never parses an input that cannot affect the outcome: a wildcard bypass short-circuits before the proxy URLs are read, and the bypass list is left unparsed when no proxy is configured. Otherwise a leftover NO_PROXY in an environment with no proxy at all would newly refuse to start. Errors and logs name an entry by position and never echo its text, since a NO_PROXY value can hold a mistakenly pasted proxy URL with credentials and redact_proxy_spec does not cover text after a slash.

Weak mode merged the parent's NO_PROXY into the value it set for the jailed process. Every host that value named was then reached directly by that process, skipping httpjail and any rule evaluation — the jail's purpose, silently defeated by an environment variable. The hole predates the upstream proxy work, but that work is what gives NO_PROXY a reason to be set, so it turns a latent problem into a likely one. The parent's NO_PROXY is no longer merged; the jailed process gets only localhost, 127.0.0.1 and ::1, which exist to keep local traffic from looping back through httpjail. ALL_PROXY was never stripped anywhere. It can carry the upstream proxy's credentials, and a process that prefers it over the scheme-specific variables would talk to that proxy instead of to httpjail. It is now removed alongside HTTP_PROXY and HTTPS_PROXY in weak mode, native Linux jails and Docker. All of these are handled in both spellings. Removing only the uppercase one leaves the hole open, and for NO_PROXY the lowercase spelling is the dangerous one: curl reads it first. The three call sites now share PARENT_PROXY_ENV_VARS and remove_parent_proxy_env() so the list cannot drift apart again. The weak mode test sets both spellings of each variable to different values on the parent, so a leak through either one fails and names itself; asserting only on the uppercase spelling would have passed while the lowercase one leaked.

curl decides how to read a NO_PROXY entry from what the destination is, not from what the entry looks like. When the destination is a host name every entry — including a bare address — goes through the domain suffix match, so `NO_PROXY=127.0.0.1` also covers `foo.127.0.0.1` and `127.0.0.1.`; the latter is the plausible one, since a trailing dot stops the destination from parsing as an address. Classifying by the entry instead sent both of those through the proxy. The entry now keeps its original text alongside the parsed address and offers it as a domain candidate, which restores curl's behavior without reclassifying anything per request. Label boundaries still apply, so `x127.0.0.1` does not match. An IPv6 entry carries its text as well and simply never matches on that path, because a host name cannot contain a colon — the same outcome curl reaches by comparing the token as a string.

read_connect_status awaited one read per byte so that it could stop exactly at the end of the headers: reading further would have consumed tunnel data with nowhere to put it. A short "HTTP/1.1 200 Connection established" answer therefore cost around forty awaits and syscalls, against the project's minimal-latency goal. The response is now read in 1 KiB chunks and the bytes that overshoot the headers are carried on the connection instead of being dropped. ProxyStream replays them before it touches the socket, so the destination TLS handshake sees exactly the byte order it would have seen before. Details worth keeping: - The header terminator is searched from three bytes before the newly read data, so a \r\n\r\n split across two reads is still found. - The size cap is applied to the header end, not to everything buffered. A single read can legitimately return headers within the cap plus tunnel data that pushes the total past it, and that is a success. - Only the headers are decoded as UTF-8. What follows is a TLS ClientHello and arbitrary binary. - BytesMut reports nearly unbounded space, so each read is capped with BufMut::limit rather than letting the buffer size the read. - One timeout still covers the whole exchange rather than each read, or a proxy dribbling out a byte at a time would never trip a deadline.

The CONNECT exchange and the plain-HTTP forwarding path each had tests, but nothing exercised them composed: an HTTPS destination reached by opening a tunnel through the proxy, running the destination TLS handshake inside it, and getting a response back. That is the most intricate path in the feature and it was the one with no coverage. The test stands up a real TLS origin with a self-signed certificate and a proxy that answers CONNECT and then splices the two sockets, so the handshake has to succeed over the tunnel for the request to arrive at all. It asserts the CONNECT names the destination authority rather than the proxy's own address, and that the origin receives the expected path. One timeout covers the request, the body collection and the two task joins together. Bounding only the request would let a regression that stalls after the response headers, or one that leaves the tunnel copy running, hang the suite instead of failing it. Verified to have teeth: skipping the CONNECT fails in three seconds rather than hanging on the thirty second upstream setup timeout, and a stall after the headers trips the same bound. Marking the tunnel proxied, which the test cannot detect, is not a gap this test should try to close: hyper-util's absolute_form() falls back to origin-form for HTTPS URIs on its own, so the flag cannot change what the origin sees. The comment says so rather than claiming coverage it does not have. Everything binds to port 0 and no external network, DNS, sudo or Docker is involved, so it runs in parallel with the rest of the suite.

ProxyConnector::new and UpstreamProxies::all were public but never reached from production code: new() only wrapped all(), and all() only existed to feed new(). Both were left over from before from_specs() became the way a configuration is built, and they advertised a second construction path that nothing supports. new() is gone, with_config() is now pub(crate) since its only caller is UpstreamClient::new in this crate, and all() is #[cfg(test)] so it does not appear in a normal build at all. The two upstream proxy tests build their connector the same way now, through with_config. No behavior or public documentation changes.

Revert the follow-up work after 7301b5a so the review feedback can be reapplied as three focused commits.

Resolve HTTP and HTTPS upstream proxies from the standard environment, keep the upstream client scoped to each ProxyServer, and defer TLS connections to the proxy itself.

Route matching domains and IP networks directly while keeping rule evaluation in place. Strip the parent proxy environment from jailed processes so they cannot bypass httpjail or reuse upstream credentials.

Read CONNECT responses in bounded chunks while replaying tunnel bytes read past the headers. Add an end-to-end HTTPS tunnel test and keep Proxy-Authorization out of debug output.

eraow · 2026-09-08T11:25:37Z

Hi, @ammario, @ammar-agent,

I cleaned up the post-review history in this branch.
I reverted the commits which have been pushed since you reviewed in a single commit, then reapplied the necessary changes as three focused commits:
These 3 commits address your last feedback.

c51d2bb: scopes the upstream client to each server, restores derived Debug, and defers TLS-to-proxy support.
c0f25ba: implements NO_PROXY and prevents the parent proxy environment from reaching jailed processes.
326d51a: adds buffered CONNECT response handling, preserves read-ahead bytes, narrows test-only APIs, and adds an end-to-end test for HTTPS through an upstream CONNECT proxy.

I also reran formatting, Clippy, unit tests, and the weak-mode integration tests successfully.
Could you please take another look?

eraow added 6 commits July 10, 2026 21:07

Add upstream proxy feature

4b3a9a6

fix: redact upstream proxy credentials

a28754e

fix: time out upstream proxy TCP connects

1bd3203

fix: support IPv6 upstream proxy addresses

d08e61c

refactor: use libraries for proxy credential encoding

3d11be8

refactor: parse upstream proxy URLs with url crate

7301b5a

eraow marked this pull request as ready for review July 13, 2026 14:24

ammario requested changes Jul 20, 2026

View reviewed changes

eraow requested a review from ammario July 28, 2026 06:33

eraow added 7 commits August 7, 2026 17:37

eraow marked this pull request as draft August 8, 2026 03:45

eraow added 9 commits August 8, 2026 03:49

docs: clarify weak-mode proxy credential exposure

5f15d3a

test: isolate max-tx proxy listeners

a4f8276

fix: time out destination TLS setup

604ba13

fix: accept binary CONNECT header values

d858936

fix: preserve proxy credential bytes

9621c81

fix: mark proxy authorization sensitive

f235947

This comment was marked as outdated.

Sign in to view

eraow marked this pull request as ready for review August 9, 2026 13:08

eraow added 4 commits September 8, 2026 09:33

revert: replace post-review upstream proxy changes

4581fb2

Revert the follow-up work after 7301b5a so the review feedback can be reapplied as three focused commits.

feat: use standard proxy environment

c51d2bb

Resolve HTTP and HTTPS upstream proxies from the standard environment, keep the upstream client scoped to each ProxyServer, and defer TLS connections to the proxy itself.

feat: honor NO_PROXY without exposing parent settings

c0f25ba

Route matching domains and IP networks directly while keeping rule evaluation in place. Strip the parent proxy environment from jailed processes so they cannot bypass httpjail or reuse upstream credentials.

fix: stream and test CONNECT tunnels

326d51a

Read CONNECT responses in bounded chunks while replaying tunnel bytes read past the headers. Add an end-to-end HTTPS tunnel test and keep Proxy-Authorization out of debug output.

Navigation Menu

Add upstream proxy - #104

Add upstream proxy #104
eraow wants to merge 27 commits into
coder:mainfrom
eraow:feature/upstream-proxy

eraow commented Jul 13, 2026 •

edited

Loading

Uh oh!

ammario Jul 20, 2026

Uh oh!

eraow Jul 21, 2026

Uh oh!

eraow Jul 21, 2026

Uh oh!

eraow Aug 1, 2026 •

edited

Loading

Uh oh!

ammar-agent commented Aug 3, 2026

Uh oh!

This comment was marked as outdated.

eraow commented Sep 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

eraow commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Behavior

Usage

Manual verification

Allowed request

Blocked request

NO_PROXY bypass

Test results

Uh oh!

ammario Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

eraow Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

eraow Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

eraow Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ammar-agent commented Aug 3, 2026

Uh oh!

This comment was marked as outdated.

eraow commented Sep 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eraow commented Jul 13, 2026 •

edited

Loading

`NO_PROXY` bypass

eraow Aug 1, 2026 •

edited

Loading