Skip to content

[build] fix release workflow#17437

Merged
titusfortner merged 1 commit into
trunkfrom
fix_release
May 11, 2026
Merged

[build] fix release workflow#17437
titusfortner merged 1 commit into
trunkfrom
fix_release

Conversation

@titusfortner
Copy link
Copy Markdown
Member

💥 What does this PR do?

  • Fix permissions
  • Doesn't even spin up a runner if one of the version files wasn't updated (doesn't affect manual kickoff)
  • Adds the missing event info to slack message

🔄 Types of changes

  • Bug fix (backwards compatible)

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix release workflow permissions and notifications

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add path filters to prevent workflow runs on unrelated PRs
• Update permissions to enable write access for releases
• Include missing job results in Slack failure notifications
• Add missing job dependencies to failure handler
Diagram
flowchart LR
  A["Release Workflow"] -->|"Add path filters"| B["Version file changes only"]
  A -->|"Update permissions"| C["contents: write, packages: write"]
  A -->|"Add job dependencies"| D["create-language-tag, github-release-draft"]
  D -->|"Include in notifications"| E["Slack failure message"]
Loading

Grey Divider

File Changes

1. .github/workflows/release.yml 🐞 Bug fix +11/-2

Fix release workflow permissions and notifications

• Add path filters to trigger workflow only on version file changes
• Update permissions from read-only to write access for contents and packages
• Add missing job dependencies to on-release-failure job
• Include create-language-tag and github-release-draft results in Slack notifications

.github/workflows/release.yml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 11, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1)

Grey Divider


Action required

1. Unescaped git diff args 📘 Rule violation ⛨ Security
Description
The task builds a shell command using unescaped base_rev/head_rev derived from task arguments,
enabling shell injection or unexpected behavior if those values contain special characters. This
violates the requirement for safe argument handling in build/scripts tooling.
Code

rake_tasks/bazel.rake[29]

+  changed_files = `git diff --name-only #{base_rev}...#{head_rev}`.split("\n").map(&:strip).reject(&:empty?)
Evidence
PR Compliance ID 12 requires safe/robust argument handling in scripts. The modified line executes a
shell command via backticks with unescaped interpolated revisions (#{base_rev}...#{head_rev}) that
originate from task arguments, violating the safe argument handling requirement.

rake_tasks/bazel.rake[15-31]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`rake_tasks/bazel.rake` executes `git diff` via backticks with interpolated `base_rev`/`head_rev` derived from rake args. This is unsafe because it invokes a shell and can be exploited or can break on unexpected characters.

## Issue Context
Compliance requires robust/safe argument handling in scripts (quote/escape inputs or avoid shell invocation).

## Fix Focus Areas
- rake_tasks/bazel.rake[15-31]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Wrong dotnet trigger path 🐞 Bug ≡ Correctness
Description
The pull_request.paths filter includes dotnet/selenium-dotnet-version.bzl, but the repo’s dotnet
version is stored in dotnet/version.bzl, so a dotnet-only release PR that updates the real version
file will not start this workflow at all. This can prevent the release pipeline from running for
valid dotnet release-preparation PRs.
Code

.github/workflows/release.yml[R6-11]

+    paths:
+      - 'java/version.bzl'
+      - 'rb/lib/selenium/webdriver/version.rb'
+      - 'py/selenium/__init__.py'
+      - 'dotnet/selenium-dotnet-version.bzl'
+      - 'javascript/selenium-webdriver/package.json'
Evidence
The workflow is configured to trigger only when specific files change, but dotnet version changes
are made to dotnet/version.bzl (which contains SE_VERSION and is the file dotnet rake tasks
edit). Because the trigger watches a different dotnet path, dotnet-only version bumps won’t match
the filter and the workflow won’t run.

.github/workflows/release.yml[3-12]
dotnet/version.bzl[1-5]
rake_tasks/dotnet.rake[3-7]
rake_tasks/dotnet.rake[93-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The release workflow trigger’s `pull_request.paths` list points at a dotnet version file path that is not the one used by the repository’s dotnet build/release tooling, so dotnet version bumps may not trigger the release workflow.

## Issue Context
Dotnet version updates are performed against `dotnet/version.bzl` (it contains `SE_VERSION` and is read/rewritten by dotnet rake tasks). The workflow should therefore include `dotnet/version.bzl` in the PR path filter.

## Fix Focus Areas
- .github/workflows/release.yml[3-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Misleading commit-range logs 🐞 Bug ◔ Observability
Description
rake_tasks/bazel.rake now computes changed files using git diff base...head (merge-base
semantics) but still prints/raises errors using base..head, making it misleading to
reproduce/debug affected-target calculations from the logs.
Code

rake_tasks/bazel.rake[29]

+  changed_files = `git diff --name-only #{base_rev}...#{head_rev}`.split("\n").map(&:strip).reject(&:empty?)
Evidence
The rake task prints a two-dot range while executing a three-dot diff, so the logged range does not
correspond to the actual changed-files computation.

rake_tasks/bazel.rake[21-31]
.github/workflows/ci.yml[41-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`affected_targets` uses `git diff --name-only BASE...HEAD`, but surrounding log/error messages still describe the range as `BASE..HEAD`. This mismatch can mislead anyone trying to reproduce the exact diff locally based on the printed range.

### Issue Context
The rake task prints the commit range and raises an error message containing the range string; both currently show `..` even though the command uses `...`.

### Fix Focus Areas
- rake_tasks/bazel.rake[21-31]
- .github/workflows/ci.yml[41-48]

### Expected fix
- Update the `puts "Commit range: ..."` line to reflect `...` (or explicitly mention merge-base semantics).
- Update the raised error message similarly.
- (Optional) Update the usage comments at the top of the rake task to clarify that explicit `a..b` input will be evaluated via `a...b` diff internally, so users aren’t surprised.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Overbroad token permissions 🐞 Bug ⛨ Security
Description
Setting workflow-level permissions to contents: write and packages: write grants write access
to jobs that don’t need it (including on-release-failure, which runs third-party actions),
increasing the impact of any compromised action or accidental credential use. This is unnecessary
because write permissions can be scoped to only the jobs that actually publish or push changes.
Code

.github/workflows/release.yml[R21-23]

permissions:
-  contents: read
+  contents: write
+  packages: write
Evidence
The workflow now defaults GITHUB_TOKEN to contents/packages write. The on-release-failure job
does not override permissions and runs actions/checkout@v4 and rtCamp/action-slack-notify@v2, so
it inherits these write permissions despite only needing to send Slack notifications.

.github/workflows/release.yml[18-24]
.github/workflows/release.yml[293-320]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Workflow-wide `GITHUB_TOKEN` permissions are set to write for contents and packages, which implicitly grants broad write access to jobs that don’t require it (notably the failure-notification job that uses third-party actions).

## Issue Context
Many jobs already declare their own `permissions:` (or can), so the workflow-level permission can be reduced (or removed) and only the publishing/pushing jobs should receive write permissions.

## Fix Focus Areas
- .github/workflows/release.yml[18-24]
- .github/workflows/release.yml[293-320]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 0887467

Results up to commit b4ecc3c


🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)


Action required
1. Wrong dotnet trigger path 🐞 Bug ≡ Correctness
Description
The pull_request.paths filter includes dotnet/selenium-dotnet-version.bzl, but the repo’s dotnet
version is stored in dotnet/version.bzl, so a dotnet-only release PR that updates the real version
file will not start this workflow at all. This can prevent the release pipeline from running for
valid dotnet release-preparation PRs.
Code

.github/workflows/release.yml[R6-11]

+    paths:
+      - 'java/version.bzl'
+      - 'rb/lib/selenium/webdriver/version.rb'
+      - 'py/selenium/__init__.py'
+      - 'dotnet/selenium-dotnet-version.bzl'
+      - 'javascript/selenium-webdriver/package.json'
Evidence
The workflow is configured to trigger only when specific files change, but dotnet version changes
are made to dotnet/version.bzl (which contains SE_VERSION and is the file dotnet rake tasks
edit). Because the trigger watches a different dotnet path, dotnet-only version bumps won’t match
the filter and the workflow won’t run.

.github/workflows/release.yml[3-12]
dotnet/version.bzl[1-5]
rake_tasks/dotnet.rake[3-7]
rake_tasks/dotnet.rake[93-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The release workflow trigger’s `pull_request.paths` list points at a dotnet version file path that is not the one used by the repository’s dotnet build/release tooling, so dotnet version bumps may not trigger the release workflow.

## Issue Context
Dotnet version updates are performed against `dotnet/version.bzl` (it contains `SE_VERSION` and is read/rewritten by dotnet rake tasks). The workflow should therefore include `dotnet/version.bzl` in the PR path filter.

## Fix Focus Areas
- .github/workflows/release.yml[3-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Overbroad token permissions 🐞 Bug ⛨ Security
Description
Setting workflow-level permissions to contents: write and packages: write grants write access
to jobs that don’t need it (including on-release-failure, which runs third-party actions),
increasing the impact of any compromised action or accidental credential use. This is unnecessary
because write permissions can be scoped to only the jobs that actually publish or push changes.
Code

.github/workflows/release.yml[R21-23]

permissions:
-  contents: read
+  contents: write
+  packages: write
Evidence
The workflow now defaults GITHUB_TOKEN to contents/packages write. The on-release-failure job
does not override permissions and runs actions/checkout@v4 and rtCamp/action-slack-notify@v2, so
it inherits these write permissions despite only needing to send Slack notifications.

.github/workflows/release.yml[18-24]
.github/workflows/release.yml[293-320]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Workflow-wide `GITHUB_TOKEN` permissions are set to write for contents and packages, which implicitly grants broad write access to jobs that don’t require it (notably the failure-notification job that uses third-party actions).

## Issue Context
Many jobs already declare their own `permissions:` (or can), so the workflow-level permission can be reduced (or removed) and only the publishing/pushing jobs should receive write permissions.

## Fix Focus Areas
- .github/workflows/release.yml[18-24]
- .github/workflows/release.yml[293-320]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 871676e


🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)


Action required
1. Unescaped git diff args 📘 Rule violation ⛨ Security
Description
The task builds a shell command using unescaped base_rev/head_rev derived from task arguments,
enabling shell injection or unexpected behavior if those values contain special characters. This
violates the requirement for safe argument handling in build/scripts tooling.
Code

rake_tasks/bazel.rake[29]

+  changed_files = `git diff --name-only #{base_rev}...#{head_rev}`.split("\n").map(&:strip).reject(&:empty?)
Evidence
PR Compliance ID 12 requires safe/robust argument handling in scripts. The modified line executes a
shell command via backticks with unescaped interpolated revisions (#{base_rev}...#{head_rev}) that
originate from task arguments, violating the safe argument handling requirement.

rake_tasks/bazel.rake[15-31]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`rake_tasks/bazel.rake` executes `git diff` via backticks with interpolated `base_rev`/`head_rev` derived from rake args. This is unsafe because it invokes a shell and can be exploited or can break on unexpected characters.

## Issue Context
Compliance requires robust/safe argument handling in scripts (quote/escape inputs or avoid shell invocation).

## Fix Focus Areas
- rake_tasks/bazel.rake[15-31]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Misleading commit-range logs 🐞 Bug ◔ Observability
Description
rake_tasks/bazel.rake now computes changed files using git diff base...head (merge-base
semantics) but still prints/raises errors using base..head, making it misleading to
reproduce/debug affected-target calculations from the logs.
Code

rake_tasks/bazel.rake[29]

+  changed_files = `git diff --name-only #{base_rev}...#{head_rev}`.split("\n").map(&:strip).reject(&:empty?)
Evidence
The rake task prints a two-dot range while executing a three-dot diff, so the logged range does not
correspond to the actual changed-files computation.

rake_tasks/bazel.rake[21-31]
.github/workflows/ci.yml[41-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`affected_targets` uses `git diff --name-only BASE...HEAD`, but surrounding log/error messages still describe the range as `BASE..HEAD`. This mismatch can mislead anyone trying to reproduce the exact diff locally based on the printed range.

### Issue Context
The rake task prints the commit range and raises an error message containing the range string; both currently show `..` even though the command uses `...`.

### Fix Focus Areas
- rake_tasks/bazel.rake[21-31]
- .github/workflows/ci.yml[41-48]

### Expected fix
- Update the `puts "Commit range: ..."` line to reflect `...` (or explicitly mention merge-base semantics).
- Update the raised error message similarly.
- (Optional) Update the usage comments at the top of the rake task to clarify that explicit `a..b` input will be evaluated via `a...b` diff internally, so users aren’t surprised.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label May 11, 2026
Comment thread .github/workflows/release.yml
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 11, 2026

Persistent review updated to latest commit 871676e

Comment thread rake_tasks/bazel.rake Outdated
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 11, 2026

Persistent review updated to latest commit 0887467

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the GitHub Actions release workflow to better scope when it runs, correct permissions for release-related operations, and improve failure notifications.

Changes:

  • Add a pull_request.paths filter so the workflow only triggers on merged release-preparation PRs that update version files.
  • Update workflow permissions and adjust failure-job dependencies.
  • Include additional job results in the Slack failure notification.

Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
@titusfortner titusfortner merged commit c799aff into trunk May 11, 2026
33 checks passed
@titusfortner titusfortner deleted the fix_release branch May 11, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants