diff --git a/build-all.sh b/build-all.sh index a953024..d937401 100755 --- a/build-all.sh +++ b/build-all.sh @@ -17,7 +17,7 @@ log() { echo -e "${GREEN}[nexus]${NC} $1"; } warn() { echo -e "${YELLOW}[nexus]${NC} $1"; } err() { echo -e "${RED}[nexus]${NC} $1" >&2; } -MIN_GO_VERSION="1.25.0" +MIN_GO_VERSION="1.25.12" WAILS_VERSION="$(awk '$1 == "github.com/wailsapp/wails/v2" { print $2; exit }' go.mod)" WAILS_BIN="" WAILS_USE_GO_RUN=0 diff --git a/build.sh b/build.sh index d8e3d1d..e79f261 100755 --- a/build.sh +++ b/build.sh @@ -18,7 +18,7 @@ log() { echo -e "${GREEN}[nexus]${NC} $1"; } warn() { echo -e "${YELLOW}[nexus]${NC} $1"; } err() { echo -e "${RED}[nexus]${NC} $1" >&2; } -MIN_GO_VERSION="1.25.0" +MIN_GO_VERSION="1.25.12" WAILS_VERSION="$(nexus_wails_version go.mod)" WAILS_BIN="" WAILS_USE_GO_RUN=0 diff --git a/cmd/vscode-bridge/main.go b/cmd/vscode-bridge/main.go index 362383a..af219e3 100644 --- a/cmd/vscode-bridge/main.go +++ b/cmd/vscode-bridge/main.go @@ -184,8 +184,11 @@ func newBridgeApp() (*bridgeApp, error) { } func (a *bridgeApp) close() { + if a.service != nil { + a.service.Shutdown() + } if a.manager != nil { - a.manager.CloseAll() + a.manager.Shutdown() } if a.store != nil { _ = a.store.Close() @@ -243,7 +246,7 @@ func (a *bridgeApp) handleRuntimeEvent(w http.ResponseWriter, r *http.Request) { value, _ := req.Payload["value"].(string) cancelled, _ := req.Payload["cancelled"].(bool) a.service.RespondSecret(requestID, value, cancelled) - a.manager.RespondSecret(requestID, value) + a.manager.RespondSecret(requestID, value, cancelled) case "session:hostkey:response": requestID, _ := req.Payload["requestID"].(string) accepted, _ := req.Payload["accepted"].(bool) diff --git a/frontend/scripts/test-regressions.js b/frontend/scripts/test-regressions.js index f4f1dc9..e937850 100644 --- a/frontend/scripts/test-regressions.js +++ b/frontend/scripts/test-regressions.js @@ -315,21 +315,6 @@ assertContains( "VNC Connections", "in-app help must document VNC connection profiles", ); -assertContains( - "vscode/nexus/extension.js", - "crypto.randomBytes(16).toString('base64')", - "Nexus VS Code webview CSP nonces must use cryptographic randomness", -); -assertContains( - "vscode/leviathan/extension.js", - "crypto.randomBytes(16).toString('base64')", - "Leviathan VS Code webview CSP nonces must use cryptographic randomness", -); -assertTreeNotContains( - "vscode", - "const nonce = String(Date.now());", - "VS Code webview CSP nonces must not be predictable timestamps", -); assertContains( "docs/user-manual.md", "## 11. Leviathan binary workbench", @@ -558,8 +543,8 @@ assertContains( ); assertContains( "scripts/build-common.sh", - "-O2 -g0", - "production builds must remove cgo debug output by default", + "CGO_ENABLED=0", + "production builds must keep the pure-Go SQLite toolchain", ); assertContains( "Makefile", diff --git a/go.mod b/go.mod index 5bf441d..302309b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,8 @@ module github.com/andydixon/nexus -go 1.25.0 +go 1.25.12 + +toolchain go1.26.5 require ( github.com/creack/pty v1.1.24 diff --git a/internal/api/api.go b/internal/api/api.go index a4095c2..d4d5fb4 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -16,6 +16,7 @@ import ( "path/filepath" "runtime" "sort" + "strconv" "strings" "sync" "time" @@ -48,6 +49,7 @@ const ( maxInlineSFTPUploadBytes = 25 * 1024 * 1024 maxSFTPUploadChunkBytes = 1024 * 1024 maxStreamingSFTPUploadSize = int64(2 * 1024 * 1024 * 1024) + maxConcurrentSFTPUploads = 16 maxSSHPublicKeyBytes = 64 * 1024 sshKeygenTimeout = 30 * time.Second ) @@ -64,8 +66,10 @@ type Service struct { eventSink func(string, interface{}) // secretChans maps request IDs to channels awaiting user secret input. - secretMu sync.Mutex - secretChans map[string]chan secretResponse + secretMu sync.Mutex + secretChans map[string]chan secretResponse + secretListenerRegistered bool + secretListenerCancel func() uploadMu sync.Mutex uploads map[string]*sftpUpload @@ -108,6 +112,30 @@ func (s *Service) SetContext(ctx context.Context) { s.RegisterSecretResponseListener() } +// Shutdown releases active uploads, prompt waiters, and app-wide Wails +// listeners. It is safe to call when no Wails context was configured. +func (s *Service) Shutdown() { + s.cancelSFTPUploads("") + + s.secretMu.Lock() + cancel := s.secretListenerCancel + s.secretListenerCancel = nil + s.secretListenerRegistered = false + waiters := s.secretChans + s.secretChans = make(map[string]chan secretResponse) + s.secretMu.Unlock() + + if cancel != nil { + cancel() + } + for _, ch := range waiters { + select { + case ch <- secretResponse{cancelled: true}: + default: + } + } +} + // SetEventSink registers an alternate UI event sink for non-Wails hosts such // as the VS Code bridge. func (s *Service) SetEventSink(sink func(string, interface{})) { @@ -438,11 +466,13 @@ func (s *Service) RestartLocalSession(sessionID string, cols int, rows int) erro // DisconnectSession closes a session by ID. func (s *Service) DisconnectSession(sessionID string) error { + s.cancelSFTPUploads(sessionID) return s.sessions.Disconnect(sessionID) } // ReconnectSession closes and re-establishes a session. func (s *Service) ReconnectSession(sessionID string) error { + s.cancelSFTPUploads(sessionID) return s.sessions.Reconnect(sessionID, s.promptSecret) } @@ -482,6 +512,7 @@ func (s *Service) PinSession(sessionID string, pinned bool) error { // CloseAllSessions terminates all active sessions. func (s *Service) CloseAllSessions() { + s.cancelSFTPUploads("") s.sessions.CloseAll() } @@ -1538,6 +1569,9 @@ func (s *Service) SFTPUploadStart(sessionID string, remoteDir string, fileName s if totalBytes > maxStreamingSFTPUploadSize { return "", fmt.Errorf("upload is too large (max %d bytes)", maxStreamingSFTPUploadSize) } + if err := s.checkSFTPUploadCapacity(); err != nil { + return "", err + } safeName, err := safeSFTPUploadName(fileName) if err != nil { @@ -1551,6 +1585,14 @@ func (s *Service) SFTPUploadStart(sessionID string, remoteDir string, fileName s uploadID := uuid.New().String() s.uploadMu.Lock() + if len(s.uploads) >= maxConcurrentSFTPUploads { + s.uploadMu.Unlock() + _ = writer.Close() + if s.sessions != nil { + _ = s.sessions.SFTPRemove(sessionID, remotePath) + } + return "", fmt.Errorf("too many active uploads (max %d)", maxConcurrentSFTPUploads) + } s.uploads[uploadID] = &sftpUpload{ writer: writer, sessionID: sessionID, @@ -1716,7 +1758,7 @@ func (s *Service) TestConnection(host string, port int) (map[string]interface{}, return nil, fmt.Errorf("port must be between 1 and 65535") } - addr := fmt.Sprintf("%s:%d", host, port) + addr := net.JoinHostPort(host, strconv.Itoa(port)) start := time.Now() conn, err := net.DialTimeout("tcp", addr, 10*time.Second) @@ -1798,7 +1840,16 @@ func (s *Service) RegisterSecretResponseListener() { if s.ctx == nil { return } - wailsRuntime.EventsOn(s.ctx, "session:secret:response", func(optionalData ...interface{}) { + + s.secretMu.Lock() + if s.secretListenerRegistered { + s.secretMu.Unlock() + return + } + s.secretListenerRegistered = true + s.secretMu.Unlock() + + cancel := wailsRuntime.EventsOn(s.ctx, "session:secret:response", func(optionalData ...interface{}) { if len(optionalData) == 0 { return } @@ -1815,9 +1866,15 @@ func (s *Service) RegisterSecretResponseListener() { s.secretMu.Unlock() if exists { - ch <- secretResponse{value: value, cancelled: cancelled} + select { + case ch <- secretResponse{value: value, cancelled: cancelled}: + default: + } } }) + s.secretMu.Lock() + s.secretListenerCancel = cancel + s.secretMu.Unlock() } // RespondSecret supplies a credential prompt response for non-Wails hosts. @@ -1827,7 +1884,10 @@ func (s *Service) RespondSecret(requestID string, value string, cancelled bool) s.secretMu.Unlock() if exists { - ch <- secretResponse{value: value, cancelled: cancelled} + select { + case ch <- secretResponse{value: value, cancelled: cancelled}: + default: + } } } @@ -2127,6 +2187,39 @@ func (s *Service) removeSFTPUpload(uploadID string) { delete(s.uploads, uploadID) } +func (s *Service) checkSFTPUploadCapacity() error { + s.uploadMu.Lock() + defer s.uploadMu.Unlock() + if len(s.uploads) >= maxConcurrentSFTPUploads { + return fmt.Errorf("too many active uploads (max %d)", maxConcurrentSFTPUploads) + } + return nil +} + +// cancelSFTPUploads closes and removes active uploads. An empty session ID +// cancels every upload; otherwise only uploads owned by that session are +// affected. +func (s *Service) cancelSFTPUploads(sessionID string) { + s.uploadMu.Lock() + uploads := make([]*sftpUpload, 0) + for uploadID, upload := range s.uploads { + if sessionID == "" || upload.sessionID == sessionID { + delete(s.uploads, uploadID) + uploads = append(uploads, upload) + } + } + s.uploadMu.Unlock() + + for _, upload := range uploads { + upload.mu.Lock() + _ = upload.writer.Close() + upload.mu.Unlock() + if s.sessions != nil { + _ = s.sessions.SFTPRemove(upload.sessionID, upload.remotePath) + } + } +} + func validateImportStrategy(strategy string) error { switch strategy { case "skip", "overwrite", "duplicate": diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 2ecc64a..e21e54d 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -13,6 +13,19 @@ import ( "github.com/andydixon/nexus/internal/storage" ) +type closeTrackingWriter struct { + closed int +} + +func (w *closeTrackingWriter) Write(p []byte) (int, error) { + return len(p), nil +} + +func (w *closeTrackingWriter) Close() error { + w.closed++ + return nil +} + func TestSSHKeyPairPathsAcceptsSafeName(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -128,6 +141,65 @@ func TestSFTPUploadChunkRejectsOversizedChunkBeforeUploadLookup(t *testing.T) { } } +func TestCancelSFTPUploadsClosesOnlyMatchingSession(t *testing.T) { + first := &closeTrackingWriter{} + second := &closeTrackingWriter{} + svc := &Service{ + uploads: map[string]*sftpUpload{ + "first": {writer: first, sessionID: "session-1", remotePath: "/first"}, + "second": {writer: second, sessionID: "session-2", remotePath: "/second"}, + }, + } + + svc.cancelSFTPUploads("session-1") + + if first.closed != 1 { + t.Fatalf("matching writer closed %d times, want 1", first.closed) + } + if second.closed != 0 { + t.Fatalf("unrelated writer closed %d times, want 0", second.closed) + } + if len(svc.uploads) != 1 || svc.uploads["second"] == nil { + t.Fatalf("remaining uploads = %#v, want only session-2", svc.uploads) + } +} + +func TestSFTPUploadCapacityIsBounded(t *testing.T) { + svc := &Service{uploads: make(map[string]*sftpUpload)} + for i := 0; i < maxConcurrentSFTPUploads; i++ { + svc.uploads[string(rune('a'+i))] = &sftpUpload{writer: &closeTrackingWriter{}} + } + if err := svc.checkSFTPUploadCapacity(); err == nil { + t.Fatal("expected active upload limit to be enforced") + } +} + +func TestServiceShutdownReleasesUploadsAndSecretWaiters(t *testing.T) { + writer := &closeTrackingWriter{} + waiter := make(chan secretResponse, 1) + svc := &Service{ + uploads: map[string]*sftpUpload{"upload": {writer: writer}}, + secretChans: map[string]chan secretResponse{"prompt": waiter}, + } + + svc.Shutdown() + + if writer.closed != 1 { + t.Fatalf("writer closed %d times, want 1", writer.closed) + } + if len(svc.uploads) != 0 { + t.Fatalf("active uploads = %d, want 0", len(svc.uploads)) + } + select { + case response := <-waiter: + if !response.cancelled { + t.Fatal("shutdown response was not cancelled") + } + default: + t.Fatal("shutdown did not release secret prompt waiter") + } +} + func TestSafeSFTPUploadNamePreservesHTMLCharacters(t *testing.T) { got, err := safeSFTPUploadName(`../report .txt`) if err != nil { diff --git a/internal/app/app.go b/internal/app/app.go index 7f136c4..2824c5f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -118,8 +118,12 @@ func (a *App) Shutdown(ctx context.Context) { a.mu.Lock() defer a.mu.Unlock() + if a.Service != nil { + a.Service.Shutdown() + } + if a.SessionManager != nil { - a.SessionManager.CloseAll() + a.SessionManager.Shutdown() } if a.Store != nil { diff --git a/internal/exportimport/exportimport.go b/internal/exportimport/exportimport.go index dc7cdcf..9364ff5 100644 --- a/internal/exportimport/exportimport.go +++ b/internal/exportimport/exportimport.go @@ -68,6 +68,7 @@ func EncryptAndExport(profiles []models.Profile, password, filePath string) erro if err != nil { return fmt.Errorf("exportimport: failed to serialise profiles: %w", err) } + defer clear(plaintext) // Generate random salt and nonce. salt := make([]byte, saltLength) @@ -81,14 +82,17 @@ func EncryptAndExport(profiles []models.Profile, password, filePath string) erro } // Derive encryption key from password. + passwordBytes := []byte(password) + defer clear(passwordBytes) key := argon2.IDKey( - []byte(password), + passwordBytes, salt, exportArgonTime, exportArgonMemory, exportArgonThreads, exportArgonKeyLen, ) + defer clear(key) // Encrypt with AES-256-GCM. block, err := aes.NewCipher(key) @@ -104,11 +108,14 @@ func EncryptAndExport(profiles []models.Profile, password, filePath string) erro ciphertext := gcm.Seal(nil, nonce, plaintext, nil) // Write the file: version + salt + nonce + ciphertext. - f, err := os.Create(filePath) + f, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) if err != nil { return fmt.Errorf("exportimport: failed to create file: %w", err) } defer f.Close() + if err := f.Chmod(0600); err != nil { + return fmt.Errorf("exportimport: failed to restrict file permissions: %w", err) + } if _, err := f.Write([]byte{formatVersion}); err != nil { return fmt.Errorf("exportimport: write version failed: %w", err) @@ -163,14 +170,17 @@ func DecryptAndImport(filePath, password string) ([]models.Profile, error) { ciphertext := data[1+saltLength+nonceLength:] // Derive decryption key. + passwordBytes := []byte(password) + defer clear(passwordBytes) key := argon2.IDKey( - []byte(password), + passwordBytes, salt, exportArgonTime, exportArgonMemory, exportArgonThreads, exportArgonKeyLen, ) + defer clear(key) // Decrypt with AES-256-GCM. block, err := aes.NewCipher(key) @@ -187,6 +197,7 @@ func DecryptAndImport(filePath, password string) ([]models.Profile, error) { if err != nil { return nil, errors.New("exportimport: decryption failed — wrong password or corrupt file") } + defer clear(plaintext) // Parse the JSON bundle. var bundle ExportBundle diff --git a/internal/exportimport/exportimport_test.go b/internal/exportimport/exportimport_test.go index d464b6d..9f11255 100644 --- a/internal/exportimport/exportimport_test.go +++ b/internal/exportimport/exportimport_test.go @@ -3,6 +3,7 @@ package exportimport import ( "os" "path/filepath" + "runtime" "testing" "github.com/andydixon/nexus/internal/models" @@ -104,6 +105,26 @@ func TestEncryptEmptyProfiles(t *testing.T) { } } +func TestEncryptAndExportRestrictsExistingFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permission bits are not enforced on Windows") + } + path := filepath.Join(t.TempDir(), "profiles.nexus") + if err := os.WriteFile(path, []byte("old"), 0644); err != nil { + t.Fatal(err) + } + if err := EncryptAndExport([]models.Profile{{ID: "1"}}, "password", path); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0600 { + t.Fatalf("export permissions = %04o, want 0600", got) + } +} + func TestEncryptEmptyPassword(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "test.nexus") diff --git a/internal/ircclient/ircclient.go b/internal/ircclient/ircclient.go index 06357ed..3660584 100644 --- a/internal/ircclient/ircclient.go +++ b/internal/ircclient/ircclient.go @@ -608,7 +608,7 @@ func (c *Client) newSSHDialer() (*sshDialer, error) { if err != nil { return nil, err } - sshAddr := fmt.Sprintf("%s:%d", c.cfg.SSHHost, c.cfg.SSHPort) + sshAddr := net.JoinHostPort(c.cfg.SSHHost, strconv.Itoa(c.cfg.SSHPort)) sshConfig := &ssh.ClientConfig{ User: c.cfg.SSHUsername, Auth: authMethods, diff --git a/internal/leviathan/binsnoop/binsnoop.go b/internal/leviathan/binsnoop/binsnoop.go index 7f79085..3c38222 100644 --- a/internal/leviathan/binsnoop/binsnoop.go +++ b/internal/leviathan/binsnoop/binsnoop.go @@ -134,11 +134,11 @@ func CarveResult(data []byte, result SignatureResult, outputDir string) error { if result.Offset < 0 || result.Offset+result.Size > len(data) { return fmt.Errorf("binsnoop: result offset/size exceeds data bounds") } - if err := os.MkdirAll(outputDir, 0755); err != nil { + if err := os.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("binsnoop: mkdir: %w", err) } outPath := filepath.Join(outputDir, fmt.Sprintf("%x_%s.bin", result.Offset, result.Name)) - return os.WriteFile(outPath, data[result.Offset:result.Offset+result.Size], 0644) + return os.WriteFile(outPath, data[result.Offset:result.Offset+result.Size], 0600) } // SignatureCount returns the number of loaded signatures. diff --git a/internal/leviathan/binsnoop/common_test.go b/internal/leviathan/binsnoop/common_test.go index e3a933c..d686b0c 100644 --- a/internal/leviathan/binsnoop/common_test.go +++ b/internal/leviathan/binsnoop/common_test.go @@ -3,6 +3,7 @@ package binsnoop import ( "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -20,3 +21,27 @@ func TestReadFileLimitedRejectsOversizedFile(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +func TestCarveUsesPrivatePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permission bits are not enforced on Windows") + } + path := filepath.Join(t.TempDir(), "private", "carved.bin") + if err := Carve(path, []byte("secret"), 0, 6); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0600 { + t.Fatalf("carved file permissions = %04o, want 0600", got) + } + dirInfo, err := os.Stat(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if got := dirInfo.Mode().Perm(); got != 0700 { + t.Fatalf("carve directory permissions = %04o, want 0700", got) + } +} diff --git a/internal/leviathan/binsnoop/extract.go b/internal/leviathan/binsnoop/extract.go index 5891729..4e0318a 100644 --- a/internal/leviathan/binsnoop/extract.go +++ b/internal/leviathan/binsnoop/extract.go @@ -14,10 +14,10 @@ func Carve(outputPath string, data []byte, offset, size int) error { return fmt.Errorf("carve: offset %d + size %d exceeds data length %d", offset, size, len(data)) } dir := filepath.Dir(outputPath) - if err := os.MkdirAll(dir, 0755); err != nil { + if err := os.MkdirAll(dir, 0700); err != nil { return fmt.Errorf("carve: mkdir: %w", err) } - return os.WriteFile(outputPath, data[offset:offset+size], 0644) + return os.WriteFile(outputPath, data[offset:offset+size], 0600) } // executeExternal runs an external extraction tool. @@ -27,7 +27,7 @@ func executeExternal(ext *Extractor, carvedFilePath, outputDir string) *Extracti OutputDirectory: outputDir, } - if err := os.MkdirAll(outputDir, 0755); err != nil { + if err := os.MkdirAll(outputDir, 0700); err != nil { return result } @@ -100,7 +100,7 @@ func ExecuteExtraction(data []byte, filePath string, sig *SignatureResult, ext * case ExtractorTypeExternal: // Carve data to a temp file for external tools - if err := os.MkdirAll(outputDir, 0755); err != nil { + if err := os.MkdirAll(outputDir, 0700); err != nil { return &ExtractionResult{OutputDirectory: outputDir} } @@ -109,7 +109,7 @@ func ExecuteExtraction(data []byte, filePath string, sig *SignatureResult, ext * if endOffset > len(data) { endOffset = len(data) } - if err := os.WriteFile(carvedFile, data[sig.Offset:endOffset], 0644); err != nil { + if err := os.WriteFile(carvedFile, data[sig.Offset:endOffset], 0600); err != nil { return &ExtractionResult{OutputDirectory: outputDir} } diff --git a/internal/sessions/manager.go b/internal/sessions/manager.go index dbbbed9..d31afc1 100644 --- a/internal/sessions/manager.go +++ b/internal/sessions/manager.go @@ -82,8 +82,16 @@ type Manager struct { hostKeyMu sync.Mutex hostKeyChans map[string]chan bool hostKeyListenerRegistered bool + hostKeyListenerCancel func() secretMu sync.Mutex - secretChans map[string]chan string + secretChans map[string]chan secretPromptResponse + secretListenerRegistered bool + secretListenerCancel func() +} + +type secretPromptResponse struct { + value string + cancelled bool } // NewManager creates a Manager wired to the given backing services. @@ -102,6 +110,7 @@ func NewManager(store *storage.Store, secretStore *secrets.SecretStore, keychain func (m *Manager) SetContext(ctx context.Context) { m.ctx = ctx m.registerHostKeyResponseListener() + m.registerSecretResponseListener() } // SetEventSink registers an alternate UI event sink for non-Wails hosts. @@ -662,6 +671,46 @@ func (m *Manager) CloseAll() { } } +// Shutdown closes all sessions, unregisters app-wide Wails listeners, and +// releases any prompts still waiting for frontend input. +func (m *Manager) Shutdown() { + m.CloseAll() + + m.hostKeyMu.Lock() + hostKeyCancel := m.hostKeyListenerCancel + m.hostKeyListenerCancel = nil + m.hostKeyListenerRegistered = false + hostKeyChans := m.hostKeyChans + m.hostKeyChans = make(map[string]chan bool) + m.hostKeyMu.Unlock() + if hostKeyCancel != nil { + hostKeyCancel() + } + for _, ch := range hostKeyChans { + select { + case ch <- false: + default: + } + } + + m.secretMu.Lock() + secretCancel := m.secretListenerCancel + m.secretListenerCancel = nil + m.secretListenerRegistered = false + secretChans := m.secretChans + m.secretChans = make(map[string]chan secretPromptResponse) + m.secretMu.Unlock() + if secretCancel != nil { + secretCancel() + } + for _, ch := range secretChans { + select { + case ch <- secretPromptResponse{cancelled: true}: + default: + } + } +} + // GetSessionHistory returns the RAW session history (RAW protocol only). func (m *Manager) GetSessionHistory(sessionID string) ([]rawclient.HistoryEntry, error) { m.mu.RLock() @@ -1990,7 +2039,10 @@ func (m *Manager) registerHostKeyResponseListener() { m.hostKeyListenerRegistered = true m.hostKeyMu.Unlock() - wailsRuntime.EventsOn(ctx, "session:hostkey:response", m.handleHostKeyResponse) + cancel := wailsRuntime.EventsOn(ctx, "session:hostkey:response", m.handleHostKeyResponse) + m.hostKeyMu.Lock() + m.hostKeyListenerCancel = cancel + m.hostKeyMu.Unlock() } func (m *Manager) handleHostKeyResponse(optionalData ...interface{}) { @@ -2467,13 +2519,13 @@ func (m *Manager) scheduleReconnect(sess *Session, sessionID string) { var err error switch sess.profile.Protocol { case models.ProtocolSSH: - err = m.connectSSH(sess, m.buildInteractiveSecretCallback(sessionID)) + err = m.connectSSH(sess, m.buildInteractiveSecretCallback(ctx, sessionID)) case models.ProtocolTelnet: err = m.connectTelnet(sess) case models.ProtocolRAW: err = m.connectRAW(sess) case models.ProtocolIRC: - err = m.connectIRC(sess, m.buildInteractiveSecretCallback(sessionID)) + err = m.connectIRC(sess, m.buildInteractiveSecretCallback(ctx, sessionID)) default: err = fmt.Errorf("unsupported protocol for reconnect: %s", sess.profile.Protocol) } @@ -2522,14 +2574,14 @@ func (m *Manager) scheduleReconnect(sess *Session, sessionID string) { // buildInteractiveSecretCallback creates a getSecret callback that prompts // the user via Wails events. Used during auto-reconnect flows where the // original callback is no longer available. -func (m *Manager) buildInteractiveSecretCallback(sessionID string) func(prompt string) (string, error) { +func (m *Manager) buildInteractiveSecretCallback(ctx context.Context, sessionID string) func(prompt string) (string, error) { return func(prompt string) (string, error) { requestID := uuid.New().String() - ch := make(chan string, 1) + ch := make(chan secretPromptResponse, 1) m.secretMu.Lock() if m.secretChans == nil { - m.secretChans = make(map[string]chan string) + m.secretChans = make(map[string]chan secretPromptResponse) } m.secretChans[requestID] = ch m.secretMu.Unlock() @@ -2546,48 +2598,67 @@ func (m *Manager) buildInteractiveSecretCallback(sessionID string) func(prompt s "message": prompt, }) - // Wait for the response with a timeout. - var response string - - if m.ctx != nil { - wailsRuntime.EventsOn(m.ctx, "session:secret:response", func(optionalData ...interface{}) { - if len(optionalData) == 0 { - return - } - data, ok := optionalData[0].(map[string]interface{}) - if !ok { - return - } - if rid, _ := data["requestID"].(string); rid == requestID { - val, _ := data["value"].(string) - select { - case ch <- val: - default: - } - } - }) - } - select { - case response = <-ch: + case response := <-ch: + if response.cancelled { + return "", fmt.Errorf("credential prompt cancelled during reconnect") + } + return response.value, nil + case <-ctx.Done(): + return "", fmt.Errorf("credential prompt cancelled during reconnect: %w", ctx.Err()) case <-time.After(60 * time.Second): return "", fmt.Errorf("credential prompt timed out during reconnect") } - - return response, nil } } +func (m *Manager) registerSecretResponseListener() { + ctx := m.ctx + if ctx == nil { + return + } + + m.secretMu.Lock() + if m.secretChans == nil { + m.secretChans = make(map[string]chan secretPromptResponse) + } + if m.secretListenerRegistered { + m.secretMu.Unlock() + return + } + m.secretListenerRegistered = true + m.secretMu.Unlock() + + cancel := wailsRuntime.EventsOn(ctx, "session:secret:response", m.handleSecretResponse) + m.secretMu.Lock() + m.secretListenerCancel = cancel + m.secretMu.Unlock() +} + +func (m *Manager) handleSecretResponse(optionalData ...interface{}) { + if len(optionalData) == 0 { + return + } + data, ok := optionalData[0].(map[string]interface{}) + if !ok { + return + } + requestID, _ := data["requestID"].(string) + value, _ := data["value"].(string) + cancelled, _ := data["cancelled"].(bool) + m.RespondSecret(requestID, value, cancelled) +} + // RespondSecret supplies a credential prompt response for reconnect prompts in // non-Wails hosts. -func (m *Manager) RespondSecret(requestID string, value string) { +func (m *Manager) RespondSecret(requestID string, value string, cancelled bool) { m.secretMu.Lock() ch, exists := m.secretChans[requestID] m.secretMu.Unlock() if exists { select { - case ch <- value: + case ch <- secretPromptResponse{value: value, cancelled: cancelled}: default: } } diff --git a/internal/sessions/manager_test.go b/internal/sessions/manager_test.go index ca76215..3c236de 100644 --- a/internal/sessions/manager_test.go +++ b/internal/sessions/manager_test.go @@ -1,6 +1,7 @@ package sessions import ( + "context" "crypto/ed25519" "crypto/rand" "encoding/base64" @@ -377,6 +378,101 @@ func TestHandleHostKeyResponseIgnoresOtherRequestIDs(t *testing.T) { } } +func TestHandleSecretResponseRoutesByRequestID(t *testing.T) { + mgr := NewManager(nil, nil, nil, nil, nil) + ch := make(chan secretPromptResponse, 1) + mgr.secretChans = map[string]chan secretPromptResponse{"request-1": ch} + + mgr.handleSecretResponse(map[string]interface{}{ + "requestID": "request-1", + "value": "secret", + "cancelled": false, + }) + + select { + case response := <-ch: + if response.value != "secret" || response.cancelled { + t.Fatalf("response = %#v, want non-cancelled secret", response) + } + default: + t.Fatal("expected secret response to be routed") + } +} + +func TestInteractiveSecretPromptIsReleasedByReconnectCancellation(t *testing.T) { + mgr := NewManager(nil, nil, nil, nil, nil) + prompts := make(chan map[string]interface{}, 1) + mgr.SetEventSink(func(name string, data interface{}) { + if name == "session:secret:prompt" { + prompts <- data.(map[string]interface{}) + } + }) + + ctx, cancel := context.WithCancel(context.Background()) + result := make(chan error, 1) + go func() { + _, err := mgr.buildInteractiveSecretCallback(ctx, "session-1")("Password") + result <- err + }() + + select { + case <-prompts: + case <-time.After(time.Second): + t.Fatal("timed out waiting for secret prompt") + } + cancel() + + select { + case err := <-result: + if err == nil || !strings.Contains(err.Error(), "cancelled") { + t.Fatalf("callback error = %v, want cancellation", err) + } + case <-time.After(time.Second): + t.Fatal("secret prompt did not stop after reconnect cancellation") + } + + mgr.secretMu.Lock() + pending := len(mgr.secretChans) + mgr.secretMu.Unlock() + if pending != 0 { + t.Fatalf("pending secret prompts = %d, want 0", pending) + } +} + +func TestInteractiveSecretPromptHonoursFrontendCancellation(t *testing.T) { + mgr := NewManager(nil, nil, nil, nil, nil) + prompts := make(chan map[string]interface{}, 1) + mgr.SetEventSink(func(name string, data interface{}) { + if name == "session:secret:prompt" { + prompts <- data.(map[string]interface{}) + } + }) + + result := make(chan error, 1) + go func() { + _, err := mgr.buildInteractiveSecretCallback(context.Background(), "session-1")("Password") + result <- err + }() + + var requestID string + select { + case prompt := <-prompts: + requestID, _ = prompt["requestID"].(string) + case <-time.After(time.Second): + t.Fatal("timed out waiting for secret prompt") + } + mgr.RespondSecret(requestID, "", true) + + select { + case err := <-result: + if err == nil || !strings.Contains(err.Error(), "cancelled") { + t.Fatalf("callback error = %v, want frontend cancellation", err) + } + case <-time.After(time.Second): + t.Fatal("secret prompt did not stop after frontend cancellation") + } +} + func testPublicKey(t *testing.T) ssh.PublicKey { t.Helper() pub, _, err := ed25519.GenerateKey(rand.Reader) diff --git a/internal/vncclient/vncclient.go b/internal/vncclient/vncclient.go index 3bf069e..ce54347 100644 --- a/internal/vncclient/vncclient.go +++ b/internal/vncclient/vncclient.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/url" + "strconv" "strings" "sync" "time" @@ -85,11 +86,11 @@ func (c *Client) Connect(cfg Config) (string, error) { var vncConn net.Conn var err error - vncAddr := fmt.Sprintf("%s:%d", cfg.VNCHost, cfg.VNCPort) + vncAddr := net.JoinHostPort(cfg.VNCHost, strconv.Itoa(cfg.VNCPort)) if cfg.SSHTunnel { // Establish SSH connection first - sshAddr := fmt.Sprintf("%s:%d", cfg.SSHHost, cfg.SSHPort) + sshAddr := net.JoinHostPort(cfg.SSHHost, strconv.Itoa(cfg.SSHPort)) if cfg.SSHHostKeyCallback == nil { return "", fmt.Errorf("SSH host key callback is required") }