From a852a8d96cf94d1d23fb7fc9fcb2970d92c5fa74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20D=C3=B6tsch?= Date: Sat, 20 Sep 2025 10:35:48 +0200 Subject: [PATCH 1/2] cleanups --- cmd/main.go | 8 -------- pkg/commands.go | 4 ++-- pkg/interactive.go | 8 ++++---- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 5a8384d..daaefee 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,9 +1,7 @@ package main import ( - "bufio" "fmt" - "log" "os" "github.com/brainexe/gosh/pkg" @@ -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 { diff --git a/pkg/commands.go b/pkg/commands.go index 7ddb7be..dcf1b95 100644 --- a/pkg/commands.go +++ b/pkg/commands.go @@ -11,8 +11,8 @@ 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 diff --git a/pkg/interactive.go b/pkg/interactive.go index 6121922..6dac7af 100644 --- a/pkg/interactive.go +++ b/pkg/interactive.go @@ -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 { @@ -109,7 +109,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) @@ -158,7 +158,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() From 169aa80cbe984e059e2246409a9c1b8b725832d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20D=C3=B6tsch?= Date: Tue, 23 Sep 2025 18:26:35 +0200 Subject: [PATCH 2/2] refactor autocomplete and connection management for improved clarity and functionality --- .gitignore | 3 +++ AGENTS.md | 3 ++- Makefile | 3 +++ pkg/autocomplete.go | 32 +++++++++++--------------------- pkg/commands.go | 2 +- pkg/interactive.go | 7 +++++-- pkg/main_test.go | 32 +++++++++++++++++++++++++++----- pkg/ssh.go | 16 +++++++--------- test_data/README.md | 3 --- test_data/test_server_key | 27 --------------------------- test_data/test_server_key.pub | 1 - 11 files changed, 59 insertions(+), 70 deletions(-) delete mode 100644 test_data/README.md delete mode 100644 test_data/test_server_key delete mode 100644 test_data/test_server_key.pub diff --git a/.gitignore b/.gitignore index 2003bf3..5e01085 100644 --- a/.gitignore +++ b/.gitignore @@ -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/ diff --git a/AGENTS.md b/AGENTS.md index 62cf7a3..cdfa363 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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: diff --git a/Makefile b/Makefile index 45b0fcd..5e26593 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/pkg/autocomplete.go b/pkg/autocomplete.go index cf2dae6..83ccaf4 100644 --- a/pkg/autocomplete.go +++ b/pkg/autocomplete.go @@ -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 @@ -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 @@ -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{} @@ -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+"/") @@ -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 @@ -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 @@ -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{} } @@ -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{} @@ -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) // For all completions (commands and paths), return suffixes as expected by readline var filteredCompletions []string diff --git a/pkg/commands.go b/pkg/commands.go index dcf1b95..4d6c34a 100644 --- a/pkg/commands.go +++ b/pkg/commands.go @@ -18,7 +18,7 @@ func executeCommandStreaming(ctx context.Context, cm *SSHConnectionManager, host for i, host := range hosts { wg.Go(func() { - cm.runSSHPersistentStreaming(ctx, host, command, i, maxHostLen, noColor) + cm.runSSHStreaming(ctx, host, command, i, maxHostLen, noColor) }) } diff --git a/pkg/interactive.go b/pkg/interactive.go index 6dac7af..5b73c46 100644 --- a/pkg/interactive.go +++ b/pkg/interactive.go @@ -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 @@ -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", } diff --git a/pkg/main_test.go b/pkg/main_test.go index 45f0468..dcf966e 100644 --- a/pkg/main_test.go +++ b/pkg/main_test.go @@ -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 ( @@ -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) } @@ -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 { @@ -829,7 +831,7 @@ func TestSSHConnectionManager(t *testing.T) { } // Clean up - cm.cleanupAllConnections() + cm.closeAllConnections() }) } } @@ -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 { @@ -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 @@ -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 diff --git a/pkg/ssh.go b/pkg/ssh.go index 24819d5..e65c044 100644 --- a/pkg/ssh.go +++ b/pkg/ssh.go @@ -34,7 +34,6 @@ type SSHConnectionManager struct { type SSHConnection struct { host string socketPath string - connected bool } // NewSSHConnectionManager creates a new connection manager @@ -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{ @@ -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 @@ -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 diff --git a/test_data/README.md b/test_data/README.md deleted file mode 100644 index d6b03f0..0000000 --- a/test_data/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# Test Data - SSH Server Keys - -This directory contains SSH key pairs used exclusively for testing purposes with a locally spawned SSH server. These are test-only keys and should not be used for any real SSH connections. diff --git a/test_data/test_server_key b/test_data/test_server_key deleted file mode 100644 index 8de9874..0000000 --- a/test_data/test_server_key +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN OPENSSH PRIVATE KEY----- -b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn -NhAAAAAwEAAQAAAQEAtcun3/iytupEes+ncHw6FIXltb7IiaWZSWDxob7awmcZVi7SzmEp -j/CaGgYgXPJGVYLDEegXFMiVgkHQeKoW1Ke7m9/ZX3AcAz9AYRcPLRkhgo1DL2SwZmYSZW -JMD/eKXUi6KXpEZpMoGsipgl3GP4kbw3Y9T5BIH5lP4aDNduc42jQ1SahQzas2ilf39iDe -2wGZNKISFOq/guGqmNBGfpaqVu3y/39ZiNeff3W0Jly+hHqTF5mqk4+5YCVtOsaN66eQ8t -0SzWZVM4x1fRvEnoM52qQcI1Ok35cQEQVOY9FojSWjbzZeXBEOSc3fza0qE5tX1N3SAB4+ -St7rflCPuQAAA8gTGNBAExjQQAAAAAdzc2gtcnNhAAABAQC1y6ff+LK26kR6z6dwfDoUhe -W1vsiJpZlJYPGhvtrCZxlWLtLOYSmP8JoaBiBc8kZVgsMR6BcUyJWCQdB4qhbUp7ub39lf -cBwDP0BhFw8tGSGCjUMvZLBmZhJlYkwP94pdSLopekRmkygayKmCXcY/iRvDdj1PkEgfmU -/hoM125zjaNDVJqFDNqzaKV/f2IN7bAZk0ohIU6r+C4aqY0EZ+lqpW7fL/f1mI159/dbQm -XL6EepMXmaqTj7lgJW06xo3rp5Dy3RLNZlUzjHV9G8SegznapBwjU6TflxARBU5j0WiNJa -NvNl5cEQ5Jzd/NrSoTm1fU3dIAHj5K3ut+UI+5AAAAAwEAAQAAAQAru4WzYdrwGLQHjSuc -6i14oWtMiMwqHK2e0tTd0ZFDgdS9AD+TCRmb+EfB6eZgJaIY0P+HL7tZsxUQRC/Xzyb12j -HOAhADHu+GMnGUyZzLfweqJbxbStAKmhRj0j00/BoLbcLm1nRc7pqPsuTgKRnXT+7fWvLN -0IohG9r1Dp9OslCgjJdHrQ/DecOpMt7rRpiorBTBLfrg2YLHScDX462+N71pzY9iAaN6uB -aHBU2ip+ywx5QOxqc+v5dKegDa/1AvFaVPDp4vNdxw9xBIO79K61C4SlyexFrXUQg+x3Cd -7DUGYVE91grdWvtiaxp4s081SPnJqZGXQ4iKibeVqay5AAAAgEo18OEq/+74F7VC773veC -O9qJovLzelMC/lbkiHZ4ICXWGFvgspBf+pGnp9ENacE9GnF/fuOKNIYw7IZdQ5BSABMyXP -IdeQTTEUlqCWE5LV5g2IXYLb+JkXWcvz4mLOKAJ5EatFl5dvNxjM9h3sRnvg334Drm5K/r -WLe8QsBb9wAAAAgQD/Ez3navxpcfLjf5PTpP+fK8dYV/Lcd5KRneRFgYIC8eMZXkNm9Dz0 -7dpfwAR9aCAZ6sT0M09Ym3DpFegfATxhErzbxNt3QvwokUufOsEZLD6jctNMfFw5X9ZJQk -2Sawkj0W9yVB5jT3+r4C2U1uNakB4F3pVV8tcnmzFSB0W0nQAAAIEAtnRlg0fELjZOp0kL -5UdWjyQGBFa91sNgKH2RztavsIx9mOL9KbSlRC4zpoAC4/I/BjpfMlXGMPu8aSlUEu6Gjy -2Zx4JU0sbom+Y5tFdhw+oP2jaxxg/EUTT6qfz14wjWHs7ZyJRUeuqyrTCuAfHpkMKpEn5e -MM7IpYXFlLvMRs0AAAAPdGVzdC1zZXJ2ZXIta2V5AQIDBA== ------END OPENSSH PRIVATE KEY----- diff --git a/test_data/test_server_key.pub b/test_data/test_server_key.pub deleted file mode 100644 index f5f076d..0000000 --- a/test_data/test_server_key.pub +++ /dev/null @@ -1 +0,0 @@ -ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC1y6ff+LK26kR6z6dwfDoUheW1vsiJpZlJYPGhvtrCZxlWLtLOYSmP8JoaBiBc8kZVgsMR6BcUyJWCQdB4qhbUp7ub39lfcBwDP0BhFw8tGSGCjUMvZLBmZhJlYkwP94pdSLopekRmkygayKmCXcY/iRvDdj1PkEgfmU/hoM125zjaNDVJqFDNqzaKV/f2IN7bAZk0ohIU6r+C4aqY0EZ+lqpW7fL/f1mI159/dbQmXL6EepMXmaqTj7lgJW06xo3rp5Dy3RLNZlUzjHV9G8SegznapBwjU6TflxARBU5j0WiNJaNvNl5cEQ5Jzd/NrSoTm1fU3dIAHj5K3ut+UI+5 test-server-key