Skip to content

Commit 2afa6b0

Browse files
authored
fix: return error on invalid SSH private key instead of silent nil (#153)
1 parent 0bcc947 commit 2afa6b0

2 files changed

Lines changed: 39 additions & 16 deletions

File tree

internal/git/manager.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,17 @@ func (m *managerImpl) CloneRepository(ctx context.Context, repoUrl, subPath, ref
5454
return nil, fmt.Errorf("failed to create temporary directory: %w", err)
5555
}
5656

57+
clientOpts, err := m.getClientOptions(parsedURL.Scheme, auth)
58+
if err != nil {
59+
return nil, fmt.Errorf("failed to configure auth: %w", err)
60+
}
61+
5762
repo, err := git.PlainCloneContext(ctx, targetDir, &git.CloneOptions{
5863
URL: repoUrl,
5964
ReferenceName: plumbing.ReferenceName(reference),
6065
SingleBranch: true,
6166
Depth: 1,
62-
ClientOptions: m.getClientOptions(parsedURL.Scheme, auth),
67+
ClientOptions: clientOpts,
6368
})
6469
if err != nil {
6570
return nil, fmt.Errorf("failed to clone repo: %w", err)
@@ -78,11 +83,11 @@ func (m *managerImpl) CloneRepository(ctx context.Context, repoUrl, subPath, ref
7883
}, nil
7984
}
8085

81-
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) []client.Option {
86+
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) ([]client.Option, error) {
8287
if scheme == "ssh" {
8388
return m.getSSHClientOptions(authSecret)
8489
}
85-
return m.getHTTPClientOptions(authSecret)
90+
return m.getHTTPClientOptions(authSecret), nil
8691
}
8792

8893
func (m *managerImpl) getHTTPClientOptions(authSecret map[string][]byte) []client.Option {
@@ -125,21 +130,21 @@ func ensureKnownHostsExists() error {
125130
return nil
126131
}
127132

128-
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) []client.Option {
133+
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) ([]client.Option, error) {
129134
privateKey, hasKey := authSecret["sshPrivateKey"]
130135
if !hasKey {
131136
return []client.Option{
132137
client.WithSSHAuth(&gitssh.Password{
133138
User: "git",
134139
HostKeyCallbackHelper: gitssh.HostKeyCallbackHelper{HostKeyCallback: gossh.InsecureIgnoreHostKey()},
135140
}),
136-
}
141+
}, nil
137142
}
138143

139144
password := string(authSecret["sshPrivateKeyPassword"])
140145
auth, err := gitssh.NewPublicKeys("git", privateKey, password)
141146
if err != nil {
142-
return nil
147+
return nil, fmt.Errorf("failed to parse SSH private key: %w", err)
143148
}
144149
auth.HostKeyCallback = gossh.InsecureIgnoreHostKey()
145150

@@ -157,5 +162,5 @@ func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) []client
157162
}
158163
}
159164

160-
return []client.Option{client.WithSSHAuth(auth)}
165+
return []client.Option{client.WithSSHAuth(auth)}, nil
161166
}

internal/git/manager_test.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ JtdGRlLmNzYgECAw==
2020
func TestGetClientOptions_HTTPToken(t *testing.T) {
2121
m := &managerImpl{}
2222
secret := map[string][]byte{"token": []byte("my-token")}
23-
opts := m.getClientOptions("https", secret)
23+
opts, err := m.getClientOptions("https", secret)
24+
if err != nil {
25+
t.Fatalf("unexpected error: %v", err)
26+
}
2427
if len(opts) != 1 {
2528
t.Fatalf("expected 1 option, got %d", len(opts))
2629
}
@@ -29,31 +32,43 @@ func TestGetClientOptions_HTTPToken(t *testing.T) {
2932
func TestGetClientOptions_HTTPUsernamePassword(t *testing.T) {
3033
m := &managerImpl{}
3134
secret := map[string][]byte{"username": []byte("user"), "password": []byte("pass")}
32-
opts := m.getClientOptions("http", secret)
35+
opts, err := m.getClientOptions("http", secret)
36+
if err != nil {
37+
t.Fatalf("unexpected error: %v", err)
38+
}
3339
if len(opts) != 1 {
3440
t.Fatalf("expected 1 option, got %d", len(opts))
3541
}
3642
}
3743

3844
func TestGetClientOptions_HTTPEmpty(t *testing.T) {
3945
m := &managerImpl{}
40-
opts := m.getClientOptions("https", nil)
46+
opts, err := m.getClientOptions("https", nil)
47+
if err != nil {
48+
t.Fatalf("unexpected error: %v", err)
49+
}
4150
if opts != nil {
4251
t.Fatalf("expected nil options, got %v", opts)
4352
}
4453
}
4554

4655
func TestGetClientOptions_SSHNoSecret(t *testing.T) {
4756
m := &managerImpl{}
48-
opts := m.getClientOptions(sshScheme, nil)
57+
opts, err := m.getClientOptions(sshScheme, nil)
58+
if err != nil {
59+
t.Fatalf("unexpected error: %v", err)
60+
}
4961
if len(opts) != 1 {
5062
t.Fatalf("expected 1 option for insecure SSH, got %d", len(opts))
5163
}
5264
}
5365

5466
func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
5567
m := &managerImpl{}
56-
opts := m.getClientOptions(sshScheme, map[string][]byte{})
68+
opts, err := m.getClientOptions(sshScheme, map[string][]byte{})
69+
if err != nil {
70+
t.Fatalf("unexpected error: %v", err)
71+
}
5772
if len(opts) != 1 {
5873
t.Fatalf("expected 1 option for insecure SSH, got %d", len(opts))
5974
}
@@ -62,7 +77,10 @@ func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
6277
func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
6378
m := &managerImpl{}
6479
secret := map[string][]byte{"sshPrivateKey": []byte(testEd25519PrivateKey)}
65-
opts := m.getClientOptions(sshScheme, secret)
80+
opts, err := m.getClientOptions(sshScheme, secret)
81+
if err != nil {
82+
t.Fatalf("unexpected error: %v", err)
83+
}
6684
if len(opts) != 1 {
6785
t.Fatalf("expected 1 option, got %d", len(opts))
6886
}
@@ -71,9 +89,9 @@ func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
7189
func TestGetClientOptions_SSHWithInvalidKey(t *testing.T) {
7290
m := &managerImpl{}
7391
secret := map[string][]byte{"sshPrivateKey": []byte("not-a-valid-key")}
74-
opts := m.getClientOptions(sshScheme, secret)
75-
if opts != nil {
76-
t.Fatalf("expected nil options for invalid key, got %v", opts)
92+
_, err := m.getClientOptions(sshScheme, secret)
93+
if err == nil {
94+
t.Fatal("expected error for invalid SSH key, got nil")
7795
}
7896
}
7997

0 commit comments

Comments
 (0)