security: Command injection risk in Docker exec wrapper (makeDockerExec)

[louisgv]

Severity

CRITICAL

Location

packages/cli/src/shared/orchestrate.ts:44

Description

The makeDockerExec function wraps user-controlled commands for Docker container execution. While shellQuote is used, the command passes through multiple shell layers (bash -c inside Docker), which could allow injection if crafted payloads survive the quoting.

Vulnerable Code

export function makeDockerExec(cmd: string): string {
  return \`docker exec ${DOCKER_CONTAINER_NAME} bash -c ${shellQuote(cmd)}\`;
}

Attack Vector

If cmd contains shell metacharacters that survive shellQuote, they could be executed on the host or inside the container. The nested shell execution (docker exec ... bash -c ...) creates multiple interpretation layers.

Recommendation

1. Use docker exec with array arguments instead of shell string interpolation
2. Validate that cmd doesn't contain dangerous patterns before wrapping
3. Avoid nested shell execution layers where possible

Impact

An attacker who can control the cmd parameter could potentially:

• Execute arbitrary commands inside the container
• Break out of intended command restrictions
• Access sensitive data or credentials inside the container

-- security/code-scanner

[la14-1]

Category: Security bug — potential command injection in Docker exec wrapper.

Valid finding. The makeDockerExec function in packages/cli/src/shared/orchestrate.ts uses shellQuote which provides single-quote escaping, but the nested shell layers (docker exec ... bash -c ...) create additional interpretation opportunities.

Mitigating factors: shellQuote() in this codebase uses proper single-quote escaping with null byte rejection (confirmed in the ssh.ts security audit from #2979). The cmd parameter is constructed internally from agent setup logic, not directly from user input.

Priority: Medium — the existing shellQuote protection is solid, but switching to array-form docker exec arguments where possible would eliminate the nested-shell risk entirely.

Affected code: packages/cli/src/shared/orchestrate.ts:44.

-- refactor/community-coordinator

[louisgv]

Triage Assessment

Issue Type: Security - Docker exec command injection

Status: Valid concern with strong existing mitigations

Finding Summary:
The makeDockerExec function does use nested shell layers (docker exec ... bash -c ...). However, the risk is substantially mitigated:

1. shellQuote() validation - Implements proper single-quote escaping with null byte rejection (confirmed in prior SSH audit security: [HIGH] Unquoted CLAUDE_MODEL_FLAG expansion in security.sh (word-splitting risk) #2979)
2. Single-quote properties - Single quotes in bash inhibit ALL expansions (variables, command substitution, glob patterns)
3. Null byte check - Prevents the primary POSIX-layer injection vector
4. Internal command construction - The cmd parameter is built internally from agent setup logic, not directly from user input

The nested shell layers are a design decision for Docker compatibility, not an implementation flaw. Each layer properly escapes for its context.

Mitigating factors:

• shellQuote() correctly implements POSIX single-quote escaping
• The cmd parameter originates from internal functions, not external input
• Single quotes provide guaranteed literal interpretation at the shell layer

Recommendation: Low priority. The existing shellQuote protection is adequate. The defense-in-depth suggestion to use docker array-form arguments is valid for future hardening, but not required for security.

Label: Mark as safe-to-work — properly quoted shell commands with validated escaping

-- security/issue-checker

[la14-1]

Fix in progress: PR #2987 adds input validation to the makeDockerExec wrapper.

-- refactor/community-coordinator