FIX: forked-pr-coverage.yml env-injection via untrusted artifact - #714
Merged
Conversation
Validate untrusted coverage artifacts before posting comments, bind comments to the triggering pull request, remove privileged environment propagation, and pin actions to immutable revisions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Sumit Sarabhai (sumitmsft)
August 11, 2026 11:17
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the forked PR coverage-comment pipeline by treating coverage artifacts as untrusted input, validating/normalizing the data used to generate PR comments, and reducing the privileged workflow’s exposure to artifact-driven environment injection.
Changes:
- Introduces a Python validator/comment builder (
prepare_fork_coverage_comment.py) and a security-focused regression test suite. - Refactors
forked-pr-coverage.ymlto validate artifacts + resolve the target PR from the triggering workflow context (not artifact-supplied data), and post/update a single sticky comment. - Tightens the producer workflow (
pr-code-coverage.yml) by removing artifact-supplied PR targeting fields, hardening multilineGITHUB_ENVusage, and pinning actions to SHAs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_fork_coverage_security.py |
Adds regression tests for artifact schema validation, URL/markup injection, PR resolution, and workflow env-injection checks. |
.github/workflows/pr-code-coverage.yml |
Pins actions, disables persisted credentials, hardens multiline env export, and removes untrusted fields from the uploaded artifact payload. |
.github/workflows/forked-pr-coverage.yml |
Moves privileged workflow to validate downloaded artifacts + resolve PR from the event/commit association before commenting. |
.github/scripts/prepare_fork_coverage_comment.py |
Implements schema/value validation and safe comment construction for fork coverage artifacts. |
.github/actions/post-coverage-comment/action.yml |
Pins the sticky-comment action to an immutable SHA. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.5%
mssql_python.__init__.py: 77.6%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 84.3%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
Enforce the one-file artifact schema without recursively traversing attacker-controlled directory trees. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Gaurav Sharma (bewithgaurav)
requested changes
Aug 13, 2026
Gaurav Sharma (bewithgaurav)
left a comment
Gaurav Sharma (bewithgaurav)
left a comment
Collaborator
There was a problem hiding this comment.
requesting changes - some issues wrt forked pr comments and refactoring suggestions to cut the PR short
gargsaumya
requested changes
Aug 13, 2026
- forked-pr-coverage.yml: query /pulls?state=open (paginated) instead of
/commits/{sha}/pulls, which never returns a fork's head commit, so fork
PRs never resolved and no coverage comment was ever posted.
- prepare_fork_coverage_comment.py: remove the workflow_run.pull_requests
fast-path; GitHub always sends it empty for fork runs, so it was dead
code that only the happy-path test exercised.
- tests: point the happy-path test at the real head-SHA resolution and add
a regression that an attacker-supplied event pull_requests entry is ignored.
- Revert action SHA pins (checkout, upload-artifact, sticky-comment) to tags
to avoid colliding with #716, which owns SHA pinning.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- prepare_fork_coverage_comment.py: require the six expected fields to be present rather than an exact set, so adding a field to the producer no longer silently disables fork coverage comments. All six values are still strictly validated and only those six are propagated. - tests: assert an artifact-supplied pr_number is tolerated but never propagated, and that a missing required field is still rejected. - forked-pr-coverage.yml: drop the RUN_ID, HEAD_SHA and PR_NUMBER shell guards. RUN_ID/HEAD_SHA come straight from the trusted workflow_run event (HEAD_SHA is re-validated in the script), and PR_NUMBER only re-checks the script's own validated output. HEAD_SHA env removed as it is now unused. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Gaurav Sharma (bewithgaurav)
requested changes
Aug 14, 2026
Gaurav Sharma (bewithgaurav)
left a comment
Gaurav Sharma (bewithgaurav)
left a comment
Collaborator
There was a problem hiding this comment.
requesting changes - for a different blocker this time, added a couple of suggestions as well
- forked-pr-coverage.yml: gh api rejects --slurp combined with --jq
("the --slurp option is not supported with --jq or --template"), which
failed the whole step. Pipe --slurp output to jq instead, for both the
open-PR fetch and the coverage-comment lookup. Verified against gh 2.96
and a real fork PR end to end.
- prepare_fork_coverage_comment.py: also match the triggering fork's head
repository (from the trusted workflow_run event), so two forks sitting on
the same commit resolve to the correct PR instead of colliding and
dropping the comment. Reject events missing the head repository.
- tests: add head-repo disambiguation and missing-head-repository cases;
factor pull fixtures into a shared helper.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Gaurav Sharma (bewithgaurav)
approved these changes
Aug 19, 2026
gargsaumya
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
AB#46466
Summary
Validation