Skip to content

Commit 32f8e52

Browse files
committed
fix: keep SSH known_hosts temp file alive until repository cleanup
The known_hosts temp file was deleted via defer in getSSHClientOptions, which runs before the SSH connection is established in CloneRepository. This caused host key verification to fail silently when known_hosts data was provided. The temp file is now tracked in Repository.knownHostFile and cleaned up by Cleanup(). A defer in CloneRepository ensures cleanup on error paths where no Repository is returned.
1 parent 2afa6b0 commit 32f8e52

3 files changed

Lines changed: 39 additions & 24 deletions

File tree

internal/git/manager.go

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,15 @@ 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)
57+
clientOpts, tempFile, err := m.getClientOptions(parsedURL.Scheme, auth)
5858
if err != nil {
5959
return nil, fmt.Errorf("failed to configure auth: %w", err)
6060
}
61+
defer func() {
62+
if err != nil && tempFile != "" {
63+
_ = os.Remove(tempFile)
64+
}
65+
}()
6166

6267
repo, err := git.PlainCloneContext(ctx, targetDir, &git.CloneOptions{
6368
URL: repoUrl,
@@ -76,18 +81,20 @@ func (m *managerImpl) CloneRepository(ctx context.Context, repoUrl, subPath, ref
7681
}
7782

7883
return &Repository{
79-
CloneDir: targetDir,
80-
SubPath: subPath,
81-
Commit: head.Hash().String(),
82-
Branch: reference,
84+
CloneDir: targetDir,
85+
SubPath: subPath,
86+
Commit: head.Hash().String(),
87+
Branch: reference,
88+
knownHostFile: tempFile,
8389
}, nil
8490
}
8591

86-
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) ([]client.Option, error) {
92+
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) ([]client.Option, string, error) {
8793
if scheme == "ssh" {
8894
return m.getSSHClientOptions(authSecret)
8995
}
90-
return m.getHTTPClientOptions(authSecret), nil
96+
opts := m.getHTTPClientOptions(authSecret)
97+
return opts, "", nil
9198
}
9299

93100
func (m *managerImpl) getHTTPClientOptions(authSecret map[string][]byte) []client.Option {
@@ -130,37 +137,38 @@ func ensureKnownHostsExists() error {
130137
return nil
131138
}
132139

133-
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) ([]client.Option, error) {
140+
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) ([]client.Option, string, error) {
134141
privateKey, hasKey := authSecret["sshPrivateKey"]
135142
if !hasKey {
136143
return []client.Option{
137144
client.WithSSHAuth(&gitssh.Password{
138145
User: "git",
139146
HostKeyCallbackHelper: gitssh.HostKeyCallbackHelper{HostKeyCallback: gossh.InsecureIgnoreHostKey()},
140147
}),
141-
}, nil
148+
}, "", nil
142149
}
143150

144151
password := string(authSecret["sshPrivateKeyPassword"])
145152
auth, err := gitssh.NewPublicKeys("git", privateKey, password)
146153
if err != nil {
147-
return nil, fmt.Errorf("failed to parse SSH private key: %w", err)
154+
return nil, "", fmt.Errorf("failed to parse SSH private key: %w", err)
148155
}
149156
auth.HostKeyCallback = gossh.InsecureIgnoreHostKey()
150157

158+
var tempFilePath string
151159
if knownHostsData, ok := authSecret["known_hosts"]; ok {
152160
tmpFile, err := os.CreateTemp("", "known_hosts-*")
153161
if err == nil {
154-
defer os.Remove(tmpFile.Name())
155162
if _, err := tmpFile.Write(knownHostsData); err == nil {
156163
_ = tmpFile.Close()
157-
cb, err := gitssh.NewKnownHostsCallback(tmpFile.Name())
164+
tempFilePath = tmpFile.Name()
165+
cb, err := gitssh.NewKnownHostsCallback(tempFilePath)
158166
if err == nil {
159167
auth.HostKeyCallback = cb
160168
}
161169
}
162170
}
163171
}
164172

165-
return []client.Option{client.WithSSHAuth(auth)}, nil
173+
return []client.Option{client.WithSSHAuth(auth)}, tempFilePath, nil
166174
}

internal/git/manager_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,22 @@ JtdGRlLmNzYgECAw==
2020
func TestGetClientOptions_HTTPToken(t *testing.T) {
2121
m := &managerImpl{}
2222
secret := map[string][]byte{"token": []byte("my-token")}
23-
opts, err := m.getClientOptions("https", secret)
23+
opts, tmpFile, err := m.getClientOptions("https", secret)
2424
if err != nil {
2525
t.Fatalf("unexpected error: %v", err)
2626
}
2727
if len(opts) != 1 {
2828
t.Fatalf("expected 1 option, got %d", len(opts))
2929
}
30+
if tmpFile != "" {
31+
t.Fatalf("expected no temp file, got %s", tmpFile)
32+
}
3033
}
3134

3235
func TestGetClientOptions_HTTPUsernamePassword(t *testing.T) {
3336
m := &managerImpl{}
3437
secret := map[string][]byte{"username": []byte("user"), "password": []byte("pass")}
35-
opts, err := m.getClientOptions("http", secret)
38+
opts, _, err := m.getClientOptions("http", secret)
3639
if err != nil {
3740
t.Fatalf("unexpected error: %v", err)
3841
}
@@ -43,7 +46,7 @@ func TestGetClientOptions_HTTPUsernamePassword(t *testing.T) {
4346

4447
func TestGetClientOptions_HTTPEmpty(t *testing.T) {
4548
m := &managerImpl{}
46-
opts, err := m.getClientOptions("https", nil)
49+
opts, _, err := m.getClientOptions("https", nil)
4750
if err != nil {
4851
t.Fatalf("unexpected error: %v", err)
4952
}
@@ -54,7 +57,7 @@ func TestGetClientOptions_HTTPEmpty(t *testing.T) {
5457

5558
func TestGetClientOptions_SSHNoSecret(t *testing.T) {
5659
m := &managerImpl{}
57-
opts, err := m.getClientOptions(sshScheme, nil)
60+
opts, _, err := m.getClientOptions(sshScheme, nil)
5861
if err != nil {
5962
t.Fatalf("unexpected error: %v", err)
6063
}
@@ -65,7 +68,7 @@ func TestGetClientOptions_SSHNoSecret(t *testing.T) {
6568

6669
func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
6770
m := &managerImpl{}
68-
opts, err := m.getClientOptions(sshScheme, map[string][]byte{})
71+
opts, _, err := m.getClientOptions(sshScheme, map[string][]byte{})
6972
if err != nil {
7073
t.Fatalf("unexpected error: %v", err)
7174
}
@@ -77,7 +80,7 @@ func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
7780
func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
7881
m := &managerImpl{}
7982
secret := map[string][]byte{"sshPrivateKey": []byte(testEd25519PrivateKey)}
80-
opts, err := m.getClientOptions(sshScheme, secret)
83+
opts, _, err := m.getClientOptions(sshScheme, secret)
8184
if err != nil {
8285
t.Fatalf("unexpected error: %v", err)
8386
}
@@ -89,7 +92,7 @@ func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
8992
func TestGetClientOptions_SSHWithInvalidKey(t *testing.T) {
9093
m := &managerImpl{}
9194
secret := map[string][]byte{"sshPrivateKey": []byte("not-a-valid-key")}
92-
_, err := m.getClientOptions(sshScheme, secret)
95+
_, _, err := m.getClientOptions(sshScheme, secret)
9396
if err == nil {
9497
t.Fatal("expected error for invalid SSH key, got nil")
9598
}

internal/git/repository.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,20 @@ import (
66
)
77

88
type Repository struct {
9-
CloneDir string
10-
SubPath string
11-
Commit string
12-
Branch string
9+
CloneDir string
10+
SubPath string
11+
Commit string
12+
Branch string
13+
knownHostFile string
1314
}
1415

1516
func (r *Repository) Path() string {
1617
return path.Join(r.CloneDir, r.SubPath)
1718
}
1819

1920
func (r *Repository) Cleanup() error {
21+
if r.knownHostFile != "" {
22+
_ = os.Remove(r.knownHostFile)
23+
}
2024
return os.RemoveAll(r.CloneDir)
2125
}

0 commit comments

Comments
 (0)