Bug | Allow rogue_sanity on PRs

[yuval-qf]

Description

Motivation and Context

Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ New feature (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
• 📝 Documentation update
• 🎨 Code style/refactoring (no functional changes)
• 🧪 Test updates
• 🔧 Configuration/build changes

Changes Made

Screenshots/Examples (if applicable)

Checklist

• I have read the CONTRIBUTING.md guide
• My code follows the code style of this project (PEP 8, type hints, docstrings)
• I have run uv run black . to format my code
• I have run uv run flake8 . and fixed all issues
• I have run uv run mypy --config-file .mypy.ini . and addressed type checking issues
• I have run uv run bandit -c .bandit.yaml -r . for security checks
• I have added tests that prove my fix is effective or that my feature works
• I have run uv run pytest and all tests pass
• I have manually tested my changes
• I have updated the documentation accordingly
• I have added/updated type hints for new/modified functions
• My changes generate no new warnings
• I have checked my code for security issues
• Any dependent changes have been merged and published

Testing

Test Configuration:

• Python version:
• OS:
• Other relevant details:

Test Steps:
1.
2.
3.

Additional Notes

Related Issues/PRs

• Fixes #
• Related to #
• Depends on #

---

[coderabbitai]

Summary by CodeRabbit

• Chores
  • Updated internal GitHub Actions workflow configuration and security settings.

Walkthrough

Modified the GitHub Actions workflow to use pull_request_target instead of pull_request trigger, added an environment reference to the rogue_sanity job, and updated the Checkout step to fetch the PR head commit while disabling credential persistence to prevent exposing secrets to untrusted code.

Changes

Cohort / File(s)	Summary
GitHub Actions Workflow Security Configuration .github/workflows/rogue.yml	Changed workflow trigger from pull_request to pull_request_target, added rogue-sanity-ci-secrets environment to rogue_sanity job, and updated Checkout action to fetch PR head commit (${{ github.event.pull_request.head.sha }}) with persist-credentials: false

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• qualifire-dev/rogue#107: The workflow changes directly address enabling secure CI execution for pull requests from forks by switching to pull_request_target trigger and preventing credential exposure to untrusted code.

Suggested reviewers

• drorIvry

Poem

🐰 With pull_request_target we dance,
Secrets safe from fork's advance,
Credentials locked, PR heads fetched true,
Security and CI, both shiny and new! 🔐✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description is entirely empty, containing only the template structure with no actual content filled in. All critical sections are blank: Description, Motivation and Context, Type of Change (no checkboxes marked), Changes Made (no items listed), Testing details, and Related Issues/PRs (only placeholders remain). While the source branch name "107-bug---rogue-sanity-ci-not-running-on-prs-from-forks" suggests this relates to bug fix #107, this connection is not documented in the PR description itself.	The author should fill in the PR description with meaningful content, particularly: a clear description of the changes, the motivation (reference to issue #107), the type of change (marking "Bug fix" and "Configuration/build changes"), an explanation of what was changed in the workflow file, and relevant testing information. Even a brief description explaining why this security configuration change was needed would significantly improve clarity for reviewers.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Bug | Allow rogue_sanity on PRs" directly describes the main change in the pull request. The raw summary shows that the changes modify the GitHub Actions workflow to enable the rogue_sanity job to run on pull requests (by switching the trigger from pull_request to pull_request_target and adding necessary security measures). The title is concise, clear, and accurately reflects the primary purpose of this changeset.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 107-bug---rogue-sanity-ci-not-running-on-prs-from-forks

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc5e988 and 2f6e115.

📒 Files selected for processing (1)

• .github/workflows/rogue.yml (2 hunks)

🔇 Additional comments (3)

.github/workflows/rogue.yml (3)

4-4: Using pull_request_target to support fork PRs — verify this is intentional.

Switching from pull_request to pull_request_target allows fork PRs to trigger this workflow with access to secrets (via the environment). This is the standard approach for safely running CI on fork contributions, but it comes with a tradeoff: untrusted code from the PR is now executed in the workflow (lines 52, 63, 75).

Ensure this security model aligns with your team's expectations and threat model. The environment-based secret protection and persist-credentials: false mitigate exfiltration risk, but execution of untrusted code is still a consideration.

---

14-14: Verify the rogue-sanity-ci-secrets environment is configured in GitHub.

The workflow references an environment that must be pre-configured in your GitHub repository settings. If this environment does not exist, the workflow will fail. Confirm that:

1. The environment rogue-sanity-ci-secrets is created in repository settings.
2. Appropriate secret restrictions and approval policies are in place (if needed).
3. All required secrets (e.g., OPENAI_API_KEY on line 47) are added to this environment.

If this environment does not yet exist, create it or update the reference to an existing environment.

---

18-24: Good: Checkout configuration correctly prevents credential exposure.

The changes to the Checkout step are well-designed:

• ref: ${{ github.event.pull_request.head.sha }} ensures the actual PR code is checked out (required for pull_request_target).
• persist-credentials: false prevents the GITHUB_TOKEN from being exposed to the untrusted code, which is essential.

However, verify that github.event.pull_request.head.sha is available and correctly set for pull_request_target events triggered by fork PRs. This event property should exist, but it's worth confirming in a test run on a fork PR.

---

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