fix(sec): Forgejo Actions web routes (#6839)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6839
This commit is contained in:
Earl Warren 2025-02-08 06:21:18 +00:00
commit 6ef900899e
16 changed files with 396 additions and 42 deletions

View file

@ -282,27 +282,22 @@ func UpdateRunner(ctx context.Context, r *ActionRunner, cols ...string) error {
}
// DeleteRunner deletes a runner by given ID.
func DeleteRunner(ctx context.Context, id int64) error {
runner, err := GetRunnerByID(ctx, id)
if err != nil {
return err
}
func DeleteRunner(ctx context.Context, r *ActionRunner) error {
// Replace the UUID, which was either based on the secret's first 16 bytes or an UUIDv4,
// with a sequence of 8 0xff bytes followed by the little-endian version of the record's
// identifier. This will prevent the deleted record's identifier from colliding with any
// new record.
b := make([]byte, 8)
binary.LittleEndian.PutUint64(b, uint64(id))
runner.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
binary.LittleEndian.PutUint64(b, uint64(r.ID))
r.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
b[0], b[1], b[2], b[3], b[4], b[5], b[6], b[7])
err = UpdateRunner(ctx, runner, "UUID")
err := UpdateRunner(ctx, r, "UUID")
if err != nil {
return err
}
_, err = db.DeleteByID[ActionRunner](ctx, id)
_, err = db.DeleteByID[ActionRunner](ctx, r.ID)
return err
}

View file

@ -34,7 +34,7 @@ func TestDeleteRunner(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
err := DeleteRunner(db.DefaultContext, recordID)
err := DeleteRunner(db.DefaultContext, &ActionRunner{ID: recordID})
require.NoError(t, err)
var after ActionRunner

View file

@ -86,7 +86,7 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab
}
func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) {
count, err := db.GetEngine(ctx).ID(variable.ID).Cols("name", "data").
count, err := db.GetEngine(ctx).ID(variable.ID).Where("owner_id = ? AND repo_id = ?", variable.OwnerID, variable.RepoID).Cols("name", "data").
Update(&ActionVariable{
Name: variable.Name,
Data: variable.Data,
@ -94,11 +94,9 @@ func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error)
return count != 0, err
}
func DeleteVariable(ctx context.Context, id int64) error {
if _, err := db.DeleteByID[ActionVariable](ctx, id); err != nil {
return err
}
return nil
func DeleteVariable(ctx context.Context, variableID, ownerID, repoID int64) (bool, error) {
count, err := db.GetEngine(ctx).Table("action_variable").Where("id = ? AND owner_id = ? AND repo_id = ?", variableID, ownerID, repoID).Delete()
return count != 0, err
}
func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string, error) {

View file

@ -3894,6 +3894,7 @@ variables.deletion.description = Removing a variable is permanent and cannot be
variables.description = Variables will be passed to certain actions and cannot be read otherwise.
variables.id_not_exist = Variable with ID %d does not exist.
variables.edit = Edit Variable
variables.not_found = Failed to find the variable.
variables.deletion.failed = Failed to remove variable.
variables.deletion.success = The variable has been removed.
variables.creation.failed = Failed to add variable.

View file

@ -450,7 +450,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.Params("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
if _, err := actions_service.UpdateVariable(ctx, v.ID, ctx.Org.Organization.ID, 0, opt.Name, opt.Value); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View file

@ -414,7 +414,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.Params("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
if _, err := actions_service.UpdateVariable(ctx, v.ID, 0, ctx.Repo.Repository.ID, opt.Name, opt.Value); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View file

@ -228,7 +228,7 @@ func UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.Params("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
if _, err := actions_service.UpdateVariable(ctx, v.ID, ctx.Doer.ID, 0, opt.Name, opt.Value); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View file

@ -179,7 +179,7 @@ func RunnerDeletePost(ctx *context.Context) {
ctx.ServerError("getRunnersCtx", err)
return
}
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.OwnerID, rCtx.RepoID, rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
}
func RedirectToDefaultSetting(ctx *context.Context) {

View file

@ -127,7 +127,7 @@ func VariableUpdate(ctx *context.Context) {
return
}
shared.UpdateVariable(ctx, vCtx.RedirectLink)
shared.UpdateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
}
func VariableDelete(ctx *context.Context) {
@ -136,5 +136,5 @@ func VariableDelete(ctx *context.Context) {
ctx.ServerError("getVariablesCtx", err)
return
}
shared.DeleteVariable(ctx, vCtx.RedirectLink)
shared.DeleteVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
}

View file

@ -142,10 +142,21 @@ func RunnerResetRegistrationToken(ctx *context.Context, ownerID, repoID int64, r
}
// RunnerDeletePost response for deleting a runner
func RunnerDeletePost(ctx *context.Context, runnerID int64,
func RunnerDeletePost(ctx *context.Context, runnerID, ownerID, repoID int64,
successRedirectTo, failedRedirectTo string,
) {
if err := actions_model.DeleteRunner(ctx, runnerID); err != nil {
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
if err != nil {
ctx.ServerError("GetRunnerByID", err)
return
}
if !runner.Editable(ownerID, repoID) {
ctx.NotFound("Editable", util.NewPermissionDeniedErrorf("no permission to edit this runner"))
return
}
if err := actions_model.DeleteRunner(ctx, runner); err != nil {
log.Warn("DeleteRunnerPost.UpdateRunner failed: %v, url: %s", err, ctx.Req.URL)
ctx.Flash.Warning(ctx.Tr("actions.runners.delete_runner_failed"))

View file

@ -39,25 +39,33 @@ func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL str
ctx.JSONRedirect(redirectURL)
}
func UpdateVariable(ctx *context.Context, redirectURL string) {
func UpdateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
id := ctx.ParamsInt64(":variable_id")
form := web.GetForm(ctx).(*forms.EditVariableForm)
if ok, err := actions_service.UpdateVariable(ctx, id, form.Name, form.Data); err != nil || !ok {
log.Error("UpdateVariable: %v", err)
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
if ok, err := actions_service.UpdateVariable(ctx, id, ownerID, repoID, form.Name, form.Data); err != nil || !ok {
if !ok {
ctx.JSONError(ctx.Tr("actions.variables.not_found"))
} else {
log.Error("UpdateVariable: %v", err)
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
}
return
}
ctx.Flash.Success(ctx.Tr("actions.variables.update.success"))
ctx.JSONRedirect(redirectURL)
}
func DeleteVariable(ctx *context.Context, redirectURL string) {
func DeleteVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
id := ctx.ParamsInt64(":variable_id")
if err := actions_service.DeleteVariableByID(ctx, id); err != nil {
log.Error("Delete variable [%d] failed: %v", id, err)
ctx.JSONError(ctx.Tr("actions.variables.deletion.failed"))
if ok, err := actions_model.DeleteVariable(ctx, id, ownerID, repoID); err != nil || !ok {
if !ok {
ctx.JSONError(ctx.Tr("actions.variables.not_found"))
} else {
log.Error("Delete variable [%d] failed: %v", id, err)
ctx.JSONError(ctx.Tr("actions.variables.deletion.failed"))
}
return
}
ctx.Flash.Success(ctx.Tr("actions.variables.deletion.success"))

View file

@ -31,7 +31,7 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data strin
return v, nil
}
func UpdateVariable(ctx context.Context, variableID int64, name, data string) (bool, error) {
func UpdateVariable(ctx context.Context, variableID, ownerID, repoID int64, name, data string) (bool, error) {
if err := secret_service.ValidateName(name); err != nil {
return false, err
}
@ -41,16 +41,14 @@ func UpdateVariable(ctx context.Context, variableID int64, name, data string) (b
}
return actions_model.UpdateVariable(ctx, &actions_model.ActionVariable{
ID: variableID,
Name: strings.ToUpper(name),
Data: util.ReserveLineBreakForTextarea(data),
ID: variableID,
Name: strings.ToUpper(name),
Data: util.ReserveLineBreakForTextarea(data),
OwnerID: ownerID,
RepoID: repoID,
})
}
func DeleteVariableByID(ctx context.Context, variableID int64) error {
return actions_model.DeleteVariable(ctx, variableID)
}
func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name string) error {
if err := secret_service.ValidateName(name); err != nil {
return err
@ -69,7 +67,8 @@ func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name strin
return err
}
return actions_model.DeleteVariable(ctx, v.ID)
_, err = actions_model.DeleteVariable(ctx, v.ID, ownerID, repoID)
return err
}
func GetVariable(ctx context.Context, opts actions_model.FindVariablesOpts) (*actions_model.ActionVariable, error) {

View file

@ -0,0 +1,150 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
package integration
import (
"fmt"
"net/http"
"testing"
actions_model "code.gitea.io/gitea/models/actions"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
forgejo_context "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
)
func TestActionVariablesModification(t *testing.T) {
defer tests.AddFixtures("tests/integration/fixtures/TestActionVariablesModification")()
defer tests.PrepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
userVariable := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: 1001, OwnerID: user.ID})
userURL := "/user/settings/actions/variables"
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
orgVariable := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: 1002, OwnerID: org.ID})
orgURL := "/org/" + org.Name + "/settings/actions/variables"
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, OwnerID: user.ID})
repoVariable := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: 1003, RepoID: repo.ID})
repoURL := "/" + repo.FullName() + "/settings/actions/variables"
admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true})
globalVariable := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: 1004}, "owner_id = 0 AND repo_id = 0")
adminURL := "/admin/actions/variables"
adminSess := loginUser(t, admin.Name)
adminCSRF := GetCSRF(t, adminSess, "/")
sess := loginUser(t, user.Name)
csrf := GetCSRF(t, sess, "/")
type errorJSON struct {
Error string `json:"errorMessage"`
}
test := func(t *testing.T, fail bool, baseURL string, id int64) {
defer tests.PrintCurrentTest(t, 1)()
t.Helper()
sess := sess
csrf := csrf
if baseURL == adminURL {
sess = adminSess
csrf = adminCSRF
}
req := NewRequestWithValues(t, "POST", baseURL+fmt.Sprintf("/%d/edit", id), map[string]string{
"_csrf": csrf,
"name": "glados_quote",
"data": "I'm fine. Two plus two is...ten, in base four, I'm fine!",
})
if fail {
resp := sess.MakeRequest(t, req, http.StatusBadRequest)
var error errorJSON
DecodeJSON(t, resp, &error)
assert.EqualValues(t, "Failed to find the variable.", error.Error)
} else {
sess.MakeRequest(t, req, http.StatusOK)
flashCookie := sess.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.EqualValues(t, "success%3DThe%2Bvariable%2Bhas%2Bbeen%2Bedited.", flashCookie.Value)
}
req = NewRequestWithValues(t, "POST", baseURL+fmt.Sprintf("/%d/delete", id), map[string]string{
"_csrf": csrf,
})
if fail {
resp := sess.MakeRequest(t, req, http.StatusBadRequest)
var error errorJSON
DecodeJSON(t, resp, &error)
assert.EqualValues(t, "Failed to find the variable.", error.Error)
} else {
sess.MakeRequest(t, req, http.StatusOK)
flashCookie := sess.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.EqualValues(t, "success%3DThe%2Bvariable%2Bhas%2Bbeen%2Bremoved.", flashCookie.Value)
}
}
t.Run("User variable", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, userVariable.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, userVariable.ID)
})
t.Run("Admin", func(t *testing.T) {
test(t, true, adminURL, userVariable.ID)
})
t.Run("User", func(t *testing.T) {
test(t, false, userURL, userVariable.ID)
})
})
t.Run("Organisation variable", func(t *testing.T) {
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, orgVariable.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, orgVariable.ID)
})
t.Run("Admin", func(t *testing.T) {
test(t, true, adminURL, userVariable.ID)
})
t.Run("Organisation", func(t *testing.T) {
test(t, false, orgURL, orgVariable.ID)
})
})
t.Run("Repository variable", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, repoVariable.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, repoVariable.ID)
})
t.Run("Admin", func(t *testing.T) {
test(t, true, adminURL, userVariable.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, false, repoURL, repoVariable.ID)
})
})
t.Run("Global variable", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, globalVariable.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, globalVariable.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, globalVariable.ID)
})
t.Run("Admin", func(t *testing.T) {
test(t, false, adminURL, globalVariable.ID)
})
})
}

View file

@ -0,0 +1,31 @@
-
id: 1001
name: GLADOS_QUOTE
owner_id: 2
repo_id: 0
data: ""
created_unix: 1737000000
-
id: 1002
name: GLADOS_QUOTE
owner_id: 3
repo_id: 0
data: ""
created_unix: 1737000001
-
id: 1003
name: GLADOS_QUOTE
owner_id: 0
repo_id: 1
data: ""
created_unix: 1737000002
-
id: 1004
name: GLADOS_QUOTE
owner_id: 0
repo_id: 0
data: ""
created_unix: 1737000003

View file

@ -0,0 +1,31 @@
-
id: 1001
uuid: "43b5d4d3-401b-42f9-94df-a9d45b447b82"
name: "User runner"
owner_id: 2
repo_id: 0
deleted: 0
-
id: 1002
uuid: "bdc77f4f-2b2b-442d-bd44-e808f4306347"
name: "Organisation runner"
owner_id: 3
repo_id: 0
deleted: 0
-
id: 1003
uuid: "9268bc8c-efbf-4dbe-aeb5-945733cdd098"
name: "Repository runner"
owner_id: 0
repo_id: 1
deleted: 0
-
id: 1004
uuid: "fb857e63-c0ce-4571-a6c9-fde26c128073"
name: "Global runner"
owner_id: 0
repo_id: 0
deleted: 0

View file

@ -0,0 +1,130 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: GPL-3.0-or-later
package integration
import (
"fmt"
"net/http"
"testing"
actions_model "code.gitea.io/gitea/models/actions"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
forgejo_context "code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
)
func TestRunnerModification(t *testing.T) {
defer tests.AddFixtures("tests/integration/fixtures/TestRunnerModification")()
defer tests.PrepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
userRunner := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: 1001, OwnerID: user.ID})
userURL := "/user/settings/actions/runners"
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
orgRunner := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: 1002, OwnerID: org.ID})
orgURL := "/org/" + org.Name + "/settings/actions/runners"
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1, OwnerID: user.ID})
repoRunner := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: 1003, RepoID: repo.ID})
repoURL := "/" + repo.FullName() + "/settings/actions/runners"
admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{IsAdmin: true})
globalRunner := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: 1004}, "owner_id = 0 AND repo_id = 0")
adminURL := "/admin/actions/runners"
adminSess := loginUser(t, admin.Name)
adminCSRF := GetCSRF(t, adminSess, "/")
sess := loginUser(t, user.Name)
csrf := GetCSRF(t, sess, "/")
test := func(t *testing.T, fail bool, baseURL string, id int64) {
defer tests.PrintCurrentTest(t, 1)()
t.Helper()
sess := sess
csrf := csrf
if baseURL == adminURL {
sess = adminSess
csrf = adminCSRF
}
req := NewRequestWithValues(t, "POST", baseURL+fmt.Sprintf("/%d", id), map[string]string{
"_csrf": csrf,
"description": "New Description",
})
if fail {
sess.MakeRequest(t, req, http.StatusNotFound)
} else {
sess.MakeRequest(t, req, http.StatusSeeOther)
flashCookie := sess.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.EqualValues(t, "success%3DRunner%2Bupdated%2Bsuccessfully", flashCookie.Value)
}
req = NewRequestWithValues(t, "POST", baseURL+fmt.Sprintf("/%d/delete", id), map[string]string{
"_csrf": csrf,
})
if fail {
sess.MakeRequest(t, req, http.StatusNotFound)
} else {
sess.MakeRequest(t, req, http.StatusOK)
flashCookie := sess.GetCookie(forgejo_context.CookieNameFlash)
assert.NotNil(t, flashCookie)
assert.EqualValues(t, "success%3DRunner%2Bdeleted%2Bsuccessfully", flashCookie.Value)
}
}
t.Run("User runner", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, userRunner.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, userRunner.ID)
})
t.Run("User", func(t *testing.T) {
test(t, false, userURL, userRunner.ID)
})
})
t.Run("Organisation runner", func(t *testing.T) {
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, orgRunner.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, orgRunner.ID)
})
t.Run("Organisation", func(t *testing.T) {
test(t, false, orgURL, orgRunner.ID)
})
})
t.Run("Repository runner", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, repoRunner.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, repoRunner.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, false, repoURL, repoRunner.ID)
})
})
t.Run("Global runner", func(t *testing.T) {
t.Run("Organisation", func(t *testing.T) {
test(t, true, orgURL, globalRunner.ID)
})
t.Run("User", func(t *testing.T) {
test(t, true, userURL, globalRunner.ID)
})
t.Run("Repository", func(t *testing.T) {
test(t, true, repoURL, globalRunner.ID)
})
t.Run("Admin", func(t *testing.T) {
test(t, false, adminURL, globalRunner.ID)
})
})
}