Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/issues/issue_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ func applySubscribedCondition(sess *xorm.Session, subscriberID int64) {
),
builder.Eq{"issue.poster_id": subscriberID},
builder.In("issue.repo_id", builder.
Select("id").
Select("repo_id").
From("watch").
Where(builder.And(builder.Eq{"user_id": subscriberID},
builder.In("mode", repo_model.WatchModeNormal, repo_model.WatchModeAuto))),
Expand Down
6 changes: 6 additions & 0 deletions models/issues/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@ func TestIssues(t *testing.T) {
},
[]int64{2},
},
{
issues_model.IssuesOptions{
SubscriberID: 11,
},
[]int64{11, 5, 9, 8, 3, 2, 1},
},
} {
issues, err := issues_model.Issues(t.Context(), &test.Opts)
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion modules/packages/debian/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var (
// https://www.debian.org/doc/debian-policy/ch-controlfields.html#source
namePattern = regexp.MustCompile(`\A[a-z0-9][a-z0-9+-.]+\z`)
// https://www.debian.org/doc/debian-policy/ch-controlfields.html#version
versionPattern = regexp.MustCompile(`\A(?:[0-9]:)?[a-zA-Z0-9.+~]+(?:-[a-zA-Z0-9.+-~]+)?\z`)
versionPattern = regexp.MustCompile(`\A(?:(0|[1-9][0-9]*):)?[a-zA-Z0-9.+~]+(?:-[a-zA-Z0-9.+-~]+)?\z`)
)

type Package struct {
Expand Down
8 changes: 8 additions & 0 deletions modules/packages/debian/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,12 @@ func TestParseControlFile(t *testing.T) {
assert.Equal(t, []string{"a", "b"}, p.Metadata.Dependencies)
assert.Equal(t, full, p.Control)
})

t.Run("ValidVersions", func(t *testing.T) {
for _, version := range []string{"1.0", "0:1.2", "9:1.0", "10:1.0", "900:1a.2b-x-y_z~1+2"} {
p, err := ParseControlFile(buildContent("testpkg", version, "amd64"))
assert.NoError(t, err, "ParseControlFile with version %q", version)
assert.NotNil(t, p)
}
})
}
3 changes: 2 additions & 1 deletion modules/web/routing/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
}
logf := logInfo
// lower the log level for some specific requests, in most cases these logs are not useful
if strings.HasPrefix(req.RequestURI, "/assets/") /* static assets */ ||
if status > 0 && status < 400 &&
strings.HasPrefix(req.RequestURI, "/assets/") /* static assets */ ||
req.RequestURI == "/user/events" /* Server-Sent Events (SSE) handler */ ||
req.RequestURI == "/api/actions/runner.v1.RunnerService/FetchTask" /* Actions Runner polling */ {
logf = logTrace
Expand Down
48 changes: 25 additions & 23 deletions routers/api/v1/repo/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package repo

import (
"bytes"
gocontext "context"
"errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -173,7 +173,7 @@ func Migrate(ctx *context.APIContext) {
opts.AWSSecretAccessKey = form.AWSSecretAccessKey
}

repo, err := repo_service.CreateRepositoryDirectly(ctx, ctx.Doer, repoOwner, repo_service.CreateRepoOptions{
createdRepo, err := repo_service.CreateRepositoryDirectly(ctx, ctx.Doer, repoOwner, repo_service.CreateRepoOptions{
Name: opts.RepoName,
Description: opts.Description,
OriginalURL: form.CloneAddr,
Expand All @@ -187,35 +187,37 @@ func Migrate(ctx *context.APIContext) {
return
}

opts.MigrateToRepoID = repo.ID
opts.MigrateToRepoID = createdRepo.ID

defer func() {
if e := recover(); e != nil {
var buf bytes.Buffer
fmt.Fprintf(&buf, "Handler crashed with error: %v", log.Stack(2))

err = errors.New(buf.String())
}

if err == nil {
notify_service.MigrateRepository(ctx, ctx.Doer, repoOwner, repo)
return
}

if repo != nil {
if errDelete := repo_service.DeleteRepositoryDirectly(ctx, repo.ID); errDelete != nil {
log.Error("DeleteRepository: %v", errDelete)
doLongTimeMigrate := func(ctx gocontext.Context, doer *user_model.User) (migratedRepo *repo_model.Repository, retErr error) {
defer func() {
if e := recover(); e != nil {
log.Error("MigrateRepository panic: %v\n%s", e, log.Stack(2))
if errDelete := repo_service.DeleteRepositoryDirectly(ctx, createdRepo.ID); errDelete != nil {
log.Error("Unable to delete repo after MigrateRepository panic: %v", errDelete)
}
retErr = errors.New("MigrateRepository panic") // no idea why it would happen, just legacy code
}
}()

migratedRepo, err := migrations.MigrateRepository(ctx, doer, repoOwner.Name, opts, nil)
if err != nil {
return nil, err
}
}()
notify_service.MigrateRepository(ctx, doer, repoOwner, migratedRepo)
return migratedRepo, nil
}

if repo, err = migrations.MigrateRepository(graceful.GetManager().HammerContext(), ctx.Doer, repoOwner.Name, opts, nil); err != nil {
// use a background context, don't cancel the migration even if the client goes away
// HammerContext doesn't seem right (from https://github.com/go-gitea/gitea/pull/9335/files)
// There are other abuses, maybe most HammerContext abuses should be fixed together in the future.
migratedRepo, err := doLongTimeMigrate(graceful.GetManager().HammerContext(), ctx.Doer)
if err != nil {
handleMigrateError(ctx, repoOwner, err)
return
}

log.Trace("Repository migrated: %s/%s", repoOwner.Name, form.RepoName)
ctx.JSON(http.StatusCreated, convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeAdmin}))
ctx.JSON(http.StatusCreated, convert.ToRepo(ctx, migratedRepo, access_model.Permission{AccessMode: perm.AccessModeAdmin}))
}

func handleMigrateError(ctx *context.APIContext, repoOwner *user_model.User, err error) {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func getPullInfo(ctx *context.Context) (issue *issues_model.Issue, ok bool) {
ctx.Data["Issue"] = issue

if !issue.IsPull {
ctx.NotFound(nil)
ctx.Redirect(issue.Link())
return nil, false
}

Expand Down
34 changes: 0 additions & 34 deletions services/actions/variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ package actions

import (
"context"
"regexp"

actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
secret_service "code.gitea.io/gitea/services/secrets"
)
Expand All @@ -18,10 +16,6 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data, desc
return nil, err
}

if err := envNameCIRegexMatch(name); err != nil {
return nil, err
}

v, err := actions_model.InsertVariable(ctx, ownerID, repoID, name, util.ReserveLineBreakForTextarea(data), description)
if err != nil {
return nil, err
Expand All @@ -35,10 +29,6 @@ func UpdateVariableNameData(ctx context.Context, variable *actions_model.ActionV
return false, err
}

if err := envNameCIRegexMatch(variable.Name); err != nil {
return false, err
}

variable.Data = util.ReserveLineBreakForTextarea(variable.Data)

return actions_model.UpdateVariableCols(ctx, variable, "name", "data", "description")
Expand All @@ -49,14 +39,6 @@ func DeleteVariableByID(ctx context.Context, variableID int64) error {
}

func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name string) error {
if err := secret_service.ValidateName(name); err != nil {
return err
}

if err := envNameCIRegexMatch(name); err != nil {
return err
}

v, err := GetVariable(ctx, actions_model.FindVariablesOpts{
OwnerID: ownerID,
RepoID: repoID,
Expand All @@ -79,19 +61,3 @@ func GetVariable(ctx context.Context, opts actions_model.FindVariablesOpts) (*ac
}
return vars[0], nil
}

// some regular expression of `variables` and `secrets`
// reference to:
// https://docs.github.com/en/actions/learn-github-actions/variables#naming-conventions-for-configuration-variables
// https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets
var (
forbiddenEnvNameCIRx = regexp.MustCompile("(?i)^CI")
)

func envNameCIRegexMatch(name string) error {
if forbiddenEnvNameCIRx.MatchString(name) {
log.Error("Env Name cannot be ci")
return util.NewInvalidArgumentErrorf("env name cannot be ci")
}
return nil
}
4 changes: 0 additions & 4 deletions services/secrets/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) erro
}

func DeleteSecretByName(ctx context.Context, ownerID, repoID int64, name string) error {
if err := ValidateName(name); err != nil {
return err
}

s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{
OwnerID: ownerID,
RepoID: repoID,
Expand Down
24 changes: 16 additions & 8 deletions services/secrets/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,29 @@ package secrets

import (
"regexp"
"strings"
"sync"

"code.gitea.io/gitea/modules/util"
)

// https://docs.github.com/en/actions/learn-github-actions/variables#naming-conventions-for-configuration-variables
// https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets
var (
namePattern = regexp.MustCompile("(?i)^[A-Z_][A-Z0-9_]*$")
forbiddenPrefixPattern = regexp.MustCompile("(?i)^GIT(EA|HUB)_")

ErrInvalidName = util.NewInvalidArgumentErrorf("invalid secret name")
)
var globalVars = sync.OnceValue(func() (ret struct {
namePattern, forbiddenPrefixPattern *regexp.Regexp
},
) {
ret.namePattern = regexp.MustCompile("(?i)^[A-Z_][A-Z0-9_]*$")
ret.forbiddenPrefixPattern = regexp.MustCompile("(?i)^GIT(EA|HUB)_")
return ret
})

func ValidateName(name string) error {
if !namePattern.MatchString(name) || forbiddenPrefixPattern.MatchString(name) {
return ErrInvalidName
vars := globalVars()
if !vars.namePattern.MatchString(name) ||
vars.forbiddenPrefixPattern.MatchString(name) ||
strings.EqualFold(name, "CI") /* CI is always set to true in GitHub Actions*/ {
return util.NewInvalidArgumentErrorf("invalid variable or secret name")
}
return nil
}
29 changes: 29 additions & 0 deletions services/secrets/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestValidateName(t *testing.T) {
cases := []struct {
name string
valid bool
}{
{"FOO", true},
{"FOO1_BAR2", true},
{"_FOO", true}, // really? why support this
{"1FOO", false},
{"giteA_xx", false},
{"githuB_xx", false},
{"cI", false},
}
for _, c := range cases {
err := ValidateName(c.name)
assert.Equal(t, c.valid, err == nil, "ValidateName(%q)", c.name)
}
}
4 changes: 2 additions & 2 deletions templates/mail/repo/release.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@
<ul>
{{if not .DisableDownloadSourceArchives}}
<li>
<a href="{{.Release.Repo.Link}}/archive/{{.Release.TagName | PathEscapeSegments}}.zip" rel="nofollow"><strong>{{.locale.Tr "mail.release.download.zip"}}</strong></a>
<a href="{{.Release.Repo.HTMLURL}}/archive/{{.Release.TagName | PathEscapeSegments}}.zip" rel="nofollow"><strong>{{.locale.Tr "mail.release.download.zip"}}</strong></a>
</li>
<li>
<a href="{{.Release.Repo.Link}}/archive/{{.Release.TagName | PathEscapeSegments}}.tar.gz" rel="nofollow"><strong>{{.locale.Tr "mail.release.download.targz"}}</strong></a>
<a href="{{.Release.Repo.HTMLURL}}/archive/{{.Release.TagName | PathEscapeSegments}}.tar.gz" rel="nofollow"><strong>{{.locale.Tr "mail.release.download.targz"}}</strong></a>
</li>
{{end}}
{{if .Release.Attachments}}
Expand Down
4 changes: 0 additions & 4 deletions tests/integration/api_repo_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,5 @@ func TestAPIRepoSecrets(t *testing.T) {
req = NewRequest(t, "DELETE", url).
AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)

req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/actions/secrets/000", repo.FullName())).
AddTokenAuth(token)
MakeRequest(t, req, http.StatusBadRequest)
})
}
4 changes: 0 additions & 4 deletions tests/integration/api_user_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,5 @@ func TestAPIUserSecrets(t *testing.T) {
req = NewRequest(t, "DELETE", url).
AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)

req = NewRequest(t, "DELETE", "/api/v1/user/actions/secrets/000").
AddTokenAuth(token)
MakeRequest(t, req, http.StatusBadRequest)
})
}
8 changes: 6 additions & 2 deletions tests/integration/issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,13 @@ func TestIssueRedirect(t *testing.T) {
resp = session.MakeRequest(t, req, http.StatusSeeOther)
assert.Equal(t, "/user2/repo1/pulls/2", test.RedirectURL(resp))

repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: 1, Type: unit.TypeIssues})
// by the way, test PR redirection
req = NewRequest(t, "GET", "/user2/repo1/pulls/1")
resp = session.MakeRequest(t, req, http.StatusSeeOther)
assert.Equal(t, "/user2/repo1/issues/1", test.RedirectURL(resp))

// disable issue unit, it will be reset
// disable issue unit
repoUnit := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoUnit{RepoID: 1, Type: unit.TypeIssues})
_, err := db.DeleteByID[repo_model.RepoUnit](t.Context(), repoUnit.ID)
assert.NoError(t, err)

Expand Down