🐛 Fix GitHub Actions workflow bugs and typos#6865
Conversation
- reusable-lint-providers: pass BASE_SHA from PR context so only-new-issues actually filters to new issues instead of silently doing nothing - pr-test-lint: add pull_request trigger so fork PRs get tested - providers.yaml: quote step outputs to prevent shell word-splitting - Fix typos: "Provider PAth" → "Provider Path", "becasue" → "because", "select to build" → "selected to build" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/review carefully analyze the pr-test-lint.yml file for potential security issues, as this will be run for fork PRs that we have no control over |
.github/workflows/pr-test-lint.yml
Outdated
| - ".github/workflows/pr-test-lint.yml" | ||
| - ".github/workflows/reusable-lint-providers.yml" | ||
| - "**.toml" # run tests when any recording changed | ||
| pull_request: |
There was a problem hiding this comment.
this would allow running the workflow against forked PR - i'd be hesitant to do it as-is; at the very least, we have steps that run on our self-hosted runners, and allowing 3rd party code to execute against those is probably not a good idea
Self-hosted runners should not execute untrusted fork code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Good catch — reverted the |
| - name: Run golangci-lint on all providers | ||
| env: | ||
| ONLY_NEW_ISSUES: ${{ inputs.only-new-issues }} | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} |
There was a problem hiding this comment.
🔴 critical — Good catch — BASE_SHA was referenced in the script but never set as an environment variable. This fix is correct and necessary for the --new-from-rev logic to work at all.
| run: | | ||
| echo "=== Providers detected:" | ||
| echo ${{ steps.providers.outputs.providers }} | ||
| echo '${{ steps.providers.outputs.providers }}' |
There was a problem hiding this comment.
🟡 warning — Good fix. Quoting the expression with single quotes prevents shell word-splitting and glob expansion on the output, which also mitigates a potential script injection vector if the output contained attacker-controlled content. Correct as-is.
Summary
BASE_SHAwas never set, so theonly-new-issuesinput (used bypr-extended-linting.yml) silently did nothing. Now passesgithub.event.pull_request.base.shaso--new-from-revactually filters to new issues.${{ steps.providers.outputs.* }}in the debug echo to prevent shell word-splitting of JSON arrays. Also fixed "select" → "selected" typo.Test plan
pr-extended-lintingnow shows only new lint issues on a PR with known lint changes🤖 Generated with Claude Code