Skip to content

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

Merged
matthyx merged 2 commits into
kubescape:mainfrom
ANAMASGARD:fix/995-workflow-token-permissions
Oct 20, 2026
Merged

matthyx merged 2 commits into
kubescape:mainfrom
ANAMASGARD:fix/995-workflow-token-permissions

Conversation

@ANAMASGARD

@ANAMASGARD ANAMASGARD commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Overview

Declare read-only workflow defaults and grant write access only to the jobs that need it for CodeQL uploads, benchmark PR comments, releases, image signing and attestations. Remove unnecessary GitHub Packages access because images are published to Quay using registry credentials.

Six workflows use permissions: read-all. The four reusable workflows use the narrower contents: read default to fit within their callers' explicit permission ceilings. Required job-level grants remain explicit at every level of each nested call.

Fixes #995

Target branch

main is the intended target. Verified on 2026-10-16: upstream's default branch is main, no refs/heads/dev exists, and the release workflow handles merged PRs targeting main. The template's request to target dev cannot currently be followed. This confirms my target choice; maintainer approval is still required.

How to Test

Reproducible check and permission-path evidence: docs/WORKFLOW_PERMISSIONS.md.

python3 scripts/check-workflow-permissions.py
actionlint -shellcheck= -pyflakes= -oneline
bash scripts/check-inspektor-gadget-pin.sh
git diff --check

Local validation for 6a7779045d8c0b7eaa416b3944cb595e58c4014e:

  • Permission check passed: 10 read-only defaults, five local call edges including nested paths, and 11 operation-specific permission maps. It checks callee defaults as well as every effective job grant, from both release entrypoints and through the complete CodeQL chain.
  • Rejected 23 negative cases in temporary copies: four broad reusable defaults and 19 removals of required write grants.
  • YAML comparison confirmed only workflow permission declarations changed. The follow-up adds the checker and validation documentation.
  • Actionlint v1.7.12: no new diagnostic identities compared with base 67e455e040520a3f1f5a4ac20fbf5fe25f676d43; eight pre-existing findings remain, including obsolete CodeQL v2 runtimes.
  • Pin script: local temporary-pin marker check passed; best-effort remote ancestry lookup was inconclusive because GitHub was unreachable from the local host.
  • git diff --check and Python syntax validation passed.
  • The earlier Scorecard v5.5.0 result of 10/10 was for 8863e231. It was not rerun for this head because the binary download failed during the host network outage.
  • Application Go and cluster tests were not run locally for this workflow-permission change.

Hosted evidence and approval gate

The component suite and pin check succeeded for earlier head 8863e2312d97c3bf3f6d90c9a92597deedf4e3c7. They do not prove release or CodeQL execution, or validate this follow-up. This corrects the earlier statement that no workflows ran.

The original workflow-only diff was excluded by the release/CodeQL trigger filters. The new Python checker makes this follow-up eligible for the PR-created chain; its hosted result must be inspected separately. The merged-release path still needs a qualifying upstream event.

The evidence above validates declared permissions statically. Runtime token issuance, OIDC, registry publishing, release creation, SARIF upload, private E2E and benchmark execution remain unverified. Maintainer acceptance of this static gate or upstream execution of the affected operations remains necessary. The nonblocking observation about unused release-benchmark PR-write authority remains documented.

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
  • Added a repeatable workflow permission check
  • New and existing application unit tests pass locally with my changes

Application tests are not the local validation gate for this workflow-permission change; the exact checks and remaining hosted gate are recorded above.

Summary by CodeRabbit

  • Chores
    • Updated repository automation to explicitly scope access for benchmark, build, pull request, and security-reporting tasks.
    • Reduced requested permissions for selected test and workflow jobs.
    • Added automated checks to validate workflow permission settings and how they are passed between workflows.
  • Documentation
    • Added guidance on workflow permission scopes, credential use, and validation coverage.

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 →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fc122739-93b8-4262-a532-c3f932c71c3d
📥 Commits

Reviewing files that changed from the base of the PR and between 8863e23 and 6a77790.

📒 Files selected for processing: 6
  • .github/workflows/benchmark.yaml
  • .github/workflows/go-basic-tests.yaml
  • .github/workflows/incluster-comp-pr-created.yaml
  • .github/workflows/incluster-comp-pr-merged.yaml
  • docs/WORKFLOW_PERMISSIONS.md
  • scripts/check-workflow-permissions.py

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

Nine GitHub Actions workflows now declare workflow-level permission ceilings and selected jobs declare explicit token scopes. A new script validates permission declarations and local reusable-workflow calls. New documentation records permission scopes, validation commands, results, and limits.

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
Workflows add or adjust workflow-level permission ceilings. Selected jobs receive explicit read, write, or empty permission maps.
Permission declaration validator
scripts/check-workflow-permissions.py
The new command-line script checks permission scopes and required job maps. It follows local reusable-workflow calls, checks caller permission ceilings, and detects cycles.
Permission checks and validation record
docs/WORKFLOW_PERMISSIONS.md
The document describes workflow permissions, checker coverage, local validation commands, recorded results, and limits on static and runtime validation.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: matthyx

Merge Risk: ⚪ Minimal · up to 6a777

No actionable permission issue is established; merge after the usual workflow checks.

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: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (5 skipped: 5 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #995 requires least-privilege workflow permissions. The supplied whole-PR summary reports read-only defaults for the named workflows and additional workflows. It reports job-level grants for Cod…
Out of Scope Changes check Passed The workflow changes restrict token permissions and cover the use cases in issue #995. The validator and workflow-permission documentation test and explain those declarations. The check-ig-pin.yaml …
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely summarizes the main change: enforcing least-privilege GitHub Actions token permissions across workflows.
Full details: Linked Issues check

Explanation

Issue #995 requires least-privilege workflow permissions. The supplied whole-PR summary reports read-only defaults for the named workflows and additional workflows. It reports job-level grants for CodeQL uploads, benchmark comments, releases, image signing, and attestations. It also reports removal of unnecessary package and pull-request access and permission propagation through reusable-workflow callers. The validator and documentation support repeatable verification. The DCO requirement is administrative, not a coding requirement.

Full details: Out of Scope Changes check

Explanation

The workflow changes restrict token permissions and cover the use cases in issue #995. The validator and workflow-permission documentation test and explain those declarations. The check-ig-pin.yaml permission change applies the same repository-wide hardening objective. No unrelated product or source-code changes are shown. The incremental diff was unavailable, but the supplied whole-PR summary supports this scope assessment.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ 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.

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
Signed-off-by: Gauarv Chaudhary <137998824+ANAMASGARD@users.noreply.github.com>
@ANAMASGARD
ANAMASGARD requested a review from matthyx October 17, 2026 10:55

@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.

Rechecked head 6a7779045d8c0b7eaa416b3944cb595e58c4014e against main at 67e455e040520a3f1f5a4ac20fbf5fe25f676d43. Verdict: needs clarification on the remaining release validation gate; no confirmed correctness blockers.

The follow-up resolves the target-branch question: main is the default and dev does not exist. It also replaces broad reusable-workflow defaults with contents: read and adds a repeatable checker. The hardening remains needed for #995: the target still lacks nine explicit workflow defaults. All five nested call edges now have compatible declarations.

Fresh validation:

  • Ran python scripts/check-workflow-permissions.py in a restricted, credential-free, network-disabled container: all 10 defaults, five call edges and 11 operation maps passed. The same checker rejects the target revision and all 23 negative mutations (four broad reusable defaults and 19 removed write grants).
  • git diff --check passed. Actionlint reports the same eight existing diagnostic identities on base and head, with no new diagnostics. No application tests were run locally.
  • The exact-head PR-created run passed; its logs explicitly confirm successful SARIF upload and completed processing. This resolves the earlier CodeQL evidence gap.
  • Exact-head component tests and pin check passed.
  • The benchmark run completed and uploaded artifacts, but its PR comment received HTTP 403 (Resource not accessible by integration); the fork token had read-only PR permission. Existing error tolerance makes the run green. This is not evidence of successful comment publication or a newly introduced defect.

History rechecked across open/closed/merged PRs and issues using permissions, read-all, Token-Permissions, CodeQL and startup_failure terms (100 results/query; no query reached the cap). #135 was closed unmerged without a documented rejection; #778's caller-ceiling failure is addressed here. #710/#833 remain incorporated predecessors. #1015 has since merged and pins actions; it does not supersede permission hardening. No superseding fix was found within these search limits.

Independent correctness review found no actionable defect. Architecture remains WATCH for unused PR-write authority in release benchmarks, already documented by CodeRabbit; no duplicate inline feedback added.

One earlier gate remains: no run inspected exercises this head's merged-release/bypass chain, including OIDC, attestation, release creation and private E2E. Is the reproduced static permission check acceptable for those release-only paths, or is an upstream validation run required? Approval remains withheld pending that decision. This does not require a real production release merely to test the PR.

Immediately before this verdict, the PR remained open, non-draft and mergeable, with the same head/base and no new human review or inline comments. Branch protection still requires an approval including last-push approval. No merge, source edits or repository-setting changes were made.

@matthyx
matthyx merged commit ef6ef6a into kubescape:main Oct 20, 2026
71 of 72 checks passed
@matthyx matthyx moved this from Waiting on Author to To Archive in KS PRs tracking Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Archive

Development

Successfully merging this pull request may close these issues.

Enforce minimal token permissions across GitHub Actions workflows

2 participants