From f4cbbe64291ddbf06c9759b9a6ee0aef309034d0 Mon Sep 17 00:00:00 2001 From: Adonis Ling Date: Wed, 8 Nov 2023 11:23:13 +0800 Subject: [PATCH] [chore](workflow) Fix security issues with pull_request_target (#26525) In the workflow Code Checks, we use the event pull_request_target which has write permission to enable the actions to comment on our PRs. We should be careful with the write permission and must forbid from running any user code. The previous PR #24761 tried its best to achieve this goal. However, there is a scenario lacking of consideration (See #26494). #26494 attacks the workflow by git submodule way. This PR fixes this scenario by checkouting the external action explicitly in the workflow. --- .github/actions/action-sh-checker | 1 - .github/actions/clang-format-lint-action | 1 - .github/actions/clang-tidy-review | 1 - .github/workflows/clang-format.yml | 20 ++++++++++++-- .github/workflows/code-checks.yml | 33 ++++++++++++++++++------ .github/workflows/license-eyes.yml | 3 --- .gitmodules | 9 ------- 7 files changed, 43 insertions(+), 25 deletions(-) delete mode 160000 .github/actions/action-sh-checker delete mode 160000 .github/actions/clang-format-lint-action delete mode 160000 .github/actions/clang-tidy-review diff --git a/.github/actions/action-sh-checker b/.github/actions/action-sh-checker deleted file mode 160000 index 76ab0b22e1..0000000000 --- a/.github/actions/action-sh-checker +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 76ab0b22e1f194e4a582edc7969df6485c4e9246 diff --git a/.github/actions/clang-format-lint-action b/.github/actions/clang-format-lint-action deleted file mode 160000 index 6adbe14579..0000000000 --- a/.github/actions/clang-format-lint-action +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 6adbe14579e5b8e19eb3e31e5ff2479f3bd302c7 diff --git a/.github/actions/clang-tidy-review b/.github/actions/clang-tidy-review deleted file mode 160000 index 2c55ef8cfc..0000000000 --- a/.github/actions/clang-tidy-review +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 2c55ef8cfc9acb3715d433e58aea086dcec9b206 diff --git a/.github/workflows/clang-format.yml b/.github/workflows/clang-format.yml index f69cb95368..c804b753a3 100644 --- a/.github/workflows/clang-format.yml +++ b/.github/workflows/clang-format.yml @@ -31,14 +31,21 @@ jobs: uses: actions/checkout@v3 with: persist-credentials: false - submodules: recursive - name: Checkout ${{ github.ref }} ( ${{ github.event.pull_request.head.sha }} ) if: ${{ github.event_name == 'pull_request_target' }} uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} - submodules: recursive + + - name: Checkout paths-filter + run: | + rm -rf ./.github/actions/paths-filter + git clone https://github.com/dorny/paths-filter .github/actions/paths-filter + + pushd .github/actions/paths-filter &>/dev/null + git checkout 4512585405083f25c027a35db413c2b3b9006d50 + popd &>/dev/null - name: Paths filter uses: ./.github/actions/paths-filter @@ -49,6 +56,15 @@ jobs: - 'be/src/**' - 'be/test/**' + - name: Checkout clang-format-lint-action + run: | + rm -rf ./.github/actions/clang-format-lint-action + git clone https://github.com/DoozyX/clang-format-lint-action .github/actions/clang-format-lint-action + + pushd .github/actions/clang-format-lint-action &>/dev/null + git checkout 6adbe14579e5b8e19eb3e31e5ff2479f3bd302c7 + popd &>/dev/null + - name: "Format it!" if: ${{ steps.filter.outputs.be_changes == 'true' }} uses: ./.github/actions/clang-format-lint-action diff --git a/.github/workflows/code-checks.yml b/.github/workflows/code-checks.yml index 652aa7f81e..9e314bfb6d 100644 --- a/.github/workflows/code-checks.yml +++ b/.github/workflows/code-checks.yml @@ -27,21 +27,22 @@ jobs: - name: Checkout ${{ github.ref }} ( ${{ github.sha }} ) if: ${{ github.event_name != 'pull_request_target' }} uses: actions/checkout@v3 - with: - submodules: recursive - name: Checkout ${{ github.ref }} ( ${{ github.event.pull_request.head.sha }} ) if: ${{ github.event_name == 'pull_request_target' }} uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} - submodules: recursive - - name: Patch + - name: Checkout action-sh-checker run: | - pushd .github/actions/action-sh-checker >/dev/null + rm -rf ./.github/actions/action-sh-checker + git clone https://github.com/luizm/action-sh-checker .github/actions/action-sh-checker + + pushd .github/actions/action-sh-checker &>/dev/null + git checkout 76ab0b22e1f194e4a582edc7969df6485c4e9246 sed -i 's/\[ "$GITHUB_EVENT_NAME" == "pull_request" \]/\[\[ "$GITHUB_EVENT_NAME" == "pull_request" || "$GITHUB_EVENT_NAME" == "pull_request_target" \]\]/' entrypoint.sh - popd >/dev/null + popd &>/dev/null - name: Run ShellCheck uses: ./.github/actions/action-sh-checker @@ -63,7 +64,15 @@ jobs: uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} - submodules: recursive + + - name: Checkout paths-filter + run: | + rm -rf ./.github/actions/paths-filter + git clone https://github.com/dorny/paths-filter .github/actions/paths-filter + + pushd .github/actions/paths-filter &>/dev/null + git checkout 4512585405083f25c027a35db413c2b3b9006d50 + popd &>/dev/null - name: Paths Filter uses: ./.github/actions/paths-filter @@ -117,7 +126,6 @@ jobs: uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} - submodules: recursive - name: Download uses: actions/download-artifact@v3 @@ -125,6 +133,15 @@ jobs: name: compile_commands path: ./be/build_Release + - name: Checkout clang-tidy review + run: | + rm -rf ./.github/actions/clang-tidy-review + git clone https://github.com/ZedThree/clang-tidy-review .github/actions/clang-tidy-review + + pushd .github/actions/clang-tidy-review &>/dev/null + git checkout 2c55ef8cfc9acb3715d433e58aea086dcec9b206 + popd &>/dev/null + - name: Run clang-tidy review uses: ./.github/actions/clang-tidy-review id: review diff --git a/.github/workflows/license-eyes.yml b/.github/workflows/license-eyes.yml index 823ac845b3..890efb2d9d 100644 --- a/.github/workflows/license-eyes.yml +++ b/.github/workflows/license-eyes.yml @@ -30,15 +30,12 @@ jobs: - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" if: ${{ github.event_name != 'pull_request_target' }} uses: actions/checkout@v3 - with: - submodules: recursive - name: Checkout ${{ github.ref }} ( ${{ github.event.pull_request.head.sha }} ) if: ${{ github.event_name == 'pull_request_target' }} uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} - submodules: recursive - name: Check License uses: apache/skywalking-eyes@v0.2.0 diff --git a/.gitmodules b/.gitmodules index 9fe51bfd1d..a4e579b179 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,9 +4,6 @@ [submodule ".github/actions/get-workflow-origin"] path = .github/actions/get-workflow-origin url = https://github.com/potiuk/get-workflow-origin.git -[submodule ".github/actions/clang-format-lint-action"] - path = .github/actions/clang-format-lint-action - url = https://github.com/DoozyX/clang-format-lint-action.git [submodule ".github/actions/setup-maven"] path = .github/actions/setup-maven url = https://github.com/stCarolas/setup-maven.git @@ -19,12 +16,6 @@ [submodule ".github/actions/ccache-action"] path = .github/actions/ccache-action url = https://github.com/hendrikmuhs/ccache-action -[submodule ".github/actions/action-sh-checker"] - path = .github/actions/action-sh-checker - url = https://github.com/luizm/action-sh-checker -[submodule ".github/actions/clang-tidy-review"] - path = .github/actions/clang-tidy-review - url = https://github.com/ZedThree/clang-tidy-review.git [submodule "be/src/apache-orc"] path = be/src/apache-orc url = https://github.com/apache/doris-thirdparty.git