Remove duplicated code

client/ssh/server/command_execution.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"runtime"
 	"time"
 
 	"github.com/gliderlabs/ssh"
@@ -25,7 +24,7 @@ func (s *Server) handleCommand(logger *log.Entry, session ssh.Session, privilege
 
 	logger.Infof("executing %s for %s from %s: %s", commandType, localUser.Username, session.RemoteAddr(), safeLogCommand(session.Command()))
 
-	execCmd, err := s.createCommandWithPrivileges(privilegeResult, session, hasPty)
+	execCmd, err := s.createCommand(privilegeResult, session, hasPty)
 	if err != nil {
 		logger.Errorf("%s creation failed: %v", commandType, err)
 
@@ -44,38 +43,19 @@ func (s *Server) handleCommand(logger *log.Entry, session ssh.Session, privilege
 		return
 	}
 
-	var success bool
+	if s.executeCommand(logger, session, execCmd) {
-	if hasPty {
+		logger.Debugf("%s execution completed", commandType)
-		success = s.handlePty(logger, session, privilegeResult, ptyReq, winCh)
-	} else {
-		success = s.executeCommand(logger, session, execCmd)
 	}
-
-	if !success {
-		return
-	}
-
-	logger.Debugf("%s execution completed", commandType)
 }
 
-func (s *Server) createCommandWithPrivileges(privilegeResult PrivilegeCheckResult, session ssh.Session, hasPty bool) (*exec.Cmd, error) {
+func (s *Server) createCommand(privilegeResult PrivilegeCheckResult, session ssh.Session, hasPty bool) (*exec.Cmd, error) {
 	localUser := privilegeResult.User
 
-	var cmd *exec.Cmd
+	// Try su first for system integration (PAM/audit) when privileged
-	var err error
+	cmd, err := s.createSuCommand(session, localUser, hasPty)
-
+	if err != nil || privilegeResult.UsedFallback {
-	// If we used fallback (unprivileged process), skip su and use direct execution
+		log.Debugf("su command failed, falling back to executor: %v", err)
-	if privilegeResult.UsedFallback {
+		cmd, err = s.createExecutorCommand(session, localUser, hasPty)
-		log.Debugf("using fallback - direct execution for current user")
-		cmd, err = s.createDirectCommand(session, localUser)
-	} else {
-		// Try su first for system integration (PAM/audit) when privileged
-		cmd, err = s.createSuCommand(session, localUser)
-		if err != nil {
-			// Always fall back to executor if su fails
-			log.Debugf("su command failed, falling back to executor: %v", err)
-			cmd, err = s.createExecutorCommand(session, localUser, hasPty)
-		}
 	}
 
 	if err != nil {
@@ -86,20 +66,6 @@ func (s *Server) createCommandWithPrivileges(privilegeResult PrivilegeCheckResul
 	return cmd, nil
 }
 
-// getShellCommandArgs returns the shell command and arguments for executing a command string
-func (s *Server) getShellCommandArgs(shell, cmdString string) []string {
-	if runtime.GOOS == "windows" {
-		if cmdString == "" {
-			return []string{shell, "-NoLogo"}
-		}
-		return []string{shell, "-Command", cmdString}
-	}
-
-	if cmdString == "" {
-		return []string{shell}
-	}
-	return []string{shell, "-c", cmdString}
-}
 
 // executeCommand executes the command and handles I/O and exit codes
 func (s *Server) executeCommand(logger *log.Entry, session ssh.Session, execCmd *exec.Cmd) bool {

client/ssh/server/command_execution_unix.go

@@ -19,7 +19,7 @@ import (
 )
 
 // createSuCommand creates a command using su -l -c for privilege switching
-func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*exec.Cmd, error) {
+func (s *Server) createSuCommand(session ssh.Session, localUser *user.User, hasPty bool) (*exec.Cmd, error) {
 	suPath, err := exec.LookPath("su")
 	if err != nil {
 		return nil, fmt.Errorf("su command not available: %w", err)
@@ -30,7 +30,7 @@ func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*ex
 		return nil, fmt.Errorf("no command specified for su execution")
 	}
 
-	// Use su -l -c to execute the command as the target user with login environment
+	// TODO: handle pty flag if available
 	args := []string{"-l", localUser.Username, "-c", command}
 
 	cmd := exec.CommandContext(session.Context(), suPath, args...)
@@ -39,6 +39,14 @@ func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*ex
 	return cmd, nil
 }
 
+// getShellCommandArgs returns the shell command and arguments for executing a command string
+func (s *Server) getShellCommandArgs(shell, cmdString string) []string {
+	if cmdString == "" {
+		return []string{shell, "-l"}
+	}
+	return []string{shell, "-l", "-c", cmdString}
+}
+
 // prepareCommandEnv prepares environment variables for command execution on Unix
 func (s *Server) prepareCommandEnv(localUser *user.User, session ssh.Session) []string {
 	env := prepareUserEnv(localUser, getUserShell(localUser.Uid))
@@ -94,7 +102,7 @@ func (s *Server) handlePty(logger *log.Entry, session ssh.Session, privilegeResu
 	localUser := privilegeResult.User
 	logger.Infof("executing Pty command for %s from %s: %s", localUser.Username, session.RemoteAddr(), safeLogCommand(cmd))
 
-	execCmd, err := s.createPtyCommandWithPrivileges(cmd, privilegeResult, ptyReq, session)
+	execCmd, err := s.createPtyCommand(privilegeResult, ptyReq, session)
 	if err != nil {
 		logger.Errorf("Pty command creation failed: %v", err)
 		errorMsg := "User switching failed - login command not available\r\n"

client/ssh/server/command_execution_windows.go

@@ -264,6 +264,14 @@ func (s *Server) handlePty(logger *log.Entry, session ssh.Session, privilegeResu
 	return true
 }
 
+// getShellCommandArgs returns the shell command and arguments for executing a command string
+func (s *Server) getShellCommandArgs(shell, cmdString string) []string {
+	if cmdString == "" {
+		return []string{shell, "-NoLogo"}
+	}
+	return []string{shell, "-Command", cmdString}
+}
+
 func (s *Server) handlePtyWithUserSwitching(logger *log.Entry, session ssh.Session, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, _ <-chan ssh.Window, _ []string) {
 	localUser := privilegeResult.User
 

client/ssh/server/executor_unix.go

@@ -99,8 +99,10 @@ func (pd *PrivilegeDropper) DropPrivileges(targetUID, targetGID uint32, suppleme
 	originalUID := os.Geteuid()
 	originalGID := os.Getegid()
 
-	if err := pd.setGroupsAndIDs(targetUID, targetGID, supplementaryGroups); err != nil {
+	if originalUID != int(targetUID) || originalGID != int(targetGID) {
-		return err
+		if err := pd.setGroupsAndIDs(targetUID, targetGID, supplementaryGroups); err != nil {
+			return fmt.Errorf("set groups and IDs: %w", err)
+		}
 	}
 
 	if err := pd.validatePrivilegeDropSuccess(targetUID, targetGID, originalUID, originalGID); err != nil {
@@ -188,8 +190,7 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config
 
 	// TODO: Implement Pty support for executor path
 	if config.PTY {
-		log.Warnf("Pty requested but executor does not support Pty yet - continuing without Pty")
+		config.PTY = false
-		config.PTY = false // Disable Pty and continue
 	}
 
 	if err := pd.DropPrivileges(config.UID, config.GID, config.Groups); err != nil {

client/ssh/server/executor_windows.go

@@ -522,6 +522,6 @@ func (pd *PrivilegeDropper) createProcessWithToken(ctx context.Context, sourceTo
 }
 
 // createSuCommand creates a command using su -l -c for privilege switching (Windows stub)
-func (s *Server) createSuCommand(ssh.Session, *user.User) (*exec.Cmd, error) {
+func (s *Server) createSuCommand(ssh.Session, *user.User, bool) (*exec.Cmd, error) {
 	return nil, fmt.Errorf("su command not available on Windows")
 }

client/ssh/server/server_test.go

@@ -475,8 +475,9 @@ func TestSSHServer_WindowsShellHandling(t *testing.T) {
 		// Test Unix shell behavior
 		args := server.getShellCommandArgs("/bin/sh", "echo test")
 		assert.Equal(t, "/bin/sh", args[0])
-		assert.Equal(t, "-c", args[1])
+		assert.Equal(t, "-l", args[1])
-		assert.Equal(t, "echo test", args[2])
+		assert.Equal(t, "-c", args[2])
+		assert.Equal(t, "echo test", args[3])
 	}
 }
 

client/ssh/server/userswitching_unix.go

@@ -59,41 +59,9 @@ func isFullyNumeric(username string) bool {
 	return true
 }
 
-// createSecurePtyUserSwitchCommand creates a Pty command with proper user switching
+// createPtyLoginCommand creates a Pty command using login for privileged processes
-// For privileged processes, uses login command. For non-privileged, falls back to shell.
+func (s *Server) createPtyLoginCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
-func (s *Server) createPtyUserSwitchCommand(_ []string, localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
+	loginPath, args, err := s.getLoginCmd(localUser.Username, session.RemoteAddr())
-	if !isCurrentProcessPrivileged() {
-		// Non-privileged process: fallback to shell with login flag
-		return s.createNonPrivilegedPtyCommand(localUser, ptyReq, session)
-	}
-
-	// Privileged process: use login command for proper user switching
-	return s.createPrivilegedPtyLoginCommand(localUser, ptyReq, session)
-}
-
-// createNonPrivilegedPtyCommand creates a Pty command for non-privileged processes
-func (s *Server) createNonPrivilegedPtyCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
-	shell := getUserShell(localUser.Uid)
-	args := []string{shell, "-l"}
-
-	execCmd := exec.CommandContext(session.Context(), args[0], args[1:]...)
-	execCmd.Dir = localUser.HomeDir
-	execCmd.Env = s.preparePtyEnv(localUser, ptyReq, session)
-
-	return execCmd, nil
-}
-
-// createPrivilegedPtyLoginCommand creates a Pty command using login for privileged processes
-func (s *Server) createPrivilegedPtyLoginCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
-	rawCmd := session.RawCommand()
-
-	// If there's a command to execute, use su -l -c instead of login
-	if rawCmd != "" {
-		return s.createPrivilegedPtySuCommand(localUser, ptyReq, session, rawCmd)
-	}
-
-	// For interactive sessions (no command), use login
-	loginPath, args, err := s.getRootLoginCmd(localUser.Username, session.RemoteAddr())
 	if err != nil {
 		return nil, fmt.Errorf("get login command: %w", err)
 	}
@@ -121,8 +89,8 @@ func (s *Server) createPrivilegedPtySuCommand(localUser *user.User, ptyReq ssh.P
 	return execCmd, nil
 }
 
-// getRootLoginCmd returns the login command and args for privileged Pty user switching
+// getLoginCmd returns the login command and args for privileged Pty user switching
-func (s *Server) getRootLoginCmd(username string, remoteAddr net.Addr) (string, []string, error) {
+func (s *Server) getLoginCmd(username string, remoteAddr net.Addr) (string, []string, error) {
 	loginPath, err := exec.LookPath("login")
 	if err != nil {
 		return "", nil, fmt.Errorf("login command not available: %w", err)
@@ -244,23 +212,29 @@ func enableUserSwitching() error {
 	return nil
 }
 
-// createPtyCommandWithPrivileges creates the exec.Cmd for Pty execution respecting privilege check results
+// createPtyCommand creates the exec.Cmd for Pty execution respecting privilege check results
-func (s *Server) createPtyCommandWithPrivileges(cmd []string, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
+func (s *Server) createPtyCommand(privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) {
 	localUser := privilegeResult.User
 
-	if privilegeResult.RequiresUserSwitching {
+	if privilegeResult.UsedFallback {
-		return s.createPtyUserSwitchCommand(cmd, localUser, ptyReq, session)
+		return s.createDirectPtyCommand(session, localUser, ptyReq), nil
 	}
 
-	// No user switching needed - create direct Pty command
+	return s.createPtyLoginCommand(localUser, ptyReq, session)
-	shell := getUserShell(localUser.Uid)
+}
-	rawCmd := session.RawCommand()
-	args := s.getShellCommandArgs(shell, rawCmd)
-	execCmd := exec.CommandContext(session.Context(), args[0], args[1:]...)
 
-	execCmd.Dir = localUser.HomeDir
+// createDirectPtyCommand creates a direct Pty command without privilege dropping
-	execCmd.Env = s.preparePtyEnv(localUser, ptyReq, session)
+func (s *Server) createDirectPtyCommand(session ssh.Session, localUser *user.User, ptyReq ssh.Pty) *exec.Cmd {
-	return execCmd, nil
+	log.Debugf("creating direct Pty command for user %s (no user switching needed)", localUser.Username)
+
+	shell := getUserShell(localUser.Uid)
+	args := s.getShellCommandArgs(shell, session.RawCommand())
+
+	cmd := exec.CommandContext(session.Context(), args[0], args[1:]...)
+	cmd.Dir = localUser.HomeDir
+	cmd.Env = s.preparePtyEnv(localUser, ptyReq, session)
+
+	return cmd
 }
 
 // preparePtyEnv prepares environment variables for Pty execution

client/ssh/server/userswitching_windows.go

@@ -79,9 +79,17 @@ func (s *Server) createExecutorCommand(session ssh.Session, localUser *user.User
 	return s.createUserSwitchCommand(localUser, session, hasPty)
 }
 
-// createDirectCommand is not supported on Windows - always use user switching with token creation
+// createDirectCommand creates a command that runs without privilege dropping
 func (s *Server) createDirectCommand(session ssh.Session, localUser *user.User) (*exec.Cmd, error) {
-	return nil, fmt.Errorf("direct command execution not supported on Windows - use user switching with token creation")
+	log.Debugf("creating direct command for user %s (no user switching needed)", localUser.Username)
+
+	shell := getUserShell(localUser.Uid)
+	args := s.getShellCommandArgs(shell, session.RawCommand())
+
+	cmd := exec.CommandContext(session.Context(), args[0], args[1:]...)
+	cmd.Dir = localUser.HomeDir
+
+	return cmd, nil
 }
 
 // createUserSwitchCommand creates a command with Windows user switching