Repository navigation
ci: post ESLint's findings on the lines a pull request adds as review comments #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,69 @@ | ||||||
| name: Review comments | ||||||
|
|
||||||
| # Posts ESLint's findings on the lines a pull request adds as review comments, | ||||||
| # through reviewdog. It never fails: the build job's lint step in ci.yml is | ||||||
| # unchanged, and this workflow is not a required check. | ||||||
| # | ||||||
| # There is no path filter. reviewdog deletes its earlier comments whose finding | ||||||
| # is gone, but only in a run, so a push that drops a pull request's last change | ||||||
| # under src/ still needs one. | ||||||
|
|
||||||
| on: | ||||||
| pull_request: | ||||||
| # edited runs the job only when the base branch changed (see the job's if), | ||||||
| # so the comments follow the new diff; a title or body edit runs nothing. | ||||||
| types: [opened, synchronize, reopened, edited] | ||||||
|
|
||||||
| # reviewdog's github-pr-review reporter creates, lists and deletes review | ||||||
| # comments; pull-requests is the one scope it writes. | ||||||
| permissions: | ||||||
| contents: read | ||||||
| pull-requests: write | ||||||
|
|
||||||
| # A title or body edit takes a group of its own (its run id), so it cancels no | ||||||
| # lint run; any other run replaces the one before it on the same pull request. | ||||||
| concurrency: | ||||||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number }}-${{ github.event.action == 'edited' && !github.event.changes.base && github.run_id || 'eslint' }} | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Keep skipped Dependabot runs out of the lint concurrency group. If a maintainer starts lint on a Dependabot pull request and Dependabot updates that pull request before lint finishes, the new workflow run cancels the maintainer’s run. The job condition then skips the new run, so neither run posts findings. Give Dependabot-triggered runs a unique group, as this expression already does for title- and body-only edits. GitHub applies this concurrency setting to workflow runs, not only to jobs that pass their conditions. (docs.github.com) Proposed change--- "a/.github/workflows/review-comments.yml"
+++ "b/.github/workflows/review-comments.yml"
@@ -23,7 +23,7 @@
# A title or body edit takes a group of its own (its run id), so it cancels no
# lint run; any other run replaces the one before it on the same pull request.
concurrency:
- group: ${{ github.workflow }}-${{ github.event.pull_request.number }}-${{ github.event.action == 'edited' && !github.event.changes.base && github.run_id || 'eslint' }}
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number }}-${{ (github.actor == 'dependabot[bot]' || (github.event.action == 'edited' && !github.event.changes.base)) && github.run_id || 'eslint' }}
cancel-in-progress: true
jobs:📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| cancel-in-progress: true | ||||||
|
|
||||||
| jobs: | ||||||
| eslint: | ||||||
| # Forks are skipped: their token is read-only, so reviewdog could not post. | ||||||
| # Runs that Dependabot triggers are skipped: Dependabot changes manifests, | ||||||
| # never a line under src/, and skipping them keeps a newly bumped ESLint | ||||||
| # package from running with this token. A maintainer's push to a Dependabot | ||||||
| # branch still runs. | ||||||
| # A branch of this repository runs its own packages and ESLint config with | ||||||
| # this token; that gives its author nothing new, since they can already | ||||||
| # change any workflow on the branch. | ||||||
| if: >- | ||||||
| github.event.pull_request.head.repo.full_name == github.repository && | ||||||
| github.actor != 'dependabot[bot]' && | ||||||
| (github.event.action != 'edited' || github.event.changes.base) | ||||||
| runs-on: ubuntu-latest | ||||||
| timeout-minutes: 15 | ||||||
| steps: | ||||||
| # The pull request's head, so a finding's line number is its line in the diff. | ||||||
| - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 | ||||||
| with: | ||||||
| ref: ${{ github.event.pull_request.head.sha }} | ||||||
| persist-credentials: false | ||||||
| - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 | ||||||
| with: | ||||||
| node-version: '22' | ||||||
|
|
||||||
| # ESLint and its plugins are plain JavaScript: no build, no native modules. | ||||||
| - name: Install dependencies | ||||||
| run: npm install --ignore-scripts --no-audit --no-fund | ||||||
|
|
||||||
| - name: Post ESLint findings on added lines | ||||||
| uses: reviewdog/action-eslint@3ee2a450da17e060b9dfa4bffd53976906c6b2b0 # v1.36.1 | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Critical · Untrusted lint code gets write access When a same-repository pull request changes ESLint configuration or a dependency to execute attacker-controlled JavaScript, the ESLint action runs that JavaScript while receiving Run head-controlled linting without the write token, and keep credentialed comment posting in a step that does not execute pull-request-controlled code. Also at Prompt for AI agentsIn `.github/workflows/review-comments.yml` at line 50:
**Untrusted lint code gets write access**
When a same-repository pull request changes ESLint configuration or a dependency to execute attacker-controlled JavaScript, the ESLint action runs that JavaScript while receiving `GITHUB_TOKEN` with `pull-requests: write`, allowing it to make unauthorized pull-request API changes, including altering reviews or comments.
Run head-controlled linting without the write token, and keep credentialed comment posting in a step that does not execute pull-request-controlled code.
Also at `.github/workflows/review-comments.yml:21`, `.github/workflows/review-comments.yml:39`, `.github/workflows/review-comments.yml:52-53`. |
||||||
| with: | ||||||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||||||
| reporter: github-pr-review | ||||||
| filter_mode: added | ||||||
| # The action deprecates fail_on_error; fail_level none is its replacement and never fails. | ||||||
| fail_level: none | ||||||
| level: warning | ||||||
| # The pattern of npm run lint. The inner quotes leave the glob for ESLint to expand. | ||||||
| eslint_flags: '"src/**/*.{ts,tsx}"' | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,8 @@ By participating in this project, you agree to abide by our code of conduct: be | |
| 8. Push to your fork | ||
| 9. Open a Pull Request | ||
|
|
||
| On a pull request from a branch of this repository (not a fork), the Review comments workflow posts ESLint's findings on the lines the pull request adds under `src/` as review comments. Each run also deletes its earlier comments whose finding is gone, unless someone has replied to the comment. The workflow never fails the pull request. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Document the Dependabot exclusion. This sentence says the workflow posts comments on pull requests from repository branches. The workflow skips Dependabot pull requests. State that exception so contributors do not expect a run on those updates. 🧰 Tools🪛 LanguageTool[style] ~41-~41: Try using a synonym here to strengthen your wording. (COMMENT_REMARK) 🤖 Prompt for AI Agents |
||
|
|
||
| ## Development Setup | ||
|
|
||
| ### Prerequisites | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.