Skip to content

ci: improve test comment #258

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

Merged
hfudev merged 1 commit into from
Mar 18, 2026
Merged

ci: improve test comment #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/workflows/pr-test-summary.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: PR Test Summary

on:
workflow_run:
workflows:
- Test Python Package
types:
- completed
Comment on lines +3 to +8
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

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

This workflow triggers on all completed runs of “Test Build IDF Apps”, including push runs on main. Since the artifact is only uploaded for pull_request runs, the download/comment steps will fail for push-triggered runs. Add a guard (e.g. job/step if: github.event.workflow_run.event == 'pull_request') or otherwise skip when the upstream run wasn’t a PR.

Copilot uses AI. Check for mistakes.

permissions:
actions: read
checks: write
pull-requests: write

jobs:
report:
runs-on: ubuntu-latest
steps:
- name: Download test report
uses: dawidd6/action-download-artifact@v14
with:
name: test-results
workflow: ${{ github.event.workflow_run.workflow_id }}
run_id: ${{ github.event.workflow_run.id }}
Comment thread
hfudev marked this conversation as resolved.

- name: Read PR number
run: |
if [ -f pr_number.txt ]; then
echo "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV
else
echo "pr_number.txt not found, skipping comment."
exit 0
fi
Comment on lines +27 to +33

Check failure

Code scanning / CodeQL

Environment variable built from user-controlled sources Critical

Potential environment variable injection in
if [ -f pr_number.txt ]; thenecho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENVelseecho "pr_number.txt not found, skipping comment."exit 0fi
Loading
, which may be controlled by an external user (
workflow_run
Loading
).

Copilot Autofix

AI about 1 month ago

In general, the fix is to prevent arbitrary content from pr_number.txt from being written directly into the GITHUB_ENV file. We should (1) strip newlines so the variable assignment cannot be split across multiple lines, and (2) optionally validate that the value is a numeric PR number before using it. This ensures an attacker cannot inject additional environment variables by crafting pr_number.txt with embedded newlines or KEY=VALUE lines.

The best minimal fix here, without changing the workflow’s overall behavior, is to read pr_number.txt into a shell variable, normalize it to a single line, validate that it looks like a PR number (digits only), and then write it to $GITHUB_ENV using the hardened echo pattern recommended by GitHub: echo "VAR=$(echo "$VAR" | tr -d '\n')" >> "$GITHUB_ENV". If validation fails, the step should fail early rather than proceeding with a malicious or malformed value.

Concretely, in .github/workflows/pr-test-summary.yml lines 27–33, replace the simple cat-and-echo sequence with a small shell script that:

  • Checks the file exists (as now).
  • Reads PR_NUMBER from the file.
  • Strips whitespace/newlines.
  • Verifies it matches ^[0-9]+$.
  • Writes PR_NUMBER=<sanitized_value> to $GITHUB_ENV using double quotes around $GITHUB_ENV.

No external dependencies are needed; we can use standard POSIX tools (tr, grep/shell pattern) available in ubuntu-latest.

Suggested changeset 1
.github/workflows/pr-test-summary.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/pr-test-summary.yml b/.github/workflows/pr-test-summary.yml
--- a/.github/workflows/pr-test-summary.yml
+++ b/.github/workflows/pr-test-summary.yml
@@ -26,7 +26,13 @@
       - name: Read PR number
         run: |
           if [ -f pr_number.txt ]; then
-            echo "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV
+            PR_NUMBER=$(cat pr_number.txt | tr -d '\r\n')
+            # Ensure PR_NUMBER consists only of digits to avoid environment injection
+            if ! printf '%s\n' "$PR_NUMBER" | grep -Eq '^[0-9]+$'; then
+              echo "Invalid PR number in pr_number.txt: '$PR_NUMBER'"
+              exit 1
+            fi
+            echo "PR_NUMBER=$PR_NUMBER" >> "$GITHUB_ENV"
           else
             echo "pr_number.txt not found, skipping comment."
             exit 0
EOF
@@ -26,7 +26,13 @@
- name: Read PR number
run: |
if [ -f pr_number.txt ]; then
echo "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV
PR_NUMBER=$(cat pr_number.txt | tr -d '\r\n')
# Ensure PR_NUMBER consists only of digits to avoid environment injection
if ! printf '%s\n' "$PR_NUMBER" | grep -Eq '^[0-9]+$'; then
echo "Invalid PR number in pr_number.txt: '$PR_NUMBER'"
exit 1
fi
echo "PR_NUMBER=$PR_NUMBER" >> "$GITHUB_ENV"
else
echo "pr_number.txt not found, skipping comment."
exit 0
Copilot is powered by AI and may make mistakes. Always verify output.

- name: Comment on PR
uses: MishaKav/pytest-coverage-comment@v1
with:
pytest-coverage-path: ./pytest-coverage.txt
junitxml-path: ./pytest.xml
issue-number: ${{ env.PR_NUMBER }}
24 changes: 16 additions & 8 deletions .github/workflows/test-build-idf-apps.yml → .github/workflows/test-python-package.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Test Build IDF Apps
name: Test Python Package

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.run_id }}
Expand All @@ -7,8 +7,8 @@ concurrency:
on:
pull_request:
paths:
- 'idf_build_apps/**'
- '.github/workflows/test-build-idf-apps.yml'
- "idf_build_apps/**"
- ".github/workflows/test-build-idf-apps.yml"
push:
branches:
- main
Expand Down Expand Up @@ -43,7 +43,8 @@ jobs:
needs: build-python-packages
strategy:
matrix:
idf-branch: [ release-v5.1, release-v5.2, release-v5.3, release-v5.4, release-v5.5 ]
idf-branch:
[release-v5.2, release-v5.3, release-v5.4, release-v5.5, release-v6.0]
runs-on: ubuntu-latest
container:
image: espressif/idf:${{ matrix.idf-branch }}
Expand Down Expand Up @@ -82,9 +83,16 @@ jobs:
-p $IDF_PATH/examples/get-started/hello_world \
--size-file size_info.json
pytest --cov idf_build_apps --cov-report term-missing:skip-covered --junit-xml pytest.xml | tee pytest-coverage.txt
- name: Pytest coverage comment
- name: Save PR number
if: github.event_name == 'pull_request'
uses: MishaKav/pytest-coverage-comment@main
run: echo ${{ github.event.number }} > pr_number.txt
- name: Upload test results
uses: actions/upload-artifact@v6
if: always() && github.event_name == 'pull_request'
Comment thread
hfudev marked this conversation as resolved.
with:
pytest-coverage-path: pytest-coverage.txt
junitxml-path: pytest.xml
name: test-results
path: |
pr_number.txt
pytest-coverage.txt
pytest.xml
retention-days: 7
Loading