diff --git a/routers/api/v1/shared/runners.go b/routers/api/v1/shared/runners.go index d42f330d1c..e9834aff9f 100644 --- a/routers/api/v1/shared/runners.go +++ b/routers/api/v1/shared/runners.go @@ -67,6 +67,28 @@ func ListRunners(ctx *context.APIContext, ownerID, repoID int64) { ctx.JSON(http.StatusOK, &res) } +func getRunnerByID(ctx *context.APIContext, ownerID, repoID, runnerID int64) (*actions_model.ActionRunner, bool) { + if ownerID != 0 && repoID != 0 { + setting.PanicInDevOrTesting("ownerID and repoID should not be both set") + } + + runner, err := actions_model.GetRunnerByID(ctx, runnerID) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.APIErrorNotFound("Runner not found") + } else { + ctx.APIErrorInternal(err) + } + return nil, false + } + + if !runner.EditableInContext(ownerID, repoID) { + ctx.APIErrorNotFound("No permission to access this runner") + return nil, false + } + return runner, true +} + // GetRunner get the runner for api route validated ownerID and repoID // ownerID == 0 and repoID == 0 means any runner including global runners // ownerID == 0 and repoID != 0 means any runner for the given repo @@ -77,13 +99,8 @@ func GetRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) { if ownerID != 0 && repoID != 0 { setting.PanicInDevOrTesting("ownerID and repoID should not be both set") } - runner, err := actions_model.GetRunnerByID(ctx, runnerID) - if err != nil { - ctx.APIErrorNotFound(err) - return - } - if !runner.EditableInContext(ownerID, repoID) { - ctx.APIErrorNotFound("No permission to get this runner") + runner, ok := getRunnerByID(ctx, ownerID, repoID, runnerID) + if !ok { return } ctx.JSON(http.StatusOK, convert.ToActionRunner(ctx, runner)) @@ -96,20 +113,12 @@ func GetRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) { // ownerID != 0 and repoID != 0 undefined behavior // Access rights are checked at the API route level func DeleteRunner(ctx *context.APIContext, ownerID, repoID, runnerID int64) { - if ownerID != 0 && repoID != 0 { - setting.PanicInDevOrTesting("ownerID and repoID should not be both set") - } - runner, err := actions_model.GetRunnerByID(ctx, runnerID) - if err != nil { - ctx.APIErrorInternal(err) - return - } - if !runner.EditableInContext(ownerID, repoID) { - ctx.APIErrorNotFound("No permission to delete this runner") + runner, ok := getRunnerByID(ctx, ownerID, repoID, runnerID) + if !ok { return } - err = actions_model.DeleteRunner(ctx, runner.ID) + err := actions_model.DeleteRunner(ctx, runner.ID) if err != nil { ctx.APIErrorInternal(err) return diff --git a/tests/integration/api_actions_runner_test.go b/tests/integration/api_actions_runner_test.go index ace7aa381a..87b82e2ce9 100644 --- a/tests/integration/api_actions_runner_test.go +++ b/tests/integration/api_actions_runner_test.go @@ -329,4 +329,12 @@ func testActionsRunnerRepo(t *testing.T) { req := NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/user2/repo1/actions/runners/%d", 34349)).AddTokenAuth(token) MakeRequest(t, req, http.StatusNotFound) }) + + t.Run("DeleteAdminRunnerNotFoundUnknownID", func(t *testing.T) { + userUsername := "user2" + token := getUserToken(t, userUsername, auth_model.AccessTokenScopeWriteRepository) + // Verify delete a runner by unknown id is not found + req := NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/user2/repo1/actions/runners/%d", 4384797347934)).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) + }) }