Skip to content

Commit

Permalink
fix(compile): relative permissions should be retained as relative (de…
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed May 6, 2024
1 parent f698bc7 commit 2dcbef2
Show file tree
Hide file tree
Showing 12 changed files with 624 additions and 364 deletions.
832 changes: 547 additions & 285 deletions cli/args/flags.rs

Large diffs are not rendered by default.

65 changes: 7 additions & 58 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,10 +1524,6 @@ impl CliOptions {
&self.flags.cache_path
}

pub fn no_prompt(&self) -> bool {
resolve_no_prompt(&self.flags)
}

pub fn no_remote(&self) -> bool {
self.flags.no_remote
}
Expand All @@ -1540,45 +1536,12 @@ impl CliOptions {
self.flags.config_flag == deno_config::ConfigFlag::Disabled
}

pub fn permissions_options(&self) -> PermissionsOptions {
PermissionsOptions {
allow_all: self.flags.allow_all,
allow_env: self.flags.allow_env.clone(),
deny_env: self.flags.deny_env.clone(),
allow_hrtime: self.flags.allow_hrtime,
deny_hrtime: self.flags.deny_hrtime,
allow_net: self.flags.allow_net.clone(),
deny_net: self.flags.deny_net.clone(),
allow_ffi: convert_option_str_to_path_buf(
&self.flags.allow_ffi,
self.initial_cwd(),
),
deny_ffi: convert_option_str_to_path_buf(
&self.flags.deny_ffi,
self.initial_cwd(),
),
allow_read: convert_option_str_to_path_buf(
&self.flags.allow_read,
self.initial_cwd(),
),
deny_read: convert_option_str_to_path_buf(
&self.flags.deny_read,
self.initial_cwd(),
),
allow_run: self.flags.allow_run.clone(),
deny_run: self.flags.deny_run.clone(),
allow_sys: self.flags.allow_sys.clone(),
deny_sys: self.flags.deny_sys.clone(),
allow_write: convert_option_str_to_path_buf(
&self.flags.allow_write,
self.initial_cwd(),
),
deny_write: convert_option_str_to_path_buf(
&self.flags.deny_write,
self.initial_cwd(),
),
prompt: !self.no_prompt(),
}
pub fn permission_flags(&self) -> &PermissionFlags {
&self.flags.permissions
}

pub fn permissions_options(&self) -> Result<PermissionsOptions, AnyError> {
self.flags.permissions.to_options(Some(&self.initial_cwd))
}

pub fn reload_flag(&self) -> bool {
Expand Down Expand Up @@ -1871,7 +1834,7 @@ fn resolve_files(
}

/// Resolves the no_prompt value based on the cli flags and environment.
pub fn resolve_no_prompt(flags: &Flags) -> bool {
pub fn resolve_no_prompt(flags: &PermissionFlags) -> bool {
flags.no_prompt || has_flag_env_var("DENO_NO_PROMPT")
}

Expand All @@ -1887,20 +1850,6 @@ pub fn npm_pkg_req_ref_to_binary_command(
binary_name.to_string()
}

fn convert_option_str_to_path_buf(
flag: &Option<Vec<String>>,
initial_cwd: &Path,
) -> Option<Vec<PathBuf>> {
if let Some(allow_ffi_paths) = &flag {
let mut full_paths = Vec::new();
full_paths
.extend(allow_ffi_paths.iter().map(|path| initial_cwd.join(path)));
Some(full_paths)
} else {
None
}
}

#[cfg(test)]
mod test {
use crate::util::fs::FileCollector;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/testing/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl TestRun {
// `PermissionsContainer` - otherwise granting/revoking permissions in one
// file would have impact on other files, which is undesirable.
let permissions =
Permissions::from_options(&factory.cli_options().permissions_options())?;
Permissions::from_options(&factory.cli_options().permissions_options()?)?;
test::check_specifiers(
factory.cli_options(),
factory.file_fetcher()?,
Expand Down
6 changes: 3 additions & 3 deletions cli/standalone/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use deno_core::futures::AsyncSeekExt;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_npm::NpmSystemInfo;
use deno_runtime::permissions::PermissionsOptions;
use deno_semver::package::PackageReq;
use deno_semver::VersionReqSpecifierParseError;
use log::Level;
Expand All @@ -37,6 +36,7 @@ use crate::args::CaData;
use crate::args::CliOptions;
use crate::args::CompileFlags;
use crate::args::PackageJsonDepsProvider;
use crate::args::PermissionFlags;
use crate::args::UnstableConfig;
use crate::cache::DenoDir;
use crate::file_fetcher::FileFetcher;
Expand Down Expand Up @@ -134,7 +134,7 @@ pub enum NodeModules {
pub struct Metadata {
pub argv: Vec<String>,
pub seed: Option<u64>,
pub permissions: PermissionsOptions,
pub permissions: PermissionFlags,
pub location: Option<Url>,
pub v8_flags: Vec<String>,
pub log_level: Option<Level>,
Expand Down Expand Up @@ -621,7 +621,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
argv: compile_flags.args.clone(),
seed: cli_options.seed(),
location: cli_options.location_flag().clone(),
permissions: cli_options.permissions_options(),
permissions: cli_options.permission_flags().clone(),
v8_flags: cli_options.v8_flags().clone(),
unsafely_ignore_certificate_errors: cli_options
.unsafely_ignore_certificate_errors()
Expand Down
4 changes: 3 additions & 1 deletion cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,9 @@ pub async fn run(
};

let permissions = {
let mut permissions = metadata.permissions;
let maybe_cwd = std::env::current_dir().ok();
let mut permissions =
metadata.permissions.to_options(maybe_cwd.as_deref())?;
// if running with an npm vfs, grant read access to it
if let Some(vfs_root) = maybe_vfs_root {
match &mut permissions.allow_read {
Expand Down
4 changes: 2 additions & 2 deletions cli/tools/bench/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ pub async fn run_benchmarks(
// `PermissionsContainer` - otherwise granting/revoking permissions in one
// file would have impact on other files, which is undesirable.
let permissions =
Permissions::from_options(&cli_options.permissions_options())?;
Permissions::from_options(&cli_options.permissions_options()?)?;

let specifiers = collect_specifiers(
bench_options.files,
Expand Down Expand Up @@ -519,7 +519,7 @@ pub async fn run_benchmarks_with_watch(
// `PermissionsContainer` - otherwise granting/revoking permissions in one
// file would have impact on other files, which is undesirable.
let permissions =
Permissions::from_options(&cli_options.permissions_options())?;
Permissions::from_options(&cli_options.permissions_options()?)?;

let graph = module_graph_creator
.create_graph(graph_kind, bench_modules)
Expand Down
30 changes: 23 additions & 7 deletions cli/tools/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ async fn resolve_shim_data(
executable_args.push("--cached-only".to_string());
}

if resolve_no_prompt(flags) {
if resolve_no_prompt(&flags.permissions) {
executable_args.push("--no-prompt".to_string());
}

Expand Down Expand Up @@ -527,6 +527,7 @@ fn is_in_path(dir: &Path) -> bool {
mod tests {
use super::*;

use crate::args::PermissionFlags;
use crate::args::UninstallFlagsGlobal;
use crate::args::UnstableConfig;
use crate::util::fs::canonicalize_path;
Expand Down Expand Up @@ -878,8 +879,11 @@ mod tests {
async fn install_with_flags() {
let shim_data = resolve_shim_data(
&Flags {
allow_net: Some(vec![]),
allow_read: Some(vec![]),
permissions: PermissionFlags {
allow_net: Some(vec![]),
allow_read: Some(vec![]),
..Default::default()
},
type_check_mode: TypeCheckMode::None,
log_level: Some(Level::Error),
..Flags::default()
Expand Down Expand Up @@ -914,7 +918,10 @@ mod tests {
async fn install_prompt() {
let shim_data = resolve_shim_data(
&Flags {
no_prompt: true,
permissions: PermissionFlags {
no_prompt: true,
..Default::default()
},
..Flags::default()
},
&InstallFlagsGlobal {
Expand Down Expand Up @@ -943,7 +950,10 @@ mod tests {
async fn install_allow_all() {
let shim_data = resolve_shim_data(
&Flags {
allow_all: true,
permissions: PermissionFlags {
allow_all: true,
..Default::default()
},
..Flags::default()
},
&InstallFlagsGlobal {
Expand Down Expand Up @@ -973,7 +983,10 @@ mod tests {
let temp_dir = canonicalize_path(&env::temp_dir()).unwrap();
let shim_data = resolve_shim_data(
&Flags {
allow_all: true,
permissions: PermissionFlags {
allow_all: true,
..Default::default()
},
..Flags::default()
},
&InstallFlagsGlobal {
Expand Down Expand Up @@ -1006,7 +1019,10 @@ mod tests {
async fn install_npm_no_lock() {
let shim_data = resolve_shim_data(
&Flags {
allow_all: true,
permissions: PermissionFlags {
allow_all: true,
..Default::default()
},
no_lock: true,
..Flags::default()
},
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/repl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result<i32, AnyError> {
let cli_options = factory.cli_options();
let main_module = cli_options.resolve_main_module()?;
let permissions = PermissionsContainer::new(Permissions::from_options(
&cli_options.permissions_options(),
&cli_options.permissions_options()?,
)?);
let npm_resolver = factory.npm_resolver().await?.clone();
let resolver = factory.resolver().await?.clone();
Expand Down
8 changes: 4 additions & 4 deletions cli/tools/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ To grant permissions, set them before the script argument. For example:
maybe_npm_install(&factory).await?;

let permissions = PermissionsContainer::new(Permissions::from_options(
&cli_options.permissions_options(),
&cli_options.permissions_options()?,
)?);
let worker_factory = factory.create_cli_main_worker_factory().await?;
let mut worker = worker_factory
Expand All @@ -86,7 +86,7 @@ pub async fn run_from_stdin(flags: Flags) -> Result<i32, AnyError> {
let file_fetcher = factory.file_fetcher()?;
let worker_factory = factory.create_cli_main_worker_factory().await?;
let permissions = PermissionsContainer::new(Permissions::from_options(
&cli_options.permissions_options(),
&cli_options.permissions_options()?,
)?);
let mut source = Vec::new();
std::io::stdin().read_to_end(&mut source)?;
Expand Down Expand Up @@ -132,7 +132,7 @@ async fn run_with_watch(
let _ = watcher_communicator.watch_paths(cli_options.watch_paths());

let permissions = PermissionsContainer::new(Permissions::from_options(
&cli_options.permissions_options(),
&cli_options.permissions_options()?,
)?);
let mut worker = factory
.create_cli_main_worker_factory()
Expand Down Expand Up @@ -182,7 +182,7 @@ pub async fn eval_command(
});

let permissions = PermissionsContainer::new(Permissions::from_options(
&cli_options.permissions_options(),
&cli_options.permissions_options()?,
)?);
let worker_factory = factory.create_cli_main_worker_factory().await?;
let mut worker = worker_factory
Expand Down
4 changes: 2 additions & 2 deletions cli/tools/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ pub async fn run_tests(
// `PermissionsContainer` - otherwise granting/revoking permissions in one
// file would have impact on other files, which is undesirable.
let permissions =
Permissions::from_options(&cli_options.permissions_options())?;
Permissions::from_options(&cli_options.permissions_options()?)?;
let log_level = cli_options.log_level();

let specifiers_with_mode = fetch_specifiers_with_test_mode(
Expand Down Expand Up @@ -1834,7 +1834,7 @@ pub async fn run_tests_with_watch(
}?;

let permissions =
Permissions::from_options(&cli_options.permissions_options())?;
Permissions::from_options(&cli_options.permissions_options()?)?;
let graph = module_graph_creator
.create_graph(graph_kind, test_modules)
.await?;
Expand Down
26 changes: 26 additions & 0 deletions tests/specs/compile/relative_permissions/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"tempDir": true,
"steps": [{
"if": "unix",
"args": "compile --output=main --no-prompt --allow-read=a.txt main.ts",
"output": "[WILDCARD]"
}, {
"if": "unix",
"commandName": "./main",
"args": [],
"output": "No such file[WILDCARD]"
}, {
"if": "unix",
"args": [
"eval",
"Deno.mkdirSync('sub_dir');"
],
"output": "[WILDCARD]"
}, {
"if": "unix",
"commandName": "../main",
"cwd": "sub_dir",
"args": [],
"output": "No such file[WILDCARD]"
}]
}
5 changes: 5 additions & 0 deletions tests/specs/compile/relative_permissions/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
try {
Deno.readTextFileSync("a.txt");
} catch (err) {
console.log(err.message);
}

0 comments on commit 2dcbef2

Please sign in to comment.