fix(sandbox): stop resolving sandbox and ACP env values a second time (#6673)

* fix(sandbox): stop resolving sandbox and ACP env values a second time

AppConfig.resolve_env_variables already replaces every $VAR when
config.yaml loads. The AIO, E2B and OpenSandbox providers and the ACP
tool then resolved the result again, so a secret that itself starts with
$ (a bcrypt hash, a generated password) was looked up as a variable name
and reached the sandbox or agent as "" -- or as an unrelated host
variable's value when that name existed.

Pass the loaded values through unchanged, as BoxLite and the local
provider already do. The removed helpers' str() coercion was dead: both
fields are typed dict[str, str] and reject non-strings at load.

* docs(changelog): reference #6673

* docs(config): pin when sandbox and ACP env values are resolved

ACPAgentConfig.env and the SandboxConfig docstring said $ values are
resolved from the host environment without saying when, which reads as
"at spawn time" and invites a consumer-side second pass again. Both now
say it happens when config is loaded, matching SandboxConfig.environment.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Hyeonsang Cho 2026-10-11 18:50:04 +09:00 committed by GitHub
parent 37ff61e87b
commit 29a958100e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 147 additions and 55 deletions

View File

@ -4177,6 +4177,15 @@ This release closes that milestone with **439 merged pull requests**.
word order cannot be recovered. Passing both `--print` and `--json` now exits
2 instead of running `--print` and dropping the `--json` message. ([#6636])
- **sandbox:** The AIO, E2B and OpenSandbox providers and ACP agents no longer
resolve `sandbox.environment` / `acp_agents.<name>.env` values a second time.
`config.yaml` loading already replaces each `$VAR`, so a resolved secret that
itself starts with `$` (a bcrypt hash such as `$2b$12$...`, a generated
password) was looked up again as a variable name and reached the sandbox or
agent as an empty string, or as an unrelated host variable's value when that
name existed. Values are now passed through as loaded, as BoxLite and the
local provider already did. ([#6673])
### Security
- **authz:** Enforce permission checks on routes that only had authentication, and
@ -9588,3 +9597,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
[#6635]: https://github.com/bytedance/deer-flow/pull/6635
[#6636]: https://github.com/bytedance/deer-flow/pull/6636
[#6666]: https://github.com/bytedance/deer-flow/pull/6666
[#6673]: https://github.com/bytedance/deer-flow/pull/6673

View File

@ -3209,6 +3209,13 @@
由于词序无法还原,会以退出码 2 退出并提示加引号。同时传入 `--print` 和 `--json`
现在会以退出码 2 退出,而不是运行 `--print` 并丢弃 `--json` 的消息。([#6636])
- **沙箱:** AIO、E2B、OpenSandbox 提供者和 ACP agent 不再对
`sandbox.environment` / `acp_agents.<name>.env` 的值做第二次解析。加载
`config.yaml` 时已替换每个 `$VAR`,因此本身以 `$` 开头的已解析密钥(如
`$2b$12$...` 形式的 bcrypt 哈希或生成的密码)会再次被当作变量名查找,传入沙箱
或 agent 时变成空字符串;若同名主机变量存在,则变成该变量的值。现在这些值按
加载结果原样传递,与 BoxLite 和本地提供者一致。([#6673])
### 安全
- **鉴权:** 仅有认证而无权限校验的路由现在会强制执行权限检查,且
@ -7708,3 +7715,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
[#6635]: https://github.com/bytedance/deer-flow/pull/6635
[#6636]: https://github.com/bytedance/deer-flow/pull/6636
[#6666]: https://github.com/bytedance/deer-flow/pull/6666
[#6673]: https://github.com/bytedance/deer-flow/pull/6673

View File

@ -18,7 +18,6 @@ import errno
import hashlib
import logging
import math
import os
import signal
import threading
import time
@ -392,7 +391,9 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider):
if not isinstance(configured_skills_path, str):
configured_skills_path = DEFAULT_SKILLS_CONTAINER_PATH
environment = self._resolve_env_vars(sandbox_config.environment or {})
# $VARS in config.yaml are already resolved by AppConfig.resolve_env_variables;
# resolving again would rewrite a secret that itself starts with $.
environment = dict(sandbox_config.environment or {})
command_timeout = self._positive_float(
"bash_command_timeout",
getattr(sandbox_config, "bash_command_timeout", None),
@ -460,18 +461,6 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider):
return False
return self._backend.decide_network_policy_request(sandbox_id, request_id, decision)
@staticmethod
def _resolve_env_vars(env_config: dict[str, str]) -> dict[str, str]:
"""Resolve environment variable references (values starting with $)."""
resolved = {}
for key, value in env_config.items():
if isinstance(value, str) and value.startswith("$"):
env_name = value[1:]
resolved[key] = os.environ.get(env_name, "")
else:
resolved[key] = str(value)
return resolved
# ── Cross-instance ownership leases ───────────────────────────────────
def _publish_ownership(self, sandbox_id: str) -> None:

View File

@ -431,7 +431,9 @@ class E2BSandboxProvider(SandboxProvider):
"acquire_timeout": acquire_timeout,
"burst_limit": burst_limit,
"mounts": _opt("mounts") or [],
"environment": self._resolve_env_vars(_opt("environment") or {}),
# $VARS in config.yaml are already resolved by AppConfig.resolve_env_variables;
# resolving again would rewrite a secret that itself starts with $.
"environment": dict(_opt("environment") or {}),
"ownership": _opt("ownership"),
"stream_bridge": getattr(app_config, "stream_bridge", None),
"reconciliation_interval_seconds": max(
@ -461,16 +463,6 @@ class E2BSandboxProvider(SandboxProvider):
"mount_upload_deadline_seconds": self._resolve_mount_upload_deadline(_opt),
}
@staticmethod
def _resolve_env_vars(env_config: dict[str, str]) -> dict[str, str]:
resolved: dict[str, str] = {}
for key, value in env_config.items():
if isinstance(value, str) and value.startswith("$"):
resolved[key] = os.environ.get(value[1:], "")
else:
resolved[key] = "" if value is None else str(value)
return resolved
@staticmethod
def _resolve_mount_upload_deadline(_opt: Callable[[str, Any], Any]) -> int:
raw = _opt("mount_upload_deadline_seconds", _MOUNT_PASS_DEADLINE_SECONDS)

View File

@ -36,8 +36,9 @@ reach the OpenSandbox management service but cannot directly reach sandbox
`execd` endpoints.
Values in `sandbox.environment` that start with `$` are resolved from the
Gateway process environment when the provider starts. Missing variables resolve
to an empty string, matching the E2B provider.
Gateway process environment once, when `config.yaml` loads; a missing variable
fails the load. The provider injects the resolved values unchanged, so a secret
that itself starts with `$` reaches the sandbox intact.
## Lifecycle and contract

View File

@ -114,6 +114,8 @@ class OpenSandboxProvider(WarmPoolLifecycleMixin[OpenSandboxSandbox], SandboxPro
logger.warning("OpenSandboxProvider: no api_key or domain configured (set sandbox.api_key/sandbox.domain in config.yaml or OPEN_SANDBOX_API_KEY/OPEN_SANDBOX_DOMAIN). The SDK will default to unauthenticated localhost:8080.")
if _uses_insecure_remote_http(effective_domain, protocol):
logger.warning("OpenSandboxProvider: remote OpenSandbox domain uses HTTP; use HTTPS to protect credentials and sandbox traffic.")
# $VARS in config.yaml are already resolved by AppConfig.resolve_env_variables;
# resolving again would rewrite a secret that itself starts with $.
environment = dict(option("environment") or {})
_validate_extra_env(environment)
replicas = option("replicas")
@ -135,21 +137,11 @@ class OpenSandboxProvider(WarmPoolLifecycleMixin[OpenSandboxSandbox], SandboxPro
"ready_timeout": self._positive_float("ready_timeout", option("ready_timeout"), DEFAULT_READY_TIMEOUT),
"sandbox_timeout": None if sandbox_timeout == 0 else sandbox_timeout,
"command_timeout": self._positive_float("bash_command_timeout", option("bash_command_timeout"), DEFAULT_COMMAND_TIMEOUT),
"environment": self._resolve_env_vars(environment),
"environment": environment,
"replicas": replicas if replicas is not None else self.DEFAULT_REPLICAS,
"idle_timeout": idle_timeout if idle_timeout is not None else self.DEFAULT_IDLE_TIMEOUT,
}
@staticmethod
def _resolve_env_vars(env_config: dict[str, str]) -> dict[str, str]:
resolved: dict[str, str] = {}
for key, value in env_config.items():
if isinstance(value, str) and value.startswith("$"):
resolved[key] = os.environ.get(value[1:], "")
else:
resolved[key] = "" if value is None else str(value)
return resolved
def _get_sdk(self) -> tuple[type[SandboxSync], type[ConnectionConfigSync], type[RunCommandOpts]]:
with self._lock:
sdk = self._sdk

View File

@ -82,6 +82,8 @@ The legacy locations are anchored to the installed harness source, not to the ca
`scripts/doctor.py` calls `AppConfig.resolve_config_path` rather than copying this order.
Config values starting with `$` are resolved as environment variables (e.g., `$OPENAI_API_KEY`).
This happens once, at load: consumers such as sandbox providers and ACP `env` use the values verbatim,
because a resolved secret may itself start with `$` (`tests/test_sandbox_environment_passthrough.py`).
`ModelConfig` also declares `use_responses_api` and `output_version` so OpenAI `/v1/responses` can be enabled explicitly while still using `langchain_openai:ChatOpenAI`.
`ModelConfig.request_admission` is optional and is not a provider parameter.

View File

@ -15,7 +15,7 @@ class ACPAgentConfig(BaseModel):
command: str = Field(description="Command to launch the ACP agent subprocess")
args: list[str] = Field(default_factory=list, description="Additional command arguments")
env: dict[str, str] = Field(default_factory=dict, description="Environment variables to inject into the agent subprocess. Values starting with $ are resolved from host environment variables.")
env: dict[str, str] = Field(default_factory=dict, description="Environment variables to inject into the agent subprocess. Values starting with $ are resolved from host environment variables when config is loaded.")
description: str = Field(description="Description of the agent's capabilities (shown in tool description)")
model: str | None = Field(default=None, description="Model hint passed to the agent (optional)")
auto_approve_permissions: bool = Field(

View File

@ -182,9 +182,10 @@ class SandboxConfig(BaseModel):
process-local accounting.
idle_timeout: Idle timeout in seconds before released warm sandboxes/VMs are stopped (default: 600 = 10 minutes). Set to 0 to disable.
environment: Environment variables to inject into the sandbox (values starting with $ are
resolved from host env). Injected entries bypass the env-policy scrubber as
operator-authorized values; LocalSandboxProvider masks injected credentials from bash
tool output and command errors while preserving benign configuration values.
resolved from host env when config is loaded; providers use them verbatim). Injected
entries bypass the env-policy scrubber as operator-authorized values;
LocalSandboxProvider masks injected credentials from bash tool output and command
errors while preserving benign configuration values.
BoxliteProvider specific options:
health_check_skip_seconds: Optional reclaim-time skip window in seconds for recently released warm VMs. Default behavior is 0.0 = always validate before reuse.

View File

@ -265,7 +265,9 @@ def build_invoke_acp_agent_tool(agents: dict) -> BaseTool:
client = _CollectingClient()
agent_env: dict[str, str] | None = None
if agent_config.env:
agent_env = {k: (os.environ.get(v[1:], "") if v.startswith("$") else v) for k, v in agent_config.env.items()}
# $VARS in config.yaml are already resolved by AppConfig.resolve_env_variables;
# resolving again would rewrite a secret that itself starts with $.
agent_env = dict(agent_config.env)
# Resolve against the PATH the agent subprocess will see, not the Gateway's:
# the SDK merges agent_env over its inherited environment at spawn time.
cmd = await asyncio.to_thread(_resolve_agent_command, agent_config.command, _agent_path(agent_env))

View File

@ -11,6 +11,7 @@ from types import SimpleNamespace
import pytest
from deerflow.config.acp_config import ACPAgentConfig
from deerflow.config.app_config import AppConfig
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig, reset_extensions_config, set_extensions_config
from deerflow.tools.builtins.invoke_acp_agent_tool import (
_agent_path,
@ -434,7 +435,7 @@ async def test_invoke_acp_agent_uses_per_thread_workspace_when_thread_id_in_conf
@pytest.mark.anyio
async def test_invoke_acp_agent_passes_env_to_spawn(monkeypatch, tmp_path):
"""env map in ACPAgentConfig is passed to spawn_agent_process; $VAR values are resolved."""
"""env map in ACPAgentConfig reaches spawn_agent_process exactly as AppConfig resolved it."""
from deerflow.config import paths as paths_module
monkeypatch.setattr(paths_module, "get_paths", lambda: paths_module.Paths(base_dir=tmp_path))
@ -442,7 +443,8 @@ async def test_invoke_acp_agent_passes_env_to_spawn(monkeypatch, tmp_path):
"deerflow.config.extensions_config.ExtensionsConfig.from_file",
classmethod(lambda cls: ExtensionsConfig(mcp_servers={}, skills={})),
)
monkeypatch.setenv("TEST_OPENAI_KEY", "sk-from-env")
# A resolved secret that itself starts with $ must not be resolved again.
monkeypatch.setenv("TEST_OPENAI_KEY", "$2b$12$from-env")
captured: dict[str, object] = {}
@ -511,7 +513,7 @@ async def test_invoke_acp_agent_passes_env_to_spawn(monkeypatch, tmp_path):
"codex": ACPAgentConfig(
command="codex-acp",
description="Codex CLI",
env={"OPENAI_API_KEY": "$TEST_OPENAI_KEY", "FOO": "bar"},
env=AppConfig.resolve_env_variables({"OPENAI_API_KEY": "$TEST_OPENAI_KEY", "FOO": "bar"}),
)
}
)
@ -522,7 +524,7 @@ async def test_invoke_acp_agent_passes_env_to_spawn(monkeypatch, tmp_path):
sys.modules.pop("acp", None)
sys.modules.pop("acp.schema", None)
assert captured["env"] == {"OPENAI_API_KEY": "sk-from-env", "FOO": "bar"}
assert captured["env"] == {"OPENAI_API_KEY": "$2b$12$from-env", "FOO": "bar"}
@pytest.mark.anyio

View File

@ -311,8 +311,6 @@ def test_provider_defers_sdk_import_until_acquire(monkeypatch: pytest.MonkeyPatc
def test_create_passes_connection_lifetime_scope_and_environment(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture) -> None:
monkeypatch.setenv("OPEN_SANDBOX_TEST_VALUE", "resolved")
monkeypatch.delenv("OPEN_SANDBOX_ABSENT_VALUE", raising=False)
provider, sdk = _install(
monkeypatch,
config={
@ -324,11 +322,9 @@ def test_create_passes_connection_lifetime_scope_and_environment(monkeypatch: py
"ready_timeout": 18,
"sandbox_timeout": 7200,
"use_server_proxy": True,
"environment": {
"BASE": "1",
"FROM_ENV": "$OPEN_SANDBOX_TEST_VALUE",
"MISSING_ENV": "$OPEN_SANDBOX_ABSENT_VALUE",
},
# Already resolved by AppConfig at load time; a value that starts
# with $ is the secret itself, not another reference.
"environment": {"BASE": "1", "TOKEN": "$2b$12$resolved-secret"},
},
)
provider.acquire("thread-1", user_id="user-1")
@ -336,7 +332,7 @@ def test_create_passes_connection_lifetime_scope_and_environment(monkeypatch: py
assert call["image"] == "python:3.12"
assert call["timeout"] == timedelta(seconds=7200)
assert call["ready_timeout"] == timedelta(seconds=18)
assert call["env"] == {"BASE": "1", "FROM_ENV": "resolved", "MISSING_ENV": ""}
assert call["env"] == {"BASE": "1", "TOKEN": "$2b$12$resolved-secret"}
assert call["metadata"] == {
"deer_flow_provider": "opensandbox",
"deer_flow_thread": "thread-1",

View File

@ -0,0 +1,97 @@
"""``$VAR`` config values are resolved exactly once, when ``config.yaml`` loads.
``AppConfig`` replaces each ``$VAR`` reference at load time. A consumer that
resolves the result again treats a secret that itself starts with ``$`` (a
bcrypt hash, a generated password) as another reference and injects the wrong
value -- ``""``, or an unrelated host variable's value when that name exists.
These tests start from YAML text so they also fail if load-time resolution is
ever dropped for these sections.
"""
import importlib
from pathlib import Path
import pytest
from deerflow.config.app_config import AppConfig
from deerflow.config.extensions_config import ExtensionsConfig
_PROVIDERS = [
pytest.param("deerflow.community.aio_sandbox.aio_sandbox_provider", "AioSandboxProvider", id="aio"),
pytest.param("deerflow.community.e2b_sandbox.e2b_sandbox_provider", "E2BSandboxProvider", id="e2b"),
pytest.param("deerflow.community.opensandbox.provider", "OpenSandboxProvider", id="opensandbox"),
pytest.param("deerflow.community.boxlite.provider", "BoxliteProvider", id="boxlite"),
]
_RESOLVED = {
"PASSWORD_HASH": "$2b$12$abcdefghijklmnopqrstuv",
"API_SECRET": "$DEERFLOW_TEST_HOST_ONLY",
"NODE_ENV": "production",
}
@pytest.fixture
def secret_env(monkeypatch):
# One value names no host variable (blanked by a second pass), one names a
# real host variable (swapped for the host value by a second pass).
monkeypatch.setenv("DEERFLOW_TEST_HASH", _RESOLVED["PASSWORD_HASH"])
monkeypatch.setenv("DEERFLOW_TEST_SECRET", _RESOLVED["API_SECRET"])
monkeypatch.setenv("DEERFLOW_TEST_HOST_ONLY", "host-value")
def _load(monkeypatch, tmp_path: Path, yaml_text: str) -> AppConfig:
"""Run the real parse -> resolve -> validate pipeline without touching process singletons."""
monkeypatch.setattr(AppConfig, "_apply_singleton_configs", classmethod(lambda cls, config, acp_agents: None))
path = tmp_path / "config.yaml"
path.write_text(yaml_text, encoding="utf-8")
return AppConfig._from_yaml_text(yaml_text, path, extensions=ExtensionsConfig(mcp_servers={}, skills={}))
@pytest.mark.usefixtures("secret_env")
@pytest.mark.parametrize(("module_name", "class_name"), _PROVIDERS)
def test_provider_receives_sandbox_environment_resolved_once(monkeypatch, tmp_path, module_name, class_name):
app_config = _load(
monkeypatch,
tmp_path,
f"""
sandbox:
use: {module_name}:{class_name}
environment:
PASSWORD_HASH: $DEERFLOW_TEST_HASH
API_SECRET: $DEERFLOW_TEST_SECRET
NODE_ENV: production
""",
)
module = importlib.import_module(module_name)
monkeypatch.setattr(module, "get_app_config", lambda: app_config)
provider_cls = getattr(module, class_name)
provider = provider_cls.__new__(provider_cls)
loaded = provider._load_config()
assert loaded["environment"] == _RESOLVED
@pytest.mark.usefixtures("secret_env")
def test_acp_agent_env_is_resolved_once_at_load(monkeypatch, tmp_path):
# invoke_acp_agent passes this env through unchanged; see
# test_invoke_acp_agent_tool.py::test_invoke_acp_agent_passes_env_to_spawn.
app_config = _load(
monkeypatch,
tmp_path,
"""
sandbox:
use: deerflow.sandbox.local:LocalSandboxProvider
acp_agents:
codex:
command: codex-acp
description: Codex CLI
env:
PASSWORD_HASH: $DEERFLOW_TEST_HASH
API_SECRET: $DEERFLOW_TEST_SECRET
NODE_ENV: production
""",
)
assert app_config.acp_agents["codex"].env == _RESOLVED