feat: add upstream proxy configuration for outbound requests by Andreybest · Pull Request #1458 · finos/git-proxy

Andreybest · 2026-03-16T03:17:50Z

Resolves #759.

Adds upstream proxy support. Uses values from both configuration file in upstreamProxy fields and environment variables (HTTPS_PROXY/HTTP_PROXY, NO_PROXY)

netlify · 2026-03-16T03:17:55Z

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name	Link
🔨 Latest commit	`790aa6c`
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/69e80298d27d8a0008495679

linux-foundation-easycla · 2026-03-16T03:17:57Z

The committers listed above are authorized under a signed CLA.

✅ login: Andreybest / name: Andrew (45624bf)
✅ login: Andreybest / name: Andrii Kotliar (548e84c, 7534cf3, 790aa6c, 9336511, b0a4b43, be51974)
✅ login: coopernetes / name: Thomas Cooper (45624bf)
✅ login: jescalada / name: Juan Escalada (9af1d69)
✅ login: kriswest / name: Kris West (84b5b27, b7fa2b3)

codecov · 2026-03-16T03:20:34Z

Codecov Report

❌ Patch coverage is 81.44330% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.06%. Comparing base (6981427) to head (790aa6c).

Files with missing lines	Patch %	Lines
src/proxy/routes/index.ts	81.11%	17 Missing ⚠️
src/config/index.ts	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1458      +/-   ##
==========================================
- Coverage   90.21%   90.06%   -0.16%     
==========================================
  Files          69       69              
  Lines        5511     5607      +96     
  Branches      944      976      +32     
==========================================
+ Hits         4972     5050      +78     
- Misses        521      539      +18     
  Partials       18       18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jescalada

@Andreybest Thanks for the contribution! 🚀

I left some comments for code refactoring. It'd be very helpful if you could upload images or a video demonstrating that this feature works as intended.

@coopernetes Wondering if this implements your original requirements (#759) and works on your end 🤔

dcoric

Thanks for tackling #759. I've left a few comments focused on correctness and robustness, especially around NO_PROXY pattern handling which is important to get right for the target use case. The main items are the missing */leading-dot NO_PROXY support and proxy URL validation.

dcoric · 2026-03-22T21:50:06Z

+    return undefined;
+  }
+
+  return new HttpsProxyAgent(proxyUrl);


The proxyUrl (from config or env vars) is passed directly to new HttpsProxyAgent(proxyUrl) without validation. If the URL is malformed (typo, missing protocol, empty string after trimming), this throws a cryptic Invalid URL error that surfaces through express-http-proxy's error handler with no indication that the upstream proxy config is the problem.

Consider wrapping this in a try-catch or validating first:

try { new URL(proxyUrl); // validate before constructing agent } catch { throw new Error(`Invalid upstream proxy URL: check your upstreamProxy.url config or HTTPS_PROXY env var`); } return new HttpsProxyAgent(proxyUrl);

This comment might still need fixing! 🙂

Don't know how I've missed it, added, thanks @jescalada and @dcoric !

Andreybest · 2026-03-25T21:03:19Z

Thanks for the reviews @jescalada and @dcoric!
Made changes and left comments for your comments. Please recheck :)

Andreybest · 2026-03-25T21:05:20Z

@jescalada
Here is the video on how the proxy works:

Screen.Recording.2026-03-25.at.21.35.35_no_audio.mov

Co-authored-by: Thomas Cooper <[email protected]> Signed-off-by: Andrew <[email protected]>

jescalada

LGTM after fixing Denis' comment about validating/error handling the HttpsProxyAgent creation.

@coopernetes Does everything work as you expected? 🤔

jescalada · 2026-04-08T03:08:18Z

+
+const getOrCreateProxyAgent = (proxyUrl: string): HttpsProxyAgent<string> => {
+  if (!_cachedProxyAgent || _cachedProxyAgent.proxyUrl !== proxyUrl) {
+    _cachedProxyAgent = { proxyUrl, agent: new HttpsProxyAgent(proxyUrl) };


Just repeating Denis earlier comment - this should be properly validated and/or throw a descriptive error

Added, thanks for remind :)

jescalada · 2026-04-08T03:08:53Z

+    return undefined;
+  }
+
+  return new HttpsProxyAgent(proxyUrl);


This comment might still need fixing! 🙂

dcoric · 2026-04-20T16:35:11Z

+
+  const proxyUrl = url || getEnvProxyUrl();
+
+  if (enabled === false || !proxyUrl) {


I think the env-var-only path is still broken. proxy.config.json is imported as the default config, and it now contains upstreamProxy.enabled: false. When a user omits upstreamProxy from their config and relies only on HTTPS_PROXY/HTTP_PROXY, the merged config still has enabled === false, so buildUpstreamProxyAgent() returns before using the env var. This contradicts the docs saying env vars enable proxying when upstreamProxy is not configured. The current test mocks getUpstreamProxyConfig() as {}, so it does not cover the real default-config path. Can we either remove upstreamProxy from the shipped default config, distinguish “unset” from explicitly disabled, or change the documented behavior and add a test for the default merge?

I don't know how I've missed it, but now it's fixed. Unset enable now is treated as false. Thanks!

dcoric · 2026-04-20T16:38:37Z

+const getOrCreateProxyAgent = (proxyUrl: string): HttpsProxyAgent<string> => {
+  if (!_cachedProxyAgent || _cachedProxyAgent.proxyUrl !== proxyUrl) {
+    try {
+      new URL(proxyUrl);


This validation still accepts values that are syntactically parseable URLs but not valid HTTP(S) proxy URLs. For example, new URL("socks5://proxy.example.com:1080"), new URL("ftp://proxy.example.com:21"), and even new URL("localhost:8081") all succeed. HttpsProxyAgent also constructs for those values, then treats non-https: schemes as plain TCP proxy connections, so the user gets a later connectivity failure instead of a descriptive config error. Can we parse once, require protocol to be http: or https:, require a non-empty hostname, and add tests for unsupported schemes and missing protocol?

Added checks to accept only https and http, and for hostname to exist.

coopernetes

LGTM. The only rub is the lack of direct authentication support in the proxy agent. Many client and server libraries do not implement auth directly so it's fine to leave it out for now. Worth making a follow up issue to add HTTP Basic and NTLM (Windows) auth support which are the two most common methods (at least that I'm aware of).

kriswest · 2026-04-21T12:41:02Z

Worth making a follow up issue to add HTTP Basic and NTLM (Windows) auth support which are the two most common methods (at least that I'm aware of).

I'd suggest making that HTTP Basic, NTLM and Negotiate (the negotiate protocol is used to detect support for NTLM or Kerberos auth and then kick off which ever is supported).

…false'

Andreybest · 2026-04-21T23:06:46Z

@dcoric Thank you again for the review! Can you please check again? :)

feat: add upstream proxy configuration for outbound requests

be51974

Andreybest requested a review from a team as a code owner March 16, 2026 03:17

Andreybest requested a review from kriswest March 16, 2026 03:17

github-actions Bot added the feature label Mar 16, 2026

jescalada requested changes Mar 17, 2026

View reviewed changes

dcoric suggested changes Mar 22, 2026

View reviewed changes

Andreybest added 2 commits March 23, 2026 14:47

Merge branch 'main' into 759-upstream-proxy

b0a4b43

fix: changes for review

548e84c

github-advanced-security AI found potential problems Mar 25, 2026

View reviewed changes

Comment thread src/config/index.ts Fixed

fix: improve regular expression for redactions

9336511

coopernetes reviewed Mar 26, 2026

View reviewed changes

Comment thread proxy.config.json Outdated

Update proxy.config.json

45624bf

Co-authored-by: Thomas Cooper <[email protected]> Signed-off-by: Andrew <[email protected]>

jescalada reviewed Apr 8, 2026

View reviewed changes

jescalada and others added 2 commits April 8, 2026 12:28

Merge branch 'main' into 759-upstream-proxy

9af1d69

feat: validate upstream proxy URL before creating proxy agent

7534cf3

kriswest mentioned this pull request Apr 20, 2026

20th April 2026 - GitProxy Meeting Minutes #1506

Open

66 tasks

Merge branch 'main' into 759-upstream-proxy

84b5b27

kriswest requested review from coopernetes, dcoric and jescalada April 20, 2026 15:33

dcoric reviewed Apr 20, 2026

View reviewed changes

coopernetes reviewed Apr 21, 2026

View reviewed changes

Merge branch 'main' into 759-upstream-proxy

b7fa2b3

fix: add check for protocol and hostname and treat unset enabled as '…

790aa6c

…false'

Andreybest requested a review from dcoric April 21, 2026 23:06


		const proxyUrl = url \|\| getEnvProxyUrl();

		if (enabled === false \|\| !proxyUrl) {

Conversation

Andreybest commented Mar 16, 2026

Uh oh!

netlify Bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Uh oh!

linux-foundation-easycla Bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jescalada left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dcoric left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Andreybest commented Mar 25, 2026

Uh oh!

Andreybest commented Mar 25, 2026

Uh oh!

Uh oh!

jescalada left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coopernetes left a comment

Choose a reason for hiding this comment

Uh oh!

kriswest commented Apr 21, 2026

Uh oh!

Andreybest commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

netlify Bot commented Mar 16, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Mar 16, 2026 •

edited

Loading

codecov Bot commented Mar 16, 2026 •

edited

Loading