Skip to content

Commit

Permalink
fix(windows): fix env var glob casing (#9429)
Browse files Browse the repository at this point in the history
### Description

In #9424 a user had `path` as
their Windows env var instead of `Path` or `PATH`. On Windows
environment variables are case insensitive e.g. `process.env.PATH` and
`process.env.Path` would both return the same. While fetching them might
be the same
[`std::env::vars`](https://doc.rust-lang.org/std/env/fn.vars.html)
returns the environment variable names cased as they were declared even
though [`std::env::var`](https://doc.rust-lang.org/std/env/fn.var.html)
is case insensitive on Windows.

Take for example the following code:
```
for (key, value) in std::env::vars() {
  println!("{key}: {value}");
  let lowercase = key.to_ascii_lowercase();
  println!("{lowercase}: {:?}", std::env::var(&lowercase));
}
```
On Windows it outputs:
```
...
PUBLIC: C:\Users\Public
public: Ok("C:\\Users\\Public")
SESSIONNAME: Console
sessionname: Ok("Console")
SystemDrive: C:
systemdrive: Ok("C:")
SystemRoot: C:\WINDOWS
systemroot: Ok("C:\\WINDOWS")
TEMP: C:\Users\Chris\AppData\Local\Temp
temp: Ok("C:\\Users\\Chris\\AppData\\Local\\Temp")
TMP: C:\Users\Chris\AppData\Local\Temp
tmp: Ok("C:\\Users\\Chris\\AppData\\Local\\Temp")
```

On macOs it outputs:
```
...
TMPDIR: /var/folders/3m/rxkycvgs5jgfvs0k9xcgp6km0000gn/T/
tmpdir: Err(NotPresent)
TMUX: /private/tmp/tmux-501/default,3104,0
tmux: Err(NotPresent)
TMUX_PANE: %4
tmux_pane: Err(NotPresent)
USER: olszewski
user: Err(NotPresent)
USER_ZDOTDIR: /Users/olszewski
user_zdotdir: Err(NotPresent)
```

Previously we had special casing for well known Windows env vars that
were set differently between Command Prompt and Powershell, but would
break if a user overwrote `Path` or `PATH` to `path`. This was brittle
and a correct solution would require that we include all casing
variations of all environment variables and users would also need to
know how env vars were declared on their Windows system.

This fix is to make our env var wildcard handling case in
### Testing Instructions

Added unit test to make sure we
  • Loading branch information
chris-olszewski authored Nov 14, 2024
1 parent 48d6762 commit e680dab
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
28 changes: 25 additions & 3 deletions crates/turborepo-env/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
ops::{Deref, DerefMut},
};

use regex::Regex;
use regex::RegexBuilder;
use serde::Serialize;
use sha2::{Digest, Sha256};
use thiserror::Error;
Expand Down Expand Up @@ -180,8 +180,13 @@ impl EnvironmentVariableMap {
let include_regex_string = format!("^({})$", include_patterns.join("|"));
let exclude_regex_string = format!("^({})$", exclude_patterns.join("|"));

let include_regex = Regex::new(&include_regex_string)?;
let exclude_regex = Regex::new(&exclude_regex_string)?;
let case_insensitive = cfg!(windows);
let include_regex = RegexBuilder::new(&include_regex_string)
.case_insensitive(case_insensitive)
.build()?;
let exclude_regex = RegexBuilder::new(&exclude_regex_string)
.case_insensitive(case_insensitive)
.build()?;
for (env_var, env_value) in &self.0 {
if !include_patterns.is_empty() && include_regex.is_match(env_var) {
output.inclusions.insert(env_var.clone(), env_value.clone());
Expand Down Expand Up @@ -304,6 +309,8 @@ pub fn get_global_hashable_env_vars(
mod tests {
use test_case::test_case;

use super::*;

#[test_case("LITERAL_\\*", "LITERAL_\\*" ; "literal star")]
#[test_case("\\*LEADING", "\\*LEADING" ; "leading literal star")]
#[test_case("\\!LEADING", "\\\\!LEADING" ; "leading literal bang")]
Expand All @@ -313,4 +320,19 @@ mod tests {
let actual = super::wildcard_to_regex_pattern(pattern);
assert_eq!(actual, expected);
}

#[test]
fn test_case_sensitivity() {
let start = EnvironmentVariableMap(
vec![("Turbo".to_string(), "true".to_string())]
.into_iter()
.collect(),
);
let actual = start.from_wildcards(&["TURBO"]).unwrap();
if cfg!(windows) {
assert_eq!(actual.get("Turbo").map(|s| s.as_str()), Some("true"));
} else {
assert_eq!(actual.get("Turbo"), None);
}
}
}
6 changes: 0 additions & 6 deletions crates/turborepo-lib/src/task_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,12 +506,6 @@ impl<'a> TaskHasher<'a> {
"PROGRAMDATA",
"SYSTEMROOT",
"SYSTEMDRIVE",
// Powershell casing of env variables
"Path",
"ProgramData",
"SystemRoot",
"AppData",
"SystemDrive",
])?;
let tracker_env = self
.task_hash_tracker
Expand Down

0 comments on commit e680dab

Please sign in to comment.