Skip to content

fix: propagate Docker policy to file tools - #65367

Open
0112358DEUS wants to merge 1 commit into
NousResearch:mainfrom
0112358DEUS:fix/file-tool-docker-policy-propagation
Open

fix: propagate Docker policy to file tools#65367
0112358DEUS wants to merge 1 commit into
NousResearch:mainfrom
0112358DEUS:fix/file-tool-docker-policy-propagation

Conversation

@0112358DEUS

Copy link
Copy Markdown

Summary

Propagate Docker policy fields from terminal_tool._get_env_config() through the file-tool environment creation path.

When a file tool created the Docker environment first, tools/file_tools.py omitted policy fields that terminal_tool._create_environment() consumes. The consumer therefore fell back to its defaults, silently ignoring explicit configuration such as docker_persist_across_processes: false.

This forwards:

  • docker_env (default {})
  • docker_extra_args (default [])
  • docker_persist_across_processes (default true)
  • docker_orphan_reaper (default true)

The defaults match the terminal-tool path exactly, and .get() preserves explicit false and empty collection values.

Tests

Added deterministic propagation/default tests, including the original docker_persist_across_processes=False failure mode.

CI-parity per-file runner:

scripts/run_tests.sh \
  tests/tools/test_file_tools_container_config.py \
  tests/tools/test_container_cwd_sanitize.py \
  tests/tools/test_docker_orphan_reaper_integration.py \
  tests/tools/test_modal_sandbox_fixes.py \
  tests/tools/test_docker_environment.py \
  -q

5 files, 128 tests passed, 0 failed

Additional checks:

  • python3 -m py_compile tools/file_tools.py tests/tools/test_file_tools_container_config.py
  • git diff --check
  • independent read-only review: SHIP, correctness 9/10, risk 2/10

No Docker daemon is required for these tests.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) backend/docker Docker container execution sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 16, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the file-first environment path. The file-tools change matches the existing terminal creation path, but the same policy-propagation bug remains in a sibling lazy factory.

Problems

  • tools/code_execution_tool.py:680-688 builds a container_config for an execute_code-first environment but omits docker_env, docker_extra_args, docker_persist_across_processes, and docker_orphan_reaper. The shared consumer reads those values at tools/terminal_tool.py:1419-1446, so this path still falls back to defaults when it creates the Docker environment.

Suggested changes

  • Carry the same four fields through tools/code_execution_tool.py:680-688 and add a captured-argument regression test for explicit false and non-empty values.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/docker Docker container execution P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants