-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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: warn when using --allow-run
with no allow list
#25215
feat: warn when using --allow-run
with no allow list
#25215
Conversation
@@ -15,8 +15,10 @@ | |||
"output": "5\n" | |||
}, | |||
{ | |||
"commandName": "mv", |
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.
No mv
command on Windows (the Windows CI adds it, but it doesn't exist normally)
I believe that to effectively serve their purpose and avoid becoming footguns, these warnings should be accompanied by a mechanism to suppress them, simillar to |
I don't agree. These warnings are for what is essentially |
// discourage using --allow-run without an allow list | ||
if allow_run_list.is_empty() { | ||
log::warn!( | ||
"{} --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)", |
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.
"{} --allow-run can be trivially exploited. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)", | |
"{} --allow-run without an allow list is susceptible to exploits. Prefer specifying an allow list (https://docs.deno.com/runtime/fundamentals/security/#running-subprocesses)", |
--allow-run
without an allow list is security theatre. Any script could just launchDeno.execPath()
with full permissions or do something likedeno eval '<something malicious goes here>'
.