Skip to content

fix(runners): skip unregistered runners for remote tasks#3923

Closed
cursor[bot] wants to merge 1 commit into
developfrom
cursor/critical-bug-investigation-7328
Closed

fix(runners): skip unregistered runners for remote tasks#3923
cursor[bot] wants to merge 1 commit into
developfrom
cursor/critical-bug-investigation-7328

Conversation

@cursor

@cursor cursor Bot commented Jun 6, 2026

Copy link
Copy Markdown

Bug and impact

Unregistered runners (created for Terraform pre-provisioning with active: true) could be selected by RemoteJob when no registered runner had a recent heartbeat or webhook. Tasks were assigned to runners that have no auth token and cannot pick up work, causing tasks to hang in starting until max_task_duration_sec (or indefinitely when that limit is unset).

A secondary issue: POST /api/runners/register returned HTTP 200 with an empty token when the registration token matched neither a one-time smrs_ token nor the global registration token (regression from the prefix-check refactor in 3e9a9c9).

Root cause

  • af0889f2 intentionally allows unregistered runners to be created with active: true, but RemoteJob only filtered on the active DB flag and heartbeat/webhook — not on IsRegistered() (presence of auth token).
  • RegisterRunner handler lost its final else branch when registration flow was split by smrs_ prefix.

Fix and validation

  • Filter out unregistered runners in RemoteJob before runner selection (filterRegisteredRunners).
  • Return 400 for unrecognized registration tokens in RegisterRunner.
  • Updated runner_svc_test to reflect intentional active preservation for unregistered runners.
  • Added unit tests for filterRegisteredRunners.
go test ./services/tasks/... -run TestFilterRegisteredRunners -v
go test ./services/server/... -run TestRunnerService -v
Open in Web View Automation 

Unregistered runners can be created with active=true (for pre-provisioning
before registration). RemoteJob was selecting them in the fallback pass
when they had no heartbeat or webhook, assigning tasks that could never run
and hanging until timeout.

Also return 400 from POST /api/runners/register when the token matches
neither a one-time smrs_ token nor the global registration token, instead
of 200 with an empty auth token.

Co-authored-by: Denis Gukov <fiftin@outlook.com>
@fiftin fiftin marked this pull request as ready for review June 7, 2026 17:11
@fiftin fiftin closed this Jun 7, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security review

Outcome: No medium, high, or critical vulnerabilities found in this PR.

This change is security-positive: it closes two real gaps in the runner registration and remote-task dispatch paths.

Changes reviewed

  1. RegisterRunner — reject unrecognized registration tokens (400)
    Before this fix, tokens that were neither a valid smrs_ one-time token nor the configured global token fell through and returned HTTP 200 with {"token":""} and logged runner_id: 0. That is a misleading success response and a minor token-validation oracle (invalid smrs_ → 400 vs. other invalid tokens → 200). The new else branch correctly rejects those requests.

  2. filterRegisteredRunners in RemoteJob
    Unregistered runners (Token == "") could previously be selected for remote tasks when active: true (Terraform pre-provisioning). That allowed callRunnerWebhook to POST task metadata (project_id, task_id, template_id, runner_id) to a runner webhook even though the runner had no auth token and could not execute work. Filtering on IsRegistered() before selection closes that path. Runner creation still requires admin/project-manage permissions or the global registration secret (pre-existing controls).

  3. Test updates — documentation-only; no new attack surface.

Prior threads

No unresolved automation review threads were present on this PR.

Recommendation: Merge from a security perspective.

Open in Web View Automation 

Sent by Cursor Automation: Find vulnerabilities

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants