Compare commits

...
Author SHA1 Message Date
Nicolas f25811942c fix: re-check branch write permission for every ref in a push
The pre-receive hook cached the result of CanWriteCode() after the first
ref in a batch push, but CanMaintainerWriteToBranch depends on the current
branch name. A user holding a per-branch maintainer-edit grant (an open PR
with "allow edits from maintainers") could batch that branch with protected
branches or tags and have the cached approval reused, escalating to full
repository write. Evaluate the permission fresh for every ref; the pusher
and base permission remain cached via loadPusherAndPermission.

Assisted-by: Claude:claude-opus-4-8
2026-06-13 18:38:06 +02:00
Nicolas c0c11c551c fix: enforce single-use TOTP passcodes across all 2FA surfaces
The web 2FA login and password-reset paths validated the passcode and then
wrote LastUsedPasscode in a non-atomic read-check-write sequence, so two
parallel submissions of the same code could each authenticate (TOCTOU). The
Basic-Auth X-Gitea-OTP path never recorded the used passcode at all, letting
a captured code be replayed for its whole validity window.

Add TwoFactor.ValidateAndConsumeTOTP, which validates and atomically marks
the passcode used via a conditional UPDATE (rejecting replays and racing
duplicates), and route the web login, password-reset, and Basic-Auth paths
through it.

Assisted-by: Claude:claude-opus-4-8
2026-06-13 18:38:06 +02:00
Nicolas e99e24cb04 fix: stop trusting all proxies by default in docker app.ini templates
The Docker app.ini templates hard-coded REVERSE_PROXY_TRUSTED_PROXIES = *,
so with ENABLE_REVERSE_PROXY_AUTHENTICATION enabled any source IP reaching
the container could impersonate any user via the X-WEBAUTH-USER header.
Align the templates with the documented loopback-only default
(127.0.0.0/8,::1/128), matching app.example.ini and the in-code default.

Assisted-by: Claude:claude-opus-4-8
2026-06-13 18:38:06 +02:00
Nicolas c20df84548 fix: block fork sync when base repo is no longer readable
POST /api/v1/repos/{owner}/{repo}/merge-upstream kept importing commits
from the parent repository even after the parent was switched from public
to private, leaking commits a fork owner could no longer access directly.
Require the doer to still have read access to the base repo's code before
syncing, and map the permission error to 403 (API) / not-found (web).

Assisted-by: Claude:claude-opus-4-8
2026-06-13 17:36:35 +02:00
Nicolas 24ce5ae082 fix: enforce org visibility on organization label read endpoints
The GET /api/v1/orgs/{org}/labels and GET /api/v1/orgs/{org}/labels/{id}
endpoints did not check whether the caller could see the organization, so
labels of a private org were disclosed to non-members (and anonymously for
the list route). Add a reqOrgVisible() middleware mirroring the visibility
check used by org.Get and apply it to the labels group.
2026-06-13 17:30:10 +02:00
16 changed files with 255 additions and 34 deletions
-2
View File
@@ -51,8 +51,6 @@ ROOT_PATH = /data/gitea/log
[security]
INSTALL_LOCK = $INSTALL_LOCK
SECRET_KEY = $SECRET_KEY
REVERSE_PROXY_LIMIT = 1
REVERSE_PROXY_TRUSTED_PROXIES = *
[service]
DISABLE_REGISTRATION = $DISABLE_REGISTRATION
-2
View File
@@ -48,8 +48,6 @@ ROOT_PATH = $GITEA_WORK_DIR/data/log
[security]
INSTALL_LOCK = $INSTALL_LOCK
SECRET_KEY = $SECRET_KEY
REVERSE_PROXY_LIMIT = 1
REVERSE_PROXY_TRUSTED_PROXIES = *
[service]
DISABLE_REGISTRATION = $DISABLE_REGISTRATION
+23
View File
@@ -21,6 +21,7 @@ import (
"github.com/pquerna/otp/totp"
"golang.org/x/crypto/pbkdf2"
"xorm.io/builder"
)
//
@@ -118,6 +119,28 @@ func (t *TwoFactor) ValidateTOTP(passcode string) (bool, error) {
return totp.Validate(passcode, secretStr), nil
}
// ValidateAndConsumeTOTP validates the passcode and atomically records it as used so that the
// same passcode cannot be redeemed more than once (RFC 6238 §5.2). It returns false for an
// invalid passcode as well as for a replay, including the case where a concurrent request with
// the same passcode won the race first. All TOTP login surfaces must go through this helper.
func (t *TwoFactor) ValidateAndConsumeTOTP(ctx context.Context, passcode string) (bool, error) {
ok, err := t.ValidateTOTP(passcode)
if err != nil || !ok {
return false, err
}
// Conditional update: only a row whose stored passcode differs from this one is updated, so a
// replay (or a concurrent duplicate) matches zero rows and is rejected. The row lock taken by
// the UPDATE serializes racing requests, closing the read-validate-write TOCTOU window.
t.LastUsedPasscode = passcode
n, err := db.GetEngine(ctx).ID(t.ID).
Where(builder.Or(builder.IsNull{"last_used_passcode"}, builder.Neq{"last_used_passcode": passcode})).
Cols("last_used_passcode").Update(t)
if err != nil {
return false, err
}
return n == 1, nil
}
// NewTwoFactor creates a new two-factor authentication token.
func NewTwoFactor(ctx context.Context, t *TwoFactor) error {
_, err := db.GetEngine(ctx).Insert(t)
+47
View File
@@ -0,0 +1,47 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package auth_test
import (
"testing"
"time"
auth_model "gitea.dev/models/auth"
"gitea.dev/models/unittest"
"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestTwoFactorValidateAndConsumeTOTP(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
key, err := totp.Generate(totp.GenerateOpts{SecretSize: 40, Issuer: "gitea-test", AccountName: "consume"})
require.NoError(t, err)
tfa := &auth_model.TwoFactor{UID: 1}
require.NoError(t, tfa.SetSecret(key.Secret()))
require.NoError(t, auth_model.NewTwoFactor(t.Context(), tfa))
passcode, err := totp.GenerateCode(key.Secret(), time.Now())
require.NoError(t, err)
// first use of a valid passcode succeeds
ok, err := tfa.ValidateAndConsumeTOTP(t.Context(), passcode)
require.NoError(t, err)
assert.True(t, ok)
// replaying the same passcode is refused, even when still inside the TOTP validity window
reloaded, err := auth_model.GetTwoFactorByUID(t.Context(), tfa.UID)
require.NoError(t, err)
ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), passcode)
require.NoError(t, err)
assert.False(t, ok)
// an invalid passcode is rejected without consuming anything
ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), "000000")
require.NoError(t, err)
assert.False(t, ok)
}
+16 -1
View File
@@ -504,6 +504,21 @@ func reqOrgOwnership() func(ctx *context.APIContext) {
}
}
// reqOrgVisible requires the organization to be visible to the doer, or a site admin
func reqOrgVisible() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
if ctx.Org.Organization == nil {
setting.PanicInDevOrTesting("reqOrgVisible: unprepared context")
ctx.APIErrorInternal(errors.New("reqOrgVisible: unprepared context"))
return
}
if !organization.HasOrgOrUserVisible(ctx, ctx.Org.Organization.AsUser(), ctx.Doer) {
ctx.APIErrorNotFound()
return
}
}
}
// reqTeamMembership user should be an team member, or a site admin
func reqTeamMembership() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
@@ -1673,7 +1688,7 @@ func Routes() *web.Router {
m.Combo("/{id}").Get(reqToken(), org.GetLabel).
Patch(reqToken(), reqOrgOwnership(), bind(api.EditLabelOption{}), org.EditLabel).
Delete(reqToken(), reqOrgOwnership(), org.DeleteLabel)
})
}, reqOrgVisible())
m.Group("/hooks", func() {
m.Combo("").Get(org.ListHooks).
Post(bind(api.CreateHookOption{}), org.CreateHook)
+3
View File
@@ -1336,6 +1336,9 @@ func MergeUpstream(ctx *context.APIContext) {
} else if errors.Is(err, util.ErrNotExist) {
ctx.APIError(http.StatusNotFound, err.Error())
return
} else if errors.Is(err, util.ErrPermissionDenied) {
ctx.APIError(http.StatusForbidden, err.Error())
return
}
ctx.APIErrorInternal(err)
return
+6 -10
View File
@@ -40,9 +40,6 @@ type preReceiveContext struct {
canCreatePullRequest bool
checkedCanCreatePullRequest bool
canWriteCode bool
checkedCanWriteCode bool
protectedTags []*git_model.ProtectedTag
gotProtectedTags bool
@@ -55,14 +52,13 @@ type preReceiveContext struct {
// CanWriteCode returns true if pusher can write code
func (ctx *preReceiveContext) CanWriteCode() bool {
if !ctx.checkedCanWriteCode {
if !ctx.loadPusherAndPermission() {
return false
}
ctx.canWriteCode = issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
ctx.checkedCanWriteCode = true
if !ctx.loadPusherAndPermission() {
return false
}
return ctx.canWriteCode
// Must not be cached: CanMaintainerWriteToBranch is evaluated against ctx.branchName, which
// differs for each ref in a batch push. Caching the first result would let a per-branch
// maintainer-edit grant on one ref authorize writes to every other ref in the same push.
return issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
}
// AssertCanWriteCode returns true if pusher can write code
+66
View File
@@ -0,0 +1,66 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package private
import (
"testing"
issues_model "gitea.dev/models/issues"
"gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
"gitea.dev/services/contexttest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestPreReceiveCanWriteCodePerBranch ensures the maintainer-edit write grant is evaluated against
// the current branch on every call, instead of being cached from the first ref of a batch push.
// Otherwise a per-branch grant (an open PR with "allow edits from maintainers") could be batched
// together with a protected branch to escalate into full repository write.
func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11})
require.NoError(t, baseRepo.LoadOwner(t.Context()))
require.NoError(t, headRepo.LoadOwner(t.Context()))
// An open PR from the head repo owner, with maintainer edits allowed: this grants the base
// repo owner write access to exactly this head branch and nothing else.
pr := &issues_model.PullRequest{
Issue: &issues_model.Issue{
RepoID: baseRepo.ID,
PosterID: headRepo.OwnerID,
},
HeadRepoID: headRepo.ID,
BaseRepoID: baseRepo.ID,
HeadBranch: "granted-branch",
BaseBranch: "master",
AllowMaintainerEdit: true,
}
require.NoError(t, issues_model.NewPullRequest(t.Context(), baseRepo, pr.Issue, nil, nil, pr))
// The pusher is the base repo owner (the maintainer) with only read access on the head repo.
maintainer := baseRepo.Owner
headPerm, err := access.GetIndividualUserRepoPermission(t.Context(), headRepo, maintainer)
require.NoError(t, err)
mockCtx, _ := contexttest.MockPrivateContext(t, "/")
ctx := &preReceiveContext{
PrivateContext: mockCtx,
loadedPusher: true,
user: maintainer,
userPerm: headPerm,
}
// The granted branch must be writable...
ctx.branchName = "granted-branch"
assert.True(t, ctx.CanWriteCode())
// ...but another branch in the same push must NOT inherit that grant.
ctx.branchName = "master"
assert.False(t, ctx.CanWriteCode())
}
+3 -9
View File
@@ -58,14 +58,14 @@ func TwoFactorPost(ctx *context.Context) {
return
}
// Validate the passcode with the stored TOTP secret.
ok, err := twofa.ValidateTOTP(form.Passcode)
// Validate the passcode and atomically consume it to prevent reuse/replay.
ok, err := twofa.ValidateAndConsumeTOTP(ctx, form.Passcode)
if err != nil {
ctx.ServerError("UserSignIn", err)
return
}
if ok && twofa.LastUsedPasscode != form.Passcode {
if ok {
remember := ctx.Session.Get("twofaRemember").(bool)
u, err := user_model.GetUserByID(ctx, id)
if err != nil {
@@ -81,12 +81,6 @@ func TwoFactorPost(ctx *context.Context) {
}
}
twofa.LastUsedPasscode = form.Passcode
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
ctx.ServerError("UserSignIn", err)
return
}
_ = ctx.Session.Set(session.KeyUserHasTwoFactorAuth, true)
handleSignIn(ctx, u, remember)
return
+2 -8
View File
@@ -177,23 +177,17 @@ func ResetPasswdPost(ctx *context.Context) {
regenerateScratchToken = true
} else {
passcode := ctx.FormString("passcode")
ok, err := twofa.ValidateTOTP(passcode)
ok, err := twofa.ValidateAndConsumeTOTP(ctx, passcode)
if err != nil {
ctx.HTTPError(http.StatusInternalServerError, "ValidateTOTP", err.Error())
return
}
if !ok || twofa.LastUsedPasscode == passcode {
if !ok {
ctx.Data["IsResetForm"] = true
ctx.Data["Err_Passcode"] = true
ctx.RenderWithErrDeprecated(ctx.Tr("auth.twofa_passcode_incorrect"), tplResetPassword, nil)
return
}
twofa.LastUsedPasscode = passcode
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
ctx.ServerError("ResetPasswdPost: UpdateTwoFactor", err)
return
}
}
}
+1 -1
View File
@@ -264,7 +264,7 @@ func MergeUpstream(ctx *context.Context) {
branchName := ctx.FormString("branch")
_, err := repo_service.MergeUpstream(ctx, ctx.Doer, ctx.Repo.Repository, branchName, false)
if err != nil {
if errors.Is(err, util.ErrNotExist) {
if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrPermissionDenied) {
ctx.JSONErrorNotFound()
return
} else if pull_service.IsErrMergeConflicts(err) {
+2 -1
View File
@@ -176,7 +176,8 @@ func validateTOTP(req *http.Request, u *user_model.User) error {
}
return err
}
if ok, err := twofa.ValidateTOTP(req.Header.Get("X-Gitea-OTP")); err != nil {
// Consume the passcode atomically so a captured OTP cannot be replayed within its validity window.
if ok, err := twofa.ValidateAndConsumeTOTP(req.Context(), req.Header.Get("X-Gitea-OTP")); err != nil {
return err
} else if !ok {
return util.NewInvalidArgumentErrorf("invalid provided OTP")
+13
View File
@@ -8,7 +8,9 @@ import (
"fmt"
issue_model "gitea.dev/models/issues"
access_model "gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
user_model "gitea.dev/models/user"
"gitea.dev/modules/git"
"gitea.dev/modules/gitrepo"
@@ -26,6 +28,17 @@ func MergeUpstream(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_
if err = repo.GetBaseRepo(ctx); err != nil {
return "", err
}
// The doer must still be able to read the base repository's code. Otherwise a fork created
// while the base repo was public could keep pulling commits after it turned private.
basePerm, err := access_model.GetDoerRepoPermission(ctx, repo.BaseRepo, doer)
if err != nil {
return "", err
}
if !basePerm.CanRead(unit.TypeCode) {
return "", util.NewPermissionDeniedErrorf("permission denied to read base repo %d", repo.BaseRepo.ID)
}
divergingInfo, err := GetUpstreamDivergingInfo(ctx, repo, branch)
if err != nil {
return "", err
+48
View File
@@ -11,6 +11,7 @@ import (
"time"
auth_model "gitea.dev/models/auth"
issues_model "gitea.dev/models/issues"
org_model "gitea.dev/models/organization"
"gitea.dev/models/perm"
repo_model "gitea.dev/models/repo"
@@ -292,3 +293,50 @@ func testAPIDeleteOrgRepos(t *testing.T) {
MakeRequest(t, req, http.StatusNoContent) // The org contains no repositories, so the API should return StatusNoContent
})
}
// TestAPIOrgLabelsVisibility ensures the organization label read endpoints honor
// the organization visibility: labels of a private org must not be disclosed to
// users who cannot see the org (GHSA: unauthorized access to private org labels).
func TestAPIOrgLabelsVisibility(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// privated_org (id 23) is a private organization; user5 is its only member.
privateOrg := unittest.AssertExistsAndLoadBean(t, &org_model.Organization{ID: 23})
label := &issues_model.Label{OrgID: privateOrg.ID, Name: "internal-label", Color: "#aabbcc", Description: "private organization label"}
require.NoError(t, issues_model.NewLabel(t.Context(), label))
listURL := fmt.Sprintf("/api/v1/orgs/%s/labels", privateOrg.Name)
getURL := fmt.Sprintf("/api/v1/orgs/%s/labels/%d", privateOrg.Name, label.ID)
t.Run("NonMemberDenied", func(t *testing.T) {
// user2 is not a member of the private org and must not see its labels.
token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadOrganization)
MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(token), http.StatusNotFound)
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(token), http.StatusNotFound)
})
t.Run("AnonymousDenied", func(t *testing.T) {
MakeRequest(t, NewRequest(t, "GET", listURL), http.StatusNotFound)
MakeRequest(t, NewRequest(t, "GET", getURL), http.StatusNotFound)
})
t.Run("MemberAllowed", func(t *testing.T) {
token := getUserToken(t, "user5", auth_model.AccessTokenScopeReadOrganization)
resp := MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(token), http.StatusOK)
labels := DecodeJSON(t, resp, &[]*api.Label{})
assert.Len(t, *labels, 1)
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(token), http.StatusOK)
})
t.Run("SiteAdminAllowed", func(t *testing.T) {
token := getUserToken(t, "user1", auth_model.AccessTokenScopeReadOrganization)
MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(token), http.StatusOK)
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(token), http.StatusOK)
})
t.Run("PublicOrgStillReadable", func(t *testing.T) {
// org3 (id 3) is a public org with labels; non-members may read them.
token := getUserToken(t, "user2", auth_model.AccessTokenScopeReadOrganization)
MakeRequest(t, NewRequest(t, "GET", "/api/v1/orgs/org3/labels").AddTokenAuth(token), http.StatusOK)
})
}
+6
View File
@@ -51,6 +51,12 @@ func TestAPITwoFactor(t *testing.T) {
AddBasicAuth(user.Name)
req.Header.Set("X-Gitea-OTP", passcode)
MakeRequest(t, req, http.StatusOK)
// the same passcode must not be replayable on the basic-auth surface (RFC 6238 single-use)
req = NewRequest(t, "GET", "/api/v1/user").
AddBasicAuth(user.Name)
req.Header.Set("X-Gitea-OTP", passcode)
MakeRequest(t, req, http.StatusUnauthorized)
}
func TestBasicAuthWithWebAuthn(t *testing.T) {
@@ -171,5 +171,24 @@ func TestRepoMergeUpstream(t *testing.T) {
}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusBadRequest)
})
t.Run("BasePrivateBlocksSync", func(t *testing.T) {
// add a new commit to the base repo, then make the base repo private
require.NoError(t, createOrReplaceFileInBranch(baseUser, baseRepo, "secret.txt", "master", "private-content"))
baseRepo.IsPrivate = true
_, err := db.GetEngine(t.Context()).ID(baseRepo.ID).Cols("is_private").Update(baseRepo)
require.NoError(t, err)
defer func() {
baseRepo.IsPrivate = false
_, err := db.GetEngine(t.Context()).ID(baseRepo.ID).Cols("is_private").Update(baseRepo)
require.NoError(t, err)
}()
// the fork owner can no longer read the base repo, so syncing must be refused
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/test-repo-fork/merge-upstream", forkUser.Name), &api.MergeUpstreamRequest{
Branch: "fork-branch",
}).AddTokenAuth(token)
MakeRequest(t, req, http.StatusForbidden)
})
})
}