-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: remove --allow-run warning when using deno without args or subcommand #25684
fix: remove --allow-run warning when using deno without args or subcommand #25684
Conversation
cli/args/mod.rs
Outdated
}; | ||
// discourage using --allow-run without an allow list, but | ||
// allow it when no args deno is used [deno] without subcommand | ||
if allow_run_list.is_empty() && !is_no_args_deno { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change this to instead set flags.permissions.allow_all = true;
:
Lines 1370 to 1376 in f360cae
flags.permissions.allow_net = Some(vec![]); | |
flags.permissions.allow_env = Some(vec![]); | |
flags.permissions.allow_run = Some(vec![]); | |
flags.permissions.allow_read = Some(vec![]); | |
flags.permissions.allow_sys = Some(vec![]); | |
flags.permissions.allow_write = Some(vec![]); | |
flags.permissions.allow_ffi = Some(vec![]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's actually a flags.allow_all();
and I think internally that should just set flags.permissions.allow_all = true
only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
I broke this. Looking into it. |
Edit: No need, all checks have passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR addresses issue #25676
Approach
Don't warn when no args deno is used. [command typed as
deno
]Changes
deno repl
. The check is to see if the subcommand is arepl
command with the is_default_command property true, if yes, then this is a no_args_deno and there's no need to warn.no_args_deno
and theallow-run
flag is found, then the warning is shown.Issue root-cause
when using no_args_deno (invoking deno with no args or subcommands), then all of the
allow-*
flags are set by default which caused the warning to get triggered because there's no allow-run-list specified with it.