github/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-07-12 07:30:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix git commit committer parsing and add some tests (#35007)

Browse Source 
* Fix #34991
* Fix #34882

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao 2025-07-11 03:03:36 +08:00 committed by GitHub
parent a5a3d9b101
commit 7a15334656
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 95 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Dockerfile
View File

@ -39,7 +39,6 @@ RUN chmod 755 /tmp/local/usr/bin/entrypoint \
/tmp/local/etc/s6/.s6-svscan/* \
/go/src/code.gitea.io/gitea/gitea \
/go/src/code.gitea.io/gitea/environment-to-ini
RUN chmod 644 /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete
FROM docker.io/library/alpine:3.22
LABEL maintainer="maintainers@gitea.io"
@ -83,4 +82,3 @@ CMD ["/usr/bin/s6-svscan", "/etc/s6"]
COPY --from=build-env /tmp/local /
COPY --from=build-env /go/src/code.gitea.io/gitea/gitea /app/gitea/gitea
COPY --from=build-env /go/src/code.gitea.io/gitea/environment-to-ini /usr/local/bin/environment-to-ini
COPY --from=build-env /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete /etc/profile.d/gitea_bash_autocomplete.sh

2
Dockerfile.rootless
View File

@ -37,7 +37,6 @@ RUN chmod 755 /tmp/local/usr/local/bin/docker-entrypoint.sh \
/tmp/local/usr/local/bin/gitea \
/go/src/code.gitea.io/gitea/gitea \
/go/src/code.gitea.io/gitea/environment-to-ini
RUN chmod 644 /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete
FROM docker.io/library/alpine:3.22
LABEL maintainer="maintainers@gitea.io"
@ -72,7 +71,6 @@ RUN chown git:git /var/lib/gitea /etc/gitea
COPY --from=build-env /tmp/local /
COPY --from=build-env --chown=root:root /go/src/code.gitea.io/gitea/gitea /app/gitea/gitea
COPY --from=build-env --chown=root:root /go/src/code.gitea.io/gitea/environment-to-ini /usr/local/bin/environment-to-ini
COPY --from=build-env /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete /etc/profile.d/gitea_bash_autocomplete.sh
# git:git
USER 1000:1000

23
models/asymkey/gpg_key_commit_verification.go
View File

@ -15,25 +15,6 @@ import (
"github.com/ProtonMail/go-crypto/openpgp/packet"
)
// __________________ ________ ____ __.
// / _____/\______ \/ _____/ | |/ _|____ ___.__.
// / \ ___ | ___/ \ ___ | <_/ __ < | |
// \ \_\ \| | \ \_\ \ | | \ ___/\___ |
// \______ /|____| \______ / |____|__ \___ > ____|
// \/ \/ \/ \/\/
// _________ .__ __
// \_ ___ \ ____ _____ _____ |__|/ |_
// / \ \/ / _ \ / \ / \| \ __\
// \ \___( <_> ) Y Y \ Y Y \ || |
// \______ /\____/|__|_| /__|_| /__||__|
// \/ \/ \/
// ____ ____ .__ _____.__ __ .__
// \ \ / /___________|__|/ ____\__| ____ _____ _/ |_|__| ____ ____
// \ Y // __ \_ __ \ \ __\| |/ ___\\__ \\ __\ |/ _ \ / \
// \ /\ ___/| | \/ || | | \ \___ / __ \| | | ( <_> ) | \
// \___/ \___ >__| |__||__| |__|\___ >____ /__| |__|\____/|___| /
// \/ \/ \/ \/
// This file provides functions relating commit verification
// CommitVerification represents a commit validation of signature
@ -41,8 +22,8 @@ type CommitVerification struct {
Verified bool
Warning bool
Reason string
SigningUser *user_model.User
CommittingUser *user_model.User
SigningUser *user_model.User // if Verified, then SigningUser is non-nil
CommittingUser *user_model.User // if Verified, then CommittingUser is non-nil
SigningEmail string
SigningKey *GPGKey
SigningSSHKey *PublicKey

16
models/user/user.go
View File

@ -1166,12 +1166,6 @@ func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) ([
for _, c := range oldCommits {
user := emailUserMap.GetByEmail(c.Author.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"?
if user == nil {
user = &User{
Name: c.Author.Name,
Email: c.Author.Email,
}
}
newCommits = append(newCommits, &UserCommit{
User: user,
Commit: c,
@ -1195,12 +1189,14 @@ func GetUsersByEmails(ctx context.Context, emails []string) (*EmailUserMap, erro
needCheckEmails := make(container.Set[string])
needCheckUserNames := make(container.Set[string])
noReplyAddressSuffix := "@" + strings.ToLower(setting.Service.NoReplyAddress)
for _, email := range emails {
if strings.HasSuffix(email, "@"+setting.Service.NoReplyAddress) {
username := strings.TrimSuffix(email, "@"+setting.Service.NoReplyAddress)
needCheckUserNames.Add(strings.ToLower(username))
emailLower := strings.ToLower(email)
if noReplyUserNameLower, ok := strings.CutSuffix(emailLower, noReplyAddressSuffix); ok {
needCheckUserNames.Add(noReplyUserNameLower)
needCheckEmails.Add(emailLower)
} else {
needCheckEmails.Add(strings.ToLower(email))
needCheckEmails.Add(emailLower)
}
}

5
models/user/user_test.go
View File

@ -85,6 +85,11 @@ func TestUserEmails(t *testing.T) {
testGetUserByEmail(t, c.Email, c.UID)
})
}
t.Run("NoReplyConflict", func(t *testing.T) {
setting.Service.NoReplyAddress = "example.com"
testGetUserByEmail(t, "user1-2@example.COM", 1)
})
})
}

6
modules/git/commit.go
View File

@ -22,9 +22,9 @@ import (
type Commit struct {
Tree // FIXME: bad design, this field can be nil if the commit is from "last commit cache"
ID ObjectID // The ID of this commit object
Author *Signature
Committer *Signature
ID ObjectID
Author *Signature // never nil
Committer *Signature // never nil
CommitMessage string
Signature *CommitSignature

62
services/asymkey/commit.go
View File

@ -24,47 +24,43 @@ import (
// ParseCommitWithSignature check if signature is good against keystore.
func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model.CommitVerification {
var committer *user_model.User
if c.Committer != nil {
var err error
// Find Committer account
committer, err = user_model.GetUserByEmail(ctx, c.Committer.Email) // This finds the user by primary email or activated email so commit will not be valid if email is not
if err != nil { // Skipping not user for committer
committer = &user_model.User{
Name: c.Committer.Name,
Email: c.Committer.Email,
}
// We can expect this to often be an ErrUserNotExist. in the case
// it is not, however, it is important to log it.
if !user_model.IsErrUserNotExist(err) {
log.Error("GetUserByEmail: %v", err)
return &asymkey_model.CommitVerification{
CommittingUser: committer,
Verified: false,
Reason: "gpg.error.no_committer_account",
}
}
committer, err := user_model.GetUserByEmail(ctx, c.Committer.Email)
if err != nil && !user_model.IsErrUserNotExist(err) {
log.Error("GetUserByEmail: %v", err)
return &asymkey_model.CommitVerification{
Verified: false,
Reason: "gpg.error.no_committer_account", // this error is not right, but such error should seldom happen
}
}
return ParseCommitWithSignatureCommitter(ctx, c, committer)
}
// ParseCommitWithSignatureCommitter parses a commit's GPG or SSH signature.
// If the commit is singed by an instance key, then committer can be nil.
// If the signature exists, even if committer is nil, the returned CommittingUser will be a non-nil fake user.
func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification {
// If no signature just report the committer
// If no signature, just report the committer
if c.Signature == nil {
return &asymkey_model.CommitVerification{
CommittingUser: committer,
Verified: false, // Default value
Reason: "gpg.error.not_signed_commit", // Default value
Verified: false,
Reason: "gpg.error.not_signed_commit",
}
}
// If this a SSH signature handle it differently
if strings.HasPrefix(c.Signature.Signature, "-----BEGIN SSH SIGNATURE-----") {
return ParseCommitWithSSHSignature(ctx, c, committer)
// to support instance key, we need a fake committer user (not really needed, but legacy code accesses the committer without nil-check)
if committer == nil {
committer = &user_model.User{
Name: c.Committer.Name,
Email: c.Committer.Email,
}
}
if strings.HasPrefix(c.Signature.Signature, "-----BEGIN SSH SIGNATURE-----") {
return parseCommitWithSSHSignature(ctx, c, committer)
}
return parseCommitWithGPGSignature(ctx, c, committer)
}
func parseCommitWithGPGSignature(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification {
// Parsing signature
sig, err := asymkey_model.ExtractSignature(c.Signature.Signature)
if err != nil { // Skipping failed to extract sign
@ -165,7 +161,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi
}
if err := gpgSettings.LoadPublicKeyContent(); err != nil {
log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err)
} else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil {
} else if commitVerification := verifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil {
if commitVerification.Reason == asymkey_model.BadSignature {
defaultReason = asymkey_model.BadSignature
} else {
@ -180,7 +176,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi
} else if defaultGPGSettings == nil {
log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String())
} else if defaultGPGSettings.Sign {
if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil {
if commitVerification := verifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil {
if commitVerification.Reason == asymkey_model.BadSignature {
defaultReason = asymkey_model.BadSignature
} else {
@ -295,7 +291,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s
}
}
func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification {
func verifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification {
// First try to find the key in the db
if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email); commitVerification != nil {
return commitVerification
@ -375,8 +371,8 @@ func verifySSHCommitVerificationByInstanceKey(c *git.Commit, committerUser, sign
return verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, sshPubKey, committerUser, signerUser, committerGitEmail)
}
// ParseCommitWithSSHSignature check if signature is good against keystore.
func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUser *user_model.User) *asymkey_model.CommitVerification {
// parseCommitWithSSHSignature check if signature is good against keystore.
func parseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUser *user_model.User) *asymkey_model.CommitVerification {
// Now try to associate the signature with the committer, if present
if committerUser.ID != 0 {
keys, err := db.Find[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{

2
services/asymkey/commit_test.go
View File

@ -41,7 +41,7 @@ Initial commit with signed file
Name: "User Two",
Email: "user2@example.com",
}
ret := ParseCommitWithSSHSignature(t.Context(), commit, committingUser)
ret := parseCommitWithSSHSignature(t.Context(), commit, committingUser)
require.NotNil(t, ret)
assert.True(t, ret.Verified)
assert.False(t, ret.Warning)

7
services/git/commit.go
View File

@ -35,13 +35,6 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository,
for _, c := range oldCommits {
committerUser := emailUsers.GetByEmail(c.Committer.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"?
if committerUser == nil {
committerUser = &user_model.User{
Name: c.Committer.Name,
Email: c.Committer.Email,
}
}
signCommit := &asymkey_model.SignCommit{
UserCommit: c,
Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committerUser),

2
templates/repo/commit_page.tmpl
View File

@ -147,7 +147,7 @@
<div class="flex-text-inline">
{{if or (ne .Commit.Committer.Name .Commit.Author.Name) (ne .Commit.Committer.Email .Commit.Author.Email)}}
<span class="text grey">{{ctx.Locale.Tr "repo.diff.committed_by"}}</span>
{{if ne .Verification.CommittingUser.ID 0}}
{{if and .Verification.CommittingUser .Verification.CommittingUser.ID}}
{{ctx.AvatarUtils.Avatar .Verification.CommittingUser 20}}
<a href="{{.Verification.CommittingUser.HomeLink}}"><strong>{{.Commit.Committer.Name}}</strong></a>
{{else}}

2
templates/repo/commits_list.tmpl
View File

@ -16,7 +16,7 @@
<td class="author">
<div class="tw-flex">
{{$userName := .Author.Name}}
{{if and .User (gt .User.ID 0)}} /* User with id == 0 is a fake user from git author */
{{if .User}}
{{if and .User.FullName DefaultShowFullName}}
{{$userName = .User.FullName}}
{{end}}

74
tests/integration/repo_commits_test.go
View File

@ -24,39 +24,59 @@ import (
func TestRepoCommits(t *testing.T) {
defer tests.PrepareTestEnv(t)()
session := loginUser(t, "user2")
// Request repository commits page
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
resp := session.MakeRequest(t, req, http.StatusOK)
t.Run("CommitList", func(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo16/commits/branch/master")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitURL, exists := doc.doc.Find("#commits-table .commit-id-short").Attr("href")
assert.True(t, exists)
assert.NotEmpty(t, commitURL)
}
func Test_ReposGitCommitListNotMaster(t *testing.T) {
defer tests.PrepareTestEnv(t)()
session := loginUser(t, "user2")
req := NewRequest(t, "GET", "/user2/repo16/commits/branch/master")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
var commits []string
doc.doc.Find("#commits-table .commit-id-short").Each(func(i int, s *goquery.Selection) {
commitURL, _ := s.Attr("href")
commits = append(commits, path.Base(commitURL))
var commits, userHrefs []string
doc := NewHTMLParser(t, resp.Body)
doc.doc.Find("#commits-table .commit-id-short").Each(func(i int, s *goquery.Selection) {
commits = append(commits, path.Base(s.AttrOr("href", "")))
})
doc.doc.Find("#commits-table .author-wrapper").Each(func(i int, s *goquery.Selection) {
userHrefs = append(userHrefs, s.AttrOr("href", ""))
})
assert.Equal(t, []string{"69554a64c1e6030f051e5c3f94bfbd773cd6a324", "27566bd5738fc8b4e3fef3c5e72cce608537bd95", "5099b81332712fe655e34e8dd63574f503f61811"}, commits)
assert.Equal(t, []string{"/user2", "/user21", "/user2"}, userHrefs)
})
assert.Equal(t, []string{"69554a64c1e6030f051e5c3f94bfbd773cd6a324", "27566bd5738fc8b4e3fef3c5e72cce608537bd95", "5099b81332712fe655e34e8dd63574f503f61811"}, commits)
var userHrefs []string
doc.doc.Find("#commits-table .author-wrapper").Each(func(i int, s *goquery.Selection) {
userHref, _ := s.Attr("href")
userHrefs = append(userHrefs, userHref)
t.Run("LastCommit", func(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo16")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "")
authorHref := doc.doc.Find(".latest-commit .author-wrapper").AttrOr("href", "")
assert.Equal(t, "/user2/repo16/commit/69554a64c1e6030f051e5c3f94bfbd773cd6a324", commitHref)
assert.Equal(t, "/user2", authorHref)
})
t.Run("CommitListNonExistingCommiter", func(t *testing.T) {
// check the commit list for a repository with no gitea user
// * commit 985f0301dba5e7b34be866819cd15ad3d8f508ee (branch2)
// * Author: 6543 <6543@obermui.de>
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/branch2")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitHref := doc.doc.Find("#commits-table tr:first-child .commit-id-short").AttrOr("href", "")
assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref)
authorElem := doc.doc.Find("#commits-table tr:first-child .author-wrapper")
assert.Equal(t, "6543", authorElem.Text())
assert.Equal(t, "span", authorElem.Nodes[0].Data)
})
t.Run("LastCommitNonExistingCommiter", func(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo1/src/branch/branch2")
resp := session.MakeRequest(t, req, http.StatusOK)
doc := NewHTMLParser(t, resp.Body)
commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "")
assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref)
authorElem := doc.doc.Find(".latest-commit .author-wrapper")
assert.Equal(t, "6543", authorElem.Text())
assert.Equal(t, "span", authorElem.Nodes[0].Data)
})
assert.Equal(t, []string{"/user2", "/user21", "/user2"}, userHrefs)
}
func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {