lib: add security warning on io full access #53609
Conversation
|
Review requested:
|
nodejs-github-bot
added
needs-ci
doc/api/permissions.md
Outdated
| When `--allow-fs-write=*` is permitted, it may inadvertently lead to invalidating | ||
| the permission model because of unintended file access, like full write access | ||
| to memory with `/proc/self/mem`. |
There was a problem hiding this comment.
How does it apply in a Node.js application? I understand the motivation, but I can't see how this could be exposable.
At this point I don't think we need a runtime warning for that. I think adding a doc warning is ok.
There was a problem hiding this comment.
Many projects and programs may recommend permitting all IO operations - and even if they don't, users may do it anyways (like an 'issue' raised in node with pm2, where the author permitted all read operations). A doc warning might be noticed by some, but the immediate jump for libraries that need access to system files outside of the current directory might grant all IO permissions instead of properly checking which ones to grant, which was very common in Deno CLIs (example 1 - inside deno, example 2 - slack install script)
There was a problem hiding this comment.
FWIW, I think this section might be too specific. There are many paths that users should probably never grant (read or write) access to.
Also, /proc/self/mem is a super specific example, and /proc is only one of many problematic things. (And yet, many applications will want to access /proc because it's the only way to implement many monitoring mechanisms on Linux.)
|
Thanks for the PR! |
|
@RafaelGSS @tniessen did those two previous commits address your concerns? |
Adds #53598, warning about access granted to all files.
Should there be a note in the docs, or should that be inlined into the warning?