fix: allow proxy to be false and fallback an empty string

[Swiftwork]

Good work on the library!

I found an issue relating to our enterprise solution, where both our GitHub Actions workflows and our GitHub Enterprise service is on a private network behind a proxy. The HTTP_PROXY variable is set for all external connections, but of course we don't want this between resources on the private network as it will fail. Unfortunately I don't get much info from the pipeline:

[1:50:00 PM] [semantic-release] › ✖  Failed step "publish" of plugin "@semantic-release/github"
[1:50:00 PM] [semantic-release] › ✖  An error occurred while running semantic-release: RequestError [HttpError]
    at /__w/cms-sync/cms-sync/node_modules/@octokit/request/dist-node/index.js:66:23
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  status: 403,

But I suspect after looking the code on lines resolve-config.js L22 and get-client.js L36-L40 that this is the culprit. Proxy cannot be disabled by setting false as it will fallback to process.env.HTTP_PROXY, nor can process.env.HTTP_PROXY be "unset" to empty string '' as this fails verify:

[3:41:01 PM] [semantic-release] › ✖  Failed step "fail" of plugin "@semantic-release/github"
[3:41:01 PM] [semantic-release] › ✖  EINVALIDPROXY Invalid `proxy` option.
The proxy option (https://github.com/semantic-release/github/blob/master/README.md#proxy) must be a String  or an Objects with a host and a port property.

Your configuration for the proxy option is ``.

This pull-request is a simple change that should allow proxy to be set to false and process.env.http_proxy || process.env.HTTP_PROXY to be set to empty string.

[gr2m]

nor can process.env.HTTP_PROXY be "unset" to empty string '' as this fails verify

wouldn't the better solution be to not use process.env.HTTP_PROXY if it's falsy?

[Swiftwork]

Hey @gr2m,

Thanks for taking you time to answer and I agree, however I haven't found a way to do that. Unless you mean changing the verify method to check if proxy is a valid url? I'll try to illustrate my issue more clearly using a GitHub Workflow.

name: Basic-Workflow # Runs inside the corporate network

jobs:
  build:
    runs-on: corporate-runner # Specialized with certificates and HTTP_PROXY env needed for external resources

    steps:
      - name: Checkout code
        uses: actions/checkout@v2

      - name: Run release # GitHub Enterprise hosted inside the same corporate network
        run: npx semantic-release
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          HTTP_PROXY: "" # Attempt to disable inherited proxy settings, but this is invalid for semantic-release/github
          # ....

Above is a simplified workflow, but at you might guess developers can't really control what HTTP_PROXY the parent runner has set (created by corporate). Set is also the NO_PROXY env, that includes the GitHub Enterprise address, but semantic-release/github doesn't seem to take this into account either. However, using NO_PROXY would add unnecessary complexity as semantic-release/github only accesses a single domain (GitHub API). Does this make more sense?

[Swiftwork]

I originally created a NO_PROXY check, but decided against it. Here is the code if that is a more interesting path.

/**
 * Determines if the proxy should be used for a given request.
 *
 * @param {string} url Address of the request
 * @param {string|string[]|Object[]} [noProxy] configuration for proxy exceptions
 * @returns {boolean} Flag dictating the necessity of using the proxy
 */
 module.exports = (url, noProxy) => {
  // No proxy is unset, assume proxy is needed
  if (!noProxy || !noProxy.length) return true;

  const request = new URL(url);
  let noProxies = noProxy;
  
  // If string is given, split on comma or semicolon and trim white spaces between
  if (typeof noProxy === "string") noProxies = noProxy.split(/\s*(,|;)\s*/g);

  for (const noProxyEntry of noProxies) {
    let noProxyHostname;
    let noProxyPort;

    if (typeof noProxyEntry === "string") {
      // If each entry is a string parse out hostname and port
      [noProxyHostname, noProxyPort] = noProxyEntry.split(":").map(part => part.trim());
    } else {
      // Else expect entry to be an object
      noProxyHostname = noProxyEntry.host.trim();
      noProxyPort = (noProxyEntry.port + '').trim();
    }

    // Trim leading dot, split into segments, and reverse for matching
    const noProxySegments = noProxyHostname
      .replace(/^\./, "")
      .split(".")
      .reverse();
    // Split into segments and reverse for matching
    const requestSegments = request.hostname.split(".").reverse();
    // Check if each segment matches or is a wildcard
    const hostnameMatches = noProxySegments.every((segment, i) =>
      segment === "*" ||
      (requestSegments[i] && segment.toLowerCase() === requestSegments[i].toLowerCase())
    );
    // Check if no proxy port is specified and if it matches with request port
    const portMatches =
      !noProxyPort || noProxyPort === "*" || noProxyPort === request.port;
    // No proxy is needed
    if (hostnameMatches && portMatches) return false;
  }

  // Proxy is needed
  return true;
};

[gr2m]

Okay so you are setting the HTTP_PROXY environment variable for self-hosted runners as described here?

https://docs.github.com/en/actions/hosting-your-own-runners/using-a-proxy-server-with-self-hosted-runners

And you need to overwrite the environment variable for a single step?

If that's the case, maybe there is a way to entirely delete the env variable for this step and then recover it after?

[Swiftwork]

Hi, your understanding is correct and that is how the environmental variables are set by our corporate overlords. To the best of my knowledge GitHub Actions add these global environment variables to each step, so I cannot call unset HTTP_PROXY on a step before as this will be overwritten again in the next step by the global env. Nor do I think having to store data over steps regarding proxy is a very elegant solution. Seems more like a work around from my perspective. The best I can do is to set HTTP_PROXY: ''.

I believe that the changes in this PR are minimal and focus on making the code less error prone with the addition of a proper isNil() check, rather than type coercion to Boolean which is done now. This means that even if I set proxy: null or proxy: false it will still fallback HTTP_PROXY, which we have established can't really be affected. What are the changes in this PR that don't quite harmonize with you :)?

[gr2m]

changes are fine, I'm just trying to understand the full context of the problem.

Could you add a test to avoid future regressions?

[Swiftwork]

I can try to add a regression test on Sunday. Have a nice easter!

[Swiftwork]

@gr2m good morning!

I have added two tests, do you feel these cover the changes in the PR?

Regards Erik Hughes

[gr2m]

Thanks!

[github-actions]

🎉 This PR is included in version 7.2.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀