mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-15 07:41:20 +00:00
fix(sandbox): sanitize config-failure logs and map archive errors to 503 (#8185)
This commit is contained in:
parent
c263b887b5
commit
ae871431a2
@ -78,6 +78,7 @@ from deerflow.runtime.goal import is_active_goal
|
||||
from deerflow.runtime.runs.store.base import format_run_cursor_created_at, normalize_run_created_at_iso
|
||||
from deerflow.runtime.secret_context import redact_config_secrets, redact_metadata_secrets
|
||||
from deerflow.runtime.user_context import get_effective_user_id
|
||||
from deerflow.sandbox.exceptions import SandboxAuthorizationError
|
||||
from deerflow.utils.llm_text import strip_leading_think_blocks
|
||||
from deerflow.utils.messages import ORIGINAL_USER_CONTENT_KEY, get_original_user_content_text, message_to_text
|
||||
from deerflow.utils.thread_id import ThreadId
|
||||
@ -1881,7 +1882,10 @@ async def create_run_artifact_archive(
|
||||
|
||||
raw_owner_user_id = get_trusted_internal_owner_user_id(request)
|
||||
effective_user_id = make_safe_user_id(raw_owner_user_id) if raw_owner_user_id else get_effective_user_id()
|
||||
app_config = await safe_app_config_async()
|
||||
try:
|
||||
app_config = await safe_app_config_async()
|
||||
except SandboxAuthorizationError as exc:
|
||||
raise HTTPException(status_code=503, detail="Artifact archive is temporarily unavailable") from exc
|
||||
custom_tool_output_dir = getattr(getattr(app_config, "tool_output", None), "storage_subdir", None)
|
||||
extra_reserved_dir_names = {custom_tool_output_dir} if isinstance(custom_tool_output_dir, str) else set()
|
||||
paths = get_paths()
|
||||
|
||||
@ -22,29 +22,69 @@ from typing import Any
|
||||
from deerflow.authz.principal import build_principal_from_context
|
||||
from deerflow.authz.provider import AuthorizationProvider, AuthzDecision, AuthzRequest, Principal
|
||||
from deerflow.authz.runtime import construct_authorization_provider, resolve_authorization_provider, resolve_authorization_provider_spec
|
||||
from deerflow.config.app_config import AppConfig
|
||||
from deerflow.config.app_config import AppConfig, peek_loaded_app_config
|
||||
from deerflow.sandbox.exceptions import SandboxAuthorizationError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def safe_app_config() -> AppConfig | None:
|
||||
"""Load the global AppConfig, returning None when unavailable.
|
||||
_authorization_config_loaded = False
|
||||
|
||||
Authorization can only be enabled via config, so no readable config ⇒ the
|
||||
sandbox gate is a no-op (``authorize_sandbox_execution`` treats a ``None``
|
||||
app_config the same as ``authorization.enabled: false``). This keeps the
|
||||
gate safe in environments without a ``config.yaml`` (e.g. CI runners and
|
||||
direct-call tests) where ``get_app_config`` would raise ``FileNotFoundError``.
|
||||
|
||||
def _authorization_enabled(config: AppConfig | None) -> bool:
|
||||
authz_config = getattr(config, "authorization", None)
|
||||
return getattr(authz_config, "enabled", None) is True
|
||||
|
||||
|
||||
def safe_app_config() -> AppConfig | None:
|
||||
"""Load the global AppConfig, returning None only before authorization is configured.
|
||||
|
||||
``None`` is reserved for a process that has never loaded an
|
||||
authorization-enabled configuration (CI runners and direct-call tests).
|
||||
Once authorization has been enabled, a later config read failure means the
|
||||
policy is unknown, not absent, so the sandbox gate fails closed instead of
|
||||
silently no-oping.
|
||||
|
||||
This fail-closed decision is intentionally unconditional: when the config
|
||||
itself cannot be read, the ``authorization.fail_closed`` value is part of
|
||||
the unavailable policy and cannot be trusted to permit the call. On a read
|
||||
failure we also consult ``peek_loaded_app_config()``, because gateway
|
||||
startup and upload paths can load an authorization-enabled config through
|
||||
the ordinary loader before this helper is ever called. The error
|
||||
deliberately says the sandbox is temporarily unavailable rather than
|
||||
claiming a role denial; the cause is logged.
|
||||
"""
|
||||
global _authorization_config_loaded
|
||||
|
||||
try:
|
||||
from deerflow.config import get_app_config
|
||||
|
||||
return get_app_config()
|
||||
except Exception:
|
||||
logger.debug("App config unavailable; sandbox:execute gate is a no-op", exc_info=True)
|
||||
config = get_app_config()
|
||||
except Exception as exc:
|
||||
if not _authorization_config_loaded:
|
||||
try:
|
||||
loaded_config = peek_loaded_app_config()
|
||||
except Exception:
|
||||
loaded_config = None
|
||||
if _authorization_enabled(loaded_config):
|
||||
_authorization_config_loaded = True
|
||||
|
||||
if _authorization_config_loaded:
|
||||
logger.warning(
|
||||
"Authorization config became unavailable; denying sandbox:execute (%s)",
|
||||
type(exc).__name__,
|
||||
)
|
||||
raise SandboxAuthorizationError("Sandbox execution is temporarily unavailable") from None
|
||||
logger.debug(
|
||||
"App config unavailable; sandbox:execute gate is a no-op (%s)",
|
||||
type(exc).__name__,
|
||||
)
|
||||
return None
|
||||
|
||||
if _authorization_enabled(config):
|
||||
_authorization_config_loaded = True
|
||||
return config
|
||||
|
||||
|
||||
async def safe_app_config_async() -> AppConfig | None:
|
||||
"""Load AppConfig without running config file I/O on the event loop."""
|
||||
|
||||
@ -78,6 +78,22 @@ def provisioner_module():
|
||||
# context should mark themselves ``@pytest.mark.no_auto_user``.
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_sandbox_authz_config_state():
|
||||
"""Reset the process-global authorization-config flag between tests.
|
||||
|
||||
Several suites exercise the real ``safe_app_config()`` with an
|
||||
authorization-enabled config, while config-less suites rely on its no-op
|
||||
branch. Reset the flag for every test so collection order and pytest shard
|
||||
ordering cannot leak authorization state across modules.
|
||||
"""
|
||||
from deerflow.authz import sandbox_authz
|
||||
|
||||
sandbox_authz._authorization_config_loaded = False
|
||||
yield
|
||||
sandbox_authz._authorization_config_loaded = False
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_skill_storage_singleton():
|
||||
"""Reset the SkillStorage singleton between tests to prevent cross-test contamination."""
|
||||
|
||||
@ -18,6 +18,7 @@ from app.gateway.routers import thread_runs
|
||||
from deerflow.runtime.events.store.memory import MemoryRunEventStore
|
||||
from deerflow.runtime.runs.manager import RunManager
|
||||
from deerflow.runtime.runs.store.memory import MemoryRunStore
|
||||
from deerflow.sandbox.exceptions import SandboxAuthorizationError
|
||||
|
||||
THREAD_ID = "thread-archive"
|
||||
RUN_ID = "run-archive"
|
||||
@ -318,6 +319,27 @@ def test_archive_hides_a_run_from_another_thread(tmp_path, monkeypatch, method:
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
def test_archive_returns_503_when_sandbox_config_is_unavailable(tmp_path, monkeypatch) -> None:
|
||||
outputs = tmp_path / "outputs"
|
||||
outputs.mkdir()
|
||||
client, _, _ = _archive_app(
|
||||
monkeypatch,
|
||||
outputs,
|
||||
paths=["/mnt/user-data/outputs/report.txt"],
|
||||
)
|
||||
|
||||
async def unavailable():
|
||||
raise SandboxAuthorizationError("Sandbox execution is temporarily unavailable")
|
||||
|
||||
monkeypatch.setattr(thread_runs, "safe_app_config_async", unavailable)
|
||||
|
||||
with client:
|
||||
response = client.post(ARCHIVE_URL)
|
||||
|
||||
assert response.status_code == 503
|
||||
assert response.json()["detail"] == "Artifact archive is temporarily unavailable"
|
||||
|
||||
|
||||
def test_archive_conflicts_with_an_active_run(tmp_path, monkeypatch) -> None:
|
||||
outputs = tmp_path / "outputs"
|
||||
outputs.mkdir()
|
||||
|
||||
@ -654,6 +654,77 @@ def test_artifact_sandbox_sync_skipped_when_denied(monkeypatch, tmp_path):
|
||||
assert artifact_path.read_bytes() == b"after"
|
||||
|
||||
|
||||
def test_safe_app_config_denies_after_authz_config_becomes_unreadable(monkeypatch):
|
||||
"""A config that was authorization-enabled must fail closed after becoming unreadable."""
|
||||
from deerflow.authz import sandbox_authz as mod
|
||||
|
||||
app_config = _make_app_config()
|
||||
_enable_authz(app_config)
|
||||
monkeypatch.setattr("deerflow.config.get_app_config", lambda: app_config)
|
||||
assert mod.safe_app_config() is app_config
|
||||
|
||||
def _unreadable():
|
||||
raise FileNotFoundError("config.yaml was removed after startup")
|
||||
|
||||
monkeypatch.setattr("deerflow.config.get_app_config", _unreadable)
|
||||
with pytest.raises(SandboxAuthorizationError) as exc_info:
|
||||
mod.safe_app_config()
|
||||
assert "temporarily unavailable" in str(exc_info.value)
|
||||
|
||||
|
||||
def test_safe_app_config_still_denies_with_fail_closed_false(monkeypatch):
|
||||
"""The unavailable-policy error is intentional even if the old policy said fail_open."""
|
||||
from deerflow.authz import sandbox_authz as mod
|
||||
|
||||
app_config = _make_app_config()
|
||||
_enable_authz(app_config, fail_closed=False)
|
||||
monkeypatch.setattr("deerflow.config.get_app_config", lambda: app_config)
|
||||
assert mod.safe_app_config() is app_config
|
||||
|
||||
monkeypatch.setattr(
|
||||
"deerflow.config.get_app_config",
|
||||
lambda: (_ for _ in ()).throw(FileNotFoundError("config.yaml removed")),
|
||||
)
|
||||
with pytest.raises(SandboxAuthorizationError):
|
||||
mod.safe_app_config()
|
||||
|
||||
|
||||
def test_safe_app_config_uses_loaded_config_before_first_helper_call(monkeypatch):
|
||||
"""A config loaded by the ordinary loader must arm the guard before the first helper call."""
|
||||
from deerflow.authz import sandbox_authz as mod
|
||||
|
||||
app_config = _make_app_config()
|
||||
_enable_authz(app_config)
|
||||
monkeypatch.setattr("deerflow.authz.sandbox_authz.peek_loaded_app_config", lambda: app_config)
|
||||
monkeypatch.setattr(
|
||||
"deerflow.config.get_app_config",
|
||||
lambda: (_ for _ in ()).throw(FileNotFoundError("config.yaml removed")),
|
||||
)
|
||||
|
||||
with pytest.raises(SandboxAuthorizationError) as exc_info:
|
||||
mod.safe_app_config()
|
||||
assert "temporarily unavailable" in str(exc_info.value)
|
||||
|
||||
|
||||
def test_safe_app_config_does_not_log_config_exception_details(monkeypatch, caplog):
|
||||
"""The deny warning records the exception type, not configuration values."""
|
||||
from deerflow.authz import sandbox_authz as mod
|
||||
|
||||
mod._authorization_config_loaded = True
|
||||
secret = "token=SUPERSECRET"
|
||||
|
||||
def _unreadable():
|
||||
raise ValueError(secret)
|
||||
|
||||
monkeypatch.setattr("deerflow.config.get_app_config", _unreadable)
|
||||
with caplog.at_level("WARNING", logger=mod.__name__):
|
||||
with pytest.raises(SandboxAuthorizationError):
|
||||
mod.safe_app_config()
|
||||
|
||||
assert secret not in caplog.text
|
||||
assert "ValueError" in caplog.text
|
||||
|
||||
|
||||
def test_authorize_sandbox_no_config_file_is_noop(monkeypatch):
|
||||
"""No readable config.yaml (CI environments) → the gate is a no-op.
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user