From 770663b23dc978a68ec3ab21ef61fe7f05c81cb9 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 23 Mar 2026 19:35:57 -0400 Subject: [PATCH] update based on feedback --- go.mod | 2 +- models/repo/mirror_ssh_keypair.go | 11 ++-- modules/ssh/agent.go | 75 +++----------------------- modules/ssh/agent_unix.go | 65 ++++++++++++++++++++++ modules/ssh/agent_windows.go | 42 +++++++++++++++ modules/ssh/mirror.go | 2 +- routers/web/org/setting_ssh_keys.go | 7 ++- services/repository/migrate.go | 84 ++++++++++++++--------------- templates/repo/migrate/git.tmpl | 2 +- 9 files changed, 168 insertions(+), 122 deletions(-) create mode 100644 modules/ssh/agent_unix.go create mode 100644 modules/ssh/agent_windows.go diff --git a/go.mod b/go.mod index 24c18d5703..4ab25e2dc2 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/azcore v1.19.0 github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.2 github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 + github.com/Microsoft/go-winio v0.6.2 github.com/ProtonMail/go-crypto v1.3.0 github.com/PuerkitoBio/goquery v1.11.0 github.com/SaveTheRbtz/zstd-seekable-format-go/pkg v0.8.0 @@ -136,7 +137,6 @@ require ( filippo.io/edwards25519 v1.1.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/internal v1.11.2 // indirect github.com/DataDog/zstd v1.5.7 // indirect - github.com/Microsoft/go-winio v0.6.2 // indirect github.com/RoaringBitmap/roaring/v2 v2.10.0 // indirect github.com/STARRY-S/zip v0.2.3 // indirect github.com/andybalholm/brotli v1.2.0 // indirect diff --git a/models/repo/mirror_ssh_keypair.go b/models/repo/mirror_ssh_keypair.go index 4cb5d90d2b..6ac6c170f5 100644 --- a/models/repo/mirror_ssh_keypair.go +++ b/models/repo/mirror_ssh_keypair.go @@ -7,8 +7,6 @@ import ( "context" "crypto/ed25519" "crypto/rand" - "crypto/sha256" - "encoding/hex" "fmt" "strings" @@ -78,8 +76,7 @@ func CreateUserSSHKeypair(ctx context.Context, ownerID int64) (*UserSSHKeypair, publicKeyStr := string(ssh.MarshalAuthorizedKey(sshPublicKey)) - fingerprint := sha256.Sum256(sshPublicKey.Marshal()) - fingerprintStr := hex.EncodeToString(fingerprint[:]) + fingerprintStr := ssh.FingerprintSHA256(sshPublicKey) privateKeyEncrypted, err := secret.EncryptSecret(setting.SecretKey, string(privateKey)) if err != nil { @@ -133,7 +130,7 @@ func (k *UserSSHKeypair) GetPublicKeyWithComment(ctx context.Context) (string, e domain = "gitea" } - keyID := k.Fingerprint + keyID := strings.TrimPrefix(k.Fingerprint, "SHA256:") if len(keyID) > 8 { keyID = keyID[:8] } @@ -158,7 +155,9 @@ func DeleteUserSSHKeypair(ctx context.Context, ownerID int64) error { // RegenerateUserSSHKeypair regenerates an SSH keypair for the given owner func RegenerateUserSSHKeypair(ctx context.Context, ownerID int64) (*UserSSHKeypair, error) { return db.WithTx2(ctx, func(ctx context.Context) (*UserSSHKeypair, error) { - _ = DeleteUserSSHKeypair(ctx, ownerID) + if err := DeleteUserSSHKeypair(ctx, ownerID); err != nil { + return nil, fmt.Errorf("failed to delete existing keypair: %w", err) + } newKeypair, err := CreateUserSSHKeypair(ctx, ownerID) if err != nil { diff --git a/modules/ssh/agent.go b/modules/ssh/agent.go index d6b6a9c674..de118f8942 100644 --- a/modules/ssh/agent.go +++ b/modules/ssh/agent.go @@ -7,11 +7,7 @@ import ( "crypto/ed25519" "fmt" "net" - "os" - "path/filepath" - "runtime" "sync" - "time" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" @@ -33,61 +29,16 @@ type Agent struct { // NewSSHAgent creates a new SSH agent with the given private key func NewSSHAgent(privateKey ed25519.PrivateKey) (*Agent, error) { - var listener net.Listener - var socketPath string - var tempDir string - var err error - - // Setup cleanup function for early returns - var cleanup func() + listener, socketPath, cleanup, err := createAgentListener() + if err != nil { + return nil, err + } defer func() { if cleanup != nil { cleanup() } }() - if runtime.GOOS == "windows" { - // On Windows, use named pipes - agentID, err := util.CryptoRandomString(16) - if err != nil { - return nil, fmt.Errorf("failed to generate agent ID: %w", err) - } - socketPath = `\\.\pipe\gitea-ssh-agent-` + agentID - listener, err = net.Listen("pipe", socketPath) - if err != nil { - return nil, fmt.Errorf("failed to create named pipe: %w", err) - } - cleanup = func() { - listener.Close() - } - } else { - tempDir, err = os.MkdirTemp("", "gitea-ssh-agent-") - if err != nil { - return nil, fmt.Errorf("failed to create temporary directory: %w", err) - } - cleanup = func() { - os.RemoveAll(tempDir) - } - - if err := os.Chmod(tempDir, 0o700); err != nil { - return nil, fmt.Errorf("failed to set temporary directory permissions: %w", err) - } - - socketPath = filepath.Join(tempDir, "agent.sock") - listener, err = net.Listen("unix", socketPath) - if err != nil { - return nil, fmt.Errorf("failed to create Unix socket: %w", err) - } - cleanup = func() { - listener.Close() - os.RemoveAll(tempDir) - } - - if err := os.Chmod(socketPath, 0o600); err != nil { - return nil, fmt.Errorf("failed to set socket permissions: %w", err) - } - } - sshAgent := agent.NewKeyring() if len(privateKey) != ed25519.PrivateKeySize { @@ -136,14 +87,7 @@ func (sa *Agent) serve() { return default: // Set a timeout for Accept to avoid blocking indefinitely - if runtime.GOOS != "windows" { - // On Windows, named pipes don't support SetDeadline in the same way - if listener, ok := sa.listener.(*net.UnixListener); ok { - if err := listener.SetDeadline(time.Now().Add(100 * time.Millisecond)); err != nil { - log.Debug("Failed to set listener deadline: %v", err) - } - } - } + setListenerAcceptDeadline(sa.listener) conn, err := sa.listener.Accept() if err != nil { @@ -175,14 +119,7 @@ func (sa *Agent) serve() { // cleanup removes the socket file and temporary directory func (sa *Agent) cleanup() { - if sa.socketPath != "" { - if runtime.GOOS != "windows" { - // On Windows, named pipes are automatically cleaned up when closed - // On Unix-like systems, remove the temporary directory - tempDir := filepath.Dir(sa.socketPath) - os.RemoveAll(tempDir) - } - } + cleanupAgentSocket(sa.socketPath) } // GetSocketPath returns the path to the SSH agent socket diff --git a/modules/ssh/agent_unix.go b/modules/ssh/agent_unix.go new file mode 100644 index 0000000000..ba04c89799 --- /dev/null +++ b/modules/ssh/agent_unix.go @@ -0,0 +1,65 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +//go:build !windows + +package ssh + +import ( + "fmt" + "net" + "os" + "path/filepath" + "time" +) + +// createAgentListener creates a Unix domain socket listener for the SSH agent. +// Returns the listener, socket path, and a cleanup function for early-return error paths. +func createAgentListener() (net.Listener, string, func(), error) { + tempDir, err := os.MkdirTemp("", "gitea-ssh-agent-") + if err != nil { + return nil, "", nil, fmt.Errorf("failed to create temporary directory: %w", err) + } + cleanupDir := func() { + os.RemoveAll(tempDir) + } + + if err := os.Chmod(tempDir, 0o700); err != nil { + cleanupDir() + return nil, "", nil, fmt.Errorf("failed to set temporary directory permissions: %w", err) + } + + socketPath := filepath.Join(tempDir, "agent.sock") + listener, err := net.Listen("unix", socketPath) + if err != nil { + cleanupDir() + return nil, "", nil, fmt.Errorf("failed to create Unix socket: %w", err) + } + + cleanup := func() { + listener.Close() + os.RemoveAll(tempDir) + } + + if err := os.Chmod(socketPath, 0o600); err != nil { + cleanup() + return nil, "", nil, fmt.Errorf("failed to set socket permissions: %w", err) + } + + return listener, socketPath, cleanup, nil +} + +// setListenerAcceptDeadline sets a short deadline on the listener for non-blocking accept loops. +func setListenerAcceptDeadline(listener net.Listener) { + if unixListener, ok := listener.(*net.UnixListener); ok { + _ = unixListener.SetDeadline(time.Now().Add(100 * time.Millisecond)) + } +} + +// cleanupAgentSocket removes the socket file and its temporary directory. +func cleanupAgentSocket(socketPath string) { + if socketPath != "" { + tempDir := filepath.Dir(socketPath) + os.RemoveAll(tempDir) + } +} diff --git a/modules/ssh/agent_windows.go b/modules/ssh/agent_windows.go new file mode 100644 index 0000000000..ae88bb6140 --- /dev/null +++ b/modules/ssh/agent_windows.go @@ -0,0 +1,42 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +//go:build windows + +package ssh + +import ( + "fmt" + "net" + + "code.gitea.io/gitea/modules/util" + + "github.com/Microsoft/go-winio" +) + +// createAgentListener creates a Windows named pipe listener for the SSH agent. +// Returns the listener, pipe path, and a cleanup function for early-return error paths. +func createAgentListener() (net.Listener, string, func(), error) { + agentID, err := util.CryptoRandomString(16) + if err != nil { + return nil, "", nil, fmt.Errorf("failed to generate agent ID: %w", err) + } + pipePath := `\\.\pipe\gitea-ssh-agent-` + agentID + + listener, err := winio.ListenPipe(pipePath, nil) + if err != nil { + return nil, "", nil, fmt.Errorf("failed to create named pipe: %w", err) + } + + cleanup := func() { + listener.Close() + } + + return listener, pipePath, cleanup, nil +} + +// setListenerAcceptDeadline is a no-op on Windows; named pipes don't support SetDeadline. +func setListenerAcceptDeadline(_ net.Listener) {} + +// cleanupAgentSocket is a no-op on Windows; named pipes are automatically cleaned up when closed. +func cleanupAgentSocket(_ string) {} diff --git a/modules/ssh/mirror.go b/modules/ssh/mirror.go index 16bb5796b5..9a3cbd303c 100644 --- a/modules/ssh/mirror.go +++ b/modules/ssh/mirror.go @@ -67,7 +67,7 @@ func GetSSHKeypairForRepository(ctx context.Context, repo *repo_model.Repository // Returns nil if the URL is not an SSH URL func GetSSHKeypairForURL(ctx context.Context, repo *repo_model.Repository, url string) (*repo_model.UserSSHKeypair, error) { if !IsSSHURL(url) { - return nil, nil + return nil, nil //nolint:nilnil // non-SSH URLs don't need a keypair } return GetSSHKeypairForRepository(ctx, repo) } diff --git a/routers/web/org/setting_ssh_keys.go b/routers/web/org/setting_ssh_keys.go index 6e03ec1aaa..94edb84e8a 100644 --- a/routers/web/org/setting_ssh_keys.go +++ b/routers/web/org/setting_ssh_keys.go @@ -6,6 +6,7 @@ package org import ( "net/http" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/templates" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" @@ -33,7 +34,11 @@ func SSHKeys(ctx *context.Context) { return } - ctx.Data["SSHKeypair"] = keypair + publicKeyWithComment, _ := keypair.GetPublicKeyWithComment(ctx) + ctx.Data["SSHKeypair"] = struct { + *repo_model.UserSSHKeypair + PublicKeyWithComment string + }{keypair, publicKeyWithComment} ctx.HTML(http.StatusOK, tplSettingsSSHKeys) } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index ae8899abf5..9b08efe387 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -28,6 +28,43 @@ import ( "code.gitea.io/gitea/modules/util" ) +func setupMigrationSSHAuth(ctx context.Context, repo *repo_model.Repository, remoteURL string) (string, func(), error) { + if !ssh_module.IsSSHURL(remoteURL) { + return "", func() {}, nil + } + + keypair, err := ssh_module.GetSSHKeypairForRepository(ctx, repo) + if err != nil { + return "", nil, fmt.Errorf("failed to get SSH keypair for repository: %w", err) + } + if keypair == nil { + return "", func() {}, nil + } + + privateKey, err := keypair.GetDecryptedPrivateKey() + if err != nil { + return "", nil, fmt.Errorf("failed to decrypt private key: %w", err) + } + + socketPath, cleanup, err := ssh_module.CreateTemporaryAgent(privateKey) + if err != nil { + return "", nil, fmt.Errorf("failed to create SSH agent: %w", err) + } + + return socketPath, cleanup, nil +} + +func cloneExternalRepoWithSSHAuth(ctx context.Context, repo *repo_model.Repository, remoteURL string, storageRepo gitrepo.Repository, cloneOpts git.CloneRepoOptions) error { + sshAuthSock, cleanup, err := setupMigrationSSHAuth(ctx, repo, remoteURL) + if err != nil { + return err + } + defer cleanup() + + cloneOpts.SSHAuthSock = sshAuthSock + return gitrepo.CloneExternalRepo(ctx, remoteURL, storageRepo, cloneOpts) +} + func cloneWiki(ctx context.Context, repo *repo_model.Repository, opts migration.MigrateOptions, migrateTimeout time.Duration) (string, error) { wikiRemoteURL := repo_module.WikiRemoteURL(ctx, opts.CloneAddr) if wikiRemoteURL == "" { @@ -52,27 +89,7 @@ func cloneWiki(ctx context.Context, repo *repo_model.Repository, opts migration. SkipTLSVerify: setting.Migrations.SkipTLSVerify, } - if ssh_module.IsSSHURL(wikiRemoteURL) { - keypair, err := ssh_module.GetSSHKeypairForRepository(ctx, repo) - if err != nil { - log.Error("Failed to get SSH keypair for wiki clone: %v", err) - } else if keypair != nil { - privateKey, err := keypair.GetDecryptedPrivateKey() - if err != nil { - log.Error("Failed to decrypt private key for wiki clone: %v", err) - } else { - socketPath, cleanup, err := ssh_module.CreateTemporaryAgent(privateKey) - if err != nil { - log.Error("Failed to create SSH agent for wiki clone: %v", err) - } else { - cloneOpts.SSHAuthSock = socketPath - defer cleanup() - } - } - } - } - - if err := gitrepo.CloneExternalRepo(ctx, wikiRemoteURL, storageRepo, cloneOpts); err != nil { + if err := cloneExternalRepoWithSSHAuth(ctx, repo, wikiRemoteURL, storageRepo, cloneOpts); err != nil { log.Error("Clone wiki failed, err: %v", err) cleanIncompleteWikiPath() return "", err @@ -120,30 +137,11 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, SkipTLSVerify: setting.Migrations.SkipTLSVerify, } - if ssh_module.IsSSHURL(opts.CloneAddr) { - if repo.Owner == nil { - repo.Owner = u - } - keypair, err := ssh_module.GetSSHKeypairForRepository(ctx, repo) - if err != nil { - return repo, fmt.Errorf("failed to get SSH keypair for repository: %w", err) - } - if keypair != nil { - privateKey, err := keypair.GetDecryptedPrivateKey() - if err != nil { - return repo, fmt.Errorf("failed to decrypt private key: %w", err) - } - - socketPath, cleanup, err := ssh_module.CreateTemporaryAgent(privateKey) - if err != nil { - return repo, fmt.Errorf("failed to create SSH agent: %w", err) - } - cloneOpts.SSHAuthSock = socketPath - defer cleanup() - } + if ssh_module.IsSSHURL(opts.CloneAddr) && repo.Owner == nil { + repo.Owner = u } - if err := gitrepo.CloneExternalRepo(ctx, opts.CloneAddr, repo, cloneOpts); err != nil { + if err := cloneExternalRepoWithSSHAuth(ctx, repo, opts.CloneAddr, repo, cloneOpts); err != nil { if errors.Is(err, context.DeadlineExceeded) { return repo, fmt.Errorf("clone timed out, consider increasing [git.timeout] MIGRATE in app.ini, underlying err: %w", err) } diff --git a/templates/repo/migrate/git.tmpl b/templates/repo/migrate/git.tmpl index 080a9c5000..c280587f68 100644 --- a/templates/repo/migrate/git.tmpl +++ b/templates/repo/migrate/git.tmpl @@ -19,7 +19,7 @@
{{ctx.Locale.Tr "repo.migrate.ssh_helper_title"}}: {{ctx.Locale.Tr "repo.migrate.ssh_helper_desc"}} - {{ctx.Locale.Tr "repo.migrate.ssh_helper_link"}} + {{ctx.Locale.Tr "repo.migrate.ssh_helper_link"}}