Repository navigation
fix: register runners with --no-default-labels - #83
Merged
Merged
Conversation
Every runner now carries only its unique per-run label, never the implicit self-hosted/Linux/X64 set. Without this any job in the repo (e.g. a fork PR's own job once approved, or from a returning contributor) could target runs-on: [self-hosted, linux, x64], wait in the queue for up to 24h and take the next runner a trusted run starts, together with its instance role, VPC/EIP access and the registration token held in IMDS user-data. Applied to both registration paths: the cold-launch bootstrap and the warm-pool (reuse: stop) per-boot register script. Requires actions/runner >= 2.299.0 (the pinned default is 2.337.0). Signed-off-by: yuriyryabikov <22548029+kurok@users.noreply.github.com>
kurok
added a commit
to namecheap/terraform-provider-namecheap
that referenced
this pull request
Oct 6, 2026
## Problem `start-runner` registers the EC2 runner with its unique label but the pinned action does not pass `--no-default-labels`, so the runner also gets `self-hosted`, `Linux` and `X64`. The `if:` gates on `start-runner` / `acceptance_test` only cover this workflow's own jobs. A fork PR can add a job with `runs-on: [self-hosted, linux, x64]`; it waits in the queue (up to 24h) and takes the runner the next time a trusted run starts one, gaining the instance role credentials, VPC subnet access, the whitelisted sandbox EIP and the registration token in user-data (which can register a rogue runner under the same label and receive the stalled `acceptance_test` job carrying `NAMECHEAP_API_KEY`). Precondition today: the repo already requires approval for workflows from **all** outside collaborators (`fork-pr-contributor-approval` = `all_external_contributors`), so this is not currently exploitable without a maintainer approving the fork's workflow. This PR closes the hole regardless of that setting. ## Fix - Pin `namecheap/ec2-github-runner` to `9d745d6` (main, merge of namecheap/ec2-github-runner#83), which registers with `--no-default-labels` on both the cold-launch and warm-pool paths. `acceptance_test` already uses `runs-on: ${{ needs.start-runner.outputs.label }}`, so nothing else changes. - `SECURITY_COMPLIANCE.md`: document the label guarantee and the fork approval policy under "Fork-safe pull-request CI". ## Verification Full CI on this branch with the new action: `Start self-hosted EC2 runner`, `Acceptance test` and `Stop self-hosted EC2 runner` all passed, so the runner is still scheduled via its unique label. ## Not done here - Just-in-time runner config instead of a registration token in user-data: the JIT API has no `--no-default-labels` equivalent and it is undocumented whether default labels are attached server-side, so it would risk reopening this hole. Follow-up on the action. - Keep the fork approval policy at `all_external_contributors`; it is a repo setting, not code. Signed-off-by: yuriyryabikov <22548029+kurok@users.noreply.github.com>
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
1 participant
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.
Problem
The runner is registered with
config.sh --labels <unique>but without--no-default-labels, so GitHub also attachesself-hosted,LinuxandX64. Consumer workflows gate their own jobs onneeds.start-runner.outputs.label, but nothing stops another job in the same repository from usingruns-on: [self-hosted, linux, x64]. On a public repo a fork PR can add such a job: it waits in the queue (up to 24h) and takes the runner the next time a trusted run starts one, which exposes the instance role credentials, VPC/subnet and whitelisted EIP access, and the registration token in IMDS user-data (reusable within its 1h validity to register a rogue runner under the same label and receive the stalled trusted job).Found while reviewing
namecheap/terraform-provider-namecheapCI (ci.ymlstart-runner/acceptance_test).Fix
--no-default-labelson bothconfig.shinvocations: the cold-launch bootstrap and the warm-pool (reuse: stop) per-boot register script.tests/userdata.test.js;tests/phone-home-detail.test.jsmarkers updated.runs-on: self-hostedno longer matches these runners.dist/rebuilt (npm ci && npm run package).Requires
actions/runner>= 2.299.0; the pinned default is 2.337.0.Why not just-in-time runner config
Switching to
generate-jitconfigwas considered for the token-in-user-data part. The JIT API has no--no-default-labelsequivalent, and whether GitHub attaches the read-only default labels to JIT runners server-side is not documented (field reports conflict), so it cannot be relied on to close the label hole this PR closes. Left as a follow-up to verify empirically; the registration token stays scoped to IMDS on the instance (hop limit 1), which only trusted-label jobs can now reach.Verification
npm test: 247 passednpm run lint: cleanverify-distshould pass (dist rebuilt from a cleannpm ci)