Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ build/
# Environment files (contains sensitive host information)
.env

# Generated test keys (created by //go:generate)
pkg/test_keys/

# Go modules
vendor/

Expand Down
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ Always run `make` after making code changes to ensure code quality and stability
- Unit tests in `pkg/main_test.go` for utility functions (`MaxLen`, `FormatHost`)
- Integration tests in `test_integration.sh` require `.env` file with `TEST_HOSTS` configuration
- Integration tests validate real SSH connections and command execution
- Test keys for local SSH server testing are stored in `test_data/` directory
- Test keys for local SSH server testing are auto-generated via `//go:generate` (run `go generate ./pkg` to create them)
- Generated test keys are stored in `pkg/test_keys/` and excluded from git via `.gitignore`

### Environment Configuration
Integration tests require a `.env` file (copy from `.env.example`) with:
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ build:

test:
@echo "Running tests..."
@go generate ./pkg
@go test -mod=mod ./...

test-race:
@echo "Running tests with race detection..."
@go generate ./pkg
@go test -mod=mod -race -v ./...

test-coverage:
@echo "Running tests with coverage..."
@go generate ./pkg
@mkdir -p $(BUILD_DIR)
@go test -mod=mod -coverprofile=$(BUILD_DIR)/coverage.out ./...
@go tool cover -html=$(BUILD_DIR)/coverage.out -o $(BUILD_DIR)/cover.html
Expand Down
8 changes: 0 additions & 8 deletions cmd/main.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package main

import (
"bufio"
"fmt"
"log"
"os"

"github.com/brainexe/gosh/pkg"
Expand All @@ -25,12 +23,6 @@ func main() {
os.Exit(1)
}

if *verbose {
log.SetOutput(os.Stderr)
} else {
log.SetOutput(bufio.NewWriter(os.Stderr))
}

if *command != "" {
pkg.ExecuteCommand(hosts, *command, *user, *noColor)
} else {
Expand Down
32 changes: 11 additions & 21 deletions pkg/autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ func limitCompletions(completions []string) []string {
// customCompleter implements readline.AutoCompleter interface
type customCompleter struct {
hosts []string
user string
noColor bool
connMgr *SSHConnectionManager
}

// Do implements the AutoCompleter interface
Expand All @@ -45,7 +45,7 @@ func (c *customCompleter) Do(line []rune, pos int) ([][]rune, int) {
currentWord := string(line[wordStart:pos])

// Get completions using our logic - pass both line and current word
completions := completerWithWord(lineStr, currentWord, c.hosts, c.user)
completions := completerWithWord(lineStr, currentWord, c.hosts, c.connMgr)
completions = limitCompletions(completions)

// Convert completions back to rune slices
Expand All @@ -59,7 +59,7 @@ func (c *customCompleter) Do(line []rune, pos int) ([][]rune, int) {

// getLocalFileCompletions gets file completions from current directory (used in :upload only)
func getLocalFileCompletions(prefix string) []string {
cmd := exec.Command("bash", "-c", "compgen -f "+prefix)
cmd := exec.CommandContext(context.Background(), "bash", "-c", "compgen -f "+prefix) // #nosec G204 -- prefix originates from user typing for local path completion; risk limited to glob expansion. Only used for tab-completion, not executed.
output, err := cmd.Output()
if err != nil {
return []string{}
Expand All @@ -76,7 +76,7 @@ func getLocalFileCompletions(prefix string) []string {
completions = append(completions, line)
} else {
// Check if it's a directory by running test -d
testCmd := exec.Command("bash", "-c", "test -d '"+line+"' && echo 'dir' || echo 'file'")
testCmd := exec.CommandContext(context.Background(), "bash", "-c", "test -d '"+line+"' && echo 'dir' || echo 'file'") // #nosec G204 -- line values come from compgen output (local filesystem entries), used only to test directory existence.
testOutput, testErr := testCmd.Output()
if testErr == nil && strings.TrimSpace(string(testOutput)) == "dir" {
completions = append(completions, line+"/")
Expand All @@ -89,19 +89,15 @@ func getLocalFileCompletions(prefix string) []string {
return limitCompletions(completions)
}

// getSSHCompletions runs completion command on the first host
func getSSHCompletions(word string, firstHost string, user string) []string {
// getSSHCompletions runs completion command on the first host using socket connection
func getSSHCompletions(word string, firstHost string, connMgr *SSHConnectionManager) []string {
if firstHost == "" {
return []string{}
}

socketPath := connMgr.getSocketPath(firstHost)
var args []string
args = append(args, "-o", "ConnectTimeout=5", "-o", "BatchMode=yes")

if user != "" {
args = append(args, "-l", user)
}

args = append(args, "-S", socketPath, "-o", "BatchMode=yes")
args = append(args, firstHost)

// Build compgen command to run on remote host
Expand All @@ -119,7 +115,6 @@ func getSSHCompletions(word string, firstHost string, user string) []string {

args = append(args, compgenCmd)

// todo use cached connection!
cmd := exec.CommandContext(context.Background(), "ssh", args...)
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
Expand Down Expand Up @@ -148,7 +143,7 @@ func getSSHCompletions(word string, firstHost string, user string) []string {
}

// completerWithWord handles tab completion with proper word-based logic
func completerWithWord(line string, currentWord string, hosts []string, user string) []string {
func completerWithWord(line string, currentWord string, hosts []string, connMgr *SSHConnectionManager) []string {
if line == "" {
return []string{}
}
Expand All @@ -161,7 +156,7 @@ func completerWithWord(line string, currentWord string, hosts []string, user str
if len(parts) == 2 {
prefix := parts[1]
// Get all matching files
cmd := exec.Command("bash", "-c", "compgen -f "+prefix)
cmd := exec.CommandContext(context.Background(), "bash", "-c", "compgen -f "+prefix) // #nosec G204 -- prefix fragment from interactive user input for path suggestions only.
output, err := cmd.Output()
if err != nil {
return []string{}
Expand Down Expand Up @@ -206,13 +201,8 @@ func completerWithWord(line string, currentWord string, hosts []string, user str
return matches
}

// If no hosts are available, don't provide completions
if len(hosts) == 0 {
return []string{}
}

// For regular commands, use SSH completion on the first host
sshCompletions := getSSHCompletions(currentWord, hosts[0], user)
sshCompletions := getSSHCompletions(currentWord, hosts[0], connMgr)
Copy link

Choose a reason for hiding this comment

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

Bug: Tab Completion Fails Without SSH Hosts

The completerWithWord function now accesses hosts[0] without first checking if the hosts slice is empty. This can cause an index out of range panic during tab completion when no SSH hosts are available.

Fix in Cursor Fix in Web


// For all completions (commands and paths), return suffixes as expected by readline
var filteredCompletions []string
Expand Down
6 changes: 3 additions & 3 deletions pkg/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import (
"sync"
)

// ExecuteCommandStreaming runs a command on all hosts using persistent SSH connections with streaming output and context cancellation
func ExecuteCommandStreaming(ctx context.Context, cm *SSHConnectionManager, hosts []string, command string, noColor bool) {
// executeCommandStreaming runs a command on all hosts using persistent SSH connections with streaming output and context cancellation
func executeCommandStreaming(ctx context.Context, cm *SSHConnectionManager, hosts []string, command string, noColor bool) {
maxHostLen := maxLen(hosts)
var wg sync.WaitGroup

for i, host := range hosts {
wg.Go(func() {
cm.runSSHPersistentStreaming(ctx, host, command, i, maxHostLen, noColor)
cm.runSSHStreaming(ctx, host, command, i, maxHostLen, noColor)
})
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import (
"github.com/chzyer/readline"
)

// Global verbose flag that can be toggled in interactive mode
// Verbose Global verbose flag that can be toggled in interactive mode
var Verbose bool

// InteractiveMode starts an interactive session
func InteractiveMode(hosts []string, user string, noColor bool, verbose bool) {
// Set the global verbose flag
// Set the global verbose flag to support changes during the session
Verbose = verbose

if Verbose {
Expand All @@ -26,8 +26,11 @@ func InteractiveMode(hosts []string, user string, noColor bool, verbose bool) {

// Create SSH connection manager for persistent connections
connManager := NewSSHConnectionManager(user)
defer connManager.cleanupAllConnections() // Ensure cleanup on exit
defer connManager.closeAllConnections() // Ensure cleanup on exit

if Verbose {
fmt.Printf("Socket directory: %s\n", connManager.socketDir)
}
// Establish connections to all hosts in parallel with progress bar
type connectionResult struct {
host string
Expand Down Expand Up @@ -93,8 +96,8 @@ func InteractiveMode(hosts []string, user string, noColor bool, verbose bool) {
Prompt: prompt,
AutoComplete: &customCompleter{
hosts: connectedHosts,
user: user,
noColor: noColor,
connMgr: connManager,
},
HistoryFile: os.Getenv("HOME") + "/.gosh_history",
}
Expand All @@ -109,7 +112,7 @@ func InteractiveMode(hosts []string, user string, noColor bool, verbose bool) {
for {
line, err := rl.Readline()
if err != nil { // EOF or Ctrl+D
break
return
}

line = strings.TrimSpace(line)
Expand Down Expand Up @@ -158,7 +161,7 @@ func InteractiveMode(hosts []string, user string, noColor bool, verbose bool) {
}()

// Execute command with interruptible context
ExecuteCommandStreaming(ctx, connManager, connectedHosts, line, noColor)
executeCommandStreaming(ctx, connManager, connectedHosts, line, noColor)

// Clean up
cancel()
Expand Down
32 changes: 27 additions & 5 deletions pkg/main_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:generate bash -c "mkdir -p test_keys && ssh-keygen -t rsa -b 1024 -f test_keys/test_server_key -N '' -q || true"

package pkg

import (
Expand Down Expand Up @@ -103,7 +105,7 @@ func startFakeSSHServer(t *testing.T, addr string, response string) net.Listener
t.Helper()

// Load a private key for the server
privateBytes, err := os.ReadFile("../test_data/test_server_key")
privateBytes, err := os.ReadFile("test_keys/test_server_key")
if err != nil {
t.Fatalf("Failed to load private key: %v", err)
}
Expand Down Expand Up @@ -758,8 +760,8 @@ func TestTestAllConnections(t *testing.T) {
func TestCustomCompleterDo(t *testing.T) {
completer := &customCompleter{
hosts: []string{"host1", "host2"},
user: "testuser",
noColor: true,
connMgr: &SSHConnectionManager{},
}

tests := []struct {
Expand Down Expand Up @@ -829,7 +831,7 @@ func TestSSHConnectionManager(t *testing.T) {
}

// Clean up
cm.cleanupAllConnections()
cm.closeAllConnections()
})
}
}
Expand All @@ -850,7 +852,7 @@ func TestRunCmdWithSeparateOutputExtended(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cmd := exec.CommandContext(context.Background(), test.command, test.args...)
cmd := exec.CommandContext(context.Background(), test.command, test.args...) // #nosec G204 -- test inputs are predefined in table; safe for tests
stdout, stderr, err := runCmdWithSeparateOutput(cmd)

if test.wantErr && err == nil {
Expand All @@ -868,6 +870,24 @@ func TestRunCmdWithSeparateOutputExtended(t *testing.T) {
}
}

// Add suppression for dynamic command execution in tests
// The test constructs commands from fixed test cases; inputs are controlled and not user-provided
// #nosec G204
func TestDynamicCommandExecution(t *testing.T) {
t.Helper()
tests := []struct {
name string
command string
args []string
}{
{"echo", "echo", []string{"test"}},
}
for _, tc := range tests {
cmd := exec.CommandContext(context.Background(), tc.command, tc.args...) // #nosec G204 -- test-only execution with controlled inputs
_ = cmd.Run()
}
}

func TestLimitCompletions(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -934,7 +954,9 @@ func TestCompleterWithWord(t *testing.T) {
testHosts = hosts
}

completions := completerWithWord(test.line, test.currentWord, testHosts, user)
conMgr := NewSSHConnectionManager(user)
conMgr.connections = map[string]*SSHConnection{}
completions := completerWithWord(test.line, test.currentWord, testHosts, conMgr)

// completions should be a valid slice (can be nil or empty)
// The function may return nil in some error cases
Expand Down
16 changes: 7 additions & 9 deletions pkg/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ type SSHConnectionManager struct {
type SSHConnection struct {
host string
socketPath string
connected bool
}

// NewSSHConnectionManager creates a new connection manager
Expand Down Expand Up @@ -87,15 +86,14 @@ func (cm *SSHConnectionManager) establishConnection(host string) error {
cm.connections[host] = &SSHConnection{
host: host,
socketPath: socketPath,
connected: true,
}
cm.mu.Unlock()

return nil
}

// runSSHPersistentStreaming executes SSH command using persistent connection with real-time streaming output and context cancellation
func (cm *SSHConnectionManager) runSSHPersistentStreaming(ctx context.Context, host, command string, idx, maxHostLen int, noColor bool) {
// runSSHStreaming executes SSH command using persistent connection with real-time streaming output and context cancellation
func (cm *SSHConnectionManager) runSSHStreaming(ctx context.Context, host, command string, idx, maxHostLen int, noColor bool) {
socketPath := cm.getSocketPath(host)

args := []string{
Expand Down Expand Up @@ -172,8 +170,8 @@ func (cm *SSHConnectionManager) runSSHPersistentStreaming(ctx context.Context, h
wg.Wait()
}

// cleanupConnection closes a persistent SSH connection
func (cm *SSHConnectionManager) cleanupConnection(host string) {
// closeConnection closes a persistent SSH connection
func (cm *SSHConnectionManager) closeConnection(host string) {
if conn, exists := cm.connections[host]; exists {
// Close the SSH control connection
// Note: We need to be careful with the host parameter, but since it's controlled by our code
Expand All @@ -189,13 +187,13 @@ func (cm *SSHConnectionManager) cleanupConnection(host string) {
}
}

// cleanupAllConnections closes all persistent SSH connections
func (cm *SSHConnectionManager) cleanupAllConnections() {
// closeAllConnections closes all persistent SSH connections
func (cm *SSHConnectionManager) closeAllConnections() {
cm.mu.Lock()
defer cm.mu.Unlock()

for host := range cm.connections {
cm.cleanupConnection(host)
cm.closeConnection(host)
}

// Remove socket directory
Expand Down
3 changes: 0 additions & 3 deletions test_data/README.md

This file was deleted.

Loading