Python: Harden Foundry Responses integrations and sandbox isolation - #8899
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Required Cosmos and Memory negative tests are missing, and the upload dependency and routing guidance need correction.
Review effort: Balanced
Findings: 3
Open (5)
What changed in this PR
Hardens Foundry-hosted Responses samples with request-scoped resources, trusted identity boundaries, and sandbox isolation.
Changes:
- Adds request-owned agents, clients, credentials, providers, and cleanup.
- Secures file access, Cosmos sessions, and user-scoped Memory.
- Updates telemetry defaults, dependencies, documentation, and type-checker resolution.
| File | Description |
|---|---|
python/ty.samples.toml |
Resolves sibling sample imports. |
.../responses/tools/README.md |
Documents lifecycle and shell risk. |
.../responses/tools/main.py |
Uses a request-owned tools agent. |
.../responses/observability/README.md |
Documents safe telemetry behavior. |
.../responses/observability/main.py |
Scopes telemetry clients per request. |
.../responses/observability/agent.yaml |
Disables sensitive telemetry. |
.../responses/observability/agent.manifest.yaml |
Disables sensitive telemetry. |
.../responses/monty_codeact/README.md |
Documents Monty lifecycle and execution. |
.../responses/monty_codeact/main.py |
Creates request-scoped Monty providers. |
.../responses/monty_codeact/agent.yaml |
Disables sensitive telemetry. |
.../responses/monty_codeact/agent.manifest.yaml |
Disables sensitive telemetry. |
.../responses/mcp/README.md |
Documents PAT and identity boundaries. |
.../responses/mcp/main.py |
Creates request-owned GitHub MCP tools. |
.../responses/foundry_toolbox/README.md |
Explains request-owned Toolbox sessions. |
.../responses/foundry_toolbox/main.py |
Scopes Toolbox connections per request. |
.../responses/foundry_toolbox_mcp_skills/README.md |
Documents skill-session isolation. |
.../responses/foundry_toolbox_mcp_skills/main.py |
Scopes Toolbox skills per request. |
.../responses/foundry_memory/README.md |
Documents trusted user-wide Memory. |
.../responses/foundry_memory/provision_memory_store.py |
Uses Azure CLI authentication. |
.../responses/foundry_memory/main.py |
Hashes trusted user Memory scopes. |
.../responses/foundry_memory/.env.example |
Adds explicit local Memory identity. |
.../responses/files/upload_file.py |
Adds bounded local/hosted uploads. |
.../responses/files/tests/test_file_access.py |
Tests file-access isolation boundaries. |
.../responses/files/README.md |
Documents secure upload and routing. |
.../responses/files/main.py |
Restricts tools to explicit uploads. |
.../responses/files/file_access.py |
Implements descriptor-relative file access. |
.../responses/custom_storage/README.md |
Documents scoped Cosmos persistence. |
.../responses/custom_storage/main.py |
Adds conditional Cosmos session storage. |
.../responses/custom_storage/agent.yaml |
Configures managed-identity Cosmos access. |
.../responses/custom_storage/agent.manifest.yaml |
Configures the Cosmos endpoint. |
.../responses/custom_storage/.env.example |
Replaces the connection string. |
.../responses/azure_search_rag/README.md |
Documents shared-index boundaries. |
.../responses/azure_search_rag/main.py |
Scopes Search resources per request. |
.../foundry-hosted-agents/README.md |
Summarizes hosting isolation conventions. |
.../claw_step04_production_ready/README.md |
Documents hosted claw isolation. |
.../claw_step04_production_ready/hosted.py |
Adds scoped memory and task cleanup. |
.../claw_step04_production_ready/agent.py |
Supports caller-owned chat clients. |
python/pyrefly.samples.toml |
Enables sibling-import fallback. |
python/packages/foundry_hosting/README.md |
Documents factory lifecycle guidance. |
.../agent_framework_foundry_hosting/_toolbox.py |
Clarifies Toolbox context capture. |
python/.github/skills/python-code-quality/SKILL.md |
Documents sample import resolution. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (4 commit(s)): 1db411c8a7db, 0a4796786a58, bbd3f0e52056, 515b57de002b
Model: gpt-5.6-sol
Overview
The PR moves hosted integrations to per-request factories, derives state keys from trusted platform identity, uses conditional ETags, and hardens upload reads with descriptor-relative no-follow checks and explicit byte bounds. Those guards address cross-request context reuse, storage races, and path traversal. Residual risk remains in unbounded persistent file memory, unretained custom Cosmos snapshots, and an unbounded background-task teardown wait.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (1 high, 2 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/samples/02-agents/harness/build_your_own_claw/claw_step04_production_ready/hosted.py, python/samples/04-hosting/foundry-hosted-agents/responses/custom_storage/main.py
|
Thanks, this is a careful slice. The descriptor-relative read path and the per-request factories look solid, and the Copilot and MAF-bot threads are all addressed. A few additional points that aren't covered in the existing threads, most important first. Each one was checked against 1. Claw streaming teardown is skipped when the stream is closed before its first update (
|
Approval was submitted on the wrong pull request.
|
Thanks for the detailed review in #8899 (comment). Addressed the scoped corrections in 2a13c74: idempotent outer-stream cleanup covers never-started and partially consumed streams; developer-selected uploads use portable bounded reads while sandbox access still rejects symlinks and hard links; GitHub MCP is limited to three read-only tools; Cosmos snapshots have an explicit conservative per-item budget and actionable 413 handling; and the README now states intentional same-user sharing across agents using the same project Memory Store. The maintainer explicitly asked to remove tests from these illustrative samples rather than register or relocate them, so the final added sample-test module and its README command are removed and the PR no longer claims sample-suite CI coverage. The rejected Cosmos/Memory modules remain removed. Existing framework cleanup regressions and credential-free development checks were used without adding a new suite or invoking Azure. The shared request-client helper/API remains a separate design follow-up rather than broadening this slice; retention and aggregate quotas remain in #8900 and #8901. |


Motivation & Context
Complete the Responses agent integrations and isolation slice of the coordinated Foundry-hosting redesign. The integrations must use trusted request identity and request-owned resources, not arbitrary filesystem paths, user-only state keys, literal Memory scope placeholders or a preceding caller's MCP connection context.
This builds on the merged foundation and Responses agent API, without a dependency on the parallel Invocations slice.
Description & Review Guide
history_source="agent_server"API. Sensitive telemetry payload capture is opt-in. Additional sample imports are declared in PEP 723 script metadata, not workspace/package/sample pyproject dependencies. The GitHub MCP example enforces an explicit read-only tool allowlist. No generic Agent/provider lifecycle, workflow API, shared-client ownership, Invocations API, dependency release or steering guard is changed./scope_keycontainer layout without unsafe legacy fallback, and local Memory needs an explicit single-user setting. External RBAC/resources, GitHub PAT/OAuth, Telegram/Key Vault and live telemetry need separate setup; live Azure deployments were not exercised. Hyperlight requires a hypervisor unavailable in the default Foundry runtime. Per maintainer direction, no new test modules are shipped in sample directories. Existing hosting/lifecycle regressions and credential-free boundary and mock-transport checks were used during development; those checks are not a guarantee that a configured live deployment will work.Related Issue
Closes #8746
Part of #8742.
Builds on merged #8741 and #8794.
Related, not closed: #6558, #7690 and #7916. Long-lived Toolbox reconnection/header propagation, full session-documentation acceptance and native generated-file citations remain separate concerns.
Per maintainer direction, hosted file-memory capacity/retention and Cosmos snapshot retention/quotas are tracked separately in #8900 and #8901. This PR does not claim to implement those limits.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.