Skip to content

Commit 64246c1

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 0bcc947 commit 64246c1

3 files changed

Lines changed: 40 additions & 24 deletions

File tree

internal/git/manager.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,19 @@ 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, tempFile := m.getClientOptions(parsedURL.Scheme, auth)
58+
defer func() {
59+
if err != nil && tempFile != "" {
60+
_ = os.Remove(tempFile)
61+
}
62+
}()
63+
5764
repo, err := git.PlainCloneContext(ctx, targetDir, &git.CloneOptions{
5865
URL: repoUrl,
5966
ReferenceName: plumbing.ReferenceName(reference),
6067
SingleBranch: true,
6168
Depth: 1,
62-
ClientOptions: m.getClientOptions(parsedURL.Scheme, auth),
69+
ClientOptions: clientOpts,
6370
})
6471
if err != nil {
6572
return nil, fmt.Errorf("failed to clone repo: %w", err)
@@ -71,18 +78,19 @@ func (m *managerImpl) CloneRepository(ctx context.Context, repoUrl, subPath, ref
7178
}
7279

7380
return &Repository{
74-
CloneDir: targetDir,
75-
SubPath: subPath,
76-
Commit: head.Hash().String(),
77-
Branch: reference,
81+
CloneDir: targetDir,
82+
SubPath: subPath,
83+
Commit: head.Hash().String(),
84+
Branch: reference,
85+
knownHostFile: tempFile,
7886
}, nil
7987
}
8088

81-
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) []client.Option {
89+
func (m *managerImpl) getClientOptions(scheme string, authSecret map[string][]byte) ([]client.Option, string) {
8290
if scheme == "ssh" {
8391
return m.getSSHClientOptions(authSecret)
8492
}
85-
return m.getHTTPClientOptions(authSecret)
93+
return m.getHTTPClientOptions(authSecret), ""
8694
}
8795

8896
func (m *managerImpl) getHTTPClientOptions(authSecret map[string][]byte) []client.Option {
@@ -125,37 +133,38 @@ func ensureKnownHostsExists() error {
125133
return nil
126134
}
127135

128-
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) []client.Option {
136+
func (m *managerImpl) getSSHClientOptions(authSecret map[string][]byte) ([]client.Option, string) {
129137
privateKey, hasKey := authSecret["sshPrivateKey"]
130138
if !hasKey {
131139
return []client.Option{
132140
client.WithSSHAuth(&gitssh.Password{
133141
User: "git",
134142
HostKeyCallbackHelper: gitssh.HostKeyCallbackHelper{HostKeyCallback: gossh.InsecureIgnoreHostKey()},
135143
}),
136-
}
144+
}, ""
137145
}
138146

139147
password := string(authSecret["sshPrivateKeyPassword"])
140148
auth, err := gitssh.NewPublicKeys("git", privateKey, password)
141149
if err != nil {
142-
return nil
150+
return nil, ""
143151
}
144152
auth.HostKeyCallback = gossh.InsecureIgnoreHostKey()
145153

154+
var tempFilePath string
146155
if knownHostsData, ok := authSecret["known_hosts"]; ok {
147156
tmpFile, err := os.CreateTemp("", "known_hosts-*")
148157
if err == nil {
149-
defer os.Remove(tmpFile.Name())
150158
if _, err := tmpFile.Write(knownHostsData); err == nil {
151159
_ = tmpFile.Close()
152-
cb, err := gitssh.NewKnownHostsCallback(tmpFile.Name())
160+
tempFilePath = tmpFile.Name()
161+
cb, err := gitssh.NewKnownHostsCallback(tempFilePath)
153162
if err == nil {
154163
auth.HostKeyCallback = cb
155164
}
156165
}
157166
}
158167
}
159168

160-
return []client.Option{client.WithSSHAuth(auth)}
169+
return []client.Option{client.WithSSHAuth(auth)}, tempFilePath
161170
}

internal/git/manager_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,40 +20,43 @@ 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, tmpFile := m.getClientOptions("https", secret)
2424
if len(opts) != 1 {
2525
t.Fatalf("expected 1 option, got %d", len(opts))
2626
}
27+
if tmpFile != "" {
28+
t.Fatalf("expected no temp file, got %s", tmpFile)
29+
}
2730
}
2831

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, _ := m.getClientOptions("http", secret)
3336
if len(opts) != 1 {
3437
t.Fatalf("expected 1 option, got %d", len(opts))
3538
}
3639
}
3740

3841
func TestGetClientOptions_HTTPEmpty(t *testing.T) {
3942
m := &managerImpl{}
40-
opts := m.getClientOptions("https", nil)
43+
opts, _ := m.getClientOptions("https", nil)
4144
if opts != nil {
4245
t.Fatalf("expected nil options, got %v", opts)
4346
}
4447
}
4548

4649
func TestGetClientOptions_SSHNoSecret(t *testing.T) {
4750
m := &managerImpl{}
48-
opts := m.getClientOptions(sshScheme, nil)
51+
opts, _ := m.getClientOptions(sshScheme, nil)
4952
if len(opts) != 1 {
5053
t.Fatalf("expected 1 option for insecure SSH, got %d", len(opts))
5154
}
5255
}
5356

5457
func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
5558
m := &managerImpl{}
56-
opts := m.getClientOptions(sshScheme, map[string][]byte{})
59+
opts, _ := m.getClientOptions(sshScheme, map[string][]byte{})
5760
if len(opts) != 1 {
5861
t.Fatalf("expected 1 option for insecure SSH, got %d", len(opts))
5962
}
@@ -62,7 +65,7 @@ func TestGetClientOptions_SSHEmptySecret(t *testing.T) {
6265
func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
6366
m := &managerImpl{}
6467
secret := map[string][]byte{"sshPrivateKey": []byte(testEd25519PrivateKey)}
65-
opts := m.getClientOptions(sshScheme, secret)
68+
opts, _ := m.getClientOptions(sshScheme, secret)
6669
if len(opts) != 1 {
6770
t.Fatalf("expected 1 option, got %d", len(opts))
6871
}
@@ -71,7 +74,7 @@ func TestGetClientOptions_SSHWithPrivateKey(t *testing.T) {
7174
func TestGetClientOptions_SSHWithInvalidKey(t *testing.T) {
7275
m := &managerImpl{}
7376
secret := map[string][]byte{"sshPrivateKey": []byte("not-a-valid-key")}
74-
opts := m.getClientOptions(sshScheme, secret)
77+
opts, _ := m.getClientOptions(sshScheme, secret)
7578
if opts != nil {
7679
t.Fatalf("expected nil options for invalid key, got %v", opts)
7780
}

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)