Skip to content

Commit 1375583

Browse files
committed
fix: return error on invalid SSH private key instead of silent nil
When gitssh.NewPublicKeys failed (e.g. corrupted key), getSSHClientOptions silently returned nil, causing the clone to proceed without authentication and fail with a confusing error. Now the key parse error is propagated through getClientOptions to CloneRepository, giving users a clear "failed to parse SSH private key" message.
1 parent 0bcc947 commit 1375583

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)