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	`45624bf`
🔍 Latest deploy log	https://app.netlify.com/projects/endearing-brigadeiros-63f9d0/deploys/69c52f5b5d947a0008202266

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

The committers listed above are authorized under a signed CLA.

✅ login: Andreybest / name: Andrew (45624bf, 548e84c, 9336511, b0a4b43)
✅ login: coopernetes / name: Thomas Cooper (45624bf)

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

Codecov Report

❌ Patch coverage is 89.87342% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.67%. Comparing base (68bf3db) to head (45624bf).
⚠️ Report is 31 commits behind head on main.

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1458   +/-   ##
=======================================
  Coverage   89.66%   89.67%           
=======================================
  Files          68       68           
  Lines        4869     4947   +78     
  Branches      888      918   +30     
=======================================
+ Hits         4366     4436   +70     
- Misses        485      493    +8     
  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 🤔

jescalada · 2026-03-17T03:23:36Z

config.schema.json

        }
      }
+    },
+    "upstreamProxy": {


Sample values or just empty entries should be added to proxy.config.json so it's easier to see that the upstreamProxy is configurable and how to configure it:

"upstreamConfig": { "enabled": true, "url": "", "noProxy": [] }

jescalada · 2026-03-17T03:32:16Z

src/proxy/routes/index.ts

+    headers: OutgoingHttpHeaders;
+  },
+) => {
+  const upstreamProxyConfig = getUpstreamProxyConfig();


Refactored for simplicity:

Suggested change

const upstreamProxyConfig = getUpstreamProxyConfig();

const { enabled, url, noProxy } = getUpstreamProxyConfig();

jescalada · 2026-03-17T03:33:34Z

src/proxy/routes/index.ts

+  const configuredUrl = upstreamProxyConfig.url;
+  const envUrl = getEnvProxyUrl();
+
+  const proxyUrl = configuredUrl || envUrl;


Short-circuiting so we don't execute getEnvProxyUrl() all the time

Suggested change

const proxyUrl = configuredUrl || envUrl;

const proxyUrl = url || getEnvProxyUrl();

(must also remove const envUrl = getEnvProxyUrl(); from above)

jescalada · 2026-03-17T03:35:34Z

src/proxy/routes/index.ts

+) => {
+  const upstreamProxyConfig = getUpstreamProxyConfig();
+
+  const configuredUrl = upstreamProxyConfig.url;


Suggested change

const configuredUrl = upstreamProxyConfig.url;

jescalada · 2026-03-17T03:35:45Z

src/proxy/routes/index.ts

+  const upstreamProxyConfig = getUpstreamProxyConfig();
+
+  const configuredUrl = upstreamProxyConfig.url;
+  const envUrl = getEnvProxyUrl();


Suggested change

const envUrl = getEnvProxyUrl();

jescalada · 2026-03-17T03:43:56Z

src/proxy/routes/index.ts

+
+  const proxyUrl = configuredUrl || envUrl;
+
+  // If nothing is configured, do not use a proxy


Code here is self-descriptive 🙂

Suggested change

// If nothing is configured, do not use a proxy

jescalada · 2026-03-17T03:44:13Z

src/proxy/routes/index.ts

+    return undefined;
+  }
+
+  // If config explicitly disabled the proxy, do not use it


Same as above

Suggested change

// If config explicitly disabled the proxy, do not use it

jescalada · 2026-03-17T03:50:06Z

src/proxy/routes/index.ts

+
+  const host: string | null | undefined = proxyReqOpts.host || proxyReqOpts.hostname;
+
+  const configNoProxy = upstreamProxyConfig.noProxy ? upstreamProxyConfig.noProxy : [];


Not sure what was the intention here 🤔

Resolved in comments below

jescalada · 2026-03-17T03:53:16Z

src/proxy/routes/index.ts

+
+  const configNoProxy = upstreamProxyConfig.noProxy ? upstreamProxyConfig.noProxy : [];
+  const envNoProxy = getEnvNoProxyList();
+  const combinedNoProxy = [...configNoProxy, ...envNoProxy];


Do we want to combine the configuration + env values, or do we want to default to the env values if the noProxy from the config is missing despite upstreamProxy being enabled?

If we want to combine, this could be done in a single line like such:

const combinedNoProxy = [...noProxy || [], ...getEnvNoProxyList()]

If we want to use the env values as fallbacks, we can further simplify by stuffing everything in the if-statement below:

if (hostMatchesNoProxy(host, noProxy || getEnvNoProxyList())) { return undefined; }

I think we need to combine, selected first approach :)

jescalada · 2026-03-17T04:01:56Z

test/upstreamProxy.test.ts

Let's aim for >90% coverage 😃

89.87%, hopefully this passes :)

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

src/proxy/routes/index.ts

+    .filter((entry) => entry.length > 0);
+};
+
+const hostMatchesNoProxy = (host: string | null | undefined, noProxyList: string[]): boolean => {


hostMatchesNoProxy does not handle two standard NO_PROXY conventions that are universally used in corporate environments - exactly the target audience for this feature:

Wildcard * - means "bypass proxy for all hosts." Currently * would fall through to the .endsWith check and never match.

Leading-dot patterns like .example.com - the current .endsWith(`.${trimmed}`) logic would look for ..example.com, which never matches anything.

These conventions are supported by curl, wget, Go stdlib, Python requests, and Node's global-agent. Since this feature targets corporate proxy environments, both patterns will appear in real NO_PROXY values.

Suggested fix:

const trimmed = pattern.trim().replace(/^\./, ''); // strip leading dot if (trimmed === '*') return true; // wildcard - bypass all

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

src/proxy/routes/index.ts

+    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);

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

docs/Architecture.md

+Configures routing of outbound requests from the GitProxy server to upstream Git hosts (e.g. GitHub, GitLab) via an HTTP(S) proxy. Use this when the server runs in an environment where direct Internet access is not allowed and all traffic must go through a corporate web proxy ("proxying the proxy").
+
+- **`enabled`** (boolean): When `true`, outbound connections to upstream Git hosts use the configured proxy. When `false`, the proxy is not used even if `url` or environment variables are set.
+- **`url`** (string): The HTTP(S) proxy URL (e.g. `http://proxy.corp.local:8080` or `http://user:pass@proxy.corp.local:8080`). If omitted, GitProxy falls back to the `HTTPS_PROXY`, `https_proxy`, `HTTP_PROXY` or `http_proxy` environment variables (first defined wins).


The docs show http://user:pass@proxy.corp.local:8080 as a valid proxy URL format, which is correct - but the code has no redaction layer for this value. The proxyUrl containing credentials flows through buildUpstreamProxyAgent and into the request options. If express-http-proxy debug logging is enabled, or if logConfiguration prints the full config at startup, credentials will appear in plaintext in logs.

At minimum, consider:

Adding a warning comment in the code that proxyUrl may contain credentials and must not be logged

If any logging is added in the future around proxy setup, use a helper that redacts the userinfo portion of the URL (everything between :// and @)

Added warnings in comments. Also added function for reduction if it will be used in future

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

src/proxy/routes/index.ts

+};
+
+const proxyReqOptDecorator: ProxyOptions['proxyReqOptDecorator'] = (proxyReqOpts, _srcReq) => {
+  const agent = buildUpstreamProxyAgent(proxyReqOpts);


buildUpstreamProxyAgent is called from proxyReqOptDecorator on every proxied request. Each invocation creates a new HttpsProxyAgent(proxyUrl) and also calls getUpstreamProxyConfig() which runs loadFullConfiguration(). For high-throughput operations like large clones with many pack requests, this creates unnecessary object allocation and GC pressure.

Consider caching the agent instance and only recreating it when the proxy URL or configuration changes - for example, using a simple memoization keyed on proxyUrl.

Considering that loadFullConfiguration() returns cached object:

function loadFullConfiguration(): GitProxyConfig { if (_currentConfig) { return _currentConfig; }

I don't think we don't have to worry about it, for new HttpsProxyAgent(proxyUrl) - cache is implemented

src/config/index.ts

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

proxy.config.json

Co-authored-by: Thomas Cooper <57812123+coopernetes@users.noreply.github.com> Signed-off-by: Andrew <andrey255@live.com>

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 bot found potential problems Mar 25, 2026

View reviewed changes

src/config/index.ts Fixed Show fixed Hide fixed

fix: improve regular expression for redactions

9336511

coopernetes reviewed Mar 26, 2026

View reviewed changes

proxy.config.json Outdated Show resolved Hide resolved

Update proxy.config.json

45624bf

Co-authored-by: Thomas Cooper <57812123+coopernetes@users.noreply.github.com> Signed-off-by: Andrew <andrey255@live.com>

	const upstreamProxyConfig = getUpstreamProxyConfig();
	const { enabled, url, noProxy } = getUpstreamProxyConfig();

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


		const proxyUrl = configuredUrl \|\| envUrl;

		// If nothing is configured, do not use a proxy


		const host: string \| null \| undefined = proxyReqOpts.host \|\| proxyReqOpts.hostname;

		const configNoProxy = upstreamProxyConfig.noProxy ? upstreamProxyConfig.noProxy : [];

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!

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!

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

dcoric 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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Andreybest commented Mar 25, 2026

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