From fb35cd0ef496fee9aa65daadf542057f18d6063f Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 9 Sep 2021 08:38:47 -0400 Subject: [PATCH] fix: permission prompt stuffing (#11931) Fixes #9750 --- cli/tests/integration/run_tests.rs | 109 ++++++++++++++---- .../testdata/061_permissions_request.ts.out | 3 - .../062_permissions_request_global.ts.out | 3 - cli/tests/testdata/066_prompt.ts.out | 10 -- .../testdata/090_run_permissions_request.ts | 9 ++ .../090_run_permissions_request.ts.out | 3 - cli/tests/testdata/issue9750.js | 6 + runtime/Cargo.toml | 2 +- runtime/permissions.rs | 23 ++++ test_util/src/lib.rs | 56 ++++++--- 10 files changed, 169 insertions(+), 55 deletions(-) delete mode 100644 cli/tests/testdata/061_permissions_request.ts.out delete mode 100644 cli/tests/testdata/062_permissions_request_global.ts.out delete mode 100644 cli/tests/testdata/066_prompt.ts.out delete mode 100644 cli/tests/testdata/090_run_permissions_request.ts.out create mode 100644 cli/tests/testdata/issue9750.js diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 4fd825507e..88aff09862 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -338,11 +338,17 @@ itest!(_089_run_allow_list { #[cfg(unix)] #[test] fn _090_run_permissions_request() { - let args = "run 090_run_permissions_request.ts"; - let output = "090_run_permissions_request.ts.out"; - let input = b"y\nn\n"; - - util::test_pty(args, output, input); + let args = "run --quiet 090_run_permissions_request.ts"; + use util::PtyData::*; + util::test_pty2(args, vec![ + Output("⚠️ ️Deno requests run access to \"ls\". Allow? [y/n (y = yes allow, n = no deny)] "), + Input("y\n"), + Output("⚠️ ️Deno requests run access to \"cat\". Allow? [y/n (y = yes allow, n = no deny)] "), + Input("n\n"), + Output("granted\r\n"), + Output("prompt\r\n"), + Output("denied\r\n"), + ]); } itest!(_091_use_define_for_class_fields { @@ -1717,21 +1723,31 @@ mod permissions { #[cfg(unix)] #[test] fn _061_permissions_request() { - let args = "run 061_permissions_request.ts"; - let output = "061_permissions_request.ts.out"; - let input = b"y\nn\n"; - - util::test_pty(args, output, input); + let args = "run --quiet 061_permissions_request.ts"; + use util::PtyData::*; + util::test_pty2(args, vec![ + Output("⚠️ ️Deno requests read access to \"foo\". Allow? [y/n (y = yes allow, n = no deny)] "), + Input("y\n"), + Output("⚠️ ️Deno requests read access to \"bar\". Allow? [y/n (y = yes allow, n = no deny)] "), + Input("n\n"), + Output("granted\r\n"), + Output("prompt\r\n"), + Output("denied\r\n"), + ]); } #[cfg(unix)] #[test] fn _062_permissions_request_global() { - let args = "run 062_permissions_request_global.ts"; - let output = "062_permissions_request_global.ts.out"; - let input = b"y\n"; - - util::test_pty(args, output, input); + let args = "run --quiet 062_permissions_request_global.ts"; + use util::PtyData::*; + util::test_pty2(args, vec![ + Output("⚠️ ️Deno requests read access. Allow? [y/n (y = yes allow, n = no deny)] "), + Input("y\n"), + Output("PermissionStatus { state: \"granted\", onchange: null }\r\n"), + Output("PermissionStatus { state: \"granted\", onchange: null }\r\n"), + Output("PermissionStatus { state: \"granted\", onchange: null }\r\n"), + ]); } itest!(_063_permissions_revoke { @@ -1747,12 +1763,45 @@ mod permissions { #[cfg(unix)] #[test] fn _066_prompt() { - let args = "run --unstable 066_prompt.ts"; - let output = "066_prompt.ts.out"; - // These are answers to prompt, confirm, and alert calls. - let input = b"John Doe\n\nfoo\nY\nN\nyes\n\nwindows\r\n\n\n"; - - util::test_pty(args, output, input); + let args = "run --quiet --unstable 066_prompt.ts"; + use util::PtyData::*; + util::test_pty2( + args, + vec![ + Output("What is your name? [Jane Doe] "), + Input("John Doe\n"), + Output("Your name is John Doe.\r\n"), + Output("What is your name? [Jane Doe] "), + Input("\n"), + Output("Your name is Jane Doe.\r\n"), + Output("Prompt "), + Input("foo\n"), + Output("Your input is foo.\r\n"), + Output("Question 0 [y/N] "), + Input("Y\n"), + Output("Your answer is true\r\n"), + Output("Question 1 [y/N] "), + Input("N\n"), + Output("Your answer is false\r\n"), + Output("Question 2 [y/N] "), + Input("yes\n"), + Output("Your answer is false\r\n"), + Output("Confirm [y/N] "), + Input("\n"), + Output("Your answer is false\r\n"), + Output("What is Windows EOL? "), + Input("windows\n"), + Output("Your answer is \"windows\"\r\n"), + Output("Hi [Enter] "), + Input("\n"), + Output("Alert [Enter] "), + Input("\n"), + Output("The end of test\r\n"), + Output("What is EOF? "), + Input("\n"), + Output("Your answer is null\r\n"), + ], + ); } itest!(dynamic_import_permissions_remote_remote { @@ -1806,6 +1855,24 @@ itest!(byte_order_mark { output: "byte_order_mark.out", }); +#[cfg(unix)] +#[test] +fn issue9750() { + use util::PtyData::*; + util::test_pty2( + "run --prompt issue9750.js", + vec![ + Output("Enter 'yy':\r\n"), + Input("yy\n"), + Output("⚠️ ️Deno requests env access. Allow? [y/n (y = yes allow, n = no deny)] "), + Input("n\n"), + Output("⚠️ ️Deno requests env access to \"SECRET\". Allow? [y/n (y = yes allow, n = no deny)] "), + Input("n\n"), + Output("error: Uncaught (in promise) PermissionDenied: Requires env access to \"SECRET\", run again with the --allow-env flag\r\n"), + ], + ); +} + // Regression test for https://github.com/denoland/deno/issues/11451. itest!(dom_exception_formatting { args: "run dom_exception_formatting.ts", diff --git a/cli/tests/testdata/061_permissions_request.ts.out b/cli/tests/testdata/061_permissions_request.ts.out deleted file mode 100644 index 362425876a..0000000000 --- a/cli/tests/testdata/061_permissions_request.ts.out +++ /dev/null @@ -1,3 +0,0 @@ -[WILDCARD]granted -prompt -denied diff --git a/cli/tests/testdata/062_permissions_request_global.ts.out b/cli/tests/testdata/062_permissions_request_global.ts.out deleted file mode 100644 index 57b5aa7d84..0000000000 --- a/cli/tests/testdata/062_permissions_request_global.ts.out +++ /dev/null @@ -1,3 +0,0 @@ -[WILDCARD]PermissionStatus { state: "granted", onchange: null } -PermissionStatus { state: "granted", onchange: null } -PermissionStatus { state: "granted", onchange: null } diff --git a/cli/tests/testdata/066_prompt.ts.out b/cli/tests/testdata/066_prompt.ts.out deleted file mode 100644 index 7defc51e55..0000000000 --- a/cli/tests/testdata/066_prompt.ts.out +++ /dev/null @@ -1,10 +0,0 @@ -[WILDCARD]What is your name? [Jane Doe] Your name is John Doe. -What is your name? [Jane Doe] Your name is Jane Doe. -Prompt Your input is foo. -Question 0 [y/N] Your answer is true -Question 1 [y/N] Your answer is false -Question 2 [y/N] Your answer is false -Confirm [y/N] Your answer is false -What is Windows EOL? Your answer is "windows" -Hi [Enter] Alert [Enter] The end of test -What is EOF? Your answer is null diff --git a/cli/tests/testdata/090_run_permissions_request.ts b/cli/tests/testdata/090_run_permissions_request.ts index 044bc6e8e2..8ecad2b3bf 100644 --- a/cli/tests/testdata/090_run_permissions_request.ts +++ b/cli/tests/testdata/090_run_permissions_request.ts @@ -1,9 +1,18 @@ const status1 = (await Deno.permissions.request({ name: "run", command: "ls" })).state; +if (status1 != "granted") { + throw Error(`unexpected status1 ${status1}`); +} const status2 = (await Deno.permissions.query({ name: "run", command: "cat" })).state; +if (status2 != "prompt") { + throw Error(`unexpected status2 ${status2}`); +} const status3 = (await Deno.permissions.request({ name: "run", command: "cat" })).state; +if (status3 != "denied") { + throw Error(`unexpected status3 ${status3}`); +} console.log(status1); console.log(status2); console.log(status3); diff --git a/cli/tests/testdata/090_run_permissions_request.ts.out b/cli/tests/testdata/090_run_permissions_request.ts.out deleted file mode 100644 index 362425876a..0000000000 --- a/cli/tests/testdata/090_run_permissions_request.ts.out +++ /dev/null @@ -1,3 +0,0 @@ -[WILDCARD]granted -prompt -denied diff --git a/cli/tests/testdata/issue9750.js b/cli/tests/testdata/issue9750.js new file mode 100644 index 0000000000..89fd61629d --- /dev/null +++ b/cli/tests/testdata/issue9750.js @@ -0,0 +1,6 @@ +// Run without permissions. +const buf = new Uint8Array(1); +console.log("Enter 'yy':"); +await Deno.stdin.read(buf); +await Deno.permissions.request({ "name": "env" }); +console.log("\n\nOwned", Deno.env.get("SECRET")); diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 98983b5ded..03a58fdd0c 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -85,7 +85,7 @@ uuid = { version = "0.8.2", features = ["v4"] } [target.'cfg(windows)'.dependencies] fwdansi = "1.1.0" -winapi = { version = "0.3.9", features = ["knownfolders", "mswsock", "objbase", "shlobj", "tlhelp32", "winbase", "winerror", "winsock2"] } +winapi = { version = "0.3.9", features = ["commapi", "knownfolders", "mswsock", "objbase", "shlobj", "tlhelp32", "winbase", "winerror", "winsock2"] } [target.'cfg(unix)'.dependencies] nix = "0.22.1" diff --git a/runtime/permissions.rs b/runtime/permissions.rs index 9e97ac234d..d1ee7f9993 100644 --- a/runtime/permissions.rs +++ b/runtime/permissions.rs @@ -1194,6 +1194,29 @@ fn permission_prompt(message: &str) -> bool { if !atty::is(atty::Stream::Stdin) || !atty::is(atty::Stream::Stderr) { return false; }; + + #[cfg(unix)] + fn clear_stdin() { + let r = unsafe { libc::tcflush(0, libc::TCIFLUSH) }; + assert_eq!(r, 0); + } + + #[cfg(not(unix))] + fn clear_stdin() { + unsafe { + let stdin = winapi::um::processenv::GetStdHandle( + winapi::um::winbase::STD_INPUT_HANDLE, + ); + let flags = + winapi::um::winbase::PURGE_TXCLEAR | winapi::um::winbase::PURGE_RXCLEAR; + winapi::um::commapi::PurgeComm(stdin, flags); + } + } + + // For security reasons we must consume everything in stdin so that previously + // buffered data cannot effect the prompt. + clear_stdin(); + let opts = "[y/n (y = yes allow, n = no deny)] "; let msg = format!( "{} ️Deno requests {}. Allow? {}", diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 3d9913576a..5eaedbaa01 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -1584,27 +1584,55 @@ pub fn pattern_match(pattern: &str, s: &str, wildcard: &str) -> bool { t.1.is_empty() } -/// Kind of reflects `itest!()`. Note that the pty's output (which also contains -/// stdin content) is compared against the content of the `output` path. +pub enum PtyData { + Input(&'static str), + Output(&'static str), +} + #[cfg(unix)] -pub fn test_pty(args: &str, output_path: &str, input: &[u8]) { +pub fn test_pty2(args: &str, data: Vec) { use pty::fork::Fork; + use std::io::BufRead; let tests_path = testdata_path(); let fork = Fork::from_ptmx().unwrap(); - if let Ok(mut master) = fork.is_parent() { - let mut output_actual = String::new(); - master.write_all(input).unwrap(); - master.read_to_string(&mut output_actual).unwrap(); - fork.wait().unwrap(); + if let Ok(master) = fork.is_parent() { + let mut buf_reader = std::io::BufReader::new(master); + for d in data { + match d { + PtyData::Input(s) => { + println!("INPUT {}", s.escape_debug()); + buf_reader.get_mut().write_all(s.as_bytes()).unwrap(); - let output_expected = - std::fs::read_to_string(tests_path.join(output_path)).unwrap(); - if !wildcard_match(&output_expected, &output_actual) { - println!("OUTPUT\n{}\nOUTPUT", output_actual); - println!("EXPECTED\n{}\nEXPECTED", output_expected); - panic!("pattern match failed"); + // Because of tty echo, we should be able to read the same string back. + assert!(s.ends_with('\n')); + let mut echo = String::new(); + buf_reader.read_line(&mut echo).unwrap(); + println!("ECHO: {}", echo.escape_debug()); + assert!(echo.starts_with(&s.trim())); + } + PtyData::Output(s) => { + let mut line = String::new(); + if s.ends_with('\n') { + buf_reader.read_line(&mut line).unwrap(); + } else { + while s != line { + let mut buf = [0; 64 * 1024]; + let _n = buf_reader.read(&mut buf).unwrap(); + let buf_str = std::str::from_utf8(&buf) + .unwrap() + .trim_end_matches(char::from(0)); + line += buf_str; + assert!(s.starts_with(&line)); + } + } + println!("OUTPUT {}", line.escape_debug()); + assert_eq!(line, s); + } + } } + + fork.wait().unwrap(); } else { deno_cmd() .current_dir(tests_path)