From fe7e3a12ca02792215f7598302c42113bcdc4458 Mon Sep 17 00:00:00 2001
From: Asher Gomez <ashersaupingomez@gmail.com>
Date: Thu, 24 Nov 2022 14:00:31 +1100
Subject: [PATCH] feat(cli): add warning for incorrectly ordered flags (#16734)

This code checks if permission flags are incorrectly defined after the
module name (e.g. `deno run mod.ts --allow-read` instead of the correct
`deno run --allow-read mod.ts`). If so, a simple warning is displayed.
---
 cli/args/flags.rs                          | 44 ++++++++++++++++++++++
 cli/main.rs                                | 11 ++++++
 cli/tests/integration/run_tests.rs         | 11 ++++++
 cli/tests/testdata/run/permission_args.out |  4 ++
 4 files changed, 70 insertions(+)
 create mode 100644 cli/tests/testdata/run/permission_args.out

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index e322aa1e7b..829051bcee 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -493,6 +493,32 @@ impl Flags {
       prompt: !self.no_prompt,
     }
   }
+
+  pub fn has_permission(&self) -> bool {
+    self.allow_all
+      || self.allow_hrtime
+      || self.allow_env.is_some()
+      || self.allow_ffi.is_some()
+      || self.allow_net.is_some()
+      || self.allow_read.is_some()
+      || self.allow_run.is_some()
+      || self.allow_sys.is_some()
+      || self.allow_write.is_some()
+  }
+
+  pub fn has_permission_in_argv(&self) -> bool {
+    self.argv.iter().any(|arg| {
+      arg == "--allow-all"
+        || arg == "--allow-hrtime"
+        || arg.starts_with("--allow-env")
+        || arg.starts_with("--allow-ffi")
+        || arg.starts_with("--allow-net")
+        || arg.starts_with("--allow-read")
+        || arg.starts_with("--allow-run")
+        || arg.starts_with("--allow-sys")
+        || arg.starts_with("--allow-write")
+    })
+  }
 }
 
 static ENV_VARIABLES_HELP: &str = r#"ENVIRONMENT VARIABLES:
@@ -3388,6 +3414,24 @@ mod tests {
     );
   }
 
+  #[test]
+  fn has_permission() {
+    let r = flags_from_vec(svec!["deno", "run", "--allow-read", "x.ts"]);
+    assert_eq!(r.unwrap().has_permission(), true);
+
+    let r = flags_from_vec(svec!["deno", "run", "x.ts"]);
+    assert_eq!(r.unwrap().has_permission(), false);
+  }
+
+  #[test]
+  fn has_permission_in_argv() {
+    let r = flags_from_vec(svec!["deno", "run", "x.ts", "--allow-read"]);
+    assert_eq!(r.unwrap().has_permission_in_argv(), true);
+
+    let r = flags_from_vec(svec!["deno", "run", "x.ts"]);
+    assert_eq!(r.unwrap().has_permission_in_argv(), false);
+  }
+
   #[test]
   fn script_args() {
     let r = flags_from_vec(svec![
diff --git a/cli/main.rs b/cli/main.rs
index b91540c379..f18c3c9769 100644
--- a/cli/main.rs
+++ b/cli/main.rs
@@ -686,6 +686,17 @@ async fn run_command(
     return run_from_stdin(flags).await;
   }
 
+  if !flags.has_permission() && flags.has_permission_in_argv() {
+    log::warn!(
+      "{}",
+      crate::colors::yellow(
+        r#"Permission flags have likely been incorrectly set after the script argument.
+To grant permissions, set them before the script argument. For example:
+    deno run --allow-read=. main.js"#
+      )
+    );
+  }
+
   if flags.watch.is_some() {
     return run_with_watch(flags, run_flags.script).await;
   }
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 192fff620d..866fd2793c 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -3647,3 +3647,14 @@ itest!(flash_shutdown {
   args: "run --unstable --allow-net run/flash_shutdown/main.ts",
   exit_code: 0,
 });
+
+itest!(permission_args {
+  args: "run run/001_hello.js --allow-net",
+  output: "run/permission_args.out",
+  envs: vec![("NO_COLOR".to_string(), "1".to_string())],
+});
+
+itest!(permission_args_quiet {
+  args: "run --quiet run/001_hello.js --allow-net",
+  output: "run/001_hello.js.out",
+});
diff --git a/cli/tests/testdata/run/permission_args.out b/cli/tests/testdata/run/permission_args.out
new file mode 100644
index 0000000000..6a1e1787ca
--- /dev/null
+++ b/cli/tests/testdata/run/permission_args.out
@@ -0,0 +1,4 @@
+Permission flags have likely been incorrectly set after the script argument.
+To grant permissions, set them before the script argument. For example:
+    deno run --allow-read=. main.js
+Hello World