Skip to content

ci: add explicit top-level permissions to all workflows (least privilege)#39

Merged
entlein merged 2 commits intomainfrom
chore/token-permissions-least-privilege
May 6, 2026
Merged

ci: add explicit top-level permissions to all workflows (least privilege)#39
entlein merged 2 commits intomainfrom
chore/token-permissions-least-privilege

Conversation

@entlein
Copy link
Copy Markdown

@entlein entlein commented May 4, 2026

Why

Scorecard / GitHub code-scanning has 16 open Token-Permissions alerts on this repo (security_severity: high). Most are "no topLevel permission defined" — every workflow file is implicitly using GITHUB_TOKEN's broad default scope.

What

  • Adds permissions: read-all at the top of every workflow that was missing one (9 files).
  • Existing job-level permissions blocks (build.yaml's build + trigger-component-tests, component-tests.yaml's build-and-push-image, sign-object.yaml top-level, pr-created.yaml, pr-merged.yaml, incluster-comp-*.yaml) are unchanged — they already grant exactly what those jobs need.
  • Adds explicit job-level permissions to bypass.yaml's pr-merged caller. That job invokes incluster-comp-pr-merged.yaml as a reusable workflow; reusable workflows can't request scopes the caller hasn't granted, so top-level read-all + no-perms on the caller would starve the inner docker-build / create-release-and-retag jobs. The grants mirror pr-merged.yaml's pr-merged job exactly: id-token: write, packages: write, contents: write.

Coverage

Closes 9 of 16 open Token-Permissions alerts ("no topLevel permission defined" type).

The remaining 7 are "jobLevel '' permission set to write" findings that flag legitimate write uses:

  • build.yaml:87actions: write for trigger-component-tests (cross-workflow dispatch)
  • pr-created.yaml:18 + incluster-comp-pr-created.yaml:40security-events: write (CodeQL upload)
  • pr-merged.yaml:33,34 + incluster-comp-pr-merged.yaml:355packages: write / contents: write (image push, release retag)
  • sign-object.yaml:26 — top-level packages: write (image push)

These are required for the workflow's actual job and aren't removable without breaking it.

Risk

Low. No-op for any job whose existing permissions are correct (which is most of them). The one substantive change is the bypass.yaml permission grant, which mirrors the corresponding pr-merged.yaml grant verbatim — it's bringing bypass.yaml's reusable invocation in line with the explicit pattern already used elsewhere.

YAML validated: all 10 workflow files parse cleanly with yaml.safe_load.

Test plan

  • CI green on this PR (no privilege-related failures)
  • bypass.yaml workflow_dispatch run succeeds end-to-end
  • gh api repos/k8sstormcenter/node-agent/code-scanning/alerts?state=open count drops by 9 after merge

…ege)

Scorecard flagged 9 'no topLevel permission defined' alerts (Token-
Permissions, security_severity: high). GITHUB_TOKEN's default scope
includes write access to most things; opting into 'permissions: read-all'
at the workflow level scopes that down to read, then jobs that genuinely
need write scopes override locally.

Existing job-level permissions blocks (build / sign-object / pr-merged /
pr-created / incluster-* / docker-build) are unchanged.

Added job-level permissions to bypass.yaml's pr-merged caller — needed
because that job invokes incluster-comp-pr-merged.yaml as a reusable
workflow. Reusable workflows can't request scopes the caller hasn't
granted, so top-level read-all + the implicit no-perms on the caller
would starve the called workflow's docker-build / create-release jobs.
Mirrors pr-merged.yaml's perms exactly.

Closes 9 of 16 open Token-Permissions alerts. The remaining 7 are
'jobLevel X permission set to write' findings that flag legitimate write
uses (image push, sigstore signing, security-events upload) — Scorecard
wants those audit-acknowledged but they're not removable without
breaking the workflow's purpose.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cab182ee-5ddd-4636-9348-76b1b6ed0646

📥 Commits

Reviewing files that changed from the base of the PR and between 80859ac and 47b33c5.

📒 Files selected for processing (2)
  • .github/workflows/benchmark.yaml
  • .github/workflows/go-basic-tests.yaml

📝 Walkthrough

Walkthrough

Adds a top-level permissions: read-all block to multiple GitHub Actions workflows to set a least-privilege default; several jobs retain or add per-job permissions overrides where elevated scopes are required (e.g., contents, issues, pull-requests, id-token, packages, security-events).

Changes

GitHub Actions workflow permission hardening

Layer / File(s) Summary
Workflow-Level Permission Defaults
.github/workflows/benchmark.yaml, .github/workflows/build.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
Each workflow now declares permissions: read-all at the top level with comments indicating least-privilege defaults.
Job-Level Permission Overrides
.github/workflows/bypass.yaml, .github/workflows/benchmark.yaml, .github/workflows/go-basic-tests.yaml, .github/workflows/incluster-comp-pr-created.yaml
Specific jobs add or retain per-job permissions blocks that grant elevated scopes required by those jobs: id-token: write, packages: write, contents: write/read, issues: write, pull-requests: write, security-events: write.
Comments / Rationale Inline
(various workflow files)
Explanatory comments added near permissions entries describing least-privilege defaults and that jobs should override when elevated scopes are necessary.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding explicit top-level permissions with a least-privilege approach across all workflows.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the security motivation, detailed implementation across 9-10 files, coverage metrics, risk assessment, and test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/token-permissions-least-privilege

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/benchmark.yaml:
- Around line 27-29: The workflow-level permissions were set to read-all which
removes write access for the benchmark job; in the `benchmark` job definition
add a job-level permissions override that grants `issues: write` and
`pull-requests: write` so the `Comment on PR` step has the required write scope
to post benchmark results back to the PR thread.

In @.github/workflows/go-basic-tests.yaml:
- Around line 39-41: The workflow-level permissions are too restrictive for
CodeQL upload; in the reusable workflow add a job-level permissions override for
the CodeQL job (Environment-Test) to restore upload rights by setting
permissions: contents: read and security-events: write for the job that runs
github/codeql-action/analyze, and update any callers of this workflow_call to
grant security-events: write when invoking the reusable workflow so uploads
succeed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9ff4a244-c1cc-4a4e-a60f-58eb835e5a7e

📥 Commits

Reviewing files that changed from the base of the PR and between f0d0f72 and 80859ac.

📒 Files selected for processing (9)
  • .github/workflows/benchmark.yaml
  • .github/workflows/build.yaml
  • .github/workflows/bypass.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

Comment thread .github/workflows/benchmark.yaml
Comment thread .github/workflows/go-basic-tests.yaml
Addresses CodeRabbit's two findings on PR #39:

1. benchmark.yaml:29 (Minor) — peter-evans/create-or-update-comment
   needs issues:write + pull-requests:write to post benchmark results
   to the PR thread. Failure was masked by continue-on-error on the
   step. Add job-level permissions on 'benchmark'.

2. go-basic-tests.yaml:41 (Major) — github/codeql-action/analyze needs
   security-events:write to upload SARIF results. Failure was masked
   by continue-on-error on the CodeQL steps. Add job-level permissions
   on 'Environment-Test'.

Note for #2: this file is invoked as a reusable workflow (workflow_call)
from pr-created.yaml's pr-created job, which already grants
security-events:write — so the caller envelope is already correct, only
the inner job's override was missing.

YAML validated.
@entlein entlein merged commit 83bc87a into main May 6, 2026
1 check passed
@entlein entlein deleted the chore/token-permissions-least-privilege branch May 6, 2026 17:50
entlein pushed a commit that referenced this pull request May 9, 2026
Chore PR #39 added 'permissions: read-all' to every workflow file
including the workflow_call reusables. That broke the reusable-call
contract: a called workflow's top-level permissions are the FLOOR the
caller must grant. Callers of these reusables (pr-created.yaml's
pr-created job, pr-merged.yaml's pr-merged job, etc.) only grant a
narrow set of scopes (id-token:write, packages:write, security-events:
write, pull-requests:write, contents). They do NOT grant the full
read-all set (actions:read, artifact-metadata:read, attestations:read,
checks:read, deployments:read, discussions:read, issues:read,
models:read, vulnerability-alerts:read, packages:read, pages:read,
repository-projects:read, statuses:read, id-token:read).

Result: every PR opened on this fork since chore #39 merged (May 6)
has had pr-created.yaml startup_failure with:

  Error calling workflow ... incluster-comp-pr-created.yaml@<sha>.
  The workflow is requesting 'actions: read, ...', but is only allowed [...]

Fix: strip top-level 'permissions: read-all' from the four reusables:
- benchmark.yaml (also direct-trigger; falls back to per-job perms)
- go-basic-tests.yaml
- incluster-comp-pr-created.yaml
- incluster-comp-pr-merged.yaml

Each reusable's per-job 'permissions:' blocks remain intact and
correctly request only what the job needs. Callers' existing grants
are sufficient.

Top-level 'permissions: read-all' stays on the OUTERMOST workflows
(pr-created.yaml, pr-merged.yaml, build.yaml, bypass.yaml,
component-tests.yaml, sign-object.yaml, scorecard.yml) — they're
event-triggered, not workflow_call'd, so the read-all default still
hardens GITHUB_TOKEN there.
@coderabbitai coderabbitai Bot mentioned this pull request May 9, 2026
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.

1 participant