fix: Audit 09/05

CHANGELOG.md

@@ -2,6 +2,21 @@
 
 ## Unreleased
 
+#### 🚀 Updates
+
+- Added caching around `bun bun.lockb` commands, instead of running them for every task.
+- Updated and loosened identifier naming restrictions.
+- Updated environment variable substitution to support different outputs when a variable is missing,
+  based on a trailing flag syntax.
+  - `$FOO` or `${FOO}` - If variable is missing, keeps the original syntax (current default).
+  - `$FOO?` or `${FOO?}` - If variable is missing, replaces with an empty string.
+  - `$FOO!` or `${FOO!}` - Ignores variable substitution and preserves the syntax (without !).
+
+#### 🐞 Fixes
+
+- Fixed an issue where an affected task with files that contain non-standard characters would fail
+  to run because Bash expansion fails. We now quote file paths that contain `*`, `$`, `+`, and `[]`.
+
 #### ⚙️ Internal
 
 - Updated Rust to v1.81.

crates/cli/tests/snapshots/run_test__output_styles__stream.snap

@@ -4,13 +4,11 @@ expression: assert.output()
 ---
 ▪▪▪▪ npm install
 ▪▪▪▪ outputStyles:stream
-       outputStyles:stream | stdout
+outputStyles:stream | stdout
 ▪▪▪▪ outputStyles:stream (100ms)
 ▪▪▪▪ outputStyles:streamPrimary (no op)
 
 Tasks: 2 completed
  Time: 100ms
 
-       outputStyles:stream | stderr
-
-
+outputStyles:stream | stderr

crates/common/src/id.rs

@@ -13,7 +13,7 @@ pub static ID_CLEAN: OnceLock<Regex> = OnceLock::new();
 
 #[derive(Error, Debug, Diagnostic)]
 #[diagnostic(code(id::invalid_format))]
-#[error("Invalid format for {}, may only contain alpha-numeric characters, dashes (-), slashes (/), underscores (_), and dots (.), and must start with an alpha character.", .0.style(Style::Id))]
+#[error("Invalid format for {}, may only contain alpha-numeric characters, dashes (-), slashes (/), underscores (_), and periods (.).", .0.style(Style::Id))]
 pub struct IdError(String);
 
 #[derive(Clone, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
@@ -24,9 +24,8 @@ impl Id {
         let id = id.as_ref();
 
         // The @ is to support npm package scopes!
-        let pattern = ID_PATTERN.get_or_init(|| {
-            Regex::new(format!("^([A-Za-z@_]{{1}}{})$", ID_CHARS).as_str()).unwrap()
-        });
+        let pattern =
+            ID_PATTERN.get_or_init(|| Regex::new(format!("^(@?{})$", ID_CHARS).as_str()).unwrap());
 
         if !pattern.is_match(id) {
             return Err(IdError(id.to_owned()));
@@ -40,7 +39,7 @@ impl Id {
         // with a leading -, causing pattern failures
         let id = id.as_ref().replace('@', "");
 
-        // This is to clean and ID and remove unwanted characters
+        // This is to clean an ID and remove unwanted characters
         let pattern = ID_CLEAN.get_or_init(|| Regex::new(r"[^0-9A-Za-z/\._-]+").unwrap());
 
         Id::new(pattern.replace_all(&id, "-"))

crates/config/src/patterns.rs

@@ -12,8 +12,14 @@ macro_rules! pattern {
 pattern!(ENV_VAR, "\\$([A-Z0-9_]+)"); // $ENV_VAR
 pattern!(ENV_VAR_DISTINCT, "^\\$([A-Z0-9_]+)$"); // $ENV_VAR
 pattern!(ENV_VAR_GLOB_DISTINCT, "^\\$([A-Z0-9_*]+)$"); // $ENV_*
-pattern!(ENV_VAR_SUBSTITUTE, "\\$(?:\\{([A-Z0-9_]+)\\}|([A-Z0-9_]+))"); // $ENV_VAR, ${ENV_VAR}
-pattern!(ENV_VAR_SUBSTITUTE_STRICT, "\\$\\{([A-Z0-9_]+)\\}"); // ${ENV_VAR}
+pattern!(
+    ENV_VAR_SUBSTITUTE,
+    "(?:\\$\\{(?P<name1>[A-Z0-9_]+)(?P<flag1>[!?]{1})?\\})|(?:\\$(?P<name2>[A-Z0-9_]+)(?P<flag2>[!?]{1})?)"
+); // $ENV_VAR, ${ENV_VAR}
+pattern!(
+    ENV_VAR_SUBSTITUTE_STRICT,
+    "\\$\\{(?P<name>[A-Z0-9_]+)(?P<flag>[!?]{1})?\\}"
+); // ${ENV_VAR}
 
 // Task tokens
 

crates/project-expander/src/expander_context.rs

@@ -45,30 +45,87 @@ pub fn substitute_env_var(
     patterns::ENV_VAR_SUBSTITUTE.replace_all(
         value,
         |caps: &patterns::Captures| {
-            // First with wrapping {} then without: ${FOO} -> $FOO
-            let name = caps.get(1).or_else(|| caps.get(2)).unwrap().as_str();
+            let Some(name) = caps.name("name1")
+                .or_else(|| caps.name("name2"))
+                .map(|cap| cap.as_str())
+                else {
+                return String::new();
+            };
+
+            let flag = caps.name("flag1").or_else(|| caps.name("flag2")).map(|cap| cap.as_str());
 
             // If the variable is referencing itself, don't pull
             // from the local map, and instead only pull from the
             // system environment. Otherwise we hit recursion!
-            let maybe_var = if !base_name.is_empty() && base_name == name {
-                env::var(name).ok()
-            } else {
-                env_map.get(name).cloned().or_else(|| env::var(name).ok())
+            let get_replacement_value = || {
+                if !base_name.is_empty() && base_name == name {
+                    env::var(name).ok()
+                } else {
+                    env_map.get(name).cloned().or_else(|| env::var(name).ok())
+                }
             };
 
-            match maybe_var {
-                Some(var) => var,
-                None => {
+            match flag {
+                // Don't substitute
+                Some("!") => {
+                    format!("${name}")
+                },
+                // Substitute with empty string when missing
+                Some("?") =>{
+                    debug!(
+                        "Task value `{}` contains the environment variable ${}, but this variable is not set. Replacing with an empty value.",
+                        value,
+                        name
+                    );
+
+                    get_replacement_value().unwrap_or_default()
+                },
+                // Substitute with self when missing
+                _ => {
                     debug!(
-                        "Task value `{}` contains the environment variable ${}, but this variable is not set. Not substituting and keeping as-is.",
+                        "Task value `{}` contains the environment variable ${}, but this variable is not set. Not substituting and keeping as-is. Append with ? or ! to change outcome.",
                         value,
                         name
                     );
 
-                    caps.get(0).unwrap().as_str().to_owned()
+                    get_replacement_value()
+                        .unwrap_or_else(|| caps.get(0).unwrap().as_str().to_owned())
                 }
             }
         })
     .to_string()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn handles_flags_when_missing() {
+        let envs = FxHashMap::default();
+
+        assert_eq!(substitute_env_var("", "$KEY", &envs), "$KEY");
+        assert_eq!(substitute_env_var("", "${KEY}", &envs), "${KEY}");
+
+        assert_eq!(substitute_env_var("", "$KEY!", &envs), "$KEY");
+        assert_eq!(substitute_env_var("", "${KEY!}", &envs), "$KEY");
+
+        assert_eq!(substitute_env_var("", "$KEY?", &envs), "");
+        assert_eq!(substitute_env_var("", "${KEY?}", &envs), "");
+    }
+
+    #[test]
+    fn handles_flags_when_not_missing() {
+        let mut envs = FxHashMap::default();
+        envs.insert("KEY".to_owned(), "value".to_owned());
+
+        assert_eq!(substitute_env_var("", "$KEY", &envs), "value");
+        assert_eq!(substitute_env_var("", "${KEY}", &envs), "value");
+
+        assert_eq!(substitute_env_var("", "$KEY!", &envs), "$KEY");
+        assert_eq!(substitute_env_var("", "${KEY!}", &envs), "$KEY");
+
+        assert_eq!(substitute_env_var("", "$KEY?", &envs), "value");
+        assert_eq!(substitute_env_var("", "${KEY?}", &envs), "value");
+    }
+}

crates/project-graph/src/projects_locator.rs

@@ -4,11 +4,11 @@ use moon_config::{ProjectSourceEntry, ProjectsSourcesList};
 use moon_vcs::BoxedVcs;
 use starbase_utils::{fs, glob};
 use std::path::Path;
-use tracing::{instrument, warn};
+use tracing::{debug, instrument, warn};
 
 /// Infer a project name from a source path, by using the name of
 /// the project folder.
-pub fn infer_project_id_and_source(
+fn infer_project_id_and_source(
     path: &str,
     workspace_root: &Path,
 ) -> miette::Result<ProjectSourceEntry> {
@@ -113,10 +113,17 @@ where
                 }
             }
 
-            sources.push(infer_project_id_and_source(
-                &project_source,
-                workspace_root,
-            )?)
+            let (id, source) = infer_project_id_and_source(&project_source, workspace_root)?;
+
+            if id.starts_with(".") {
+                debug!(
+                    id = id.as_str(),
+                    source = source.as_str(),
+                    "Received a project for a hidden folder. These are not supported through globs, but can be mapped explicitly with project sources!"
+                );
+            } else {
+                sources.push((id, source));
+            }
         }
     }
 

crates/project-graph/tests/project_graph_test.rs

@@ -193,8 +193,7 @@ mod project_graph {
         }
 
         #[tokio::test]
-        #[should_panic(expected = "Invalid format for .foo")]
-        async fn errors_for_dot_folders() {
+        async fn filters_dot_folders() {
             let sandbox = create_sandbox("dependencies");
             sandbox.create_file(".foo/moon.yml", "");
 

crates/task-runner/src/command_builder.rs

@@ -272,8 +272,16 @@ impl<'task> CommandBuilder<'task> {
                 self.command.arg_if_missing(".");
             } else {
                 // Mimic relative from ("./")
-                self.command
-                    .args(files.iter().map(|file| format!("./{file}")));
+                self.command.args(files.iter().map(|file| {
+                    let arg = format!("./{file}");
+
+                    // Escape files with special characters
+                    if arg.contains(['*', '$', '+', '[', ']']) {
+                        format!("\"{arg}\"")
+                    } else {
+                        arg
+                    }
+                }));
             }
         }
 

crates/task-runner/src/command_executor.rs

@@ -250,12 +250,16 @@ impl<'task> CommandExecutor<'task> {
         };
 
         // Transitive targets may run concurrently, so differentiate them with a prefix.
-        if self.stream && (is_ci_env() || !is_primary || context.primary_targets.len() > 1) {
-            let prefix_max_width = context
-                .primary_targets
-                .iter()
-                .map(|target| target.id.len())
-                .max();
+        if !is_primary || is_ci_env() || context.primary_targets.len() > 1 {
+            let prefix_max_width = if context.primary_targets.len() > 1 {
+                context
+                    .primary_targets
+                    .iter()
+                    .map(|target| target.id.len())
+                    .max()
+            } else {
+                None
+            };
 
             self.command
                 .set_prefix(&self.task.target.id, prefix_max_width);

crates/task-runner/tests/__fixtures__/builder/project/moon.yml

@@ -7,3 +7,4 @@ tasks:
     inputs:
       - 'input.txt'
       - 'file.*'
+      - 'routes/**/*'

crates/task-runner/tests/command_builder_test.rs

@@ -367,5 +367,43 @@ mod command_builder {
 
             assert_eq!(get_args(&command), vec!["arg", "--opt", "./input.txt"]);
         }
+
+        #[tokio::test]
+        async fn quotes_files_with_special_chars() {
+            let container = TaskRunnerContainer::new("builder").await;
+
+            let mut context = ActionContext::default();
+            context.affected_only = true;
+            context.touched_files.insert("project/file.txt".into());
+            context.touched_files.insert("project/routes/*.ts".into());
+            context
+                .touched_files
+                .insert("project/routes/[id].ts".into());
+            context
+                .touched_files
+                .insert("project/routes/$slug.tsx".into());
+            context
+                .touched_files
+                .insert("project/routes/+page.svelte".into());
+
+            let command = container
+                .create_command_with_config(context, |task, _| {
+                    task.options.affected_files = Some(TaskOptionAffectedFiles::Args);
+                })
+                .await;
+
+            assert_eq!(
+                get_args(&command),
+                vec![
+                    "arg",
+                    "--opt",
+                    "./file.txt",
+                    "\"./routes/$slug.tsx\"",
+                    "\"./routes/*.ts\"",
+                    "\"./routes/+page.svelte\"",
+                    "\"./routes/[id].ts\""
+                ]
+            );
+        }
     }
 }

legacy/bun/lang/src/bun_lockb.rs

@@ -2,11 +2,12 @@ use cached::proc_macro::cached;
 use miette::IntoDiagnostic;
 use moon_lang::LockfileDependencyVersions;
 use rustc_hash::FxHashMap;
+use std::sync::Arc;
 use yarn_lock_parser::{parse_str, Entry};
 
 #[cached(result)]
 pub fn load_lockfile_dependencies(
-    lockfile_text: String,
+    lockfile_text: Arc<String>,
 ) -> miette::Result<LockfileDependencyVersions> {
     let mut deps: LockfileDependencyVersions = FxHashMap::default();
 

legacy/bun/tool/Cargo.toml

@@ -16,6 +16,7 @@ moon_utils = { path = "../../core/utils" }
 miette = { workspace = true }
 proto_core = { workspace = true }
 rustc-hash = { workspace = true }
+scc = { workspace = true }
 starbase_utils = { workspace = true }
 tracing = { workspace = true }