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

List all required permissions at once #3655

Closed
isAdrisal opened this issue Jan 11, 2020 · 15 comments
Closed

List all required permissions at once #3655

isAdrisal opened this issue Jan 11, 2020 · 15 comments
Labels
permissions related to --allow-* flags

Comments

@isAdrisal
Copy link

isAdrisal commented Jan 11, 2020

Testing out Deno for the first time on Windows and ran into an ergonomic issue with permissions.

Running deno test without permissions flags takes around 30 seconds before aborting and prompting for --allow-env.

Running again with --allow-env takes another 30 seconds before aborting and prompting for --allow-write.

Is it possible to have all required permissions listed after the first attempt to run a particular script?

@bartlomieju
Copy link
Member

bartlomieju commented Jan 11, 2020

Permissions are are queried during runtime so there's really no way to do that.

However, before #3296 we had this behavior that prompted user user about permissions in terminal instead of throwing error, I really wish it's brought back under --prompt flag.

@ry thoughts?

@hayd
Copy link
Contributor

hayd commented Jan 11, 2020

For deno test, in std, the permissions could be asserted prior to beginning the tests.
Similarly for anyone writing their own script.

@bartlomieju
Copy link
Member

For deno test, in std, the permissions could be asserted prior to beginning the tests.
Similarly for anyone writing their own script.

@hayd can you elaborate?

@rsp
Copy link
Contributor

rsp commented Jan 11, 2020

@isAdrisal, as @hayd pointed out, the deno test is special here but this may be a problem for programs other than tests so it may be worth to think about.

I see two ways to write a program that doesn't suffer from the delayed exception:

  1. With the current API:
    • If my program needs to read an env var some time later, I can move it to the beginning of the program to make it fail fast.
    • If it needs to write a file later then I cannot move it to the beginning because at that point I may have nothing to write, but I can try to open a file for writing and make it fail fast if I have no permissions.
  2. With a new API to test for permissions:
    • For every operation that needs some permission there could be a way to test for having that permission.

E.g.

assert.allowWrite('/tmp'); // throws just like writing
// but with no need to write a dummy file just to see if it can

test.allowWrite('/tmp'); // returns boolean

It would also help in case when there is --prompt because it's more convenient to be asked about the permissions instantly than 30 seconds later, especially when I start a program, see that it's busy and go for a coffee only to find out 10 minutes later that it got stuck after 30 seconds.

@rsp
Copy link
Contributor

rsp commented Jan 11, 2020

By the way, as for the permissions for deno test, it always concerns me that it asks for --allow-write while I believe it only needs --allow-write=. or even --allow-write=./one-file

@bartlomieju
Copy link
Member

By the way, as for the permissions for deno test, it always concerns me that it asks for --allow-write while I believe it only needs --allow-write=. or even --allow-write=./one-file

👍 feel free to open issue for that, it's because of a hack I used in test runner, might be possible to remove it

@isAdrisal
Copy link
Author

isAdrisal commented Jan 11, 2020

Thanks for the detailed explanations!

@rsp I've had a brief play with the current permissions.request() api using examples from #3296. Including something like this at the beginning of a script provides instant prompts in the terminal for the required permissions. Is this similar to what you meant in your first scenario?

const requestPerms = async (perms) => {
  for (const perm of perms) {
    await Deno.permissions.request({ name: perm });
  };
};

requestPerms(["read", "write"]);

@bartlomieju This api provides fairly nice user prompts. Would it be possible to implement something similar in deno test?

perm-prompts

@rsp
Copy link
Contributor

rsp commented Jan 11, 2020

@bartlomieju I didn't know it was your hack - I thought it was quite clever when I discovered what's going on after I saw the .deno.test.ts file flashing.

@David-Else
Copy link

David-Else commented Jan 12, 2020

I just faced the frustration of:

deno test/test_test.ts > run again with the --allow-env flag
deno test test/test_test.ts --allow-env > run again with the --allow-write flag
deno test test/test_test.ts --allow-env --allow-write flag > run again with the --allow-net flag
deno test test/test_test.ts --allow-env --allow-write --allow-net flag > Uncaught PermissionDenied: run again with the --allow-run flag

There was no long delay like the OP reported, but It is bordering on unusable like this. I hope a solution can be found. . Thanks.

@rsp
Copy link
Contributor

rsp commented Jan 13, 2020

@David-Else actually not only a single message would be nice but also those wide privileges are most likely not needed at all. I'm sure it doesn't need --allow-write to the entire filesystem, but when we grant it that then any mistake in the script could potentially corrupt all your data.

When I got a series of those "run again" messages, my first wishes were:

  • tell about all privileges at once
  • tell me specific --allow-write=x instead of all-powerful --allow-write and --allow-run
  • tell me what line of code needs it, so I can review it myself (first time I saw a program asking me for basically full access to all my data just to run tests, I thought that it might be some malicious code and didn't run it as it asked me before digging into the code)

@nayeemrmn
Copy link
Collaborator

We can't know about permissions required by code that hasn't run yet. User code needs to use Deno.permissions.query() to assert all required privileges ahead of time, as @hayd suggested.

We should provide a convenience function to do this: assert a set of permissions and provide a complete error message specifying all that aren't met.

@sholladay
Copy link

If we could do something like this:

await Deno.permissions.request(['one', 'two']);

... then a program could more easily request all of its permissions up front and it would be possible to display only a single prompt.

@hayd
Copy link
Contributor

hayd commented Jan 13, 2020

Something like that might make sense in std asserts.ts?

@rsp
Copy link
Contributor

rsp commented Jan 14, 2020

This:

await Deno.permissions.request(['one', 'two']);

Looks similar to:

await navigator.permissions.requestAll(...);

using the Web Permissions API. Maybe it would be a good idea to have a similar API with:

  • navigator.permissions.query()
  • navigator.permissions.request()
  • navigator.permissions.requestAll()

See:

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Apr 26, 2020

Closed in #4258. An std module exists for this now. --prompt discussion is tracked in #3811. deno test no longer requires technical permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
permissions related to --allow-* flags
Projects
None yet
Development

No branches or pull requests

7 participants