Skip to content
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

Jump to bottom

plasma-infra: Handle external pull request #844

Merged
merged 7 commits into from
Nov 22, 2023
Merged

plasma-infra: Handle external pull request #844

merged 7 commits into from
Nov 22, 2023

Conversation

Yakutoc
Copy link
Collaborator

@Yakutoc Yakutoc commented Nov 13, 2023 •
edited
Loading

Release Notes

Добавлена возможность запуска внешних pull request через fork

What/why Changed

Подробное описание Handling external pull request by fork - #805

@Yakutoc Yakutoc self-assigned this Nov 13, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 13, 2023
View reviewed changes
.github/workflows/performance-test-pr.yml
Comment on lines +37 to +42
- uses: actions/checkout@v4
with:
show-progress: false
ref: refs/pull/${{github.event.pull_request.number}}/merge

Check warning

Code scanning / Semgrep

Semgrep Finding: yaml.github-actions.security.pull-request-target-code-checkout.pull-request-target-code-checkout

This GitHub Actions workflow file uses `pull_request_target` and checks out code from the incoming pull request. When using `pull_request_target`, the Action runs in the context of the target repository, which includes access to all repository secrets. Normally, this is safe because the Action only runs code from the target repository, not the incoming PR. However, by checking out the incoming PR code, you're now using the incoming code for the rest of the action. You may be inadvertently executing arbitrary code from the incoming PR with access to repository secrets, which would let an attacker steal repository secrets. This normally happens by running build scripts (e.g., `npm build` and `make`) or dependency installation scripts (e.g., `python setup.py install`). Audit your workflow file to make sure no code from the incoming PR is executed. Please see https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ for additional mitigations.
.github/workflows/theme-builder-pr.yml Fixed Show fixed Hide fixed
@Yakutoc Yakutoc force-pushed the ci_pull_request_target branch 2 times, most recently from c98bbca to d019175 Compare November 13, 2023 07:17
@Yakutoc Yakutoc marked this pull request as ready for review November 13, 2023 07:27
@Yakutoc Yakutoc requested review from TitanKuzmich and removed request for neretin-trike November 13, 2023 07:28
@Yakutoc
Copy link
Collaborator Author

Yakutoc commented Nov 13, 2023

@Yeti-or @kayman233 @TitanKuzmich

Новые конфигурации не применяются сейчас потому что надо их сперва влить в base branch.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

GITHUB_REF === PR base branch

@Yakutoc Yakutoc mentioned this pull request Nov 20, 2023
Merged
@Yakutoc Yakutoc force-pushed the ci_pull_request_target branch from d019175 to 6ec98ce Compare November 20, 2023 08:41
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-844/

@Yakutoc Yakutoc force-pushed the ci_pull_request_target branch from 6ec98ce to c4d4676 Compare November 20, 2023 09:01
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-844/

@Yakutoc Yakutoc force-pushed the ci_pull_request_target branch from c4d4676 to f3e3f11 Compare November 20, 2023 11:17
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-844/

@Yakutoc
Copy link
Collaborator Author

Yakutoc commented Nov 21, 2023

#859 пример такого как работает

Yeti-or
Yeti-or approved these changes Nov 21, 2023
View reviewed changes
.github/workflows/change-detection.yml
@@ -11,6 +11,10 @@ on:
description: 'Get scope as "[plasma-temple,plasma-core,plasma-ui]", etc'
default: false
type: boolean
ref:
type: string
description: "Manual set repo ref"
Copy link
Contributor

Choose a reason for hiding this comment

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

а что значит ручное выставление рефки на репозиторий?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Yeti-or

  scope:
    needs: [authorize]
    uses: ./.github/workflows/change-detection.yml
    with:
      exclude-dependents: true
      ref: refs/pull/${{github.event.pull_request.number}}/merge # <--- вот ручками поставили
    secrets: inherit

Copy link
Contributor

Choose a reason for hiding this comment

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

ручками: типа в конфиге опция которую надо задать?

.github/workflows/clean-pr-documentation.yml Show resolved Hide resolved
Yakutoc
Yakutoc commented Nov 22, 2023
View reviewed changes
.github/workflows/documentation-pr.yml
uses: ./.github/workflows/change-detection.yml
with:
as-enumeration: true
ref: refs/pull/${{github.event.pull_request.number}}/merge
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Yeti-or

Вот здесь ручное выставление ref раньше оно само высчитывало.

Но теперь из-за того как работает pull_request_target нам самим нужно указать, а на что сделать checkout

@Yakutoc Yakutoc merged commit 43a3a75 into dev Nov 22, 2023
4 checks passed
@Yakutoc Yakutoc deleted the ci_pull_request_target branch November 22, 2023 03:28
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

5 similar comments
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva Salute-Eva mentioned this pull request Jan 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yeti-or Yeti-or Yeti-or approved these changes

@kayman233 kayman233 kayman233 approved these changes

@TitanKuzmich TitanKuzmich TitanKuzmich approved these changes

Assignees

@Yakutoc Yakutoc

Labels
plasma-infra released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Yakutoc @Salute-Eva @Yeti-or @TitanKuzmich @kayman233