Handling external pull request by fork

[Yakutoc]

Цель

Запуск pull request созданных через fork должен корректно обрабатывать все secrets и не падать с ошибкой, что GITHUB_TOKEN не найден.

TD;LR

Использование события pull_request_target без должных мер предосторожности может повлечь за собой раскрытие всех секретов неавторизованным пользователям GitHub.

DOCs pull_request

GITHUB_SHA

Last merge commit on the GITHUB_REF branch

GITHUB_REF

PR merge branch refs/pull/:prNumber/merge

DOCs pull_request_target

GITHUB_SHA

Last commit on the PR base branch

GITHUB_REF

PR base branch

Из-за этой разницы в том на что/куда смотрит target события, в шаге с checkout нужно будет явно указать ref

- name: Checkout
  uses: actions/checkout@v4
  with:
    show-progress: false
    fetch-depth: 0
    ref: refs/pull/${{github.event.pull_request.number}}/merge

И как раз это является потенциальным вектором атаки.

В статье Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests подробно об этом рассказывается.

TL;DR: Combining pull_request_target workflow trigger with an explicit checkout of an untrusted PR is a dangerous practice that may lead to repository compromise.

Возможные решения

Изменить подход к организации наших workflow

Полностью меняем подход к тому как мы обрабатываем внешний PR's через fork.

Исключаем workflow для таких PR где так или иначе фигурируют secrets, типа github_token.

Например "Publish Canary" и deploy документации с песочницей.

Использование "Еnvironment" для получение approve

Добавляем ограничение на запуск для environment === 'external'.

Переделанная конфигурация для "Publish Canary".

name: Publish Canary

on:
  pull_request_target:
    branches:
      - dev

jobs:
  authorize:
    runs-on: ubuntu-latest

    ## github.event_name == 'pull_request_target' - защита от дурака
    ## github.event.pull_request.head.repo.full_name != github.repository - убеждаемся что PR сделан через fork
    ## Если да используем environment: external и тогда нужен approve от члена команды для запуска этого workflow
     
    environment: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != github.repository && 'external' || 'internal' }}

    steps:
      - name: Echo
        run: |
          echo "External pull request approved by code owners"

  publish:
    name: Canary version
    needs: [ authorize ]
    uses: ./.github/workflows/publish-common.yml
    with:
      ref: refs/pull/${{github.event.pull_request.number}}/merge
    secrets:
      gh_token: ${{ secrets.GH_TOKEN }}
      npm_registry_token: ${{ secrets.NPM_REGISTRY_TOKEN }}

Внешний pull request не запустит ни один workflow в котором будут подобные ограничения.

Членам команды будут отправлены уведомления о том что нужно поставить approve на запуск workflow.

И тем самым исключаем запуск в целевом репозитории untrusted code.

[Yakutoc]

Publish RC [RnD]

Для этого workflow событие pull_request_target не походит.

После закрытия PR и запуска auto shipit считает что до сих пор находится в ветке PR и запускает публикацию canary версии.

Поэтом предлагаю перейти на on: push.

В нашем случае после того как прошли обязательные проверки в merge-queue, происходит push в dev ботом.

С учетом этого и сделана новая конфигурация.

name: Publish RC

on:
  push:
    branches:
      - dev

jobs:
  publish:
    name: Publish RC version

    ## Push от друго actor не запустит этот workflow
    if: ${{ github.actor == 'github-merge-queue[bot]' }}

    uses: ./.github/workflows/publish-common.yml
    with:
      with-update-package-lock: true
      commit-message: "chore: update package-locks"
    secrets:
      gh_token: ${{ secrets.GH_TOKEN }}
      npm_registry_token: ${{ secrets.NPM_REGISTRY_TOKEN }}

NOTE: Вливаем всегда через merge-queue

[Yakutoc]

Какие workflows нужно будет адаптировать

• Publish canary
• Publish RC
• PR theme-builder
• Component Performance Testing (PR)
• PR Documentation and Storybook

Все эти workflows имеют запись

on:
  pull_request:
    branches:
      - dev
      - master

И для master нужно будет оставить как раз on: pull_request.

То есть будет

on:
  pull_request:
    branches:
      - master
  pull_request_target:
    branches:
      - dev

[Yakutoc]

Что еще можно почитать на тему работы с pull_request_target

https://iterative.ai/blog/testing-external-contributions-using-github-actions-secrets

https://glasnt.com/blog/pull_request_target_labels/

https://dev.to/petrsvihlik/using-environment-protection-rules-to-secure-secrets-when-building-external-forks-with-pullrequesttarget-hci

[Yeti-or]

пока сложно я прочитал первую статью, из неё понял что pull_request_target опасен если делать checkout в ref.

Только я так и не понял куда оно там чекаутится и почему это безопасно?

pull_request_target runs in the context of the target repository of the PR, rather than in the merge commit. This means the standard checkout action uses the target repository to prevent accidental usage of the user supplied code.

что значит context target repository.

я понял две вещи что мы можем переделать часть workflow на то что мы получаем pr через pull_request а потом делаем upload артифактов уже в другой джобе кажется это подходит для perfomance джобы

не понятно мне пока как мы можем использовать Еnvironment
нужна статья или ликбез

еще не ясно почему у нас есть джоба Publish RC которую мы хотим запускать из пулл-реквестов

про canary я понимаю нам как раз нужно использовать pull_request_target потому что давать собирать без секретов пакет а потом его автоматом паблишить в npm вооот точно не хочется.

но кажется мы тут как раз можем воспользоваться механимзмом которые они предлагают про лейблы:

Add a condition to the pull_request_target to run only if a certain label is assigned the PR

но они говорят что не безопасно ставить лейбл а потом это еще вливать все дела, тогда я предлагаю сделать следущий лайф-хак ставить label=run-ci-on-#{hashCommit} типа мы руками хотим запустить внешний пр и прописываем конкретный коммит на котором можно запустить, тогда если туда что-то докомитят то надо будет еще раз чекать и прописывать новый хэш

или я понял environment решает ту же проблемы?
как будто по твоему коду: authorize да,

Еще вопрос включается ли target автоматом ? в том плане мы свои пр тоже должны окать ? что в варианте с environment что в варианте с label ? или можем как-то чекать права во время запуска? или еще что?

[Yeti-or]

Прочитал https://iterative.ai/blog/testing-external-contributions-using-github-actions-secrets
кажется подход с environment то же самое что label, вопрос только в том нужно ли ему на каждый доп коммит запрашивать approve или нет?

[Yakutoc]

@Yeti-or @neretin-trike @kayman233 @TitanKuzmich

К разговору о том как себя ведет Github CI/CD когда изменяем файлы конфигурации -
PoligonSa/plasma-sandbox#212

В этом PR (через fork) я удалил блок с авторизацией и все же workflow запустился именно master config.

То есть он не стал применять новые настройки.