Skip to content

Onboard new backport-pr re-usable github workflow (OpenSearch) - #22310

Merged
peterzhuamazon merged 1 commit into
opensearch-project:mainfrom
peterzhuamazon:update-backport-workflow
Jun 25, 2026
Merged

Onboard new backport-pr re-usable github workflow (OpenSearch)#22310
peterzhuamazon merged 1 commit into
opensearch-project:mainfrom
peterzhuamazon:update-backport-workflow

Conversation

@peterzhuamazon

Copy link
Copy Markdown
Member

Description

Onboard new backport-pr re-usable github workflow (OpenSearch)

Issues Resolved

opensearch-project/opensearch-build#6270

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit b13eca6.

PathLineSeverityDescription
.github/workflows/backport.yml12highExternal reusable workflow referenced at an unpinned mutable branch (`@main`) rather than a pinned commit SHA. This is a supply chain risk: any change to `opensearch-project/opensearch-build/.github/workflows/backport-pr.yml` on `main` will be automatically picked up without review, allowing a compromised or malicious commit in that repo to execute arbitrary code in this workflow's context.
.github/workflows/backport.yml8highThe original workflow explicitly guarded against `pull_request_target` abuse by requiring `github.event.pull_request.merged == true` (with a comment citing GitHub's security documentation). This guard has been removed. The `pull_request_target` event runs with write repository permissions even for PRs from forks, making it a well-known vector for privilege escalation (PWN requests). The replacement condition only checks repository name, not whether the PR was actually merged.
.github/workflows/backport.yml13medium`OPENSEARCH_CI_BOT_TOKEN` is forwarded as a secret to the unpinned external workflow (`@main`). If the external workflow is modified maliciously, the token is exposed to attacker-controlled code. The risk is compounded by the unpinned reference flagged above.

The table above displays the top 10 most important findings.

Total: 3 | Critical: 0 | High: 2 | Medium: 1 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@peterzhuamazon peterzhuamazon moved this from Backlog to In review in OpenSearch Engineering Effectiveness Jun 24, 2026
@peterzhuamazon peterzhuamazon added the skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. label Jun 24, 2026
@peterzhuamazon

Copy link
Copy Markdown
Member Author

Expected update, re-usable workflow already have guard.

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 8fe0084)

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Restrict backport to merged PRs only

The reusable workflow is invoked without filtering on the merged/labeled conditions
previously enforced, so it will run on every closed or labeled event regardless of
whether the PR was merged or carries a backport label. Re-add an if guard that also
checks github.event.pull_request.merged and the backport label to avoid
unnecessary/insecure runs on unmerged PRs.

.github/workflows/backport.yml [9]

 jobs:
   backport:
-    if: github.repository == 'opensearch-project/OpenSearch'
+    if: >
+      github.repository == 'opensearch-project/OpenSearch'
+      && github.event.pull_request.merged
+      && (
+        github.event.action == 'closed'
+        || (github.event.action == 'labeled' && contains(github.event.label.name, 'backport'))
+      )
     uses: opensearch-project/opensearch-build/.github/workflows/backport-pr.yml@main
     secrets:
       OPENSEARCH_CI_BOT_TOKEN: ${{ secrets.OPENSEARCH_CI_BOT_TOKEN }}
Suggestion importance[1-10]: 5

__

Why: The suggestion raises a valid concern that the previous if guard checking for merged PRs was removed, but the reusable workflow at backport-pr.yml@main likely contains its own guards. Without verifying the reusable workflow's contents, the recommendation may be redundant.

Low
Pin reusable workflow to commit SHA

Pinning a reusable workflow to a mutable @main ref is a supply-chain risk because
upstream changes execute immediately with this repo's secrets. Pin the reusable
workflow to a specific commit SHA (and document the version) to ensure reproducible
and auditable behavior.

.github/workflows/backport.yml [10]

-uses: opensearch-project/opensearch-build/.github/workflows/backport-pr.yml@main
+uses: opensearch-project/opensearch-build/.github/workflows/backport-pr.yml@<full-commit-sha>
Suggestion importance[1-10]: 4

__

Why: Pinning to a SHA is a reasonable security practice, but pinning to @main for an organization-internal reusable workflow is a common and accepted pattern in this project context.

Low

@peterzhuamazon peterzhuamazon added the backport 3.7 backport to 3.7 branch label Jun 24, 2026
Comment thread .github/workflows/backport.yml Outdated
- Replace old backport workflow (VachaShah/backport + GitHub App) with reusable workflow
- Remove delete_backport_branch.yml (now handled by reusable workflow)

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
@peterzhuamazon
peterzhuamazon force-pushed the update-backport-workflow branch from b13eca6 to 8fe0084 Compare June 25, 2026 00:03
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 8fe0084

@reta reta left a comment

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.

Thanks @peterzhuamazon !

@peterzhuamazon
peterzhuamazon merged commit a6387ac into opensearch-project:main Jun 25, 2026
13 checks passed
@github-project-automation github-project-automation Bot moved this from 👀 In Review to ✅ Done in Engineering Effectiveness Board Jun 25, 2026
@peterzhuamazon
peterzhuamazon deleted the update-backport-workflow branch June 25, 2026 17:16
@opensearch-ci-bot

Copy link
Copy Markdown
Contributor

The backport to 2.19 failed. Please backport manually. See failed workflow run: https://github.com/opensearch-project/OpenSearch/actions/runs/28187818679

KhishorekumarBS pushed a commit to KhishorekumarBS/OpenSearch that referenced this pull request Jul 3, 2026
…earch-project#22310)

- Replace old backport workflow (VachaShah/backport + GitHub App) with reusable workflow
- Remove delete_backport_branch.yml (now handled by reusable workflow)

Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19 backport 3.7 backport to 3.7 branch enhancement Enhancement or improvement to existing feature or request skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis.

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants