Handle back compat

pkg/cmd/codespace/ssh.go

@@ -24,6 +24,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// In 2.13.0 these commands started automatically generating key pairs named 'codespaces' and 'codespaces.pub'
+// which could collide with suggested the ssh config also named 'codespaces'. We now use 'codespaces.auto'
+// and 'codespaces.auto.pub' in order to avoid that collision.
+const automaticPrivateKeyNameOld = "codespaces"
 const automaticPrivateKeyName = "codespaces.auto"
 
 type sshOptions struct {
@@ -48,9 +52,7 @@ func newSSHCmd(app *App) *cobra.Command {
 			run 'gh cs ssh', select a codespace interactively, and connect.
 			
 			By default, the 'ssh' command will create a public/private ssh key pair to  
-			authenticate with the codespace (if they haven't already been created) inside the 
-			~/.ssh directory. Any private keys for which the public key has been uploaded to 
-			your GitHub profile may also be used instead, if desired.
+			authenticate with the codespace inside the ~/.ssh directory.
 
 			The 'ssh' command also supports deeper integration with OpenSSH using a
 			'--config' option that generates per-codespace ssh configuration in OpenSSH
@@ -131,9 +133,9 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
 	sshContext := ssh.Context{}
 	startSSHOptions := liveshare.StartSSHServerOptions{}
 
-	if shouldGenerateSSHKeys(args, opts) && sshContext.HasKeygen() {
-		keyPair, err := sshContext.GenerateSSHKey(automaticPrivateKeyName, "")
-		if err != nil && !errors.Is(err, ssh.ErrKeyAlreadyExists) {
+	if shouldUseAutomaticSSHKeys(args, opts) {
+		keyPair, err := setupAutomaticSSHKeys(sshContext)
+		if err != nil {
 			return fmt.Errorf("failed to generate ssh keys: %w", err)
 		}
 
@@ -214,7 +216,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
 	}
 }
 
-func shouldGenerateSSHKeys(args []string, opts sshOptions) bool {
+func shouldUseAutomaticSSHKeys(args []string, opts sshOptions) bool {
 	if opts.profile != "" {
 		// The profile may specify the identity file so cautiously don't override anything with that option
 		return false
@@ -235,6 +237,62 @@ func shouldGenerateSSHKeys(args []string, opts sshOptions) bool {
 	return true
 }
 
+func setupAutomaticSSHKeys(sshContext ssh.Context) (*ssh.KeyPair, error) {
+	keyPair := checkAndUpdateOldKeyPair(sshContext)
+	if keyPair != nil {
+		return keyPair, nil
+	}
+
+	keyPair, err := sshContext.GenerateSSHKey(automaticPrivateKeyName, "")
+	if err != nil && !errors.Is(err, ssh.ErrKeyAlreadyExists) {
+		return nil, err
+	}
+
+	return keyPair, nil
+}
+
+func checkAndUpdateOldKeyPair(sshContext ssh.Context) *ssh.KeyPair {
+	publicKeys, err := sshContext.LocalPublicKeys()
+	if err != nil {
+		return nil
+	}
+
+	for _, publicKey := range publicKeys {
+		if !strings.HasSuffix(publicKey, automaticPrivateKeyNameOld+".pub") {
+			continue
+		}
+
+		privateKey := strings.TrimSuffix(publicKey, ".pub")
+		_, err := os.Stat(privateKey)
+		if err != nil {
+			continue
+		}
+
+		// Both old public and private keys exist, rename them to the new name
+
+		privateKeyNew := strings.Replace(privateKey, automaticPrivateKeyNameOld, automaticPrivateKeyName, -1)
+		err = os.Rename(privateKey, privateKeyNew)
+		if err != nil {
+			return nil
+		}
+
+		publicKeyNew := strings.Replace(publicKey, automaticPrivateKeyNameOld, automaticPrivateKeyName, -1)
+		err = os.Rename(publicKey, publicKeyNew)
+		if err != nil {
+			return nil
+		}
+
+		keyPair := &ssh.KeyPair{
+			PublicKeyPath:  publicKeyNew,
+			PrivateKeyPath: privateKeyNew,
+		}
+
+		return keyPair
+	}
+
+	return nil
+}
+
 func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err error) {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
@@ -390,9 +448,7 @@ func newCpCmd(app *App) *cobra.Command {
 			provided by untrusted users; see <https://lwn.net/Articles/835962/> for discussion.
 			
 			By default, the 'cp' command will create a public/private ssh key pair to authenticate with 
-			the codespace (if they haven't already been created) inside the ~/.ssh directory. Any private 
-			keys for which the public key has been uploaded to your GitHub profile may also be used
-			instead, if desired.
+			the codespace inside the ~/.ssh directory.
 		`, "`"),
 		Example: heredoc.Doc(`
 			$ gh codespace cp -e README.md 'remote:/workspaces/$RepositoryName/'

pkg/cmd/codespace/ssh_test.go

@@ -2,10 +2,14 @@ package codespace
 
 import (
 	"context"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/cli/cli/v2/internal/codespaces/api"
 	"github.com/cli/cli/v2/pkg/iostreams"
+	"github.com/cli/cli/v2/pkg/ssh"
 )
 
 func TestPendingOperationDisallowsSSH(t *testing.T) {
@@ -20,6 +24,100 @@ func TestPendingOperationDisallowsSSH(t *testing.T) {
 	}
 }
 
+func TestAutomaticSSHKeyPairs(t *testing.T) {
+	dir := t.TempDir()
+
+	sshContext := ssh.Context{
+		ConfigDir: dir,
+	}
+
+	tests := []struct {
+		// These files exist when calling setupAutomaticSSHKeys
+		existingFiles []string
+		// These files should exist after setupAutomaticSSHKeys finishes
+		wantFinalFiles []string
+	}{
+		// Basic case: no existing keys, they should be created
+		{
+			nil,
+			[]string{automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+		},
+		// Basic case: keys already exist
+		{
+			[]string{automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+			[]string{automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+		},
+		// Backward compatibility: both old keys exist, they should be renamed
+		{
+			[]string{automaticPrivateKeyNameOld, automaticPrivateKeyNameOld + ".pub"},
+			[]string{automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+		},
+		// Backward compatibility: old private key exists but not the public key, the new keys should be created
+		{
+			[]string{automaticPrivateKeyNameOld},
+			[]string{automaticPrivateKeyNameOld, automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+		},
+		// Backward compatibility: old public key exists but not the private key, the new keys should be created
+		{
+			[]string{automaticPrivateKeyNameOld + ".pub"},
+			[]string{automaticPrivateKeyNameOld + ".pub", automaticPrivateKeyName, automaticPrivateKeyName + ".pub"},
+		},
+	}
+
+	for _, tt := range tests {
+		err := os.RemoveAll(dir)
+		if err != nil {
+			t.Errorf("Failed to clean test directory: %v", err)
+		}
+		os.MkdirAll(dir, 0711)
+		if err != nil {
+			t.Errorf("Failed to set up test directory: %v", err)
+		}
+
+		for _, file := range tt.existingFiles {
+			if _, err := os.Create(filepath.Join(dir, file)); err != nil {
+				t.Errorf("Failed to setup test files: %v", err)
+			}
+		}
+
+		keyPair, err := setupAutomaticSSHKeys(sshContext)
+		if err != nil {
+			t.Errorf("Unexpected error from setupAutomaticSSHKeys: %v", err)
+		}
+		if keyPair == nil {
+			t.Error("Unexpected nil KeyPair from setupAutomaticSSHKeys")
+		}
+
+		// Check that all the expected files are present
+		for _, file := range tt.wantFinalFiles {
+			if _, err := os.Stat(filepath.Join(dir, file)); err != nil {
+				t.Errorf("Want file %q to exist after setupAutomaticSSHKeys but it doesn't", file)
+			}
+		}
+
+		// Check that no unexpected files are present
+		allExistingFiles, err := ioutil.ReadDir(dir)
+		if err != nil {
+			t.Errorf("Failed to list files in test directory: %v", err)
+		}
+		for _, file := range allExistingFiles {
+			filename := file.Name()
+			isWantedFile := false
+			for _, wantedFile := range tt.wantFinalFiles {
+				if filename == wantedFile {
+					isWantedFile = true
+					break
+				}
+			}
+
+			if !isWantedFile {
+				t.Errorf("Unexpected file %q exists after setupAutomaticSSHKeys", filename)
+			}
+		}
+	}
+
+}
+
 func testingSSHApp() *App {
 	user := &api.User{Login: "monalisa"}
 	disabledCodespace := &api.Codespace{