fix: changes for review

Author: Andreybest

proxy.config.json

@@ -156,6 +156,11 @@
       }
     }
   ],
+  "upstreamProxy": {
+    "enabled": true,
+    "url": "http://localhost:8081",
+    "noProxy": []
+  },
   "tls": {
     "enabled": false,
     "key": "certs/key.pem",

src/config/index.ts

@@ -149,6 +149,17 @@ export const getProxyUrl = (): string | undefined => {
   return config.proxyUrl;
 };
 
+/**
+ * Redacts the userinfo (credentials) from a proxy URL for safe logging.
+ * e.g. http://user:pass@proxy.corp.local:8080 → http://<redacted>@proxy.corp.local:8080
+ *
+ * WARNING: proxyUrl may contain plaintext credentials in the userinfo portion.
+ * Never log a raw proxy URL — always pass it through this helper first.
+ */
+export const redactProxyUrl = (url: string): string => {
+  return url.replace(/(https?:\/\/)[^@]+@/, '$1<redacted>@');
+};
+
 // Get upstream proxy configuration
 export const getUpstreamProxyConfig = () => {
   const config = loadFullConfiguration();

src/proxy/routes/index.ts

@@ -22,7 +22,7 @@ import { executeChain } from '../chain';
 import { processUrlPath, validGitRequest } from './helper';
 import { getAllProxiedHosts } from '../../db';
 import { ProxyOptions } from 'express-http-proxy';
-import { getErrorMessage, handleErrorAndLog } from '../../utils/errors';
+import { handleErrorAndLog } from '../../utils/errors';
 import { getUpstreamProxyConfig } from '../../config';
 import { HttpsProxyAgent } from 'https-proxy-agent';
 import { OutgoingHttpHeaders, RequestOptions } from 'http';
@@ -175,7 +175,10 @@ const hostMatchesNoProxy = (host: string | null | undefined, noProxyList: string
     if (!pattern) {
       return false;
     }
-    const trimmed = pattern.trim();
+
+    const trimmed = pattern.trim().replace(/^\./, ''); // strip leading dot
+    if (trimmed === '*') return true; // wildcard - bypass all
+
     if (trimmed === '') {
       return false;
     }
@@ -194,39 +197,40 @@ const hostMatchesNoProxy = (host: string | null | undefined, noProxyList: string
   });
 };
 
+// WARNING: proxyUrl may contain plaintext credentials in the userinfo portion
+// (e.g. http://user:pass@proxy.corp.local:8080). Never log it directly — use
+// redactProxyUrl() from config for any log statements involving this value.
+let _cachedProxyAgent: { proxyUrl: string; agent: HttpsProxyAgent<string> } | null = null;
+
+const getOrCreateProxyAgent = (proxyUrl: string): HttpsProxyAgent<string> => {
+  if (!_cachedProxyAgent || _cachedProxyAgent.proxyUrl !== proxyUrl) {
+    _cachedProxyAgent = { proxyUrl, agent: new HttpsProxyAgent(proxyUrl) };
+  }
+  return _cachedProxyAgent.agent;
+};
+
 const buildUpstreamProxyAgent = (
   proxyReqOpts: Omit<RequestOptions, 'headers'> & {
     headers: OutgoingHttpHeaders;
   },
 ) => {
-  const upstreamProxyConfig = getUpstreamProxyConfig();
-
-  const configuredUrl = upstreamProxyConfig.url;
-  const envUrl = getEnvProxyUrl();
-
-  const proxyUrl = configuredUrl || envUrl;
+  const { enabled, url, noProxy } = getUpstreamProxyConfig();
 
-  // If nothing is configured, do not use a proxy
-  if (!proxyUrl) {
-    return undefined;
-  }
+  const proxyUrl = url || getEnvProxyUrl();
 
-  // If config explicitly disabled the proxy, do not use it
-  if (upstreamProxyConfig.enabled === false) {
+  if (enabled === false || !proxyUrl) {
     return undefined;
   }
 
   const host: string | null | undefined = proxyReqOpts.host || proxyReqOpts.hostname;
 
-  const configNoProxy = upstreamProxyConfig.noProxy ? upstreamProxyConfig.noProxy : [];
-  const envNoProxy = getEnvNoProxyList();
-  const combinedNoProxy = [...configNoProxy, ...envNoProxy];
+  const combinedNoProxy = [...(noProxy || []), ...getEnvNoProxyList()];
 
   if (hostMatchesNoProxy(host, combinedNoProxy)) {
     return undefined;
   }
 
-  return new HttpsProxyAgent(proxyUrl);
+  return getOrCreateProxyAgent(proxyUrl);
 };
 
 const proxyReqOptDecorator: ProxyOptions['proxyReqOptDecorator'] = (proxyReqOpts, _srcReq) => {
@@ -370,4 +374,5 @@ export {
   extractRawBody,
   validGitRequest,
   buildUpstreamProxyAgent,
+  hostMatchesNoProxy,
 };

test/hostMatchesNoProxy.test.ts

@@ -0,0 +1,114 @@
+/**
+ * Copyright 2026 GitProxy Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { hostMatchesNoProxy } from '../src/proxy/routes';
+
+describe('hostMatchesNoProxy', () => {
+  describe('null / undefined / empty host', () => {
+    it('returns false for null host', () => {
+      expect(hostMatchesNoProxy(null, ['example.com'])).toBe(false);
+    });
+
+    it('returns false for undefined host', () => {
+      expect(hostMatchesNoProxy(undefined, ['example.com'])).toBe(false);
+    });
+
+    it('returns false for empty string host', () => {
+      expect(hostMatchesNoProxy('', ['example.com'])).toBe(false);
+    });
+  });
+
+  describe('empty noProxyList', () => {
+    it('returns false when list is empty', () => {
+      expect(hostMatchesNoProxy('github.com', [])).toBe(false);
+    });
+  });
+
+  describe('exact match', () => {
+    it('matches host exactly', () => {
+      expect(hostMatchesNoProxy('github.com', ['github.com'])).toBe(true);
+    });
+
+    it('does not match a different host', () => {
+      expect(hostMatchesNoProxy('gitlab.com', ['github.com'])).toBe(false);
+    });
+
+    it('strips port before matching', () => {
+      expect(hostMatchesNoProxy('github.com:443', ['github.com'])).toBe(true);
+    });
+
+    it('does not match a subdomain as exact', () => {
+      expect(hostMatchesNoProxy('api.github.com', ['github.com'])).toBe(true); // suffix match applies
+    });
+  });
+
+  describe('domain suffix match', () => {
+    it('matches subdomain when pattern is the parent domain', () => {
+      expect(hostMatchesNoProxy('api.github.com', ['github.com'])).toBe(true);
+    });
+
+    it('matches deeply nested subdomain', () => {
+      expect(hostMatchesNoProxy('foo.bar.corp.local', ['corp.local'])).toBe(true);
+    });
+
+    it('does not match unrelated domain that happens to end with same string', () => {
+      expect(hostMatchesNoProxy('notgithub.com', ['github.com'])).toBe(false);
+    });
+  });
+
+  describe('leading dot in pattern', () => {
+    it('strips leading dot and still matches subdomain', () => {
+      expect(hostMatchesNoProxy('api.github.com', ['.github.com'])).toBe(true);
+    });
+
+    it('strips leading dot and still matches exact host', () => {
+      expect(hostMatchesNoProxy('github.com', ['.github.com'])).toBe(true);
+    });
+  });
+
+  describe('wildcard pattern', () => {
+    it('matches any host when pattern is *', () => {
+      expect(hostMatchesNoProxy('anything.example.com', ['*'])).toBe(true);
+    });
+
+    it('matches bare hostname when pattern is *', () => {
+      expect(hostMatchesNoProxy('localhost', ['*'])).toBe(true);
+    });
+  });
+
+  describe('blank / whitespace patterns', () => {
+    it('ignores empty string pattern', () => {
+      expect(hostMatchesNoProxy('github.com', [''])).toBe(false);
+    });
+
+    it('ignores whitespace-only pattern', () => {
+      expect(hostMatchesNoProxy('github.com', ['   '])).toBe(false);
+    });
+  });
+
+  describe('multiple patterns', () => {
+    it('returns true when host matches any pattern in the list', () => {
+      expect(hostMatchesNoProxy('github.com', ['gitlab.com', 'github.com', 'bitbucket.org'])).toBe(
+        true,
+      );
+    });
+
+    it('returns false when host matches none of the patterns', () => {
+      expect(hostMatchesNoProxy('github.com', ['gitlab.com', 'bitbucket.org'])).toBe(false);
+    });
+  });
+});

test/redactProxyUrl.test.ts

@@ -0,0 +1,62 @@
+/**
+ * Copyright 2026 GitProxy Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { redactProxyUrl } from '../src/config';
+
+describe('redactProxyUrl', () => {
+  describe('URLs with credentials', () => {
+    it('redacts user and password from http URL', () => {
+      expect(redactProxyUrl('http://user:pass@proxy.corp.local:8080')).toBe(
+        'http://<redacted>@proxy.corp.local:8080',
+      );
+    });
+
+    it('redacts user and password from https URL', () => {
+      expect(redactProxyUrl('https://user:pass@proxy.corp.local:8080')).toBe(
+        'https://<redacted>@proxy.corp.local:8080',
+      );
+    });
+
+    it('redacts username-only (no password) from URL', () => {
+      expect(redactProxyUrl('http://user@proxy.corp.local:8080')).toBe(
+        'http://<redacted>@proxy.corp.local:8080',
+      );
+    });
+
+    it('redacts credentials when no port is present', () => {
+      expect(redactProxyUrl('http://user:pass@proxy.corp.local')).toBe(
+        'http://<redacted>@proxy.corp.local',
+      );
+    });
+
+    it('redacts credentials containing special characters', () => {
+      expect(redactProxyUrl('http://user:p%40ssw0rd!@proxy.corp.local:3128')).toBe(
+        'http://<redacted>@proxy.corp.local:3128',
+      );
+    });
+  });
+
+  describe('URLs without credentials', () => {
+    it('leaves http URL without credentials unchanged', () => {
+      expect(redactProxyUrl('http://proxy.corp.local:8080')).toBe('http://proxy.corp.local:8080');
+    });
+
+    it('leaves https URL without credentials unchanged', () => {
+      expect(redactProxyUrl('https://proxy.corp.local:8080')).toBe('https://proxy.corp.local:8080');
+    });
+  });
+});