feat(cli): don't check permissions for statically analyzable dynamic imports (#18713)

Closes #17697
Closes #17658

cli/cache/mod.rs

@@ -45,10 +45,9 @@ pub const CACHE_PERM: u32 = 0o644;
 /// a concise interface to the DENO_DIR when building module graphs.
 pub struct FetchCacher {
   emit_cache: EmitCache,
-  dynamic_permissions: PermissionsContainer,
   file_fetcher: Arc<FileFetcher>,
   file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
-  root_permissions: PermissionsContainer,
+  permissions: PermissionsContainer,
   cache_info_enabled: bool,
   maybe_local_node_modules_url: Option<ModuleSpecifier>,
 }
@@ -58,16 +57,14 @@ impl FetchCacher {
     emit_cache: EmitCache,
     file_fetcher: Arc<FileFetcher>,
     file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
-    root_permissions: PermissionsContainer,
+    permissions: PermissionsContainer,
-    dynamic_permissions: PermissionsContainer,
     maybe_local_node_modules_url: Option<ModuleSpecifier>,
   ) -> Self {
     Self {
       emit_cache,
-      dynamic_permissions,
       file_fetcher,
       file_header_overrides,
-      root_permissions,
+      permissions,
       cache_info_enabled: false,
       maybe_local_node_modules_url,
     }
@@ -105,7 +102,7 @@ impl Loader for FetchCacher {
   fn load(
     &mut self,
     specifier: &ModuleSpecifier,
-    is_dynamic: bool,
+    _is_dynamic: bool,
   ) -> LoadFuture {
     if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() {
       // The specifier might be in a completely different symlinked tree than
@@ -124,11 +121,7 @@ impl Loader for FetchCacher {
       }
     }
 
-    let permissions = if is_dynamic {
+    let permissions = self.permissions.clone();
-      self.dynamic_permissions.clone()
-    } else {
-      self.root_permissions.clone()
-    };
     let file_fetcher = self.file_fetcher.clone();
     let file_header_overrides = self.file_header_overrides.clone();
     let specifier = specifier.clone();

cli/graph_util.rs

@@ -314,23 +314,18 @@ impl ModuleGraphBuilder {
 
   /// Creates the default loader used for creating a graph.
   pub fn create_graph_loader(&self) -> cache::FetchCacher {
-    self.create_fetch_cacher(
+    self.create_fetch_cacher(PermissionsContainer::allow_all())
-      PermissionsContainer::allow_all(),
-      PermissionsContainer::allow_all(),
-    )
   }
 
   pub fn create_fetch_cacher(
     &self,
-    root_permissions: PermissionsContainer,
+    permissions: PermissionsContainer,
-    dynamic_permissions: PermissionsContainer,
   ) -> cache::FetchCacher {
     cache::FetchCacher::new(
       self.emit_cache.clone(),
       self.file_fetcher.clone(),
       self.options.resolve_file_header_overrides(),
-      root_permissions,
+      permissions,
-      dynamic_permissions,
       self.options.node_modules_dir_specifier(),
     )
   }

cli/lsp/testing/execution.rs

@@ -226,7 +226,6 @@ impl TestRun {
       Permissions::from_options(&ps.options.permissions_options())?;
     test::check_specifiers(
       &ps,
-      permissions.clone(),
       self
         .queue
         .iter()

cli/module_loader.rs

@@ -109,15 +109,12 @@ impl ModuleLoadPreparer {
     roots: Vec<ModuleSpecifier>,
     is_dynamic: bool,
     lib: TsTypeLib,
-    root_permissions: PermissionsContainer,
+    permissions: PermissionsContainer,
-    dynamic_permissions: PermissionsContainer,
   ) -> Result<(), AnyError> {
     log::debug!("Preparing module load.");
     let _pb_clear_guard = self.progress_bar.clear_guard();
 
-    let mut cache = self
+    let mut cache = self.module_graph_builder.create_fetch_cacher(permissions);
-      .module_graph_builder
-      .create_fetch_cacher(root_permissions, dynamic_permissions);
     let maybe_imports = self.options.to_maybe_imports()?;
     let graph_resolver = self.resolver.as_graph_resolver();
     let graph_npm_resolver = self.resolver.as_graph_npm_resolver();
@@ -216,7 +213,6 @@ impl ModuleLoadPreparer {
         false,
         lib,
         PermissionsContainer::allow_all(),
-        PermissionsContainer::allow_all(),
       )
       .await
   }
@@ -537,7 +533,6 @@ impl ModuleLoader for CliModuleLoader {
     let specifier = specifier.clone();
     let module_load_preparer = self.module_load_preparer.clone();
 
-    let dynamic_permissions = self.dynamic_permissions.clone();
     let root_permissions = if is_dynamic {
       self.dynamic_permissions.clone()
     } else {
@@ -547,13 +542,7 @@ impl ModuleLoader for CliModuleLoader {
 
     async move {
       module_load_preparer
-        .prepare_module_load(
+        .prepare_module_load(vec![specifier], is_dynamic, lib, root_permissions)
-          vec![specifier],
-          is_dynamic,
-          lib,
-          root_permissions,
-          dynamic_permissions,
-        )
         .await
     }
     .boxed_local()

cli/tests/integration/run_tests.rs

@@ -2661,6 +2661,11 @@ mod permissions {
       });
   }
 
+  itest!(dynamic_import_static_analysis_no_permissions {
+    args: "run --quiet --reload --no-prompt dynamic_import/static_analysis_no_permissions.ts",
+    output: "dynamic_import/static_analysis_no_permissions.ts.out",
+  });
+
   itest!(dynamic_import_permissions_remote_remote {
     args: "run --quiet --reload --allow-net=localhost:4545 dynamic_import/permissions_remote_remote.ts",
     output: "dynamic_import/permissions_remote_remote.ts.out",

cli/tests/testdata/dynamic_import/permissions_remote_remote.ts

@@ -1,3 +1,3 @@
 await import(
-  "http://localhost:4545/dynamic_import/static_remote.ts"
+  "" + "http://localhost:4545/dynamic_import/static_remote.ts"
 );

cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts

@@ -0,0 +1,13 @@
+try {
+  await import("./empty_1.ts");
+  console.log("✅ Succeeded importing statically analyzable specifier");
+} catch {
+  console.log("❌ Failed importing statically analyzable specifier");
+}
+
+try {
+  await import("" + "./empty_2.ts");
+  console.log("❌ Succeeded importing non-statically analyzable specifier");
+} catch {
+  console.log("✅ Failed importing non-statically analyzable specifier");
+}

cli/tests/testdata/dynamic_import/static_analysis_no_permissions.ts.out

@@ -0,0 +1,2 @@
+✅ Succeeded importing statically analyzable specifier
+✅ Failed importing non-statically analyzable specifier

cli/tests/testdata/run/error_015_dynamic_import_permissions.js

@@ -1,3 +1,3 @@
 (async () => {
-  await import("http://localhost:4545/subdir/mod4.js");
+  await import("" + "http://localhost:4545/subdir/mod4.js");
 })();

cli/tests/testdata/run/error_015_dynamic_import_permissions.out

@@ -1,4 +1,4 @@
 error: Uncaught (in promise) TypeError: Requires net access to "localhost:4545", run again with the --allow-net flag
-  await import("http://localhost:4545/subdir/mod4.js");
+  await import("" + "http://localhost:4545/subdir/mod4.js");
   ^
     at async file://[WILDCARD]/error_015_dynamic_import_permissions.js:2:3

cli/tests/testdata/workers/dynamic_remote.ts

@@ -1,2 +1,2 @@
 // This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
-await import("https://example.com/some/file.ts");
+await import("" + "https://example.com/some/file.ts");

cli/tests/testdata/workers/permissions_dynamic_remote.ts.out

@@ -1,5 +1,5 @@
 error: Uncaught (in worker "") (in promise) TypeError: Requires net access to "example.com", run again with the --allow-net flag
-await import("https://example.com/some/file.ts");
+await import("" + "https://example.com/some/file.ts");
 ^
     at async http://localhost:4545/workers/dynamic_remote.ts:2:1
 [WILDCARD]error: Uncaught (in promise) Error: Unhandled error in child worker.

cli/tools/bench.rs

@@ -418,7 +418,6 @@ impl BenchReporter for ConsoleReporter {
 /// Type check a collection of module and document specifiers.
 async fn check_specifiers(
   ps: &ProcState,
-  permissions: Permissions,
   specifiers: Vec<ModuleSpecifier>,
 ) -> Result<(), AnyError> {
   let lib = ps.options.ts_type_lib_window();
@@ -428,10 +427,8 @@ async fn check_specifiers(
       false,
       lib,
       PermissionsContainer::allow_all(),
-      PermissionsContainer::new(permissions),
     )
     .await?;
-
   Ok(())
 }
 
@@ -654,7 +651,7 @@ pub async fn run_benchmarks(
     return Err(generic_error("No bench modules found"));
   }
 
-  check_specifiers(&ps, permissions.clone(), specifiers.clone()).await?;
+  check_specifiers(&ps, specifiers.clone()).await?;
 
   if bench_options.no_run {
     return Ok(());
@@ -813,7 +810,7 @@ pub async fn run_benchmarks_with_watch(
           .filter(|specifier| modules_to_reload.contains(specifier))
           .collect::<Vec<ModuleSpecifier>>();
 
-      check_specifiers(&ps, permissions.clone(), specifiers.clone()).await?;
+      check_specifiers(&ps, specifiers.clone()).await?;
 
       if bench_options.no_run {
         return Ok(());

cli/tools/test.rs

@@ -1230,7 +1230,6 @@ async fn fetch_inline_files(
 /// Type check a collection of module and document specifiers.
 pub async fn check_specifiers(
   ps: &ProcState,
-  permissions: Permissions,
   specifiers: Vec<(ModuleSpecifier, TestMode)>,
 ) -> Result<(), AnyError> {
   let lib = ps.options.ts_type_lib_window();
@@ -1265,7 +1264,6 @@ pub async fn check_specifiers(
         false,
         lib,
         PermissionsContainer::new(Permissions::allow_all()),
-        PermissionsContainer::new(permissions.clone()),
       )
       .await?;
   }
@@ -1287,7 +1285,6 @@ pub async fn check_specifiers(
       false,
       lib,
       PermissionsContainer::allow_all(),
-      PermissionsContainer::new(permissions),
     )
     .await?;
 
@@ -1648,8 +1645,7 @@ pub async fn run_tests(
     return Err(generic_error("No test modules found"));
   }
 
-  check_specifiers(&ps, permissions.clone(), specifiers_with_mode.clone())
+  check_specifiers(&ps, specifiers_with_mode.clone()).await?;
-    .await?;
 
   if test_options.no_run {
     return Ok(());
@@ -1821,8 +1817,7 @@ pub async fn run_tests_with_watch(
       .filter(|(specifier, _)| modules_to_reload.contains(specifier))
       .collect::<Vec<(ModuleSpecifier, TestMode)>>();
 
-      check_specifiers(&ps, permissions.clone(), specifiers_with_mode.clone())
+      check_specifiers(&ps, specifiers_with_mode.clone()).await?;
-        .await?;
 
       if test_options.no_run {
         return Ok(());