Merge pull request #9154 from desktop/env-for-proxy

Automatically set Git proxy environment variables from system configuration

app/src/lib/feature-flag.ts

@@ -116,6 +116,14 @@ export function enableCreateForkFlow(): boolean {
   return true
 }
 
+/**
+ * Whether or not to enable support for automatically resolving the
+ * system-configured proxy url and passing that to Git.
+ */
+export function enableAutomaticGitProxyConfiguration(): boolean {
+  return enableDevelopmentFeatures()
+}
+
 /**
  * Should we show the "Create Issue on GitHub" item under
  * "Repository" in the app menu?

app/src/lib/git/environment.ts

@@ -1,5 +1,7 @@
 import { envForAuthentication } from './authentication'
 import { IGitAccount } from '../../models/git-account'
+import { enableAutomaticGitProxyConfiguration } from '../feature-flag'
+import { resolveGitProxy } from '../resolve-git-proxy'
 import { getDotComAPIEndpoint } from '../api'
 import { Repository } from '../../models/repository'
 
@@ -66,5 +68,69 @@ export async function envForRemoteOperation(
 ) {
   return {
     ...envForAuthentication(account),
+    ...(await envForProxy(remoteUrl)),
   }
 }
+
+/**
+ * Not intended to be used directly. Exported only in order to
+ * allow for testing.
+ *
+ * @param remoteUrl The remote url to resolve a proxy for.
+ * @param env       The current environment variables, defaults
+ *                  to `process.env`
+ * @param resolve   The method to use when resolving the proxy url,
+ *                  defaults to `resolveGitProxy`
+ */
+export async function envForProxy(
+  remoteUrl: string,
+  env: NodeJS.ProcessEnv = process.env,
+  resolve: (url: string) => Promise<string | undefined> = resolveGitProxy
+): Promise<NodeJS.ProcessEnv | undefined> {
+  if (!enableAutomaticGitProxyConfiguration()) {
+    return undefined
+  }
+
+  // We'll play it safe and say that if the user has configured
+  // the ALL_PROXY environment variable they probably know what
+  // they're doing and wouldn't want us to override it with a
+  // protocol-specific proxy. cURL supports both lower and upper
+  // case, see:
+  // https://github.com/curl/curl/blob/14916a82e/lib/url.c#L2180-L2185
+  if ('ALL_PROXY' in env || 'all_proxy' in env) {
+    log.info(`proxy url not resolved, ALL_PROXY already set`)
+    return
+  }
+
+  const protocolMatch = /^(https?):\/\//i.exec(remoteUrl)
+
+  // We can only resolve and use a proxy for the protocols where cURL
+  // would be involved (i.e http and https). git:// relies on ssh.
+  if (protocolMatch === null) {
+    log.info(`proxy url not resolved, protocol not supported`)
+    return
+  }
+
+  // Note that HTTPS here doesn't mean that the proxy is HTTPS, only
+  // that all requests to HTTPS protocols should be proxied. The
+  // proxy protocol is defined by the url returned by `this.resolve()`
+  const proto = protocolMatch[1].toLowerCase() // http or https
+
+  // Lower case environment variables due to
+  // https://ec.haxx.se/usingcurl/usingcurl-proxies#http_proxy-in-lower-case-only
+  const envKey = `${proto}_proxy` // http_proxy or https_proxy
+
+  // If the user has already configured a proxy in the environment
+  // for the protocol we're not gonna override it.
+  if (envKey in env || (proto === 'https' && 'HTTPS_PROXY' in env)) {
+    log.info(`proxy url not resolved, ${envKey} already set`)
+    return
+  }
+
+  const proxyUrl = await resolve(remoteUrl).catch(err => {
+    log.error('Failed resolving Git proxy', err)
+    return undefined
+  })
+
+  return proxyUrl === undefined ? undefined : { [envKey]: proxyUrl }
+}

app/src/lib/resolve-git-proxy.ts

@@ -0,0 +1,49 @@
+import { remote } from 'electron'
+import { enableAutomaticGitProxyConfiguration } from './feature-flag'
+import { parsePACString } from './parse-pac-string'
+
+export async function resolveGitProxy(
+  url: string
+): Promise<string | undefined> {
+  if (!enableAutomaticGitProxyConfiguration()) {
+    return undefined
+  }
+
+  // resolveProxy doesn't throw an error (at least not in the
+  // current Electron version) but it could in the future and
+  // it's also possible that the IPC layer could throw an
+  // error (if the URL we're given is null or undefined despite
+  // our best type efforts for example).
+  // Better safe than sorry.
+  const pacString = await remote.session.defaultSession
+    .resolveProxy(url)
+    .catch(err => {
+      log.error(`Failed resolving proxy for '${url}'`, err)
+      return 'DIRECT'
+    })
+
+  const proxies = parsePACString(pacString)
+
+  if (proxies === null) {
+    return undefined
+  }
+
+  for (const proxy of proxies) {
+    // On Windows GitHub Desktop relies on the `schannel` `http.sslBackend` to
+    // be used in order to support things like self-signed certificates.
+    // Unfortunately it doesn't support https proxies so we'll exclude those
+    // here. Luckily for us https proxies are really rare. On macOS we use
+    // the OpenSSL ssl backend which does support https proxies.
+    //
+    // See
+    // https://github.com/jeroen/curl/issues/186#issuecomment-494560890
+    // "The Schannel backend doesn't support HTTPS proxy"
+    if (__WIN32__ && proxy.startsWith('https://')) {
+      log.warn('ignoring https proxy, not supported in cURL/schannel')
+    } else {
+      return proxy
+    }
+  }
+
+  return undefined
+}

app/test/unit/git/environment-test.ts

@@ -0,0 +1,108 @@
+import { envForProxy } from '../../../src/lib/git/environment'
+
+describe('git/environmnent', () => {
+  const httpProxyUrl = 'http://proxy:8888/'
+  const httpsProxyUrl = 'https://proxy:8888/'
+
+  const nullResolver = () => Promise.resolve(undefined)
+  const throwingResolver = () => Promise.reject(new Error('such error'))
+  const defaultResolver = async (url: string) => {
+    if (url.startsWith('http://')) {
+      return httpProxyUrl
+    } else if (url.startsWith('https://')) {
+      return httpsProxyUrl
+    } else {
+      return undefined
+    }
+  }
+
+  describe('envForProxy', () => {
+    it('sets the correct environment variable based on protocol', async () => {
+      expect(
+        await envForProxy('https://github.com/', {}, defaultResolver)
+      ).toEqual({
+        https_proxy: httpsProxyUrl,
+      })
+
+      expect(
+        await envForProxy('http://github.com/', {}, defaultResolver)
+      ).toEqual({
+        http_proxy: httpProxyUrl,
+      })
+    })
+
+    it('fails gracefully if resolver throws', async () => {
+      expect(
+        await envForProxy('https://github.com/', {}, throwingResolver)
+      ).toEqual(undefined)
+    })
+
+    it("it doesn't set any variables if resolver returns undefined", async () => {
+      expect(
+        await envForProxy('https://github.com/', {}, nullResolver)
+      ).toEqual(undefined)
+    })
+
+    it('sets the correct environment variable based on protocol', async () => {
+      expect(
+        await envForProxy('https://github.com/', {}, defaultResolver)
+      ).toEqual({
+        https_proxy: httpsProxyUrl,
+      })
+
+      expect(
+        await envForProxy('http://github.com/', {}, defaultResolver)
+      ).toEqual({
+        http_proxy: httpProxyUrl,
+      })
+    })
+
+    it('ignores unknown protocols', async () => {
+      expect(
+        await envForProxy('ftp://github.com/', {}, defaultResolver)
+      ).toEqual(undefined)
+    })
+
+    it('does not override existing environment variables', async () => {
+      expect(
+        await envForProxy(
+          'https://github.com/',
+          { https_proxy: 'foo' },
+          defaultResolver
+        )
+      ).toEqual(undefined)
+
+      expect(
+        await envForProxy(
+          'https://github.com/',
+          { HTTPS_PROXY: 'foo' },
+          defaultResolver
+        )
+      ).toEqual(undefined)
+
+      expect(
+        await envForProxy(
+          'http://github.com/',
+          { http_proxy: 'foo' },
+          defaultResolver
+        )
+      ).toEqual(undefined)
+
+      expect(
+        await envForProxy(
+          'https://github.com/',
+          { ALL_PROXY: 'foo' },
+          defaultResolver
+        )
+      ).toEqual(undefined)
+
+      expect(
+        await envForProxy(
+          'https://github.com/',
+          { all_proxy: 'foo' },
+          defaultResolver
+        )
+      ).toEqual(undefined)
+    })
+  })
+})