From 3477bbac0e0cc772ec18f5be2b6334815faa099d Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:04:12 -0500 Subject: [PATCH] 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 --- internal/database/actions_test.go | 52 +++++++++++++++++++++++++++++++ internal/database/ssh_key.go | 16 +++++----- internal/database/ssh_key_test.go | 24 +++++++------- 3 files changed, 72 insertions(+), 20 deletions(-) diff --git a/internal/database/actions_test.go b/internal/database/actions_test.go index 17dac6f8e..e5b16f783 100644 --- a/internal/database/actions_test.go +++ b/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{}) diff --git a/internal/database/ssh_key.go b/internal/database/ssh_key.go index b03a013d1..c2d0731d0 100644 --- a/internal/database/ssh_key.go +++ b/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) diff --git a/internal/database/ssh_key_test.go b/internal/database/ssh_key_test.go index ab62078ef..a428c562b 100644 --- a/internal/database/ssh_key_test.go +++ b/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) })