Skip to content

ci: address CodeQL security findings (cache poisoning + permissions)#31

Merged
alandtse merged 1 commit into
devfrom
ci/security-findings
May 21, 2026
Merged

ci: address CodeQL security findings (cache poisoning + permissions)#31
alandtse merged 1 commit into
devfrom
ci/security-findings

Conversation

@alandtse
Copy link
Copy Markdown
Owner

Summary

Addresses all 14 open code-scanning alerts surfaced after CodeQL was enabled.

  • 7 Highactions/cache-poisoning/poisonable-step in pr-checks.yaml and _shared-build.yaml. PR builds running under pull_request_target had write access to a cache later restored by trusted default-branch runs.
  • 7 Mediumactions/missing-workflow-permissions in nexus-upload.yaml, pr-wip.yaml, maint-todo-issues.yaml. Workflows ran with implicit full-scope GITHUB_TOKEN.

Cache poisoning fix

setup-build-environment/action.yaml: split the single combined cache step into two conditionally-invoked steps that share the same key:

Event class Step Behavior
Trusted (push, workflow_dispatch, workflow_call) actions/cache@v5 restore + auto-save at job-end
Untrusted (pull_request, pull_request_target) actions/cache/restore@v5 restore-only

PR builds still get the cache speedup (restoring caches warmed by trusted runs) but can't write back to them. Trust crosses the boundary in only the safe direction.

Permissions fix

Per-workflow least-privilege blocks:

Workflow Permissions added Why
nexus-upload.yaml contents: read (workflow-level) Reads releases via gh api / gh release download. Writes go to Nexus via UNEX_* secrets, not GitHub.
pr-wip.yaml statuses: write, pull-requests: read wip/action sets a commit status on the PR head and reads PR metadata.
maint-todo-issues.yaml contents: read, issues: write alstr/todo-to-issue-action creates GitHub issues from TODO comments in the diff.

Verification

Once merged, the 14 alerts should auto-close on the next CodeQL run against dev. No other alerts expected from this change.

Test plan

  • Open this PR → confirm pr-checks.yaml runs and the PR build still gets a cache restore (look for Cache restored from key: in the Restore CMake build cache (untrusted PR, restore-only) step).
  • Verify the PR build does NOT save back: should see Cache CMake build output (trusted, restore + save) skipped in the step list.
  • After merge to dev, the next push-triggered build should populate/save the cache normally.
  • Subsequent CodeQL run on dev shows 0 open alerts.

🤖 Generated with Claude Code

14 alerts surfaced after CodeQL was enabled. Two classes:

**Cache poisoning (7 alerts, High severity)** — `actions/cache@v5` was
used in `setup-build-environment` from workflows that run under
`pull_request_target`. That context runs untrusted PR code with the
default branch's secrets/permissions, so a malicious PR could write a
poisoned build cache that later trusted runs on `dev` would restore.

Fix in `.github/actions/setup-build-environment/action.yaml`: split the
single combined cache step into two conditionally-invoked steps that
share the same cache key:

- Trusted events (push, workflow_dispatch, workflow_call from release
  pipeline) keep the combined `actions/cache@v5` — restore at this
  step, auto-save at job-end.
- Untrusted events (`pull_request`, `pull_request_target`) use the
  restore-only sub-action `actions/cache/restore@v5`. PR builds still
  benefit from caches warmed by trusted runs but cannot themselves
  write back. Trust crosses the boundary in only the safe direction.

**Missing workflow permissions (7 alerts, Medium severity)** — three
workflows ran with implicit full-scope GITHUB_TOKEN. Add explicit
least-privilege `permissions:` blocks:

- `nexus-upload.yaml`: workflow-level `contents: read`. Every job
  only reads from GitHub (release listing via `gh api`, asset
  download via `gh release download`) and writes only to Nexus via
  UNEX_* secrets.
- `pr-wip.yaml`: `statuses: write` + `pull-requests: read` for the
  wip/action's commit-status set + PR metadata read.
- `maint-todo-issues.yaml`: `contents: read` for checkout +
  `issues: write` for the create-issue call.
Copilot AI review requested due to automatic review settings May 21, 2026 06:49
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

Warning

Rate limit exceeded

@alandtse has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 37 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52790ef3-3698-49c7-a03f-fad6e30c8315

📥 Commits

Reviewing files that changed from the base of the PR and between 20a38a1 and 8bb775b.

📒 Files selected for processing (4)
  • .github/actions/setup-build-environment/action.yaml
  • .github/workflows/maint-todo-issues.yaml
  • .github/workflows/nexus-upload.yaml
  • .github/workflows/pr-wip.yaml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/security-findings

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 and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the repository’s CI against CodeQL-reported GitHub Actions risks by preventing cache writes from untrusted PR contexts and by applying least-privilege GITHUB_TOKEN permissions to affected workflows.

Changes:

  • Split the CMake build cache step in the setup-build-environment composite action into trusted (restore+save) vs untrusted (restore-only) variants to eliminate cache-poisoning risk.
  • Add explicit least-privilege permissions: blocks to workflows that previously relied on implicit default token scopes.
  • Document the intent of the permission scopes and the cache trust-boundary behavior inline in workflow/action YAML.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
.github/workflows/pr-wip.yaml Adds explicit minimal permissions for wip/action (commit status write + PR metadata read).
.github/workflows/nexus-upload.yaml Sets workflow-level contents: read to avoid implicit broad token permissions while using gh to read releases/assets.
.github/workflows/maint-todo-issues.yaml Sets minimal permissions needed for checkout + creating issues from TODO/FIXME markers.
.github/actions/setup-build-environment/action.yaml Prevents untrusted PR-triggered jobs from saving caches while still allowing cache restore for speed.

@alandtse alandtse merged commit 4f23e36 into dev May 21, 2026
14 checks passed
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.

2 participants