diff options
author | David Sherret <dsherret@users.noreply.github.com> | 2024-09-05 00:57:49 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-09-04 22:57:49 +0000 |
commit | dd208a6df02e99dbd7e1cb7b197fde8ccfeb0f88 (patch) | |
tree | 18844cfdf09df71d7b67ffc635ae55c1daf52dc4 | |
parent | 195b17ae1298f80209e3c2c5ef4d133e6975ff58 (diff) |
fix(regression): do not expose resolved path in Deno.Command permission denied error (#25434)
Regression from https://github.com/denoland/deno/pull/25370
-rw-r--r-- | runtime/ops/process.rs | 13 | ||||
-rw-r--r-- | runtime/permissions/lib.rs | 93 | ||||
-rw-r--r-- | tests/specs/compile/permissions_denied/main.out | 2 | ||||
-rw-r--r-- | tests/specs/permission/path_not_permitted/main.out | 4 | ||||
-rw-r--r-- | tests/specs/permission/path_not_permitted/main.ts | 2 | ||||
-rw-r--r-- | tests/specs/permission/path_not_permitted/sub.ts | 4 | ||||
-rw-r--r-- | tests/testdata/run/089_run_allow_list.ts.out | 2 |
7 files changed, 99 insertions, 21 deletions
diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs index eb53151ce..c2fa212d3 100644 --- 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 index 2eacd8bcc..da9e493d3 100644 --- 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 index 47a4707cc..e9ea45c81 100644 --- 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]
\ No newline at end of file diff --git a/tests/specs/permission/path_not_permitted/main.out b/tests/specs/permission/path_not_permitted/main.out index 3817c2ca5..77f800158 100644 --- 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 index 9e8d627f2..0cc141e7a 100644 --- 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 index f2b6d6b37..ea527a938 100644 --- 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 index 0fc1c80c2..68a4a2ac5 100644 --- 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 |