fix(runners): restore project runners list endpoint#3909
Open
cursor[bot] wants to merge 1 commit into
Open
Conversation
Commit ff963e7 accidentally replaced GetRunners with an empty array, breaking the /project/{id}/runners API for all users. Restore fetching project runners and merge active tagged global runners, excluding inactive global runners as originally intended. Co-authored-by: Denis Gukov <fiftin@outlook.com>
Author
There was a problem hiding this comment.
Security review
Outcome: No medium, high, or critical vulnerabilities found in this PR.
Scope reviewed
pro/api/projects/runners.go— restoredGetRunnersandmergeProjectRunnersListpro/api/projects/runners_test.go— unit test for merge logicpro/go.mod— test dependency only
Prior automation threads
No unresolved security-review threads were present on this PR.
Analysis
Authentication / authorization
GET /api/project/{project_id}/runnersremains behindauthenticatedAPI,ProjectMiddleware, andGetMustCanMiddleware(db.CanManageProjectResources)(api/router.go). Callers must be project members with resource-management permission;project_idcomes from the route and is validated in middleware, not from request body input.
Sensitive fields
db.Runneromitstokenandpublic_keyfrom JSON (json:"-"). The response exposes metadata (name, tags, webhook, activity) consistent with the existing admin runners API and the project runners UI.
Global runner merge
- Listing active tagged global runners alongside project runners matches the tagged-global-runner feature (#3804) and what the project Runners UI expects. Inactive and untagged globals are filtered in
mergeProjectRunnersList. This is a functional restore after commitff963e7daccidentally returned[]; it is not an auth bypass for create/update/delete (those handlers still return 404 in pro).
Injection / other classes
- No user-controlled strings reach SQL or shell sinks in the changed code; store calls use typed parameters and server-side
project.ID.
Slack summary
PR #3909 security review: clean — no medium+ issues. Restores project runner listing with intended global-runner merge; auth and secret redaction look sound.
Sent by Cursor Automation: Find vulnerabilities
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Bug and impact
Commit
ff963e7d(fix(runners): do not include inactive global runners to endpoint /project/runners) accidentally replaced the entireGetRunnershandler with[]any{}. The/api/project/{project_id}/runnersendpoint always returned an empty list, so project runner management in the UI was completely broken for all users.Root cause
The intended change was to exclude inactive global runners from the merged project/global runner list. Instead, the handler body was removed and replaced with a hard-coded empty response.
Fix
store.GetRunnersmergeProjectRunnersListunit testValidation
go test ./api/projects/...in thepromodule (passes)