fix(workflows): complete #3016 interpolation triage follow-up - #3023
Conversation
Document 103 reviewed constrained script interpolations, convert remaining free-text inputs to env indirection, and extend the regression guard so new inputs.* / github.event.* hits cannot land without an explicit allowlist entry. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 seconds Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSelected workflow scripts now receive inputs through environment variables. The interpolation regression test scans workflow and event expressions, while documentation defines review rules. The campaign contract test verifies environment-based sync-hash handling. ChangesWorkflow interpolation hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
🤖 Keepalive Loop StatusPR #3023 | Agent: Codex | Iteration 0/12 Current State
Last Codex Run
To retry immediately:
Or wait for the next successful Gate run to automatically retry. 🔍 Failure Classification| Error type | infrastructure |
|
Keepalive Work Log (click to expand)
|
|
Runner dispatch state for codex on PR #3023. Do not edit. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e526b6f6f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [ | ||
| _normalize_expression(body) | ||
| for body in _actions_expression_bodies(script) | ||
| if re.search(r"\b(inputs\.|github\.event\.)", body) |
There was a problem hiding this comment.
Detect bracket-form workflow inputs in the guard
The new scanner only recognizes dot notation, so an interpolation such as ${{ inputs['repos'] }} or ${{ github['event']['issue']['body'] }} produces no hit and bypasses both the banned-expression check and the reviewed allowlist. This weakens the previous repository scan, whose _references_untrusted_input path explicitly recognized bracket-form inputs; use that parser or extend this matcher so changing a banned interpolation to equivalent bracket syntax still fails the regression test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR completes the #3016 “untrusted interpolation in run:/script: bodies” triage follow-up by documenting the reviewed inventory, expanding the regression test to enforce an explicit allowlist, and converting remaining free-text exemplars to step-level env: indirection in affected workflows.
Changes:
- Expanded the regression test to (a) ban specific free-text
inputs.*fields in script bodies and (b) require an explicit allowlist entry for every remaininginputs.*/github.event.*expression found inrun:/with.script:scalars. - Added repository documentation for the triage and the allowlist/test gate.
- Updated several workflows to pass previously-interpolated values via
env:and read them from shell/process.env.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/workflows/test_no_untrusted_interpolation.py |
Adds banned free-text list + reviewed allowlist gate for expressions inside script bodies. |
docs/workflows/script-interpolation-triage.md |
Documents the repo-wide triage results and the intended test gate behavior. |
.github/workflows/reusable-18-autofix.yml |
Moves inputs.commit_prefix consumption out of script interpolation into step env: indirection. |
.github/workflows/maint-82-sync-dependency-campaign.yml |
Moves github-script interpolations to env: + process.env reads. |
.github/workflows/maint-72-fix-pr-body-conflicts.yml |
Moves inputs.target_repo consumption to step env: indirection. |
.github/workflows/agents-keepalive-branch-sync.yml |
Moves inputs.head_is_fork / inputs.head_repository consumption to step env: indirection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _script_interpolation_hits(script: str) -> list[str]: | ||
| return [ | ||
| _normalize_expression(body) | ||
| for body in _actions_expression_bodies(script) | ||
| if re.search(r"\b(inputs\.|github\.event\.)", body) | ||
| ] |
| def _is_banned_free_text(body: str) -> bool: | ||
| normalized = _normalize_expression(body) | ||
| return any( | ||
| normalized == banned | ||
| or normalized.startswith(f"{banned} ") | ||
| or normalized.startswith(f"{banned}||") | ||
| for banned in BANNED_FREE_TEXT_EXPRESSIONS | ||
| ) |
| - name: Set commit prefix env | ||
| id: prefix | ||
| run: echo "AUTOFIX_COMMIT_PREFIX=${{ inputs.commit_prefix }}" >> "$GITHUB_ENV" | ||
| env: | ||
| COMMIT_PREFIX: ${{ inputs.commit_prefix }} | ||
| run: echo "AUTOFIX_COMMIT_PREFIX=$COMMIT_PREFIX" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
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/reusable-18-autofix.yml:
- Around line 155-157: Validate COMMIT_PREFIX for carriage-return or line-feed
characters before writing it to GITHUB_ENV, rejecting multiline input. In the
step that exports AUTOFIX_COMMIT_PREFIX, use printf for the environment
assignment rather than echo, while preserving the existing input value for valid
single-line prefixes.
In `@tests/workflows/test_no_untrusted_interpolation.py`:
- Around line 203-218: Update `_script_interpolation_hits` to detect indexed
context references as well as dotted `inputs` and `github.event` references,
including `inputs['commit_message']` and `github['event']['issue']['title']`.
Preserve existing matches and add regression cases covering both indexed forms.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5835a4d7-5335-4e8e-ba39-9a2859010d74
📒 Files selected for processing (7)
.github/workflows/agents-keepalive-branch-sync.yml.github/workflows/maint-72-fix-pr-body-conflicts.yml.github/workflows/maint-82-sync-dependency-campaign.yml.github/workflows/reusable-18-autofix.ymldocs/workflows/script-interpolation-triage.mdlangsmith-fleet-worker-attempt.jsontests/workflows/test_no_untrusted_interpolation.py
There was a problem hiding this comment.
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/reusable-10-ci-python.yml:
- Around line 2001-2010: Update the Python coverage-threshold validation in
validate_number to reject non-finite parsed values such as NaN and infinities
before range checks or comparisons. Add a regression case covering coverage-min:
NaN, ensuring it fails validation and is not passed to coverage_trend.py as a
valid minimum.
In `@docs/workflows/script-interpolation-triage.md`:
- Line 13: Update the reviewed-interpolation count in the documentation table
and the remaining constrained-tuples statement to use the same value derived
from REVIEWED_SCRIPT_INTERPOLATIONS, replacing the inconsistent hardcoded counts
while preserving the surrounding wording.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e727061-aa3c-4d06-9692-71cbab6d71c0
📒 Files selected for processing (7)
.github/workflows/maint-45-cosmetic-repair.yml.github/workflows/reusable-10-ci-python.yml.github/workflows/reusable-18-autofix.ymldocs/workflows/script-interpolation-triage.mdlangsmith-fleet-worker-attempt.jsontests/workflows/test_maint82_sync_campaign_contract.pytests/workflows/test_no_untrusted_interpolation.py
💤 Files with no reviewable changes (1)
- langsmith-fleet-worker-attempt.json
| | --- | ---: | --- | | ||
| | Free-text inputs moved to `env:` in #3020 | 3 fields | `commit_message`, `codex_args`, `repos` | | ||
| | Additional free-text fixes in this PR | 4 fields | `target_repo`, `commit_prefix`, `head_repository`, campaign script outputs | | ||
| | Reviewed constrained interpolations | 97 | Explicit allowlist in `test_no_untrusted_interpolation.py` | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the reviewed-interpolation count.
Line 13 states 97 reviewed constrained interpolations. Line 32 states 103 remaining constrained tuples. Use the count from REVIEWED_SCRIPT_INTERPOLATIONS consistently in both locations.
🤖 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 `@docs/workflows/script-interpolation-triage.md` at line 13, Update the
reviewed-interpolation count in the documentation table and the remaining
constrained-tuples statement to use the same value derived from
REVIEWED_SCRIPT_INTERPOLATIONS, replacing the inconsistent hardcoded counts
while preserving the surrounding wording.
🤖 Bot Comment Handler
The agent has been assigned to this PR to address the bot review comments. Instructions for agent
The bot comment handler workflow has prepared context in the artifacts. |
|
Closer update: reconciled the reviewed-interpolation count to the current allowlist (103) in a725b5d; |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Closes #3016
Automated Status Summary
Scope
Scope section missing from source issue.
Context for Agent
Related Issues/PRs
Tasks
maint-70-fix-integration-formatting.yml:124(inputs.commit_message) andreusable-codex-run.yml(inputs.codex_args) — move each to a step-levelenv:entry and reference"$VAR"in the script.maint-69-sync-labels.yml's 3×${{ inputs.repos }}in the "Determine target repos" step the same way..github/workflows/*.ymlinventory: convertrun:/script:interpolations toenv:indirection where free-form; document deliberate exceptions inline where provably constrained (e.g. boolean or number)..github/workflows/and fails on${{ inputs.* }}/${{ github.event.* }}inside arun:/script:block scalar, with an explicit allowlist for reviewed exceptions.Acceptance criteria
tests/workflows/test_no_untrusted_interpolation.py::test_no_untrusted_expressions_in_script_bodies— parses each workflow, walksrun:/script:block scalars, and asserts noinputs./github.event.expression appears except those in an explicit, commented allowlist.git commit -m "${{ inputs.commit_message }}"inmaint-70-fix-integration-formatting.yml→ confirm the named test FAILS → revert.maint-70withcommit_messageset toa"band confirm the step succeeds (today it produces a brokengit commitcommand). Note this dispatch is currently blocked by [P1][ops] All agent issue-automation workflows halt at action_required with zero jobs — agents:auto-pilot is a no-op #3009.Summary by CodeRabbit
Bug Fixes
Documentation
Tests
Chores