Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions .github/workflows/auto-fix-on-failure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ on:
- "Verify Build (Cross-Platform)"
- "PolyPilot Integration Test"
types: [completed]
branches-ignore:
- main

env:
GH_TOKEN: ${{ github.token }}
Expand All @@ -23,7 +21,7 @@ jobs:
if: github.event.workflow_run.conclusion == 'failure'
permissions:
actions: write
pull-requests: read
pull-requests: write

steps:
- name: Find associated PR
Expand All @@ -36,21 +34,32 @@ jobs:
echo "Failed workflow: $FAILED_WORKFLOW"
echo "Failed run: $FAILED_RUN"

# Find open PR for this branch
PR_NUMBER=$(gh pr list --repo ${{ github.repository }} \
--head "$BRANCH" --state open \
--json number --jq '.[0].number // empty')
# workflow_dispatch runs execute on main but carry the PR number
# in their inputs. Try reading it from the API first.
PR_NUMBER=$(gh api "repos/${{ github.repository }}/actions/runs/$FAILED_RUN" \
--jq '.inputs.pr_number // empty' 2>/dev/null || true)
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.

🟡 MODERATE — Defense-in-depth: validate pr_number before use

The value from .inputs.pr_number is user-controllable (set at workflow dispatch time) and written to $GITHUB_OUTPUT with simple echo "key=value" syntax. A newline in the value could inject extra output keys.

Practical risk is low because both upstream workflows (verify-build.yml, polypilot-integration.yml) declare this input as type: number, and all subsequent outputs overwrite any injected duplicates. Still, a one-line guard follows the principle of least trust:

Suggested change
--jq '.inputs.pr_number // empty' 2>/dev/null || true)
PR_NUMBER=$(gh api "repos/$\{\{ github.repository }}/actions/runs/$FAILED_RUN" \
--jq '.inputs.pr_number // empty' 2>/dev/null || true)
# Sanitize: must be a bare integer (defense-in-depth)
[[ "$PR_NUMBER" =~ ^[0-9]+$ ]] || PR_NUMBER=""


# Fall back to branch-based lookup for push/PR-triggered runs
if [ -z "$PR_NUMBER" ]; then
echo "No open PR found for branch $BRANCH — skipping"
PR_NUMBER=$(gh pr list --repo ${{ github.repository }} \
--head "$BRANCH" --state open \
--json number --jq '.[0].number // empty')
fi

if [ -z "$PR_NUMBER" ]; then
echo "No associated PR found — skipping"
echo "has_pr=false" >> $GITHUB_OUTPUT
exit 0
fi

echo "Found PR #$PR_NUMBER"
# Resolve the PR's head branch for the comment
PR_BRANCH=$(gh pr view "$PR_NUMBER" --repo ${{ github.repository }} \
Comment on lines 53 to +56
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.

🟡 MODERATE — PR_NUMBER should be validated as numeric before use (Flagged by 3/3 reviewers)

PR_NUMBER originates from the failed run's .inputs.pr_number (line 39), which is attacker-influenceable if the dispatch chain can be reached from a PR author. Before using it in API calls or writing it to $GITHUB_OUTPUT, validate it is a bare integer:

          if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then
            echo "Invalid PR number '$PR_NUMBER' — skipping"
            echo "has_pr=false" >> $GITHUB_OUTPUT
            exit 0
          fi

This also mitigates the 🔴 shell injection finding below by sanitizing the value at the source before it flows into downstream steps.

--json headRefName --jq '.headRefName' 2>/dev/null || echo "$BRANCH")

echo "Found PR #$PR_NUMBER (branch: $PR_BRANCH)"
echo "pr_number=$PR_NUMBER" >> $GITHUB_OUTPUT
echo "has_pr=true" >> $GITHUB_OUTPUT
echo "branch=$BRANCH" >> $GITHUB_OUTPUT
echo "branch=$PR_BRANCH" >> $GITHUB_OUTPUT
echo "failed_workflow=$FAILED_WORKFLOW" >> $GITHUB_OUTPUT
echo "failed_run=$FAILED_RUN" >> $GITHUB_OUTPUT

Expand Down
Loading