Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 0 < status && 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
46 changes: 23 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,35 @@ 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(graceful.GetManager().HammerContext(), 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
migratedRepo, err := doLongTimeMigrate(graceful.GetManager().ShutdownContext(), 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