Start adding tests and fix it not working for run

denoland · dsherret · Sep 26, 2024 · Sep 5, 2024 · Sep 5, 2024 · Sep 17, 2024
commit 479d046019d022b9ebbac9f3495618d8a29d04da
diff --git a/cli/factory.rs b/cli/factory.rs
@@ -824,6 +824,7 @@ impl CliFactory {
       npm_resolver.clone(),
       self.permission_desc_parser()?.clone(),
       self.root_cert_store_provider().clone(),
+      self.root_permissions_container()?.clone(),
       StorageKeyResolver::from_options(cli_options),
       cli_options.sub_command().clone(),
       self.create_cli_main_worker_options()?,

diff --git a/cli/graph_container.rs b/cli/graph_container.rs
@@ -84,6 +84,7 @@ impl MainModuleGraphContainer {
       )
       .await?;
     graph_permit.commit();
+    self.root_permissions.mark_loaded_static_graph();
     Ok(())
   }
 

diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs
@@ -687,6 +687,7 @@ pub async fn run(
       Permissions::from_options(desc_parser.as_ref(), &permissions)?;
     PermissionsContainer::new(desc_parser, permissions)
   };
+  permissions.mark_loaded_static_graph();
   let feature_checker = Arc::new({
     let mut checker = FeatureChecker::default();
     checker.set_exit_cb(Box::new(crate::unstable_exit_cb));
@@ -713,6 +714,7 @@ pub async fn run(
     npm_resolver,
     permission_desc_parser,
     root_cert_store_provider,
+    permissions.clone(),
     StorageKeyResolver::empty(),
     crate::args::DenoSubcommand::Run(Default::default()),
     CliMainWorkerOptions {

diff --git a/cli/tools/info.rs b/cli/tools/info.rs
@@ -644,8 +644,21 @@ impl<'a> GraphDisplayContext<'a> {
       ModuleError::InvalidTypeAssertion { .. } => {
         self.build_error_msg(specifier, "(invalid import attribute)")
       }
-      ModuleError::LoadingErr(_, _, _) => {
-        self.build_error_msg(specifier, "(loading error)")
+      ModuleError::LoadingErr(_, _, err) => {
+        use deno_graph::ModuleLoadError::*;
+        let message = match err {
+          HttpsChecksumIntegrity(_) => "(checksum integrity error)",
+          Decode(_) => "(loading decode error)",
+          Loader(err) => match deno_core::error::get_custom_error_class(err) {
+            Some("NotCapable") => "(not capable, requires --allow-imports)",
+            _ => "(loading error)",
+          },
+          Jsr(_) => "(loading error)",
+          NodeUnknownBuiltinModule(_) => "(unknown node built-in error)",
+          Npm(_) => "(npm loading error)",
+          TooManyRedirects => "(too many redirects error)",
+        };
+        self.build_error_msg(specifier, message.as_ref())
       }
       ModuleError::ParseErr(_, _) => {
         self.build_error_msg(specifier, "(parsing error)")

diff --git a/cli/worker.rs b/cli/worker.rs
@@ -136,6 +136,7 @@ struct SharedWorkerState {
   npm_resolver: Arc<dyn CliNpmResolver>,
   permission_desc_parser: Arc<RuntimePermissionDescriptorParser>,
   root_cert_store_provider: Arc<dyn RootCertStoreProvider>,
+  root_permissions: PermissionsContainer,
   shared_array_buffer_store: SharedArrayBufferStore,
   storage_key_resolver: StorageKeyResolver,
   options: CliMainWorkerOptions,
@@ -432,6 +433,7 @@ impl CliMainWorkerFactory {
     npm_resolver: Arc<dyn CliNpmResolver>,
     permission_parser: Arc<RuntimePermissionDescriptorParser>,
     root_cert_store_provider: Arc<dyn RootCertStoreProvider>,
+    root_permissions: PermissionsContainer,
     storage_key_resolver: StorageKeyResolver,
     subcommand: DenoSubcommand,
     options: CliMainWorkerOptions,
@@ -452,6 +454,7 @@ impl CliMainWorkerFactory {
         npm_resolver,
         permission_desc_parser: permission_parser,
         root_cert_store_provider,
+        root_permissions,
         shared_array_buffer_store: Default::default(),
         storage_key_resolver,
         options,
@@ -481,7 +484,7 @@ impl CliMainWorkerFactory {
     &self,
     mode: WorkerExecutionMode,
     main_module: ModuleSpecifier,
-    permissions: PermissionsContainer,
+    dynamic_permissions: PermissionsContainer,
     custom_extensions: Vec<Extension>,
     stdio: deno_runtime::deno_io::Stdio,
   ) -> Result<CliMainWorker, AnyError> {
@@ -532,10 +535,8 @@ impl CliMainWorkerFactory {
 
     let ModuleLoaderAndSourceMapGetter { module_loader } =
       shared.module_loader_factory.create_for_main(
-        PermissionsContainer::allow_all(
-          self.shared.permission_desc_parser.clone(),
-        ),
-        permissions.clone(),
+        self.shared.root_permissions.clone(),
+        dynamic_permissions.clone(),
       );
     let maybe_inspector_server = shared.maybe_inspector_server.clone();
 
@@ -633,11 +634,9 @@ impl CliMainWorkerFactory {
       v8_code_cache: shared.code_cache.clone(),
     };
 
-    permissions.mark_loaded_static_graph();
-
     let mut worker = MainWorker::bootstrap_from_options(
       main_module.clone(),
-      permissions,
+      dynamic_permissions,
       options,
     );
 

diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs
@@ -2146,6 +2146,10 @@ impl PermissionsContainer {
       "data" => Ok(()),
       "blob" => Ok(()),
       _ => {
+        if inner.import.is_allow_all() {
+          return Ok(()); // avoid allocation below
+        }
+
         let desc = self
           .descriptor_parser
           .parse_import_descriptor_from_url(specifier)?;

diff --git a/tests/specs/permission/allow_imports/__test__.jsonc b/tests/specs/permission/allow_imports/__test__.jsonc
@@ -0,0 +1,28 @@
+{
+  "tests": {
+    "info": {
+      "args": "info main.ts",
+      "output": "info.out"
+    },
+    "cache": {
+      "args": "cache main.ts",
+      "output": "cache.out",
+      "exitCode": 1
+    },
+    "check": {
+      "args": "check main.ts",
+      "output": "check.out",
+      "exitCode": 1
+    },
+    "compile": {
+      "args": "compile main.ts",
+      "output": "compile.out",
+      "exitCode": 1
+    },
+    "run": {
+      "args": "run main.ts",
+      "output": "run.out",
+      "exitCode": 1
+    }
+  }
+}
diff --git a/tests/specs/permission/allow_imports/cache.out b/tests/specs/permission/allow_imports/cache.out
@@ -0,0 +1,2 @@
+error: Requires import access to "example.com:443", run again with the --allow-import flag
+    at file:///[WILDLINE]/allow_imports/main.ts:1:8
diff --git a/tests/specs/permission/allow_imports/check.out b/tests/specs/permission/allow_imports/check.out
@@ -0,0 +1,2 @@
+error: Requires import access to "example.com:443", run again with the --allow-import flag
+    at file:///[WILDLINE]/allow_imports/main.ts:1:8
diff --git a/tests/specs/permission/allow_imports/compile.out b/tests/specs/permission/allow_imports/compile.out
@@ -0,0 +1,2 @@
+error: Requires import access to "example.com:443", run again with the --allow-import flag
+    at file:///[WILDLINE]/allow_imports/main.ts:1:8
diff --git a/tests/specs/permission/allow_imports/info.out b/tests/specs/permission/allow_imports/info.out
@@ -0,0 +1,7 @@
+local: [WILDLINE]main.ts
+type: TypeScript
+dependencies: 0 unique
+size: [WILDLINE]
+
+file:///[WILDLINE]/allow_imports/main.ts ([WILDLINE])
+└── https://example.com/malicious_string (not capable, requires --allow-imports)
diff --git a/tests/specs/permission/allow_imports/main.ts b/tests/specs/permission/allow_imports/main.ts
@@ -0,0 +1 @@
+import "https://example.com/malicious_string";
diff --git a/tests/specs/permission/allow_imports/run.out b/tests/specs/permission/allow_imports/run.out
@@ -0,0 +1,2 @@
+error: Requires import access to "example.com:443", run again with the --allow-import flag
+    at file:///[WILDLINE]/allow_imports/main.ts:1:8