fix(cli): Use safe shell escaping in `deno install` (#7613)

3 files changed, 98 insertions, 17 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 62ace88e2..882962755 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -444,6 +444,7 @@ dependencies = [
  "rustyline-derive",
  "semver-parser 0.9.0",
  "serde",
+ "shell-escape",
  "sourcemap",
  "swc_bundler",
  "swc_common",
@@ -2148,6 +2149,12 @@ dependencies = [
 ]
 
 [[package]]
+name = "shell-escape"
+version = "0.1.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "45bb67a18fa91266cc7807181f62f9178a6873bfad7dc788c42e6430db40184f"
+
+[[package]]
 name = "signal-hook-registry"
 version = "1.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index 6e02e7d9d..8459139bf 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -61,6 +61,7 @@ ring = "0.16.15"
 rustyline = { version = "6.3.0", default-features = false }
 rustyline-derive = "0.3.1"
 serde = { version = "1.0.116", features = ["derive"] }
+shell-escape = "0.1.5"
 sys-info = "0.7.0"
 sourcemap = "6.0.1"
 swc_bundler = "=0.16.1"
diff --git a/cli/installer.rs b/cli/installer.rs
index 4a177b3ea..dab81bc8d 100644
--- a/cli/installer.rs
+++ b/cli/installer.rs
@@ -65,11 +65,15 @@ fn generate_executable_file(
   file_path: PathBuf,
   args: Vec<String>,
 ) -> Result<(), AnyError> {
-  let args: Vec<String> = args.iter().map(|c| format!("\"{}\"", c)).collect();
+  use shell_escape::escape;
+  let args: Vec<String> = args
+    .into_iter()
+    .map(|c| escape(c.into()).into_owned())
+    .collect();
   let template = format!(
     r#"#!/bin/sh
 # generated by deno install
-deno {} "$@"
+exec deno {} "$@"
 "#,
     args.join(" "),
   );
@@ -429,8 +433,13 @@ mod tests {
     // It's annoying when shell scripts don't have NL at the end.
     assert_eq!(content.chars().last().unwrap(), '\n');
 
-    assert!(content
-      .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    if cfg!(windows) {
+      assert!(content
+        .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    } else {
+      assert!(content
+        .contains(r#"run 'http://localhost:4545/cli/tests/echo_server.ts'"#));
+    }
     if let Some(home) = original_home {
       env::set_var("HOME", home);
     }
@@ -470,9 +479,15 @@ mod tests {
 
     let content = fs::read_to_string(file_path).unwrap();
     println!("this is the file path {:?}", content);
-    assert!(content.contains(
-      r#""run" "--unstable" "http://localhost:4545/cli/tests/echo_server.ts"#
-    ));
+    if cfg!(windows) {
+      assert!(content.contains(
+        r#""run" "--unstable" "http://localhost:4545/cli/tests/echo_server.ts""#
+      ));
+    } else {
+      assert!(content.contains(
+        r#"run --unstable 'http://localhost:4545/cli/tests/echo_server.ts'"#
+      ));
+    }
   }
 
   #[test]
@@ -498,8 +513,13 @@ mod tests {
 
     assert!(file_path.exists());
     let content = fs::read_to_string(file_path).unwrap();
-    assert!(content
-      .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    if cfg!(windows) {
+      assert!(content
+        .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    } else {
+      assert!(content
+        .contains(r#"run 'http://localhost:4545/cli/tests/echo_server.ts'"#));
+    }
   }
 
   #[test]
@@ -525,8 +545,13 @@ mod tests {
 
     assert!(file_path.exists());
     let content = fs::read_to_string(file_path).unwrap();
-    assert!(content
-      .contains(r#""run" "http://localhost:4545/cli/tests/subdir/main.ts""#));
+    if cfg!(windows) {
+      assert!(content
+        .contains(r#""run" "http://localhost:4545/cli/tests/subdir/main.ts""#));
+    } else {
+      assert!(content
+        .contains(r#"run 'http://localhost:4545/cli/tests/subdir/main.ts'"#));
+    }
   }
 
   #[test]
@@ -552,8 +577,13 @@ mod tests {
 
     assert!(file_path.exists());
     let content = fs::read_to_string(file_path).unwrap();
-    assert!(content
-      .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    if cfg!(windows) {
+      assert!(content
+        .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    } else {
+      assert!(content
+        .contains(r#"run 'http://localhost:4545/cli/tests/echo_server.ts'"#));
+    }
   }
 
   #[test]
@@ -582,8 +612,13 @@ mod tests {
 
     assert!(file_path.exists());
     let content = fs::read_to_string(file_path).unwrap();
-    assert!(content
-      .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    if cfg!(windows) {
+      assert!(content
+        .contains(r#""run" "http://localhost:4545/cli/tests/echo_server.ts""#));
+    } else {
+      assert!(content
+        .contains(r#"run 'http://localhost:4545/cli/tests/echo_server.ts'"#));
+    }
     if let Some(install_root) = original_install_root {
       env::set_var("DENO_INSTALL_ROOT", install_root);
     }
@@ -618,8 +653,11 @@ mod tests {
 
     assert!(file_path.exists());
     let content = fs::read_to_string(file_path).unwrap();
-    dbg!(&content);
-    assert!(content.contains(r#""run" "--allow-read" "--allow-net" "--quiet" "--no-check" "http://localhost:4545/cli/tests/echo_server.ts" "--foobar""#));
+    if cfg!(windows) {
+      assert!(content.contains(r#""run" "--allow-read" "--allow-net" "--quiet" "--no-check" "http://localhost:4545/cli/tests/echo_server.ts" "--foobar""#));
+    } else {
+      assert!(content.contains(r#"run --allow-read --allow-net --quiet --no-check 'http://localhost:4545/cli/tests/echo_server.ts' --foobar"#));
+    }
   }
 
   #[test]
@@ -737,4 +775,39 @@ mod tests {
     let content = fs::read_to_string(file_path).unwrap();
     assert!(content == "{}");
   }
+
+  // TODO: enable on Windows after fixing batch escaping
+  #[cfg(not(windows))]
+  #[test]
+  fn install_shell_escaping() {
+    let temp_dir = TempDir::new().expect("tempdir fail");
+    let bin_dir = temp_dir.path().join("bin");
+    std::fs::create_dir(&bin_dir).unwrap();
+
+    install(
+      Flags::default(),
+      "http://localhost:4545/cli/tests/echo_server.ts",
+      vec!["\"".to_string()],
+      Some("echo_test".to_string()),
+      Some(temp_dir.path().to_path_buf()),
+      false,
+    )
+    .expect("Install failed");
+
+    let mut file_path = bin_dir.join("echo_test");
+    if cfg!(windows) {
+      file_path = file_path.with_extension("cmd");
+    }
+
+    assert!(file_path.exists());
+    let content = fs::read_to_string(file_path).unwrap();
+    println!("{}", content);
+    if cfg!(windows) {
+      // TODO: see comment above this test
+    } else {
+      assert!(content.contains(
+        r#"run 'http://localhost:4545/cli/tests/echo_server.ts' '"'"#
+      ));
+    }
+  }
 }