From 2092f0c69748f589ce1deee285a0f92cf485ad10 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 2 Jan 2025 16:55:03 -0500 Subject: [PATCH] fix(permissions): implicit `--allow-import` when using `--cached-only` (#27530) `--cached-only` cannot communicate with a remote server Closes https://github.com/denoland/deno/issues/27498 --- cli/args/flags.rs | 120 -------------- cli/args/mod.rs | 150 ++++++++++++++++-- cli/standalone/binary.rs | 5 +- cli/standalone/mod.rs | 3 +- tests/specs/info/import_map/__test__.jsonc | 10 +- .../allow_import_cached_only/__test__.jsonc | 22 +++ .../allow_import_cached_only/deno.jsonc | 3 + .../allow_import_cached_only/fail.out | 2 + .../allow_import_cached_only/main.ts | 1 + .../allow_import_cached_only/success.out | 1 + .../allow_import_info/__test__.jsonc | 22 +++ .../allow_import_info/import_allowed.out | 8 + .../allow_import_info/import_not_allowed.out | 7 + .../permission/allow_import_info/main.ts | 1 + .../permission/allow_import_info/success.out | 7 + 15 files changed, 214 insertions(+), 148 deletions(-) create mode 100644 tests/specs/permission/allow_import_cached_only/__test__.jsonc create mode 100644 tests/specs/permission/allow_import_cached_only/deno.jsonc create mode 100644 tests/specs/permission/allow_import_cached_only/fail.out create mode 100644 tests/specs/permission/allow_import_cached_only/main.ts create mode 100644 tests/specs/permission/allow_import_cached_only/success.out create mode 100644 tests/specs/permission/allow_import_info/__test__.jsonc create mode 100644 tests/specs/permission/allow_import_info/import_allowed.out create mode 100644 tests/specs/permission/allow_import_info/import_not_allowed.out create mode 100644 tests/specs/permission/allow_import_info/main.ts create mode 100644 tests/specs/permission/allow_import_info/success.out diff --git a/cli/args/flags.rs b/cli/args/flags.rs index c57b81ae6d..fb64b4eeaa 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1,6 +1,5 @@ // Copyright 2018-2025 the Deno authors. MIT license. -use std::borrow::Cow; use std::collections::HashSet; use std::env; use std::ffi::OsString; @@ -34,7 +33,6 @@ use deno_core::url::Url; use deno_graph::GraphKind; use deno_path_util::normalize_path; use deno_path_util::url_to_file_path; -use deno_runtime::deno_permissions::PermissionsOptions; use deno_runtime::deno_permissions::SysDescriptor; use deno_telemetry::OtelConfig; use deno_telemetry::OtelConsoleConfig; @@ -44,8 +42,6 @@ use serde::Deserialize; use serde::Serialize; use super::flags_net; -use super::jsr_url; -use crate::args::resolve_no_prompt; use crate::util::fs::canonicalize_path; #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -692,97 +688,6 @@ impl PermissionFlags { || self.deny_write.is_some() || self.allow_import.is_some() } - - pub fn to_options(&self, cli_arg_urls: &[Cow]) -> PermissionsOptions { - fn handle_allow( - allow_all: bool, - value: Option, - ) -> Option { - if allow_all { - assert!(value.is_none()); - Some(T::default()) - } else { - value - } - } - - fn handle_imports( - cli_arg_urls: &[Cow], - imports: Option>, - ) -> Option> { - if imports.is_some() { - return imports; - } - - let builtin_allowed_import_hosts = [ - "jsr.io:443", - "deno.land:443", - "esm.sh:443", - "cdn.jsdelivr.net:443", - "raw.githubusercontent.com:443", - "gist.githubusercontent.com:443", - ]; - - let mut imports = - Vec::with_capacity(builtin_allowed_import_hosts.len() + 1); - imports - .extend(builtin_allowed_import_hosts.iter().map(|s| s.to_string())); - - // also add the JSR_URL env var - if let Some(jsr_host) = allow_import_host_from_url(jsr_url()) { - imports.push(jsr_host); - } - // include the cli arg urls - for url in cli_arg_urls { - if let Some(host) = allow_import_host_from_url(url) { - imports.push(host); - } - } - - Some(imports) - } - - PermissionsOptions { - allow_all: self.allow_all, - allow_env: handle_allow(self.allow_all, self.allow_env.clone()), - deny_env: self.deny_env.clone(), - allow_net: handle_allow(self.allow_all, self.allow_net.clone()), - deny_net: self.deny_net.clone(), - allow_ffi: handle_allow(self.allow_all, self.allow_ffi.clone()), - deny_ffi: self.deny_ffi.clone(), - allow_read: handle_allow(self.allow_all, self.allow_read.clone()), - deny_read: self.deny_read.clone(), - allow_run: handle_allow(self.allow_all, self.allow_run.clone()), - deny_run: self.deny_run.clone(), - allow_sys: handle_allow(self.allow_all, self.allow_sys.clone()), - deny_sys: self.deny_sys.clone(), - allow_write: handle_allow(self.allow_all, self.allow_write.clone()), - deny_write: self.deny_write.clone(), - allow_import: handle_imports( - cli_arg_urls, - handle_allow(self.allow_all, self.allow_import.clone()), - ), - prompt: !resolve_no_prompt(self), - } - } -} - -/// Gets the --allow-import host from the provided url -fn allow_import_host_from_url(url: &Url) -> Option { - let host = url.host()?; - if let Some(port) = url.port() { - Some(format!("{}:{}", host, port)) - } else { - use deno_core::url::Host::*; - match host { - Domain(domain) if domain == "jsr.io" && url.scheme() == "https" => None, - _ => match url.scheme() { - "https" => Some(format!("{}:443", host)), - "http" => Some(format!("{}:80", host)), - _ => None, - }, - } - } } fn join_paths(allowlist: &[String], d: &str) -> String { @@ -11549,8 +11454,6 @@ mod tests { ..Default::default() } ); - // just make sure this doesn't panic - let _ = flags.permissions.to_options(&[]); } #[test] @@ -11626,29 +11529,6 @@ Usage: deno repl [OPTIONS] [-- [ARGS]...]\n" ) } - #[test] - fn test_allow_import_host_from_url() { - fn parse(text: &str) -> Option { - allow_import_host_from_url(&Url::parse(text).unwrap()) - } - - assert_eq!(parse("https://jsr.io"), None); - assert_eq!( - parse("http://127.0.0.1:4250"), - Some("127.0.0.1:4250".to_string()) - ); - assert_eq!(parse("http://jsr.io"), Some("jsr.io:80".to_string())); - assert_eq!( - parse("https://example.com"), - Some("example.com:443".to_string()) - ); - assert_eq!( - parse("http://example.com"), - Some("example.com:80".to_string()) - ); - assert_eq!(parse("file:///example.com"), None); - } - #[test] fn allow_all_conflicts_allow_perms() { let flags = [ diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 1ad61a0a78..35f79a9c3e 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1526,20 +1526,100 @@ impl CliOptions { self.flags.no_npm } - pub fn permission_flags(&self) -> &PermissionFlags { - &self.flags.permissions - } - pub fn permissions_options(&self) -> PermissionsOptions { - fn files_to_urls(files: &[String]) -> Vec> { - files - .iter() - .filter_map(|f| Url::parse(f).ok().map(Cow::Owned)) - .collect() + // bury this in here to ensure people use cli_options.permissions_options() + fn flags_to_options(flags: &PermissionFlags) -> PermissionsOptions { + fn handle_allow( + allow_all: bool, + value: Option, + ) -> Option { + if allow_all { + assert!(value.is_none()); + Some(T::default()) + } else { + value + } + } + + PermissionsOptions { + allow_all: flags.allow_all, + allow_env: handle_allow(flags.allow_all, flags.allow_env.clone()), + deny_env: flags.deny_env.clone(), + allow_net: handle_allow(flags.allow_all, flags.allow_net.clone()), + deny_net: flags.deny_net.clone(), + allow_ffi: handle_allow(flags.allow_all, flags.allow_ffi.clone()), + deny_ffi: flags.deny_ffi.clone(), + allow_read: handle_allow(flags.allow_all, flags.allow_read.clone()), + deny_read: flags.deny_read.clone(), + allow_run: handle_allow(flags.allow_all, flags.allow_run.clone()), + deny_run: flags.deny_run.clone(), + allow_sys: handle_allow(flags.allow_all, flags.allow_sys.clone()), + deny_sys: flags.deny_sys.clone(), + allow_write: handle_allow(flags.allow_all, flags.allow_write.clone()), + deny_write: flags.deny_write.clone(), + allow_import: handle_allow(flags.allow_all, flags.allow_import.clone()), + prompt: !resolve_no_prompt(flags), + } } - // get a list of urls to imply for --allow-import - let cli_arg_urls = self + let mut permissions_options = flags_to_options(&self.flags.permissions); + self.augment_import_permissions(&mut permissions_options); + permissions_options + } + + fn augment_import_permissions(&self, options: &mut PermissionsOptions) { + // do not add if the user specified --allow-all or --allow-import + if !options.allow_all && options.allow_import.is_none() { + options.allow_import = Some(self.implicit_allow_import()); + } + } + + fn implicit_allow_import(&self) -> Vec { + // allow importing from anywhere when using cached only + if self.cache_setting() == CacheSetting::Only { + vec![] // allow all imports + } else { + // implicitly allow some trusted hosts and the CLI arg urls + let cli_arg_urls = self.get_cli_arg_urls(); + let builtin_allowed_import_hosts = [ + "jsr.io:443", + "deno.land:443", + "esm.sh:443", + "cdn.jsdelivr.net:443", + "raw.githubusercontent.com:443", + "gist.githubusercontent.com:443", + ]; + let mut imports = Vec::with_capacity( + builtin_allowed_import_hosts.len() + cli_arg_urls.len() + 1, + ); + imports + .extend(builtin_allowed_import_hosts.iter().map(|s| s.to_string())); + // also add the JSR_URL env var + if let Some(jsr_host) = allow_import_host_from_url(jsr_url()) { + if jsr_host != "jsr.io:443" { + imports.push(jsr_host); + } + } + // include the cli arg urls + for url in cli_arg_urls { + if let Some(host) = allow_import_host_from_url(&url) { + imports.push(host); + } + } + imports + } + } + + fn get_cli_arg_urls(&self) -> Vec> { + fn files_to_urls(files: &[String]) -> Vec> { + files.iter().filter_map(|f| file_to_url(f)).collect() + } + + fn file_to_url(file: &str) -> Option> { + Url::parse(file).ok().map(Cow::Owned) + } + + self .resolve_main_module() .ok() .map(|url| vec![Cow::Borrowed(url)]) @@ -1551,18 +1631,18 @@ impl CliOptions { Some(files_to_urls(&check_flags.files)) } DenoSubcommand::Install(InstallFlags::Global(flags)) => { - Url::parse(&flags.module_url) - .ok() - .map(|url| vec![Cow::Owned(url)]) + file_to_url(&flags.module_url).map(|url| vec![url]) } DenoSubcommand::Doc(DocFlags { source_files: DocSourceFileFlag::Paths(paths), .. }) => Some(files_to_urls(paths)), + DenoSubcommand::Info(InfoFlags { + file: Some(file), .. + }) => file_to_url(file).map(|url| vec![url]), _ => None, }) - .unwrap_or_default(); - self.flags.permissions.to_options(&cli_arg_urls) + .unwrap_or_default() } pub fn reload_flag(&self) -> bool { @@ -1998,6 +2078,20 @@ fn load_env_variables_from_env_file(filename: Option<&Vec>) { } } +/// Gets the --allow-import host from the provided url +fn allow_import_host_from_url(url: &Url) -> Option { + let host = url.host()?; + if let Some(port) = url.port() { + Some(format!("{}:{}", host, port)) + } else { + match url.scheme() { + "https" => Some(format!("{}:443", host)), + "http" => Some(format!("{}:80", host)), + _ => None, + } + } +} + #[derive(Debug, Clone, Copy)] pub enum NpmCachingStrategy { Eager, @@ -2005,7 +2099,7 @@ pub enum NpmCachingStrategy { Manual, } -pub(crate) fn otel_runtime_config() -> OtelRuntimeConfig { +pub fn otel_runtime_config() -> OtelRuntimeConfig { OtelRuntimeConfig { runtime_name: Cow::Borrowed("deno"), runtime_version: Cow::Borrowed(crate::version::DENO_VERSION_INFO.deno), @@ -2102,4 +2196,26 @@ mod test { let reg_api_url = jsr_api_url(); assert!(reg_api_url.as_str().ends_with('/')); } + + #[test] + fn test_allow_import_host_from_url() { + fn parse(text: &str) -> Option { + allow_import_host_from_url(&Url::parse(text).unwrap()) + } + + assert_eq!( + parse("http://127.0.0.1:4250"), + Some("127.0.0.1:4250".to_string()) + ); + assert_eq!(parse("http://jsr.io"), Some("jsr.io:80".to_string())); + assert_eq!( + parse("https://example.com"), + Some("example.com:443".to_string()) + ); + assert_eq!( + parse("http://example.com"), + Some("example.com:80".to_string()) + ); + assert_eq!(parse("file:///example.com"), None); + } } diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 7ca25fca51..f07c645d89 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -51,6 +51,7 @@ use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_fs::RealFs; use deno_runtime::deno_io::fs::FsError; use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_permissions::PermissionsOptions; use deno_semver::npm::NpmVersionReqParseError; use deno_semver::package::PackageReq; use deno_semver::Version; @@ -188,7 +189,7 @@ pub struct Metadata { pub argv: Vec, pub seed: Option, pub code_cache_key: Option, - pub permissions: PermissionFlags, + pub permissions: PermissionsOptions, pub location: Option, pub v8_flags: Vec, pub log_level: Option, @@ -793,7 +794,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { seed: self.cli_options.seed(), code_cache_key, location: self.cli_options.location_flag().clone(), - permissions: self.cli_options.permission_flags().clone(), + permissions: self.cli_options.permissions_options(), v8_flags: self.cli_options.v8_flags().clone(), unsafely_ignore_certificate_errors: self .cli_options diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 6ed192071f..1e21e69eb9 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -920,8 +920,7 @@ pub async fn run( }; let permissions = { - let mut permissions = - metadata.permissions.to_options(/* cli_arg_urls */ &[]); + let mut permissions = metadata.permissions; // grant read access to the vfs match &mut permissions.allow_read { Some(vec) if vec.is_empty() => { diff --git a/tests/specs/info/import_map/__test__.jsonc b/tests/specs/info/import_map/__test__.jsonc index 7aba603e0b..725276925e 100644 --- a/tests/specs/info/import_map/__test__.jsonc +++ b/tests/specs/info/import_map/__test__.jsonc @@ -1,9 +1,5 @@ { - "steps": [ - { - "args": "info preact/debug", - "output": "with_import_map.out", - "exitCode": 0 - } - ] + "args": "info preact/debug", + "output": "with_import_map.out", + "exitCode": 0 } diff --git a/tests/specs/permission/allow_import_cached_only/__test__.jsonc b/tests/specs/permission/allow_import_cached_only/__test__.jsonc new file mode 100644 index 0000000000..a86a8796c2 --- /dev/null +++ b/tests/specs/permission/allow_import_cached_only/__test__.jsonc @@ -0,0 +1,22 @@ +{ + "tempDir": true, + "tests": { + "no_flag": { + // ensure what we're testing will fail without the flags + "args": "run main.ts", + "output": "fail.out", + "exitCode": 1 + }, + "with_flags": { + "steps": [{ + "args": "cache --allow-import main.ts", + "output": "[WILDLINE]", + "exitCode": 0 + }, { + "args": "run --cached-only main.ts", + "output": "success.out", + "exitCode": 0 + }] + } + } +} diff --git a/tests/specs/permission/allow_import_cached_only/deno.jsonc b/tests/specs/permission/allow_import_cached_only/deno.jsonc new file mode 100644 index 0000000000..090481af96 --- /dev/null +++ b/tests/specs/permission/allow_import_cached_only/deno.jsonc @@ -0,0 +1,3 @@ +{ + "lock": true +} diff --git a/tests/specs/permission/allow_import_cached_only/fail.out b/tests/specs/permission/allow_import_cached_only/fail.out new file mode 100644 index 0000000000..517f53b9f4 --- /dev/null +++ b/tests/specs/permission/allow_import_cached_only/fail.out @@ -0,0 +1,2 @@ +error: Requires import access to "localhost:4545", run again with the --allow-import flag + at file:///[WILDLINE]/main.ts:1:8 diff --git a/tests/specs/permission/allow_import_cached_only/main.ts b/tests/specs/permission/allow_import_cached_only/main.ts new file mode 100644 index 0000000000..7556f22667 --- /dev/null +++ b/tests/specs/permission/allow_import_cached_only/main.ts @@ -0,0 +1 @@ +import "http://localhost:4545/welcome.ts"; diff --git a/tests/specs/permission/allow_import_cached_only/success.out b/tests/specs/permission/allow_import_cached_only/success.out new file mode 100644 index 0000000000..8432170eee --- /dev/null +++ b/tests/specs/permission/allow_import_cached_only/success.out @@ -0,0 +1 @@ +Welcome to Deno! diff --git a/tests/specs/permission/allow_import_info/__test__.jsonc b/tests/specs/permission/allow_import_info/__test__.jsonc new file mode 100644 index 0000000000..adcd0f2190 --- /dev/null +++ b/tests/specs/permission/allow_import_info/__test__.jsonc @@ -0,0 +1,22 @@ +{ + "envs": { + "JSR_URL": "" + }, + "tests": { + "implicit": { + "args": "info http://localhost:4545/welcome.ts", + "output": "success.out", + "exitCode": 0 + }, + "via_import_not_allowed": { + "args": "info main.ts", + "output": "import_not_allowed.out", + "exitCode": 0 + }, + "via_import_allowed": { + "args": "info --allow-import main.ts", + "output": "import_allowed.out", + "exitCode": 0 + } + } +} diff --git a/tests/specs/permission/allow_import_info/import_allowed.out b/tests/specs/permission/allow_import_info/import_allowed.out new file mode 100644 index 0000000000..95b61b27ef --- /dev/null +++ b/tests/specs/permission/allow_import_info/import_allowed.out @@ -0,0 +1,8 @@ +Download http://localhost:4545/welcome.ts +local: [WILDLINE] +type: TypeScript +dependencies: [WILDLINE] +size: [WILDLINE] + +file:///[WILDLINE]/main.ts ([WILDLINE]) +└── http://localhost:4545/welcome.ts ([WILDLINE]B) diff --git a/tests/specs/permission/allow_import_info/import_not_allowed.out b/tests/specs/permission/allow_import_info/import_not_allowed.out new file mode 100644 index 0000000000..c73eced93a --- /dev/null +++ b/tests/specs/permission/allow_import_info/import_not_allowed.out @@ -0,0 +1,7 @@ +local: [WILDLINE] +type: TypeScript +dependencies: [WILDLINE] +size: [WILDLINE] + +file:///[WILDLINE]/allow_import_info/main.ts ([WILDLINE]) +└── http://localhost:4545/welcome.ts (not capable, requires --allow-import) diff --git a/tests/specs/permission/allow_import_info/main.ts b/tests/specs/permission/allow_import_info/main.ts new file mode 100644 index 0000000000..7556f22667 --- /dev/null +++ b/tests/specs/permission/allow_import_info/main.ts @@ -0,0 +1 @@ +import "http://localhost:4545/welcome.ts"; diff --git a/tests/specs/permission/allow_import_info/success.out b/tests/specs/permission/allow_import_info/success.out new file mode 100644 index 0000000000..1b43d71444 --- /dev/null +++ b/tests/specs/permission/allow_import_info/success.out @@ -0,0 +1,7 @@ +Download http://localhost:4545/welcome.ts +local: [WILDLINE] +type: TypeScript +dependencies: [WILDLINE] +size: [WILDLINE] + +http://localhost:4545/welcome.ts ([WILDLINE]B)