Skip to content

Commit

Permalink
fix: static import permissions in dynamic imports
Browse files Browse the repository at this point in the history
Co-authored-by: Bartek Iwańczuk <[email protected]>
  • Loading branch information
lucacasonato and bartlomieju committed May 17, 2021
1 parent 910935c commit 5151afa
Show file tree
Hide file tree
Showing 39 changed files with 271 additions and 65 deletions.
1 change: 1 addition & 0 deletions cli/lsp/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub async fn cache(
let handler = Arc::new(Mutex::new(FetchHandler::new(
&program_state,
Permissions::allow_all(),
Permissions::allow_all(),
)?));
let mut builder = GraphBuilder::new(handler, maybe_import_map.clone(), None);
builder.add(specifier, false).await
Expand Down
5 changes: 5 additions & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ async fn info_command(
// info accesses dynamically imported modules just for their information
// so we allow access to all of them.
Permissions::allow_all(),
Permissions::allow_all(),
)?));
let mut builder = module_graph::GraphBuilder::new(
handler,
Expand Down Expand Up @@ -478,6 +479,7 @@ async fn cache_command(
specifier,
lib.clone(),
Permissions::allow_all(),
Permissions::allow_all(),
false,
program_state.maybe_import_map.clone(),
)
Expand Down Expand Up @@ -544,6 +546,7 @@ async fn create_module_graph_and_maybe_check(
// when bundling, dynamic imports are only access for their type safety,
// therefore we will allow the graph to access any module.
Permissions::allow_all(),
Permissions::allow_all(),
)?));
let mut builder = module_graph::GraphBuilder::new(
handler,
Expand Down Expand Up @@ -780,6 +783,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<(), AnyError> {
let handler = Arc::new(Mutex::new(FetchHandler::new(
&program_state,
Permissions::allow_all(),
Permissions::allow_all(),
)?));
let mut builder = module_graph::GraphBuilder::new(
handler,
Expand Down Expand Up @@ -942,6 +946,7 @@ async fn test_command(
let handler = Arc::new(Mutex::new(FetchHandler::new(
&program_state,
Permissions::allow_all(),
Permissions::allow_all(),
)?));

let paths_to_watch: Vec<_> = include.iter().map(PathBuf::from).collect();
Expand Down
17 changes: 13 additions & 4 deletions cli/module_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1775,7 +1775,7 @@ impl GraphBuilder {
}
Some(Ok(cached_module)) => {
let is_root = &cached_module.specifier == specifier;
self.visit(cached_module, is_root)?;
self.visit(cached_module, is_root, is_dynamic)?;
}
_ => {}
}
Expand Down Expand Up @@ -1823,6 +1823,7 @@ impl GraphBuilder {
&mut self,
cached_module: CachedModule,
is_root: bool,
is_root_dynamic: bool,
) -> Result<(), AnyError> {
let specifier = cached_module.specifier.clone();
let requested_specifier = cached_module.requested_specifier.clone();
Expand Down Expand Up @@ -1859,14 +1860,22 @@ impl GraphBuilder {
for (_, dep) in module.dependencies.iter() {
let maybe_referrer = Some(dep.location.clone());
if let Some(specifier) = dep.maybe_code.as_ref() {
self.fetch(specifier, &maybe_referrer, dep.is_dynamic);
self.fetch(
specifier,
&maybe_referrer,
is_root_dynamic || dep.is_dynamic,
);
}
if let Some(specifier) = dep.maybe_type.as_ref() {
self.fetch(specifier, &maybe_referrer, dep.is_dynamic);
self.fetch(
specifier,
&maybe_referrer,
is_root_dynamic || dep.is_dynamic,
);
}
}
if let Some((_, specifier)) = module.maybe_types.as_ref() {
self.fetch(specifier, &None, false);
self.fetch(specifier, &None, is_root_dynamic);
}
if specifier != requested_specifier {
self
Expand Down
25 changes: 9 additions & 16 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ pub struct CliModuleLoader {
/// import map file will be resolved and set.
pub import_map: Option<ImportMap>,
pub lib: TypeLib,
/// The initial set of permissions used to resolve the imports in the worker.
/// They are decoupled from the worker permissions since read access errors
/// must be raised based on the parent thread permissions
pub initial_permissions: Rc<RefCell<Option<Permissions>>>,
/// The initial set of permissions used to resolve the static imports in the
/// worker. They are decoupled from the worker (dynamic) permissions since
/// read access errors must be raised based on the parent thread permissions.
pub root_permissions: Permissions,
pub program_state: Arc<ProgramState>,
}

Expand All @@ -42,7 +42,7 @@ impl CliModuleLoader {
Rc::new(CliModuleLoader {
import_map,
lib,
initial_permissions: Rc::new(RefCell::new(None)),
root_permissions: Permissions::allow_all(),
program_state,
})
}
Expand All @@ -60,7 +60,7 @@ impl CliModuleLoader {
Rc::new(CliModuleLoader {
import_map: None,
lib,
initial_permissions: Rc::new(RefCell::new(Some(permissions))),
root_permissions: permissions,
program_state,
})
}
Expand Down Expand Up @@ -125,16 +125,8 @@ impl ModuleLoader for CliModuleLoader {
let maybe_import_map = self.import_map.clone();
let state = op_state.borrow();

// The permissions that should be applied to any dynamically imported module
let dynamic_permissions =
// If there are initial permissions assigned to the loader take them
// and use only once for top level module load.
// Otherwise use permissions assigned to the current worker.
if let Some(permissions) = self.initial_permissions.borrow_mut().take() {
permissions
} else {
state.borrow::<Permissions>().clone()
};
let root_permissions = self.root_permissions.clone();
let dynamic_permissions = state.borrow::<Permissions>().clone();

let lib = self.lib.clone();
drop(state);
Expand All @@ -145,6 +137,7 @@ impl ModuleLoader for CliModuleLoader {
.prepare_module_load(
specifier,
lib,
root_permissions,
dynamic_permissions,
is_dynamic,
maybe_import_map,
Expand Down
18 changes: 7 additions & 11 deletions cli/ops/runtime_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,14 @@ async fn op_emit(
// when we are actually resolving modules without provided sources, we should
// treat the root module as a dynamic import so that runtime permissions are
// applied.
let mut is_dynamic = false;
let handler: Arc<Mutex<dyn SpecifierHandler>> =
if let Some(sources) = args.sources {
Arc::new(Mutex::new(MemoryHandler::new(sources)))
} else {
is_dynamic = true;
Arc::new(Mutex::new(FetchHandler::new(
&program_state,
runtime_permissions.clone(),
runtime_permissions.clone(),
)?))
};
let maybe_import_map = if let Some(import_map_str) = args.import_map_path {
Expand Down Expand Up @@ -103,15 +102,12 @@ async fn op_emit(
};
let mut builder = GraphBuilder::new(handler, maybe_import_map, None);
let root_specifier = resolve_url_or_path(&root_specifier)?;
builder
.add(&root_specifier, is_dynamic)
.await
.map_err(|_| {
type_error(format!(
"Unable to handle the given specifier: {}",
&root_specifier
))
})?;
builder.add(&root_specifier, false).await.map_err(|_| {
type_error(format!(
"Unable to handle the given specifier: {}",
&root_specifier
))
})?;
let bundle_type = match args.bundle {
Some(RuntimeBundleType::Module) => BundleType::Module,
Some(RuntimeBundleType::Classic) => BundleType::Classic,
Expand Down
22 changes: 11 additions & 11 deletions cli/program_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,14 @@ impl ProgramState {
self: &Arc<Self>,
specifiers: Vec<ModuleSpecifier>,
lib: TypeLib,
runtime_permissions: Permissions,
root_permissions: Permissions,
dynamic_permissions: Permissions,
maybe_import_map: Option<ImportMap>,
) -> Result<(), AnyError> {
let handler = Arc::new(Mutex::new(FetchHandler::new(
self,
runtime_permissions.clone(),
root_permissions,
dynamic_permissions,
)?));

let mut builder =
Expand Down Expand Up @@ -221,19 +223,17 @@ impl ProgramState {
self: &Arc<Self>,
specifier: ModuleSpecifier,
lib: TypeLib,
mut runtime_permissions: Permissions,
root_permissions: Permissions,
dynamic_permissions: Permissions,
is_dynamic: bool,
maybe_import_map: Option<ImportMap>,
) -> Result<(), AnyError> {
let specifier = specifier.clone();
// Workers are subject to the current runtime permissions. We do the
// permission check here early to avoid "wasting" time building a module
// graph for a module that cannot be loaded.
if lib == TypeLib::DenoWorker || lib == TypeLib::UnstableDenoWorker {
runtime_permissions.check_specifier(&specifier)?;
}
let handler =
Arc::new(Mutex::new(FetchHandler::new(self, runtime_permissions)?));
let handler = Arc::new(Mutex::new(FetchHandler::new(
self,
root_permissions,
dynamic_permissions,
)?));
let mut builder =
GraphBuilder::new(handler, maybe_import_map, self.lockfile.clone());
builder.add(&specifier, is_dynamic).await?;
Expand Down
20 changes: 12 additions & 8 deletions cli/specifier_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,19 @@ impl CompiledFileMetadata {
pub struct FetchHandler {
/// An instance of disk where generated (emitted) files are stored.
disk_cache: DiskCache,
/// The set of current runtime permissions which need to be applied to
/// dynamic imports.
runtime_permissions: Permissions,
/// The set permissions which are used for root modules (static imports).
root_permissions: Permissions,
/// The set of permissions which are used for dynamic imports.
dynamic_permissions: Permissions,
/// A clone of the `program_state` file fetcher.
file_fetcher: FileFetcher,
}

impl FetchHandler {
pub fn new(
program_state: &Arc<ProgramState>,
runtime_permissions: Permissions,
root_permissions: Permissions,
dynamic_permissions: Permissions,
) -> Result<Self, AnyError> {
let custom_root = env::var("DENO_DIR").map(String::into).ok();
let deno_dir = DenoDir::new(custom_root)?;
Expand All @@ -241,7 +243,8 @@ impl FetchHandler {

Ok(FetchHandler {
disk_cache,
runtime_permissions,
root_permissions,
dynamic_permissions,
file_fetcher,
})
}
Expand All @@ -258,9 +261,9 @@ impl SpecifierHandler for FetchHandler {
// permissions need to be applied. Other static imports have all
// permissions.
let mut permissions = if is_dynamic {
self.runtime_permissions.clone()
self.dynamic_permissions.clone()
} else {
Permissions::allow_all()
self.root_permissions.clone()
};
let file_fetcher = self.file_fetcher.clone();
let disk_cache = self.disk_cache.clone();
Expand Down Expand Up @@ -603,7 +606,8 @@ pub mod tests {

let fetch_handler = FetchHandler {
disk_cache,
runtime_permissions: Permissions::default(),
root_permissions: Permissions::allow_all(),
dynamic_permissions: Permissions::default(),
file_fetcher,
};

Expand Down
4 changes: 4 additions & 0 deletions cli/tests/dynamic_import/permissions_blob_local.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
const code = `import "file:https:///local_file.ts";`;
const blob = new Blob([code]);
await import(URL.createObjectURL(blob));
5 changes: 5 additions & 0 deletions cli/tests/dynamic_import/permissions_blob_local.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Uncaught (in promise) TypeError: Requires read access to "/local_file.ts", run again with the --allow-read flag
at blob:null/[WILDCARD]:1:0
await import(URL.createObjectURL(blob));
^
at async file:https:///[WILDCARD]/cli/tests/dynamic_import/permissions_blob_local.ts:4:1
4 changes: 4 additions & 0 deletions cli/tests/dynamic_import/permissions_blob_remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
const code = `import "https://example.com/some/file.ts";`;
const blob = new Blob([code]);
await import(URL.createObjectURL(blob));
5 changes: 5 additions & 0 deletions cli/tests/dynamic_import/permissions_blob_remote.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Uncaught (in promise) TypeError: Requires net access to "example.com", run again with the --allow-net flag
at blob:null/[WILDCARD]:1:0
await import(URL.createObjectURL(blob));
^
at async file:https:///[WILDCARD]/cli/tests/dynamic_import/permissions_blob_remote.ts:4:1
3 changes: 3 additions & 0 deletions cli/tests/dynamic_import/permissions_data_local.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
const code = `import "file:https:///local_file.ts";`;
await import(`data:application/javascript;base64,${btoa(code)}`);
5 changes: 5 additions & 0 deletions cli/tests/dynamic_import/permissions_data_local.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Uncaught (in promise) TypeError: Requires read access to "/local_file.ts", run again with the --allow-read flag
at data:application/javascript;base64,aW1wb3J0ICJmaWxlOi8vL2xvY2FsX2ZpbGUudHMiOw==:1:0
await import(`data:application/javascript;base64,${btoa(code)}`);
^
at async file:https:///[WILDCARD]/cli/tests/dynamic_import/permissions_data_local.ts:3:1
3 changes: 3 additions & 0 deletions cli/tests/dynamic_import/permissions_data_remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
const code = `import "https://example.com/some/file.ts";`;
await import(`data:application/javascript;base64,${btoa(code)}`);
5 changes: 5 additions & 0 deletions cli/tests/dynamic_import/permissions_data_remote.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Uncaught (in promise) TypeError: Requires net access to "example.com", run again with the --allow-net flag
at data:application/javascript;base64,aW1wb3J0ICJodHRwczovL2V4YW1wbGUuY29tL3NvbWUvZmlsZS50cyI7:1:0
await import(`data:application/javascript;base64,${btoa(code)}`);
^
at async file:https:///[WILDCARD]/cli/tests/dynamic_import/permissions_data_remote.ts:3:1
3 changes: 3 additions & 0 deletions cli/tests/dynamic_import/permissions_remote_remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
await import(
"http:https://localhost:4545/cli/tests/dynamic_import/static_remote.ts"
);
5 changes: 5 additions & 0 deletions cli/tests/dynamic_import/permissions_remote_remote.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: Uncaught (in promise) TypeError: Requires net access to "example.com", run again with the --allow-net flag
at http:https://localhost:4545/cli/tests/dynamic_import/static_remote.ts:2:0
await import(
^
at async file:https:///[WILDCARD]/cli/tests/dynamic_import/permissions_remote_remote.ts:1:1
2 changes: 2 additions & 0 deletions cli/tests/dynamic_import/static_remote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file doesn't really exist, but it doesn't matter, a "PermissionsDenied" error should be thrown.
import "https://example.com/some/file.ts";
Loading

0 comments on commit 5151afa

Please sign in to comment.