fix(security): complete dependency confusion fix — all pip install agent-os replaced#328
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add EU AI Act, Colorado AI Act, and GPAI obligations timeline with AGT coverage mapping. Reference Microsoft Purview DSPM for AI as complementary data governance layer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Scorecard API rejects workflows with write permissions at the workflow level. id-token: write and security-events: write must be scoped to the job level only. Restores permissions: read-all at workflow level while keeping job-level write permissions intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ft#324) Add Google-style docstrings with Args, Returns, Raises, Attributes, and Example sections to MCPMessageType, MCPAdapter, and MCPServer classes. Also enhances docstrings for key methods including handle_message, _handle_tools_call, _handle_resources_read, and _map_tool_to_action. Fixes microsoft#316
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
…s (dependency confusion) (microsoft#325) - Replace !pip install agent-os with !pip install -e ../.. in all 6 notebooks; agent-os is not on PyPI and installing it from PyPI is a dependency confusion vector - Replace zendesk-sdk/freshdesk-sdk with zenpy/freshdesk (the real published SDKs) in customer-service/requirements.txt - Remove hashlib-compat from healthcare-hipaa/requirements.txt; hashlib is stdlib and hashlib-compat is not a real PyPI package
…stall agent-os with agent-os-kernel Replace all remaining instances of `pip install agent-os` (unregistered on PyPI) with `pip install agent-os-kernel` (the actual package) across docs, examples, TypeScript extensions, CLI source, tests, and SVG assets. Also fixes `pip install emk` references to point to `agent-os-kernel[full]` since emk is a submodule, not a standalone PyPI package. Completes the fix started in PR microsoft#325 which only covered notebooks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🤖 AI Agent: breaking-change-detector🔍 API Compatibility ReportSummaryThis pull request primarily replaces references to the Findings
Migration GuideNo migration steps are necessary as no breaking changes were introduced. Users should update their dependencies to use Additional Notes
✅ No breaking changes detected. |
🤖 AI Agent: docs-sync-checker📝 Documentation Sync ReportIssues Found
Suggestions
Additional Notes
Action Items
Let me know if you need further assistance! |
🤖 AI Agent: test-generator🧪 Test Coverage Analysis
|
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This pull request addresses a critical security issue by replacing all instances of pip install agent-os with pip install agent-os-kernel, ensuring that the project no longer references a potentially malicious or unregistered package on PyPI. The PR also includes updates to documentation, examples, and CI/CD workflows to reflect this change. Additionally, there are some updates to the MCPAdapter class, including improved docstrings and governance checks.
🔴 CRITICAL
-
Dependency Confusion Risk Mitigation
- The replacement of
pip install agent-oswithpip install agent-os-kernelis a critical fix to mitigate dependency confusion attacks. However, ensure that theagent-os-kernelpackage is properly registered and secured on PyPI or a private package index. If it is hosted on a private index, ensure that the index is secure and access is restricted to authorized users only.
- The replacement of
-
Governance Checks in MCPAdapter
- The
MCPAdapterclass has been updated with improved docstrings and governance checks. However, thehandle_messagemethod does not seem to sanitize or validate themessageinput thoroughly before processing. This could lead to potential injection or deserialization vulnerabilities. Add validation for themessagestructure and its fields to ensure they conform to the expected JSON-RPC 2.0 format.
- The
🟡 WARNING
-
Breaking Change
- Replacing
pip install agent-oswithpip install agent-os-kernelis a breaking change for users who rely on the previous package name. While this change is necessary for security, it should be clearly communicated in the release notes and documentation. Consider adding a migration guide for users to transition to the new package name.
- Replacing
-
Backward Compatibility
- If
agent-os-kernelintroduces any API changes compared toagent-os, ensure that these changes are documented and that the new package is backward-compatible with the previous one. If not, provide clear guidance on how users can adapt their code.
- If
💡 SUGGESTIONS
-
Documentation Updates
- The documentation updates are comprehensive, but consider adding a section explicitly explaining why the package name was changed (e.g., to mitigate dependency confusion risks). This will help users understand the rationale behind the change and build trust in the new package.
-
Testing
- While the PR mentions that 75 CLI tests pass, ensure that there is adequate test coverage for the
agent-os-kernelpackage, especially for critical governance and security features. Consider adding tests to validate the new governance checks in theMCPAdapterclass.
- While the PR mentions that 75 CLI tests pass, ensure that there is adequate test coverage for the
-
CI/CD Enhancements
- The addition of the
link-check.ymlworkflow is a good step toward improving documentation quality. However, consider adding a badge to theREADME.mdto display the status of the link-check workflow.
- The addition of the
-
Code Comments
- The updated docstrings in the
MCPAdapterclass are detailed and helpful. However, consider adding inline comments for complex logic within methods like_handle_tools_calland_handle_resources_readto improve code readability.
- The updated docstrings in the
-
Error Handling
- The
handle_messagemethod raises aPermissionErrorfor governance violations, which is then converted to a JSON-RPC error response. While this is acceptable, consider creating a custom exception class (e.g.,GovernanceError) to make the intent clearer and to allow for more granular error handling in the future.
- The
-
Performance Testing
- Since the MCPAdapter class is central to the governance functionality, consider conducting performance tests to ensure that the additional governance checks do not introduce significant latency, especially under high concurrency.
-
OWASP Agentic Top 10 Compliance
- The PR does not explicitly address OWASP Agentic Top 10 compliance. Ensure that the changes align with the relevant OWASP recommendations, particularly around dependency management, input validation, and secure defaults.
Final Recommendation
- Approve with Changes: The PR addresses a critical security issue and makes necessary updates to the codebase. However, the changes introduce a breaking change and require additional validation and documentation updates. Address the critical and warning issues before merging.
🤖 AI Agent: security-scannerSecurity Analysis of Pull RequestThis pull request addresses a critical issue related to dependency confusion by replacing all instances of 1. Dependency Confusion Mitigation
2. Supply Chain Risks in Dependencies
3. Policy Engine Circumvention
4. Credential Exposure
5. Prompt Injection Defense Bypass
6. Trust Chain Weaknesses
7. Sandbox Escape
8. Deserialization Attacks
9. Race Conditions
Summary of Findings
Final RecommendationThe PR addresses a critical dependency confusion vulnerability, which is a significant improvement. However, there are several areas in the codebase that require further review and testing to ensure robust security, particularly around governance checks, logging, and input validation. These issues should be addressed in subsequent PRs to strengthen the security posture of the library. |
Completes the fix from PR #325. Replaces all 25+ instances of bare \pip install agent-os\ (unregistered on PyPI) with \pip install agent-os-kernel\ across 23 files. 75 CLI tests pass.