Skip to content

Commit 95f3c50

Browse files
committed
fix(cli): tighten Wasm tool validation
Signed-off-by: akrm al-hakimi <alhakimiakrmj@gmail.com>
1 parent b7c661c commit 95f3c50

1 file changed

Lines changed: 68 additions & 7 deletions

File tree

host/src/wasm_host_fns.rs

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,17 @@ impl WasmToolOptions {
6161
}
6262

6363
let mut merged_env = Vec::with_capacity(env.len() + inherit_env.len());
64-
for spec in env {
65-
merged_env.push(parse_env_pair(spec)?);
66-
}
6764
for key in inherit_env {
6865
if key.is_empty() {
6966
bail!("--tool-wasi-env-inherit key must not be empty");
7067
}
7168
let value = std::env::var(key)
7269
.with_context(|| format!("inherit environment variable {key}"))?;
73-
merged_env.push((key.clone(), value));
70+
set_env_pair(&mut merged_env, key.clone(), value);
71+
}
72+
for spec in env {
73+
let (key, value) = parse_env_pair(spec)?;
74+
set_env_pair(&mut merged_env, key, value);
7475
}
7576

7677
Ok(Self {
@@ -134,7 +135,7 @@ impl WasmTool {
134135
}
135136

136137
pub fn invoke(&self, args: Value) -> Result<Value> {
137-
let request = serde_json::json!({ "name": self.name, "args": args });
138+
let request = serde_json::json!({ "name": &self.name, "args": args });
138139
let stdin = serde_json::to_vec(&request)?;
139140
let stdout = MemoryOutputPipe::new(self.options.output_limit);
140141
let stderr = MemoryOutputPipe::new(self.options.output_limit);
@@ -244,10 +245,15 @@ fn parse_wasi_dir(spec: &str, read_only: bool) -> Result<WasiDir> {
244245
let (host, guest) = if let Some(idx) = spec.rfind(':') {
245246
let (host, guest) = spec.split_at(idx);
246247
let guest = &guest[1..];
247-
if guest.starts_with('/') || guest == "." || guest.starts_with("./") {
248+
if is_windows_drive_path(spec, idx) {
249+
(spec, "/host")
250+
} else if guest.starts_with('/') || guest == "." || guest.starts_with("./") {
248251
(host, guest)
249252
} else {
250-
(spec, "/host")
253+
bail!(
254+
"invalid WASI preopen guest path {:?}: expected absolute path, '.', or './path'",
255+
guest
256+
);
251257
}
252258
} else {
253259
(spec, "/host")
@@ -279,6 +285,31 @@ fn parse_env_pair(spec: &str) -> Result<(String, String)> {
279285
Ok((key.to_string(), value.to_string()))
280286
}
281287

288+
fn set_env_pair(env: &mut Vec<(String, String)>, key: String, value: String) {
289+
if let Some((_, existing)) = env
290+
.iter_mut()
291+
.find(|(existing_key, _)| existing_key == &key)
292+
{
293+
*existing = value;
294+
} else {
295+
env.push((key, value));
296+
}
297+
}
298+
299+
fn is_windows_drive_path(spec: &str, colon_idx: usize) -> bool {
300+
colon_idx == 1
301+
&& spec
302+
.as_bytes()
303+
.first()
304+
.map(|b| b.is_ascii_alphabetic())
305+
.unwrap_or(false)
306+
&& spec
307+
.as_bytes()
308+
.get(2)
309+
.map(|b| *b == b'/' || *b == b'\\')
310+
.unwrap_or(false)
311+
}
312+
282313
fn pipe_text(pipe: &MemoryOutputPipe) -> String {
283314
String::from_utf8_lossy(&pipe.contents()).into_owned()
284315
}
@@ -847,6 +878,27 @@ mod tests {
847878
assert_eq!(opts.dirs[0].guest, "/host");
848879
}
849880

881+
#[test]
882+
fn cli_options_use_last_explicit_env_value_for_duplicate_keys() {
883+
let opts = WasmToolOptions::from_cli(
884+
&[],
885+
&[],
886+
&[
887+
"A=first".to_string(),
888+
"B=only".to_string(),
889+
"A=second".to_string(),
890+
],
891+
&[],
892+
1,
893+
1,
894+
)
895+
.unwrap();
896+
assert_eq!(
897+
opts.env,
898+
vec![("A".into(), "second".into()), ("B".into(), "only".into())]
899+
);
900+
}
901+
850902
#[test]
851903
fn cli_options_reject_invalid_values() {
852904
let dir = tempdir("invalid-options");
@@ -857,6 +909,15 @@ mod tests {
857909
);
858910
assert!(WasmToolOptions::from_cli(&[], &[], &["=value".to_string()], &[], 1, 1).is_err());
859911
assert!(WasmToolOptions::from_cli(&[], &[], &[], &["".to_string()], 1, 1).is_err());
912+
assert!(WasmToolOptions::from_cli(
913+
&[format!("{}:relative", dir.display())],
914+
&[],
915+
&[],
916+
&[],
917+
1,
918+
1,
919+
)
920+
.is_err());
860921
assert!(WasmToolOptions::from_cli(
861922
&[
862923
format!("{}:/dup", dir.display()),

0 commit comments

Comments
 (0)