Skip to content

fix: run E2E tests after i18n completes on release PRs #8111

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.

Sign up for GitHub

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

Closed
Myestery wants to merge 6 commits into from
Closed

fix: run E2E tests after i18n completes on release PRs #8111

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50ef83e
fix: run E2E tests after i18n completes on release PRs
Myestery Jan 16, 2026
af69acd
ci: Exclude version-bump branches from e2e workflow pull request trig…
Myestery Jan 16, 2026
821cf4a
ci: extract get-pr-info job to avoid duplicate PR lookups
Myestery Jan 16, 2026
7bc0161
ci: use env vars to prevent script injection in PR comment steps
Myestery Jan 16, 2026
187e364
ci: simplify PR lookup using gh pr view
Myestery Jan 16, 2026
4ae0773
feat: Add GitHub Actions workflow for Playwright E2E tests with Cloud…
Myestery Jan 17, 2026
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
69 changes: 58 additions & 11 deletions .github/workflows/ci-tests-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ on:
branches: [main, master, core/*, desktop/*]
pull_request:
branches-ignore:
[wip/*, draft/*, temp/*, vue-nodes-migration, sno-playwright-*]
[wip/*, draft/*, temp/*, vue-nodes-migration, version-bump-*]
# Run after i18n workflow completes for version-bump PRs
workflow_run:
workflows: ['i18n: Update Core']
types: [completed]
Comment on lines +10 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing branch filter on workflow_run trigger.

The workflow_run trigger fires for any completion of the i18n workflow, not just version-bump PRs. This will cause E2E tests to run after every i18n update regardless of branch, which is not the intended behavior per the PR objectives.

Add a condition to the setup job (or all jobs that should only run for version-bump PRs via workflow_run) to filter by branch:

if: >
  github.event_name != 'workflow_run' ||
  startsWith(github.event.workflow_run.head_branch, 'version-bump-')

Alternatively, consider filtering earlier at the workflow level using a preliminary job that gates downstream jobs.

🤖 Prompt for AI Agents 
In @.github/workflows/ci-tests-e2e.yaml around lines 10 - 13, The workflow_run
trigger currently fires for any completion of the i18n workflow; update the job
gating so E2E jobs only run for version-bump PRs by adding the provided
conditional check to the relevant jobs (e.g., the setup job) referencing the
workflow_run trigger and the setup job name: add an if condition that checks
github.event_name and startsWith(github.event.workflow_run.head_branch,
'version-bump-') so the workflow only proceeds when not triggered by
workflow_run or when the head branch starts with "version-bump-".

Copy link
Contributor

@DrJKL DrJKL Jan 16, 2026

Choose a reason for hiding this comment

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

https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#onworkflow_runbranchesbranches-ignore

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

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

@DrJKL Great point! You can specify what branches the triggering workflow must run on using the branches or branches-ignore filters directly on the workflow_run event, which is much cleaner than adding job-level conditionals.

The workflow can be updated to use native branch filtering:

workflow_run:
  workflows: ['i18n: Update Core']
  types: [completed]
  branches:
    - 'version-bump-*'

This will ensure the E2E workflow only triggers via workflow_run when the i18n workflow completes on version-bump branches, which is exactly the intended behavior described in the PR objectives.

🧠 Learnings used 
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Run quality gates before PR: `pnpm lint`, `pnpm typecheck`, `pnpm knip`, and ensure relevant tests pass

Copy link

@claude claude bot Jan 16, 2026

Choose a reason for hiding this comment

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

[architecture] medium Priority

Issue: Potential race condition with workflow_run trigger
Context: workflow_run trigger runs on completed workflows, but doesn't verify if the i18n workflow actually succeeded - it will run even if i18n failed
Suggestion: Add condition to check workflow_run.conclusion == 'success' to ensure E2E only runs after successful i18n completion

Comment on lines +10 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add branch filter and success check to workflow_run trigger.

The workflow_run trigger currently fires for any completion of the i18n workflow on any branch, not just version-bump PRs. Additionally, types: [completed] fires regardless of whether the i18n workflow succeeded or failed.

Add a branches filter and check workflow_run.conclusion in downstream jobs:

   workflow_run:
     workflows: ['i18n: Update Core']
     types: [completed]
+    branches:
+      - 'version-bump-*'

Then add a conclusion check to the setup job (or relevant jobs):

if: >
  github.event_name != 'workflow_run' ||
  github.event.workflow_run.conclusion == 'success'
🤖 Prompt for AI Agents 
In @.github/workflows/ci-tests-e2e.yaml around lines 10 - 13, The workflow_run
trigger currently listens to any completion of the i18n workflow; add a branches
filter under workflow_run (e.g., branches:
[<version-bump-branch-name-or-pattern>]) to restrict it to the version-bump
branches and keep workflows: ['i18n: Update Core'] and types: ['completed'];
then update downstream jobs (for example the setup job) to gate execution on the
upstream conclusion by adding an if condition that checks github.event_name !=
'workflow_run' || github.event.workflow_run.conclusion == 'success' so the job
only runs when the workflow_run succeeded or when triggered by other events.


concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
group: ${{ github.workflow }}-${{ github.event.workflow_run.head_branch || github.ref }}
Copy link

@claude claude bot Jan 16, 2026

Choose a reason for hiding this comment

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

[architecture] low Priority

Issue: Concurrency group may cause unexpected cancellations across different event types
Context: Using the same concurrency group for both pull_request and workflow_run events could cause workflows to cancel each other when they should run independently
Suggestion: Consider using separate concurrency groups for different event types to prevent cross-event cancellation

cancel-in-progress: true

jobs:
Expand All @@ -18,6 +22,8 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}
- name: Setup frontend
uses: ./.github/actions/setup-frontend
with:
Expand Down Expand Up @@ -52,6 +58,8 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}
- name: Download built frontend
uses: actions/download-artifact@v4
with:
Expand Down Expand Up @@ -99,6 +107,8 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}
- name: Download built frontend
uses: actions/download-artifact@v4
with:
Expand Down Expand Up @@ -143,6 +153,8 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}

- name: Install pnpm
uses: pnpm/action-setup@v4
Expand Down Expand Up @@ -175,15 +187,43 @@ jobs:
# when using pull_request event, we have permission to comment directly
# if its a forked repo, we need to use workflow_run event in a separate workflow (pr-playwright-deploy.yaml)

# Get PR info once for reuse by comment jobs
get-pr-info:
runs-on: ubuntu-latest
if: |
(github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false) ||
(github.event_name == 'workflow_run')
outputs:
pr_number: ${{ steps.get-pr.outputs.number }}
branch: ${{ steps.get-pr.outputs.branch }}
steps:
- name: Get PR number
id: get-pr
env:
GH_TOKEN: ${{ github.token }}
PR_TARGET_REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_BRANCH: ${{ github.head_ref || github.event.workflow_run.head_branch }}
run: |
Copy link

@claude claude bot Jan 16, 2026

Choose a reason for hiding this comment

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

[quality] medium Priority

Issue: Missing error handling for gh command failures
Context: If the gh pr view command fails (e.g., PR deleted, permissions issue), the workflow will continue with empty PR_NUMBER
Suggestion: Add error checking after the gh command and fail the job if PR info cannot be retrieved: set -e at the beginning of the script block or explicit error checking

echo "branch=${PR_BRANCH}" >> $GITHUB_OUTPUT
if [ -n "$PR_NUMBER" ]; then
echo "number=${PR_NUMBER}" >> $GITHUB_OUTPUT
else
gh pr view --repo "${PR_TARGET_REPO}" "${PR_BRANCH}" \
Copy link

@claude claude bot Jan 16, 2026

Choose a reason for hiding this comment

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

[security] medium Priority

Issue: Environment variables used in conditional logic without validation
Context: Using gh commands with environment variables can lead to shell injection if branch names contain special characters
Suggestion: Add proper input validation and quoting around all variables passed to shell commands

--json 'number' --jq '"number=\(.number)"' >> $GITHUB_OUTPUT
fi

Comment on lines +190 to +215
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

GitHub Actions script injection github.head_ref workflow_run branch name security

💡 Result:

Short answer: branch names (e.g. github.event.pull_request.head.ref / github.head_ref) and workflow_run triggers are untrusted inputs and have been used to inject shell/Actions expressions to execute arbitrary commands on runners. Treat them as attacker-controlled, avoid interpolating them directly into run: scripts, and apply the mitigations below. [1][2][3][7]

What the risk looks like

  • Attackers can create PRs or branches whose names or PR titles contain shell metacharacters or Actions expressions; if your workflow directly inserts those values into a run: block or unescapes outputs, the runner may execute them. Examples and real advisories/CVEs exist (expression/script injection and a branch-names action command-injection advisory/CVE). [1][2][3][6]

Minimal, effective mitigations (apply all that make sense)

  1. Never inject untrusted values directly into run: scripts. Use environment variables (env:) or pass values safely so they are treated as literals, not shell code. [1][5]
  2. Prefer immutable refs (head.sha) instead of branch names (head.ref) when checking out code. Avoid git checkout of a mutable branch name coming from a PR. [3][7]
  3. Sanitize/escape before writing to GITHUB_OUTPUT or files; avoid eval or patterns that unescape input (e.g., do not use eval printf "%s"). Use safe printf "%s" or proper quoting. (See patched guidance for branch-names action.) [1][2]
  4. For workflow_run workflows: restrict branch filters, verify origin (e.g., require github.event.workflow_run.event != 'pull_request' or check workflow_run.conclusion/source), and treat all artifacts as untrusted (download to $RUNNER_TEMP, validate contents). Limit permissions. [4]
  5. Don’t run privileged workflows (with write/contents/admin permissions or GITHUB_TOKEN with repo write) on events that can be triggered/poisoned by untrusted PRs (pull_request, workflow_run triggered by PRs). Require manual approval or use protected branch policies where appropriate. [3][4][7]
  6. Pin third‑party actions and inspect their code for unsafe shell usage (look for eval, unescaped printf, or writing unsanitized outputs). Update or replace vulnerable actions. [2][4]

References (examples and guidance)

  • GitHub Docs: Script injections, list of risky github context fields (body, head_ref, ref, title, etc.) and mitigation guidance. [1]
  • GitHub Security Lab advisories (expression injection examples in workflows). [3][2]
  • GitHub advisory / CVE for tj-actions/branch-names (command injection via unsafe unescaping; shows real PoC and fix). [6]
  • GitHub Well‑Architected / Actions security recommendations (workflow_run hardening, branch filters, treat artifacts as untrusted). [4]
  • Industry writeups on Actions script/code injection and safe patterns (examples of env usage vs direct interpolation). [5]

If you want, paste the specific workflow YAML you’re concerned about and I’ll point to exact lines that are vulnerable and show a fixed version.

🌐 Web query:

GitHub Actions environment context variables security trusted untrusted

💡 Result:

Quick summary — how GitHub Actions treats environment/context variables and what to do for security (trusted vs untrusted code):

Key rules

  • Treat any data originating from external actors (PRs from forks, issue/event fields, user inputs) as untrusted — don’t pass it directly into shell commands, eval, or programmatic templates without sanitizing. [1][2]
  • Never expose repository/organization secrets or long-lived tokens to untrusted code (fork PRs or any code you didn’t review). Use workflows that run PR code with no secrets. Use pull_request_target only for metadata tasks — it runs with base-repo permissions and can access secrets, so it’s dangerous if it checks out or runs untrusted code. [2][3]
  • Limit secrets/tokens scope and job permissions (permissions:, protected environments, branch protections) so even a compromised runner has least privilege. [3][4]
  • Don’t create environment variables from untrusted contexts (github.event.*, issue titles, PR body) and then export/use them in steps that run external code; these can be printed, injected, or used to exfiltrate secrets. [1][2][4]
  • Pin third‑party actions to full commit SHAs (not @main/@latest) and audit action code that will run in your workflow. [3]

Practical controls

  • Use secrets for sensitive values (secrets.*); GitHub masks secrets in logs but avoid accidental logging by not echoing untrusted-derived env values. [5]
  • Use separate jobs: one (pull_request_target or workflow_dispatch) that can access secrets for controlled orchestration, and another (pull_request) that checks out and runs untrusted PR code with no secrets. Gate secret-using jobs behind manual approvals or protected environments. [3]
  • Sanitize/whitelist inputs from events before using them in expressions or runs; prefer safe expression-time checks (if:) rather than shell interpolation when possible. [1][2]
  • Avoid writing secrets into files or envs accessible to later steps run by untrusted code; use ephemeral scoped tokens and protect deployment steps. [2][4]

References

  • Contexts and warnings about untrusted input (github, env, etc.). [1]
  • GitHub Security Lab: untrusted input risks and examples (exfiltration via env/echo). [2]
  • Community & GitHub guidance on safe use of pull_request_target, gating, least privilege, and workflow patterns. [3]
  • Variables/Environment docs and default-variable behavior. [4]
  • Secret masking and logging guidance. [5]

If you want, I can produce a short example workflow pattern that separates trusted and untrusted jobs and demonstrates the safe handling above.

🏁 Script executed:

# Check if there are other similar patterns in the workflow file that might indicate
# the project's current security practices
cd .github/workflows && cat ci-tests-e2e.yaml | grep -n "run:" | head -20

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 681

Pass untrusted branch names safely to avoid shell injection.

github.head_ref and workflow_run.head_branch are user-controlled and can contain shell metacharacters. Even within double quotes, they can break out or inject code. Use an environment variable with printf %s to treat the value as a literal string:

Safer output write 
      - name: Get branch name
        id: get-branch
-        run: echo "branch=${{ github.head_ref || github.event.workflow_run.head_branch }}" >> $GITHUB_OUTPUT
+        env:
+          BRANCH: ${{ github.head_ref || github.event.workflow_run.head_branch }}
+        run: |
+          printf 'branch=%s\n' "$BRANCH" >> "$GITHUB_OUTPUT"
🧰 Tools
🪛 actionlint (1.7.10)

232-232: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details

(expression)

🤖 Prompt for AI Agents 
In @.github/workflows/ci-tests-e2e.yaml around lines 190 - 233, The get-branch
step currently writes untrusted inputs (github.head_ref /
github.event.workflow_run.head_branch) to $GITHUB_OUTPUT using echo, which can
be subject to shell injection; update the get-branch step to capture the branch
into a safe environment variable and write it with a literal-safe formatter (use
printf '%s' or equivalent) instead of echo so the value is treated as data not
shell code; specifically, change the run in the get-branch step that sets branch
to use printf '%s' with the expression (github.head_ref ||
github.event.workflow_run.head_branch) to append the literal branch string to
$GITHUB_OUTPUT.

# Post starting comment for non-forked PRs
comment-on-pr-start:
needs: get-pr-info
runs-on: ubuntu-latest
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
permissions:
pull-requests: write
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}

- name: Get start time
id: start-time
Expand All @@ -192,25 +232,30 @@ jobs:
- name: Post starting comment
env:
GITHUB_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ needs.get-pr-info.outputs.pr_number }}
BRANCH: ${{ needs.get-pr-info.outputs.branch }}
START_TIME: ${{ steps.start-time.outputs.time }}
run: |
chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh
./scripts/cicd/pr-playwright-deploy-and-comment.sh \
"${{ github.event.pull_request.number }}" \
"${{ github.head_ref }}" \
"$PR_NUMBER" \
"$BRANCH" \
"starting" \
"${{ steps.start-time.outputs.time }}"
"$START_TIME"

# Deploy and comment for non-forked PRs only
deploy-and-comment:
needs: [playwright-tests, merge-reports]
needs: [playwright-tests, merge-reports, get-pr-info]
runs-on: ubuntu-latest
if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
if: always()
Comment on lines +248 to +250
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/ci-tests-e2e.yaml | head -300 | tail -100

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 4286

🏁 Script executed:

rg -n "deploy-and-comment|get-pr-info" .github/workflows/ci-tests-e2e.yaml -A 20

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 3103

Guard deploy-and-comment job when PR info is missing (forks or non‑PR workflow_run).

The if: always() condition allows this job to run even when get-pr-info is skipped (due to fork or non-PR workflow_run events) or when it returns empty outputs, causing the job to fail with an invalid PR number passed to the deployment script.

✅ Gate on a valid PR number 
  deploy-and-comment:
-    if: always()
+    if: ${{ always() && needs.get-pr-info.outputs.pr_number != '' }}
🤖 Prompt for AI Agents 
In @.github/workflows/ci-tests-e2e.yaml around lines 266 - 268, The
deploy-and-comment job currently uses if: always() which lets it run even when
get-pr-info was skipped or returned no PR number; change the job's if condition
to require a successful get-pr-info and a non-empty PR number, e.g. replace if:
always() with a guard like if: needs.get-pr-info.result == 'success' &&
needs.get-pr-info.outputs.pr_number != '' so the job (referenced as
deploy-and-comment and the get-pr-info job) only runs when a valid PR number is
present.

permissions:
pull-requests: write
contents: read
steps:
- name: Checkout repository
uses: actions/checkout@v5
with:
ref: ${{ github.event.workflow_run.head_branch || github.ref }}

- name: Download all playwright reports
uses: actions/download-artifact@v4
Expand All @@ -223,10 +268,12 @@ jobs:
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
GITHUB_TOKEN: ${{ github.token }}
GITHUB_SHA: ${{ github.event.pull_request.head.sha }}
GITHUB_SHA: ${{ github.event.pull_request.head.sha || github.event.workflow_run.head_sha }}
PR_NUMBER: ${{ needs.get-pr-info.outputs.pr_number }}
BRANCH: ${{ needs.get-pr-info.outputs.branch }}
run: |
bash ./scripts/cicd/pr-playwright-deploy-and-comment.sh \
"${{ github.event.pull_request.number }}" \
"${{ github.head_ref }}" \
"$PR_NUMBER" \
"$BRANCH" \
"completed"
#### END Deployment and commenting (non-forked PRs only)