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
27 changes: 20 additions & 7 deletions .github/workflows/sdk-regression.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ jobs:
regression:
name: regression
runs-on: ubuntu-latest
# Least-privilege: read code/PR data and write commit statuses only. Note an
# author-permission guard below (check-access) already restricts triggering
# to write/admin collaborators (CWE-284 / PER-8610).
# Least-privilege GITHUB_TOKEN: read code/PR data, write commit statuses, and
# read this run's Actions job metadata (for the job log URL) only. Triggering
# is further gated by the author-permission check (check-access) below, which
# restricts it to write/admin collaborators. The powerful cross-repo dispatch
# PAT is NOT granted here; it is injected only into the single trigger step.
permissions:
contents: read
pull-requests: read
statuses: write
actions: read
# Run when either:
# - a maintainer comments RUN_REGRESSION on a PR, or
# - the workflow is dispatched manually (CLI branch + optional per-SDK refs).
Expand Down Expand Up @@ -140,17 +143,22 @@ jobs:
# failure must never gate the regression itself.
continue-on-error: true
with:
github_token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }}
# Third-party action: only reads this run's job metadata on percy/cli,
# so the built-in GITHUB_TOKEN (actions: read) is sufficient. The
# cross-repo dispatch PAT is deliberately not exposed to it.
github_token: ${{ github.token }}

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.

Good — moving job-id lookup + commit-status steps to the built-in github.token and adding only actions: read is exactly right.

P2 (residual — track, don't block): the PAT is still handed to the third-party convictional/trigger-workflow-and-wait. Unavoidable for cross-repo dispatch (the built-in token can't dispatch other repos), and SHA-pinning limits the code that receives it — but to shrink blast radius, prefer a fine-grained PAT scoped to just the SDK repos + actions:write, or a short-lived GitHub App installation token minted per run. (Can't verify the PAT's scope from the workflow — flag for secret-config review.)

P3 (pre-existing): the "Output Current Job Log URL" step around line 150 echoes steps.jobs.outputs.html_url, but the step id is job-url — so it always echoes empty. Since this PR is already the workflow-cleanup PR, cheap to fix the reference or drop the step. Also consider bumping actions/github-script off the 2021-era v4.1.0 (SHA-pinned, so not urgent).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. P3 fixed in 53c174f — the "Output Current Job Log URL" step now references steps.job-url.outputs.html_url (the actual step id) instead of the empty steps.jobs...; the commit-status target_url already used the correct id. Left the SHA-pinned actions/github-script as-is (not urgent, per your note).

P2 (PAT to the third-party dispatch action): agreed it's a residual we can't remove in-workflow (the built-in token can't dispatch other repos). Flagging for secret-config review to move it to a fine-grained PAT scoped to the SDK repos + actions:write (or a per-run GitHub App installation token) — tracked as a follow-up, not in this workflow-cleanup PR.

job_name: "regression (${{ matrix.repo }})"
# The fan-out matrix is >30 jobs (currently 35); the action defaults to
# per_page=30, so jobs on page 2 resolve to null and exit 1. Cover the
# whole matrix (GitHub jobs API max page size is 100).
per_page: 100
- name: Output Current Job Log URL
run: echo ${{ steps.jobs.outputs.html_url }}
run: echo ${{ steps.job-url.outputs.html_url }}
- uses: actions/github-script@f891eff65186019cbb3f7190c4590bc0a1b76fbc # v4.1.0
with:
github-token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }}
# Writes a commit status on percy/cli itself (job-level statuses: write),
# so the built-in GITHUB_TOKEN is used instead of the cross-repo PAT.
github-token: ${{ github.token }}
script: |
const { owner, repo } = context.repo;
const sha = '${{ steps.pr.outputs.head_sha }}'
Expand Down Expand Up @@ -201,6 +209,9 @@ jobs:
with:
owner: percy
repo: ${{ steps.split.outputs._0 }}
# Only step that strictly needs the cross-repo dispatch PAT: it triggers
# test.yml in a *different* percy repo, which the built-in GITHUB_TOKEN
# cannot do. Injection is scoped to this step alone.
github_token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }}
# Most SDKs expose `test.yml`; a few use a differently-named workflow
# that carries the @percy/cli inject step:
Expand All @@ -214,7 +225,9 @@ jobs:
- name: Update Status
uses: actions/github-script@f891eff65186019cbb3f7190c4590bc0a1b76fbc # v4.1.0
with:
github-token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }}
# Writes a commit status on percy/cli itself (job-level statuses: write),
# so the built-in GITHUB_TOKEN is used instead of the cross-repo PAT.
github-token: ${{ github.token }}
script: |
const { owner, repo } = context.repo;
const sha = '${{ steps.pr.outputs.head_sha }}'
Expand Down
Loading