fix(regression): do not expose resolved path in Deno.Command permissi…

…on denied error (#25434) Regression from #25370
denoland · Sep 4, 2024 · dd208a6 · dd208a6
1 parent 195b17a
commit dd208a6
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 21 deletions.
diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs
@@ -17,6 +17,7 @@ use deno_io::ChildStderrResource;
 use deno_io::ChildStdinResource;
 use deno_io::ChildStdoutResource;
 use deno_permissions::PermissionsContainer;
+use deno_permissions::RunPathQuery;
 use serde::Deserialize;
 use serde::Serialize;
 use std::borrow::Cow;
@@ -516,7 +517,15 @@ fn compute_run_cmd_and_check_permissions(
     .with_context(|| format!("Failed to spawn '{}'", arg_cmd))?;
   let cmd = resolve_cmd(arg_cmd, &run_env)
     .with_context(|| format!("Failed to spawn '{}'", arg_cmd))?;
-  check_run_permission(state, &cmd, &run_env, api_name)?;
+  check_run_permission(
+    state,
+    RunPathQuery {
+      requested: arg_cmd,
+      resolved: &cmd,
+    },
+    &run_env,
+    api_name,
+  )?;
   Ok((cmd, run_env))
 }
 
@@ -588,7 +597,7 @@ fn resolve_path(path: &str, cwd: &Path) -> PathBuf {
 
 fn check_run_permission(
   state: &mut OpState,
-  cmd: &Path,
+  cmd: RunPathQuery,
   run_env: &RunEnv,
   api_name: &str,
 ) -> Result<(), AnyError> {

diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs
@@ -862,6 +862,12 @@ impl AsRef<str> for EnvDescriptor {
   }
 }
 
+#[derive(Clone, Eq, PartialEq, Hash, Debug, Serialize, Deserialize)]
+pub struct RunPathQuery<'a> {
+  pub requested: &'a str,
+  pub resolved: &'a Path,
+}
+
 pub enum RunDescriptorArg {
   Name(String),
   Path(PathBuf),
@@ -1321,16 +1327,16 @@ impl UnaryPermission<RunDescriptor> {
 
   pub fn check(
     &mut self,
-    cmd: &Path,
+    cmd: RunPathQuery,
     api_name: Option<&str>,
   ) -> Result<(), AnyError> {
-    debug_assert!(cmd.is_absolute());
+    debug_assert!(cmd.resolved.is_absolute());
     skip_check_if_is_permission_fully_granted!(self);
     self.check_desc(
-      Some(&RunDescriptor::Path(cmd.to_path_buf())),
+      Some(&RunDescriptor::Path(cmd.resolved.to_path_buf())),
       false,
       api_name,
-      || Some(format!("\"{}\"", cmd.display())),
+      || Some(format!("\"{}\"", cmd.requested)),
     )
   }
 
@@ -1692,7 +1698,7 @@ impl PermissionsContainer {
   #[inline(always)]
   pub fn check_run(
     &mut self,
-    cmd: &Path,
+    cmd: RunPathQuery,
     api_name: &str,
   ) -> Result<(), AnyError> {
     self.0.lock().run.check(cmd, Some(api_name))
@@ -3036,10 +3042,37 @@ mod tests {
     #[allow(clippy::disallowed_methods)]
     let cwd = std::env::current_dir().unwrap();
     prompt_value.set(true);
-    assert!(perms.run.check(&cwd.join("cat"), None).is_ok());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "cat",
+          resolved: &cwd.join("cat")
+        },
+        None
+      )
+      .is_ok());
     prompt_value.set(false);
-    assert!(perms.run.check(&cwd.join("cat"), None).is_ok());
-    assert!(perms.run.check(&cwd.join("ls"), None).is_err());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "cat",
+          resolved: &cwd.join("cat")
+        },
+        None
+      )
+      .is_ok());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "ls",
+          resolved: &cwd.join("ls")
+        },
+        None
+      )
+      .is_err());
 
     prompt_value.set(true);
     assert!(perms.env.check("HOME", None).is_ok());
@@ -3133,12 +3166,48 @@ mod tests {
     prompt_value.set(false);
     #[allow(clippy::disallowed_methods)]
     let cwd = std::env::current_dir().unwrap();
-    assert!(perms.run.check(&cwd.join("cat"), None).is_err());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "cat",
+          resolved: &cwd.join("cat")
+        },
+        None
+      )
+      .is_err());
     prompt_value.set(true);
-    assert!(perms.run.check(&cwd.join("cat"), None).is_err());
-    assert!(perms.run.check(&cwd.join("ls"), None).is_ok());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "cat",
+          resolved: &cwd.join("cat")
+        },
+        None
+      )
+      .is_err());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "ls",
+          resolved: &cwd.join("ls")
+        },
+        None
+      )
+      .is_ok());
     prompt_value.set(false);
-    assert!(perms.run.check(&cwd.join("ls"), None).is_ok());
+    assert!(perms
+      .run
+      .check(
+        RunPathQuery {
+          requested: "ls",
+          resolved: &cwd.join("ls")
+        },
+        None
+      )
+      .is_ok());
 
     prompt_value.set(false);
     assert!(perms.env.check("HOME", None).is_err());

diff --git a/tests/specs/compile/permissions_denied/main.out b/tests/specs/compile/permissions_denied/main.out
@@ -1,2 +1,2 @@
-error: Uncaught (in promise) PermissionDenied: Requires run access to "[WILDLINE]deno[WILDLINE]", specify the required permissions during compilation using `deno compile --allow-run`
+error: Uncaught (in promise) PermissionDenied: Requires run access to "deno", specify the required permissions during compilation using `deno compile --allow-run`
 [WILDCARD]
diff --git a/tests/specs/permission/path_not_permitted/main.out b/tests/specs/permission/path_not_permitted/main.out
@@ -1,10 +1,10 @@
 Running...
-PermissionDenied: Requires run access to "[WILDLINE]deno[WILDLINE]", run again with the --allow-run flag
+PermissionDenied: Requires run access to "deno", run again with the --allow-run flag
     [WILDCARD]
     at file:///[WILDLINE]/sub.ts:15:5 {
   name: "PermissionDenied"
 }
-PermissionDenied: Requires run access to "[WILDLINE]deno[WILDLINE]", run again with the --allow-run flag
+PermissionDenied: Requires run access to "deno", run again with the --allow-run flag
     [WILDCARD]
     at file:///[WILDLINE]/sub.ts:23:22 {
   name: "PermissionDenied"

diff --git a/tests/specs/permission/path_not_permitted/main.ts b/tests/specs/permission/path_not_permitted/main.ts
@@ -9,7 +9,7 @@ new Deno.Command(
       "run",
       "--allow-write",
       "--allow-read",
-      `--allow-run=${binaryName}`,
+      `--allow-run=deno`,
       "sub.ts",
     ],
     stderr: "inherit",

diff --git a/tests/specs/permission/path_not_permitted/sub.ts b/tests/specs/permission/path_not_permitted/sub.ts
@@ -6,7 +6,7 @@ Deno.copyFileSync(binaryName, "subdir/" + binaryName);
 
 try {
   const commandResult = new Deno.Command(
-    binaryName,
+    "deno",
     {
       env: { "PATH": Deno.cwd() + pathSep + "subdir" },
       stdout: "inherit",
@@ -22,7 +22,7 @@ try {
 try {
   const child = Deno.run(
     {
-      cmd: [binaryName],
+      cmd: ["deno"],
       env: { "PATH": Deno.cwd() + pathSep + "subdir" },
       stdout: "inherit",
       stderr: "inherit",

diff --git a/tests/testdata/run/089_run_allow_list.ts.out b/tests/testdata/run/089_run_allow_list.ts.out
@@ -1,3 +1,3 @@
-[WILDCARD]PermissionDenied: Requires run access to "[WILDLINE]ls[WILDLINE]", run again with the --allow-run flag
+[WILDCARD]PermissionDenied: Requires run access to "ls", run again with the --allow-run flag
 [WILDCARD]
 true