Skip to content
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

discussion: --allow-exists permission #4895

Closed
nayeemrmn opened this issue Apr 25, 2020 · 4 comments
Closed

discussion: --allow-exists permission #4895

nayeemrmn opened this issue Apr 25, 2020 · 4 comments

Comments

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Apr 25, 2020

For read and write perms, we currently have that:

  • { name: "read", path: "<path>"} implies { name: "read", path: "child of <path>"}
  • { name: "write", path: "<path>"} implies { name: "write", path: "child of <path>"}

I propose having an "exists" perm which adds that:

  • { name: "read", path: "<path>"} implies { name: "exists", path: "<path>"}
  • { name: "write", path: "<path>"} implies { name: "exists", path: "<path>"}
  • { name: "exists", path: "<path>"} implies { name: "exists", path: "parent of <path>"}

{ name: "exists", path: "<path>"} is required by:

  • Deno.cwd() when <path> is the CWD
  • Deno.chdir("<path>")
  • Deno.execPath() when <path> is the executable path

Justifications:

  1. It formalises the logical overlap between the read and write perms. It documents the possibly unintended consequence of the write perm.
  2. The natural property that the "exists" perm propagates upwards instead of downwards applies to no other permission.
  3. The ops listed above are examples of those which only need this permission. Read is unnecessarily powerful.
@bartlomieju
Copy link
Member

I'm not really in favor of adding new permission flag - I think we could use --allow-env for this purpose.

@nayeemrmn
Copy link
Collaborator Author

nayeemrmn commented Apr 25, 2020

I can see --allow-env being nicely used to refer to "shell"-ish features like checking and navigating the cwd and reading environment variables. I thought the same thing before. But how would whitelisting work? Is it granular enough? I don't think it communicates the consequence of being able to sniff around for files.

We need to consider all future ops that don't do any reading or writing, and aren't as nicely associated with the env. It's not uncommon for something to check the existence of a file. It's a unique permission with unique properties.

@nayeemrmn nayeemrmn changed the title --allow-exists permission discussion: --allow-exists permission Apr 25, 2020
@dubiousjim
Copy link
Contributor

I think #4423 is worrying about some of the same issues.

@nayeemrmn
Copy link
Collaborator Author

I think a lot of the purpose expressed here was addressed by check_read_blind() in #5642... or at least it's made this more complicated. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants