search

shell-pool / shpool

Think tmux, then aim... lower
Apache License 2.0
1.17k stars 20 forks source link

`WARNING: terminal is not fully functional` when `motd.pager.bin = "less"` #129

Closed danielmirchandani closed 1 month ago

danielmirchandani commented 2 months ago

What happened After tapping my security key, I see "WARNING: terminal is not fully functional" and "Press RETURN to continue". Pressing any key then displays the MotD, but without any terminal colors.

What I expected to happen After tapping my security key, I expect to see the MotD (in color) next.

To Reproduce Steps to reproduce the behavior:

  1. ChromeOS Version 127.0.6533.114 (Official Build) (64-bit)
  2. ".config/shpool/config.toml" is [motd.pager] bin = "less" (config file does have a new-line, but removed here for formatting)
  3. Secure Shell Version 0.67 (from https://chromewebstore.google.com/detail/secure-shell/iodihamcpbpeioajjeobimgagajmlibd)
  4. SSH Arguments -t shpool attach -f login
  5. Connect

Version info shpool 0.6.3 (cl/665149055)

Logs shpool.log

danielmirchandani commented 2 months ago

I think this has to do with $TERM. I have two SSH connections that end up starting less. In less, !echo TERM=$TERM (run a shell command and show output separately while less is still running), shows:

  1. Without shpool (-t less /any/file), "TERM=xterm-256color"
  2. With shpool (-t shpool attach -f login ), "TERM=dumb"

less appears to use $TERM to determine terminal capabilities (probably a bug in less), but where is the value "dumb" coming from?

ethanpailes commented 2 months ago

Looking at the log you attached I see

2024-08-22T15:25:22.181651Z  INFO ThreadId(46) handle_conn{cid=6}:handle_attach:spawn_subshell:inject_env: new
2024-08-22T15:25:22.181667Z  INFO ThreadId(46) handle_conn{cid=6}:handle_attach:spawn_subshell:inject_env: injecting TERM into shell Some("xterm-256color")

so it seems like shpool should be injecting the right TERM value when it spawns the shell, but I think we don't properly inject the term value into the pager process for displaying the motd. All we do to set up the pager process is this, so hopefully we can fix this bug by threading the same environment we use to setup the shell down to the pager process.

Thanks for trying this feature out an finding this bug!