Dynamic import should respect permissions

[teleclimber]

(async () => {
	const filename = "../../../../some/sensitive/dir/data.json";

	let secrets = await import(filename);   // no prompts from Deno cli
	console.log(JSON.stringify(secrets));

	let file = await Deno.open(filename);    // prompts: "Deno requests read access...."
        await Deno.copy(Deno.stdout, file);
        file.close();
})();

Now that Deno has dynamic imports, file paths no longer need to be hard-coded into the script, making it trivial for a malicious script to probe possible directories in a loop hoping to find a juicy JSON.

According to this comment import might one day be able to read more than just JSON files, making this even more problematic and basically defeating the purpose of --allow-read.

I suggest that any import that resolves to a file outside the "main" script's directory should have an explicit "read" permission, such as --allow-read or `--allow-read=/path/to/import/".

[kevinkassimo]

I agree this is indeed a problem comparing to static imports. However currently our dynamic import callback is implemented in core/ which has no idea about the whitelists or permission flags, which resides in cli/. Maybe we could add a policy check callback to core/ that takes a string, default to predicate that always return true, and allows cli/ (worker) to overwrite it?

[kitsonk]

Or we disallow dynamic import of JSON, or we disallow root breaking for dynamic imports, or we put dynamic imports behind a flag.

[ry]

Yes this is a problem. And —allow-net is needed to load remote URLs.

@kevinkassimo It’s not so bad - there is a hook in CLI for dynamic import. Security checks can be put there.

deno/cli/worker.rs

Line 41 in 54982e9

i.set_dyn_import(move |id, specifier, referrer| {