From 57c2608e9896105ef69d89e88e6216fa07f8d672 Mon Sep 17 00:00:00 2001 From: Scott Olson Date: Mon, 9 Nov 2020 19:08:12 +0000 Subject: [PATCH] fix(cli): Use safe shell escaping in `deno install` (#7613) --- Cargo.lock | 7 ++++ cli/Cargo.toml | 1 + cli/installer.rs | 107 +++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62ace88e28..8829627550 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", @@ -2147,6 +2148,12 @@ dependencies = [ "opaque-debug 0.3.0", ] +[[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" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 6e02e7d9d2..8459139bf0 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 4a177b3eab..dab81bc8d8 100644 --- a/cli/installer.rs +++ b/cli/installer.rs @@ -65,11 +65,15 @@ fn generate_executable_file( file_path: PathBuf, args: Vec, ) -> Result<(), AnyError> { - let args: Vec = args.iter().map(|c| format!("\"{}\"", c)).collect(); + use shell_escape::escape; + let args: Vec = 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' '"'"# + )); + } + } }