security: harden sdk-regression issue_comment workflow — pin actions, scope permissions + token (PER-8610)

[Shivanshu-07]

Summary

Hardens .github/workflows/sdk-regression.yml (F-012, CWE-284/CWE-269). This issue_comment-triggered workflow injects the cross-repo dispatch PAT WORKFLOW_DISPATCH_ACTIONS_TOKEN into more steps than strictly need it — including a third-party action — so a step compromise could exfiltrate a token that can dispatch workflows across other percy/* repos.

The issue_comment trigger intent, the author-permission guard (write/admin collaborators only), and the regression-trigger flow are all unchanged.

Actions SHA-pinned (verified)

All third-party action uses: are pinned to full 40-char commit SHAs with a trailing # vX.Y.Z comment. Each tag was re-resolved live against the GitHub API for this PR and confirmed correct (already pinned by an earlier hardening pass; no SHA changed):

Action	Version	Commit SHA
actions/github-script	v4.1.0	f891eff65186019cbb3f7190c4590bc0a1b76fbc
xt0rted/pull-request-comment-branch	v3.0.0	e8b8daa837e8ea7331c0003c9c316a64c6d8b0b1
actions-ecosystem/action-regex-match	v2.0.2	9e6c4fb3d5e898f505be7a1fb6e7b0a278f6665b
Tiryoh/gha-jobid-action	v1.4.0	be260d8673c9211a84cdcf37794ebd654ba81eef
winterjung/split	v2.1.0	7f51d99e7cc1f147f6f99be75acf5e641930af88
convictional/trigger-workflow-and-wait	v1.6.5	f69fa9eedd3c62a599220f4d5745230e237904be

Permissions block

Top-level permissions: contents: read (already present). The regression job keeps its least-privilege grant and adds actions: read so the built-in GITHUB_TOKEN can read this run's job metadata:

permissions:
  contents: read
  pull-requests: read
  statuses: write
  actions: read   # added

How the token injection was narrowed

WORKFLOW_DISPATCH_ACTIONS_TOKEN is now injected into exactly one step — the only one that strictly requires it:

• convictional/trigger-workflow-and-wait — keeps the PAT. It dispatches test.yml in a different percy/* repo, which the built-in GITHUB_TOKEN cannot do.
• Tiryoh/gha-jobid-action (third-party) — switched to ${{ github.token }}. It only reads this run's job metadata on percy/cli; the new actions: read grant covers it. The cross-repo PAT is no longer handed to a third-party action.
• Both actions/github-script commit-status steps — switched to ${{ github.token }}. They only write commit statuses on percy/cli, covered by the existing job-level statuses: write.

No workflow/job-level env exposed the token; the over-exposure was per-step with: inputs, which is what this PR narrows.

🤖 Generated with Claude Code

[Shivanshu-07]

Claude Code PR Review

PR: #2332 • Head: e9e1f97 • Reviewers: fallback inline checklist (no in-scope project reviewers; CI-workflow-only change reviewed against a hardening-specific rubric)

Summary

Hardens .github/workflows/sdk-regression.yml: the powerful cross-repo WORKFLOW_DISPATCH_ACTIONS_TOKEN PAT is now injected into only the single step that strictly needs it (the convictional/trigger-workflow-and-wait cross-repo dispatch); the three same-repo steps (job-log lookup + two createCommitStatus scripts) switch to the built-in github.token, and the job permissions: block gains actions: read to support the job-log lookup under least privilege. All uses: were already SHA-pinned. PER-8610.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Only references secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN / github.token; no literal secrets.
High	Security	Authentication/authorization checks present	Pass	check-access gates the comment path to write/admin collaborators; trigger if: restricts events; unchanged by this PR and preserved.
High	Security	Input validation and sanitization	Pass	Untrusted head ref passed via env and regex-validated (^[a-zA-Z0-9_/\-]+$) before any downstream use; unchanged, preserved.
High	Security	No IDOR — resource ownership validated	N/A	No user-addressable resource IDs.
High	Security	No SQL injection (parameterized queries)	N/A	No SQL.
High	Correctness	Logic is correct, handles edge cases	Pass	PAT scoped to the one cross-repo step; github.token sufficient for same-repo createCommitStatus (statuses: write) and job-metadata lookup (actions: read). Trigger if: and matrix intact after merge.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Job-URL lookup keeps continue-on-error: true (target_url is best-effort); no error handling weakened.
High	Correctness	No race conditions or concurrency issues	N/A	No new concurrency; fail-fast: false matrix behavior unchanged.
Medium	Testing	New code has corresponding tests	N/A	CI workflow file; not unit-testable. YAML parse-validated locally (OK).
Medium	Testing	Error paths and edge cases tested	N/A	See above.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Token/permission change does not alter regression semantics; same-repo API calls covered by granted scopes.
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	per_page: 100 (merged from master) covers the 35-job matrix in one page.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	SHA-pin + version-comment convention maintained; comments explain token scoping rationale.
Medium	Quality	Changes are focused (single concern)	Pass	Single file; solely token-scoping + permission hardening.
Low	Quality	Meaningful names, no dead code	Pass	No dead code introduced.
Low	Quality	Comments explain why, not what	Pass	Added comments justify PAT scoping / least-privilege; no ticket ids embedded in the changed comments.
Low	Quality	No unnecessary dependencies added	Pass	No new actions; actions: read scope is the minimum for the existing job-log action.

Hardening-specific checks (task rubric)

• All uses: SHA-pinned (40-char) + version comment: Pass — all 8 pins verified 40 hex chars with # vX.Y.Z comments (github-script f891eff, pull-request-comment-branch e8b8daa, action-regex-match 9e6c4fb, gha-jobid-action be260d8, split 7f51d99, trigger-workflow-and-wait f69fa9e).
• PAT scoped to only the step that needs it: Pass — WORKFLOW_DISPATCH_ACTIONS_TOKEN appears exactly once (line 215, the cross-repo Trigger Workflow & Wait step). Never exposed to the third-party gha-jobid-action step.
• permissions: least-privilege and sufficient: Pass — contents: read, pull-requests: read, statuses: write (for createCommitStatus), actions: read (for job-metadata lookup). Nothing over-granted; nothing required is missing.
• Trigger behavior preserved after merge: Pass — if: (issue_comment RUN_REGRESSION on a PR, gated by check-access, OR workflow_dispatch) and per_page: 100 present and correct.
• YAML / expressions: Pass — parses cleanly; github.token is a valid context expression.

Findings

No Critical/High/Medium findings. Two pre-existing (not introduced by this PR, outside the diff) informational nits:

• File: .github/workflows/sdk-regression.yml:156

• Severity: Low (informational, pre-existing)

• Reviewer: fallback inline checklist

• Issue: Output Current Job Log URL echoes steps.jobs.outputs.html_url, but the step id is job-url; this reference is empty. Purely a diagnostic echo — the commit-status steps correctly use steps.job-url.outputs.html_url.

• Suggestion: Optionally rename to steps.job-url.outputs.html_url. Out of scope for this hardening PR.

• File: .github/workflows/sdk-regression.yml:92-93

• Severity: Low (informational, pre-existing)

• Reviewer: fallback inline checklist

• Issue: Comment refers to a "label path", but the workflow triggers on issue_comment (RUN_REGRESSION), not labels. Stale wording only; logic is correct.

• Suggestion: Optionally reword. Out of scope for this PR.

---

Verdict: PASS

[Shivanshu-07]

Claude Code PR Review

PR: #2332 • Head: e9e1f97 • Reviewers: orchestrator inline review (security + correctness + quality lens)

Note: HEAD (e9e1f97) is a merge of master into the branch, but it is not a base-merge no-op — the PR carries a novel, unreviewed security commit (d9f630ba, not in master) touching .github/workflows/sdk-regression.yml. A full review was run rather than fast-path skipped.

Summary

Supply-chain / CI hardening (PER-8610) for the issue_comment-triggered sdk-regression.yml workflow: the powerful cross-repo dispatch PAT (WORKFLOW_DISPATCH_ACTIONS_TOKEN) is de-scoped from three steps (job-log lookup + two commit-status writes) to the single step that genuinely needs it (the cross-repo trigger-workflow-and-wait); those three steps now use the built-in github.token, and the job's permissions: block gains actions: read to keep the job-log lookup working under least privilege.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Only secrets.* / github.token contexts used; no literal tokens.
High	Security	Authentication/authorization checks present	Pass	check-access + Check Access Level gate the comment path to write/admin collaborators; the sole PAT step runs after that gate.
High	Security	Input validation and sanitization	Pass	Untrusted head_ref passed via env (quoted) and regex-validated ^[a-zA-Z0-9_/\-]+$ before any downstream use; sdk_refs likewise validated.
High	Security	No IDOR — resource ownership validated	N/A	No user-supplied resource IDs; status writes scoped to context.repo.
High	Security	No SQL injection (parameterized queries)	N/A	No database access.
High	Correctness	Logic is correct, handles edge cases	Pass	github.token + actions:read/statuses:write suffice for the re-scoped steps; PAT retained where cross-repo dispatch requires it.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Access gate uses exit 1; job-log lookup is continue-on-error by design (must not gate the run).
High	Correctness	No race conditions or concurrency issues	N/A	No shared mutable state; matrix jobs are independent.
Medium	Testing	New code has corresponding tests	N/A	Workflow YAML; no unit-test surface (validated by execution on the PR).
Medium	Testing	Error paths and edge cases tested	N/A	As above.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Change is scoped to token/permission wiring; functional path unchanged.
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	per_page: 100 bounds the job-metadata lookup for the 35-job matrix.
Medium	Performance	Long-running tasks use background jobs	N/A	Fan-out already runs as a matrix.
Medium	Quality	Follows existing codebase patterns	Pass	Consistent with the repo's SHA-pinning + env-passing + regex-gate conventions.
Medium	Quality	Changes are focused (single concern)	Pass	Diff limited to PAT de-scoping + actions: read + explanatory comments.
Low	Quality	Meaningful names, no dead code	Pass	See informational finding re: a pre-existing broken step reference (out of scope of this diff).
Low	Quality	Comments explain why, not what	Pass	Added comments document the token-scoping rationale clearly.
Low	Quality	No unnecessary dependencies added	Pass	No new actions added; all 6 third-party actions remain SHA-pinned.

Findings

No Critical, High, or Medium findings.

Informational (Low, pre-existing — not introduced by this PR, out of diff scope):

• File: .github/workflows/sdk-regression.yml:156
• Severity: Low
• Reviewer: orchestrator inline review
• Issue: The "Output Current Job Log URL" step runs echo ${{ steps.jobs.outputs.html_url }}, but the job-log action's step id is job-url, not jobs, so this debug echo always prints empty. Not a security issue — html_url originates from GitHub's job-metadata API (not attacker-controlled), and the actual commit-status target_url correctly references steps.job-url.outputs.html_url.
• Suggestion: Change steps.jobs to steps.job-url (or drop the debug step). Safe to defer; unrelated to this hardening PR.

Adversarial verification results:

• Token reachability by untrusted commenter: not reachable — the only WORKFLOW_DISPATCH_ACTIONS_TOKEN use (line 215) is in the cross-repo trigger step, which executes after the write/admin access gate; trigger is issue_comment/workflow_dispatch, not pull_request_target.
• All third-party actions SHA-pinned: yes — all six (actions/github-script, xt0rted/pull-request-comment-branch, actions-ecosystem/action-regex-match, Tiryoh/gha-jobid-action, winterjung/split, convictional/trigger-workflow-and-wait) pinned to full commit SHAs.
• permissions: least-privilege: yes — top-level contents: read; job-level contents: read, pull-requests: read, statuses: write, actions: read (newly added, justified by the job-log lookup).
• Workflow injection via untrusted github.event: none found — head_ref is env-passed/quoted and regex-validated before reaching client_payload; comment.user.login is read via the context object in github-script (not shell); interpolated head_sha values are git SHAs.

---

Verdict: PASS

[pranavz28]

Automated multi-agent security review. Approve — the least-privilege split is correct and each remaining permission is used: statuses: write covers the commit-status calls, the added actions: read covers the job-id lookup, and the cross-repo dispatch PAT is narrowed to the one step that genuinely needs it. Verified: no checkout of PR-author code anywhere in the job, check-access gates before the PAT step and fails closed, head_ref is regex-validated before its only raw use, and all third-party actions are full-SHA pinned. A couple of optional cleanups inline / below.

[pranavz28]

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).

[Shivanshu-07]

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.

[pranavz28]

Run regression once. I tried with content: read permissions, it doesnt work it seems.

[pranavz28]

RUN_REGRESSION

[pranavz28]

RUN_REGRESSION

[Shivanshu-07]

Claude Code PR Review

PR: #2332 • Head: 53c174f • Reviewers: orchestrator inline (fresh clone — no in-repo reviewers)

Summary

CI/supply-chain hardening of .github/workflows/sdk-regression.yml (PER-8610): the powerful cross-repo dispatch PAT (WORKFLOW_DISPATCH_ACTIONS_TOKEN) is de-scoped from the job-metadata and commit-status steps and injected only into the single cross-repo trigger step; the GITHUB_TOKEN block gains least-privilege actions: read; and the broken job-log-URL echo (steps.jobs) is fixed to reference the correct step id (steps.job-url).

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Secret referenced via secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN; no literal tokens. PR reduces the PAT's exposure surface to one step.
High	Security	Authentication/authorization checks present	Pass	Comment path gated by check-access (write/admin only, exit 1 otherwise); dispatch path requires repo write. PAT now sits behind that gate.
High	Security	Input validation and sanitization	Pass	Untrusted head_ref passed via env: then regex-validated ^[a-zA-Z0-9_/\-]+$ before use; sdk-ref validated in-shell before dispatch.
High	Security	No IDOR — resource ownership validated	N/A	No resource-ownership logic; commit statuses scoped to context.repo.
High	Security	No SQL injection (parameterized queries)	N/A	No database access.
High	Correctness	Logic is correct, handles edge cases	Pass	per_page: 100 covers the >30-job matrix; workflow-file-name and default-ref selection handled; step-id fix verified against actual id: job-url.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	continue-on-error on job-url lookup is intentional (must not gate the run) and documented; no new swallowing introduced.
High	Correctness	No race conditions or concurrency issues	Pass	issue_comment runs the workflow from the base branch; no shared-state mutation introduced.
Medium	Testing	New code has corresponding tests	N/A	CI workflow YAML; validated by execution, not unit tests.
Medium	Testing	Error paths and edge cases tested	N/A	As above.
Medium	Testing	Existing tests still pass (no regressions)	N/A	No product/runtime code changed.
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Jobs API paging bounded (per_page: 100) to the fixed matrix size.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable to a dispatch workflow.
Medium	Quality	Follows existing codebase patterns	Pass	Consistent with existing SHA-pinning and env-var-passing conventions in the file.
Medium	Quality	Changes are focused (single concern)	Pass	Diff is limited to token de-scoping, the actions: read grant, and the step-id fix.
Low	Quality	Meaningful names, no dead code	Pass	No dead code; step ids and env names are descriptive.
Low	Quality	Comments explain why, not what	Pass	Comments clearly justify why each token/permission choice was made.
Low	Quality	No unnecessary dependencies added	Pass	No new actions; all 8 uses: remain pinned to full 40-hex SHAs.

Findings

No Fail-level findings. Adversarial verification of the four hardening claims all passed: the cross-repo PAT is unreachable by an untrusted commenter (permission gate aborts before the trigger step, no if: always() bypass downstream); every third-party action is SHA-pinned; the permissions: block is least-privilege; and untrusted github.event.* data (head_ref) is env-passed and regex-validated before any downstream use and is kept out of all github-script string contexts. The job-log-URL step-id reference is now correct.

• File: .github/workflows/sdk-regression.yml:156
• Severity: Low
• Reviewer: orchestrator inline
• Issue: run: echo ${{ steps.job-url.outputs.html_url }} interpolates the value unquoted into the shell. The value is a GitHub-generated URL (trusted), so this is not exploitable, but it is a minor deviation from the "pass via env, then quote" pattern used elsewhere in this same file.
• Suggestion: Pass the URL through an env: var and echo "$URL", consistent with the Resolve PR head ref step. Non-blocking.

---

Verdict: PASS