-
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
feat(permissions): allow run permission to take values #9833
Conversation
# Conflicts: # runtime/ops/worker_host.rs
# Conflicts: # runtime/ops/worker_host.rs # runtime/permissions.rs
cli/flags.rs
Outdated
if let Some(run_wl) = matches.values_of("allow-run") { | ||
let run_allowlist: Vec<String> = run_wl.map(ToString::to_string).collect(); | ||
flags.allow_run = Some(run_allowlist); | ||
debug!("env allowlist: {:#?}", &flags.allow_run); |
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.
Remove?
.min_values(0) | ||
.takes_value(true) | ||
.use_delimiter(true) | ||
.require_equals(true) |
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 some kind of validator is needed here, similar to --allow-env
PR
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.
sure, but how would we validate passed commands? check if they actually exist? That would be somewhat weird, as we don't chck if a path exists for the path related permissions
@crowlKats this PR looks good to me, let's land it for 1.9. Please rebase |
# Conflicts: # runtime/ops/process.rs
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.
@crowlKats before landing can you add an integration test to cli/tests/integration_tests.rs
that uses an allow list? Something like _045_proxy
that runs Deno executable.
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, great feature @crowlKats!
Closes #9925