From 9d13a7a6ff7be138eb6a63cbe1e332534559469e Mon Sep 17 00:00:00 2001
From: MaxKless <34165455+MaxKless@users.noreply.github.com>
Date: Tue, 27 Aug 2024 20:05:47 +0300
Subject: [PATCH] fix(core): make sure env vars specified in run-commands
 envFile option take priority over other loaded env files (#27583)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
When you specify the `envFile` property on a `nx:run-commands` executor,
values in that file don't override values from other loaded env files
(like `.env` and others that are loaded by the task runner).

## Expected Behavior
`envFile` contents should take precedence. The properties specified in
the `env` option should still override this.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit 3fbaf7f87340a7cf2c2a751edab0d17b23362dea)
---
 .../run-commands/run-commands.impl.spec.ts    | 48 +++++++++++++++++++
 .../run-commands/run-commands.impl.ts         | 11 +++--
 packages/nx/src/tasks-runner/task-env.ts      |  6 +--
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
index 8e9e52577b2c1..95bfb5ab4d198 100644
--- a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
+++ b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
@@ -802,6 +802,7 @@ describe('Run Commands', () => {
       expect(result).toEqual(expect.objectContaining({ success: true }));
       expect(readFile(f)).toEqual('my-value');
     });
+
     it('should prioritize env setting over local dotenv files', async () => {
       writeFileSync('.env', 'MY_ENV_VAR=from-dotenv');
       const root = dirSync().name;
@@ -825,6 +826,32 @@ describe('Run Commands', () => {
       expect(result).toEqual(expect.objectContaining({ success: true }));
       expect(readFile(f)).toEqual('from-options');
     });
+
+    it('should prioritize env setting over dotenv file from envFile option', async () => {
+      const devEnv = fileSync().name;
+      writeFileSync(devEnv, 'MY_ENV_VAR=from-dotenv');
+      const root = dirSync().name;
+      const f = fileSync().name;
+      const result = await runCommands(
+        {
+          commands: [
+            {
+              command: `echo "$MY_ENV_VAR" >> ${f}`,
+            },
+          ],
+          env: {
+            MY_ENV_VAR: 'from-options',
+            envFile: devEnv,
+          },
+          parallel: true,
+          __unparsed__: [],
+        },
+        { root } as any
+      );
+
+      expect(result).toEqual(expect.objectContaining({ success: true }));
+      expect(readFile(f)).toEqual('from-options');
+    });
   });
 
   describe('dotenv', () => {
@@ -896,6 +923,27 @@ describe('Run Commands', () => {
       expect(readFile(f)).toContain('https://nx.dev/');
     });
 
+    it('should override environment variables that are present in both the specified .env file and other loaded ones', async () => {
+      const devEnv = fileSync().name;
+      writeFileSync(devEnv, 'NRWL_SITE=https://nrwl.io/override');
+      const f = fileSync().name;
+      let result = await runCommands(
+        {
+          commands: [
+            {
+              command: `echo $NRWL_SITE >> ${f}`,
+            },
+          ],
+          envFile: devEnv,
+          __unparsed__: [],
+        },
+        context
+      );
+
+      expect(result).toEqual(expect.objectContaining({ success: true }));
+      expect(readFile(f)).toContain('https://nrwl.io/override');
+    });
+
     it('should error if the specified .env file does not exist', async () => {
       const f = fileSync().name;
       try {
diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.ts b/packages/nx/src/executors/run-commands/run-commands.impl.ts
index 14b046fde9534..0656a10a6d1ab 100644
--- a/packages/nx/src/executors/run-commands/run-commands.impl.ts
+++ b/packages/nx/src/executors/run-commands/run-commands.impl.ts
@@ -21,7 +21,7 @@ const childProcesses = new Set<ChildProcess | PseudoTtyProcess>();
 
 function loadEnvVarsFile(path: string, env: Record<string, string> = {}) {
   unloadDotEnvFile(path, env);
-  const result = loadAndExpandDotEnvFile(path, env);
+  const result = loadAndExpandDotEnvFile(path, env, true);
   if (result.error) {
     throw result.error;
   }
@@ -484,14 +484,19 @@ function processEnv(
   envFile?: string
 ) {
   const localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() });
-  const res = {
+  let res = {
     ...process.env,
     ...localEnv,
-    ...env,
   };
+  // env file from envFile option takes priority over process env
   if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
     loadEnvVars(envFile, res);
   }
+  // env variables from env option takes priority over everything else
+  res = {
+    ...res,
+    ...env,
+  };
   // need to override PATH to make sure we are using the local node_modules
   if (localEnv.PATH) res.PATH = localEnv.PATH; // UNIX-like
   if (localEnv.Path) res.Path = localEnv.Path; // Windows
diff --git a/packages/nx/src/tasks-runner/task-env.ts b/packages/nx/src/tasks-runner/task-env.ts
index cbc7b4775ed5b..462de5cb73b5e 100644
--- a/packages/nx/src/tasks-runner/task-env.ts
+++ b/packages/nx/src/tasks-runner/task-env.ts
@@ -124,9 +124,9 @@ function getNxEnvVariablesForTask(
 
 /**
  * This function loads a .env file and expands the variables in it.
- * It is going to override existing environmentVariables.
- * @param filename
- * @param environmentVariables
+ * @param filename the .env file to load
+ * @param environmentVariables the object to load environment variables into
+ * @param override whether to override existing environment variables
  */
 export function loadAndExpandDotEnvFile(
   filename: string,