Add explicit permissions for CI jobs #3204
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded job-level Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/workflows/devcontainer-esp32.yml (1)
24-26: Right-size GITHUB_TOKEN scope; consider either using GITHUB_TOKEN for GHCR push or drop packages: write when using a PAT.You’re logging into GHCR with a PAT (
secrets.CONTAINER_BUILD_TOKEN). In that case, grantingpackages: writeto the job’s GITHUB_TOKEN is not strictly required. Two clean options:
- Option A (preferred for least secrets): switch the login to GITHUB_TOKEN and keep
packages: writehere.- Option B: keep the PAT-based login and remove
packages: writefrom the job to minimize token scope.Apply this diff for Option B (PAT login retained, least-privilege GITHUB_TOKEN):
permissions: contents: read - packages: writeIf you prefer Option A, adjust the login step (outside this hunk) to use GITHUB_TOKEN:
- name: Login to GitHub Container Registry uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }}Optional: set a workflow-level default to lock down other jobs by default, overriding only where needed:
permissions: contents: read.github/workflows/devcontainer-freertos-nxp.yaml (1)
24-26: Minimize token permissions given PAT-based GHCR push.Same pattern here: because the job logs into GHCR using a PAT,
packages: writeon the GITHUB_TOKEN is unnecessary unless another step uses it to push/publish. Either (A) switch to GITHUB_TOKEN-based login and keeppackages: write, or (B) retain PAT and droppackages: writefor least privilege.Apply this diff for Option B:
permissions: contents: read - packages: write.github/workflows/devcontainer-ti.yaml (1)
24-26: Align permissions with actual usage; remove packages: write if not pushing with GITHUB_TOKEN.You’re using a PAT for the GHCR login. Unless another step relies on GITHUB_TOKEN to write to GHCR,
packages: writecan be removed to adhere to least-privilege.permissions: contents: read - packages: writeAlternative: switch the GHCR login to GITHUB_TOKEN and keep
packages: write(same snippet as suggested in the ESP32 workflow)..github/workflows/devcontainer-chibios.yaml (1)
24-26: Tighten GITHUB_TOKEN scope when a PAT is already used for GHCR.As with the other build workflows, consider dropping
packages: writeon GITHUB_TOKEN since the registry push uses a PAT. Alternatively, migrate the login to GITHUB_TOKEN and keeppackages: write.permissions: contents: read - packages: writeOptional repo-wide improvement: set
permissions: contents: readat the workflow root and only elevate specific jobs that push to GHCR..github/workflows/devcontainer-all.yaml (1)
24-26: Consider dialing back packages scope or standardizing on GITHUB_TOKEN for GHCR push.Right now the push auth uses a PAT via secrets.CONTAINER_BUILD_TOKEN (Lines 51–56). If you keep PAT auth, the job’s GITHUB_TOKEN doesn’t need packages: write. Reduce blast radius by removing that grant here. Alternatively, drop the PAT and push with GITHUB_TOKEN, in which case packages: write is appropriate.
Option A (keep PAT; least privilege on GITHUB_TOKEN):
permissions: contents: read - packages: write + # packages not needed when authenticating to GHCR with PAT (docker/login-action) + # packages: writeOption B (use GITHUB_TOKEN for GHCR; remove PAT secret usage):
- - name: Login to GitHub Container Registry - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.CONTAINER_BUILD_TOKEN }} + - name: Login to GitHub Container Registry + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }}Note: Option B requires keeping
permissions.packages: write. Your existing “only push on main and when PUBLISH_DOCKER_IMAGE=='true'” guard helps mitigate exposure on PRs from forks..github/workflows/devcontainer-azurertos.yaml (1)
24-26: Reassess need for packages: write given PAT-based login.As in the “all” workflow, you authenticate to GHCR using a PAT (Lines 51–56). If retaining PAT auth, the job’s GITHUB_TOKEN doesn’t need packages: write. Trim it, or switch to GITHUB_TOKEN for login.
Option A (retain PAT; remove unnecessary packages grant):
permissions: contents: read - packages: write + # packages: writeOption B (switch login to GITHUB_TOKEN; keep packages: write):
- - name: Login to GitHub Container Registry - uses: docker/login-action@v3 - with: - registry: ghcr.io - username: ${{ github.repository_owner }} - password: ${{ secrets.CONTAINER_BUILD_TOKEN }} + - name: Login to GitHub Container Registry + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.github/workflows/devcontainer-all.yaml(1 hunks).github/workflows/devcontainer-azurertos.yaml(1 hunks).github/workflows/devcontainer-chibios.yaml(1 hunks).github/workflows/devcontainer-esp32.yml(1 hunks).github/workflows/devcontainer-freertos-nxp.yaml(1 hunks).github/workflows/devcontainer-smoketest.yaml(1 hunks).github/workflows/devcontainer-ti.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
- GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
- GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
- GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
- GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
- GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
- GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
- GitHub Check: build-target (SL_STK3701A, Debug, All)
- GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
- GitHub Check: build-target (ESP32_C3, Debug, ESP32)
- GitHub Check: build-target (ESP32_S3, Debug, ESP32)
- GitHub Check: build-target (M5Core2, Debug, ESP32)
- GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
- GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
- GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
- GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
🔇 Additional comments (3)
.github/workflows/devcontainer-all.yaml (2)
24-26: Good move: explicit, job-scoped token permissions.Setting least-privilege permissions at the job level aligns with GitHub’s guidance and avoids over-privileged defaults. Contents: read is necessary for checkout; packages permission is the only scope that could be contentious depending on auth strategy.
24-26: Sanity check across all workflows for consistent permissions policyIt looks like the
fdinvocation in the “missing permissions block” loop didn’t actually enumerate any files (it treated.github/workflowsas a pattern rather than a directory). To catch any workflows that lack apermissions:block, please re-run with one of the following updated snippets:#!/bin/bash # List all workflows that declare a permissions block: echo "Workflows with a permissions block:" rg -nP '^\s*permissions\s*:' .github/workflows -g '*.yml' -g '*.yaml' \ | sed 's/:.*//' | sort -u # List all workflow files under .github/workflows and show those missing permissions: echo -e "\nWorkflows missing a permissions block:" for f in $(fd -t f -e yml -e yaml .github/workflows); do if ! rg -qP '^\s*permissions\s*:' "$f"; then echo "$f" fi done—or using
findif you prefer:—#!/bin/bash echo "Workflows with a permissions block:" rg -nP '^\s*permissions\s*:' .github/workflows -g '*.yml' -g '*.yaml' \ | sed 's/:.*//' | sort -u echo -e "\nWorkflows missing a permissions block:" find .github/workflows -type f \( -name '*.yml' -o -name '*.yaml' \) | while read f; do if ! rg -qP '^\s*permissions\s*:' "$f"; then echo "$f" fi doneOnce you’ve rerun this, we’ll have a complete view of which workflows still need a
permissions:block added or updated..github/workflows/devcontainer-azurertos.yaml (1)
24-26: Good: explicit permissions applied at the job level.This matches least-privilege best practices; contents: read is required for checkout.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/devcontainer-smoketest.yaml (1)
38-93: Authenticate to GHCR before the devcontainers build or pulls may 401.Even with
packages: readgranted to GITHUB_TOKEN, the runner must still log in toghcr.iofor private org images. Add a GHCR login step before invokingdevcontainers/ci.Apply this diff to insert the login step right before the build:
- name: Adjust devcontainer.json for ${{ matrix.container }} source run: | # Move into the .devcontainer directory pushd .devcontainer/${{ matrix.container }} # Target the dockerfile source. sed -i -- 's|"dockerFile": "Dockerfile.${{ matrix.container }}"|"dockerFile": "Dockerfile.${{ matrix.container }}.SRC"|g' devcontainer.json # Move out of the .devcontainer directory popd + - name: Log into GHCR + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Build ${{ matrix.target }} ${{ matrix.build-type }} Firmware uses: devcontainers/[email protected] with: configFile: ./.devcontainer/${{ matrix.container }}/devcontainer.json push: never runCmd: | # Build target: cmake --preset=${{ matrix.target }} -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} cmake --build buildRun this to confirm whether GHCR images used by the devcontainers require auth (HTTP 401 implies private):
#!/bin/bash set -euo pipefail echo "Detecting GHCR references in devcontainer Dockerfiles and devcontainer.json..." rg -n --glob '.devcontainer/**/Dockerfile.*.SRC' -e '^\s*FROM\s+ghcr\.io' -C1 || true rg -n --glob '.devcontainer/**/devcontainer.json' -e '"image"\s*:\s*".*ghcr\.io' || true echo -e "\nProbing a representative set of GHCR images for public access (expects 401 if private):" imgs=$(rg -N --no-filename --glob '.devcontainer/**/Dockerfile.*.SRC' -oP '^\s*FROM\s+\Kghcr\.io/[^\s:]+' | sort -u \ ; rg -N --no-filename --glob '.devcontainer/**/devcontainer.json' -oP '"image"\s*:\s*"\Kghcr\.io/[^":]+' | sort -u) || true mapfile -t images < <(printf '%s\n' "$imgs" | sed 's/$/:latest/' | sort -u) for ref in "${images[@]}"; do name=${ref%:*} tag=${ref#*:} url="https://ghcr.io/v2/${name#ghcr.io/}/manifests/$tag" code=$(curl -s -o /dev/null -w "%{http_code}" -H "Accept: application/vnd.oci.image.manifest.v1+json" "$url") echo "$ref -> HTTP $code" done echo -e "\nIf you see HTTP 401 for any, the GHCR login step above is mandatory."
🧹 Nitpick comments (1)
.github/workflows/devcontainer-smoketest.yaml (1)
39-45: Pin third-party actions to commit SHAs for supply-chain hardening.Avoid floating tags like
@main,@v4,@v0.3. Pin to a verified commit SHA and optionally keep the tag as a comment for readability.Example:
- - uses: actions/checkout@v4 + - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c1235 # v4.1.7 - - name: Free Disk Space (Ubuntu) - uses: jlumbroso/free-disk-space@main + - name: Free Disk Space (Ubuntu) + uses: jlumbroso/free-disk-space@2a1b3e4f8b5a1c0d8e2f0f1e0a9b7c6d5e4f3a2b # main at time of pin - - name: Build ${{ matrix.target }} ${{ matrix.build-type }} Firmware - uses: devcontainers/[email protected] + - name: Build ${{ matrix.target }} ${{ matrix.build-type }} Firmware + uses: devcontainers/ci@c1d2e3f4a5b6c7d8e9f00112233445566778899a # v0.3If you want, I can open a separate PR to pin these across all workflows.
Also applies to: 85-86
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/devcontainer-smoketest.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
- GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
- GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
- GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
- GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
- GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
- GitHub Check: build-target (ESP32_C3, Debug, ESP32)
- GitHub Check: build-target (ESP32_S3, Debug, ESP32)
- GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
- GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
- GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
- GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
- GitHub Check: build-target (M5Core2, Debug, ESP32)
- GitHub Check: build-target (SL_STK3701A, Debug, All)
- GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
- GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
- GitHub Check: Analyze (actions)
🔇 Additional comments (1)
.github/workflows/devcontainer-smoketest.yaml (1)
34-36: Good move to least-privilege: packages: read fits this smoketest job.For a job that only pulls GHCR images and never pushes,
contents: read+packages: readis the right minimal scope. No issues here.
Adjust to add docker login.
This uses a different PAT.
There was a problem hiding this comment.
Pull Request Overview
This PR adds explicit permissions to GitHub Actions workflow jobs for devcontainer build processes. The changes follow GitHub's recommendation to explicitly define job-level permissions instead of relying on default permissions.
- Added
contents: readpermission to all devcontainer build jobs - Added
packages: readpermission to the smoketest job - Added Docker login step to the smoketest workflow for GHCR authentication
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/devcontainer-all.yaml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-azurertos.yaml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-chibios.yaml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-esp32.yml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-freertos-nxp.yaml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-ti.yaml |
Added explicit contents: read permission to build job |
.github/workflows/devcontainer-smoketest.yaml |
Added contents: read and packages: read permissions, plus Docker login step |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
networkfusion
changed the title
Since we build this from scratch, there should be no need to login to GHCR.
Description
Add explicit permissions for github workflow jobs.
Motivation and Context
Github now recommends adding explicit permissions for actions.
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit