From ae871431a2e38febba5567c37a27b644febbd7d4 Mon Sep 17 00:00:00 2001 From: EddieMurphy <142191175+EddieMurphy89@users.noreply.github.com> Date: Fri, 13 Nov 2026 20:41:27 +0800 Subject: [PATCH] fix(sandbox): sanitize config-failure logs and map archive errors to 503 (#8185) --- backend/app/gateway/routers/thread_runs.py | 6 +- .../harness/deerflow/authz/sandbox_authz.py | 62 +++++++++++++--- backend/tests/conftest.py | 16 +++++ backend/tests/test_artifact_archive.py | 22 ++++++ backend/tests/test_sandbox_authorization.py | 71 +++++++++++++++++++ 5 files changed, 165 insertions(+), 12 deletions(-) diff --git a/backend/app/gateway/routers/thread_runs.py b/backend/app/gateway/routers/thread_runs.py index e023b9bcaa..2825baf50d 100644 --- a/backend/app/gateway/routers/thread_runs.py +++ b/backend/app/gateway/routers/thread_runs.py @@ -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() diff --git a/backend/packages/harness/deerflow/authz/sandbox_authz.py b/backend/packages/harness/deerflow/authz/sandbox_authz.py index 8a951149f7..18b4347b0e 100644 --- a/backend/packages/harness/deerflow/authz/sandbox_authz.py +++ b/backend/packages/harness/deerflow/authz/sandbox_authz.py @@ -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.""" diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 012273b938..dc8aec3f8a 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -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.""" diff --git a/backend/tests/test_artifact_archive.py b/backend/tests/test_artifact_archive.py index 32d28bccbf..bd0977bdad 100644 --- a/backend/tests/test_artifact_archive.py +++ b/backend/tests/test_artifact_archive.py @@ -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() diff --git a/backend/tests/test_sandbox_authorization.py b/backend/tests/test_sandbox_authorization.py index 39a6618787..e4520d7b80 100644 --- a/backend/tests/test_sandbox_authorization.py +++ b/backend/tests/test_sandbox_authorization.py @@ -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.