denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.97k stars 5.35k forks source link

Include "Deny permissions" for Deno Worker #20096

Open Jimdooz opened 1 year ago

Jimdooz commented 1 year ago

It would be beneficial to include a "Deny Permission" option for Deno Workers, extending the current permission controls and enhancing security.

Reflecting on the conversations within issues #18804 and #4867, it's apparent that more nuanced handling of permissions has been a topic of interest. Perhaps the existing WorkerOptions could be extended to include a deny field, for example :

export interface WorkerOptions {
  type?: "classic" | "module";
  name?: string;
  deno?: {
    includeNamespace?: boolean;
    permissions?: {
      read?: boolean | string[] | "inherit";
      //...
      deny?: {
        read?: boolean | string[] | "inherit";
        //...
      } | "inherit";
    } | "inherit";
  };
}
benatkin commented 1 year ago

This seems like something that should be implemented, but it quite as needed as the CLI option was, because when creating a worker you're doing it from code, and the code can construct a list of permissions to allow. In some instances this will be sufficient, and in some instances not. For instance if you had a worker search the code in the current directory for something and send the results to a third party, but needed to ignore files starting with .env, you could pass it a list of all files except .env, both to the worker process and to the deno.env option (because it wouldn't be able to get a list of all the files without access to the directory).

bartlomieju commented 1 year ago

I would suggest to go with denyRead, denyWrite, etc.

It's actually a bug because we missed it in https://github.com/denoland/deno/pull/19070

Jimdooz commented 1 year ago

How about replacing read, write, etc., with allowRead, allowWrite, and so on? Since the permission system for Workers is still an unstable feature, making these changes now might help prevent any confusion between "deny" and "allow" permissions in the future. Or maybe it's too verbose, just a thought.

OoDeLally commented 1 year ago

I would love that too. My goal is to run unsafe code as worker (a bit like what jailed tries to do), and I need to deny everything from it. In addition, it seems semantically strange that allow-read=false means "ask during runtime". Imho allow-read=false should mean "deny every read", while allow-read=prompt could be the runtime prompt (current default behavior).

vfssantos commented 2 months ago

Yeah, I'm looking forward for this too. Regarding how allow-read=false and others.. What about using the exact same properties and logic as in the CLI? That way developers would not need to keep guessing about the behavior in different environments.

alexgleason commented 1 month ago

Shouldn't setting the permission to false deny the permission? However, it still prompts me.