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

Ergonomics: Prompt for permission. #1008

Closed
ry opened this issue Oct 17, 2018 · 12 comments
Closed

Ergonomics: Prompt for permission. #1008

ry opened this issue Oct 17, 2018 · 12 comments

Comments

@ry
Copy link
Member

ry commented Oct 17, 2018

When I run a program that requires elevated permissions, without any flags, I should be prompted on the command-line to elevate privlages.

This was described in the roadmap:

Program requests write access to "~/.ssh/id_rsa". Grant? [yNs]
http://gist.github.com/asdfasd.js requests network access to "www.facebook.com". Grant? [yNs]
Program requests access to environment variables. Grant? [yNs]
Program requests to spawn `rm -rf /`. Grant? [yNs]

The "s" option is meant to display a stack trace - so the user can get more information. This doesn't need to be done in the first version.

(cc @hayd - this might be up your alley since you're working on REPL input.)

Let's discuss a strategy before implementation.

@ry ry changed the title Ergnomics: Prompt for permission. Ergonomics: Prompt for permission. Oct 17, 2018
@hayd
Copy link
Contributor

hayd commented Oct 17, 2018

The repl branch exposes a readline(prompt) method in ts. However a permission prompt must be in rust only, and all it's doing is reading a single char from stdin rust. (So I don't think it would reuse any code.)

Would this be per use rather than mutate flags?

I think we ought to do away with the:

if !state.flags.allow_net

pattern. Perhaps by not having.allow_net etc. be pub, instead require a method with an appropriate prompt (if temporary additional permission is required):

# better name necessary
if state.flags.allow_net_or_prompt("www.facebook.com")

Or perhaps it could be a Result rather than a bool?

@ry
Copy link
Member Author

ry commented Oct 17, 2018

The definitely needs to be done in purely Rust, as it's privileged. Maybe something like this

permission_check_net(domain_name)?;

Where the signature would be

fn permission_check_net(domain_name: &str) -> Result<()>
fn permission_check_write(filename: &str) -> Result<()>
fn permission_check_env() -> Result<()>

@hayd
Copy link
Contributor

hayd commented Oct 18, 2018

I tried some towards this and ran into two issues:

  • Reading a single char from stdin is line buffered... (required input y<ENTER>)
  • Should permission prompt work if not in an interactive session (should check for isatty)?

(Sadly the Box<Op> doesn't accept Result's ?...)

@ry
Copy link
Member Author

ry commented Oct 18, 2018

required input y+ENTER

that's fine

Should permission prompt work if not in an interactive session (should check for isatty)?

Yes - if !isatty, throw.

Box<Op> does accept results. Maybe you want Box::new(futures::future::result(r)) or maybe

Box::new(futures::future::lazy(|| {
  permission_check_net(domain_name)?;
  Ok(empty_buf())
})

@hayd
Copy link
Contributor

hayd commented Oct 18, 2018

But I wanted to use ?! Okay, I'll try using isatty.

@hayd
Copy link
Contributor

hayd commented Oct 19, 2018

How to determine whether it's 'Program' vs 'http://gist.github.com/asdfasd.js' in ops?

@ry
Copy link
Member Author

ry commented Oct 19, 2018

I don’t understand the question. Can you elaborate?

@kitsonk
Copy link
Contributor

kitsonk commented Oct 19, 2018

I think the question is "how to know if it is the permission of the running program, or an externally loaded module"? I think the question is not germane to this topic, as the requirement I believe is:

  • When Deno is running with unspecified permissions, at any point a permission is needed, the privileged side should request that permission. This is independent of where the userland code came from.
  • If permissions are specified at startup, then a missing privilege is a fatal error. This is also independent of where the userland code came from.

@hayd
Copy link
Contributor

hayd commented Oct 19, 2018

In the roadmap (@ry's top comment):

Program requests write access to "~/.ssh/id_rsa". Grant? [yNs]
http://gist.github.com/asdfasd.js requests network access to "www.facebook.com". Grant? [yNs]
Program requests access to environment variables. Grant? [yNs]
Program requests to spawn `rm -rf /`. Grant? [yNs]

Either it's 'Program' or the external url (or even I think internal file reference would be useful).
IIUC this part is also related/required to the s (stacktrace).

Edit: aside from this part it's working.

@chrisdothtml
Copy link

How to determine whether it's 'Program'...

@hayd this doesn't really matter, because it should request for all permissions needed to run the entire compilation (including any imported scripts).

One other thought that came to mind on the topic of requesting for permission is whether it should remember your choice for a given version/compilation. If someone were to build a cli with deno, it would be annoying to have to explicitly give it permission every time you used it

@hayd
Copy link
Contributor

hayd commented Dec 19, 2018

I think you want to do permission based on a dot/config toml file, either explicitly or by location convention. (Though perhaps there's security concerns if it's a deno writeable file...)

Previously I'd imagined there ought to be a permission tree ("which modules can do what"), but that's probably far too complicated and infeasible. The extension is e.g. for net to add any/domain/url as prompt/cli options.

@bartlomieju
Copy link
Member

Done in #1081.

@ry

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

5 participants