Переглянути джерело

Add ED25519 test coverage and refactor SSH key parsing tests (#8107)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: unknwon <2946214+unknwon@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Copilot 1 тиждень тому
батько
коміт
3477bbac0e

+ 52 - 0
internal/database/actions_test.go

@@ -142,6 +142,11 @@ func actionsCommitRepo(t *testing.T, ctx context.Context, s *ActionsStore) {
 	now := time.Unix(1588568886, 0).UTC()
 
 	conf.SetMockSSH(t, conf.SSHOpts{})
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
 
 	t.Run("new commit", func(t *testing.T) {
 		t.Cleanup(func() {
@@ -432,6 +437,12 @@ func actionsListByUser(t *testing.T, ctx context.Context, s *ActionsStore) {
 }
 
 func actionsMergePullRequest(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -477,6 +488,12 @@ func actionsMergePullRequest(t *testing.T, ctx context.Context, s *ActionsStore)
 }
 
 func actionsMirrorSyncCreate(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -518,6 +535,12 @@ func actionsMirrorSyncCreate(t *testing.T, ctx context.Context, s *ActionsStore)
 }
 
 func actionsMirrorSyncDelete(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -559,6 +582,12 @@ func actionsMirrorSyncDelete(t *testing.T, ctx context.Context, s *ActionsStore)
 }
 
 func actionsMirrorSyncPush(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -624,6 +653,12 @@ func actionsMirrorSyncPush(t *testing.T, ctx context.Context, s *ActionsStore) {
 }
 
 func actionsNewRepo(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -703,6 +738,11 @@ func actionsPushTag(t *testing.T, ctx context.Context, s *ActionsStore) {
 	// to the mock server because this function holds a lock.
 	conf.SetMockServer(t, conf.ServerOpts{})
 	conf.SetMockSSH(t, conf.SSHOpts{})
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
 
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
@@ -796,6 +836,12 @@ func actionsPushTag(t *testing.T, ctx context.Context, s *ActionsStore) {
 }
 
 func actionsRenameRepo(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	repo, err := newReposStore(s.db).Create(ctx,
@@ -833,6 +879,12 @@ func actionsRenameRepo(t *testing.T, ctx context.Context, s *ActionsStore) {
 }
 
 func actionsTransferRepo(t *testing.T, ctx context.Context, s *ActionsStore) {
+	conf.SetMockUI(t, conf.UIOpts{
+		User: conf.UIUserOpts{
+			NewsFeedPagingNum: 20,
+		},
+	})
+
 	alice, err := newUsersStore(s.db).Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
 	require.NoError(t, err)
 	bob, err := newUsersStore(s.db).Create(ctx, "bob", "bob@example.com", CreateUserOptions{})

+ 8 - 8
internal/database/ssh_key.go

@@ -175,8 +175,8 @@ func parseKeyString(content string) (string, error) {
 
 // writeTmpKeyFile writes key content to a temporary file
 // and returns the name of that file, along with any possible errors.
-func writeTmpKeyFile(content string) (string, error) {
-	tmpFile, err := os.CreateTemp(conf.SSH.KeyTestPath, "gogs_keytest")
+func writeTmpKeyFile(content, keyTestPath string) (string, error) {
+	tmpFile, err := os.CreateTemp(keyTestPath, "gogs_keytest")
 	if err != nil {
 		return "", errors.Newf("TempFile: %v", err)
 	}
@@ -188,15 +188,15 @@ func writeTmpKeyFile(content string) (string, error) {
 	return tmpFile.Name(), nil
 }
 
-// SSHKeyGenParsePublicKey extracts key type and length using ssh-keygen.
-func SSHKeyGenParsePublicKey(key string) (string, int, error) {
-	tmpName, err := writeTmpKeyFile(key)
+// SSHKeygenParsePublicKey extracts key type and length using ssh-keygen.
+func SSHKeygenParsePublicKey(key, keyTestPath, keygenPath string) (string, int, error) {
+	tmpName, err := writeTmpKeyFile(key, keyTestPath)
 	if err != nil {
 		return "", 0, errors.Newf("writeTmpKeyFile: %v", err)
 	}
 	defer os.Remove(tmpName)
 
-	stdout, stderr, err := process.Exec("SSHKeyGenParsePublicKey", conf.SSH.KeygenPath, "-lf", tmpName)
+	stdout, stderr, err := process.Exec("SSHKeygenParsePublicKey", keygenPath, "-lf", tmpName)
 	if err != nil {
 		return "", 0, errors.Newf("fail to parse public key: %s - %s", err, stderr)
 	}
@@ -301,8 +301,8 @@ func CheckPublicKeyString(content string) (_ string, err error) {
 		fnName = "SSHNativeParsePublicKey"
 		keyType, length, err = SSHNativeParsePublicKey(content)
 	} else {
-		fnName = "SSHKeyGenParsePublicKey"
-		keyType, length, err = SSHKeyGenParsePublicKey(content)
+		fnName = "SSHKeygenParsePublicKey"
+		keyType, length, err = SSHKeygenParsePublicKey(content, conf.SSH.KeyTestPath, conf.SSH.KeygenPath)
 	}
 	if err != nil {
 		return "", errors.Newf("%s: %v", fnName, err)

+ 12 - 12
internal/database/ssh_key_test.go

@@ -4,13 +4,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"gogs.io/gogs/internal/conf"
+	"github.com/stretchr/testify/require"
 )
 
-func Test_SSHParsePublicKey(t *testing.T) {
-	// TODO: Refactor SSHKeyGenParsePublicKey to accept a tempPath and remove this init.
-	conf.MustInit("")
+func TestSSHParsePublicKey(t *testing.T) {
+	tempPath := t.TempDir()
 	tests := []struct {
 		name      string
 		content   string
@@ -53,20 +51,22 @@ func Test_SSHParsePublicKey(t *testing.T) {
 			expType:   "ecdsa",
 			expLength: 521,
 		},
+		{
+			name:      "ed25519-256",
+			content:   "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICGYutovQfTewtcodVN1E1UUzMk4GQfiRI5ZoP/kTlDb nocomment",
+			expType:   "ed25519",
+			expLength: 256,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			typ, length, err := SSHNativeParsePublicKey(test.content)
-			if err != nil {
-				t.Fatal(err)
-			}
+			require.NoError(t, err)
 			assert.Equal(t, test.expType, typ)
 			assert.Equal(t, test.expLength, length)
 
-			typ, length, err = SSHKeyGenParsePublicKey(test.content)
-			if err != nil {
-				t.Fatal(err)
-			}
+			typ, length, err = SSHKeygenParsePublicKey(test.content, tempPath, "ssh-keygen")
+			require.NoError(t, err)
 			assert.Equal(t, test.expType, typ)
 			assert.Equal(t, test.expLength, length)
 		})