fix: process level environment variables should take precedence over env files (#32407)

Author: Tango992

cli/util/watch_env_tracker.rs

@@ -114,6 +114,20 @@ impl WatchEnvTracker {
               let key_os = OsString::from(key);
               let value_os = OsString::from(value);
 
+              // Process-level env vars should always take precedence over env files.
+              if inner.original_env.contains_key(&key_os) {
+                #[allow(clippy::print_stderr)]
+                if log_level.map(|l| l >= log::Level::Debug).unwrap_or(false) {
+                  eprintln!(
+                    "{} Variable '{}' already exists in the process environment, skipping value from '{}'",
+                    colors::yellow("Debug"),
+                    key_os.to_string_lossy(),
+                    file_path.display()
+                  );
+                }
+                continue;
+              }
+
               // Check if this variable is already loaded from a previous file
               if inner.loaded_variables.contains(&key_os) {
                 // Variable already exists from a previous file, skip it
@@ -241,16 +255,46 @@ pub fn load_env_variables_from_env_files(
     return;
   };
 
-  for env_file_name in env_file_names.iter() {
-    match deno_dotenv::from_path(env_file_name) {
-      Ok(_) => (),
+  let original_env_keys: HashSet<OsString> =
+    env::vars_os().map(|(key, _)| key).collect();
+  let mut loaded_keys = HashSet::new();
+
+  for env_file_name in env_file_names.iter().rev() {
+    let iter = match deno_dotenv::from_path_sanitized_iter(env_file_name) {
+      Ok(iter) => iter,
       Err(error) => {
         WatchEnvTracker::handle_dotenv_error(
           error,
           env_file_name,
           flags_log_level,
         );
+        continue;
+      }
+    };
+
+    for item in iter {
+      let (key, value) = match item {
+        Ok(pair) => pair,
+        Err(error) => {
+          WatchEnvTracker::handle_dotenv_error(
+            error,
+            env_file_name,
+            flags_log_level,
+          );
+          break;
+        }
+      };
+
+      let key_os = OsString::from(key);
+      if original_env_keys.contains(&key_os) || loaded_keys.contains(&key_os) {
+        continue;
+      }
+
+      // SAFETY: We're setting environment variables with sanitized key/value strings from a .env file.
+      unsafe {
+        env::set_var(&key_os, value);
       }
+      loaded_keys.insert(key_os);
     }
   }
 }

tests/integration/watcher_tests.rs

@@ -2237,6 +2237,61 @@ console.log("---");
   check_alive_then_kill(child);
 }
 
+#[test(flaky)]
+async fn run_watch_env_file_process_env_precedence() {
+  let t = TempDir::new();
+
+  let env_file = t.path().join(".env");
+  std::fs::write(&env_file, "FOO=from_env_file\nBAR=from_env_file").unwrap();
+
+  let main_script = t.path().join("main.js");
+  std::fs::write(
+    &main_script,
+    r#"
+console.log("FOO:", Deno.env.get("FOO"));
+console.log("BAR:", Deno.env.get("BAR"));
+console.log("---");
+"#,
+  )
+  .unwrap();
+
+  let mut child = util::deno_cmd()
+    .current_dir(t.path())
+    .arg("run")
+    .arg("--watch")
+    .arg("--allow-env")
+    .arg("--env-file=.env")
+    .arg("-L")
+    .arg("debug")
+    .arg(&main_script)
+    .env("NO_COLOR", "1")
+    .env("FOO", "from_process")
+    .piped_output()
+    .spawn()
+    .unwrap();
+  let (mut stdout_lines, mut stderr_lines) = child_lines(&mut child);
+
+  wait_for_watcher("main.js", &mut stderr_lines).await;
+
+  wait_contains("FOO: from_process", &mut stdout_lines).await;
+  wait_contains("BAR: from_env_file", &mut stdout_lines).await;
+  wait_contains("---", &mut stdout_lines).await;
+  wait_contains("Process finished", &mut stderr_lines).await;
+
+  std::fs::write(
+    &env_file,
+    "FOO=updated_from_env_file\nBAR=updated_from_env_file",
+  )
+  .unwrap();
+
+  wait_contains("Restarting", &mut stderr_lines).await;
+  wait_contains("FOO: from_process", &mut stdout_lines).await;
+  wait_contains("BAR: updated_from_env_file", &mut stdout_lines).await;
+  wait_contains("---", &mut stdout_lines).await;
+
+  check_alive_then_kill(child);
+}
+
 #[test(flaky)]
 async fn run_watch_env_file_multiple_files() {
   let t = TempDir::new();

tests/specs/run/env_file_precedence/.env

@@ -0,0 +1,2 @@
+FOO=should_be_overridden_by_process_level_env
+BAR=loaded_from_env_file

tests/specs/run/env_file_precedence/__test__.jsonc

@@ -0,0 +1,4 @@
+{
+  "args": "run -A index.ts",
+  "output": "index.out"
+}

tests/specs/run/env_file_precedence/index.out

@@ -0,0 +1,3 @@
+FOO=overridden_by_process_level_env
+BAR=loaded_from_env_file
+

tests/specs/run/env_file_precedence/index.ts

@@ -0,0 +1,21 @@
+const cmd = new Deno.Command(Deno.execPath(), {
+  args: ["run", "--env-file=.env", "-A", "./spawn.ts"],
+  env: {
+    FOO: "overridden_by_process_level_env",
+  },
+  stdout: "piped",
+  stderr: "piped",
+});
+
+const child = cmd.spawn();
+const { code, stderr, stdout } = await child.output();
+const decodedStderr = new TextDecoder().decode(stderr);
+const decodedStdout = new TextDecoder().decode(stdout);
+
+if (code !== 0) {
+  console.error("Command failed with code " + code);
+  console.error("stderr: " + decodedStderr);
+  Deno.exit(code);
+}
+
+console.log(decodedStdout);

tests/specs/run/env_file_precedence/spawn.ts

@@ -0,0 +1,2 @@
+console.log(`FOO=${Deno.env.get("FOO")}`);
+console.log(`BAR=${Deno.env.get("BAR")}`);