From d2b7d85e7cb61467395604aae1b84439c320cb84 Mon Sep 17 00:00:00 2001 From: Felix Luthman <34520175+felixlut@users.noreply.github.com> Date: Mon, 13 Feb 2023 15:00:05 +0100 Subject: [PATCH] Standardize behaviour of no_proxy environmental variable (#1223) * match no_proxy to subdomains * strip leading dot + '*' match all + testcases * Update proxy.test.ts * Revert "Update proxy.test.ts" This reverts commit 0e925a6dc5a88470659d28f0809772e4f450766f. * remove support for leading dots and wildcard no_proxy * change order of tests for logic consistency * add test for working leading dot * add check for partial domain, as opposed to subdomain --- packages/http-client/__tests__/proxy.test.ts | 38 ++++++++++++++++++++ packages/http-client/src/proxy.ts | 10 +++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/http-client/__tests__/proxy.test.ts b/packages/http-client/__tests__/proxy.test.ts index 4c627f3d..ccbceec2 100644 --- a/packages/http-client/__tests__/proxy.test.ts +++ b/packages/http-client/__tests__/proxy.test.ts @@ -145,6 +145,44 @@ describe('proxy', () => { expect(bypass).toBeFalsy() }) + it('checkBypass returns true if host with subdomain in no_proxy', () => { + process.env['no_proxy'] = 'myserver.com' + const bypass = pm.checkBypass(new URL('https://sub.myserver.com')) + expect(bypass).toBeTruthy() + }) + + it('checkBypass returns false if no_proxy is subdomain', () => { + process.env['no_proxy'] = 'myserver.com' + const bypass = pm.checkBypass(new URL('https://myserver.com.evil.org')) + expect(bypass).toBeFalsy() + }) + + it('checkBypass returns false if no_proxy is part of domain', () => { + process.env['no_proxy'] = 'myserver.com' + const bypass = pm.checkBypass(new URL('https://evilmyserver.com')) + expect(bypass).toBeFalsy() + }) + + // Do not strip leading dots as per https://github.com/actions/runner/blob/97195bad5870e2ad0915ebfef1616083aacf5818/docs/adrs/0263-proxy-support.md + it('checkBypass returns false if host with leading dot in no_proxy', () => { + process.env['no_proxy'] = '.myserver.com' + const bypass = pm.checkBypass(new URL('https://myserver.com')) + expect(bypass).toBeFalsy() + }) + + it('checkBypass returns true if host with subdomain in no_proxy defined with leading "."', () => { + process.env['no_proxy'] = '.myserver.com' + const bypass = pm.checkBypass(new URL('https://sub.myserver.com')) + expect(bypass).toBeTruthy() + }) + + // Do not match wildcard ("*") as per https://github.com/actions/runner/blob/97195bad5870e2ad0915ebfef1616083aacf5818/docs/adrs/0263-proxy-support.md + it('checkBypass returns true if no_proxy is "*"', () => { + process.env['no_proxy'] = '*' + const bypass = pm.checkBypass(new URL('https://anything.whatsoever.com')) + expect(bypass).toBeFalsy() + }) + it('HttpClient does basic http get request through proxy', async () => { process.env['http_proxy'] = _proxyUrl const httpClient = new httpm.HttpClient() diff --git a/packages/http-client/src/proxy.ts b/packages/http-client/src/proxy.ts index f13409b5..e4e43a54 100644 --- a/packages/http-client/src/proxy.ts +++ b/packages/http-client/src/proxy.ts @@ -51,7 +51,15 @@ export function checkBypass(reqUrl: URL): boolean { .split(',') .map(x => x.trim().toUpperCase()) .filter(x => x)) { - if (upperReqHosts.some(x => x === upperNoProxyItem)) { + if ( + upperReqHosts.some( + x => + x === upperNoProxyItem || + x.endsWith(`.${upperNoProxyItem}`) || + (upperNoProxyItem.startsWith('.') && + x.endsWith(`${upperNoProxyItem}`)) + ) + ) { return true } }