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

Executing incomplete commands is too dangerous #8

Closed
Keith-S-Thompson opened this issue Oct 24, 2018 · 14 comments
Closed

Executing incomplete commands is too dangerous #8

Keith-S-Thompson opened this issue Oct 24, 2018 · 14 comments
Labels
enhancement New feature or request favourite What I'd like to focus on first
Milestone

Comments

@Keith-S-Thompson
Copy link

This looks like a very useful tool, but I won't use it on my system as long as it automatically executes incomplete commands as they're being typed.

It shouldn't execute a command until it's explicitly told to do so. Hitting the TAB key might be one reasonable way to do that.

@aalvarado
Copy link

whitelisting commands should be the default IMHO

@dreadnaut
Copy link

dreadnaut commented Oct 24, 2018

White/black lists are a possible solution. Alternatively, consider running the command only after the user has typed a space (a word has been completed), or wait until the next | or return. The full-realtime behaviour could still be available behind an --unsafe option.

@gregwebs
Copy link

gregwebs commented Oct 24, 2018

How about not executing the command until the user types a space? If you add a shell parser you could recognize quoting and not execute the command until the quote is closed (to avoid executing with quoted arguments with space). Its also possible to try to do this only for program names (not their arguments) and allow incomplete execution with arguments (again, a shell parser is useful to recognize pipelines). Otherwise there is a small downside of requiring a space after the last argument, but I think that is could be easy for users to learn.

I find the current behavior of running incomplete commands to be dis-orienting. At first I had the text I wanted, but now I have to look at this while typing:

 | g                                                                                                                                                                                                               
bash: g: command not found                                                                                                                                                                                         
up: exit status 127

up could instead offer auto-complete while typing a program name.

@rubenbe
Copy link

rubenbe commented Oct 24, 2018

Someone on news.ycombinator.com suggested to use Linux namespaces to make the filesystem read-only by default.

IMHO his is a cleaner solution although it's Linux only.
Maybe other operating systems contain a similar features.

https://news.ycombinator.com/item?id=18293931
https://gist.github.com/cocagne/4088467

@Keith-S-Thompson
Copy link
Author

In my opinion, neither whitelisting nor blacklisting would be a good solution (by which I mean, not good enough to induce me to use the tool or recommend it to anyone else).

Either whitelisting or blacklisting means I have to maintain a list of commands that are considered "safe" or "dangerous". No such list can be complete and accurate, and I'll very likely want to be able to execute (carefully) a command that's considered "unsafe".

(I just ran a script and found several thousand cases of a command in my $PATH whose name is a prefix of another command in my $PATH. My $PATH is admittedly rather cluttered.)

It happens that I don't have commands in my $PATH named g,gr, or gre -- but if I did, they would be executed every time I type grep into this tool.

rubenbe suggested making the file system read-only. Not all dangerous or undesirable actions necessarily involve modifying the file system -- and I might very well want to execute a command that modifies the file system.

The only solution that would let me use this tool, or recommend it to anyone else, is to require explicit user action to execute a command. Automatically executing commands as I type them is a dealbreaker.

I'm not sure that TAB is the best way to request executing a command. Tab completion would be a useful feature. Typing ENTER doesn't currently see to do anything; perhaps ENTER could cause the current command to be executed while leaving the command in place to permit more typing.

@Aralin
Copy link

Aralin commented Oct 24, 2018

rm -rf /home/$USER/tmp/*

@akavel
Copy link
Owner

akavel commented Oct 24, 2018

For the record: based on ideas from good people on lobste.rs, HN, here, reddit, etc, my current quick thoughts are like this:

  1. First of all, probably try to make it non-immediate by default, requiring to press Ctrl-Enter to execute (I think Ctrl-Enter is a bit safer, as Enter might be too easy to conflate with editors' behavior; I often press Enter accidentally in various chats). That is, if Ctrl-Enter can be detected by a tui app; if not, I'll go with Enter.

  2. Still allow for switching to a "no seatbelts rapid iteration" mode, probably with some keyboard shortcut, maybe matching a command-line parameter to the same effect (e.g.: --unsafe|-u, and shortcut Ctrl-, u? must think some more about this).

  3. I'd love to try adding some feature to enable some kind of "readonly" mode, where writing to filesystem, and maybe network access, would be disallowed (or just any chosen syscalls). Not yet sure if and how it's possible; various people mentioned keywords like:

    • "man capabilities", → from reading the manpage, IIUC capabilities on Linux seem to only be able to grant some (partial) root permissions, not to drop some regular user permissions (like writing files). :/
    • "namespaces",
    • "unshare [syscall?]",
    • "syscall interception/disabling" (?), — it's apparently possible, but I believe it would only affect the top-level bash, not all the child processes in the actual pipeline... :/
    • LD_PRELOAD (this one seems too limited AFAIU),
    • firejail
    • "drop user permissions" — though I don't understand yet what this actually means (and whether it would then allow actually executing any commands from PATH...? also, potentially reading some files, including dotfiles/rc files?)
    • overlayFS

    Those sound super interesting, though I'm not yet sure which can bring the best effect with the least work and complexity. This would be most probably be Linux-only for starters, assuming I manage to do anything like this at all.

  4. @gregwebs As to the "trailing space" idea, I thought of a similar one since quite long ago already, so I'm very happy to discover that I'm not the only one to think of it! :) I have some liking to it, though I now seem to like the dumb and brutal simplicity of the "Ctrl-Enter" approach more.

I'll try to see if anyone brings some more ideas somewhere, and to think about it some more.

Please note I can't promise how fast I'll implement anything in this area. I'm not sure to what extent I'll be excited or tired with the project once the "release adrenaline" weathers off. Thanks a lot for your patience! I totally super appreciate all your ideas, thoughts, and contributions! They already opened my horizons to some ideas I wasn't aware of, as well as helping me see the main concerns you're having with the tool, as well as the things you seem to like about it! :)

@gregwebs As to the disorienting behavior, I'm trying to ponder various options for retaining the output for longer time, esp. over failed commands. See especially #4 for some interesting idea that didn't occur to me earlier at all! Also your idea about auto-completion is not something I thought of earlier either, I believe; noted in my notebook, for sure, thanks! Though I'm generally kinda on the fence to what extent the up's editor should be developed... I like to fancy the idea of some kind of integration with general purpose editors, like vim/emacs/VSCode/... and keeping up only as a kind of a hidden service for them...

Thanks a lot again! There will be a lot of thinking to process for me after the dust settles...

@fiws
Copy link

fiws commented Oct 24, 2018

I found this related to unshare with go:

maybe it helps

@akavel akavel added this to the v0.2 milestone Oct 24, 2018
@akavel akavel added bug Something isn't working enhancement New feature or request favourite What I'd like to focus on first and removed bug Something isn't working labels Oct 24, 2018
@akavel
Copy link
Owner

akavel commented Oct 24, 2018

Ehhh, Ctrl-Enter won't work, because on Linux Enter is already Ctrl-M..... grrrrrrr >:-|

@kpcyrd
Copy link

kpcyrd commented Oct 25, 2018

Since this hasn't been mentioned before: seccomp

You can disable certain syscalls by loading a seccomp filter that denies any call to unlink(2), unlinkat(2) or the various open calls that have O_RDWR|O_WRONLY|O_CREAT set. This filter also applies to grand-children, grand-grand-children etc.

If done correctly you can ensure that no child process can alter the system, but setting up this filter correctly requires some work and only works on linux.

This approach would work better than namespaces though since you either need to be root to setup a namespace or a kernel that has user namespaces enabled, which most distros don't do due to security reasons.

@Keith-S-Thompson
Copy link
Author

Some general thoughts.

This tool is not exactly a shell, but it's very similar to one. In particular, it's a tool that can be used interactively to execute arbitrary commands on behalf of the user who's running it.

Based on that, I suggest that:

  • It should not execute any command that the user has not asked it to.
  • It should execute any command that the user has (explicitly) asked it to.

In my opinion, playing OS-level tricks to intercept system calls by child processes should be considered outside its scope. Shells don't have or need blacklists or whitelists; I don't think up should have them either. If I type a command that clobbers something, it's my responsibility (and sometimes I want to clobber something).

Tentatively executing commands as they're typed (e.g., attempting to run g, gr, and gre as I type grep) is an interesting and clever idea, but I don't think it can be implemented in an reasonable manner. (Personally, I'd prefer not even to have an option to do that; I'd be nervous about invoking it accidentally.)

I'd like to see this tool simply execute commands and show their output when I type Enter, and not before. That's how shells behave. The "special sauce" is that after I've typed Enter, I can continue typing and see the result of changing or extending the command. And it's in a curses-like window, so if the output is 1000 lines long it doesn't clobber my scrollback. Let it do that one thing well.

@j3k0
Copy link

j3k0 commented Oct 25, 2018

Just pressing Enter seems perfect. A non-standard keyboard shortcut would certainly be confusing. In a command-line environment, users will just expect Enter to execute what they typed. Anything else will probably lead to frustration... (like your "first-time-in-vi" moment of solitude)

Note: Enabling auto-completion with Tab would be awesome too.

@rsynnest
Copy link

rsynnest commented Oct 25, 2018

perhaps ENTER could cause the current command to be executed
Enabling auto-completion with Tab would be awesome too.

I wrote ishell which will pre-populate your previous command so you never have to press that pesky up arrow ever again! ishell can run any bash command, comes with submit-on-Enter functionality, tab-completion, and a built in scrolling pager! Note: ishell does not accept piped input, so you will need to cat your file manually from within ishell (ugh!).

ishell source code is available at #8 (comment):

#!/bin/bash
# ishell is hell
if [ -t 0 ]
then
  line=''
  while :
  do
    read -p "ishell>" -i "$line" -e line
    eval "$line" | less -RSFX
  done
fi
exit 0

All kidding aside, up is a very cool idea and I agree that it is too unsafe for me personally to use. I agree with Keith that "Enter" to run commands makes the most sense to me. I think auto-execution is mainly about saving time for the user, and I can see why @akavel wants to keep auto-execution, because it is a very slick and quick feature, but I think tab-completion beats auto-execution as far as time savers. Another useful time saver would be an "undo" hotkey to go back through past commands (or support for fzf history as mentioned in Future Ideas). I think all of these ideas will make auto-execution less necessary for saving time. In fact, auto-execution makes a history/undo feature less useful because you would need to scroll back through every letter typed. At the end of the day the biggest benefit of up is the interactivity (keeping the scrollable output and last command on screen after each execution), and I think replacing auto-execute with explicit execute doesn't mitigate that main benefit of the tool (and really, how long does it take to press Enter?).

@akavel
Copy link
Owner

akavel commented Oct 25, 2018

Fixed in v0.2 / v0.2.1, by changing the default behavior to "Press Enter to run".

For sketching potential alternative approaches for future, I added a wiki page titled: idea: limiting permissions. I made the wiki open for editing & collaboration, if you like to contribute some ideas. Please keep it terse so it won't get overworded (I reserve myself the right to cleanup and "censor" the wiki). Please note I am not sure if I will implement any of those ideas, but I am not sure that I won't either. They are certainly interesting, inspiring, and intelectually stimulating! And the tool would not exist without a lot of inspiration sources!

Thanks to everybody for a lot of ideas and a great discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request favourite What I'd like to focus on first
Projects
None yet
Development

No branches or pull requests