[build] fix issue trying to guard actions run on forks by titusfortner · Pull Request #16800 · SeleniumHQ/selenium

titusfortner · 2025-12-27T17:36:29Z

User description

repository_owner is case sensitive, and is trying to match seleniumhq instead of SeleniumHQ, so it is not protecting forks from running this code

PR Type

Bug fix

Description

Replace case-sensitive repository_owner check with fork detection
Use github.event.repository.fork == false across all workflows
Prevents fork repositories from running protected CI/CD actions
Fixes guard condition in 13 GitHub Actions workflow files

Diagram Walkthrough

flowchart LR
  A["Old: repository_owner == 'seleniumhq'"] -->|Case-sensitive mismatch| B["Forks bypass protection"]
  C["New: event.repository.fork == false"] -->|Reliable detection| D["Forks properly blocked"]

File Walkthrough

Relevant files

Configuration changes

11 files

ci-rbe.yml `Replace repository_owner with fork detection check`	+2/-2
ci-renovate-rbe.yml `Replace repository_owner with fork detection check`	+4/-4
ci-rust.yml `Replace repository_owner with fork detection check`	+1/-1
ci.yml `Replace repository_owner with fork detection check`	+1/-1
label-commenter.yml `Replace repository_owner with fork detection check`	+1/-1
lock.yml `Replace repository_owner with fork detection check`	+1/-1
mirror-selenium-releases.yml `Replace repository_owner with fork detection check`	+1/-1
nightly.yml `Replace repository_owner with fork detection check`	+6/-6
pin-browsers.yml `Replace repository_owner with fork detection check`	+1/-1
stage-release.yml `Replace repository_owner with fork detection check`	+2/-2
stale.yml `Replace repository_owner with fork detection check`	+1/-1

qodo-code-review · 2025-12-27T17:36:58Z

PR Compliance Guide 🔍

(Compliance updated until commit `73a5152`)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Fork check edge-case: The new guard `github.event.repository.fork == false` may not reliably block untrusted fork-originated runs for certain triggers (e.g., `pull_request_target` or other contexts where `github.event.repository` refers to the base repo), so the workflow `on:` events should be verified to ensure this condition cannot be bypassed. Referred Code github.event.repository.fork == false && (startsWith(github.head_ref, 'renovate/') != true \|\| github.event_name == 'workflow_call') Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Secrets exposure risk: The new fork-gating logic uses `github.event.repository.fork == false`, but because this workflow uses secrets (e.g., `secrets.SELENIUM_CI_TOKEN`) the triggers should be reviewed to confirm no event type (such as `pull_request_target` or similar) could cause fork-originated code to run with base-repo secrets despite `fork == false`. Referred Code if: github.event_name != 'schedule' && github.event.repository.fork == false && (github.ref == 'refs/heads/trunk' \|\| inputs.release == 'true') steps: - name: "Checkout selenium_manager_artifacts" uses: actions/checkout@v4 with: token: ${{ secrets.SELENIUM_CI_TOKEN }} Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit a381a7f

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Broken guard expression: The new workflow `if:` condition incorrectly chains `github.event.repository.fork == false ==` `'seleniumhq'`, which will evaluate incorrectly (effectively always false) and prevents the job from running even on the intended repository. Referred Code github.event.repository.fork == false == 'seleniumhq' && (startsWith(github.head_ref, 'renovate/') != true \|\| github.event_name == 'workflow_call') Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Authorization check broken: The fork-protection/authorization gate is implemented with an invalid comparison (`github.event.repository.fork == false == 'seleniumhq'`), meaning protected release actions may be unintentionally blocked or behave unpredictably rather than reliably enforcing the intended security boundary. Referred Code if: > (github.event.pull_request.merged == true && github.event.repository.fork == false == 'seleniumhq' && startsWith(github.event.pull_request.head.ref, 'release-preparation-')) \|\| (github.event_name == 'workflow_dispatch' && github.event.inputs.version != '' && github.event.repository.fork == false == 'seleniumhq') runs-on: ubuntu-latest Learn more about managing compliance generic rules or creating your own custom rules

qodo-code-review · 2025-12-27T17:38:19Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Fix incorrect chained comparison syntax~~ Suggestion Impact: The commit removed the syntactically incorrect chained comparison (`== false == 'seleniumhq'`) and replaced it with a valid check using `&&`. However, it did not add the suggested `github.repository_owner == 'seleniumhq'` condition, so the owner check part of the suggestion was not implemented. code diff: - if: github.event.repository.fork == false == 'seleniumhq' && startsWith(github.head_ref, 'renovate/') != true + if: github.event.repository.fork == false && startsWith(github.head_ref, 'renovate/') != true uses: ./.github/workflows/bazel.yml with: name: Check format script run @@ -34,7 +34,7 @@ test: name: Test - if: github.event.repository.fork == false == 'seleniumhq' && startsWith(github.head_ref, 'renovate/') != true + if: github.event.repository.fork == false && startsWith(github.head_ref, 'renovate/') != true Fix the syntactically incorrect chained comparison `github.event.repository.fork` `== false == 'seleniumhq'` by using a logical `&&` to correctly check both the fork status and repository owner. .github/workflows/ci-rbe.yml [27] -if: github.event.repository.fork == false == 'seleniumhq' && startsWith(github.head_ref, 'renovate/') != true +if: github.event.repository.fork == false && github.repository_owner == 'seleniumhq' && startsWith(github.head_ref, 'renovate/') != true `[Suggestion processed]` Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical bug in the PR where a chained comparison will always evaluate to false, preventing the CI jobs from running. The proposed fix is correct and restores the intended logic.	High
Learned best practice	✅ ~~Make fork guard null-safe~~ Suggestion Impact: The commit modified the same `if:` expressions by removing the erroneous `== 'seleniumhq'` comparison from the fork check, but it did not implement the suggested null-safe `fork != true` nor add the separate `github.repository_owner == 'seleniumhq'` guard. code diff: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'ruby' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'ruby' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: Ruby uses: ./.github/workflows/bazel.yml strategy: @@ -67,7 +67,7 @@ SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} python: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'python' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'python' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: Python uses: ./.github/workflows/bazel.yml with: @@ -94,7 +94,7 @@ SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} java: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'java' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'java' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: Java uses: ./.github/workflows/bazel.yml with: @@ -121,7 +121,7 @@ SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} dotnet: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'dotnet' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'dotnet' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: DotNet uses: ./.github/workflows/bazel.yml with: @@ -149,7 +149,7 @@ SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} grid: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'grid' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'grid' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: Grid permissions: contents: write # for creating nightly GitHub release @@ -178,7 +178,7 @@ SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK_URL }} javascript: - if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'javascript' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') + if: (github.event.repository.fork == false) && (inputs.language == 'javascript' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') name: JavaScript For `schedule`/`workflow_dispatch`, `github.event.repository` may be absent, so comparing it to `false` can incorrectly skip jobs; use `!= true` and keep the owner check separate. .github/workflows/nightly.yml [37] -if: (github.event.repository.fork == false == 'seleniumhq') && (inputs.language == 'ruby' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') +if: (github.repository_owner == 'seleniumhq' && github.event.repository.fork != true) && (inputs.language == 'ruby' \|\| inputs.language == 'all' \|\| github.event_name == 'schedule') `[Suggestion processed]` Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Validate and sanitize external inputs/contexts before use; account for missing/null GitHub event fields across different triggers.	Low
Update

[build] fix issue trying to guard actions run on forks

a381a7f

titusfortner requested a review from diemol December 27, 2025 17:36

selenium-ci added the B-build Includes scripting, bazel and CI integrations label Dec 27, 2025

qodo-code-review bot added the Review effort 2/5 label Dec 27, 2025

this is why I shouldn't push directly to trunk

73a5152

cgoldberg approved these changes Dec 27, 2025

View reviewed changes

titusfortner merged commit 4c64df2 into trunk Dec 27, 2025
18 checks passed

titusfortner deleted the check_fork branch December 27, 2025 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[build] fix issue trying to guard actions run on forks#16800

[build] fix issue trying to guard actions run on forks#16800
titusfortner merged 2 commits intotrunkfrom
check_fork

titusfortner commented Dec 27, 2025 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Dec 27, 2025 •

edited

Loading

Uh oh!

qodo-code-review bot commented Dec 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

titusfortner commented Dec 27, 2025 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

(Compliance updated until commit 73a5152)

Previous compliance checks

Uh oh!

qodo-code-review bot commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

titusfortner commented Dec 27, 2025 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Dec 27, 2025 •

edited

Loading

(Compliance updated until commit `73a5152`)

qodo-code-review bot commented Dec 27, 2025 •

edited

Loading