Skip to content

Commit

Permalink
fix: shell-escape arg() in tasks (#3453)
Browse files Browse the repository at this point in the history
Fixes #2906
  • Loading branch information
jdx authored Dec 10, 2024
1 parent 8f40147 commit d21212a
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 68 deletions.
9 changes: 5 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions e2e/run_test
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ within_isolated_env() {
GITHUB_TOKEN="${GITHUB_TOKEN:-}" \
GOPROXY="${GOPROXY:-}" \
HOME="$TEST_HOME" \
LD_LIBRARY_PATH="${LD_LIBRARY_PATH:-}" \
LLVM_PROFILE_FILE="${LLVM_PROFILE_FILE:-}" \
MISE_CACHE_DIR="$MISE_CACHE_DIR" \
MISE_CACHE_PRUNE_AGE="0" \
Expand Down
8 changes: 8 additions & 0 deletions e2e/tasks/test_task_shell
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,11 @@ EOF

assert "mise run shell" "using shell bash"
assert_fail "mise run shell-invalid" "invalid shell"

cat <<EOF >mise.toml
tasks.escapeme = "echo {{arg(name='a')}}"
tasks.escapeme_var = "echo {{arg(name='a', var=true)}}"
EOF

assert "mise run escapeme 'hello world'" "hello world"
assert "mise run escapeme_var hello 'world of mise'" "hello world of mise"
46 changes: 6 additions & 40 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,7 @@ impl Run {
file::make_executable(&file)?;
self.exec(&file, args, task, env, prefix)
} else {
let default_shell = self.clone_default_inline_shell()?;
let (program, args) = self.get_cmd_program_and_args(script, task, args, default_shell);
let (program, args) = self.get_cmd_program_and_args(script, task, args)?;
self.exec_program(&program, &args, task, env, prefix)
}
}
Expand All @@ -460,8 +459,7 @@ impl Run {
}
return Ok((display, args.to_vec()));
}
let default_shell = self.clone_default_file_shell()?;
let shell = self.get_shell(task, default_shell);
let shell = task.shell().unwrap_or(SETTINGS.default_file_shell()?);
trace!("using shell: {}", shell.join(" "));
let mut full_args = shell.clone();
full_args.push(display);
Expand All @@ -476,9 +474,8 @@ impl Run {
script: &str,
task: &Task,
args: &[String],
default_shell: Vec<String>,
) -> (String, Vec<String>) {
let shell = self.get_shell(task, default_shell);
) -> Result<(String, Vec<String>)> {
let shell = task.shell().unwrap_or(self.clone_default_inline_shell()?);
trace!("using shell: {}", shell.join(" "));
let mut full_args = shell.clone();
let mut script = script.to_string();
Expand All @@ -493,46 +490,15 @@ impl Run {
}
}
full_args.push(script);
(full_args[0].clone(), full_args[1..].to_vec())
Ok((full_args[0].clone(), full_args[1..].to_vec()))
}

fn clone_default_inline_shell(&self) -> Result<Vec<String>> {
if let Some(shell) = &self.shell {
Ok(shell_words::split(shell)?)
} else if cfg!(windows) {
Ok(shell_words::split(
&SETTINGS.windows_default_inline_shell_args,
)?)
} else {
Ok(shell_words::split(
&SETTINGS.unix_default_inline_shell_args,
)?)
}
}

fn clone_default_file_shell(&self) -> Result<Vec<String>> {
if cfg!(windows) {
Ok(shell_words::split(
&SETTINGS.windows_default_file_shell_args,
)?)
} else {
Ok(shell_words::split(&SETTINGS.unix_default_file_shell_args)?)
}
}

fn get_shell(&self, task: &Task, default_shell: Vec<String>) -> Vec<String> {
let default_shell = default_shell.clone();
if let Some(shell) = task.shell.clone() {
let shell_cmd = shell
.split_whitespace()
.map(|s| s.to_string())
.collect::<Vec<_>>();
if !shell_cmd.is_empty() && !shell_cmd[0].trim().is_empty() {
return shell_cmd;
}
warn!("invalid shell '{shell}', expected '<program> <argument>' (e.g. sh -c)");
SETTINGS.default_inline_shell()
}
default_shell
}

fn exec_file(
Expand Down
18 changes: 18 additions & 0 deletions src/config/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,24 @@ impl Settings {
let table = toml::from_str(&s)?;
Ok(table)
}

pub fn default_inline_shell(&self) -> Result<Vec<String>> {
let sa = if cfg!(windows) {
&SETTINGS.windows_default_inline_shell_args
} else {
&SETTINGS.unix_default_inline_shell_args
};
Ok(shell_words::split(sa)?)
}

pub fn default_file_shell(&self) -> Result<Vec<String>> {
let sa = if cfg!(windows) {
&SETTINGS.windows_default_file_shell_args
} else {
&SETTINGS.unix_default_file_shell_args
};
Ok(shell_words::split(sa)?)
}
}

impl Display for Settings {
Expand Down
43 changes: 25 additions & 18 deletions src/shell/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::env;
use std::fmt::{Display, Formatter};
use std::path::Path;

use crate::env;
use std::str::FromStr;

mod bash;
mod elvish;
Expand All @@ -22,22 +22,11 @@ pub enum ShellType {

impl ShellType {
pub fn load() -> Option<ShellType> {
let shell = env::var("MISE_SHELL").or(env::var("SHELL")).ok()?;
if shell.ends_with("bash") {
Some(ShellType::Bash)
} else if shell.ends_with("elvish") {
Some(ShellType::Elvish)
} else if shell.ends_with("fish") {
Some(ShellType::Fish)
} else if shell.ends_with("nu") {
Some(ShellType::Nu)
} else if shell.ends_with("xonsh") {
Some(ShellType::Xonsh)
} else if shell.ends_with("zsh") {
Some(ShellType::Zsh)
} else {
None
}
env::var("MISE_SHELL")
.or(env::var("SHELL"))
.ok()?
.parse()
.ok()
}

pub fn as_shell(&self) -> Box<dyn Shell> {
Expand Down Expand Up @@ -65,6 +54,24 @@ impl Display for ShellType {
}
}

impl FromStr for ShellType {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.to_lowercase();
let s = s.rsplit_once('/').map(|(_, s)| s).unwrap_or(&s);
match s {
"bash" | "sh" => Ok(Self::Bash),
"elvish" => Ok(Self::Elvish),
"fish" => Ok(Self::Fish),
"nu" => Ok(Self::Nu),
"xonsh" => Ok(Self::Xonsh),
"zsh" => Ok(Self::Zsh),
_ => Err(format!("unsupported shell type: {s}")),
}
}
}

pub trait Shell: Display {
fn activate(&self, exe: &Path, flags: String) -> String;
fn deactivate(&self) -> String;
Expand Down
17 changes: 16 additions & 1 deletion src/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl Task {
let (spec, scripts) = self.parse_usage_spec(cwd)?;
if has_any_args_defined(&spec) {
Ok(
replace_template_placeholders_with_args(&spec, &scripts, args)?
replace_template_placeholders_with_args(self, &spec, &scripts, args)?
.into_iter()
.map(|s| (s, vec![]))
.collect(),
Expand Down Expand Up @@ -347,6 +347,21 @@ impl Task {
pub fn cf<'a>(&self, config: &'a Config) -> Option<&'a Box<dyn ConfigFile>> {
config.config_files.get(&self.config_source)
}

pub fn shell(&self) -> Option<Vec<String>> {
self.shell.as_ref().and_then(|shell| {
let shell_cmd = shell
.split_whitespace()
.map(|s| s.to_string())
.collect::<Vec<_>>();
if shell_cmd.is_empty() || shell_cmd[0].trim().is_empty() {
warn!("invalid shell '{shell}', expected '<program> <argument>' (e.g. sh -c)");
None
} else {
Some(shell_cmd)
}
})
}
}

fn name_from_path(prefix: impl AsRef<Path>, path: impl AsRef<Path>) -> Result<String> {
Expand Down
36 changes: 31 additions & 5 deletions src/task/task_script_parser.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use crate::config::Config;
use crate::config::{Config, SETTINGS};
use crate::shell::ShellType;
use crate::task::Task;
use crate::tera::{get_tera, BASE_CONTEXT};
use eyre::Result;
use indexmap::IndexMap;
use itertools::Itertools;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use usage::parse::ParseValue;
use xx::regex;

pub struct TaskScriptParser {
Expand Down Expand Up @@ -292,10 +295,26 @@ impl TaskScriptParser {
}

pub fn replace_template_placeholders_with_args(
task: &Task,
spec: &usage::Spec,
scripts: &[String],
args: &[String],
) -> Result<Vec<String>> {
let shell_type: Option<ShellType> = task.shell().unwrap_or(SETTINGS.default_inline_shell()?)[0]
.parse()
.ok();
let escape = |v: &ParseValue| match v {
ParseValue::MultiString(_) => {
// these are already escaped
v.to_string()
}
_ => match shell_type {
Some(ShellType::Zsh | ShellType::Bash | ShellType::Fish) => {
shell_words::quote(&v.to_string()).to_string()
}
_ => v.to_string(),
},
};
let args = vec!["".to_string()]
.into_iter()
.chain(args.iter().cloned())
Expand All @@ -308,13 +327,13 @@ pub fn replace_template_placeholders_with_args(
for (arg, value) in &m.args {
script = script.replace(
&format!("MISE_TASK_ARG:{}:MISE_TASK_ARG", arg.name),
&value.to_string(),
&escape(value),
);
}
for (flag, value) in &m.flags {
script = script.replace(
&format!("MISE_TASK_ARG:{}:MISE_TASK_ARG", flag.name),
&value.to_string(),
&escape(value),
);
}
script = re.replace_all(&script, "").to_string();
Expand All @@ -333,6 +352,7 @@ mod tests {

#[test]
fn test_task_parse_arg() {
let task = Task::default();
let parser = TaskScriptParser::new(None);
let scripts = vec!["echo {{ arg(i=0, name='foo') }}".to_string()];
let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap();
Expand All @@ -341,12 +361,14 @@ mod tests {
assert_eq!(arg0.name, "foo");

let scripts =
replace_template_placeholders_with_args(&spec, &scripts, &["abc".to_string()]).unwrap();
replace_template_placeholders_with_args(&task, &spec, &scripts, &["abc".to_string()])
.unwrap();
assert_eq!(scripts, vec!["echo abc"]);
}

#[test]
fn test_task_parse_arg_var() {
let task = Task::default();
let parser = TaskScriptParser::new(None);
let scripts = vec!["echo {{ arg(var=true) }}".to_string()];
let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap();
Expand All @@ -355,6 +377,7 @@ mod tests {
assert_eq!(arg0.name, "0");

let scripts = replace_template_placeholders_with_args(
&task,
&spec,
&scripts,
&["abc".to_string(), "def".to_string()],
Expand All @@ -365,6 +388,7 @@ mod tests {

#[test]
fn test_task_parse_flag() {
let task = Task::default();
let parser = TaskScriptParser::new(None);
let scripts = vec!["echo {{ flag(name='foo') }}".to_string()];
let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap();
Expand All @@ -373,13 +397,14 @@ mod tests {
assert_eq!(&flag.name, "foo");

let scripts =
replace_template_placeholders_with_args(&spec, &scripts, &["--foo".to_string()])
replace_template_placeholders_with_args(&task, &spec, &scripts, &["--foo".to_string()])
.unwrap();
assert_eq!(scripts, vec!["echo true"]);
}

#[test]
fn test_task_parse_option() {
let task = Task::default();
let parser = TaskScriptParser::new(None);
let scripts = vec!["echo {{ option(name='foo') }}".to_string()];
let (scripts, spec) = parser.parse_run_scripts(&None, &scripts).unwrap();
Expand All @@ -388,6 +413,7 @@ mod tests {
assert_eq!(&option.name, "foo");

let scripts = replace_template_placeholders_with_args(
&task,
&spec,
&scripts,
&["--foo".to_string(), "abc".to_string()],
Expand Down

0 comments on commit d21212a

Please sign in to comment.