From d686d97f8c142a411b15847488dc614e09ca0c8a Mon Sep 17 00:00:00 2001 From: Gu1llaum-3 Date: Thu, 9 Oct 2025 22:04:36 +0200 Subject: [PATCH] fix: SSH identity file paths with spaces and edit form navigation - Quote IdentityFile paths containing spaces to prevent SSH config errors - Fix edit form ESC/Ctrl+C to return to main view instead of quitting - Improve edit form navigation consistency with add form - Fix focus management when adding host fields with Ctrl+A --- internal/config/ssh.go | 28 +++++++--- internal/config/ssh_test.go | 106 ++++++++++++++++++++++++++++++++++++ internal/ui/edit_form.go | 95 ++++++++++++++++---------------- 3 files changed, 174 insertions(+), 55 deletions(-) diff --git a/internal/config/ssh.go b/internal/config/ssh.go index cd61757..d54d71b 100644 --- a/internal/config/ssh.go +++ b/internal/config/ssh.go @@ -418,6 +418,20 @@ func getMainConfigPath() string { return absPath } +// formatSSHConfigValue formats a value for SSH config file, adding quotes if necessary +func formatSSHConfigValue(value string) string { + if value == "" { + return value + } + + // If the value contains spaces, wrap it in quotes + if strings.Contains(value, " ") { + return `"` + value + `"` + } + + return value +} + // AddSSHHost adds a new SSH host to the config file func AddSSHHost(host SSHHost) error { configPath, err := GetDefaultSSHConfigPath() @@ -495,7 +509,7 @@ func AddSSHHostToFile(host SSHHost, configPath string) error { } if host.Identity != "" { - _, err = file.WriteString(fmt.Sprintf(" IdentityFile %s\n", host.Identity)) + _, err = file.WriteString(fmt.Sprintf(" IdentityFile %s\n", formatSSHConfigValue(host.Identity))) if err != nil { return err } @@ -795,7 +809,7 @@ func UpdateSSHHostInFile(oldName string, newHost SSHHost, configPath string) err newLines = append(newLines, " Port "+newHost.Port) } if newHost.Identity != "" { - newLines = append(newLines, " IdentityFile "+newHost.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(newHost.Identity)) } if newHost.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+newHost.ProxyJump) @@ -843,7 +857,7 @@ func UpdateSSHHostInFile(oldName string, newHost SSHHost, configPath string) err newLines = append(newLines, " Port "+newHost.Port) } if newHost.Identity != "" { - newLines = append(newLines, " IdentityFile "+newHost.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(newHost.Identity)) } if newHost.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+newHost.ProxyJump) @@ -927,7 +941,7 @@ func UpdateSSHHostInFile(oldName string, newHost SSHHost, configPath string) err newLines = append(newLines, " Port "+newHost.Port) } if newHost.Identity != "" { - newLines = append(newLines, " IdentityFile "+newHost.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(newHost.Identity)) } if newHost.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+newHost.ProxyJump) @@ -975,7 +989,7 @@ func UpdateSSHHostInFile(oldName string, newHost SSHHost, configPath string) err newLines = append(newLines, " Port "+newHost.Port) } if newHost.Identity != "" { - newLines = append(newLines, " IdentityFile "+newHost.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(newHost.Identity)) } if newHost.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+newHost.ProxyJump) @@ -1469,7 +1483,7 @@ func UpdateMultiHostBlock(originalHosts, newHosts []string, commonProperties SSH newLines = append(newLines, " Port "+commonProperties.Port) } if commonProperties.Identity != "" { - newLines = append(newLines, " IdentityFile "+commonProperties.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(commonProperties.Identity)) } if commonProperties.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+commonProperties.ProxyJump) @@ -1549,7 +1563,7 @@ func UpdateMultiHostBlock(originalHosts, newHosts []string, commonProperties SSH newLines = append(newLines, " Port "+commonProperties.Port) } if commonProperties.Identity != "" { - newLines = append(newLines, " IdentityFile "+commonProperties.Identity) + newLines = append(newLines, " IdentityFile "+formatSSHConfigValue(commonProperties.Identity)) } if commonProperties.ProxyJump != "" { newLines = append(newLines, " ProxyJump "+commonProperties.ProxyJump) diff --git a/internal/config/ssh_test.go b/internal/config/ssh_test.go index 3eaad39..939f75e 100644 --- a/internal/config/ssh_test.go +++ b/internal/config/ssh_test.go @@ -1442,3 +1442,109 @@ func contains(slice []string, item string) bool { } return false } + +// Helper function to create temporary config files for testing +func createTempConfigFile(content string) (string, error) { + tempFile, err := os.CreateTemp("", "ssh_config_test_*.conf") + if err != nil { + return "", err + } + defer tempFile.Close() + + _, err = tempFile.WriteString(content) + if err != nil { + os.Remove(tempFile.Name()) + return "", err + } + + return tempFile.Name(), nil +} + +func TestFormatSSHConfigValue(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple path without spaces", + input: "/home/user/.ssh/id_rsa", + expected: "/home/user/.ssh/id_rsa", + }, + { + name: "path with spaces", + input: "/home/user/My Documents/ssh key", + expected: "\"/home/user/My Documents/ssh key\"", + }, + { + name: "Windows path with spaces", + input: `G:\My Drive\7 - Tech\9 - SSH Keys\Server_WF.opk`, + expected: `"G:\My Drive\7 - Tech\9 - SSH Keys\Server_WF.opk"`, + }, + { + name: "path with quotes but no spaces", + input: `/home/user/key"with"quotes`, + expected: `/home/user/key"with"quotes`, + }, + { + name: "path with spaces and quotes", + input: `/home/user/key "with" quotes`, + expected: `"/home/user/key "with" quotes"`, + }, + { + name: "empty path", + input: "", + expected: "", + }, + { + name: "path with single space at end", + input: "/home/user/key ", + expected: "\"/home/user/key \"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := formatSSHConfigValue(tt.input) + if result != tt.expected { + t.Errorf("formatSSHConfigValue(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestAddSSHHostWithSpacesInPath(t *testing.T) { + // Create temporary config file + configFile, err := createTempConfigFile(`Host existing + HostName existing.com +`) + if err != nil { + t.Fatalf("Failed to create config file: %v", err) + } + defer os.Remove(configFile) + + // Test adding host with path containing spaces + host := SSHHost{ + Name: "test-spaces", + Hostname: "test.com", + User: "testuser", + Identity: "/path/with spaces/key file", + } + + err = AddSSHHostToFile(host, configFile) + if err != nil { + t.Fatalf("AddSSHHostToFile failed: %v", err) + } + + // Read the file and verify quotes are added + content, err := os.ReadFile(configFile) + if err != nil { + t.Fatalf("Failed to read config file: %v", err) + } + + contentStr := string(content) + expectedIdentityLine := ` IdentityFile "/path/with spaces/key file"` + if !strings.Contains(contentStr, expectedIdentityLine) { + t.Errorf("Expected identity file line with quotes not found.\nContent:\n%s\nExpected line: %s", contentStr, expectedIdentityLine) + } +} diff --git a/internal/ui/edit_form.go b/internal/ui/edit_form.go index e14c37e..69b1810 100644 --- a/internal/ui/edit_form.go +++ b/internal/ui/edit_form.go @@ -173,12 +173,17 @@ func (m *editFormModel) addHostInput() tea.Cmd { newInput.Placeholder = "host-name" newInput.Focus() - // Unfocus current input + // Unfocus current input regardless of which area we're in if m.focusArea == focusAreaHosts && m.focused < len(m.hostInputs) { m.hostInputs[m.focused].Blur() + } else if m.focusArea == focusAreaProperties && m.focused < len(m.inputs) { + m.inputs[m.focused].Blur() } m.hostInputs = append(m.hostInputs, newInput) + + // Move focus to the new host input + m.focusArea = focusAreaHosts m.focused = len(m.hostInputs) - 1 return textinput.Blink @@ -243,12 +248,50 @@ func (m *editFormModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "ctrl+c", "esc": m.err = "" m.success = false - return m, tea.Quit + return m, func() tea.Msg { return editFormCancelMsg{} } case "ctrl+s": // Allow submission from any field with Ctrl+S (Save) return m, m.submitEditForm() + case "tab", "shift+tab", "enter", "up", "down": + s := msg.String() + + // Handle form submission + totalFields := len(m.hostInputs) + len(m.inputs) + currentGlobalIndex := m.focused + if m.focusArea == focusAreaProperties { + currentGlobalIndex = len(m.hostInputs) + m.focused + } + + if s == "enter" && currentGlobalIndex == totalFields-1 { + return m, m.submitEditForm() + } + + // Cycle inputs + if s == "up" || s == "shift+tab" { + currentGlobalIndex-- + } else { + currentGlobalIndex++ + } + + if currentGlobalIndex >= totalFields { + currentGlobalIndex = 0 + } else if currentGlobalIndex < 0 { + currentGlobalIndex = totalFields - 1 + } + + // Update focus area and focused index based on global index + if currentGlobalIndex < len(m.hostInputs) { + m.focusArea = focusAreaHosts + m.focused = currentGlobalIndex + } else { + m.focusArea = focusAreaProperties + m.focused = currentGlobalIndex - len(m.hostInputs) + } + + return m, m.updateFocus() + case "ctrl+a": // Add a new host input return m, m.addHostInput() @@ -258,50 +301,6 @@ func (m *editFormModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.focusArea == focusAreaHosts && len(m.hostInputs) > 1 { return m, m.deleteHostInput() } - - case "tab", "shift+tab": - // Switch between host area and property area - if msg.String() == "shift+tab" { - if m.focusArea == focusAreaProperties { - m.focusArea = focusAreaHosts - m.focused = len(m.hostInputs) - 1 - } else { - m.focusArea = focusAreaProperties - m.focused = len(m.inputs) - 1 - } - } else { - if m.focusArea == focusAreaHosts { - m.focusArea = focusAreaProperties - m.focused = 0 - } else { - m.focusArea = focusAreaHosts - m.focused = 0 - } - } - return m, m.updateFocus() - - case "up", "down", "enter": - // Navigate within the current area - if m.focusArea == focusAreaHosts { - if msg.String() == "up" && m.focused > 0 { - m.focused-- - } else if msg.String() == "down" && m.focused < len(m.hostInputs)-1 { - m.focused++ - } else if msg.String() == "enter" { - // Submit form on enter - return m, m.submitEditForm() - } - } else { - if msg.String() == "up" && m.focused > 0 { - m.focused-- - } else if msg.String() == "down" && m.focused < len(m.inputs)-1 { - m.focused++ - } else if msg.String() == "enter" { - // Submit form on enter - return m, m.submitEditForm() - } - } - return m, m.updateFocus() } case editFormSubmitMsg: @@ -406,10 +405,10 @@ func (m *editFormModel) View() string { // Show different help based on number of hosts if len(m.hostInputs) > 1 { - b.WriteString(m.styles.FormHelp.Render("Tab: switch sections • ↑↓: navigate • Ctrl+A: add host • Ctrl+D: delete host")) + b.WriteString(m.styles.FormHelp.Render("Tab/↑↓/Enter: navigate • Ctrl+A: add host • Ctrl+D: delete host")) b.WriteString("\n") } else { - b.WriteString(m.styles.FormHelp.Render("Tab: switch sections • ↑↓: navigate • Ctrl+A: add host")) + b.WriteString(m.styles.FormHelp.Render("Tab/↑↓/Enter: navigate • Ctrl+A: add host")) b.WriteString("\n") } b.WriteString(m.styles.FormHelp.Render("Ctrl+S: save • Ctrl+C/Esc: cancel • * Required fields"))