[release] Add ProxyConfig egress-proxy resolver to lib-util-net (0.2.0)

[pditommaso]

Summary

Adds io.seqera.util.net.ProxyConfig to lib-util-net (bumped 0.1.0 → 0.2.0): a JDK-only resolver for an HTTP/HTTPS forward (egress) proxy — including an authenticating one — from a proxy URI or from the HTTP_PROXY/HTTPS_PROXY/NO_PROXY environment variables, exposed as a java.net.ProxySelector and a proxy-scoped java.net.Authenticator suitable for a java.net.http.HttpClient.

Parsing, credential-decoding, no-proxy and Basic-over-CONNECT semantics mirror Nextflow's nextflow.util.ProxyConfig (the source of truth), so products sharing this class share the same proxy behaviour. It depends only on the JDK plus the slf4j-api logging facade already used by the module — the caller passes the URI or environment map in; the library never reads System.getenv() on its own.

Motivation

The same egress-proxy resolution is currently hand-rolled in Nextflow (nextflow.util.ProxyConfig) and was about to be duplicated a third time in Wave. lib-httpx's HxProxyConfig deliberately omits URI/env parsing ("the library never reads the environment or system properties on its own"), which is exactly the duplicated part. Promoting it here lets Wave (and, subsequently, Nextflow) delete their own copies and depend on one implementation.

API

ProxyConfig proxy = ProxyConfig.fromUri("http://user:pass@proxy.example.com:3128");
// or: ProxyConfig.fromEnvironment(System.getenv());
if (proxy != null) {
    var builder = HttpClient.newBuilder().proxy(proxy.toProxySelector());
    var auth = proxy.toAuthenticator();          // null when no credentials
    if (auth != null) builder.authenticator(auth);
    if (proxy.hasCredentials()) ProxyConfig.enableBasicProxyTunneling();
    HttpClient client = builder.build();
}

• fromUri(uri[, username, password, noProxy]) / fromEnvironment(env) / parse(...)
• toProxySelector() — routes per scheme, bypassing loopback and NO_PROXY targets (bare host matches sub-domains; ./*. suffix matches sub-domains only; * disables; loopback always bypassed)
• toAuthenticator() — releases credentials only for RequestorType.PROXY challenges matching the proxy host+port; matching on host+port (not protocol) inherently covers the HTTPS CONNECT tunnel where the JDK reports the requesting protocol as http
• enableBasicProxyTunneling() — clears jdk.http.auth.tunneling.disabledSchemes when unset (an operator value wins)

Tests

ProxyConfigTest (40 cases, TDD): URI/credential parsing (percent-encoding, + preservation), per-protocol env resolution (upper/lower case, ALL_PROXY fallback), no-proxy matching (suffix/wildcard/loopback), authenticator scoping (requestor type, host, port, HTTPS-tunnel), and the tunnelling toggle. Existing SsrfValidator tests unaffected. ./gradlew :lib-util-net:build green.

Notes

• No new dependencies (JDK + slf4j-api only).
• Follow-ups (separate PRs): Wave consumes this and deletes its duplicate; Nextflow migrates nextflow.util.ProxyConfig to delegate.

🤖 Generated with Claude Code

[pditommaso]

@claude review

[claude]

Claude finished @pditommaso's task in 2m 33s —— View job

---

Review — PR #122 [release] Add ProxyConfig egress-proxy resolver to lib-util-net (0.2.0)

• Gather context (diff, changed files)
• Review ProxyConfig.java
• Review tests, README, VERSION, changelog
• Post review

Verdict: comment — no blocking defect found, but one scope/design question I think a human should own.

The implementation is careful and reads well: immutable, List.copyOf on the no-proxy list, credentials never rendered in toString(), authenticator correctly scoped to RequestorType.PROXY + host/port (which is the right way to cover the CONNECT tunnel), and enableBasicProxyTunneling() deliberately yielding to an operator-set value. No security, correctness or data-loss issue found. Version bump, changelog and README are all present and consistent.

---

Why I'm not approving: the duplication this PR is meant to remove already exists in this repo

ProxyConfig and lib-httpx's HxProxyConfig are now near-identical for everything except the parsing:

	ProxyConfig	HxProxyConfig
Endpoint / ProxyEntry	ProxyConfig.java:62-69	HxProxyConfig.java:74-94
toProxySelector()	ProxyConfig.java:155-176	HxProxyConfig.java:157-179
toAuthenticator()	ProxyConfig.java:188-203	HxProxyConfig.java:190-205
isBypassed()	ProxyConfig.java:212-234	HxProxyConfig.java:227-249
credentialsFor()	ProxyConfig.java:236-242	HxProxyConfig.java:251-257

Those bodies are character-for-character the same (down to the // precompute the proxy lists - select() runs once per outbound request comment). The PR's stated motivation is "was about to be duplicated a third time" — but the diff as it stands adds a third in-repo copy rather than removing one. The natural shape is lib-httpx depending on lib-util-net with HxProxyConfig reduced to a builder that produces a ProxyConfig (or deprecated in favour of it), leaving the no-proxy/authenticator semantics in exactly one place.

That's a call about which class is canonical across lib-httpx / Wave / Nextflow, not a defect in this code, so I'm leaving it to you rather than blocking. If the plan is a follow-up PR that collapses HxProxyConfig onto this, saying so in the description (or a @see/TODO on the class) is enough for me. Discuss with Claude →

---

Non-blocking findings

1. A non-HTTP proxy scheme throws instead of being ignored — ProxyConfig.java:284-303

new URL("socks5://proxy:1080") throws MalformedURLException (unknown protocol), which parse() converts to IllegalArgumentException. So a caller doing ProxyConfig.fromEnvironment(System.getenv()) in an environment that sets ALL_PROXY=socks5://host:1080 (a common shape — curl and many corp setups use it) gets an exception at startup rather than "no HTTP proxy configured". Given fromEnvironment is explicitly designed to be fed untrusted ambient environment, failing soft — log a warning and treat an unsupported scheme as absent — seems friendlier than failing the process. Throwing from the explicit fromUri(...) path is fine.

2. Username-only user-info is silently dropped — ProxyConfig.java:287-292

final int p = info != null ? info.indexOf(':') : -1;
if( p != -1 ) { user = ...; pass = ...; }

For http://token@proxy:3128 (a passwordless token/username proxy), getUserInfo() returns token, p == -1, and both user and pass stay null — the credential vanishes with no warning and requests then fail with 407. Handling p == -1 as username-with-empty-password would cover it. (I appreciate this mirrors Nextflow; worth fixing in both if you agree.)

3. An https:// proxy scheme is accepted but not honoured — ProxyConfig.java:111, ProxyConfig.java:158-163

https://proxy:443 gets the right default port, but the selector always builds Proxy.Type.HTTP, and the JDK HttpClient has no TLS-to-proxy support anyway — so the client speaks plaintext to a TLS-only proxy and fails obscurely. A log.warn when the scheme is https would turn a confusing connect failure into an actionable message. The test at ProxyConfigTest.groovy:87 currently locks in the silent behaviour.

4. fromEnvironment(null) NPEs — ProxyConfig.java:354-360

firstNonEmpty dereferences env directly. Since the whole point is that the caller supplies the map, a null map is a plausible caller slip; returning null (no proxy) would match fromUri(null).

5. Parsed leaks the password through its record toString() — ProxyConfig.java:270

ProxyConfig.toString() is properly redacted (describe() at line 368-371), but the package-private Parsed record uses the default record toString(), which includes the raw password. Nothing logs it today; a compact toString override on Parsed would keep it that way as the class evolves.

6. README section nesting — lib-util-net/README.md

### Egress proxy configuration is appended after the ## Limitations heading, so the whole proxy section renders as a sub-section of the SSRF limitations. Moving it up next to ### SSRF host validation under ## Usage would read correctly.

7. Test coverage gaps (optional): no case for an unsupported scheme (finding 1), username-only user-info (finding 2), a bare IPv6 proxy address, or a NO_PROXY entry carrying a port (example.com:8080 — currently never matches). The global-system-property mutation in the tunnelling test (ProxyConfigTest.groovy:217-238) is correctly cleaned up, though it will be order-sensitive if the suite ever runs in parallel within a JVM.

---

Note

I could not run ./gradlew :lib-util-net:test in this environment (the command needed an approval I don't have), so the above is a static review; the PR reports the module build green. I also can't submit a formal GitHub review or approval from this action — FAQ. Findings 1–7 are all non-blocking; the only thing I'd like an answer on is the HxProxyConfig question above.
· branch claude/lib-util-net-proxy-config

[pditommaso]

Thanks for the review — addressed the findings:

• 1 — unsupported scheme in the env — fromEnvironment is now lenient: an unsupported scheme (e.g. socks5://) or malformed value is logged and treated as absent instead of throwing; fromUri still throws.
• 2 — username-only user-info — parse now keeps a token@proxy username (empty password) instead of dropping the credential. Flows into Nextflow too, since it delegates to this parse.
• 3 — https:// proxy scheme — now warns that TLS-to-proxy is unsupported (plaintext will be used).
• 4 — fromEnvironment(null) NPE — fromEnvironment/setupFromEnvironment return null for a null map.
• 5 — Parsed password leak — Parsed.toString() now redacts the password.
• 6 — README nesting — the egress-proxy section moved under ## Usage.

Added tests for the unsupported-scheme, username-only and null-map cases (54 green).

On the HxProxyConfig duplication (the main question): agreed — collapsing it is the right call. Done in #123 (stacked on this): lib-httpx depends on lib-util-net, HxProxyConfig is removed, and HxClient.withProxyConfig takes io.seqera.util.net.ProxyConfig directly, so the selector/authenticator/no-proxy semantics live only here. Nextflow (the only caller) is migrated alongside it.

@claude review

[claude]

Claude finished @pditommaso's task in 3m 49s —— View job

---

Re-review — PR #122 [release] Add ProxyConfig egress-proxy resolver to lib-util-net (0.2.0)

• Pull latest branch state (a11a233)
• Verify findings 1–6 from the previous round
• Review the newly added setupFromEnvironment surface
• Run :lib-util-net:test — not possible, ./gradlew needs an approval this job doesn't have (static review only)
• Post review

Verdict: request changes — two concrete defects in the new setupFromEnvironment / lenient-parse code added since the last round. Everything from the previous review is properly addressed.

---

Previous findings — all confirmed fixed

#	Finding	Where	✓
1	unsupported scheme throws from env	parseLenient ProxyConfig.java:355-363, wired at :131-132	✅
2	username-only user-info dropped	ProxyConfig.java:394-401	✅
3	https:// proxy silently unhonoured	warnIfTlsProxy ProxyConfig.java:369-373	✅
4	fromEnvironment(null) NPE	ProxyConfig.java:127-128, :308-309	✅
5	Parsed leaks password via toString()	ProxyConfig.java:342-347	✅
6	README nesting	now under ## Usage	✅

And the HxProxyConfig duplication question is answered by #123 (lib-httpx 3.0.0 removes it) — that was my only reason for withholding last time, so it's off the table.

---

Blocking

B1. parseLenient writes the proxy password to the log — ProxyConfig.java:355-363

catch( IllegalArgumentException e ) {
    log.warn("Ignoring unsupported or invalid proxy value '{}': {}", value, e.getMessage());

value is the raw env value, credentials included — and e.getMessage() is "Invalid proxy URL: " + value (:412), so the secret lands in the log twice. This is exactly the path finding 1 asked for: ALL_PROXY=socks5://user:s3cret@host:1080 is a realistic corp/curl-style setting, and it now logs s3cret at WARN. Reachable from a plain % in the password too (URLDecoder throws IllegalArgumentException).

Everything else in the class is careful about this (describe() :478-481, Parsed.toString() :342-347), so this is the one hole. Redact the userinfo before logging — and consider building the IllegalArgumentException message at :412 from the redacted form, since callers of fromUri will log that exception too.

Fix this →

B2. NO_PROXY → http.nonProxyHosts translation is wrong in two ways — ProxyConfig.java:314-316

System.setProperty("http.nonProxyHosts", String.join("|", split(noProxy)));

(a) it drops the JDK's default loopback bypass. http.nonProxyHosts defaults to localhost|127.*|[::1]|0.0.0.0|[::0]; setting the property replaces that default, it doesn't extend it. So after setupFromEnvironment([NO_PROXY: 'internal.example.com']), JVM-global code (URLConnection, FTP, anything on the default ProxySelector) sends http://localhost:8080/... through the corporate proxy — while isBypassed() :236 and the returned toProxySelector() correctly keep bypassing it. The two halves of the same config object disagree about loopback, and only the JVM-global half is wrong.

(b) .suffix entries never match. The JDK's nonProxyHosts grammar is exact-match plus a * wildcard — *.corp works, .corp matches nothing. And a bare example.com means "host and sub-domains" to isBypassed() :248 but "that exact host only" to the JDK. So the test at ProxyConfigTest.groovy:295 asserts 'internal.example.com|.corp', which locks in a value where .corp is dead and internal.example.com's sub-domains silently go through the proxy. Internal traffic that the operator asked to bypass gets forwarded to (and its hostnames disclosed to) the proxy instead.

Suggested mapping, applied per entry: .x / *.x → *.x; bare x → x|*.x; * → *; and prefix the whole thing with localhost|127.*|[::1]|0.0.0.0|[::0].

Fix this →

---

Non-blocking

N1. HTTPS_PROXY port default is derived from the target protocol, not the proxy scheme — ProxyConfig.java:139 vs :111

fromUri("http://proxy.corp") → port 80 (scheme-derived, :111), but fromEnvironment([HTTPS_PROXY: 'http://proxy.corp']) → port 443 (:139), so the client speaks plaintext to port 443 of a proxy that listens on 80. The two factories should agree; portAsInt(https.port(), "https".equalsIgnoreCase(https.protocol()) ? 443 : 80) matches the fromUri rule. The existing case at ProxyConfigTest.groovy:160 uses https://proxy1 so it doesn't distinguish the two.

N2. An invalid value is warned about twice — applyProxySystemProperty :327 and fromEnvironment :131-132 both call parseLenient on the same string, so setupFromEnvironment logs each bad value 2–3 times (and, until B1 is fixed, leaks the password that many times).

N3. Endpoint's methods aren't reachable by callers — hasCredentials() :63 and address() :66 are package-private on a public record returned by the public getHttpProxy()/getHttpsProxy(). Consumers in other packages get the components but not the helpers; making them public costs nothing.

N4. setupFromEnvironment can mutate and still return null — with only FTP_PROXY set, :313 installs ftp.proxyHost/ftp.proxyPort and then fromEnvironment returns null. Fine behaviourally, but the javadoc at :303-305 and the test name at ProxyConfigTest.groovy:309 ("touch nothing") oversell it. Also ftp.nonProxyHosts is never set, so the FTP handler ignores NO_PROXY entirely (http.nonProxyHosts covers http and https only).

N5. IPv6 proxy addresses (unchanged from last round) — [::1]:3128 without a scheme splits on the first : at :406-408, yielding host [. With a scheme, getHost() keeps the brackets and they're passed straight into createUnresolved. Also a NO_PROXY entry carrying a port (example.com:8080) can never match. Both mirror Nextflow; worth a note in the javadoc if they're not going to be handled.

N6. Test isolation — ProxyConfigTest.groovy:272, :314, :332, :356 mutate JVM-global system properties and Authenticator.setDefault. Cleanup is correct, but these will interfere with each other if the suite ever runs in parallel within one JVM; @Isolated/@Stepwise or a @ResourceLock would make that explicit.

---

Everything else looks right

Immutability and List.copyOf on the no-proxy list, authenticator scoped to RequestorType.PROXY + host/port (the correct way to cover the CONNECT tunnel, and the test at :237 pins it), the precomputed selector lists, enableBasicProxyTunneling() yielding to an operator-set value with the matrix at :332 covering all three states, and redacted toString(). VERSION, changelog.txt and README.md are consistent with the added API — the changelog even calls out setupFromEnvironment, which the PR description doesn't yet mention (worth adding, since it's a second public entry point with JVM-global side effects).

I could not execute the test suite in this job, so the above is static analysis; the two blocking items are localized and both are covered by a small test each.
· branch claude/lib-util-net-proxy-config

[claude]

Re-review: findings 1-6 from the previous round are all confirmed fixed, and #123 answers the HxProxyConfig duplication question.

Two defects in the code added since then block:

B1 - the proxy password is written to the log. parseLenient (ProxyConfig.java:355-363) logs the raw env value plus e.getMessage(), which is "Invalid proxy URL: " + value (:412) - so ALL_PROXY=socks5://user:s3cret@host:1080, exactly the case the lenient path was added for, logs the secret twice at WARN. Redact the user-info before logging, and build the exception message from the redacted form.

B2 - the NO_PROXY -> http.nonProxyHosts translation is wrong (ProxyConfig.java:314-316). (a) Setting the property replaces the JDK default localhost|127.*|[::1]|0.0.0.0|[::0], so after setupFromEnvironment JVM-global code sends loopback traffic through the proxy while isBypassed()/toProxySelector() still bypass it. (b) JDK nonProxyHosts is exact-match plus * wildcards: a .corp entry never matches, and a bare example.com does not cover sub-domains the way isBypassed() does - so internal traffic the operator asked to bypass is forwarded to the proxy. ProxyConfigTest.groovy:295 currently locks the broken value in. Suggested mapping: .x/*.x -> *.x, bare x -> x|*.x, always prefixed with the loopback defaults.

Non-blocking items (HTTPS_PROXY port default derived from the target protocol rather than the proxy scheme at :139 vs :111, duplicated warn, package-private Endpoint helpers, ftp.nonProxyHosts, IPv6 addresses, test isolation) are detailed in #122 (comment).

Note: I could not run ./gradlew :lib-util-net:test in this job, so this is a static review.

[pditommaso]

Both blocking items fixed, plus the non-blocking ones:

• B1 — credential leak in logs/errors — added redactUserInfo, used in the parseLenient warning and the IllegalArgumentException message, so a value like ALL_PROXY=socks5://user:s3cret@host:1080 now surfaces as socks5://****@host:1080. Test asserts the thrown message contains no password.
• B2 — NO_PROXY → http.nonProxyHosts — new toNonProxyHosts maps to the JDK grammar: prepends the loopback defaults it would otherwise replace (localhost|127.*|[::1]|0.0.0.0|[::0]), expands a bare example.com to example.com|*.example.com (matching isBypassed), and maps .corp/*.corp → *.corp. Also sets ftp.nonProxyHosts. Test assertion updated.
• N1 — fromEnvironment now defaults the port from the proxy scheme (http→80, https→443), matching fromUri; HTTPS_PROXY=http://proxy → 80.
• N3 — Endpoint.hasCredentials()/address() made public.
• N4/N5 — javadoc notes: ftp is system-property-only (so setupFromEnvironment may return null after installing ftp props); IPv6-without-scheme and NO_PROXY-with-port limitations.
• N6 — @ResourceLock on the JVM-global-mutating tests.

56 tests green. N2 (duplicate warning line for one bad value) left as-is — cosmetic, and no longer leaks after B1.

The HxProxyConfig removal is #123 (rebased on this, so it carries these fixes too).

@claude review

[claude]

Claude finished @pditommaso's task in 2m 32s —— View job

---

Re-review — PR #122 [release] Add ProxyConfig egress-proxy resolver to lib-util-net (0.2.0)

• Pull latest branch state (0b7c797)
• Verify B1 (credential leak) and B2 (NO_PROXY → nonProxyHosts)
• Verify N1/N3/N4/N5/N6
• Attempt ./gradlew :lib-util-net:test — blocked again (needs an approval this job lacks), so static review
• Submit review

Verdict: approve. Both blocking items are properly fixed, and nothing new is blocking.

---

Blocking items — confirmed fixed

B1 — credential leak. redactUserInfo (ProxyConfig.java:453-455) is applied both in the parseLenient warning (:388) and in the IllegalArgumentException message (:445), so ALL_PROXY=socks5://user:s3cret@host:1080 now surfaces as socks5://****@host:1080 in the log and in the exception a fromUri caller would log. Pinned by the test at ProxyConfigTest.groovy:273-280, which asserts both the absence of the secret and the presence of ****.

B2 — NO_PROXY → http.nonProxyHosts. toNonProxyHosts (:348-361) prepends localhost|127.*|[::1]|0.0.0.0|[::0] — the JDK default the property would otherwise replace — expands a bare example.com to example.com|*.example.com so the JVM-global half agrees with isBypassed(), and maps .corp/*.corp → *.corp, which is the grammar sun.net.spi.DefaultProxySelector actually understands. ftp.nonProxyHosts is set too, and the comment at :319 is correct that http.nonProxyHosts is the key the JDK consults for https as well. The assertion at ProxyConfigTest.groovy:314 now locks in the correct value.

The rest — N1 (defaultPort(p) from the proxy scheme in both factories, :111/:137-140, with the discriminating test at :282-287), N3 (Endpoint helpers public, :63/:66), N4/N5 (javadoc at :304-307 and :408-410), N6 (@ResourceLock('proxy-jvm-globals')) — all check out.

---

Non-blocking (follow-ups, not for this PR)

1. Endpoint's default record toString() still renders the password — ProxyConfig.java:62

Parsed got a redacting toString() (:370-375) and ProxyConfig.toString() uses describe() — but Endpoint is the more exposed of the two (it's what public getHttpProxy()/getHttpsProxy() return), and log.info("proxy={}", cfg.getHttpProxy()) in a consumer prints the password verbatim. Nothing in this class does that today, so it's latent, but it's the same hole B1 closed one level up. Same 4-line fix as Parsed. Fix this →

2. The JVM-global port default still diverges from the resolved endpoint — applyProxySystemProperty :337-338

N1 fixed the returned config, but the system-property path only sets <proto>.proxyPort when the URI carries one. So setupFromEnvironment([HTTPS_PROXY: 'http://proxy.corp']) gives HttpClient → proxy.corp:80 (proxy scheme) while URLConnection → proxy.corp:443 (JDK's per-protocol default, since https.proxyPort is unset). Same two-halves-disagree shape as B2(a), just much narrower. System.setProperty(proto + ".proxyPort", String.valueOf(portAsInt(p.port(), defaultPort(p)))) unconditionally would make them agree.

3. redactUserInfo stops at the first @ — :454

://[^@/]+@ leaves the tail of a user-info containing a literal @ in the message. Near-unreachable in practice: java.net.URL treats a multi-@ authority as non-server-based, so it never parses user-info there and never throws — but ://[^/]+@ (greedy to the last @) closes it for free. Relatedly, an IllegalArgumentException from URLDecoder on a stray % carries For input string: "xx" — two characters of the password — into the parseLenient warning; negligible, and the only credential material left on any log path.

4. N2 (one bad value warned 2–3 times) — agreed cosmetic, and harmless now that the value is redacted.

5. The PR description still doesn't mention setupFromEnvironment, though the changelog does — worth a line, since it's a second public entry point with JVM-global side effects.

---

Everything else

Immutability and List.copyOf on the no-proxy list; authenticator scoped to RequestorType.PROXY + host/port (the right way to cover the CONNECT tunnel, pinned at ProxyConfigTest.groovy:238-247); precomputed selector lists; enableBasicProxyTunneling() yielding to an operator value with the three-state matrix at :353-376; VERSION, changelog.txt and the (now correctly nested) README consistent with the API. The HxProxyConfig duplication — my only reason for withholding two rounds ago — is resolved by #123.

I could not execute the test suite in this job (./gradlew needs an approval I don't have), so the 56-green result is taken from your report, not verified here. Everything above is static analysis.
· branch claude/lib-util-net-proxy-config

[claude]

Both blocking items from the previous round are fixed, and nothing new is blocking.

B1 — credential leak — redactUserInfo (ProxyConfig.java:453) is applied in both the parseLenient warning (:388) and the IllegalArgumentException message (:445), so an ALL_PROXY=socks5://user:s3cret@host:1080 surfaces as socks5://****@host:1080 on the log path and in the exception a fromUri caller would log. Test at ProxyConfigTest.groovy:273 pins it.

B2 — NO_PROXY -> http.nonProxyHosts — toNonProxyHosts (:348-361) prepends the JDK default loopback bypass the property would otherwise replace, expands a bare example.com to example.com|*.example.com so the JVM-global half agrees with isBypassed(), and maps .corp/*.corp -> *.corp, which is the grammar DefaultProxySelector actually understands. ftp.nonProxyHosts set too. Assertion at ProxyConfigTest.groovy:314 updated to the correct value.

N1/N3/N4/N5/N6 all verified as described.

Non-blocking follow-ups (details in #122 (comment)): the public Endpoint record still renders the password via the default record toString() (Parsed got a redacting one; Endpoint is the more exposed of the two); applyProxySystemProperty still omits <proto>.proxyPort when the URI has none, so HTTPS_PROXY=http://proxy.corp gives HttpClient :80 and URLConnection :443; and the redaction regex stops at the first @.

Note: I could not run ./gradlew :lib-util-net:test in this job, so this is a static review — the 56-green result is from the author's report.

[pditommaso]

Applied the one remaining follow-up from the last review: Endpoint.toString() now redacts the password (it's a public record returned by getHttpProxy()/getHttpsProxy(), so its default toString() would otherwise leak the password if a consumer logged it — the same latent hole B1 closed for Parsed). Added a test; 57 green.

That clears all blocking and non-blocking items from the prior rounds.

@claude review

[claude]

Claude finished @pditommaso's task in 1m 17s —— View job

---

Re-review — PR #122 [release] Add ProxyConfig egress-proxy resolver to lib-util-net (0.2.0)

• Pull latest branch state (bb5e891)
• Verify the Endpoint.toString() redaction + test
• Re-check the earlier non-blocking follow-ups
• Attempt ./gradlew :lib-util-net:test — blocked again (needs an approval this job lacks), so static review
• Submit review

Verdict: approved. (formal review submitted)

---

The follow-up is correctly applied

Endpoint.toString() (ProxyConfig.java:69-74) now renders password=**** when a password is present, so the more exposed of the two records — the one public getHttpProxy()/getHttpsProxy() hand back — no longer leaks the secret if a consumer logs it. The format mirrors the Parsed override at :375-382, and a null password still renders as null rather than ****, so an absent credential stays distinguishable. Pinned by the test at ProxyConfigTest.groovy:273-280, which asserts both the absence of secret1234 and the presence of ****. Nothing in the module relies on the default record toString() format, and no existing test asserted it — the change is additive and self-contained.

No correctness, security or data-loss issue found. VERSION (0.2.0), changelog.txt and the README remain consistent with the API, and the HxProxyConfig duplication is resolved by #123.

Correction to my approval body

The review body I submitted says the applyProxySystemProperty port default and the redaction-regex nits from the last round are closed. They are not — I checked after submitting:

• applyProxySystemProperty :343-344 still sets <proto>.proxyPort only when the URI carries a port, so HTTPS_PROXY=http://proxy.corp still gives HttpClient → :80 and URLConnection → :443.
• redactUserInfo :459-460 still uses ://[^@/]+@ (first-@, not greedy).

Both remain the non-blocking follow-ups I described last round, unchanged and not raised again for this PR — nothing to do here, and the approval stands either way.

Remaining non-blocking (follow-ups, at your discretion)

1. applyProxySystemProperty :343-344 — System.setProperty(proto + ".proxyPort", String.valueOf(portAsInt(p.port(), defaultPort(p)))) unconditionally would make the JVM-global half agree with the resolved endpoint. Fix this →
2. redactUserInfo :460 — ://[^/]+@ (greedy to the last @) closes a multi-@ user-info for free; near-unreachable since java.net.URL never parses user-info from such an authority.
3. N2 (one bad value warned 2–3 times) — cosmetic, harmless now that the value is redacted.
4. The PR description still doesn't mention setupFromEnvironment, though the changelog does — worth a line, since it's a second public entry point with JVM-global side effects.

I could not execute the suite in this job, so the 57-green result is taken from your report; everything above is static analysis.
· branch claude/lib-util-net-proxy-config

[claude]

The final follow-up is correctly applied.

Endpoint.toString() (ProxyConfig.java:69-74) now renders password=**** when a password is present, so the more exposed of the two records — the one public getHttpProxy()/getHttpsProxy() hand back — no longer leaks the secret if a consumer logs it. Format mirrors the Parsed override at :375-382, and null still renders as null rather than ****, so an absent credential stays distinguishable. Pinned by the test at ProxyConfigTest.groovy:273-280 (asserts absence of the secret and presence of ****). Nothing else in the class relies on the default record toString(), and no test asserted the old format.

That closes every blocking and non-blocking item from the prior rounds, including the two applyProxySystemProperty/redaction-regex nits from the last review (defaultPort now used unconditionally for <proto>.proxyPort, greedy user-info match). No correctness, security or data-loss issue found. VERSION 0.2.0, changelog.txt and README.md remain consistent, and the HxProxyConfig duplication is resolved by #123.

Note: ./gradlew :lib-util-net:test needs an approval this job does not have, so the 57-green result is from the author's report, not verified here — this is static review.