BUG-HUNT: [SECURITY] Insecure temp directory usage in container executor #7058

Open
opened 2026-04-10 07:27:30 +00:00 by HAL9000 · 2 comments
Owner

Bug Report: [security] — Insecure temp directory usage in ContainerExecutor

Severity Assessment

  • Impact: Symlink attacks, permission issues, predictable path exploitation
  • Likelihood: Low to Medium — requires local attacker or misconfigured environment
  • Priority: Medium (security concern with partial mitigation already in place)
  • Category: security

Location

  • File: src/cleveragents/tool/container_executor.py
  • Function/Class: ContainerExecutor.__init__ (also applies to ContainerToolExecutor.__init__)
  • Lines: 181

Description

ContainerExecutor.__init__ uses a hardcoded "/tmp/sandbox" string as the fallback value when config.host_sandbox_path is not set:

host_root = config.host_sandbox_path or "/tmp/sandbox"
fallback_path = Path(host_root)
if not config.host_sandbox_path and fallback_path.is_symlink():
    raise ContainerExecutionError(
        f"Default sandbox path {host_root!r} is a symlink — "
        "set host_sandbox_path explicitly on ContainerConfig",
    )

While the code does check for symlinks on the fallback path, this mitigation is incomplete:

  1. Predictable path: /tmp/sandbox is a well-known, fixed path. An attacker who can write to /tmp can pre-create this directory with permissive permissions or race-condition the creation.
  2. Symlink check is insufficient: The check only catches the case where /tmp/sandbox itself is a symlink. It does not protect against:
    • A malicious directory already existing at /tmp/sandbox with world-writable permissions
    • TOCTOU (time-of-check/time-of-use) races between the is_symlink() check and subsequent use
    • Subdirectory symlink attacks within /tmp/sandbox
  3. Permission issues: /tmp is world-writable on most Unix systems. A pre-created /tmp/sandbox directory owned by another user could cause unexpected permission errors or data leakage.
  4. No uniqueness: Multiple concurrent processes or test runs all share the same /tmp/sandbox path, leading to potential data contamination between runs.

Expected Behavior

When config.host_sandbox_path is not provided, the executor should either:

  1. Raise immediately — require callers to always supply an explicit host_sandbox_path, or
  2. Create a secure temporary directory using tempfile.mkdtemp() (or tempfile.TemporaryDirectory) which generates a unique, mode-0700 directory owned by the current user, eliminating predictability and permission issues.

The preferred fix per Python security best practices is tempfile.mkdtemp(prefix="cleveragents-sandbox-"), which:

  • Creates a directory with a random suffix (unpredictable path)
  • Sets permissions to 0o700 (owner-only access)
  • Is owned by the current process user

Actual Behavior

The fallback path /tmp/sandbox is hardcoded, predictable, and only partially protected by a symlink check. The symlink check does not cover all attack vectors and is subject to TOCTOU races.

Suggested Fix

import tempfile

# In ContainerExecutor.__init__:
if config.host_sandbox_path:
    host_root = config.host_sandbox_path
    fallback_path = Path(host_root)
    if fallback_path.is_symlink():
        raise ContainerExecutionError(
            f"Sandbox path {host_root!r} is a symlink — "
            "set a non-symlink host_sandbox_path on ContainerConfig",
        )
else:
    # Use a secure, unique temp directory instead of a hardcoded path
    host_root = tempfile.mkdtemp(prefix="cleveragents-sandbox-")

Alternatively, require host_sandbox_path to always be set and remove the fallback entirely, forcing callers to be explicit.


Metadata

  • Branch: bugfix/insecure-temp-dir-container-executor
  • Commit Message: fix(tool): replace hardcoded /tmp/sandbox fallback with secure tempfile.mkdtemp() in ContainerExecutor
  • Milestone: (none — backlog)
  • Parent Epic: (orphan — needs manual linking, see comment)

Backlog note: This issue was discovered during autonomous operation
on milestone v3.5.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.


Subtasks

  • Create TDD issue (Type/Testing, title: TDD: Insecure temp directory usage in container executor) with tags @tdd_issue, @tdd_issue_<this-issue-number>, @tdd_expected_fail; link this bug issue as depending on the TDD issue
  • Write failing Behave scenario that asserts ContainerExecutor.__init__ does not use a predictable /tmp/sandbox path when host_sandbox_path is unset
  • Replace hardcoded "/tmp/sandbox" fallback with tempfile.mkdtemp(prefix="cleveragents-sandbox-") in ContainerExecutor.__init__
  • Ensure the created temp directory is cleaned up appropriately (context manager or explicit cleanup in close()/__del__)
  • Remove @tdd_expected_fail tag from the TDD scenario after fix is implemented
  • Update docstring for ContainerExecutor.__init__ to document the secure temp directory behaviour
  • Verify no other locations in container_executor.py use hardcoded /tmp/sandbox paths
  • Run nox -s security_scan to confirm Bandit no longer flags the hardcoded path

Definition of Done

  • ContainerExecutor.__init__ no longer uses /tmp/sandbox as a fallback path
  • When host_sandbox_path is unset, a unique tempfile.mkdtemp()-created directory is used with 0o700 permissions
  • Behave BDD scenario covers the secure temp directory creation path
  • Robot Framework integration test verifies executor initialises without a pre-existing /tmp/sandbox
  • @tdd_expected_fail tag removed from TDD scenario
  • nox -s security_scan passes with no new high/critical findings
  • All nox stages pass
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: new-issue-creator

## Bug Report: [security] — Insecure temp directory usage in ContainerExecutor ### Severity Assessment - **Impact**: Symlink attacks, permission issues, predictable path exploitation - **Likelihood**: Low to Medium — requires local attacker or misconfigured environment - **Priority**: Medium (security concern with partial mitigation already in place) - **Category**: security --- ### Location - **File**: `src/cleveragents/tool/container_executor.py` - **Function/Class**: `ContainerExecutor.__init__` (also applies to `ContainerToolExecutor.__init__`) - **Lines**: 181 --- ### Description `ContainerExecutor.__init__` uses a hardcoded `"/tmp/sandbox"` string as the fallback value when `config.host_sandbox_path` is not set: ```python host_root = config.host_sandbox_path or "/tmp/sandbox" fallback_path = Path(host_root) if not config.host_sandbox_path and fallback_path.is_symlink(): raise ContainerExecutionError( f"Default sandbox path {host_root!r} is a symlink — " "set host_sandbox_path explicitly on ContainerConfig", ) ``` While the code does check for symlinks on the fallback path, this mitigation is incomplete: 1. **Predictable path**: `/tmp/sandbox` is a well-known, fixed path. An attacker who can write to `/tmp` can pre-create this directory with permissive permissions or race-condition the creation. 2. **Symlink check is insufficient**: The check only catches the case where `/tmp/sandbox` itself is a symlink. It does not protect against: - A malicious directory already existing at `/tmp/sandbox` with world-writable permissions - TOCTOU (time-of-check/time-of-use) races between the `is_symlink()` check and subsequent use - Subdirectory symlink attacks within `/tmp/sandbox` 3. **Permission issues**: `/tmp` is world-writable on most Unix systems. A pre-created `/tmp/sandbox` directory owned by another user could cause unexpected permission errors or data leakage. 4. **No uniqueness**: Multiple concurrent processes or test runs all share the same `/tmp/sandbox` path, leading to potential data contamination between runs. ### Expected Behavior When `config.host_sandbox_path` is not provided, the executor should either: 1. **Raise immediately** — require callers to always supply an explicit `host_sandbox_path`, or 2. **Create a secure temporary directory** using `tempfile.mkdtemp()` (or `tempfile.TemporaryDirectory`) which generates a unique, mode-0700 directory owned by the current user, eliminating predictability and permission issues. The preferred fix per Python security best practices is `tempfile.mkdtemp(prefix="cleveragents-sandbox-")`, which: - Creates a directory with a random suffix (unpredictable path) - Sets permissions to `0o700` (owner-only access) - Is owned by the current process user ### Actual Behavior The fallback path `/tmp/sandbox` is hardcoded, predictable, and only partially protected by a symlink check. The symlink check does not cover all attack vectors and is subject to TOCTOU races. ### Suggested Fix ```python import tempfile # In ContainerExecutor.__init__: if config.host_sandbox_path: host_root = config.host_sandbox_path fallback_path = Path(host_root) if fallback_path.is_symlink(): raise ContainerExecutionError( f"Sandbox path {host_root!r} is a symlink — " "set a non-symlink host_sandbox_path on ContainerConfig", ) else: # Use a secure, unique temp directory instead of a hardcoded path host_root = tempfile.mkdtemp(prefix="cleveragents-sandbox-") ``` Alternatively, require `host_sandbox_path` to always be set and remove the fallback entirely, forcing callers to be explicit. --- ## Metadata - **Branch**: `bugfix/insecure-temp-dir-container-executor` - **Commit Message**: `fix(tool): replace hardcoded /tmp/sandbox fallback with secure tempfile.mkdtemp() in ContainerExecutor` - **Milestone**: *(none — backlog)* - **Parent Epic**: *(orphan — needs manual linking, see comment)* > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.5.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- ## Subtasks - [ ] Create TDD issue (`Type/Testing`, title: `TDD: Insecure temp directory usage in container executor`) with tags `@tdd_issue`, `@tdd_issue_<this-issue-number>`, `@tdd_expected_fail`; link this bug issue as depending on the TDD issue - [ ] Write failing Behave scenario that asserts `ContainerExecutor.__init__` does not use a predictable `/tmp/sandbox` path when `host_sandbox_path` is unset - [ ] Replace hardcoded `"/tmp/sandbox"` fallback with `tempfile.mkdtemp(prefix="cleveragents-sandbox-")` in `ContainerExecutor.__init__` - [ ] Ensure the created temp directory is cleaned up appropriately (context manager or explicit cleanup in `close()`/`__del__`) - [ ] Remove `@tdd_expected_fail` tag from the TDD scenario after fix is implemented - [ ] Update docstring for `ContainerExecutor.__init__` to document the secure temp directory behaviour - [ ] Verify no other locations in `container_executor.py` use hardcoded `/tmp/sandbox` paths - [ ] Run `nox -s security_scan` to confirm Bandit no longer flags the hardcoded path --- ## Definition of Done - [ ] `ContainerExecutor.__init__` no longer uses `/tmp/sandbox` as a fallback path - [ ] When `host_sandbox_path` is unset, a unique `tempfile.mkdtemp()`-created directory is used with `0o700` permissions - [ ] Behave BDD scenario covers the secure temp directory creation path - [ ] Robot Framework integration test verifies executor initialises without a pre-existing `/tmp/sandbox` - [ ] `@tdd_expected_fail` tag removed from TDD scenario - [ ] `nox -s security_scan` passes with no new high/critical findings - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: new-issue-creator
Author
Owner

⚠️ Orphan Issue — Needs Manual Parent Epic Linking

This issue was created during autonomous bug-hunting and no parent Epic was provided by the caller.

Per CONTRIBUTING.md, orphan issues are not permitted. A human maintainer must link this issue to an appropriate parent Epic using Forgejo's dependency system (this issue blocks the parent Epic).

Suggested parent Epics to consider:

# Title Notes
#7028 [INFRASTRUCTURE] Container Base Image Security Enhancement Needed Closest container-security Epic, but scoped to Dockerfile base image — may need a new broader "Container Executor Security" Epic

Action required:

  1. Identify or create an appropriate parent Epic for container executor / tool execution security
  2. Set the dependency: this issue (#7058) blocks the parent Epic
  3. Remove this orphan comment once linked

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: new-issue-creator

## ⚠️ Orphan Issue — Needs Manual Parent Epic Linking This issue was created during autonomous bug-hunting and **no parent Epic was provided** by the caller. Per `CONTRIBUTING.md`, orphan issues are not permitted. A human maintainer must link this issue to an appropriate parent Epic using Forgejo's dependency system (this issue **blocks** the parent Epic). ### Suggested parent Epics to consider: | # | Title | Notes | |---|---|---| | #7028 | [INFRASTRUCTURE] Container Base Image Security Enhancement Needed | Closest container-security Epic, but scoped to Dockerfile base image — may need a new broader "Container Executor Security" Epic | ### Action required: 1. Identify or create an appropriate parent Epic for container executor / tool execution security 2. Set the dependency: this issue (#7058) **blocks** the parent Epic 3. Remove this orphan comment once linked --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: new-issue-creator
Author
Owner

Verified — Security bug: insecure temp directory usage in container executor. MoSCoW: Must-have. Priority: High.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Security bug: insecure temp directory usage in container executor. MoSCoW: Must-have. Priority: High. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#7058
No description provided.