Security: Container command injection via shell string interpolation in stopContainer()

[johnwaldo]

Severity: HIGH

Summary

stopContainer() in src/container-runtime.ts:18-19 uses shell string interpolation with exec():

export function stopContainer(name: string): string {
  return `${CONTAINER_RUNTIME_BIN} stop ${name}`;
}

This string is passed to exec() at src/container-runner.ts:368, which invokes a shell. If the name parameter ever contains shell metacharacters, this is a command injection vector.

Current Mitigations

The container name is constructed from group.folder with sanitization:

// container-runner.ts:230
const safeName = group.folder.replace(/[^a-zA-Z0-9-]/g, '-');
const containerName = `nanoclaw-${safeName}-${Date.now()}`;

And group.folder is validated by isValidGroupFolder() which restricts to [A-Za-z0-9][A-Za-z0-9_-]{0,63}.

However, stopContainer() itself performs no validation. It trusts its caller to provide a safe name. This is a defense-in-depth gap — if any future code path calls stopContainer() with unsanitized input, it becomes exploitable.

The same pattern exists in cleanupOrphans() (line 67) which passes container names from docker ps output through stopContainer().

Recommended Fix

Replace exec() with execFile() which does not invoke a shell:

import { execFile } from 'child_process';

export function stopContainer(name: string): string[] {
  return [CONTAINER_RUNTIME_BIN, 'stop', name];
}

// At call sites, use execFile instead of exec:
execFile(...stopContainer(containerName), { timeout: 15000 }, (err) => { ... });

Or alternatively, use spawn with an array of arguments (no shell).

Effort

~30 minutes. Three call sites to update (container-runner.ts:368, container-runtime.ts:67, and the function signature itself).

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• src/container-runtime.ts:18-19 (function definition)
• src/container-runner.ts:368 (exec call site)
• src/container-runtime.ts:67 (cleanupOrphans call site)

[ebenezer-isaac]

#1360 adds defense-in-depth for this. The canUseTool callback blocks bash commands matching configurable regex patterns, prevents env var references to secret variables, and blocks URL patterns in commands. It does not fix the stopContainer() interpolation itself but it limits what a compromised agent can do with shell access.