fix(sec): web route delete runner

The web route to delete action runners did not check if the ID that was
given belonged to the context it was requested in, this made it possible
to delete every existing runner of a instance by a authenticated user.

The code was reworked to ensure that the caller of the delete
runner function retrieved the runner by ID and then checks if it belongs
to the context it was requested in, although this is not an optimal
solution it is consistent with the context checking of other code for
runners.

(cherry picked from commit 567765be03d56d6c8c36bb783c330c8ca70b1aca)

Conflicts:
	models/actions/runner.go
	models/actions/runner_test.go
  conflicting UUID bug fix and associated tests do not exist
This commit is contained in:
Gusted 2025-01-25 08:51:59 +01:00 committed by Earl Warren
parent 0f1cf6dade
commit 5b30b7dc6f
Failed to generate hash of commit
3 changed files with 16 additions and 9 deletions

View file

@ -252,12 +252,8 @@ 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 {
if _, err := GetRunnerByID(ctx, id); err != nil {
return err
}
_, err := db.DeleteByID[ActionRunner](ctx, id)
func DeleteRunner(ctx context.Context, r *ActionRunner) error {
_, err := db.DeleteByID[ActionRunner](ctx, r.ID)
return err
}

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

@ -143,10 +143,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"))