Skip to content

fix(ci): enforce least-privilege GitHub Actions token permissions - #1011

Open
ANAMASGARD wants to merge 1 commit into
kubescape:mainfrom
ANAMASGARD:fix/995-workflow-token-permissions
Open

ANAMASGARD wants to merge 1 commit into
kubescape:mainfrom
ANAMASGARD:fix/995-workflow-token-permissions

Conversation

@ANAMASGARD

@ANAMASGARD ANAMASGARD commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Overview

Add permissions: read-all defaults to all GitHub Actions workflows that lacked them. Grant required permissions at the job level and pass them through reusable workflow callers for CodeQL uploads, benchmark PR comments, release creation, image signing, and attestation.

Remove unnecessary package-write and PR permissions, and grant artifact-metadata access required by the pinned attestation action.

Fixes #995

How to Test

  • OpenSSF Scorecard v5.5.0 Token-Permissions: reproduced 0/10 before the changes and verified 10/10 afterward.
  • Verified read-only defaults across all ten workflows and permission compatibility across all six reusable-workflow call paths.
  • Confirmed only permission configurations changed.
  • git diff --check passed.
  • Actionlint reported no new findings; eight pre-existing findings remain.

Live GitHub Actions execution has not been verified: pushing the branch did not trigger any workflows.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • New and existing unit tests pass locally with my changes

The unchecked items do not apply to this workflow-permissions change; validation is described above.


The template requests `dev` as the target branch for non-documentation changes.


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Chores**
  * Updated automated workflows to declare and scope the access they need, including restricting permissions for selected test jobs.
  * Adjusted permissions for benchmark, pull request, build, and security-reporting tasks.
  * These changes affect repository automation and do not add or change user-facing features.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Declare read-only defaults across GitHub Actions workflows and pass required job scopes through reusable workflow callers. Limit CodeQL, benchmark comments, release creation, and image attestation to their required permissions.

Fixes kubescape#995

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 0f1e00e5-b4de-46e5-94ed-c41fb90a8e54

📥 Commits

Reviewing files that changed from the base of the PR and between 67e455e and 8863e23.

📒 Files selected for processing (9)
  • .github/workflows/benchmark.yaml
  • .github/workflows/bypass.yaml
  • .github/workflows/check-ig-pin.yaml
  • .github/workflows/component-tests.yaml
  • .github/workflows/go-basic-tests.yaml
  • .github/workflows/incluster-comp-pr-created.yaml
  • .github/workflows/incluster-comp-pr-merged.yaml
  • .github/workflows/pr-created.yaml
  • .github/workflows/pr-merged.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Nine GitHub Actions workflows now set workflow-level read-all permissions. Selected jobs declare explicit token permissions, including jobs with no permissions.

Changes

Workflow token permissions

Layer / File(s) Summary
Workflow defaults and job permissions
.github/workflows/benchmark.yaml, .github/workflows/bypass.yaml, .github/workflows/check-ig-pin.yaml, .github/workflows/component-tests.yaml, .github/workflows/go-basic-tests.yaml, .github/workflows/incluster-comp-pr-created.yaml, .github/workflows/incluster-comp-pr-merged.yaml, .github/workflows/pr-created.yaml, .github/workflows/pr-merged.yaml
The workflows add top-level read-all permissions. Selected jobs receive explicit read, write, or no permissions. Several existing job scopes are changed.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: matthyx

Merge Risk: ⚪ Minimal · up to 8863e

The reviewed changes have no demonstrated workflow regression. The possible permission expansion for PR-driven jobs depends on repository settings that are not established here, so no concrete merge blocker is confirmed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8863e

The changes narrow several permissions, but release-triggered benchmarks gain pull-request write access even though their comment step does not run for that event. This modestly increases the impact of compromised benchmark execution. The merged-main trigger limits exposure, and live execution and previous default token settings remain unverified.

Retained concerns

  • Low · security · inferred: The merged-release benchmark gains repository pull-request write authority despite its comment step being restricted to pull_request events. Benchmark scripts and downloaded dependencies execute within that privileged job. Compromising that execution could therefore mutate repository pull requests beyond the intended report target, with greater authority than the merge-base caller allowed. The merged-main trigger limits direct exposure to unmerged contributor code.
Security review details

Security Blast Radius

  • inferred — The newly established benchmark token authority can affect pull requests throughout the originating repository, not only the PR selected by the comment step. Its declared scopes do not grant repository contents-write or cross-repository authority. Existing registry, signing and dispatch credentials are separate boundaries whose effective privileges were not validated.

Security Findings and Attack Paths

  • inferred — A compromised benchmark dependency or executable script could use the job token independently of the comment-step condition. The primary release route now permits PR writes where its merge-base caller permitted only reads. This is a bounded attack path, not evidence of exploitation; the direct pull_request route's before/after authority cannot be established without historical default settings.

Trust Boundaries and Controls

  • observed — The primary release entrypoint requires a closed, merged PR targeting main. Its nested benchmark additionally requires a release label and a successful image-build dependency. Benchmark checkout does not explicitly select the contributor's PR-head ref. These controls distinguish this route from the benchmark's separate direct pull_request trigger.

Resilience and Maintainability Implications

  • observed — Ref-scoped cancellation, always-run report and artifact steps, and nonfatal comment publication are unchanged. The source configures a comment-tag input, but inspected evidence does not establish action-level deduplication, atomic publication or cancellation cleanup guarantees. Incomplete reports and partial publication are existing behaviors rather than demonstrated regressions.

Hardening Proposals

  • proposed — Use event-specific permission requirements so merged-release benchmarks do not receive unused PR-write authority. Where PR publication is required, consider separating report publication from contributor-controlled benchmark execution and binding publication to a validated originating PR.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #995 requires top-level permissions: read-all and job-level permissions for required write operations. The whole-PR diff adds permissions: read-all to all eight workflows named by the issue.…
Out of Scope Changes check ✅ Passed The changes are limited to GitHub Actions permission declarations. The additional change in check-ig-pin.yaml applies the same workflow permission hardening objective. Job-level permission additions…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enforcing least-privilege GitHub Actions token permissions across CI workflows.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matthyx matthyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 8863e2312d97c3bf3f6d90c9a92597deedf4e3c7 against main at 67e455e040520a3f1f5a4ac20fbf5fe25f676d43. Verdict: needs clarification; no confirmed correctness blocker found.

The hardening is needed for #995: parsing the target finds nine of ten workflows without top-level permissions; the head has none missing. All five local reusable-workflow edges have compatible explicit job scopes. Release creation keeps contents-write, Quay uses registry credentials, and private E2E operations use the App token. The pinned attestation action documents artifact-metadata-write for storage records; its absence previously produced a warning, not necessarily an attestation failure.

History: #135 proposed read-all in two workflows and closed unmerged, with no documented maintainer rejection. #778 removed benchmark job permissions to fix a release startup failure; this PR addresses that objection by updating both callers. #710 and #833 are incorporated permission/provenance predecessors. Open #1015 overlaps workflow files but pins actions rather than replacing this change. Searches across PR states and issues used permissions, read-all, Token-Permissions, benchmark, CodeQL and startup_failure, capped at 100/query; the broad benchmark search hit that cap. No superseding fix found within those limits.

Validation: git diff --check passed. YAML parsing and a caller/callee permission-map check passed. actionlint -shellcheck= -pyflakes= -oneline reported the same eight diagnostics on base and head, with no new findings. The component suite and pin check succeeded for this exact head. Go tests, Scorecard, release, benchmark and CodeQL execution were not run locally; repository scripts were not executed. The green checks do not validate the changed release/CodeQL chains because their triggers exclude workflow-only changes. No inline threads or human reviews were present when rechecked.

Two points before approval:

  • Please confirm main is the intended target despite the repository PR template requesting dev for non-documentation changes.
  • Please provide evidence validating the changed release and CodeQL permission paths, or maintainer confirmation of an acceptable static-validation gate for this workflow-only change. The description's statement that no workflows ran should also be updated to distinguish the successful component/pin checks from these untested paths.

The unused PR-write capability on the release benchmark is a nonblocking architecture WATCH already covered by CodeRabbit; I am not duplicating that feedback. No source changes or merge actions were performed.

This branch has not been deployed

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

Labels

None yet

Projects

Status: Waiting on Author

Development

Successfully merging this pull request may close these issues.

Enforce minimal token permissions across GitHub Actions workflows

2 participants