fix: update vulnerable dependencies without changing SDK APIs - #4963
fix: update vulnerable dependencies without changing SDK APIs#4963jbeckwith-oai wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
seratch
left a comment
There was a problem hiding this comment.
I'd like to minimize new required deps. Also, it seems the tests are failing only with Python 3.10 but I haven't checked what's the cause yet.
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed all six changed files at 51f2cd5 against actual merge base 1705dd6, with released compatibility baseline v0.22.2. No substantive introduced issues found.
Checked manifest/lock agreement, optional-extra and standalone-example ownership, Python 3.10 compatibility of the new floors, provider requirement intersections, and changed artifact metadata against PyPI. The Cloudflare cast is runtime-neutral. The MCP mapped-error change preserves the explicit-cause/None contract while clearing implicit context after raising.
I favor retaining the python-dotenv floor while MCP 1.x remains supported: its pydantic-settings dependency still permits affected dotenv versions, and the repository lockfile does not constrain published SDK consumers. The added footprint for MCP 2-only installations is real; this approval does not resolve or override the maintainer discussion about that choice.
Projected alert closures and the deferred LiteLLM/Cryptography remediation status were not independently verified. This review is not an accepted-risk waiver or a claim of repository-wide security compliance. No tests, installations, builds, lint, CI status, coverage, or mergeability were run or assessed.
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed the complete six-file diff at 0d869d36b22cde66a4fdcc30a0ca8bd7ef001089 against actual merge base 1705dd62509ebfb3e85e26baedbe43e9f183580f, including the new delta from my previous review. No substantive introduced defects found.
The update consistently removes the mandatory python-dotenv requirement from pyproject.toml and both root lock-metadata entries. All locked package versions/artifacts, direct Twilio example floors, retained dependency minimums, and runtime changes are unchanged from the previously reviewed head. The Cloudflare cast remains runtime-neutral, and the MCP mapped-error change preserves the approved explicit-cause contract while clearing implicit context.
Seratch's requested core-dependency removal is implemented. The published security floor for consumers selecting supported MCP 1.x remains unimplemented, as the PR now explicitly documents; the patched repository lockfile does not constrain those consumers. The same residual floor gap applies to existing LiteLLM/Vercel extra paths: LiteLLM 1.83.0 metadata permits python-dotenv>=0.2.0, and Vercel 0.5.6 metadata leaves it unversioned. These gaps were already present at the merge base, so this is scoped partial remediation, not an equivalent dotenv fix or a newly introduced regression. My earlier floor recommendation is not a claim that ordinary MCP settings reads reach the advisory's file-writing operation.
Projected alert closures and the remaining remediation status were not independently verified. This approval is not an accepted-risk waiver or a claim of repository-wide security compliance. No tests, installations, builds, lint, CI status, coverage, or mergeability were run or assessed.
0d869d3 to
cad6c4f
Compare
Summary
Update vulnerable dependencies and published security minimums to address 73 of the 92 open dependency alerts. Apply the requirements to the owning core dependencies, optional extras, and standalone Twilio examples. A typing-only Cloudflare WebSocket cast accommodates the updated aiohttp annotations without changing SDK APIs or runtime behavior.
The updated async test runner exposed implicit exception-context retention on Python 3.10. Clear that context in the existing MCP mapped-error helper after raising, preserving the approved explicit cause and the existing credential-redaction contract. The five failing CI assertions pass without weakening tests.
19 alerts remain (3 critical, 10 high, 4 medium, 2 low):
openai<3, which conflicts with this SDK'sopenai>=3,<4requirement.Per Seratch’s review request,
python-dotenvis no longer a mandatory core SDK dependency. Its patched lockfile version and direct Twilio example requirements remain. The published dotenv security minimum for consumers choosing MCP 1.x is deferred.These remaining updates need compatible upstream releases or a separate compatibility decision. Alert closure is projected from the dependency changes and will be confirmed by GitHub after merge.
Compatibility evidence
Rebased onto main at
fbd2dbcaaf74a2c447c6d3fa9d5645d83fd7e292without conflicts; all three task commits retain identical patches. Seratch’s mandatory-core-dotenv removal and the Python 3.10 correction are preserved.All 23 CI checks passed on commit
cad6c4f4c7b359ac21e16cfae13ac8e3ec6049ae. Complete CI run.RunStatefixtures load and rewrite successfully, including semantic checks for historical features. Artifact compatibility log.Additional local validation:
setpgidpermission failure; its focused rerun and the full stack rerun passed in the same sandbox without code or test changes. The native macOS cases skipped locally under the Codex sandbox setting passed in CI as linked above.Requires-Python: >=3.10.These results found no regressions in the tested API, persistence, packaging, and runtime contracts. The incompatible LiteLLM/Cryptography upgrades remain deferred, and the requested
python-dotenvcore-dependency removal is implemented.Checks
.agents/skills/code-change-verification/scripts/run.sh.