Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 90 additions & 20 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ pub struct PermissionFlags {
pub deny_net: Option<Vec<String>>,
pub allow_read: Option<Vec<String>>,
pub deny_read: Option<Vec<String>>,
pub ignore_read: Option<Vec<String>>,
pub allow_run: Option<Vec<String>>,
pub deny_run: Option<Vec<String>>,
pub allow_sys: Option<Vec<String>>,
Expand All @@ -848,6 +849,7 @@ impl PermissionFlags {
|| self.deny_net.is_some()
|| self.allow_read.is_some()
|| self.deny_read.is_some()
|| self.ignore_read.is_some()
|| self.allow_run.is_some()
|| self.deny_run.is_some()
|| self.allow_sys.is_some()
Expand Down Expand Up @@ -981,11 +983,22 @@ impl Flags {
}

match &self.permissions.ignore_env {
Some(env_ignorelist) if env_ignorelist.is_empty() => {
Some(ignorelist) if ignorelist.is_empty() => {
args.push("--ignore-env".to_string());
}
Some(env_ignorelist) => {
let s = format!("--ignore-env={}", env_ignorelist.join(","));
Some(ignorelist) => {
let s = format!("--ignore-env={}", ignorelist.join(","));
args.push(s);
}
_ => {}
}

match &self.permissions.ignore_read {
Some(ignorelist) if ignorelist.is_empty() => {
args.push("--ignore-read".to_string());
}
Some(ignorelist) => {
let s = format!("--ignore-read={}", ignorelist.join(","));
args.push(s);
}
_ => {}
Expand Down Expand Up @@ -4223,6 +4236,20 @@ fn permission_args(app: Command, requires: Option<&'static str>) -> Command {
}
arg
};
let make_deny_ignore_read_arg = |arg: Arg| {
let mut arg = arg
.num_args(0..)
.action(ArgAction::Append)
.require_equals(true)
.value_name("PATH")
.long_help("false")
.value_hint(ValueHint::AnyPath)
.hide(true);
if let Some(requires) = requires {
arg = arg.requires(requires)
}
arg
};
Comment on lines +4239 to +4252
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

--ignore-read parsing doesn’t split comma‑separated paths (behavior mismatch with docs and other read flags)

Right now ignore-read values are collected as raw strings:

  • deny-read goes through flat_escape_split_commas(...) to support --deny-read=/etc,/var/log.txt with the same escaping rules as --allow-read.
  • ignore-read uses the same Arg shape (make_deny_ignore_read_arg) but in permission_args_parse it just does read_wl.collect(), so --ignore-read=/etc,/var/log.txt becomes a single entry "/etc,/var/log.txt".

Given the help text and example:

  • --ignore-read[=<<PATH>...]]
  • --ignore-read="/etc,/var/log.txt"

this is surprising and can lead to ignore-read silently not matching what users expect.

To align semantics with the other read permissions and the documented example, I recommend:

 fn permission_args_parse(
   flags: &mut Flags,
   matches: &mut ArgMatches,
 ) -> clap::error::Result<()> {
@@
   if let Some(read_wl) = matches.remove_many::<String>("deny-read") {
     let read_wl = read_wl
       .flat_map(flat_escape_split_commas)
       .collect::<Result<Vec<_>, _>>()?;
     flags.permissions.deny_read = Some(read_wl);
   }
 
-
-  if let Some(read_wl) = matches.remove_many::<String>("ignore-read") {
-    flags.permissions.ignore_read = Some(read_wl.collect());
-    debug!("read ignorelist: {:#?}", &flags.permissions.ignore_read);
-  }
+  if let Some(read_wl) = matches.remove_many::<String>("ignore-read") {
+    let read_wl = read_wl
+      .flat_map(flat_escape_split_commas)
+      .collect::<Result<Vec<_>, _>>()?;
+    flags.permissions.ignore_read = Some(read_wl);
+    debug!("read ignorelist: {:#?}", &flags.permissions.ignore_read);
+  }

With this, --ignore-read=/etc,/var/log.txt behaves like the help suggests and matches how allow-read/deny-read are parsed.

Also, if you adopt this, combining it with the join_paths tweak in to_permission_args() will keep round‑tripping consistent.

Also applies to: 4294-4297, 4346-4347, 6744-6747

🤖 Prompt for AI Agents
In cli/args/flags.rs around lines 4239-4252 (and similarly at 4294-4297,
4346-4347, 6744-6747), the --ignore-read flag values are being collected as raw
strings which preserves comma-separated lists as a single entry; update the
parsing to use the same comma-splitting logic as deny-read by piping the
iterator through flat_escape_split_commas(...) (or otherwise splitting with the
same escaping rules used for allow-/deny-read) before collecting so that
`--ignore-read=/etc,/var/log.txt` produces two paths; apply the same change at
the other cited locations and ensure any downstream code that expects joined
paths is kept consistent (e.g., keep the join_paths tweak in
to_permission_args()).

app
.after_help(cstr!(r#"<y>Permission options:</>
<y>Docs</>: <c>https://docs.deno.com/go/permissions</>
Expand Down Expand Up @@ -4264,6 +4291,10 @@ fn permission_args(app: Command, requires: Option<&'static str>) -> Command {
<p(245)>--deny-ffi | --deny-ffi="./libfoo.so"</>
<g>--deny-import[=<<IP_OR_HOSTNAME>...]</> Deny importing from remote hosts. Optionally specify denied IP addresses and host names, with ports as necessary.
<p(245)>--deny-import | --deny-import="example.com:443,github.com:443"</>
<g>--ignore-env[=<<VARIABLE_NAME>...]</> Ignore access to environment variables returning `undefined`. Optionally specify ignored environment variables.
<p(245)>--ignore-env | --ignore-env="PORT,HOME,PATH"</>
<g>--ignore-read[=<<PATH>...]</> Ignore file system read access with a `NotFound` error. Optionally specify ignored paths.
<p(245)>--ignore-read | --ignore-read="/etc,/var/log.txt"</>
<g>DENO_TRACE_PERMISSIONS</> Environmental variable to enable stack traces in permission prompts.
<p(245)>DENO_TRACE_PERMISSIONS=1 deno run main.ts</>
<g>DENO_AUDIT_PERMISSIONS</> Environmental variable to generate a JSONL file with all permissions accesses.
Expand Down Expand Up @@ -4312,23 +4343,8 @@ fn permission_args(app: Command, requires: Option<&'static str>) -> Command {
arg
}
)
.arg(
{
let mut arg = Arg::new("deny-read")
.long("deny-read")
.num_args(0..)
.action(ArgAction::Append)
.require_equals(true)
.value_name("PATH")
.long_help("false")
.value_hint(ValueHint::AnyPath)
.hide(true);
if let Some(requires) = requires {
arg = arg.requires(requires)
}
arg
}
)
.arg(make_deny_ignore_read_arg(Arg::new("deny-read").long("deny-read")))
.arg(make_deny_ignore_read_arg(Arg::new("ignore-read").long("ignore-read")))
.arg(
{
let mut arg = Arg::new("allow-write")
Expand Down Expand Up @@ -6725,6 +6741,11 @@ fn permission_args_parse(
flags.permissions.deny_read = Some(read_wl);
}

if let Some(read_wl) = matches.remove_many::<String>("ignore-read") {
flags.permissions.ignore_read = Some(read_wl.collect());
debug!("read ignorelist: {:#?}", &flags.permissions.ignore_read);
}

if let Some(write_wl) = matches.remove_many::<String>("allow-write") {
let write_wl = write_wl
.flat_map(flat_escape_split_commas)
Expand Down Expand Up @@ -9211,6 +9232,55 @@ mod tests {
);
}

#[test]
fn ignore_read_ignorelist() {
let r = flags_from_vec(svec![
"deno",
"run",
"--ignore-read=something.txt",
"script.ts"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags::new_default(
"script.ts".to_string(),
)),
permissions: PermissionFlags {
ignore_read: Some(svec!["something.txt"]),
..Default::default()
},
code_cache_enabled: true,
..Flags::default()
}
);
}

#[test]
fn ignore_read_ignorelist_multiple() {
let r = flags_from_vec(svec![
"deno",
"run",
"--ignore-read=something.txt",
"--ignore-read=something2.txt",
"script.ts"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags::new_default(
"script.ts".to_string(),
)),
permissions: PermissionFlags {
ignore_read: Some(svec!["something.txt", "something2.txt"]),
..Default::default()
},
code_cache_enabled: true,
..Flags::default()
}
);
}

#[test]
fn allow_write_allowlist() {
use test_util::TempDir;
Expand Down
18 changes: 17 additions & 1 deletion cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1677,6 +1677,11 @@ fn flags_to_permissions_options(
config.and_then(|c| c.permissions.read.deny.as_ref()),
&make_fs_config_value_absolute,
),
ignore_read: handle_deny_or_ignore(
flags.ignore_read.as_ref(),
config.and_then(|c| c.permissions.read.ignore.as_ref()),
&make_fs_config_value_absolute,
),
allow_run: handle_allow(
flags.allow_all,
config.and_then(|c| c.permissions.all),
Expand Down Expand Up @@ -1819,14 +1824,17 @@ mod test {
.unwrap(),
permissions: PermissionsObject {
all: None,
read: AllowDenyPermissionConfig {
read: AllowDenyIgnorePermissionConfig {
allow: Some(PermissionConfigValue::Some(vec![
".".to_string(),
"./read-allow".to_string(),
])),
deny: Some(PermissionConfigValue::Some(vec![
"./read-deny".to_string(),
])),
ignore: Some(PermissionConfigValue::Some(vec![
"./read-ignore".to_string(),
])),
},
write: AllowDenyPermissionConfig {
allow: Some(PermissionConfigValue::Some(vec![
Expand Down Expand Up @@ -1930,6 +1938,13 @@ mod test {
.into_string()
.unwrap()
]),
ignore_read: Some(vec![
base_dir
.join("read-ignore")
.into_os_string()
.into_string()
.unwrap()
]),
allow_run: Some(vec![
"run-allow".to_string(),
base_dir
Expand Down Expand Up @@ -2005,6 +2020,7 @@ mod test {
deny_ffi: None,
allow_read: Some(vec!["./folder".to_string()]),
deny_read: None,
ignore_read: None,
allow_run: Some(vec![]),
deny_run: None,
allow_sys: Some(vec![]),
Expand Down
24 changes: 17 additions & 7 deletions libs/config/deno_json/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ pub struct PermissionsObjectWithBase {
pub struct PermissionsObject {
#[serde(default)]
pub all: Option<bool>,
#[serde(default, deserialize_with = "deserialize_allow_deny")]
pub read: AllowDenyPermissionConfig,
#[serde(default, deserialize_with = "deserialize_allow_deny_ignore")]
pub read: AllowDenyIgnorePermissionConfig,
#[serde(default, deserialize_with = "deserialize_allow_deny")]
pub write: AllowDenyPermissionConfig,
#[serde(default, deserialize_with = "deserialize_allow_deny")]
Expand Down Expand Up @@ -293,9 +293,10 @@ mod test {
.unwrap(),
PermissionsObject {
all: Some(true),
read: AllowDenyPermissionConfig {
read: AllowDenyIgnorePermissionConfig {
allow: Some(PermissionConfigValue::All),
deny: None,
ignore: None,
},
write: AllowDenyPermissionConfig {
allow: Some(PermissionConfigValue::All),
Expand Down Expand Up @@ -343,9 +344,10 @@ mod test {
.unwrap(),
PermissionsObject {
all: None,
read: AllowDenyPermissionConfig {
read: AllowDenyIgnorePermissionConfig {
allow: Some(PermissionConfigValue::Some(vec!["test".to_string()])),
deny: None
deny: None,
ignore: None,
},
write: AllowDenyPermissionConfig {
allow: Some(PermissionConfigValue::Some(vec!["test".to_string()])),
Expand Down Expand Up @@ -384,6 +386,7 @@ mod test {
"read": {
"allow": ["test"],
"deny": ["test-deny"],
"ignore": ["test-ignore"],
},
"write": [],
"sys": {
Expand All @@ -393,11 +396,14 @@ mod test {
.unwrap(),
PermissionsObject {
all: None,
read: AllowDenyPermissionConfig {
read: AllowDenyIgnorePermissionConfig {
allow: Some(PermissionConfigValue::Some(vec!["test".to_string()])),
deny: Some(PermissionConfigValue::Some(vec![
"test-deny".to_string()
])),
ignore: Some(PermissionConfigValue::Some(vec![
"test-ignore".to_string()
]))
},
write: AllowDenyPermissionConfig {
allow: Some(PermissionConfigValue::None),
Expand All @@ -416,16 +422,20 @@ mod test {
"read": {
"allow": true,
"deny": ["test-deny"],
"ignore": ["test-ignore"],
},
}))
.unwrap(),
PermissionsObject {
all: None,
read: AllowDenyPermissionConfig {
read: AllowDenyIgnorePermissionConfig {
allow: Some(PermissionConfigValue::All),
deny: Some(PermissionConfigValue::Some(vec![
"test-deny".to_string()
])),
ignore: Some(PermissionConfigValue::Some(vec![
"test-ignore".to_string()
])),
},
..Default::default()
}
Expand Down
Loading
Loading