Skip to content

Commit f68dc89

Browse files
committed
test(sshConn): close coverage gaps in config resolver
Add focused tests for previously-uncovered branches in config.go: - Error propagation from loadConfig for non-ENOENT paths (ENOTDIR via intermediate regular file) and from both user and system config loads in LoadSSHConfig. - Resolver error propagation through getValue and getAllValues when the underlying library rejects an empty HostArg. - ResolveHost paths for HostSpec.Alias precedence, invalid Port values surfacing as a wrapped error, and fallback to currentUser() when no explicit fallback is provided. - LoadIdentityFiles wrapping non-ErrNotExist read errors with the offending path. - ParseProxyJump dropping empty/whitespace-only components between commas. Statement coverage for internal/sshConn/config.go: 84.61% -> 93.75%. Remaining gaps are platform-specific (Windows shell branch in shellMatchExec), environment-dependent (currentUser env fallbacks when user.Current() fails), or structurally unreachable under normal control flow.
1 parent 663ab4a commit f68dc89

1 file changed

Lines changed: 184 additions & 0 deletions

File tree

internal/sshConn/config_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"path/filepath"
1212
"reflect"
1313
"runtime"
14+
"strings"
1415
"testing"
1516

1617
"golang.org/x/crypto/ssh"
@@ -83,6 +84,49 @@ func TestResolveHostFallbackUser(t *testing.T) {
8384
}
8485
}
8586

87+
func TestResolveHostUsesCurrentUserWhenNoFallback(t *testing.T) {
88+
// With no explicit user, no User directive, and an empty fallbackUser,
89+
// ResolveHost must fall back to the OS current user. This exercises the
90+
// `userValue = currentUser()` branch that is otherwise masked by the
91+
// explicit fallback used in every other test in this file.
92+
resolver, err := LoadSSHConfig(SSHConfigPaths{})
93+
if err != nil {
94+
t.Fatalf("unexpected error: %v", err)
95+
}
96+
resolved, err := resolver.ResolveHost(HostSpec{Host: "host-no-user"}, "")
97+
if err != nil {
98+
t.Fatalf("unexpected error: %v", err)
99+
}
100+
if resolved.User == "" {
101+
t.Fatalf("expected a non-empty OS user when fallback is empty, got %+v", resolved)
102+
}
103+
if resolved.User != currentUser() {
104+
t.Fatalf("expected OS current user %q, got %q", currentUser(), resolved.User)
105+
}
106+
}
107+
108+
func TestResolveHostPrefersExplicitAliasOverHost(t *testing.T) {
109+
// HostSpec.Alias takes precedence over Host for config lookup; this
110+
// covers the `alias = spec.Alias` branch that the other tests never hit
111+
// because they always set Host directly.
112+
cfg := "Host canonical\n HostName canonical.internal\n User deploy\n"
113+
userCfg := writeTempConfig(t, cfg)
114+
resolver, err := LoadSSHConfig(SSHConfigPaths{User: userCfg})
115+
if err != nil {
116+
t.Fatalf("unexpected error: %v", err)
117+
}
118+
resolved, err := resolver.ResolveHost(HostSpec{Host: "ignored", Alias: "canonical"}, "me")
119+
if err != nil {
120+
t.Fatalf("unexpected error: %v", err)
121+
}
122+
if resolved.Alias != "canonical" {
123+
t.Fatalf("expected alias to win over host, got %+v", resolved)
124+
}
125+
if resolved.Host != "canonical.internal" {
126+
t.Fatalf("expected HostName to resolve via alias, got %+v", resolved)
127+
}
128+
}
129+
86130
func TestLoadIdentityFilesSkipsMissing(t *testing.T) {
87131
methods, err := LoadIdentityFiles([]string{"/does/not/exist"})
88132
if err != nil {
@@ -182,6 +226,20 @@ func TestLoadIdentityFilesReturnsErrorForMalformedPrivateKey(t *testing.T) {
182226
}
183227
}
184228

229+
func TestLoadIdentityFilesWrapsNonNotExistReadError(t *testing.T) {
230+
// Passing a directory where a file is expected produces a read error
231+
// that is not os.ErrNotExist, which must be surfaced with a wrapped,
232+
// path-qualified message rather than swallowed.
233+
dir := t.TempDir()
234+
_, err := LoadIdentityFiles([]string{dir})
235+
if err == nil {
236+
t.Fatalf("expected read error for directory path")
237+
}
238+
if !strings.Contains(err.Error(), dir) {
239+
t.Fatalf("expected wrapped error to include the offending path, got %q", err.Error())
240+
}
241+
}
242+
185243
func TestResolveHostMatchExecApplies(t *testing.T) {
186244
// Models the production pattern where a jump alias resolves to a concrete
187245
// HostName only when the probe command succeeds, e.g.:
@@ -323,6 +381,16 @@ func TestParseProxyJumpNoneDisablesJumps(t *testing.T) {
323381
}
324382
}
325383

384+
func TestParseProxyJumpSkipsEmptyComponents(t *testing.T) {
385+
// Empty/whitespace-only segments between commas are dropped rather than
386+
// surfacing as blank jump entries to the dialer.
387+
got := ParseProxyJump(" ,jump1, ,jump2,")
388+
want := []string{"jump1", "jump2"}
389+
if !reflect.DeepEqual(got, want) {
390+
t.Fatalf("ParseProxyJump skipped-empty: got %v, want %v", got, want)
391+
}
392+
}
393+
326394
func TestResolveHostProxyJumpNoneClearsJumps(t *testing.T) {
327395
// OpenSSH: first matching directive wins for single-valued options, so
328396
// the specific block declaring `ProxyJump none` must come before the
@@ -654,6 +722,45 @@ func TestLoadConfigNonExistentPath(t *testing.T) {
654722
}
655723
}
656724

725+
func TestLoadConfigSurfacesNonNotExistOpenError(t *testing.T) {
726+
// Using a regular file as an intermediate path component produces an
727+
// ENOTDIR error from os.Open, which is not os.IsNotExist. loadConfig
728+
// must surface that error instead of silently returning nil.
729+
base := filepath.Join(t.TempDir(), "not_a_dir")
730+
if err := os.WriteFile(base, []byte("placeholder"), 0o600); err != nil {
731+
t.Fatalf("failed to seed placeholder file: %v", err)
732+
}
733+
cfg, err := loadConfig(filepath.Join(base, "config"))
734+
if err == nil {
735+
t.Fatalf("expected non-nil error for ENOTDIR path")
736+
}
737+
if cfg != nil {
738+
t.Fatalf("expected nil config when open errors, got %+v", cfg)
739+
}
740+
}
741+
742+
func TestLoadSSHConfigPropagatesUserLoadError(t *testing.T) {
743+
base := filepath.Join(t.TempDir(), "user_placeholder")
744+
if err := os.WriteFile(base, []byte("x"), 0o600); err != nil {
745+
t.Fatalf("failed to seed placeholder: %v", err)
746+
}
747+
_, err := LoadSSHConfig(SSHConfigPaths{User: filepath.Join(base, "config")})
748+
if err == nil {
749+
t.Fatalf("expected user loadConfig error to propagate")
750+
}
751+
}
752+
753+
func TestLoadSSHConfigPropagatesSystemLoadError(t *testing.T) {
754+
base := filepath.Join(t.TempDir(), "sys_placeholder")
755+
if err := os.WriteFile(base, []byte("x"), 0o600); err != nil {
756+
t.Fatalf("failed to seed placeholder: %v", err)
757+
}
758+
_, err := LoadSSHConfig(SSHConfigPaths{System: filepath.Join(base, "config")})
759+
if err == nil {
760+
t.Fatalf("expected system loadConfig error to propagate")
761+
}
762+
}
763+
657764
func TestResolveNilConfig(t *testing.T) {
658765
resolver := &SSHConfigResolver{}
659766
result, err := resolver.resolve(nil, "web")
@@ -665,6 +772,83 @@ func TestResolveNilConfig(t *testing.T) {
665772
}
666773
}
667774

775+
func TestResolveHostInvalidPortSurfacesError(t *testing.T) {
776+
cfg := "Host web\n Port not-a-number\n"
777+
userCfg := writeTempConfig(t, cfg)
778+
resolver, err := LoadSSHConfig(SSHConfigPaths{User: userCfg})
779+
if err != nil {
780+
t.Fatalf("unexpected error: %v", err)
781+
}
782+
_, err = resolver.ResolveHost(HostSpec{Host: "web"}, "me")
783+
if err == nil {
784+
t.Fatalf("expected invalid Port value to produce a wrapped error")
785+
}
786+
if !strings.Contains(err.Error(), "invalid port") {
787+
t.Fatalf("expected wrapped error to mention invalid port, got %q", err.Error())
788+
}
789+
}
790+
791+
func TestResolveHostEmptyAliasSurfacesResolverError(t *testing.T) {
792+
// The underlying ssh_config library requires a non-empty HostArg and
793+
// returns an error otherwise. ResolveHost must surface that rather than
794+
// continuing to connect to a nameless host.
795+
userCfg := writeTempConfig(t, "Host web\n User deploy\n")
796+
resolver, err := LoadSSHConfig(SSHConfigPaths{User: userCfg})
797+
if err != nil {
798+
t.Fatalf("unexpected error: %v", err)
799+
}
800+
_, err = resolver.ResolveHost(HostSpec{}, "")
801+
if err == nil {
802+
t.Fatalf("expected error for empty host/alias")
803+
}
804+
}
805+
806+
func TestGetValueSurfacesUserResolverError(t *testing.T) {
807+
userCfg := writeTempConfig(t, "Host web\n User deploy\n")
808+
resolver, err := LoadSSHConfig(SSHConfigPaths{User: userCfg})
809+
if err != nil {
810+
t.Fatalf("unexpected error: %v", err)
811+
}
812+
if _, err := resolver.getValue("", "User"); err == nil {
813+
t.Fatalf("expected resolver error for empty alias via user config")
814+
}
815+
}
816+
817+
func TestGetValueSurfacesSystemResolverError(t *testing.T) {
818+
// Only a system config is loaded so the error must come from the system
819+
// branch of getValue rather than the user branch.
820+
systemCfg := writeTempConfig(t, "Host web\n User deploy\n")
821+
resolver, err := LoadSSHConfig(SSHConfigPaths{System: systemCfg})
822+
if err != nil {
823+
t.Fatalf("unexpected error: %v", err)
824+
}
825+
if _, err := resolver.getValue("", "User"); err == nil {
826+
t.Fatalf("expected resolver error for empty alias via system config")
827+
}
828+
}
829+
830+
func TestGetAllValuesSurfacesUserResolverError(t *testing.T) {
831+
userCfg := writeTempConfig(t, "Host web\n IdentityFile ~/.ssh/id\n")
832+
resolver, err := LoadSSHConfig(SSHConfigPaths{User: userCfg})
833+
if err != nil {
834+
t.Fatalf("unexpected error: %v", err)
835+
}
836+
if _, err := resolver.getAllValues("", "IdentityFile"); err == nil {
837+
t.Fatalf("expected resolver error for empty alias via user config")
838+
}
839+
}
840+
841+
func TestGetAllValuesSurfacesSystemResolverError(t *testing.T) {
842+
systemCfg := writeTempConfig(t, "Host web\n IdentityFile ~/.ssh/id\n")
843+
resolver, err := LoadSSHConfig(SSHConfigPaths{System: systemCfg})
844+
if err != nil {
845+
t.Fatalf("unexpected error: %v", err)
846+
}
847+
if _, err := resolver.getAllValues("", "IdentityFile"); err == nil {
848+
t.Fatalf("expected resolver error for empty alias via system config")
849+
}
850+
}
851+
668852
func TestGetValueWithNilConfigs(t *testing.T) {
669853
resolver := &SSHConfigResolver{}
670854
val, err := resolver.getValue("web", "HostName")

0 commit comments

Comments
 (0)