Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions dotnet/src/Microsoft.Agents.AI.Tools.Shell/DockerShellExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ namespace Microsoft.Agents.AI.Tools.Shell;
/// <c>--cap-drop=ALL</c>, <c>--security-opt=no-new-privileges</c>, memory and
/// pids limits, <c>--tmpfs /tmp</c>). These are a best-effort starting point,
/// NOT a guarantee: the actual isolation you get depends on the host kernel,
/// the container runtime, the image, and any caller-supplied
/// <c>ExtraRunArgs</c>. Do not rely on this tool as your sole defense against
/// the container runtime, the image, and the complete caller configuration.
/// <see cref="DockerShellExecutorOptions.UnsafeExtraRunArgs"/> must come only
/// from developer-controlled configuration, never users, model output, or
/// other untrusted sources; it can weaken or remove isolation. The model-facing
/// function accepts only commands and cannot choose Docker options. Do not
/// rely on this tool as your sole defense against
/// untrusted input. Approval gating via <see cref="AsAIFunction"/> is the
/// primary safety control; pair it with the precautions you would normally
/// apply when running adversarial code: review the model's output before
Expand Down Expand Up @@ -92,7 +96,7 @@ public sealed class DockerShellExecutor : ShellExecutor
private readonly int _pidsLimit;
private readonly ContainerUser _user;
private readonly bool _readOnlyRoot;
private readonly IReadOnlyList<string> _extraRunArgs;
private readonly IReadOnlyList<string> _unsafeExtraRunArgs;
private readonly IReadOnlyDictionary<string, string> _env;
private readonly ShellPolicy _policy;
private readonly TimeSpan? _timeout;
Expand Down Expand Up @@ -125,6 +129,8 @@ public DockerShellExecutor(DockerShellExecutorOptions options)
{
throw new ArgumentOutOfRangeException(nameof(options), $"{nameof(options.MemoryBytes)} must be positive.");
}
string[] unsafeExtraRunArgs = options.UnsafeExtraRunArgs is null ? [] : options.UnsafeExtraRunArgs.ToArray();
ValidateUnsafeExtraRunArgs(unsafeExtraRunArgs);

this._image = options.Image;
this.ContainerName = options.ContainerName ?? GenerateContainerName();
Expand All @@ -137,7 +143,7 @@ public DockerShellExecutor(DockerShellExecutorOptions options)
this._pidsLimit = options.PidsLimit;
this._user = options.User ?? ContainerUser.Default;
this._readOnlyRoot = options.ReadOnlyRoot;
this._extraRunArgs = options.ExtraRunArgs ?? [];
this._unsafeExtraRunArgs = unsafeExtraRunArgs;
this._env = options.Environment ?? new Dictionary<string, string>();
this._policy = options.Policy ?? new ShellPolicy();
this._timeout = options.Timeout;
Expand Down Expand Up @@ -277,9 +283,8 @@ internal static string FormatMemoryBytes(long memoryBytes) =>
public override AIFunction AsAIFunction(string name = "run_shell", string? description = null, bool requireApproval = true)
{
description ??=
"Execute a single shell command inside an isolated Docker container and return its " +
"stdout, stderr, and exit code. The container has no network, no host filesystem access " +
"(except an optional read-only workspace mount), and runs as a non-root user. " +
"Execute a single shell command using the configured Docker container and return its " +
"stdout, stderr, and exit code. Container isolation depends on the application's complete configuration. " +
(this._mode == ShellMode.Persistent
? "PERSISTENT MODE: a single long-lived container handles every call; cd and exported variables persist."
: "STATELESS MODE: each call runs in a fresh container.");
Expand Down Expand Up @@ -430,10 +435,7 @@ public static IReadOnlyList<string> BuildExecArgv(string binary, string containe

private async Task StartContainerAsync(CancellationToken cancellationToken)
{
var argv = BuildRunArgv(
this.DockerBinary, this._image, this.ContainerName, this._user, this._network,
this._memoryBytes, this._pidsLimit, this._containerWorkdir, this._hostWorkdir,
this._mountReadonly, this._readOnlyRoot, this._env, this._extraRunArgs);
var argv = this.BuildRunArgvPersistent();

var (exit, _, stderr) = await RunDockerCommandAsync(argv, cancellationToken).ConfigureAwait(false);
if (exit != 0)
Expand Down Expand Up @@ -535,7 +537,7 @@ private async Task<ShellResult> RunStatelessAsync(string command, CancellationTo
TimedOut: timedOut);
}

private List<string> BuildRunArgvStateless(string perCallName)
internal List<string> BuildRunArgvStateless(string perCallName)
{
var argv = new List<string>
{
Expand Down Expand Up @@ -563,10 +565,28 @@ private List<string> BuildRunArgvStateless(string perCallName)
argv.Add("-e");
argv.Add($"{kv.Key}={kv.Value}");
}
argv.AddRange(this._extraRunArgs);
argv.AddRange(this._unsafeExtraRunArgs);
return argv;
}

internal IReadOnlyList<string> BuildRunArgvPersistent() =>
BuildRunArgv(
this.DockerBinary, this._image, this.ContainerName, this._user, this._network,
this._memoryBytes, this._pidsLimit, this._containerWorkdir, this._hostWorkdir,
this._mountReadonly, this._readOnlyRoot, this._env, this._unsafeExtraRunArgs);

private static void ValidateUnsafeExtraRunArgs(string[] args)
{
for (var index = 0; index < args.Length; index++)
{
if (args[index] is null)
{
throw new ArgumentException(
$"{nameof(DockerShellExecutorOptions.UnsafeExtraRunArgs)} contains a null token at index {index}.");
}
}
}

private async Task BestEffortKillContainerAsync(string containerName)
{
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,16 @@ public sealed class DockerShellExecutorOptions
/// <summary>When <see langword="true"/> (the default), the container root filesystem is read-only.</summary>
public bool ReadOnlyRoot { get; set; } = true;

/// <summary>Additional args appended to <c>docker run</c>.</summary>
public IReadOnlyList<string>? ExtraRunArgs { get; set; }
/// <summary>
/// Trusted application-supplied options appended verbatim to <c>docker run</c>.
/// Set this only from developer-controlled configuration; never use values
/// from users, model output, or other untrusted sources. These options can
/// weaken or completely remove container isolation. Review every option
/// carefully. In particular, avoid <c>--privileged</c>, <c>--network=host</c>,
/// host-path volume mounts, and added capabilities unless the application
/// explicitly requires and secures them.
/// </summary>
public IReadOnlyList<string>? UnsafeExtraRunArgs { get; set; }
Comment thread
westey-m marked this conversation as resolved.
Comment thread
westey-m marked this conversation as resolved.

/// <summary>Environment variables passed via <c>-e</c> to every command.</summary>
public IReadOnlyDictionary<string, string>? Environment { get; set; }
Expand Down
6 changes: 4 additions & 2 deletions dotnet/src/Microsoft.Agents.AI.Tools.Shell/ShellExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ namespace Microsoft.Agents.AI.Tools.Shell;
/// <para>
/// <see cref="LocalShellExecutor"/> runs commands directly on the host (no
/// isolation; approval-in-the-loop is the security boundary).
/// <see cref="DockerShellExecutor"/> runs them inside a container with resource
/// limits, network isolation, and a non-root user.
/// <see cref="DockerShellExecutor"/> runs them inside a configured container;
/// its default profile applies resource limits, network isolation, and a
/// non-root user, but effective isolation depends on the complete application
/// configuration.
/// </para>
/// <para>
/// This is an abstract class rather than an interface so the surface can be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,51 @@ public void BuildRunArgv_EnvAndExtraArgs_AreAppended()
Assert.Contains("owner=test", list);
}

[Fact]
public void Ctor_RejectsNullUnsafeExtraRunArg()
{
// Arrange
var options = new DockerShellExecutorOptions { UnsafeExtraRunArgs = [null!] };

// Act
var exception = Assert.Throws<ArgumentException>(() => new DockerShellExecutor(options));

// Assert
Assert.Contains("UnsafeExtraRunArgs", exception.Message, StringComparison.Ordinal);
Assert.Contains("null token", exception.Message, StringComparison.Ordinal);
}

[Fact]
public async Task Ctor_SnapshotsCallerOwnedUnsafeExtraRunArgListAsync()
{
// Arrange
var unsafeArgs = new List<string> { "--network", "host" };
await using var unsafeExecutor = new DockerShellExecutor(new() { UnsafeExtraRunArgs = unsafeArgs });

// Act
unsafeArgs.Add("--privileged");

// Assert
Assert.DoesNotContain("--privileged", unsafeExecutor.BuildRunArgvPersistent());
Assert.DoesNotContain("--privileged", unsafeExecutor.BuildRunArgvStateless("unsafe"));
}

[Fact]
public async Task Ctor_PassesUnsafeExtraRunArgsVerbatimInBothModesAsync()
{
// Arrange
string[] unsafeArgs = ["--network", "host", "--entrypoint", "/bin/custom"];
await using var executor = new DockerShellExecutor(new() { UnsafeExtraRunArgs = unsafeArgs });

// Act
var persistent = executor.BuildRunArgvPersistent();
var stateless = executor.BuildRunArgvStateless("stateless");

// Assert
AssertOrderedSubsequence(persistent, unsafeArgs);
AssertOrderedSubsequence(stateless, unsafeArgs);
}

private static readonly string[] s_expectedInteractive = new[] { "docker", "exec", "-i", "af-shell-x", "bash", "--noprofile", "--norc" };

[Fact]
Expand Down Expand Up @@ -169,6 +214,9 @@ public async Task AsAIFunction_DefaultRequireApproval_IsApprovalGatedAsync()
var fn = t.AsAIFunction();
Assert.IsType<ApprovalRequiredAIFunction>(fn);
Assert.Equal("run_shell", fn.Name);
Assert.NotNull(fn.Description);
Assert.Contains("configured Docker container", fn.Description, StringComparison.Ordinal);
Assert.DoesNotContain("has no network", fn.Description, StringComparison.Ordinal);
}

[Fact]
Expand Down Expand Up @@ -211,4 +259,15 @@ public async Task RunAsync_RejectedCommand_ThrowsShellCommandRejectedAsync()
await Assert.ThrowsAsync<ShellCommandRejectedException>(
() => t.RunAsync("rm -rf /"));
}

private static void AssertOrderedSubsequence(IReadOnlyList<string> actual, IReadOnlyList<string> expected)
{
var start = 0;
foreach (var value in expected)
{
var index = actual.ToList().FindIndex(start, item => string.Equals(item, value, StringComparison.Ordinal));
Assert.True(index >= 0, $"Expected '{value}' after index {start - 1}.");
start = index + 1;
}
}
}
13 changes: 13 additions & 0 deletions python/packages/tools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,19 @@ To expose a host directory, pass `host_workdir="/path"` (mounted
read-only by default; `mount_readonly=False` to allow writes). Swap the
container runtime with `docker_binary="podman"`.

Applications that must pass additional Docker options can use
`unsafe_extra_run_args`. These trusted setup options are appended verbatim
and must come only from developer-controlled configuration. Never use values
from users, model output, or other untrusted sources. These options can weaken
or completely remove isolation. Review every option carefully. In particular,
avoid `--privileged`, `--network=host`, host-path volume mounts, and added
capabilities unless the application explicitly requires and secures them.

Arbitrary Docker configuration is trusted application setup. The
model-facing function accepts only a shell command and cannot choose Docker
options, but the restrictive profile is effective only when the complete
application configuration preserves it.

## Sandbox tiers at a glance

| Use case | Tool | Sandbox |
Expand Down
Loading
Loading