From ed58607ca926c1a88c6b71e2e5b58721d6c7efea Mon Sep 17 00:00:00 2001
From: Tao Chen <taochen@microsoft.com>
Date: Thu, 12 Dec 2024 13:19:46 -0800
Subject: [PATCH] Python: Secure Python test coverage workflow (#9961)

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
The Python test coverage workflow needs to post comments of coverage
results on PRs, which requires the proper permissions. By default, PRs
from forks don't have the required permissions if the workflow is
triggered by the `pull_request` event.

However, one can use the `pull_request_target` event as the trigger and
check out the code from the PR for test coverage. This approach posts
various security risks as it gives the code from the PR full access to
the repository secrets.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
It's recommended to use a two-stage approach for this kind of scenarios:
https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/

In this PR,
1. a new workflow is created named `python-test-coverage-report`. This
workflow is given the proper permissions to post the coverage report in
the PR comments by using the `workflow_run` trigger. This trigger
ensures the workflow is run on the context of the default branch.
2. the existing workflow `python-test-coverage` is updated such that it
only runs the unit tests and doesn't have the permissions to post the
coverage report in the PR comments.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone :smile:
---
 .../workflows/python-test-coverage-report.yml | 38 +++++++++++++++++++
 .github/workflows/python-test-coverage.yml    | 27 ++++---------
 2 files changed, 46 insertions(+), 19 deletions(-)
 create mode 100644 .github/workflows/python-test-coverage-report.yml

diff --git a/.github/workflows/python-test-coverage-report.yml b/.github/workflows/python-test-coverage-report.yml
new file mode 100644
index 000000000000..7f0d323bb710
--- /dev/null
+++ b/.github/workflows/python-test-coverage-report.yml
@@ -0,0 +1,38 @@
+name: Python Test Coverage Report
+
+on:
+  workflow_run:
+    workflows: ["Python Test Coverage"]
+    types:
+      - completed
+
+jobs:
+  python-test-coverage-report:
+    runs-on: ubuntu-latest
+    if: github.event.workflow_run.conclusion == 'success'
+    continue-on-error: false
+    defaults:
+      run:
+        working-directory: python
+    steps:
+      - uses: actions/checkout@v4
+      - name: Download coverage report
+        uses: actions/download-artifact@v4
+        with:
+          github-token: ${{ secrets.GH_ACTIONS_PR_WRITE }}
+          run-id: ${{ github.event.workflow_run.id }}
+          path: ./python
+          merge-multiple: true
+      - name: Display structure of downloaded files
+        run: ls
+      - name: Pytest coverage comment
+        id: coverageComment
+        uses: MishaKav/pytest-coverage-comment@main
+        with:
+          pytest-coverage-path: python/python-coverage.txt
+          title: "Python Test Coverage Report"
+          badge-title: "Python Test Coverage"
+          junitxml-title: "Python Unit Test Overview"
+          junitxml-path: python/pytest.xml
+          default-branch: "main"
+          report-only-changed-files: true
diff --git a/.github/workflows/python-test-coverage.yml b/.github/workflows/python-test-coverage.yml
index 59bdb8f4aaba..7ffc9925fb34 100644
--- a/.github/workflows/python-test-coverage.yml
+++ b/.github/workflows/python-test-coverage.yml
@@ -10,11 +10,6 @@ env:
   # Configure a constant location for the uv cache
   UV_CACHE_DIR: /tmp/.uv-cache
 
-permissions:
-  contents: write
-  checks: write
-  pull-requests: write
-
 jobs:
   python-tests-coverage:
     runs-on: ubuntu-latest
@@ -26,8 +21,6 @@ jobs:
       UV_PYTHON: "3.10"
     steps:
       - uses: actions/checkout@v4
-      - name: Setup filename variables
-        run: echo "FILE_ID=${{ github.event.number }}" >> $GITHUB_ENV
       - name: Set up uv
         uses: astral-sh/setup-uv@v4
         with:
@@ -38,16 +31,12 @@ jobs:
         run: uv sync --all-extras --dev
       - name: Test with pytest
         run: uv run --frozen pytest -q --junitxml=pytest.xml --cov=semantic_kernel --cov-report=term-missing:skip-covered ./tests/unit | tee python-coverage.txt
-      - name: Pytest coverage comment
-        id: coverageComment
-        uses: MishaKav/pytest-coverage-comment@main
-        continue-on-error: false
+      - name: Upload coverage report
+        uses: actions/upload-artifact@v4
         with:
-          pytest-coverage-path: python/python-coverage.txt
-          coverage-path-prefix: "python/"
-          title: "Python Test Coverage Report"
-          badge-title: "Python Test Coverage"
-          junitxml-title: "Python Unit Test Overview"
-          junitxml-path: python/pytest.xml
-          default-branch: "main"
-          report-only-changed-files: true
+          path: |
+            python/python-coverage.txt
+            python/pytest.xml
+          overwrite: true
+          retention-days: 1
+          if-no-files-found: error