ci: opt in to allow-unsafe-pr-checkout for fork PR warehouse tests - #1045
ci: opt in to allow-unsafe-pr-checkout for fork PR warehouse tests#1045haritamar wants to merge 2 commits into
Conversation
Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
👋 @haritamar |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe warehouse test workflow now disables credential persistence and enables unsafe dbt package checkout for fork pull requests. Comments document the required approved CI secrets. ChangesWarehouse workflow
Estimated code review effort: 1 (Trivial) | ~5 minutes Mergeability Score: ⚪ Minimal · up to This localized workflow change enables the intended gated cloud warehouse tests for fork pull requests; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test-warehouse.yml (1)
95-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
actionlintor suppress this diagnostic.
actionlint1.7.12 predatesallow-unsafe-pr-checkout, butactions/checkout@v6defines this security input. Ifactionlintruns as a required check, upgrade it or add a targeted ignore. Keep the input.🤖 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 95, Update the workflow linting configuration for the diagnostic on allow-unsafe-pr-checkout: either upgrade actionlint to a version recognizing this actions/checkout@v6 input or add a targeted suppression for this specific diagnostic. Preserve the allow-unsafe-pr-checkout input unchanged.Source: Linters/SAST tools
🤖 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:
- Around line 89-95: Update both actions/checkout invocations in the workflow to
set persist-credentials to false, ensuring no GitHub credentials remain
configured before executing fork-controlled code; preserve the existing
allow-unsafe-pr-checkout and approval-gating behavior.
---
Nitpick comments:
In @.github/workflows/test-warehouse.yml:
- Line 95: Update the workflow linting configuration for the diagnostic on
allow-unsafe-pr-checkout: either upgrade actionlint to a version recognizing
this actions/checkout@v6 input or add a targeted suppression for this specific
diagnostic. Preserve the allow-unsafe-pr-checkout input 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: 1ed46cab-a46f-4115-bdaf-6f52e32aff71
📒 Files selected for processing (1)
.github/workflows/test-warehouse.yml
Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
Summary
actions/checkoutnow refuses to check out fork PR code underpull_request_targetunlessallow-unsafe-pr-checkout: trueis set (see the equivalent failure inelementary: run 31641925693), which breaks the cloud warehouse test jobs for fork PRs here too.Our
pull_request_targetusage is intentional: the cloud integration tests need the CI secrets of our test warehouses, which a fork'spull_requestrun cannot access.test-all-warehouses.ymlgates fork PRs behind theelementary_test_envenvironment (approve-forkjob), so a maintainer must approve the run before the fork code is checked out and executed.So the checkout of the PR head (
Checkout dbt package) opts in explicitly, with a comment recording the rationale. Paired with elementary-data/elementary#2317.Link to Devin session: https://app.devin.ai/sessions/9cfcaa3435b64897992abc30956db12a
Requested by: @haritamar
Summary by CodeRabbit