feat(ci): add PyPI build + NuGet pack jobs, complete package coverage#679
Conversation
- Add build-pypi job: python -m build for all 7 PyPI packages (matrix) - Add dotnet pack step to test-dotnet job for NuGet packaging - Add agent-runtime and agent-lightning to path filters, lint, test, and safety check matrices - Wire build-pypi into ci-complete gate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This pull request introduces PyPI and NuGet build validation to the CI pipeline, ensuring that packaging issues are caught during pull requests. It also expands the CI coverage to include the agent-runtime and agent-lightning packages, aligning the CI pipeline with the ESRP pipeline. While this is a valuable improvement to the CI/CD process, there are a few areas that require attention to ensure correctness, security, and maintainability.
🔴 CRITICAL
-
Missing Security Validation for PyPI Packages:
- The
build-pypijob builds Python packages but does not include any security validation for the generated artifacts (e.g., scanning for malicious code or verifying dependencies). - Action: Add a step to scan the generated
.whlfiles for vulnerabilities or malicious code using tools liketwine checkorbandit. This ensures that the packages are safe before publishing.
- The
-
NuGet Package Verification:
- The
test-dotnetjob verifies the existence of the.nupkgfile but does not validate its integrity or contents. - Action: Add a step to validate the NuGet package using
nuget verifyor a similar tool to ensure the package is correctly built and free of tampering.
- The
🟡 WARNING
-
Potential Breaking Changes in Python Version Matrix:
- The
testjob now includes Python 3.13 in the matrix, which is currently not officially released or stable. - Action: Consider removing Python 3.13 from the matrix until it is officially supported. Including it prematurely may cause unnecessary failures or compatibility issues.
- The
-
Backward Compatibility for New Packages:
- Adding
agent-runtimeandagent-lightningto the CI pipeline assumes that these packages are ready for production. If they are not backward-compatible with existing systems, this could lead to issues. - Action: Ensure that these new packages are thoroughly tested for backward compatibility before merging this PR.
- Adding
💡 SUGGESTION
-
Improve Artifact Management:
- The
build-pypiandtest-dotnetjobs generate artifacts (.whland.nupkgfiles) but do not upload them to the CI pipeline for further analysis or debugging. - Action: Use
actions/upload-artifactto store the generated artifacts, making them available for inspection or reuse in subsequent jobs.
- The
-
Add Dependency Locking for Build Tools:
- The
build-pypijob installsbuildandsetuptoolswithout locking their versions, which may lead to non-deterministic builds. - Action: Use a
requirements.txtfile or pin the versions of these tools to ensure consistent builds.
- The
-
Expand Test Coverage for New Packages:
- While
agent-runtimeandagent-lightningare added to the CI pipeline, there is no mention of additional test cases specific to these packages. - Action: Ensure that these packages have sufficient test coverage, especially for edge cases and security-related scenarios.
- While
-
Add Documentation for New CI Jobs:
- The new
build-pypiandtest-dotnetjobs are not documented in the repository. This could make it harder for contributors to understand the CI pipeline. - Action: Update the repository's CI documentation to include details about these new jobs and their purpose.
- The new
-
Consider Parallelizing CI Jobs:
- The CI pipeline now includes additional jobs, which may increase the overall runtime.
- Action: Review the pipeline's execution time and consider parallelizing jobs where possible to reduce the total runtime.
Final Assessment
This PR introduces valuable improvements to the CI/CD pipeline, particularly in catching packaging issues early. However, there are critical gaps in security validation and artifact verification that need to be addressed before merging. Additionally, attention should be given to backward compatibility, test coverage, and documentation to ensure a smooth integration.
- Merge Recommendation: Conditional approval, pending resolution of critical issues and consideration of warnings.
🤖 AI Agent: security-scanner — Security Review of PR: `feat(ci): add PyPI build + NuGet pack jobs, complete package coverage`Security Review of PR:
|
| Finding | Severity |
|---|---|
| Credential Exposure in Logs | 🔴 CRITICAL |
| Supply Chain Vulnerability: Hardcoded Dependency Versions | 🟠 HIGH |
| Potential for Dependency Confusion | 🟠 HIGH |
| Lack of Integrity Verification for Build Artifacts | 🟡 MEDIUM |
| Potential for Insecure Python Dependency Installation | 🟡 MEDIUM |
| No Verification of NuGet Package Integrity | 🟡 MEDIUM |
| Lack of Isolation in CI Jobs | 🔵 LOW |
Recommended Actions:
- Prevent Credential Exposure: Avoid printing sensitive information (e.g., filenames) directly to logs.
- Secure Dependencies: Use a trusted index URL and dependency lock files to mitigate supply chain risks.
- Verify Artifact Integrity: Add checksum generation and verification steps for all build artifacts.
- Enhance Dependency Installation Security: Use
--require-hasheswithpip installto verify package integrity. - Consider Self-Hosted Runners: For critical jobs, use isolated environments to reduce the risk of cross-job contamination.
By addressing these issues, the CI/CD pipeline will be more resilient to potential security threats, ensuring the integrity and safety of the microsoft/agent-governance-toolkit and its downstream consumers.
Summary
Adds PyPI and NuGet build validation to CI so packaging issues are caught on PRs:
Full CI coverage now matches ESRP pipeline: