From 8023a527a40d8dff50ec949bcba3f7ef6a76567f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= Date: Mon, 18 Mar 2024 13:02:11 +0100 Subject: [PATCH] fix(untrusted_co): Do not report Reusable workflows called from pull_request --- ql/src/Security/CWE-829/UntrustedCheckout.ql | 24 ++++- .../.github/workflows/changelog_from_prt.yml | 100 ++++++++++++++++++ .../workflows/changelog_required_prt.yml | 9 ++ 3 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml create mode 100644 ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml diff --git a/ql/src/Security/CWE-829/UntrustedCheckout.ql b/ql/src/Security/CWE-829/UntrustedCheckout.ql index 1be8a6ea0f5..b33c7325526 100644 --- a/ql/src/Security/CWE-829/UntrustedCheckout.ql +++ b/ql/src/Security/CWE-829/UntrustedCheckout.ql @@ -1,6 +1,6 @@ /** * @name Checkout of untrusted code in trusted context - * @description Workflows triggered on `pull_request_target` have read/write access to the base repository and access to secrets. + * @description Priveleged workflows have read/write access to the base repository and access to secrets. * By explicitly checking out and running the build script from a fork the untrusted code is running in an environment * that is able to push to the base repository and to access secrets. * @kind problem @@ -121,12 +121,26 @@ class GitCheckout extends PRHeadCheckoutStep instanceof Run { } } +predicate isSingleTriggerWorkflow(Workflow w, string trigger) { + w.getATriggerEvent() = trigger and + count(string t | w.getATriggerEvent() = t | t) = 1 +} + from Workflow w, PRHeadCheckoutStep checkout where - w.hasTriggerEvent([ - "pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review", - "workflow_run", "check_run", "check_suite", "workflow_call" - ]) and + ( + // The Workflow is triggered by an event other than `pull_request` + not isSingleTriggerWorkflow(w, "pull_request") + or + // The Workflow is only triggered by `workflow_call` and there is + // a caller workflow triggered by an event other than `pull_request` + isSingleTriggerWorkflow(w, "workflow_call") and + exists(ExternalJob call, Workflow caller | + call.getCallee() = w.getLocation().getFile().getRelativePath() and + caller = call.getWorkflow() and + not isSingleTriggerWorkflow(caller, "pull_request") + ) + ) and w.getAJob().(LocalJob).getAStep() = checkout and not exists(ControlCheck check | checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml new file mode 100644 index 00000000000..0ee850f183d --- /dev/null +++ b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml @@ -0,0 +1,100 @@ +name: changelog + +on: + workflow_call: + inputs: + create: + description: Add a log to the changelog + type: boolean + required: false + default: false + update: + description: Update the existing changelog + type: boolean + required: false + default: false + +jobs: + changelog: + runs-on: ubuntu-latest + env: + file: CHANGELOG.md + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Check ${{ env.file }} + run: | + if [[ $(git diff --name-only origin/master HEAD -- ${{ env.file }} | grep '^${{ env.file }}$' -c) -eq 0 ]]; then + echo "Expected '${{ env.file }}' to be modified" + exit 1 + fi + update: + runs-on: ubuntu-latest + needs: changelog + if: (inputs.create && failure()) || (inputs.update && success()) + continue-on-error: true + env: + file: CHANGELOG.md + next_version: next + link: '[#${{ github.event.number }}](https://github.com/fabricjs/fabric.js/pull/${{ github.event.number }})' + steps: + - uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.ref }} + - name: Update ${{ env.file }} from PR title + id: update + uses: actions/github-script@v6 + env: + log: '- ${{ github.event.pull_request.title }} ${{ env.link }}\n' + prev_log: '- ${{ github.event.changes.title.from }} ${{ env.link }}\n' + with: + result-encoding: string + script: | + const fs = require('fs'); + const file = './${{ env.file }}'; + let content = fs.readFileSync(file).toString(); + const title = '[${{ env.next_version }}]'; + const log = '${{ env.log }}'; + let exists = ${{ needs.changelog.result == 'success' }}; + + if (!content.includes(title)) { + const insertAt = content.indexOf('\n') + 1; + content = + content.slice(0, insertAt) + + `\n## ${title}\n\n\n` + + content.slice(insertAt); + } + + const insertAt = content.indexOf('\n', content.indexOf(title) + title.length + 1) + 1; + if (exists && ${{ github.event.action == 'edited' }}) { + const prevLog = '${{ env.prev_log }}'; + const index = content.indexOf(prevLog, insertAt); + if (index > -1) { + content = content.slice(0, index) + content.slice(index + prevLog.length); + exists = false; + } + } + + if (!exists) { + content = content.slice(0, insertAt) + log + content.slice(insertAt); + fs.writeFileSync(file, content); + return true; + } + + return false; + - name: Setup node + if: fromJson(steps.update.outputs.result) + uses: actions/setup-node@v3 + with: + node-version: 18.x + - name: Commit & Push + if: fromJson(steps.update.outputs.result) + run: | + npm ci + npx prettier --write ${{ env.file }} + git config user.name github-actions[bot] + git config user.email github-actions[bot]@users.noreply.github.com + git add ${{ env.file }} + git commit -m "update ${{ env.file }}" + git push diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml new file mode 100644 index 00000000000..8a3b1b02a63 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml @@ -0,0 +1,9 @@ +name: '📋' + +on: + pull_request_target: + branches: [master] + +jobs: + changelog: + uses: ./.github/workflows/changelog_from_prt.yml