Skip to content

CLDSRV-926: skip async/await diff check when PR has skip-async-migration label#6189

Draft
DarkIsDude wants to merge 1 commit into
development/9.3from
w/9.3/feature/CLDSRV-926/skip-async-check-on-pr-label
Draft

CLDSRV-926: skip async/await diff check when PR has skip-async-migration label#6189
DarkIsDude wants to merge 1 commit into
development/9.3from
w/9.3/feature/CLDSRV-926/skip-async-check-on-pr-label

Conversation

@DarkIsDude

Copy link
Copy Markdown
Contributor

Summary

  • Adds pull-requests: read permission to the tests.yaml workflow
  • The "Check async/await compliance in diff" step now checks whether the current branch's open PR carries the skip-async-migration label before running check-diff-async
  • If the label is present the step exits cleanly; otherwise the check runs as before

Behaviour

Scenario Result
PR has skip-async-migration label Step skips, exits 0
PR does not have the label check-diff-async runs normally
No open PR (direct push / workflow_dispatch) check-diff-async runs normally

Pre-requisite

The skip-async-migration label must be created in the repository settings before use.

Closes CLDSRV-926

@DarkIsDude DarkIsDude self-assigned this Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
8472 1 8471 0
View the full list of 1 ❄️ flaky test(s)
"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With v4 signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 4 times)

Stack Traces | 0.015s run time
We encountered an internal error. Please try again.

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

echo "Skipping async/await check: 'skip-async-migration' label found"
else
yarn run check-diff-async
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

${{ github.ref_name }} is user-controlled (the branch name) and interpolated directly into a run: script, which is a script injection vector. The practical risk here is low since the workflow triggers on push (attacker already has write access) and permissions are read-only, but the standard hardening is to pass it through an env var instead of inline interpolation.

```suggestion
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BRANCH_NAME: ${{ github.ref_name }}
run: |
if gh pr list --head "$BRANCH_NAME" --state open --json labels
--jq '.[].labels[].name' 2>/dev/null | grep -qx 'skip-async-migration'; then
echo "Skipping async/await check: 'skip-async-migration' label found"
else
yarn run check-diff-async
fi

@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown
  • Script injection: ${{ github.ref_name }} is interpolated directly into a run: block. Pass it through an env var instead for defense-in-depth.

    Review by Claude Code

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