Fuseteam / sshmount

bash script for reading config for mounting Remote directories locally over ssh
1 stars 2 forks source link

Some requests #2

Open r2evans opened 3 months ago

r2evans commented 3 months ago

Thanks for the script, I'm giving it a try, predominantly for the easy use of port-forwarding. (I found reference to it at https://unix.stackexchange.com/a/666390/176511.)

I had a few thoughts while starting it: mostly for discussion now, it's easy enough to break these into individual issues if you find them worthwhile. (And #3 is an easy PR.)

  1. Do not (either by option or at all) rmdir the mount-point. In my use-cases, having the base-directory persist when not mounted is important.

  2. Add DynamicForward to the list of supported port-forward rules. This worked well enough by adding simply

    DFwd=$(fetchHostAttribute "$Host" "---" "DynamicForward" "-D");
    [[ -n "$DFwd" && "$DFwd" != "2" ]] && { sshopts="$sshopts$DFwd"; }

    though (minor) it added a colon after the port. For instance, in fsconfig if I have DynamicForward 127.0.0.1:5000, then the command line includes -D 127.0.0.1:5000:. It still works, but ... I didn't investigate why that's happening (likely parsing of an expected pattern).

  3. (Bug.) sshmount -u remote unmounts but the ssh command is still running. I would expect the script to kill the ssh that it starts. I suspect this may be related to passing a ssh_command to sshfs, but I don't recall running into problems on that end before ...

  4. (Minor bug when my muscle-memory errs.) I'm accustomed to doing fusermount -u /mountpoint, but doing that results in

    $ sshmount -u /mountpoint
    sed: -e expression #1, char 14: expected newer version of sed
    sed: -e expression #1, char 14: expected newer version of sed
    sed: -e expression #1, char 14: expected newer version of sed
    2 is not mounted

    I don't think it'd be necessary to auto-detect a LocalDirectory as an argument and self-fix the mistake, but perhaps the script can better guard against this kind of "injection" in sed -n "/$1$/,/^$2/p or such. I suspect without verification that escaping or removing / in the arguments would suffice to squash the error, though it'll still likely not find something.

  5. Big ask: is it possible to retrieve all LocalForward from the actual .ssh/config file? Perhaps that's a bridge too far ...

    I'm not suggesting you remove support within .ssh/fsconfig, but there are (for me) clear use-cases where I'm not sshfs-mounting but will need the port-forwarding for other purposes. While it's not herculean to maintain both, it does introduce an opportunity to have two different forwarding rulesets. This does not require reading/parsing the .ssh/config file, most versions of ssh support the -G option, so that ssh -G remote | grep -E "(local|remote|dynamic)forward" can be used, perhaps directly. Perhaps this would be an option in .ssh/fsconfig such as PortForwardingFromSshConfig yes.

Thanks for the script! I'll be playing with it for a bit.

Fuseteam commented 1 month ago

Hi thanks for the comprehensive post, i'm open to PRs for any of these. feel free to send them in as PRs

I will also look into the bugs on my end