diff --git a/backend/packages/harness/deerflow/config/skill_evolution_config.py b/backend/packages/harness/deerflow/config/skill_evolution_config.py index 056117f6c..9e3ce32b0 100644 --- a/backend/packages/harness/deerflow/config/skill_evolution_config.py +++ b/backend/packages/harness/deerflow/config/skill_evolution_config.py @@ -12,3 +12,7 @@ class SkillEvolutionConfig(BaseModel): default=None, description="Optional model name for skill security moderation. Defaults to the primary chat model.", ) + security_fail_closed: bool = Field( + default=True, + description=("When the moderation model is unavailable, block skill writes if True (fail-closed). If False, non-executable content is allowed with a warning while executable content is still blocked."), + ) diff --git a/backend/packages/harness/deerflow/skills/security_scanner.py b/backend/packages/harness/deerflow/skills/security_scanner.py index ad8d97aca..b38145399 100644 --- a/backend/packages/harness/deerflow/skills/security_scanner.py +++ b/backend/packages/harness/deerflow/skills/security_scanner.py @@ -22,6 +22,15 @@ class ScanResult: reason: str +def _resolve_fail_closed(app_config: AppConfig | None) -> bool: + """Resolve the fail-closed policy, defaulting to True if config is unavailable.""" + try: + config = app_config or get_app_config() + return bool(getattr(config.skill_evolution, "security_fail_closed", True)) + except Exception: + return True + + def _extract_json_object(raw: str) -> dict | None: raw = raw.strip() @@ -120,10 +129,13 @@ async def scan_skill_content( return ScanResult(decision, str(parsed.get("reason") or "No reason provided.")) logger.warning("Security scan produced unparseable output: %s", raw[:200]) except Exception: - logger.warning("Skill security scan model call failed; using conservative fallback", exc_info=True) + logger.warning("Skill security scan model call failed; applying configured fail-closed/fail-open policy", exc_info=True) if model_responded: return ScanResult("block", "Security scan produced unparseable output; manual review required.") if executable: return ScanResult("block", "Security scan unavailable for executable content; manual review required.") - return ScanResult("block", "Security scan unavailable for skill content; manual review required.") + if _resolve_fail_closed(app_config): + return ScanResult("block", "Security scan unavailable for skill content; manual review required.") + logger.warning("Security scan unavailable; failing open for non-executable skill content at %s (manual review recommended)", location) + return ScanResult("warn", "Security scan unavailable for non-executable skill content; manual review recommended.") diff --git a/backend/tests/test_config_version.py b/backend/tests/test_config_version.py index 916b9385c..cf1caafe7 100644 --- a/backend/tests/test_config_version.py +++ b/backend/tests/test_config_version.py @@ -123,3 +123,68 @@ def test_newer_user_version_no_warning(caplog): config_path, ) assert "outdated" not in caplog.text + + +def _load_repo_example() -> dict: + """Load the real repo config.example.yaml (first-run template).""" + example_path = Path(__file__).resolve().parents[2] / "config.example.yaml" + with open(example_path, encoding="utf-8") as f: + return yaml.safe_load(f) or {} + + +def _merge_missing(target: dict, source: dict) -> None: + """Add-missing-keys-only recursive merge mirroring scripts/config-upgrade.sh.""" + for key, value in source.items(): + if key not in target: + import copy + + target[key] = copy.deepcopy(value) + elif isinstance(value, dict) and isinstance(target[key], dict): + _merge_missing(target[key], value) + + +def test_security_fail_closed_bumped_config_version(): + """The example must ship security_fail_closed under a version > 26 so v26 configs upgrade.""" + example = _load_repo_example() + assert example.get("config_version", 0) >= 27 + assert example["skill_evolution"]["security_fail_closed"] is True + + +def test_version_26_config_reported_outdated_against_example(caplog): + """A version-26 user config is flagged outdated against the real example version.""" + example = _load_repo_example() + example_version = example["config_version"] + with tempfile.TemporaryDirectory() as tmpdir: + config_path = _make_config_files( + Path(tmpdir), + user_config={"config_version": 26}, + example_config=example, + ) + with caplog.at_level(logging.WARNING, logger="deerflow.config.app_config"): + AppConfig._check_config_version({"config_version": 26}, config_path) + assert "outdated" in caplog.text + assert "version 26" in caplog.text + assert f"version is {example_version}" in caplog.text + + +def test_config_upgrade_adds_security_fail_closed_preserving_user_values(): + """config-upgrade merges security_fail_closed: true without touching existing skill_evolution values.""" + example = _load_repo_example() + # A version-26 user who customized skill_evolution but predates the new field. + user = { + "config_version": 26, + "skill_evolution": { + "enabled": True, + "moderation_model_name": "custom-moderation-model", + }, + } + + _merge_missing(user, example) + user["config_version"] = example["config_version"] + + # New persisted field is merged in with the example's fail-closed default. + assert user["skill_evolution"]["security_fail_closed"] is True + # The user's existing skill_evolution values are preserved unchanged. + assert user["skill_evolution"]["enabled"] is True + assert user["skill_evolution"]["moderation_model_name"] == "custom-moderation-model" + assert user["config_version"] == example["config_version"] diff --git a/backend/tests/test_security_scanner.py b/backend/tests/test_security_scanner.py index 61277efd8..9265f8a37 100644 --- a/backend/tests/test_security_scanner.py +++ b/backend/tests/test_security_scanner.py @@ -1,3 +1,4 @@ +import logging from types import SimpleNamespace import pytest @@ -139,3 +140,50 @@ async def test_scan_distinguishes_unparseable_executable(monkeypatch): # Even for executable content, unparseable uses the unparseable message assert result.decision == "block" assert "unparseable" in result.reason + + +def _make_unavailable_env(monkeypatch, *, security_fail_closed): + config = SimpleNamespace( + skill_evolution=SimpleNamespace( + moderation_model_name=None, + security_fail_closed=security_fail_closed, + ) + ) + monkeypatch.setattr("deerflow.skills.security_scanner.get_app_config", lambda: config) + monkeypatch.setattr( + "deerflow.skills.security_scanner.create_chat_model", + lambda **kwargs: (_ for _ in ()).throw(RuntimeError("boom")), + ) + + +@pytest.mark.anyio +async def test_fail_open_allows_non_executable_when_model_unavailable(monkeypatch): + _make_unavailable_env(monkeypatch, security_fail_closed=False) + result = await scan_skill_content(SKILL_CONTENT, executable=False) + assert result.decision == "warn" + assert "unavailable" in result.reason + + +@pytest.mark.anyio +async def test_fail_open_still_blocks_executable_when_model_unavailable(monkeypatch): + _make_unavailable_env(monkeypatch, security_fail_closed=False) + result = await scan_skill_content(SKILL_CONTENT, executable=True) + assert result.decision == "block" + assert "executable" in result.reason + + +@pytest.mark.anyio +async def test_fail_closed_blocks_non_executable_when_model_unavailable(monkeypatch): + _make_unavailable_env(monkeypatch, security_fail_closed=True) + result = await scan_skill_content(SKILL_CONTENT, executable=False) + assert result.decision == "block" + assert "unavailable" in result.reason + + +@pytest.mark.anyio +async def test_fail_open_logs_operator_visible_warning(monkeypatch, caplog): + _make_unavailable_env(monkeypatch, security_fail_closed=False) + with caplog.at_level(logging.WARNING, logger="deerflow.skills.security_scanner"): + result = await scan_skill_content(SKILL_CONTENT, executable=False) + assert result.decision == "warn" + assert "failing open" in caplog.text diff --git a/config.example.yaml b/config.example.yaml index 53759c105..3848691ee 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -15,7 +15,7 @@ # ============================================================================ # Bump this number when the config schema changes. # Run `make config-upgrade` to merge new fields into your local config.yaml. -config_version: 26 +config_version: 27 # ============================================================================ # Logging @@ -1577,6 +1577,7 @@ agents_api: skill_evolution: enabled: false # Set to true to allow agent-managed writes under skills/custom moderation_model_name: null # Model for LLM-based security scanning (null = use default model) + security_fail_closed: true # Moderation model unavailable: true blocks all writes; false allows non-executable content with a warning (executable is always blocked) # ============================================================================ # Checkpointer Configuration (DEPRECATED — use `database` instead) diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index 19c1320b6..741c2f0d3 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -124,7 +124,7 @@ they resolve from the `secrets` map): ```yaml config: | - config_version: 26 + config_version: 27 models: - name: gpt-4 use: langchain_openai:ChatOpenAI diff --git a/deploy/helm/deer-flow/values.yaml b/deploy/helm/deer-flow/values.yaml index 80ed249b5..5a3762bd4 100644 --- a/deploy/helm/deer-flow/values.yaml +++ b/deploy/helm/deer-flow/values.yaml @@ -240,7 +240,7 @@ ingress: # -- DeerFlow config.yaml content. Secrets MUST stay as $VAR references — never # inline literal secret values here. The default enables provisioner sandbox. config: | - config_version: 26 + config_version: 27 log_level: info models: []