Skip to content

RemoteInfo Rpc #1719

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.

Sign up for GitHub

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

Merged
oneirocosm merged 6 commits into from
Jan 12, 2025
Merged

RemoteInfo Rpc #1719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
112780a
feat: add RemoteGetInfoCommand
oneirocosm Jan 12, 2025
7a67309
fix: update generated files
oneirocosm Jan 12, 2025
3078c2a
fix: remove home dir check
oneirocosm Jan 12, 2025
ec1d2b0
fix: use correct mac shell in GetInfo
oneirocosm Jan 12, 2025
c964a60
feat: use rpc to detect shell
oneirocosm Jan 12, 2025
a36b5d4
fix: remove shell line endings for remoteinfo
oneirocosm Jan 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/wsh/cmd/wshcmd-connserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func serverRunRouter() error {
}

func checkForUpdate() error {
remoteInfo := wshutil.GetInfo(RpcContext)
remoteInfo := wshutil.GetInfo()
needsRestartRaw, err := RpcClient.SendRpcRequest(wshrpc.Command_ConnUpdateWsh, remoteInfo, &wshrpc.RpcOpts{Timeout: 60000})
if err != nil {
return fmt.Errorf("could not update: %w", err)
Comment on lines +193 to 196
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Handle Potential Errors Gracefully

After modifying the GetInfo function call, ensure that any potential errors are handled appropriately, and provide informative error messages to aid in troubleshooting.

Consider checking for errors and updating the error handling logic if necessary.

Expand Down
5 changes: 5 additions & 0 deletions frontend/app/store/wshclientapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ class RpcApiType {
return client.wshRpcCall("remotefiletouch", data, opts);
}

// command "remotegetinfo" [call]
RemoteGetInfoCommand(client: WshClient, opts?: RpcOpts): Promise<RemoteInfo> {
return client.wshRpcCall("remotegetinfo", null, opts);
}

// command "remotemkdir" [call]
RemoteMkdirCommand(client: WshClient, data: string, opts?: RpcOpts): Promise<void> {
return client.wshRpcCall("remotemkdir", data, opts);
Expand Down
1 change: 0 additions & 1 deletion frontend/types/gotypes.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,6 @@ declare global {

// wshrpc.RemoteInfo
type RemoteInfo = {
host: string;
clientarch: string;
clientos: string;
clientversion: string;
Expand Down
2 changes: 1 addition & 1 deletion pkg/remote/conncontroller/conncontroller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ Would you like to install them?

func (conn *SSHConn) UpdateWsh(ctx context.Context, clientDisplayName string, remoteInfo *wshrpc.RemoteInfo) error {
conn.Infof(ctx, "attempting to update wsh for connection %s (os:%s arch:%s version:%s)\n",
remoteInfo.ConnName, remoteInfo.ClientOs, remoteInfo.ClientArch, remoteInfo.ClientVersion)
conn.GetName(), remoteInfo.ClientOs, remoteInfo.ClientArch, remoteInfo.ClientVersion)
client := conn.GetClient()
if client == nil {
return fmt.Errorf("cannot update wsh: ssh client is not connected")
Expand Down
31 changes: 0 additions & 31 deletions pkg/remote/connutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,6 @@ func ParseOpts(input string) (*SSHOpts, error) {
return &SSHOpts{SSHHost: remoteHost, SSHUser: remoteUser, SSHPort: remotePort}, nil
}

func DetectShell(client *ssh.Client) (string, error) {
wshPath := GetWshPath(client)

session, err := client.NewSession()
if err != nil {
return "", err
}

log.Printf("shell detecting using command: %s shell", wshPath)
out, err := session.Output(wshPath + " shell")
if err != nil {
log.Printf("unable to determine shell. defaulting to /bin/bash: %s", err)
return "/bin/bash", nil
}
log.Printf("detecting shell: %s", out)

return fmt.Sprintf(`"%s"`, strings.TrimSpace(string(out))), nil
}

func GetWshPath(client *ssh.Client) string {
defaultPath := wavebase.RemoteFullWshBinPath
session, err := client.NewSession()
Expand Down Expand Up @@ -241,18 +222,6 @@ func InstallClientRcFiles(client *ssh.Client) error {
return err
}

func GetHomeDir(client *ssh.Client) string {
session, err := client.NewSession()
if err != nil {
return "~"
}
out, err := session.Output(`echo "$HOME"`)
if err == nil {
return strings.TrimSpace(string(out))
}
return "~"
}

func IsPowershell(shellPath string) bool {
// get the base path, and then check contains
shellBase := filepath.Base(shellPath)
Expand Down
35 changes: 22 additions & 13 deletions pkg/shellexec/shellexec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/creack/pty"
"github.com/wavetermdev/waveterm/pkg/genconn"
"github.com/wavetermdev/waveterm/pkg/panichandler"
"github.com/wavetermdev/waveterm/pkg/remote"
"github.com/wavetermdev/waveterm/pkg/remote/conncontroller"
Expand All @@ -27,6 +28,8 @@ import (
"github.com/wavetermdev/waveterm/pkg/util/utilfn"
"github.com/wavetermdev/waveterm/pkg/wavebase"
"github.com/wavetermdev/waveterm/pkg/waveobj"
"github.com/wavetermdev/waveterm/pkg/wshrpc"
"github.com/wavetermdev/waveterm/pkg/wshrpc/wshclient"
"github.com/wavetermdev/waveterm/pkg/wshutil"
"github.com/wavetermdev/waveterm/pkg/wsl"
)
Expand Down Expand Up @@ -286,41 +289,46 @@ func StartRemoteShellProcNoWsh(termSize waveobj.TermSize, cmdStr string, cmdOpts

func StartRemoteShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOptsType, conn *conncontroller.SSHConn) (*ShellProc, error) {
client := conn.GetClient()
connRoute := wshutil.MakeConnectionRouteId(conn.GetName())
rpcClient := wshclient.GetBareRpcClient()
remoteInfo, err := wshclient.RemoteGetInfoCommand(rpcClient, &wshrpc.RpcOpts{Route: connRoute, Timeout: 2000})
if err != nil {
return nil, fmt.Errorf("unable to obtain client info: %w", err)
}
log.Printf("client info collected: %+#v", remoteInfo)

shellPath := cmdOpts.ShellPath
if shellPath == "" {
remoteShellPath, err := remote.DetectShell(client)
if err != nil {
return nil, err
}
shellPath = remoteShellPath
shellPath = remoteInfo.Shell
}
var shellOpts []string
var cmdCombined string
log.Printf("detected shell: %s", shellPath)
log.Printf("using shell: %s", shellPath)

err := remote.InstallClientRcFiles(client)
err = remote.InstallClientRcFiles(client)
if err != nil {
log.Printf("error installing rc files: %v", err)
return nil, err
}
shellOpts = append(shellOpts, cmdOpts.ShellOpts...)

homeDir := remote.GetHomeDir(client)

if cmdStr == "" {
/* transform command in order to inject environment vars */
if isBashShell(shellPath) {
log.Printf("recognized as bash shell")
// add --rcfile
// cant set -l or -i with --rcfile
shellOpts = append(shellOpts, "--rcfile", fmt.Sprintf(`"%s"/.waveterm/%s/.bashrc`, homeDir, shellutil.BashIntegrationDir))
bashPath := genconn.SoftQuote(fmt.Sprintf("~/.waveterm/%s/.bashrc", shellutil.BashIntegrationDir))
shellOpts = append(shellOpts, "--rcfile", bashPath)
} else if isFishShell(shellPath) {
carg := fmt.Sprintf(`"set -x PATH \"%s\"/.waveterm/%s $PATH"`, homeDir, shellutil.WaveHomeBinDir)
fishDir := genconn.SoftQuote(fmt.Sprintf("~/.waveterm/%s", shellutil.WaveHomeBinDir))
carg := fmt.Sprintf(`"set -x PATH %s $PATH"`, fishDir)
shellOpts = append(shellOpts, "-C", carg)
} else if remote.IsPowershell(shellPath) {
pwshPath := genconn.SoftQuote(fmt.Sprintf("~/.waveterm/%s/wavepwsh.ps1", shellutil.PwshIntegrationDir))
// powershell is weird about quoted path executables and requires an ampersand first
shellPath = "& " + shellPath
shellOpts = append(shellOpts, "-ExecutionPolicy", "Bypass", "-NoExit", "-File", fmt.Sprintf("%s/.waveterm/%s/wavepwsh.ps1", homeDir, shellutil.PwshIntegrationDir))
shellOpts = append(shellOpts, "-ExecutionPolicy", "Bypass", "-NoExit", "-File", pwshPath)
} else {
if cmdOpts.Login {
shellOpts = append(shellOpts, "-l")
Expand Down Expand Up @@ -374,7 +382,8 @@ func StartRemoteShellProc(termSize waveobj.TermSize, cmdStr string, cmdOpts Comm
}

if isZshShell(shellPath) {
cmdCombined = fmt.Sprintf(`ZDOTDIR="%s/.waveterm/%s" %s`, homeDir, shellutil.ZshIntegrationDir, cmdCombined)
zshDir := genconn.SoftQuote(fmt.Sprintf("~/.waveterm/%s", shellutil.ZshIntegrationDir))
cmdCombined = fmt.Sprintf(`ZDOTDIR=%s %s`, zshDir, cmdCombined)
Comment on lines +385 to +386
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Environment Variable ZDOTDIR May Not Be Set Correctly

Setting ZDOTDIR after the session is created may not affect the Zsh initialization as expected. Consider setting ZDOTDIR before the session starts to ensure that the custom Zsh configuration is loaded.

Apply this diff to set ZDOTDIR before the session starts:

     for envKey, envVal := range cmdOpts.Env {
         // note these might fail depending on server settings, but we still try
         session.Setenv(envKey, envVal)
     }

+    if isZshShell(shellPath) {
+        zshDir := genconn.SoftQuote(fmt.Sprintf("~/.waveterm/%s", shellutil.ZshIntegrationDir))
+        session.Setenv("ZDOTDIR", zshDir)
+    }

     jwtToken, ok := cmdOpts.Env[wshutil.WaveJwtTokenVarName]
     if !ok {
         return nil, fmt.Errorf("no jwt token provided to connection")

Committable suggestion skipped: line range outside the PR's diff.

}

jwtToken, ok := cmdOpts.Env[wshutil.WaveJwtTokenVarName]
Expand Down
6 changes: 6 additions & 0 deletions pkg/wshrpc/wshclient/wshclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,12 @@ func RemoteFileTouchCommand(w *wshutil.WshRpc, data string, opts *wshrpc.RpcOpts
return err
}

// command "remotegetinfo", wshserver.RemoteGetInfoCommand
func RemoteGetInfoCommand(w *wshutil.WshRpc, opts *wshrpc.RpcOpts) (wshrpc.RemoteInfo, error) {
resp, err := sendRpcRequestCallHelper[wshrpc.RemoteInfo](w, "remotegetinfo", nil, opts)
return resp, err
}

// command "remotemkdir", wshserver.RemoteMkdirCommand
func RemoteMkdirCommand(w *wshutil.WshRpc, data string, opts *wshrpc.RpcOpts) error {
_, err := sendRpcRequestCallHelper[any](w, "remotemkdir", data, opts)
Expand Down
5 changes: 5 additions & 0 deletions pkg/wshrpc/wshremote/wshremote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/wavetermdev/waveterm/pkg/util/utilfn"
"github.com/wavetermdev/waveterm/pkg/wavebase"
"github.com/wavetermdev/waveterm/pkg/wshrpc"
"github.com/wavetermdev/waveterm/pkg/wshutil"
)

const MaxFileSize = 50 * 1024 * 1024 // 10M
Expand Down Expand Up @@ -387,3 +388,7 @@ func (*ServerImpl) RemoteFileDeleteCommand(ctx context.Context, path string) err
}
return nil
}

func (*ServerImpl) RemoteGetInfoCommand(ctx context.Context) (wshrpc.RemoteInfo, error) {
return wshutil.GetInfo(), nil
}
3 changes: 2 additions & 1 deletion pkg/wshrpc/wshrpctypes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const (
Command_GetVar = "getvar"
Command_SetVar = "setvar"
Command_RemoteMkdir = "remotemkdir"
Command_RemoteGetInfo = "remotegetinfo"

Command_ConnStatus = "connstatus"
Command_WslStatus = "wslstatus"
Expand Down Expand Up @@ -179,6 +180,7 @@ type WshRpcInterface interface {
RemoteFileJoinCommand(ctx context.Context, paths []string) (*FileInfo, error)
RemoteMkdirCommand(ctx context.Context, path string) error
RemoteStreamCpuDataCommand(ctx context.Context) chan RespOrErrorUnion[TimeSeriesData]
RemoteGetInfoCommand(ctx context.Context) (RemoteInfo, error)

// emain
WebSelectorCommand(ctx context.Context, data CommandWebSelectorData) ([]string, error)
Expand Down Expand Up @@ -503,7 +505,6 @@ type ConnRequest struct {
}

type RemoteInfo struct {
ConnName string `json:"host"`
ClientArch string `json:"clientarch"`
ClientOs string `json:"clientos"`
ClientVersion string `json:"clientversion"`
Expand Down
6 changes: 5 additions & 1 deletion pkg/wshrpc/wshserver/wshserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,11 @@ func (ws *WshServer) ConnReinstallWshCommand(ctx context.Context, data wshrpc.Co
}

func (ws *WshServer) ConnUpdateWshCommand(ctx context.Context, remoteInfo wshrpc.RemoteInfo) (bool, error) {
connName := remoteInfo.ConnName
handler := wshutil.GetRpcResponseHandlerFromContext(ctx)
if handler == nil {
return false, fmt.Errorf("could not determine handler from context")
}
connName := handler.GetRpcContext().Conn
if connName == "" {
return false, fmt.Errorf("invalid remote info: missing connection name")
}
Expand Down
19 changes: 16 additions & 3 deletions pkg/wshutil/wshutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"os"
"os/signal"
"runtime"
"strings"
"sync"
"sync/atomic"
"syscall"
Expand All @@ -22,6 +23,7 @@ import (
"github.com/google/uuid"
"github.com/wavetermdev/waveterm/pkg/panichandler"
"github.com/wavetermdev/waveterm/pkg/util/packetparser"
"github.com/wavetermdev/waveterm/pkg/util/shellutil"
"github.com/wavetermdev/waveterm/pkg/wavebase"
"github.com/wavetermdev/waveterm/pkg/wshrpc"
"golang.org/x/term"
Expand Down Expand Up @@ -538,13 +540,24 @@ func ExtractUnverifiedSocketName(tokenStr string) (string, error) {
return sockName, nil
}

func GetInfo(rpcContext wshrpc.RpcContext) wshrpc.RemoteInfo {
func getShell() string {
if runtime.GOOS == "darwin" {
return shellutil.GetMacUserShell()
}

shell := os.Getenv("SHELL")
if shell == "" {
return "/bin/bash"
}
return strings.TrimSpace(shell)
}

func GetInfo() wshrpc.RemoteInfo {
return wshrpc.RemoteInfo{
ConnName: rpcContext.Conn,
ClientArch: runtime.GOARCH,
ClientOs: runtime.GOOS,
ClientVersion: wavebase.WaveVersion,
Shell: os.Getenv("SHELL"),
Shell: getShell(),
}

Comment on lines +555 to 562
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Shell path existence verification needed

The review comment is valid. The code assumes shell paths exist without verification:

  • For non-Darwin: uses $SHELL or falls back to "/bin/bash"
  • For Darwin: uses UserShell from dscl or falls back to "/bin/bash"
  • No existence check is performed at any point in the chain

Suggested locations to add verification:

  • pkg/wshutil/wshutil.go:getShell(): Verify returned path exists
  • pkg/util/shellutil/shellutil.go:GetMacUserShell(): Verify shell path before returning
🔗 Analysis chain

Verify shell path existence.

The shell path returned by getShell() should be verified to exist before being used in GetInfo().

Run this script to verify the shell paths:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the default shell paths exist
# Test: Check if common shell paths exist
echo "Checking default shell paths..."
for shell in "/bin/bash" "/bin/sh" "/bin/zsh"; do
  if [ -f "$shell" ]; then
    echo "Found: $shell"
  else
    echo "Missing: $shell"
  fi
done

# Test: Check if GetMacUserShell is properly implemented
echo "Checking for shellutil.GetMacUserShell implementation..."
rg -A 5 "func GetMacUserShell"

Length of output: 911

Script:

#!/bin/bash
# Find getShell implementation and its dependencies
ast-grep --pattern 'func getShell()' -l go
rg -A 10 'func getShell\(\)' 

# Check for shell path validation
rg -i 'exists|stat|isfile|filepath' pkg/wshutil/

Length of output: 1817

Script:

#!/bin/bash
# Check GetMacUserShell implementation
rg -A 15 'func internalMacUserShell' pkg/util/shellutil/

Length of output: 1094

Script:

#!/bin/bash
# Check DefaultShellPath constant
rg "DefaultShellPath\s*=" pkg/util/shellutil/

Length of output: 116

}
Loading