Merge pull request #98 from AikidoSec/yarn-tls-errors

Don't set YARN_HTTPS_CA_FILE_PATH, it ignores all system CAs

packages/safe-chain/src/packagemanager/yarn/runYarnCommand.js

@@ -23,7 +23,9 @@ export async function runYarnCommand(args) {
 }
 
 async function fixYarnProxyEnvironmentVariables(env) {
-  // Yarn ignores standard proxy environment variables HTTPS_PROXY and NODE_EXTRA_CA_CERTS
+  // Yarn ignores standard proxy environment variable HTTPS_PROXY
+  // It does respect NODE_EXTRA_CA_CERTS for custom CA certificates though.
+  // Don't use YARN_HTTPS_CA_FILE_PATH though, as it causes to ignore all system CAs
 
   // Yarn v2/v3 and v4+ use different environment variables for proxy and CA certs
   // When setting all variables, yarn returns an error about conflicting variables
@@ -35,10 +37,8 @@ async function fixYarnProxyEnvironmentVariables(env) {
 
   if (majorVersion >= 4) {
     env.YARN_HTTPS_PROXY = env.HTTPS_PROXY;
-    env.YARN_HTTPS_CA_FILE_PATH = env.NODE_EXTRA_CA_CERTS;
   } else if (majorVersion === 2 || majorVersion === 3) {
     env.YARN_HTTPS_PROXY = env.HTTPS_PROXY;
-    env.YARN_CA_FILE_PATH = env.NODE_EXTRA_CA_CERTS;
   }
 }
 

packages/safe-chain/src/packagemanager/yarn/runYarnCommand.spec.js

@@ -0,0 +1,152 @@
+import { describe, it, beforeEach, afterEach, mock } from "node:test";
+import assert from "node:assert";
+
+describe("runYarnCommand", () => {
+  let runYarnCommand;
+  let capturedEnv;
+  let yarnVersion;
+
+  beforeEach(async () => {
+    capturedEnv = null;
+    yarnVersion = "4.1.0"; // Default to v4
+
+    // Mock safeSpawn to capture env and control yarn version
+    mock.module("../../utils/safeSpawn.js", {
+      namedExports: {
+        safeSpawn: async (command, args, options) => {
+          if (args.includes("--version")) {
+            // Mock yarn version check
+            return { status: 0, stdout: yarnVersion };
+          }
+          // Capture the env for assertions
+          capturedEnv = options.env;
+          return { status: 0 };
+        },
+      },
+    });
+
+    // Mock mergeSafeChainProxyEnvironmentVariables to return test env
+    mock.module("../../registryProxy/registryProxy.js", {
+      namedExports: {
+        mergeSafeChainProxyEnvironmentVariables: (env) => {
+          return {
+            ...env,
+            HTTPS_PROXY: "http://localhost:8080",
+            NODE_EXTRA_CA_CERTS: "/path/to/ca-cert.pem",
+          };
+        },
+      },
+    });
+
+    // Mock ui to prevent console output
+    mock.module("../../environment/userInteraction.js", {
+      namedExports: {
+        ui: {
+          writeError: () => {},
+        },
+      },
+    });
+
+    const module = await import("./runYarnCommand.js");
+    runYarnCommand = module.runYarnCommand;
+  });
+
+  afterEach(() => {
+    mock.reset();
+  });
+
+  it("should set YARN_HTTPS_PROXY for Yarn v4+", async () => {
+    yarnVersion = "4.1.0";
+    await runYarnCommand(["add", "lodash"]);
+
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_PROXY,
+      "http://localhost:8080",
+      "YARN_HTTPS_PROXY should be set to the HTTPS_PROXY value"
+    );
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_CA_FILE_PATH,
+      undefined,
+      "YARN_HTTPS_CA_FILE_PATH should NOT be set to avoid overriding system CAs"
+    );
+  });
+
+  it("should set YARN_HTTPS_PROXY for Yarn v3", async () => {
+    yarnVersion = "3.6.4";
+    await runYarnCommand(["add", "lodash"]);
+
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_PROXY,
+      "http://localhost:8080",
+      "YARN_HTTPS_PROXY should be set to the HTTPS_PROXY value"
+    );
+    assert.strictEqual(
+      capturedEnv.YARN_CA_FILE_PATH,
+      undefined,
+      "YARN_CA_FILE_PATH should NOT be set to avoid overriding system CAs"
+    );
+  });
+
+  it("should set YARN_HTTPS_PROXY for Yarn v2", async () => {
+    yarnVersion = "2.4.3";
+    await runYarnCommand(["add", "lodash"]);
+
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_PROXY,
+      "http://localhost:8080",
+      "YARN_HTTPS_PROXY should be set to the HTTPS_PROXY value"
+    );
+    assert.strictEqual(
+      capturedEnv.YARN_CA_FILE_PATH,
+      undefined,
+      "YARN_CA_FILE_PATH should NOT be set to avoid overriding system CAs"
+    );
+  });
+
+  it("should not set Yarn-specific proxy vars for Yarn v1", async () => {
+    yarnVersion = "1.22.19";
+    await runYarnCommand(["add", "lodash"]);
+
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_PROXY,
+      undefined,
+      "YARN_HTTPS_PROXY should not be set for Yarn v1"
+    );
+    assert.strictEqual(
+      capturedEnv.YARN_HTTPS_CA_FILE_PATH,
+      undefined,
+      "YARN_HTTPS_CA_FILE_PATH should not be set for Yarn v1"
+    );
+    assert.strictEqual(
+      capturedEnv.YARN_CA_FILE_PATH,
+      undefined,
+      "YARN_CA_FILE_PATH should not be set for Yarn v1"
+    );
+  });
+
+  it("should preserve NODE_EXTRA_CA_CERTS for all Yarn versions", async () => {
+    for (const version of ["4.1.0", "3.6.4", "2.4.3", "1.22.19"]) {
+      yarnVersion = version;
+      await runYarnCommand(["add", "lodash"]);
+
+      assert.strictEqual(
+        capturedEnv.NODE_EXTRA_CA_CERTS,
+        "/path/to/ca-cert.pem",
+        `NODE_EXTRA_CA_CERTS should be preserved for Yarn ${version}`
+      );
+    }
+  });
+
+  it("should preserve HTTPS_PROXY for all Yarn versions", async () => {
+    for (const version of ["4.1.0", "3.6.4", "2.4.3", "1.22.19"]) {
+      yarnVersion = version;
+      await runYarnCommand(["add", "lodash"]);
+
+      assert.strictEqual(
+        capturedEnv.HTTPS_PROXY,
+        "http://localhost:8080",
+        `HTTPS_PROXY should be preserved for Yarn ${version}`
+      );
+    }
+  });
+});