Skip to content

ci: opt in to allow-unsafe-pr-checkout for fork PR warehouse tests - #2324

Open
haritamar wants to merge 2 commits into
masterfrom
devin/1786570169-allow-unsafe-pr-checkout
Open

ci: opt in to allow-unsafe-pr-checkout for fork PR warehouse tests#2324
haritamar wants to merge 2 commits into
masterfrom
devin/1786570169-allow-unsafe-pr-checkout

Conversation

@haritamar

@haritamar haritamar commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

actions/checkout now refuses to check out fork PR code under pull_request_target unless allow-unsafe-pr-checkout: true is set, which broke all warehouse test jobs (e.g. this run):

Refusing to check out fork pull request code from a 'pull_request_target' workflow ... set 'allow-unsafe-pr-checkout: true' on the actions/checkout step.

Our pull_request_target usage here is intentional: integration tests need the CI secrets of our test warehouses, which a fork's pull_request run cannot access. test-all-warehouses.yml gates fork PRs behind the elementary_test_env environment (approve-fork job), so a maintainer must approve before the fork code is checked out, and only an explicit allowlist of secrets is forwarded to test-warehouse.yml.

So the checkout of the PR head opts in explicitly, with a comment recording the above rationale.

Link to Devin session: https://app.devin.ai/sessions/9cfcaa3435b64897992abc30956db12a
Requested by: @haritamar

Summary by CodeRabbit

  • Chores
    • Improved validation workflows for contributions submitted from forks.
    • Added safeguards and documented approval requirements to support secure checks in this scenario.
    • Updated automated testing procedures to improve reliability and reduce potential credential exposure during validation.
    • Enhanced checkout handling for external contributions while preserving the integrity of project checks.

Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
@haritamar haritamar self-assigned this Aug 12, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown
Contributor

👋 @haritamar
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b7dec96f-3ebe-47ec-b795-176a3836a7f7

📥 Commits

Reviewing files that changed from the base of the PR and between 0d9a899 and 98db355.

📒 Files selected for processing (1)
  • .github/workflows/test-warehouse.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/test-warehouse.yml

📝 Walkthrough

Walkthrough

The warehouse test workflow updates Elementary checkout controls for fork pull requests and disables persisted Git credentials for both the Elementary and dbt package checkouts.

Changes

Warehouse test workflow

Layer / File(s) Summary
Checkout credential controls
.github/workflows/test-warehouse.yml
The Elementary checkout enables unsafe pull-request checkout and documents environment approval and secret allowlisting. The Elementary and dbt package checkouts disable persisted Git credentials.

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

Mergeability Score: 🟠 High · up to 98db3

This change allows fork pull request code to run in privileged warehouse-test jobs with access to sensitive credentials and cloud permissions. A malicious fork could potentially misuse those permissions, so the PR should not merge until that security exposure is mitigated or explicitly accepted by the appropriate owner.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main CI change for fork pull request warehouse tests.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch devin/1786570169-allow-unsafe-pr-checkout

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

Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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/test-warehouse.yml:
- Line 107: Update the actionlint metadata or upgrade actionlint so it
recognizes the allow-unsafe-pr-checkout input used with actions/checkout@v6.
Preserve the existing allow-unsafe-pr-checkout configuration and align the
linter’s action metadata with the checkout version used by CI.
- Around line 100-107: Update the Checkout Elementary and Checkout dbt package
action steps to set persist-credentials to false, preventing either checkout
from leaving the base GITHUB_TOKEN available to fork-controlled code. Keep the
existing checkout configuration and allow-unsafe-pr-checkout behavior unchanged.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1cc91ac5-f01e-4155-b0b9-00c567eebe5f

📥 Commits

Reviewing files that changed from the base of the PR and between a73b355 and 0d9a899.

📒 Files selected for processing (1)
  • .github/workflows/test-warehouse.yml

Comment thread .github/workflows/test-warehouse.yml
# elementary_test_env environment, so a maintainer must approve the
# run before this fork code is checked out and executed, and only an
# explicit allowlist of secrets is passed in.
allow-unsafe-pr-checkout: true

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

curl -fsSL https://raw.githubusercontent.com/actions/checkout/v6/action.yml |
  grep -A8 '^allow-unsafe-pr-checkout:'

actionlint .github/workflows/test-warehouse.yml

Repository: elementary-data/elementary

Length of output: 164


🏁 Script executed:

#!/usr/bin/env bash
set -u

printf '%s\n' '--- workflow context ---'
sed -n '85,120p' .github/workflows/test-warehouse.yml

printf '%s\n' '--- checkout references ---'
rg -n -C 2 'actions/checkout|allow-unsafe-pr-checkout|actionlint' .github . 2>/dev/null | head -200

printf '%s\n' '--- actionlint availability ---'
if command -v actionlint >/dev/null 2>&1; then
  actionlint --version
  actionlint .github/workflows/test-warehouse.yml
  printf 'actionlint_exit=%s\n' "$?"
else
  printf '%s\n' 'actionlint: not installed'
fi

printf '%s\n' '--- checkout v6 metadata ---'
if command -v curl >/dev/null 2>&1; then
  curl -fsSL https://raw.githubusercontent.com/actions/checkout/v6/action.yml | \
    awk '/^[[:space:]]*allow-unsafe-pr-checkout:/,/^[[:space:]]*[A-Za-z0-9_-]+:/{print}' | head -10
  printf 'curl_exit=%s\n' "$?"
else
  printf '%s\n' 'curl: not installed'
fi

Repository: elementary-data/elementary

Length of output: 8102


🏁 Script executed:

#!/usr/bin/env bash
set -u

printf '%s\n' '--- actionlint references and configuration ---'
rg -n -C 3 'actionlint|reviewdog|rhysd|workflow-files|actions/checkout' \
  .github Makefile* justfile* Taskfile* package.json pyproject.toml .pre-commit-config.yaml 2>/dev/null | head -300

printf '%s\n' '--- actionlint help and embedded metadata clues ---'
actionlint -help 2>&1 | head -120

printf '%s\n' '--- checkout v6 metadata ---'
curl -fsSL https://raw.githubusercontent.com/actions/checkout/v6/action.yml | \
  sed -n '/allow-unsafe-pr-checkout/,+8p'

printf '%s\n' '--- checkout v6 ref resolution ---'
curl -fsSL https://api.github.com/repos/actions/checkout/git/ref/tags/v6 \
  -H 'Accept: application/vnd.github+json' | jq '{ref, object}'
curl -fsSL https://api.github.com/repos/actions/checkout/releases/tags/v6.1.0 \
  -H 'Accept: application/vnd.github+json' | jq '{tag_name, published_at, target_commitish, body}'

printf '%s\n' '--- actionlint releases ---'
curl -fsSL https://api.github.com/repos/rhysd/actionlint/releases/tags/1.7.12 \
  -H 'Accept: application/vnd.github+json' | jq '{tag_name, published_at, target_commitish, body}'

Repository: elementary-data/elementary

Length of output: 10422


Update actionlint metadata or upgrade actionlint.

actions/checkout@v6 defines allow-unsafe-pr-checkout, but actionlint 1.7.12 does not recognize it. Keep the input and align the linter metadata with the action version used by CI.

🧰 Tools
🪛 actionlint (1.7.12)

[error] 107-107: input "allow-unsafe-pr-checkout" is not defined in action "actions/checkout@v6". available inputs are "clean", "fetch-depth", "fetch-tags", "filter", "github-server-url", "lfs", "path", "persist-credentials", "ref", "repository", "set-safe-directory", "show-progress", "sparse-checkout", "sparse-checkout-cone-mode", "ssh-key", "ssh-known-hosts", "ssh-strict", "ssh-user", "submodules", "token"

(action)

🪛 zizmor (1.29.0)

[warning] 95-107: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for 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.

In @.github/workflows/test-warehouse.yml at line 107, Update the actionlint
metadata or upgrade actionlint so it recognizes the allow-unsafe-pr-checkout
input used with actions/checkout@v6. Preserve the existing
allow-unsafe-pr-checkout configuration and align the linter’s action metadata
with the checkout version used by CI.

Source: Linters/SAST tools

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.

Not actionable here: this repo doesn't run actionlint in CI or pre-commit, so there's no metadata to align or version to bump — the diagnostic is from CodeRabbit's own actionlint 1.7.12, which predates allow-unsafe-pr-checkout (added in actions/checkout v6.1.0). Keeping the input.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Skipped: comment is from another GitHub bot.

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