Proposal: add permissions to config file

[kitsonk]

Context

Currently Deno supports a deno.jsonc configuration file which allow users to provide a configuration file that can provide TypeScript compiler options, lint options and format options.

It does not currently support other information that can only be expressed on the command line, while this proposal is what

Semantics

• When a configuration file is applied and the "permissions" section is parsed and the permissions are applied from the "permissions" section. Any other flags on the command line are ignored.
• If there are flags on the command line and a config file is being applied, and the config file contains "permissions" a warning should be issued to stderr that permissions from the config file are being applied.
• Remote configuration files are supported, but a summary of the permissions is prompted before hand and requires user configuration to continue.
• Keys names match the flags on the command line, and values accept a true or false. If the key supports a value, these can be either a string delimited the same way on the command line, or an array of strings.
• In situations where a base path is needed for relative paths (for --allow-read and --allow-write) the config file is used as a base, versus the cwd.

Examples

An example of a configuration file using permissions:

{
  "permissions": {
    "allow-all": true,
    "allow-env": true,
    "allow-hrtime": false,
    "allow-read": [ ".", "/tmp" ],
    "allow-write": ".",
    "allow-net": "deno.land,nest.land"
  }
}

Considerations / Open Questions

• Having explicit a section of "permissions" makes it easier to understand explicitly that these effect the runtime permissions. It also allows the definition of the semantics of "permissions" to evolve independently of the rest of the configuration file, as well as opens an easy opportunity to be able to set permissions on other things in the future, like tasks/scripts independent of the top level permissions.
• Having an explicit allow in the keys provides a mechanism to introduce block in the future (for example "block-net": "deno.land") giving more granular permissions.
• If a remote configuration file is used, and there is no TTY or --no-prompt/--quit is supplied on the configuration is set, should the process just terminate or just allow the program to run with the supplied permissions?
• For remote config files, using the remote URL as a relative base for --allow-read --allow-write is not possible. Does this mean that relative paths just error, or that it defaults to the cwd in those situations?

cc/ @bartlomieju @ry

[crowlKats]

In my opinion allow-all should just be

{
 "permissions": true,
}

* Having an explicit `allow` in the keys provides a mechanism to introduce `block` in the future (for example `"block-net": "deno.land"`) giving more granular permissions.

not a fan of this. it would mean you'd be able to do something with the config file that is impossible to do with just flags.

Also I don't think it makes sense to support comma separated items in a string, only arrays should be usable (for the perms that take multiple values)

[bartlomieju]

I'm in favor of the proposal in general (especially combined with tasks/scripts), but I'm not in favor of top-level permissions key. IMO there's not much benefit of having a single top-level definition if most projects contain several entrypoint, each of them requiring different permissions. With this proposal it becomes non-obvious how permissions would be applied to different entry-points.

As for the signature of permissions object I believe we should follow definition used in TestDefinition and WorkerOptions as closely as possible:

https://github.com/denoland/deno/blob/dd91ecef502456ba39495d8e178f8101a87c0e34/cli/dts/lib.deno.ns.d.ts#L144-L275

[kitsonk]

IMO there's not much benefit of having a single top-level definition if most projects contain several entrypoint, each of them requiring different permissions.

But there are currently no semantics in the config file for entry points, so it becomes a chicken and egg.

And specifically the example of the "test" permissions seems to apply a top level "permissions" key that would be the default set of permissions to be applied. Given the current lack of a way to express entry points in the config file, it only seems logical to describe the default set of permissions, and then allow addition/different sets of permissions to be described on individual entry points when they become available.

My key point was that we shouldn't flatten permissions to be different top level keys.

[kitsonk]

not a fan of this. it would mean you'd be able to do something with the config file that is impossible to do with just flags.

That is already the case with TypeScript compiler options.

[kitsonk]

As for the signature of permissions object I believe we should follow definition used in TestDefinition and WorkerOptions as closely as possible:

I am agreeable with that, and it makes sense in the context of "top level" permissions and allowing overriding for different tasks/entry points.

[bartlomieju]

IMO there's not much benefit of having a single top-level definition if most projects contain several entrypoint, each of them requiring different permissions.

But there are currently no semantics in the config file for entry points, so it becomes a chicken and egg.

And specifically the example of the "test" permissions seems to apply a top level "permissions" key that would be the default set of permissions to be applied. Given the current lack of a way to express entry points in the config file, it only seems logical to describe the default set of permissions, and then allow addition/different sets of permissions to be described on individual entry points when they become available.

My key point was that we shouldn't flatten permissions to be different top level keys.

Okay, this is a valid point. You're also right about chicken and egg problem. I guess this is a good way to start iterating on these features. Let's do it 👍

[crowlKats]

not a fan of this. it would mean you'd be able to do something with the config file that is impossible to do with just flags.

That is already the case with TypeScript compiler options.

yes, but those are "external", as in, they arent something made by/for deno exclusively. the only other option for thsoe would implement all options as flags, which would be unrealistic. Also this would mean we have some permission related features as flags, but other features only in config file, making things just more confusing imo by having things "all over the place" (i am aware it isnt as bad as i just made it sound, but my point stands).

[Soremwar]

I'm in favor of the proposal in general (especially combined with tasks/scripts), but I'm not in favor of top-level permissions key. IMO there's not much benefit of having a single top-level definition if most projects contain several entrypoint, each of them requiring different permissions. With this proposal it becomes non-obvious how permissions would be applied to different entry-points.

This could be tackled by allowing composition of config file, an extends of sorts

[crowlKats]

{
  "permissions": {
    "allow-all": true,
    "allow-env": true,
    "allow-hrtime": false,
    "allow-read": [ ".", "/tmp" ],
    "allow-write": ".",
    "allow-net": "deno.land,nest.land"
  }
}

Thought about the blocklist thing again: how about instead

{
  "permissions": {
    "env": {
      "block": ["PATH"],
      "allow": ["HOME"]
    },
    "hrtime": false,
    "read": [".", "/tmp"],
    "write": ["."],
    "net": ["deno.land", "nest.land"],
    "prompt": true
  }
}

so to specify the blocklist, its an object instead of a proper value for the perm directly. this would all us to add blocklist support at some later point as it would be an extension from what it would be without. also this would allow for allowing specifying --prompt behaviour on a permission level instead of just a global level

[lilnasy]

I really like this proposal. It encourages fine-grained control over what directories can be read and written to, for example. At the moment, it's inconvenient to get this precise with command line options.

[jsejcksn]

I really like this proposal. It encourages fine-grained control over what directories can be read and written to, for example. At the moment, it's inconvenient to get this precise with command line options.

@crowlKats When you said

the only other option for thsoe would implement all options as flags, which would be unrealistic

by "unrealistic" did you mean "inconvenient"? My impression is that most software which executes with more than a few flags either builds them using a script or hardcodes them into a script.

Adding additional CLI flags from options actually sounds useful (as no extra filesystem i/o in the form of reading/writing a config would need to happen in dynamic execution).

[jrylan]

EDIT: I was initially in favor of this proposal as-is, but now after thinking it over the only thing I desire is a flag to opt-out of the functionality to load permissions from the config. I just think about a scenario of updating to a new version of some popular third-party module that's been hacked and having said module write to deno.json to provide new permissions for itself that could be utilized the next time the script is ran.

For the strictest security, I would want to be able to explicitly opt-out of reading permissions from the config and instead require all the permissions be provided via flags... that's what I would always want to run in production.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[jsejcksn]

For the strictest security, I would want to be able to explicitly opt-out of reading permissions from the config and instead require all the permissions be provided via flags... that's what I would always want to run in production

Related: https://github.com/denoland/deno/issues/13452 will become even more important if this feature is implemented

[sno2]

Now that we have permissions scoped to tests, it would probably be best to just unify a deno.jsonc config property permissions with Deno.test's permission object type. Also, when it is string | URL we can just only accept string. I think it would be a bad idea to have two different type of objects for specifying permissions and this would give us a chance to have a uniform behavior.

Edit: I hid this because I did not realize that this was already mentioned above.

[ry]

I'm in favor too. Added to 1.22 milestone.

[bartlomieju]

I'm working on this in https://github.com/denoland/deno/issues/12763.

For the first pass I think we should cut the scope a little bit, namely by not supporting permissions in remote configuration file. Instead we should print a warning, pointing to an issue about it. The reason is that it will cause a bifurcation in behavior depending if the config file is local or remote, in the former case we'll be resolving allowlists relative to config file, while in the latter we'd have to fallback to CWD. I think this is gonna be a surprising behavior.

[jsejcksn]

I'm working on this in #12763.

@bartlomieju You linked to this issue 🪞. Did you mean #14520?

[bartlomieju]

Yes

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[GuillaumeAmat]

:eyes:

[happy-little-one]

there is a short way to achieve this, run this in window terminal:

alias d="deno run --allow-net --allow-env --allow-read --unstable" $*

and you then d foo.ts

[pkoch]

On #14520, @bartlomieju said:

There's no consensus about this feature in the core team. Closing for now.

Is now maybe an ok time to retry corralling consensus?

[cawa-93]

I would like to be able to set default permissions for each command individually. Example:

{
    // Global permissions
    permissions: {
        'allow-read': ['./config.json']
    },
    // Additional permissions for any `deno test` command
    test: {
        permissions: {
            'allow-read': ['.'],
            'allow-write': ['tests/fixtures'],
            'allow-env':true,
        }
    }
}

[jsejcksn]

Isn't this kind of functionality practically already in place, provided by the task runner (deno task) and configurable through task definitions? IIRC, the task runner was not yet implemented at the time this issue was created.

[bartlomieju]

Isn't this kind of functionality practically already in place, provided by the task runner (deno task) and configurable through task definitions? IIRC, the task runner was not yet implemented at the time this issue was created.

Yes, you can specify the permissions via tasks definition. The core team's hesitation is coming from the fact that if you specify permissions in the config file, adding the "write" permission allows to then update the config file by rogue dependency that would be able to escalate permissions further. We're currently focusing our efforts on other parts of the runtime, but we might revisit this issue in Q1 2023.

[lilnasy]

Isn't that just as much of a concern right now with the task runner? Task aliases are also vulnerable to being edited by a rogue dependency.

[sigmaSd]

Here is another proposal https://github.com/denoland/deno/issues/17177

It has in my opinion some advantages:

• It allows specifying complex permissions, permissions that requires internal app information, and it allows expressing them in a crossplatfrom way easily using deno ecosystem
• Unlike deno.json that needs to live besides the app entry point, permissions.ts can live anywhere , so malicious actors can not rewrite it if its in remote place for example, or somewhere with no write permission

[pkoch]

Maybe the maintainers can define a starting point for the threat model? I feel we're going back-and-forth in a piecemeal fashion. I'd love to have full lay of the land to be able to propose something that's able to be well received.

[oscarotero]

My 2 cents:

• I think permissions should live inside the deno.json
• Maybe there should be a special writing permission applied to the config file used currently. Something like Deno requests write access to the config file "deno.json". It would be independent of the --allow-write flag and be configured with --allow-write-config flag or no flag at all and only using the prompt.
• For cross platform paths, I think it could be managed by Deno in the same way of Deno task has cross platform commands like cp, mkdir, etc. Maybe using an $ or somethink like that. For example: --allow-write=$HOME for the user's home directory.