Skip to content

Python: Harden Foundry Responses integrations and sandbox isolation - #8899

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 7 commits into
microsoft:mainfrom
eavanvalkenburg:foundry-responses-integrations
Oct 1, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 7 commits into
microsoft:mainfrom
eavanvalkenburg:foundry-responses-integrations

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

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

  • What are the major changes? Implement bounded, descriptor-relative UTF-8 file reads under the current sandbox's dedicated upload directory, plus portable bounded developer-source reads, safe local staging and hosted upload guidance. Implement custom Cosmos snapshots scoped by trusted user and sandbox, using managed identity, create-only writes and per-key ETag replacement/deletion and an explicit per-item size budget. Bind a fresh Foundry Memory provider/project client to the trusted user and current call ID, intentionally sharing long-term memory across that user's sandboxes and agents using the same project Memory Store.
  • What is the impact of these changes? Toolbox/skills, Search RAG, Monty, hosted MCP, observability, local tools and the hosted claw now use request-owned factories with explicit cleanup of their own clients/providers/credentials. The claw's idempotent stream cleanup also covers closure before the first update. Its background tasks are cancelled/joined within the provider's finite cleanup timeout before its transports close, including lazy streaming; tasks that ignore cancellation are abandoned and logged rather than wedging teardown. Host-owned history uses the current 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.
  • What do you want reviewers to focus on? The descriptor/TOCTOU read boundary, canonical storage keys versus inner MAF IDs, conditional writes and deletes, trusted user-wide Memory versus sandbox-specific state, MCP connect-time context and streaming cleanup. Sample behavior is intentionally stricter: only explicit uploads are readable, Cosmos uses a new /scope_key container 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

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Medium severity · 2 Low severity

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.

Comment thread python/samples/04-hosting/foundry-hosted-agents/responses/files/upload_file.py Outdated
Comment thread python/samples/04-hosting/foundry-hosted-agents/responses/files/README.md Outdated
Comment thread python/samples/04-hosting/foundry-hosted-agents/responses/files/README.md Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@CloneOfAlex

Copy link
Copy Markdown

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 72f4435.

1. Claw streaming teardown is skipped when the stream is closed before its first update (claw_step04_production_ready/hosted.py)

Release and inner.close() live inside the updates() async generator. If the host or client closes the outer ResponseStream before the first __anext__ (for example, a client disconnect between response creation and the first SSE chunk), the generator body never starts. ResponseStream.close() only aclose()s an iterator that exists and then runs cleanup_hooks. So neither release_session nor inner.close() runs, and that request's background tasks and transports are left behind.

Minimal check against ResponseStream on this branch, using the same shape as the middleware:

never iterated -> close():  NOTHING released/closed
iterated once  -> close():  ['inner closed', 'released']

Suggested fix: make the teardown idempotent and also register it as a cleanup hook, so it runs whether or not iteration started.

released = False
async def teardown() -> None:
    nonlocal released
    if released:
        return
    released = True
    try:
        await inner.close()
    finally:
        for provider in background_providers:
            await provider.release_session(session)   # keep the existing error handling

context.result = ResponseStream(updates(), finalizer=..., cleanup_hooks=[teardown])
# and call `await teardown()` in updates()' finally instead of the inline close/release

2. The new file-access boundary tests are not collected by CI

python/pyproject.toml has testpaths = ['packages/**/tests', ...], and the merge-test jobs run python/tests/samples/. Nothing picks up samples/.../responses/files/tests/test_file_access.py, so the 6 checks the description relies on for the TOCTOU boundary never run in CI. The Telegram sample tests have the same gap. Consider moving them under python/tests/samples/, or adding an explicit job or poe task for samples/**/tests.

3. upload_file.py --session-id needs POSIX no-follow on the developer's machine

main() calls read_upload_source(args.file) before choosing local or hosted. That walks the source's parents with O_NOFOLLOW | O_DIRECTORY:

  • Windows: _directory_flags() raises RuntimeError: Secure file access requires POSIX ..., so a Windows developer can't upload to a hosted sandbox at all, even though the hosted path never touches the sandbox filesystem locally.
  • macOS and Linux: any source whose parent path contains a symlinked directory fails with a bare NotADirectoryError. On macOS that includes /tmp and /var/folders/..., which are symlinks into /private. Reproduced on Linux with a symlinked parent: NotADirectoryError: [Errno 20] Not a directory: 'linkdir'.

The no-follow walk protects the sandbox's HOME/sample_files. The developer-chosen source file doesn't need it. Suggestions:

  • For --session-id, do a plain bounded read: Path.resolve(), reject non-regular files, read at most MAX_FILE_BYTES + 1, then check UTF-8.
  • Keep the strict walk for --local staging only.
  • At minimum, convert the OSError into an actionable message.

4. Hard links are not covered by the read boundary (low, defence in depth)

O_NOFOLLOW and the fstat regular-file check stop symlink and FIFO substitution, but a hard link placed in sample_files passes both. Reproduced: os.link(secret, HOME/sample_files/notes.txt) then read_uploaded_file("notes.txt") returns the secret's content. It only matters if something other than the upload API can create entries there, for example a future shell or file tool in the same sandbox. A one-line metadata.st_nlink != 1 rejection in _read_file would close it, and would make the "only explicit uploads are readable" claim hold without that assumption.

5. Cosmos snapshots have no guard for the 2 MB item limit (custom_storage/main.py)

session.to_dict() grows with session state, including approvals, todos and provider state. Once a snapshot exceeds Cosmos's maximum item size, every create_item or replace_item for that key fails with a raw CosmosHttpResponseError (413), which isn't one of the handled 409/412 codes. The session then can't be saved again. This is separate from retention and quotas (#8901). Consider:

  • checking the serialized size before writing and raising a clear error;
  • documenting the limit next to the /scope_key layout.

6. Memory scope is user-wide and shared across agents (foundry_memory/main.py)

memory_scope hashes only ["foundry-memory-user-v1", user_id]. Two different agents configured with the same MEMORY_STORE_NAME will therefore read and write the same per-user memories. That may be intended, but the README describes the sharing as "across that user's sandboxes". If cross-agent sharing isn't intended, include the agent name, e.g. FOUNDRY_AGENT_NAME, in the hashed identity. Otherwise, state the cross-agent behaviour explicitly.

7. Hosted MCP: enforce the "read-only" guidance in code (mcp/main.py)

The README correctly says to use a least-privilege, read-only PAT, but the sample registers every GitHub MCP tool with approval_mode="never_require". Since the PAT is deployment-owned and shared by all callers, passing allowed_tools=[...read-only tools...] would enforce that intent in code. get_mcp_tool already supports allowed_tools, and a dict approval_mode would work for write tools. That way a PAT with broader scopes doesn't silently widen what any caller can do.

8. Follow-up suggestion: factor out the per-request client boilerplate

The class RequestClient(FoundryChatClient) subclass with an AsyncExitStack __aexit__ is now copied into 11 samples. Since this is the pattern users will copy for production, a small supported helper in agent_framework_foundry_hosting would remove a lot of subtle lifecycle code from user apps. For example, a request-owned client factory, or an owns_credential=True option on FoundryChatClient. Happy to open a separate issue if useful.

@jpalvarezl
Jose Alvarez (jpalvarezl) dismissed their stale review October 1, 2026 08:30

Approval was submitted on the wrong pull request.

@eavanvalkenburg

Copy link
Copy Markdown
Member Author

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.

Merged via the queue into microsoft:main with commit a850ee6 Oct 1, 2026
46 checks passed
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the foundry-responses-integrations branch October 1, 2026 09:26

This branch was successfully deployed

1 active deployment
github-app-auth — 2a13c746 Deployed Oct 1, 2026 by eavanvalkenburg via add_label #24162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

4 participants