fix(tools): reconcile docker_network with --network in docker_extra_args

[davidkarban]

What does this PR do?

terminal.docker_network: false and terminal.docker_extra_args are independently documented and independently valid, but combining them was unconditionally fatal. DockerEnvironment.__init__ appended --network=none itself for the lockdown, then appended docker_extra_args verbatim, with nothing reconciling the two. Docker rejects a repeated --network:

docker: network "none" is specified multiple times

Every docker run for such a profile exited 125 and the Docker backend was unusable until one of the two settings was removed.

This reconciles them before the command line is built:

docker_network	docker_extra_args	Result
false	no network flag	--network=none — unchanged
false	--network=none	flag emitted once, logger.info
false	--network=host / named net	RuntimeError naming both keys

Why the third row raises instead of letting the extra arg win

"Extra args are appended last so they can override defaults" is the natural reading, and it is what #100248's original Proposed Fix suggested. Implementing it showed that it is wrong here, because of the cross-process reuse guard:

if not network:
    actual_mode = self._container_network_mode(container_id)
    mode_mismatch = actual_mode != "none"

Under override semantics, docker_network: false + --network=host would hand the agent a host-networked container despite the configured lockdown, and the guard would then remove and recreate that container on every startup, since its NetworkMode is permanently host and never none. A silent weakening of the lockdown plus unbounded container churn.

Those two settings genuinely contradict, so the honest resolution is to say so. This also matches the posture the module already takes next door — _extra_args_egress_collisions() raises rather than guessing when enforce_on_docker is set.

The {"--network", "--net"} flag set is lifted to a module constant so the new parser and the existing collision helper share a single definition.

How to test

Reduced to plain Docker, this is the exact shape all_run_args produced:

$ docker run --rm --network=none --user 1002:1002 --network=none --user 1009:1009 alpine id
docker: network "none" is specified multiple times
$ echo $?
125

Through Hermes:

terminal:
  backend: docker
  docker_network: false
  docker_extra_args: ["--network=none", "--user", "1009:1009"]

Before: every sandbox creation fails with exit 125. After: the container starts, --network=none appears once, and --user 1009:1009 is applied.

Automated:

$ pytest tests/tools/test_docker_network_config.py -q
10 passed

$ pytest tests/tools/*docker* tests/tools/test_terminal_tool.py -q
150 passed

The four new tests fail on the unpatched tree (verified by reverting only tools/environments/docker.py and re-running) and pass with the fix. test_lockdown_still_applies_without_extra_network_arg is the no-regression guard for the common case, and passes either way.

New coverage:

• --network=none in extra args → flag emitted exactly once
• space-separated --network none → same
• no network flag in extra args → implicit --network=none still emitted
• contradictory --network=host → raises
• parser units: --net aliases, = and space forms, non-string entries, and a trailing bare --network that must not IndexError

Platforms tested

Linux (Ubuntu 24.04, Docker 29.5.2, Python 3.11.15). The change is pure argument-list construction with no platform-specific behaviour.

Related issue

Fixes #100248.

Note for reviewers: that issue's Proposed Fix section describes the simple override semantics. I have commented there explaining why this PR uses the three-way split instead, so the issue and PR do not disagree.

Adjacent but deliberately not touched here, to keep this focused: #84027 (file_tools / code_execution_tool drop docker_extra_args from their container_config). The two interact badly in production — the exit 125 above kills the terminal-tool creation, then a file_tools creation succeeds because it drops the extra args, silently yielding a container with the wrong uid. That is #84027's to fix (#35660 / #90050 are open for it).

[Enough1122]

AI code review — automated review for reference; please use your judgment.

This PR fixes a real bug: with terminal.docker_network: false, the implicit --network=none (docker.py:1005) could be emitted alongside an operator-supplied --network=none in docker_extra_args, and Docker rejects repeated --network flags with exit 125, breaking every container start (issue #100248).

• The new _extra_args_network_mode parser (docker.py:126) correctly handles both --network=<mode> and space-separated --network <mode> forms, including the --net alias, and skips non-string entries. It is reused by _extra_args_egress_collisions via _NETWORK_FLAGS (docker.py:156), removing a previously duplicated flag set.
• Fail-closed behavior is sensible: a contradictory --network=host with docker_network: false raises a RuntimeError naming both config keys (docker.py:181) instead of silently picking a winner or handing the agent a networked container. Note this is a behavior change — configs that previously produced a duplicate --network flag will now raise loudly at environment construction rather than failing at container start, which is the intended improvement.
• Edge cases are covered: a bare trailing --network with no value parses to "" and is treated as contradictory (fail-closed), and _extra_args_network_mode(None) returns None so the implicit flag is still emitted.
• Tests are thorough: dedup for both flag syntaxes, untouched operator args, the unchanged default path, the contradictory-request failure, and a focused parser unit test.

Verdict: LGTM

[kshitijk4poor]

Landed on main via #103644 as d72cbbcf5a, with your authorship preserved on the fix commit, @davidkarban — thank you. Closing in favour of the merged salvage.