shell-pool / shpool

Think tmux, then aim... lower
Apache License 2.0
1.08k stars 14 forks source link

Add ability to rename session #57

Open mrzv opened 2 weeks ago

mrzv commented 2 weeks ago

I ran into a need to rename a (long-running) session. It would be great if there was a shpool rename old new command.

ethanpailes commented 1 week ago

I think there are a few different issues we need to address to do this well

What do we do if the target session name already exists

I think by default, we should exit with an error, but let people override the behavior with -f/--force to just clobber the old session or -s/--swap to make the two sessions swap names, retaining both.

What do we do about SHPOOL_SESSION_NAME

The SHPOOL_SESSION_NAME env var is used by the shpool detach and shpool kill subcommands, and is shown in the prompt (though we currently fully replace $SHPOOL_SESSION_NAME with the session name in the prompt prefix. To deal with this correctly, we probably need to change the way the prompt code works so it really is dynamically looking up the session name each time, and also somehow update the session name env var. There is no good way to do this that I'm aware of.

We could inject a export SHPOOL_SESSION_NAME=<new session name> command into the target session, but this will only work if the session is parked at a blank prompt. We can't be sure of this and we don't have any 100% effective way to check. We could also just tell the user they need to update the session name env var themselves, but that feels clunky.

One option that has been in the back of my mind for a while, is making an LD_PRELOAD shim that we can inject into child shells to let us change environment variables. I believe that as long as we call setenv from inside the same address, the environment variable will get updated. As long as we can somehow inject some code that runs on startup into a child process, we can have the code open up a unix domain socket, then listen for connections and pull setenv calls off the incoming connections. We can have a magic env var that holds a PID and this shim will only accept connections from that exact PID as a security measure to prevent other programs from being able to inject env vars into the target shell. This would potentially allow us to avoid the symlink trick we use for SSH_AUTH_SOCK as well. This will be pretty involved and probably deserves its own crate the same way that motd got its own crate. I doubt it is possible to accomplish this portably, so features that depend on it might be Linux specific once we get the Mac port working.

mrzv commented 1 week ago

These are complicated issues I hadn't thought of. I don't fully understand your last paragraph, but what if the mechanism to get the session name was changed? Something like running shpool name would return the name of the current session. Then instead of the SHPOOL_SESSION_NAME environment variable, one could get the name that way, and use it for whatever one wants (e.g., inside the prompt). Would something like that be simpler?

ethanpailes commented 1 week ago

In terms of my last paragraph, I just wrote a bit more about what I mean in the chook placeholder crate that I just made. For now, I don't have any code, but I think this should be possible through a combination of defining a custom _init and setting LD_PRELOAD.

I think shpool name is a pretty good idea. We could implement shpool name by adding a new RPC to the daemon where the caller presents a PID and gets the session name associated with that PID back.

There are two issues I see with the shpool name approach though:

  1. It would break backwards compatibility, which I'm a bit worried about since I think having some custom prompt code based on $SHPOOL_SESSION_NAME is probably one of the most common customizations.
  2. Assuming we change the prompt prefix to be shpool:$(shpool name), which I think we would need to if we want the prompt hooks to pick up name changes, we would be grabbing a lock on the global session table every time a new prompt is drawn. This would mean that two different side-by-side shells are contending with one another if they are both drawing new prompts. Even without the locking considerations (which we might be able to deal with via a lock free data structure of some sort), there would still be an RPC every time a new prompt is drawn.

Potentially we could do something like cache the name in a file, but it's not clear to me that that would be much better. It would still be IO on the same host.

I think I'm going to play with chook to see the degree of gross systems shenanigans that it requires to get done and then decide on the way to go based on that.

ethanpailes commented 4 days ago

I took a swing at chook, and I don't know that it can be reasonably implemented as a rust crate. The problem is that rust doesn't really want any code running before main, so you can't really use the stdlib. This is a pain for both the little RPC code I wrote and also for the user provided function. This was my first swing at it, but I never really got it working. Also, I didn't get to the point of trying to use it before deciding that this is probably untenable, but rust-ctor looks like a useful crate for this if I ever decide to pick it back up.

I'm now thinking that making a more focused inject-env crate that uses a hardcoded C file that can only tweak environment variables is probably a better way to go. It will mean doing some manual encoding rather than getting to just lean on serde, but it hopefully won't be too bad. Figuring out how to build a c .so with a custom _init routine will probably be the most annoying part.