diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml new file mode 100644 index 0000000..d12a903 --- /dev/null +++ b/.github/workflows/go.yml @@ -0,0 +1,39 @@ +name: Go Build and Test + +on: + pull_request: + branches: + - main + types: + - edited + - opened + - reopened + - synchronize + push: + branches: + - main + +permissions: + contents: read + pull-requests: read + +jobs: + build-and-test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache-dependency-path: go.sum + cache: true + + - name: Run make build + run: make build + + - name: Run make test + run: make test \ No newline at end of file diff --git a/.github/workflows/semantic-prs.yml b/.github/workflows/semantic-prs.yml new file mode 100644 index 0000000..78d04fe --- /dev/null +++ b/.github/workflows/semantic-prs.yml @@ -0,0 +1,43 @@ +name: Semantic PRs + +on: + pull_request: + types: + - edited + - opened + - reopened + - synchronize + +permissions: + pull-requests: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event.number }} + cancel-in-progress: true + +jobs: + validate_title: + name: Validate Title + runs-on: ubuntu-latest + steps: + - uses: amannn/action-semantic-pull-request@v5.5.3 + env: + GITHUB_TOKEN: ${{ secrets.GH_TOKEN }} + with: + types: | + fix + feat + improve + refactor + revert + test + ci + docs + chore + + scopes: | + ui + cli + config + parser + requireScope: false \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml index e44d3ac..b723bc9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,7 +14,14 @@ issues: - dupl - lll - + # exclude some linters for the test directory and test files + - path: test/.*|.*_test\.go + linters: + - dupl + - errcheck + - goconst + - gocyclo + - gosec linters: disable-all: true diff --git a/README.md b/README.md index 8b05377..4b50d7f 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,35 @@ Contributions are welcome! We love seeing the community make Lazyssh better 🚀 +### Semantic Pull Requests + +This repository enforces semantic PR titles via an automated GitHub Action. Please format your PR title as: + +- type(scope): short descriptive subject +Notes: +- Scope is optional and should be one of: ui, cli, config, parser. + +Allowed types in this repo: +- feat: a new feature +- fix: a bug fix +- improve: quality or UX improvements that are not a refactor or perf +- refactor: code change that neither fixes a bug nor adds a feature +- docs: documentation only changes +- test: adding or refactoring tests +- ci: CI/CD or automation changes +- chore: maintenance tasks, dependency bumps, non-code infra +- revert: reverts a previous commit + +Examples: +- feat(ui): add server pinning and sorting options +- fix(parser): handle comments at end of Host blocks +- improve(cli): show friendly error when ssh binary missing +- refactor(config): simplify backup rotation logic +- docs: add installation instructions for Homebrew +- ci: cache Go toolchain and dependencies + +Tip: If your PR touches multiple areas, pick the most relevant scope or omit the scope. + --- ## ⭐ Support diff --git a/internal/adapters/ui/validation_test.go b/internal/adapters/ui/validation_test.go index 9968372..de33639 100644 --- a/internal/adapters/ui/validation_test.go +++ b/internal/adapters/ui/validation_test.go @@ -15,6 +15,8 @@ package ui import ( + "os" + "path/filepath" "testing" ) @@ -134,6 +136,29 @@ func TestValidateBindAddress(t *testing.T) { } func TestValidateKeyPaths(t *testing.T) { + // Prepare an isolated HOME with a mock .ssh folder and key files + oldHome := os.Getenv("HOME") + t.Cleanup(func() { + _ = os.Setenv("HOME", oldHome) + }) + + tempHome := t.TempDir() + sshDir := filepath.Join(tempHome, ".ssh") + if err := os.MkdirAll(sshDir, 0o755); err != nil { + t.Fatalf("failed to create temp .ssh dir: %v", err) + } + + shouldExistFiles := []string{"id_rsa", "id_ed25519"} + for _, name := range shouldExistFiles { + p := filepath.Join(sshDir, name) + if err := os.WriteFile(p, []byte("test"), 0o644); err != nil { + t.Fatalf("failed to create mock key file %s: %v", p, err) + } + } + if err := os.Setenv("HOME", tempHome); err != nil { + t.Fatalf("failed to set HOME: %v", err) + } + tests := []struct { name string keys string diff --git a/internal/core/services/server_service.go b/internal/core/services/server_service.go index c01b18b..b43c9f5 100644 --- a/internal/core/services/server_service.go +++ b/internal/core/services/server_service.go @@ -16,7 +16,10 @@ package services import ( "bufio" + "bytes" + "errors" "fmt" + "io" "net" "os" "os/exec" @@ -34,6 +37,7 @@ import ( type serverService struct { serverRepository ports.ServerRepository logger *zap.SugaredLogger + newSSHCommand func(alias string) *exec.Cmd } // NewServerService creates a new instance of serverService. @@ -41,6 +45,9 @@ func NewServerService(logger *zap.SugaredLogger, sr ports.ServerRepository) port return &serverService{ logger: logger, serverRepository: sr, + newSSHCommand: func(alias string) *exec.Cmd { + return exec.Command("ssh", alias) + }, } } @@ -151,13 +158,23 @@ func (s *serverService) SetPinned(alias string, pinned bool) error { // SSH starts an interactive SSH session to the given alias using the system's ssh client. func (s *serverService) SSH(alias string) error { s.logger.Infow("ssh start", "alias", alias) - cmd := exec.Command("ssh", alias) + cmd := s.newSSHCommand(alias) + if cmd == nil { + err := fmt.Errorf("ssh command factory returned nil") + s.logger.Errorw("ssh command creation failed", "alias", alias, "error", err) + return err + } + stderrBuf := newLimitedBuffer(2048) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + cmd.Stderr = io.MultiWriter(os.Stderr, stderrBuf) if err := cmd.Run(); err != nil { - s.logger.Errorw("ssh command failed", "alias", alias, "error", err) - return err + if isRemoteDisconnectError(err, stderrBuf.String()) { + s.logger.Infow("ssh session ended by remote", "alias", alias) + } else { + s.logger.Errorw("ssh command failed", "alias", alias, "error", err) + return err + } } if err := s.serverRepository.RecordSSH(alias); err != nil { @@ -168,6 +185,63 @@ func (s *serverService) SSH(alias string) error { return nil } +func isRemoteDisconnectError(err error, stderr string) bool { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + return false + } + + lower := strings.ToLower(stderr) + disconnectSignals := []string{ + "connection closed by remote host", + "connection closed by foreign host", + "closed by remote host", + "closed by foreign host", + "connection reset by peer", + } + + for _, signal := range disconnectSignals { + if strings.Contains(lower, signal) { + return true + } + } + + return false +} + +type limitedBuffer struct { + buf bytes.Buffer + limit int +} + +func newLimitedBuffer(limit int) *limitedBuffer { + return &limitedBuffer{limit: limit} +} + +func (b *limitedBuffer) Write(p []byte) (int, error) { + if b == nil || b.limit <= 0 { + return len(p), nil + } + + remaining := b.limit - b.buf.Len() + if remaining <= 0 { + return len(p), nil + } + + if len(p) > remaining { + p = p[:remaining] + } + + return b.buf.Write(p) +} + +func (b *limitedBuffer) String() string { + if b == nil { + return "" + } + return b.buf.String() +} + // Ping checks if the server is reachable on its SSH port. func (s *serverService) Ping(server domain.Server) (bool, time.Duration, error) { start := time.Now() diff --git a/internal/core/services/server_service_test.go b/internal/core/services/server_service_test.go new file mode 100644 index 0000000..56ee387 --- /dev/null +++ b/internal/core/services/server_service_test.go @@ -0,0 +1,191 @@ +package services + +import ( + "errors" + "io" + "os" + "os/exec" + "testing" + + "github.com/Adembc/lazyssh/internal/core/domain" + "github.com/Adembc/lazyssh/internal/core/ports" + "go.uber.org/zap/zaptest" +) + +type mockServerRepository struct { + ports.ServerRepository + recordCalls int + lastAlias string + recordErr error +} + +func (m *mockServerRepository) ListServers(string) ([]domain.Server, error) { + return nil, nil +} + +func (m *mockServerRepository) UpdateServer(domain.Server, domain.Server) error { return nil } + +func (m *mockServerRepository) AddServer(domain.Server) error { return nil } + +func (m *mockServerRepository) DeleteServer(domain.Server) error { return nil } + +func (m *mockServerRepository) SetPinned(string, bool) error { return nil } + +func (m *mockServerRepository) RecordSSH(alias string) error { + m.recordCalls++ + m.lastAlias = alias + return m.recordErr +} + +func helperCommandFactory(scenario string) func(string) *exec.Cmd { + return func(alias string) *exec.Cmd { + cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--", scenario, alias) + cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1") + cmd.Stdout = io.Discard + cmd.Stderr = io.Discard + return cmd + } +} + +func TestServerServiceSSH_RemoteDisconnect(t *testing.T) { + repo := &mockServerRepository{} + svc := &serverService{ + logger: zaptest.NewLogger(t).Sugar(), + serverRepository: repo, + newSSHCommand: helperCommandFactory("remote"), + } + + if err := svc.SSH("example"); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + if repo.recordCalls != 1 { + t.Fatalf("expected RecordSSH to be called once, got %d", repo.recordCalls) + } +} + +func TestServerServiceSSH_ConnectionReset(t *testing.T) { + repo := &mockServerRepository{} + svc := &serverService{ + logger: zaptest.NewLogger(t).Sugar(), + serverRepository: repo, + newSSHCommand: helperCommandFactory("reset"), + } + + if err := svc.SSH("example"); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + if repo.recordCalls != 1 { + t.Fatalf("expected RecordSSH to be called once, got %d", repo.recordCalls) + } +} + +func TestServerServiceSSH_PermissionDenied(t *testing.T) { + repo := &mockServerRepository{} + svc := &serverService{ + logger: zaptest.NewLogger(t).Sugar(), + serverRepository: repo, + newSSHCommand: helperCommandFactory("permission"), + } + + if err := svc.SSH("example"); err == nil { + t.Fatalf("expected error, got nil") + } + + if repo.recordCalls != 0 { + t.Fatalf("expected RecordSSH not to be called, got %d", repo.recordCalls) + } +} + +func TestServerServiceSSH_Success(t *testing.T) { + repo := &mockServerRepository{} + svc := &serverService{ + logger: zaptest.NewLogger(t).Sugar(), + serverRepository: repo, + newSSHCommand: helperCommandFactory("success"), + } + + if err := svc.SSH("example"); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + if repo.recordCalls != 1 { + t.Fatalf("expected RecordSSH to be called once, got %d", repo.recordCalls) + } +} + +func TestServerServiceSSH_CommandFactoryReturnsNil(t *testing.T) { + repo := &mockServerRepository{} + svc := &serverService{ + logger: zaptest.NewLogger(t).Sugar(), + serverRepository: repo, + newSSHCommand: func(string) *exec.Cmd { + return nil + }, + } + + err := svc.SSH("example") + if err == nil { + t.Fatalf("expected error when command factory returns nil") + } +} + +func TestIsRemoteDisconnectError(t *testing.T) { + if isRemoteDisconnectError(errors.New("plain error"), "Connection closed by remote host") { + t.Fatalf("expected non-exit error to return false") + } + + cmd := helperCommandFactory("remote")("example") + err := cmd.Run() + if err == nil { + t.Fatalf("expected error for remote disconnect scenario") + } + if !isRemoteDisconnectError(err, "Connection to example closed by remote host.\n") { + t.Fatalf("expected remote disconnect error to be detected") + } + + cmd = helperCommandFactory("permission")("example") + err = cmd.Run() + if err == nil { + t.Fatalf("expected error for permission scenario") + } + if isRemoteDisconnectError(err, "Permission denied (publickey).\n") { + t.Fatalf("did not expect permission error to be treated as disconnect") + } +} + +func TestHelperProcess(t *testing.T) { + if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" { + return + } + + args := os.Args + for i := 0; i < len(args); i++ { + if args[i] == "--" && i+1 < len(args) { + scenario := args[i+1] + alias := "" + if i+2 < len(args) { + alias = args[i+2] + } + + switch scenario { + case "remote": + _, _ = os.Stderr.WriteString("Connection to " + alias + " closed by remote host.\n") + os.Exit(255) + case "reset": + _, _ = os.Stderr.WriteString("Read from remote host " + alias + ": Connection reset by peer\n") + os.Exit(255) + case "permission": + _, _ = os.Stderr.WriteString("Permission denied (publickey).\n") + os.Exit(255) + case "success": + os.Exit(0) + default: + _, _ = os.Stderr.WriteString("unknown scenario\n") + os.Exit(1) + } + } + } + os.Exit(1) +}