add test

Author: wxiaoguang

models/asymkey/ssh_key_authorized_keys.go

@@ -51,28 +51,32 @@ func WriteAuthorizedStringForValidKey(key *PublicKey, w io.Writer) error {
 	return err
 }
 
-// principalRegexp expresses whether a principal is considered valid.
-// This reverse engineers how sshd parses the authorized keys file,
-// see e.g. https://github.com/openssh/openssh-portable/blob/32deb00b38b4ee2b3302f261ea1e68c04e020a08/auth2-pubkeyfile.c#L221-L256
-// Any newline or # comment will be stripped when parsing, so don't allow
-// those. Also, if any space or tab is present in the principal, the part
-// proceeding this would be parsed as an option, so just avoid any whitespace
-// altogether.
-var principalRegexp = regexp.MustCompile(`^[^\s#]+$`)
+var globalVars = sync.OnceValue(func() (ret struct {
+	principalRegexp *regexp.Regexp
+},
+) {
+	// principalRegexp expresses whether a principal is considered valid.
+	// This reverse engineers how sshd parses the authorized keys file,
+	// see e.g. https://github.com/openssh/openssh-portable/blob/32deb00b38b4ee2b3302f261ea1e68c04e020a08/auth2-pubkeyfile.c#L221-L256
+	// Any newline or # comment will be stripped when parsing, so don't allow
+	// those. Also, if any space or tab is present in the principal, the part
+	// proceeding this would be parsed as an option, so just avoid any whitespace
+	// altogether.
+	ret.principalRegexp = regexp.MustCompile(`^[^\s#]+$`)
+	return ret
+})
 
 func writeAuthorizedStringForKey(key *PublicKey, w io.Writer) (keyValid bool, err error) {
 	const tpl = AuthorizedStringCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s` + "\n"
 
 	var sshKey string
 
 	if key.Type == KeyTypePrincipal {
-		principal := strings.TrimSpace(key.Content)
-
-		if matched := principalRegexp.MatchString(principal); !matched {
-			return false, fmt.Errorf("does not match %s", principalRegexp.String())
+		// TODO: actually using PublicKey to store "principal" is an abuse
+		if !globalVars().principalRegexp.MatchString(key.Content) {
+			return false, fmt.Errorf("invalid principal key: %s", key.Content)
 		}
-
-		sshKey = fmt.Sprintf("%s # user-%d", principal, key.OwnerID)
+		sshKey = fmt.Sprintf("%s # user-%d", key.Content, key.OwnerID)
 	} else {
 		pubKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key.Content))
 		if err != nil {

models/asymkey/ssh_key_authorized_keys_test.go

@@ -0,0 +1,90 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package asymkey
+
+import (
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWriteAuthorizedStringForKey(t *testing.T) {
+	defer test.MockVariableValue(&setting.AppPath, "/tmp/gitea")()
+	defer test.MockVariableValue(&setting.CustomConf, "/tmp/app.ini")()
+	writeKey := func(t *testing.T, key *PublicKey) (bool, string, error) {
+		sb := &strings.Builder{}
+		valid, err := writeAuthorizedStringForKey(key, sb)
+		return valid, sb.String(), err
+	}
+	const validKeyContent = `ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf`
+
+	testValid := func(t *testing.T, key *PublicKey, expected string) {
+		valid, content, err := writeKey(t, key)
+		assert.True(t, valid)
+		assert.Equal(t, expected, content)
+		assert.NoError(t, err)
+	}
+
+	testInvalid := func(t *testing.T, key *PublicKey) {
+		valid, content, err := writeKey(t, key)
+		assert.False(t, valid)
+		assert.Empty(t, content)
+		assert.Error(t, err)
+	}
+
+	t.Run("PublicKey", func(t *testing.T) {
+		testValid(t, &PublicKey{
+			OwnerID: 123,
+			Content: validKeyContent + " any-comment",
+			Type:    KeyTypeUser,
+		}, `# gitea public key
+command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf user-123
+`)
+	})
+
+	t.Run("PublicKeyWithNewLine", func(t *testing.T) {
+		testValid(t, &PublicKey{
+			OwnerID: 123,
+			Content: validKeyContent + "\nany-more", // the new line should be ignored
+			Type:    KeyTypeUser,
+		}, `# gitea public key
+command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf user-123
+`)
+	})
+
+	t.Run("PublicKeyInvalid", func(t *testing.T) {
+		testInvalid(t, &PublicKey{
+			OwnerID: 123,
+			Content: validKeyContent + "any-more",
+			Type:    KeyTypeUser,
+		})
+	})
+
+	t.Run("Principal", func(t *testing.T) {
+		testValid(t, &PublicKey{
+			OwnerID: 123,
+			Content: "any-content",
+			Type:    KeyTypePrincipal,
+		}, `# gitea public key
+command="/tmp/gitea --config=/tmp/app.ini serv key-0",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict any-content # user-123
+`)
+	})
+
+	t.Run("PrincipalInvalid", func(t *testing.T) {
+		testInvalid(t, &PublicKey{
+			OwnerID: 123,
+			Content: "a b",
+			Type:    KeyTypePrincipal,
+		})
+		testInvalid(t, &PublicKey{
+			OwnerID: 123,
+			Content: "a\nb",
+			Type:    KeyTypePrincipal,
+		})
+	})
+}