feat: explicit proxy configuration for the CLI

[NickJosevski]

Refs #49

Baseline: standard proxy env vars already work

Before adding anything, I checked whether the CLI loses Go's built-in proxy support. It does not, for normal API traffic.

• pkg/apiclient/client_factory.go:129 builds the http client with NewSpinnerRoundTripper(ask)
• pkg/apiclient/spinner_round_tripper.go:19 sets Next: http.DefaultTransport
• http.DefaultTransport.Proxy is http.ProxyFromEnvironment

So HTTP_PROXY, HTTPS_PROXY and NO_PROXY have always been honoured for every Octopus API call. If that is all a customer needs, no CLI change was ever required. That reframes the issue: this is not "add proxy support", it is "add explicit configuration and close two gaps".

The two real gaps

1. octopus login --ignore-ssl-errors lost the proxy. pkg/cmd/login/login.go:131 built a bare &http.Transport{}, whose Proxy field is nil — proxy support silently gone for exactly the command a new user runs first.
2. The same line panicked. httpClient.Transport.(*http.Transport) is an unchecked type assertion. When the CLI is already configured, f.GetHttpClient() returns the client whose transport is a *SpinnerRoundTripper, so octopus login --ignore-ssl-errors crashed with interface conversion. Reproducible before this change; covered by a test now.

What changed

New pkg/apiclient/proxy.go:

• ProxySettings + ProxySettingsFromConfig() — reads the config/env
• ProxyFunc() — resolution built on golang.org/x/net/http/httpproxy (the same package net/http uses), so NO_PROXY semantics match the standard library exactly
• NewHttpTransport(settings, insecureSkipVerify) — clones http.DefaultTransport instead of mutating it, keeping every standard default (including proxy) and no longer poisoning the process-wide transport
• RedactProxyUrl() — for display

Wiring: OCTOPUS_PROXY env var + ProxyUrl config key (both were already stubbed out in commented-out code across constants.go, config.go, config get, config set — this uncomments and completes them), plus config list support with redaction, and a fixed login path.

Precedence

	source	notes
1	OCTOPUS_PROXY	via viper's env binding
2	ProxyUrl in cli_config.json	octopus config set ProxyUrl ...
3	HTTPS_PROXY / HTTP_PROXY	standard behaviour, unchanged

An explicit OCTOPUS_PROXY/ProxyUrl applies to both http and https requests (it replaces both env vars). NO_PROXY is honoured in every case, including over an explicit setting. Loopback targets are never proxied (standard Go behaviour, and what you want against a local Octopus).

Credentials

user:pass@host in the url works. Separately, OCTOPUS_PROXY_USERNAME / OCTOPUS_PROXY_PASSWORD apply to whichever proxy url was resolved — including one from HTTPS_PROXY — and lose to credentials already in the url.

Deliberately environment-only: they are read with os.Getenv, not bound into viper, so they cannot be persisted to cli_config.json in plain text. config list redacts any password in ProxyUrl via url.Redacted() (http://octo:xxxxx@proxy:3128), matching how ApiKey/AccessToken are already masked at pkg/cmd/config/list/list.go:38-44. The password is never logged or echoed.

Out of scope, with reasons

• NTLM — not supported. Go has no stdlib NTLM/Negotiate; http.Transport only does Basic proxy auth. It would mean a third-party dependency (e.g. Azure/go-ntlmssp) doing a 3-leg handshake with connection affinity, plus SSPI for transparent single-sign-on on Windows. Real work, a supply-chain decision, and no test story without a Windows domain. Recommend a separate issue, driven by an actual customer request.
• SOCKS — free, and included. net/http's transport dials socks5:// and socks5h:// proxy urls itself (socks_bundle.go, transport.go:1835). No extra dependency, no extra code. OCTOPUS_PROXY=socks5://host:1080 works and is covered by a test.

Test evidence

go build ./... clean. go test ./pkg/... all green (go vet reports 4 pre-existing "unreachable code" hits in unrelated files).

• pkg/apiclient/proxy_test.go — 14-case table over proxy resolution: no config, HTTP_PROXY/HTTPS_PROXY per scheme, explicit config overriding env, scheme-less host:port, socks5, NO_PROXY against both explicit and env proxies, loopback, and the three credential paths. Plus an invalid-url error case, ProxySettingsFromConfig, and a RedactProxyUrl table asserting the password never survives.
• End-to-end through a real proxy, no network or Docker: TestNewHttpTransport_SendsRequestsThroughTheProxy stands up an httptest server as the proxy and asserts the absolute-form request URI and the Proxy-Authorization: Basic header arrive at it.
• TestNewHttpTransport_LeavesTheDefaultTransportAlone guards the shared-transport mutation regression.
• pkg/cmd/login/login_test.go — TestConfigureHttpClient covers all three branches, including the one that used to panic.
• pkg/config/config_test.go — proves OCTOPUS_PROXY is actually bound to ProxyUrl.

Every test is hermetic; clearProxyEnvironment stops the CI machine's own proxy settings leaking in.

Open questions / options

1. Is an explicit setting wanted at all, or is env-only enough?
Since HTTPS_PROXY already worked, OCTOPUS_PROXY buys one thing: pointing the CLI at a proxy without redirecting every other tool on the box. That is genuinely useful in CI, but it is new surface to document and support. Recommend keeping it — it is the thing the issue actually asks for, and it is cheap.

2. No --proxy flag, and there is a concrete reason.
--proxy is already taken: pkg/machinescommon/proxy.go:15 registers it on target ssh create, target listening-tentacle create and the worker equivalents, where it names an Octopus proxy resource. A root persistent --proxy would be shadowed by the local flag on exactly those commands — confusing for two different meanings of the word. Second obstacle: the client factory is built in cmd/octopus/main.go:53, before cobra parses flags (the same ordering the spinner round-tripper comments call out), so a flag needs either lazy per-request resolution or a reordering. Options: (a) ship env/config only — my recommendation for this PR; (b) add --proxy-url with lazy resolution, ~10 lines on top of this; (c) reorder factory construction. Happy to do (b) if the team wants a flag.

3. Credential env var names. The issue says PROXY_USERNAME/PROXY_PASSWORD; I used OCTOPUS_PROXY_USERNAME/OCTOPUS_PROXY_PASSWORD to match every other OCTOPUS_* var. Unprefixed names risk colliding with other tooling. Easy to also accept the unprefixed names as a fallback if there is a compatibility reason.

4. Should ProxyUrl accept credentials at all? It can today, and config list redacts it — but the password still sits in cli_config.json in plain text, same as ApiKey does. Alternative: reject a url containing a password on config set and force the env vars. Slightly more secure, slightly more annoying. Want that?

5. Test matrix — what squid in Docker would add. The unit tests cover resolution and one real proxy hop, but not: CONNECT tunnelling for https targets (the httptest proxy sees an absolute URI, not a CONNECT), a 407 challenge/response round, proxies that mangle or buffer chunked responses, and TLS-terminating proxies with a corporate root CA. A squid container in CI would cover the first three; the fourth needs a generated CA and is where real customer pain usually lives. Suggest one squid-based integration test (anonymous + basic-auth) in the existing integration suite, kept out of the unit run. Worth noting the integration suite has its own CI problems today, so I did not add anything that depends on it.

6. Unrelated but worth flagging: pkg/apiclient/client_factory.go:124 (before this change) set InsecureSkipVerify: true unconditionally on the global http.DefaultTransport — the CLI never verifies Octopus's TLS certificate, and login --ignore-ssl-errors is effectively always on. I preserved the behaviour rather than change it in a proxy PR (it is now scoped to the CLI's own transport instead of the whole process), but it looks like a security bug and deserves its own issue.

🤖 Generated with Claude Code