fix(templates): refresh Python dependencies and container hygiene - #2496
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
Package TarballHow to installgh release download pr-2496-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.31.0.tgz |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
This is a well-scoped dependency and base-image refresh. Spot checks:
- Dockerfile template changes (
src/assets/container/python/Dockerfile): OS upgrade step runs beforeUSER bedrock_agentcore,UV_NO_CACHE=1is set before bothuv syncinvocations, and/usr/local/bin/python -m pip uninstall -y uvruns after the final sync. The path matches wherepip install --no-cache-dir uvplacesuvonpublic.ecr.aws/docker/library/python:3.12-slim-trixie. pyproject.tomlupdates line up with the newpython-dependencies.test.tsmatrix. Snapshots and dependency test both pin>= 1.28.1, < 2.0.0formcpand>= 1.18.1forbedrock-agentcore, including the correct[strands-agents]and[a2a,strands-agents]extras.- Tests use
copyAndRenderDiragainst real temp dirs rather than mocks — good. uv.lockis generated atagentcore createtime byinstallDependencies(plainuv sync), so no lockfile needs to ship in the templates;uv sync --frozenin the Dockerfile will have a fresh lock to consume.
Non-blocking observation
The hardcoded fallback Dockerfile in src/cli/commands/import/actions.ts (around lines 414–436, only used when the starter-toolkit Dockerfile is missing) was not updated alongside the template: it still omits the apt-get upgrade, UV_NO_CACHE=1, and the trailing pip uninstall -y uv. If keeping these two code paths aligned matters for imported projects, consider a follow-up to update it (or extract a shared source). Not blocking this PR.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Review findings:
These paths still install uv without UV_NO_CACHE and leave uv in the final image. The import path using the standard Python base also omits the OS package upgrade. Imported/exported container projects can therefore retain the OpenSSL, uv-cache, and uv-binary Inspector findings this patch is intended to address. Please reuse the shared Dockerfile implementation where possible, or apply equivalent hardening and add regression coverage for these paths. For arbitrary custom/containerUri bases, the generated guidance should explicitly require refreshing the base OS packages as appropriate.
|
Description
>=1.28.1,<2) in generated agent and MCP tool projects.1.18.1and enable its declared Strands integration dependency in Strands templates.Frozen dependency installation, non-root execution, Runtime ports, and direct Python startup are retained. The separate Bedrock Managed Agents image is unchanged because it uses uv at runtime.
Related Issue
Security-related dependency refresh based on already-published advisories. No public security issue was opened, following the repository's CONTRIBUTING guidance.
Public references:
Documentation PR
Not applicable. No documentation changes are included.
Type of Change
Testing
npm run test:unit -- --maxWorkers=4, 6,342 passed.npm exec -- vitest run --project integ --maxWorkers=4, 355 passed and 1 skipped.1.24.0, MCP1.30.0, and SDK-compatible Strands1.57.1.docker build --pull --no-cache --platform linux/amd64.libssl3t64 3.5.7-1~deb13u3, no uv/uvx executables, and no/root/.cache/uv./pingHealthy as UID 1000, with external networking disabled and dummy credentials.The full suites ran before the final SDK/Strands metadata follow-up; that follow-up was verified with the generated-project regressions, fresh resolution, rebuilt image, and startup check. This is local x86 container verification, not an ARM64 AWS deployment or a new Inspector scan. No model invocation was attempted in the isolated startup smoke.
npm run test:unitand the equivalent built-CLI integration suitenpm run typechecknpm run lintsrc/assets/, I updated and committed the affected snapshotsnpm run format:checkandnpm run buildChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.