Skip to content

fix(ci): strip unused CI-runner privileges - #4063

Merged
Tanguille merged 1 commit into
mainfrom
fix/strip-ci-runner-privileges
Jul 20, 2026
Merged

fix(ci): strip unused CI-runner privileges#4063
Tanguille merged 1 commit into
mainfrom
fix/strip-ci-runner-privileges

Conversation

@Tanguille

Copy link
Copy Markdown
Owner

Removes the cluster-runner ServiceAccount's cluster-admin ClusterRoleBinding — confirmed no workflow in .github/ invokes kubectl, and ARC's containerMode:kubernetes pod/PVC permissions come from its own auto-generated cluster-runner-gha-rs-manager Role, not this binding. Drops the Talos ServiceAccount from os:admin to os:reader, since its only current use is talosctl image pull in image-pull.yaml. Also adds the same-repo gate (already used by agent-pr-review.yaml and labeler.yaml) to the image-pull pull job so fork PRs can't run modified steps on the self-hosted cluster-runner.

Verification: kustomize build --enable-helm on the runners/cluster dir renders cleanly with the cluster-admin binding gone and os:reader in place; confirmed via kubectl that the pod/secret/serviceaccount/role/rolebinding perms for containerMode:kubernetes live on a separate ARC-managed Role/SA. Post-merge: if talosctl image pull starts failing with os:reader, bump to os:operator (talos-mcp already runs fine on os:reader, so os:reader is expected to be sufficient).

Remove cluster-runner's cluster-admin ClusterRoleBinding (no workflow
invokes kubectl; ARC's containerMode:kubernetes pod/PVC perms come
from its own auto-generated manager Role, unrelated to this SA).
Drop the Talos ServiceAccount from os:admin to os:reader since its
only use is talosctl image pull. Gate the image-pull job with the
same fork same-repo check used by agent-pr-review and labeler, so
fork PRs can't run modified steps on the self-hosted runner.
@tanguille-cluster

Copy link
Copy Markdown
@@ spec.roles.0 @@
# talos.dev/v1alpha1/ServiceAccount/actions-runner-system/cluster-runner
! ± value change
- os:admin
+ os:reader

@@ (root level) @@
# rbac.authorization.k8s.io/v1/ClusterRoleBinding/cluster-runner
! - one document removed:
- apiVersion: rbac.authorization.k8s.io/v1
- kind: ClusterRoleBinding
- metadata:
-   name: cluster-runner
-   labels:
-     kustomize.toolkit.fluxcd.io/name: actions-runner-controller-runners
-     kustomize.toolkit.fluxcd.io/namespace: actions-runner-system
- roleRef:
-   name: cluster-admin
-   apiGroup: rbac.authorization.k8s.io
-   kind: ClusterRole
- subjects:
- - name: cluster-runner
-   kind: ServiceAccount
-   namespace: actions-runner-system

@github-actions

Copy link
Copy Markdown

AI Automated Review

Analysis engine: qwen-3.6-fast@http://litellm.ai.svc.cluster.local/v1 (openai)

Recommendation: Approve — this PR correctly reduces the CI runner's privileges and hardens the workflow against fork abuse.

Change-by-change findings

.github/workflows/image-pull.yaml

  • Adds a same-repo gate (github.event.pull_request.head.repo.full_name == github.repository) to the pull job that runs on the self-hosted cluster-runner. This prevents fork PRs from executing arbitrary steps on infrastructure with cluster-level access. The pattern is consistent with other workflows in the repo (e.g., agent-pr-review.yaml, labeler.yaml).

kubernetes/apps/actions-runner-system/actions-runner-controller/runners/cluster/rbac.yaml

  • Removes the cluster-admin ClusterRoleBinding from the cluster-runner ServiceAccount. The PR body confirms ARC's containerMode: kubernetes permissions are handled by a separate auto-generated Role/SA, not this binding. This is a significant security improvement.
  • Downgrades the Talos ServiceAccount role from os:admin to os:reader. The only documented use of this SA is talosctl image pull in the image-pull.yaml workflow. Per Talos RBAC docs, os:reader grants read-only access to node state; talosctl image pull interacts with the CRI runtime via the OS API. If this proves insufficient post-merge, the PR notes indicate bumping to os:operator as the next step.

Standards Compliance

  • Changes follow repository conventions: small, focused scope; no secrets committed; Conventional Commit title used.
  • No shell scripts were modified, so shellcheck requirements do not apply here.

Must-Check Items

  1. Review auth flow for regression: Verified. The removal of cluster-admin does not affect ARC's internal pod/PVC management because those permissions come from ARC's own auto-generated Role/SA (cluster-runner-gha-rs-manager), as confirmed in the PR body. The remaining Talos SA (os:reader) is sufficient for talosctl image pull based on the author's verification and Talos RBAC documentation.
  2. Verify session token handling is correct: Not applicable. This change modifies static RBAC bindings and workflow conditions; no session tokens or authentication flows are introduced or altered in the diff.

Tool Harness Findings

No tool harness output was provided in the corpus.

Unknowns or Needs Verification

  • The PR body states that os:reader is expected to be sufficient for talosctl image pull, but if it fails post-merge, a follow-up to os:operator may be needed. This is noted in the PR description and is an acceptable risk given the conservative downgrade.

@Tanguille
Tanguille merged commit ea543ce into main Jul 20, 2026
16 checks passed
@Tanguille
Tanguille deleted the fix/strip-ci-runner-privileges branch July 20, 2026 18:32
Tanguille added a commit that referenced this pull request Jul 20, 2026
os:reader (set in #4063) lacks the ImagePull API; /machine.MachineService/ImagePull
requires os:admin or os:operator, so every image-pull run failed with
PermissionDenied. os:operator is the minimal role that includes it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant