Remove --no-prompt ... require users to explicitly prompt for permission

[ry]

Currently if you run a program that, say, tries to read a file from disk, without --allow-read you will see an interactive command line prompt that let's the user elevate permissions. If --no-prompt is given, the program will exit with a PermissionDenied error.

This is not now permission escalation works in the browser. https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia https://developer.mozilla.org/en-US/docs/Web/API/Permissions_API https://w3c.github.io/permissions/

I think we should adopt the Permission.query() API and remove the --no-prompt flag.

let result = await Permissions.query({name:'fs-read'});
if (result.state == 'granted') {
 Deno.readFileSync("/etc/passwd");
} else if (result.state == 'prompt') {
  // Permission.request() ?
}

[bartlomieju]

I believe --no-prompt is useful. Would query() always display prompt? I'd rather my program die on PermissionDenied than hang on permission prompt.

[ry]

@bartlomieju I need to study the permission's API, but I believe Permission.query() just checks and Permission.request() prompts.

[kitsonk]

It sounds useful to allow code to determine how to interact if permissions aren't there, as in some permissions might be totally optional, and can be handled in code. I still think there might be a use case though where the user would just want to ensure no prompts are there, no matter what the code is. Like some container workloads, you absolutely don't want code to prompt and lock things, you just want the job to fail and exit.

[afinch7]

no-prompt is intended for when a user will not be present to answer permission prompts, so I don't really see a valid way to replace it.

[ry]

@afinch7 The idea is that no-prompt would be the default and that we would provide a way to programmatically prompt for raising permissions.

[nayeemrmn]

Ooh, whitelist scopes would finally be modelled and really well at that: https://w3c.github.io/permissions/#permissiondescriptor-stronger-than {name: "read", scope="/"} vs {name: "read", scope="/home/"}.

The Permissions object would still be accessed through Deno.permissions() (analogue of navigator.permissions but on an alternate PermissionName definition), right?

we would provide a way to programmatically prompt for raising permissions.

@ry Even if you provided an explicit request function, wouldn't you still want a flag to deny all of these? Maybe I'm misunderstanding.

[ry]

The Permissions object would still be accessed through Deno.permissions() (analogue of navigator.permissions() but on an alternate PermissionName definition), right?

We can just create a global Permissions as in the standard.

[nayeemrmn]

We can just create a global Permissions as in the standard.

@ry That's just the interface, isn't it? The actual permissions object is a singleton accessed through navigator.permissions.

[ry]

Oh, I didn't realize that.

Well maybe we should just add a navigator object then.

[nayeemrmn]

Well maybe we should just add a navigator object then.

Maybe at some point, but I think that should be reserved for strict browser compatibility.

Deno's Permissions would be using a non-standard PermissionName definition i.e. navigator.permissions.query({name: 'read'}) would reject in a browser, breaking the browser compatibility rule. Deno.permissions could comfortably swap out that definition entirely and exist as a sensible deviation in our own namespace.

[sholladay]

Having the ability to query and request permissions programmatically sounds like a cool feature, but I think I still need a way to suppress all prompts on the command line. I really don't want a prompt to cause my server to hang for all users just because I forgot to grant a permission ahead of time or the program wants to ask me and I'm not around.

I am also suspicious that this feature might be a subtle footgun. It reminds me of fs.exists() in Node, which was deprecated because using it to check for file existence before doing a read/write to that file can cause race condition issues. The solution is to try / catch a file operation and handle its non-existence if an error is thrown, rather than check ahead of time. It seems to me that Deno's permissions should work the same way. After all, conceptually it is very possible that a permission could be granted but then revoked in between the query and the attempted usage of that permission. So it's much better to just attempt to use a permission and handle its denial if an error is thrown.

[bogdanbiv]

Please consider saving permissions in a module specific JSON file.

WebExtensions contain an array of strings for this purpose (manifest.json):

"permissions": [
"webRequest", "webRequestBlocking", "https://example.com/*",
],

See https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Request_the_right_permissions for more details.

In this way, the permissions can be loaded / written with the application or module they are refering to. Which means that the app permissions could be versioned and transfered to another ENV.

Side note, somewhat related: What is missing from the manifest.json? To my knowledge,there is no crypto hash field for the module, not to speak of a PKI signature. This means WebExtensions are still(?) vulnerable to code injections. Subresource Integrity (Mozilla experiment) might be a relevant fix here: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity.

It would be nice to have the Opt-in opportunity to verify code has not changed from developer to user/s. Combine that with permissions and it would greatly reduce the attack surface for many apps.

Suggestion made after a short discussion with @bartlomieju, over IRC.

[kt3k]

Note:

• Permissions API: https://w3c.github.io/permissions/
• Permissions Request: https://wicg.github.io/permissions-request/
• Permissions Revoke: https://wicg.github.io/permissions-revoke/

W3C's Permission API spec only defines .query(), and the other 2 specs (from WICG) define .request() and .revoke().

Chrome implements the all 3 methods. Firefox implements only .query().

[bogdanbiv]

@sholladay It would be nice to have the modules as repeatable builds. My concern is that for the browsers there is no requirement to lift the permissions & bundle them together with the code they are relevant to. So the permissions query backend would be nice to be partitioned / split per their respective modules. How would DevOPS / continous deploy work?

[andyfleming]

Would we consider adding the inverse, --prompt if we make --no-prompt the default?

My fear is that if some projects have to instruct users to use a number of specific flags, they might get lazy and start suggesting --allow-all, which would default the whole purpose.

[kt3k]

@ry Thank you for merging #3183.

I'm planning the following next steps:

• [x] Replace the current JS APIs with permissions.query() and permissions.revoke() methods.
• [ ] Add permissions.request() and it prompts the user for the permissions.

[andyfleming]

I think permissions.request() should just fail unless a --prompt flag is present. I don't think we want to be in a situation where we're guessing if a prompt will ask for prompts.

If I'm running a script with the default settings (which is basically an implied non-interactive mode), I don't think a script should be able to override that.

[nayeemrmn]

@kt3k Would it be possible to make permissions.request() bring back permissions for free without a prompt if, for example, the permission was revoked in the same module? For #3055.

[ry]

@andyfleming It should fail if not connected to a TTY (using Deno.isTTY()). We should strive to reduce the number of flags, and --prompt seems quite unnecessary.

[ry]

@nayeemrmn No - if permissions were removed then it was perhaps to run some untrusted bit of code - we cannot then assume the state of the program is safe to have elevated permissions. Stupid example:

setTimeout(() => {
  Deno.remove("/etc/passwd");
}, 100000)

[jimmywarting]

Just skim-through the thread... i think the Permission api is a grate addition to Deno.

However when mixing it with the filesystem i can't help by feeling a bit conflicted about it when we got the WICG/native-file-system

[ry]

this was fixed in #3200