Repository navigation
.NET/Python: [BREAKING] Clarify docker params usage and rename param - #9453
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Both public API renames break existing callers despite the non-breaking declaration.
2 open findings
What changed in this PR
Clarifies trusted Docker configuration for the Python and .NET shell tools and explicitly names unsafe options.
Changes:
- Renames extra-run options and documents their isolation risks.
- Replaces Python flag blocking with token validation and snapshots caller-owned arguments.
- Adds validation, snapshot, and argument-forwarding tests.
| File | Description |
|---|---|
| python/packages/tools/tests/test_docker_shell_tool.py | Updates validation and forwarding tests. |
| python/packages/tools/README.md | Explains trusted configuration and isolation risks. |
| python/packages/tools/agent_framework_tools/shell/_docker.py | Renames options and replaces flag blocking. |
| dotnet/tests/Microsoft.Agents.AI.Tools.Shell.UnitTests/DockerShellExecutorTests.cs | Tests validation, snapshots, forwarding, and descriptions. |
| dotnet/src/Microsoft.Agents.AI.Tools.Shell/ShellExecutor.cs | Qualifies default isolation guarantees. |
| dotnet/src/Microsoft.Agents.AI.Tools.Shell/DockerShellExecutorOptions.cs | Renames and documents unsafe options. |
| dotnet/src/Microsoft.Agents.AI.Tools.Shell/DockerShellExecutor.cs | Snapshots and validates options; clarifies isolation descriptions. |
🧠 Review effort: Balanced
💡 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 (1 commit(s)): fdc245c4e1c2
Model: gpt-5.6-sol
Overview
The PR makes unrestricted Docker arguments an explicitly unsafe, developer-controlled configuration surface and adds input snapshots, token-shape validation, warnings, and coverage for both execution modes. Default isolation and the model-facing command boundary remain intact. However, both renamed options were already shipped publicly, and removing their old names without compatibility aliases breaks existing .NET and Python consumers during upgrade.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Tools.Shell/DockerShellExecutorOptions.cs, python/packages/tools/agent_framework_tools/shell/_docker.py
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
only looked at dotnet part

Motivation & Context
Description & Review Guide
Improve instructions around how to use docker params and rename param
Related Issue
Fixes #
AI Assistance
AI assistance details:
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.