Skip to content

Commit b44b5fa

Browse files
author
Earl Warren
committed
fix(sec): Forgejo Actions web routes (go-gitea#6844)
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6844 Reviewed-by: 0ko <[email protected]>
2 parents 19f3639 + caeb95c commit b44b5fa

File tree

16 files changed

+396
-42
lines changed

16 files changed

+396
-42
lines changed

models/actions/runner.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -282,27 +282,22 @@ func UpdateRunner(ctx context.Context, r *ActionRunner, cols ...string) error {
282282
}
283283

284284
// DeleteRunner deletes a runner by given ID.
285-
func DeleteRunner(ctx context.Context, id int64) error {
286-
runner, err := GetRunnerByID(ctx, id)
287-
if err != nil {
288-
return err
289-
}
290-
285+
func DeleteRunner(ctx context.Context, r *ActionRunner) error {
291286
// Replace the UUID, which was either based on the secret's first 16 bytes or an UUIDv4,
292287
// with a sequence of 8 0xff bytes followed by the little-endian version of the record's
293288
// identifier. This will prevent the deleted record's identifier from colliding with any
294289
// new record.
295290
b := make([]byte, 8)
296-
binary.LittleEndian.PutUint64(b, uint64(id))
297-
runner.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
291+
binary.LittleEndian.PutUint64(b, uint64(r.ID))
292+
r.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x",
298293
b[0], b[1], b[2], b[3], b[4], b[5], b[6], b[7])
299294

300-
err = UpdateRunner(ctx, runner, "UUID")
295+
err := UpdateRunner(ctx, r, "UUID")
301296
if err != nil {
302297
return err
303298
}
304299

305-
_, err = db.DeleteByID[ActionRunner](ctx, id)
300+
_, err = db.DeleteByID[ActionRunner](ctx, r.ID)
306301
return err
307302
}
308303

models/actions/runner_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestDeleteRunner(t *testing.T) {
3434
require.NoError(t, unittest.PrepareTestDatabase())
3535
before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID})
3636

37-
err := DeleteRunner(db.DefaultContext, recordID)
37+
err := DeleteRunner(db.DefaultContext, &ActionRunner{ID: recordID})
3838
require.NoError(t, err)
3939

4040
var after ActionRunner

models/actions/variable.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,19 +86,17 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab
8686
}
8787

8888
func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) {
89-
count, err := db.GetEngine(ctx).ID(variable.ID).Cols("name", "data").
89+
count, err := db.GetEngine(ctx).ID(variable.ID).Where("owner_id = ? AND repo_id = ?", variable.OwnerID, variable.RepoID).Cols("name", "data").
9090
Update(&ActionVariable{
9191
Name: variable.Name,
9292
Data: variable.Data,
9393
})
9494
return count != 0, err
9595
}
9696

97-
func DeleteVariable(ctx context.Context, id int64) error {
98-
if _, err := db.DeleteByID[ActionVariable](ctx, id); err != nil {
99-
return err
100-
}
101-
return nil
97+
func DeleteVariable(ctx context.Context, variableID, ownerID, repoID int64) (bool, error) {
98+
count, err := db.GetEngine(ctx).Table("action_variable").Where("id = ? AND owner_id = ? AND repo_id = ?", variableID, ownerID, repoID).Delete()
99+
return count != 0, err
102100
}
103101

104102
func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string, error) {

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3903,6 +3903,7 @@ variables.deletion.description = Removing a variable is permanent and cannot be
39033903
variables.description = Variables will be passed to certain actions and cannot be read otherwise.
39043904
variables.id_not_exist = Variable with ID %d does not exist.
39053905
variables.edit = Edit Variable
3906+
variables.not_found = Failed to find the variable.
39063907
variables.deletion.failed = Failed to remove variable.
39073908
variables.deletion.success = The variable has been removed.
39083909
variables.creation.failed = Failed to add variable.

routers/api/v1/org/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
475475
if opt.Name == "" {
476476
opt.Name = ctx.Params("variablename")
477477
}
478-
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
478+
if _, err := actions_service.UpdateVariable(ctx, v.ID, ctx.Org.Organization.ID, 0, opt.Name, opt.Value); err != nil {
479479
if errors.Is(err, util.ErrInvalidArgument) {
480480
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
481481
} else {

routers/api/v1/repo/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
414414
if opt.Name == "" {
415415
opt.Name = ctx.Params("variablename")
416416
}
417-
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
417+
if _, err := actions_service.UpdateVariable(ctx, v.ID, 0, ctx.Repo.Repository.ID, opt.Name, opt.Value); err != nil {
418418
if errors.Is(err, util.ErrInvalidArgument) {
419419
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
420420
} else {

routers/api/v1/user/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func UpdateVariable(ctx *context.APIContext) {
228228
if opt.Name == "" {
229229
opt.Name = ctx.Params("variablename")
230230
}
231-
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
231+
if _, err := actions_service.UpdateVariable(ctx, v.ID, ctx.Doer.ID, 0, opt.Name, opt.Value); err != nil {
232232
if errors.Is(err, util.ErrInvalidArgument) {
233233
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
234234
} else {

routers/web/repo/setting/runners.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func RunnerDeletePost(ctx *context.Context) {
179179
ctx.ServerError("getRunnersCtx", err)
180180
return
181181
}
182-
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
182+
actions_shared.RunnerDeletePost(ctx, ctx.ParamsInt64(":runnerid"), rCtx.OwnerID, rCtx.RepoID, rCtx.RedirectLink, rCtx.RedirectLink+url.PathEscape(ctx.Params(":runnerid")))
183183
}
184184

185185
func RedirectToDefaultSetting(ctx *context.Context) {

routers/web/repo/setting/variables.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func VariableUpdate(ctx *context.Context) {
127127
return
128128
}
129129

130-
shared.UpdateVariable(ctx, vCtx.RedirectLink)
130+
shared.UpdateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
131131
}
132132

133133
func VariableDelete(ctx *context.Context) {
@@ -136,5 +136,5 @@ func VariableDelete(ctx *context.Context) {
136136
ctx.ServerError("getVariablesCtx", err)
137137
return
138138
}
139-
shared.DeleteVariable(ctx, vCtx.RedirectLink)
139+
shared.DeleteVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
140140
}

routers/web/shared/actions/runners.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,21 @@ func RunnerResetRegistrationToken(ctx *context.Context, ownerID, repoID int64, r
142142
}
143143

144144
// RunnerDeletePost response for deleting a runner
145-
func RunnerDeletePost(ctx *context.Context, runnerID int64,
145+
func RunnerDeletePost(ctx *context.Context, runnerID, ownerID, repoID int64,
146146
successRedirectTo, failedRedirectTo string,
147147
) {
148-
if err := actions_model.DeleteRunner(ctx, runnerID); err != nil {
148+
runner, err := actions_model.GetRunnerByID(ctx, runnerID)
149+
if err != nil {
150+
ctx.ServerError("GetRunnerByID", err)
151+
return
152+
}
153+
154+
if !runner.Editable(ownerID, repoID) {
155+
ctx.NotFound("Editable", util.NewPermissionDeniedErrorf("no permission to edit this runner"))
156+
return
157+
}
158+
159+
if err := actions_model.DeleteRunner(ctx, runner); err != nil {
149160
log.Warn("DeleteRunnerPost.UpdateRunner failed: %v, url: %s", err, ctx.Req.URL)
150161
ctx.Flash.Warning(ctx.Tr("actions.runners.delete_runner_failed"))
151162

0 commit comments

Comments
 (0)