feat: warn when using --allow-run with no allow list (#25215)

cli/args/mod.rs

@@ -809,6 +809,8 @@ impl CliOptions {
       }
     }
 
+    warn_insecure_allow_run_flags(&flags);
+
     let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);
     let deno_dir_provider =
       Arc::new(DenoDirProvider::new(flags.cache_path.clone()));
@@ -1688,6 +1690,27 @@ impl CliOptions {
   }
 }
 
+/// Warns for specific uses of `--allow-run`. This function is not
+/// intended to catch every single possible insecure use of `--allow-run`,
+/// but is just an attempt to discourage some common pitfalls.
+fn warn_insecure_allow_run_flags(flags: &Flags) {
+  let permissions = &flags.permissions;
+  if permissions.allow_all {
+    return;
+  }
+  let Some(allow_run_list) = permissions.allow_run.as_ref() else {
+    return;
+  };
+
+  // discourage using --allow-run without an allow list
+  if allow_run_list.is_empty() {
+    log::warn!(
+      "{} --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)",
+      colors::yellow("Warning")
+    );
+  }
+}
+
 /// Resolves the path to use for a local node_modules folder.
 fn resolve_node_modules_folder(
   cwd: &Path,

tests/napi/cleanup_hook_test.js

@@ -17,9 +17,7 @@ if (import.meta.main) {
         "--config",
         Deno.realPathSync("../config/deno.json"),
         "--no-lock",
-        "--allow-read",
+        "-A",
-        "--allow-run",
-        "--allow-ffi",
         "--unstable-ffi",
         import.meta.url,
       ],

tests/specs/permission/deny_run_binary_absolute_path/main.out

@@ -1,3 +1,4 @@
+Warning --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)
 NotCapable: Requires run access to "deno", run again with the --allow-run flag
     at [WILDCARD] {
   name: "NotCapable"

tests/specs/permission/path_not_permitted/main.out

@@ -1,10 +1,10 @@
 Running...
-NotCapable: Requires run access to "deno", run again with the --allow-run flag
+NotCapable: Requires run access to "binary", run again with the --allow-run flag
     [WILDCARD]
     at file:///[WILDLINE]/sub.ts:15:5 {
   name: "NotCapable"
 }
-NotCapable: Requires run access to "deno", run again with the --allow-run flag
+NotCapable: Requires run access to "binary", run again with the --allow-run flag
     [WILDCARD]
     at file:///[WILDLINE]/sub.ts:23:22 {
   name: "NotCapable"

tests/specs/permission/path_not_permitted/main.ts

@@ -1,4 +1,4 @@
-const binaryName = Deno.build.os === "windows" ? "deno.exe" : "deno";
+const binaryName = Deno.build.os === "windows" ? "binary.exe" : "binary";
 Deno.copyFileSync(Deno.execPath(), binaryName);
 
 console.log("Running...");
@@ -9,9 +9,12 @@ new Deno.Command(
       "run",
       "--allow-write",
       "--allow-read",
-      `--allow-run=deno`,
+      `--allow-run=binary`,
       "sub.ts",
     ],
+    env: {
+      PATH: Deno.cwd(),
+    },
     stderr: "inherit",
     stdout: "inherit",
   },

tests/specs/permission/path_not_permitted/sub.ts

@@ -1,4 +1,4 @@
-const binaryName = Deno.build.os === "windows" ? "deno.exe" : "deno";
+const binaryName = Deno.build.os === "windows" ? "binary.exe" : "binary";
 const pathSep = Deno.build.os === "windows" ? "\\" : "/";
 
 Deno.mkdirSync("subdir");
@@ -6,7 +6,7 @@ Deno.copyFileSync(binaryName, "subdir/" + binaryName);
 
 try {
   const commandResult = new Deno.Command(
-    "deno",
+    "binary",
     {
       env: { "PATH": Deno.cwd() + pathSep + "subdir" },
       stdout: "inherit",
@@ -22,7 +22,7 @@ try {
 try {
   const child = Deno.run(
     {
-      cmd: ["deno"],
+      cmd: ["binary"],
       env: { "PATH": Deno.cwd() + pathSep + "subdir" },
       stdout: "inherit",
       stderr: "inherit",

tests/specs/run/allow_run_insecure_warnings/__test__.jsonc

@@ -0,0 +1,8 @@
+{
+  "tests": {
+    "no_allow_list": {
+      "args": "run --allow-run main.ts",
+      "output": "no_allow_list.out"
+    }
+  }
+}

tests/specs/run/allow_run_insecure_warnings/no_allow_list.out

@@ -0,0 +1 @@
+Warning --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)

tests/specs/test/captured_output/__test__.jsonc

@@ -1,5 +1,5 @@
 {
-  "args": "test --allow-run --allow-read captured_output.ts",
+  "args": "test -A captured_output.ts",
   "output": "main.out",
   "envs": { "NO_COLOR": "1" },
   "exitCode": 0

tests/testdata/run/deny_some_permission_args.out

@@ -1,3 +1,4 @@
+Warning --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)
 PermissionStatus { state: "granted", onchange: null, partial: true }
 PermissionStatus { state: "denied", onchange: null }
 PermissionStatus { state: "granted", onchange: null }