fix(skills): keep disabled skills disabled when the extensions config cannot be read (#7705)

* fix(skills): fail closed when the extensions config cannot be read

* test(skills): cover a disabled skill when the extensions config cannot be read

* style(tests): format the fail-closed regression for ruff

* style(tests): fit the docstring and signature inside the 240-column limit

* fix(skills): fail closed in the user-scoped listing when the config cannot be read

* test(skills): cover the user-scoped listing with an unreadable extensions config

* fix(gateway): keep skill read surfaces loud about an unreadable extensions config

Folding the user-scoped degraded mode in (#7704) also stopped the skill
read endpoints from failing on a config that cannot be read: the storage
now answers with an all-disabled listing, which regressed the #7155
contract (a generic 500 with the host path only in the server log) into
a 200 that presents a broken deployment as a legitimately empty one.

Keep both properties: the storage still fails closed for the runtime
gate, and the read surfaces re-validate the config explicitly so the
raised error reaches each handler's generic 500 branch unchanged.
First-run behavior is untouched: no config file resolves to an empty
config without raising, so default-enabled still applies.

* fix(skills): redact config-load failures and keep the read preflight off the event loop

Review follow-ups:

- Log the failure shape instead of the exception text in both degraded
  modes: from_file resolves $VAR placeholders before validation, so its
  message can embed resolved credentials. Reuse _describe_load_failure,
  the loader's own redaction helper.
- Run the gateway read preflight through asyncio.to_thread in the
  capability adapter, matching the scan it precedes.
- Cover the user-scoped degraded branch with a CUSTOM skill that has
  per-user state on disk, so the second read's guard is directly
  regression-tested (verified red with the guard removed).

---------

Co-authored-by: Jay0130-a <309071492+Jay0130-a@users.noreply.github.com>

backend/app/gateway/capabilities.py

@@ -194,6 +194,11 @@ class LarkAdapter:
 
 class SkillAdapter:
     async def list_installations(self, context: AdapterContext) -> list[CapabilityInstallation]:
+        # The listing reports the enabled flag, which the storage layer fails
+        # closed on an unreadable extensions config; keep this read surface
+        # loud about the broken config instead of serving the degraded list.
+        # Blocking filesystem IO: always off the event loop, like the scan below.
+        await asyncio.to_thread(skills._assert_extensions_config_readable)
         response = await asyncio.to_thread(lambda: skills._get_user_skill_storage(context.config).load_skills(enabled_only=False))
         response = await skills._filter_visible_skills(context.request, context.config, response)
         return [

backend/app/gateway/routers/skills.py

@@ -338,6 +338,20 @@ async def _publish_skills_cache_reset(user_id: str | None) -> str | None:
     return await asyncio.to_thread(publish_skills_cache_reset, user_id=user_id)
 
 
+def _assert_extensions_config_readable() -> None:
+    """Raise when the extensions config exists but cannot be read.
+
+    The storage layer fails closed on an unreadable config (every skill stays
+    listed, none is enabled), which keeps the runtime activation gate safe
+    without taking the listing down. The read surface must stay loud instead:
+    the ``enabled`` column it reports is derived from this config, so serving
+    a degraded list would present a broken deployment as a legitimately empty
+    one. The raised error reaches each handler's generic 500 branch, which
+    keeps the host path in the server log and out of the response body.
+    """
+    ExtensionsConfig.from_file()
+
+
 def _load_user_skills(config: AppConfig) -> list[Skill]:
     """Load every skill visible to the caller's storage, enabled or not.
 
@@ -347,6 +361,7 @@ def _load_user_skills(config: AppConfig) -> list[Skill]:
     each ``SKILL.md`` -- work that scales with the number of installed skills
     (#5747 moved the same call off the loop for ``get_custom_skill``).
     """
+    _assert_extensions_config_readable()
     return _get_user_skill_storage(config).load_skills(enabled_only=False)
 
 

backend/packages/harness/deerflow/skills/storage/skill_storage.py

@@ -372,12 +372,24 @@ class SkillStorage(ABC):
         # to enabled when no explicit config entry exists (so newly
         # installed skills appear active without requiring a manual toggle).
         try:
-            from deerflow.config.extensions_config import ExtensionsConfig
+            from deerflow.config.extensions_config import ExtensionsConfig, _describe_load_failure
 
             extensions_config = ExtensionsConfig.from_file()
             skills = [dataclasses.replace(s, enabled=extensions_config.is_skill_enabled(s.name, s.category)) for s in skills]
         except Exception as e:
-            logger.warning("Failed to load extensions config: %s", e)
+            # A config that cannot be read says nothing about which skills are
+            # enabled, so the parsed default (``enabled=True``) must not stand
+            # in for an answer: that would turn every disabled skill back on
+            # because of an unrelated defect elsewhere in the file (a malformed
+            # ``mcpServers`` entry, a trailing comma). Fail closed instead —
+            # the skills stay listed, they just cannot be activated until the
+            # config is readable again. The sandbox projection already fails
+            # closed on the same error (``projection._rebuild_public_locked``);
+            # this keeps the runtime half of the gate consistent with it.
+            # ``from_file`` resolves ``$VAR`` placeholders before validation, so
+            # only the failure shape is safe to log (see ``_describe_load_failure``).
+            logger.warning("Failed to load extensions config; treating every skill as disabled (%s)", _describe_load_failure(e))
+            skills = [dataclasses.replace(s, enabled=False) for s in skills]
 
         if enabled_only:
             skills = [s for s in skills if s.enabled]

backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py

@@ -217,15 +217,27 @@ class UserScopedSkillStorage(LocalSkillStorage):
         # extensions_config (handled by ``super().load_skills`` above). Re-read
         # from disk here too so another worker's update cannot be masked by
         # this process's singleton cache while rebuilding a user projection.
-        from deerflow.config.extensions_config import ExtensionsConfig
+        from deerflow.config.extensions_config import ExtensionsConfig, _describe_load_failure
 
-        extensions_config = ExtensionsConfig.from_file()
-        skills = [
-            dataclasses.replace(s, enabled=self.get_skill_enabled_state(s.name) and extensions_config.is_skill_enabled(s.name, s.category.value if hasattr(s.category, "value") else s.category))
-            if dataclasses.is_dataclass(s) and not isinstance(s, type) and (s.category.value if hasattr(s.category, "value") else s.category) != SkillCategory.PUBLIC.value
-            else s
-            for s in skills
-        ]
+        try:
+            extensions_config = ExtensionsConfig.from_file()
+            skills = [
+                dataclasses.replace(s, enabled=self.get_skill_enabled_state(s.name) and extensions_config.is_skill_enabled(s.name, s.category.value if hasattr(s.category, "value") else s.category))
+                if dataclasses.is_dataclass(s) and not isinstance(s, type) and (s.category.value if hasattr(s.category, "value") else s.category) != SkillCategory.PUBLIC.value
+                else s
+                for s in skills
+            ]
+        except Exception as e:
+            # Same degraded mode as ``LocalSkillStorage.load_skills``: a config
+            # that cannot be read says nothing about which skills are enabled,
+            # so the per-user AND cannot be evaluated. Raising here would take
+            # the whole user-scoped listing down over an unrelated defect in the
+            # file, so fail closed instead — every skill is listed, none is
+            # enabled. ``from_file`` resolves ``$VAR`` placeholders before
+            # validation, so only the failure shape is safe to log (see
+            # ``_describe_load_failure``).
+            logger.warning("Failed to load extensions config; treating every user-scoped skill as disabled (%s)", _describe_load_failure(e))
+            skills = [dataclasses.replace(s, enabled=False) for s in skills]
 
         if enabled_only:
             skills = [s for s in skills if s.enabled]

backend/tests/test_skills_disabled_fail_closed.py

@@ -0,0 +1,68 @@
+"""A broken extensions config must not re-enable a disabled skill.
+
+``SkillStorage.load_skills()`` merges the enabled state from ``extensions_config.json``. When that file
+cannot be read, the parsed default (``enabled=True`` from ``parse_skill_file``) must not stand in for the
+answer: an unrelated defect in the file — a malformed ``mcpServers`` entry, a trailing comma — would
+otherwise silently re-enable every skill an operator disabled.
+
+The sandbox projection fails closed on the same error (``projection._rebuild_public_locked`` clears the
+view), so the runtime half of the gate has to agree with it.
+"""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+
+import pytest
+
+from deerflow.config.extensions_config import ExtensionsConfig, reset_extensions_config
+from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
+
+_SKILL_MD = """---
+name: {name}
+description: {name} description
+---
+body
+"""
+
+
+def _write_public_skill(root: Path, name: str) -> None:
+    skill_dir = root / "public" / name
+    skill_dir.mkdir(parents=True)
+    (skill_dir / "SKILL.md").write_text(_SKILL_MD.format(name=name), encoding="utf-8")
+
+
+@pytest.fixture()
+def disabled_skill_root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
+    """A skills root whose only skill is disabled in the extensions config."""
+    root = tmp_path / "skills"
+    _write_public_skill(root, "demo")
+    config_path = tmp_path / "extensions_config.json"
+    config_path.write_text(json.dumps({"mcpServers": {}, "skills": {"demo": {"enabled": False}}}), encoding="utf-8")
+    monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(config_path))
+    reset_extensions_config()
+    return root
+
+
+def test_disabled_skill_is_disabled_while_the_config_loads(disabled_skill_root: Path) -> None:
+    storage = LocalSkillStorage(str(disabled_skill_root))
+
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False)]
+    assert storage.load_skills(enabled_only=True) == []
+
+
+def test_disabled_skill_stays_disabled_when_the_config_cannot_be_read(disabled_skill_root: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    storage = LocalSkillStorage(str(disabled_skill_root))
+
+    def _raise(*_args: object, **_kwargs: object) -> None:
+        # What one schema-invalid ``mcpServers`` entry raises from ``McpServerConfig._validate_header_names``;
+        # a malformed JSON file raises ``ValueError`` from the same call site.
+        raise ValueError("headers maps the same HTTP header under two spellings ('X-Api-Key' and 'x-api-key')")
+
+    monkeypatch.setattr(ExtensionsConfig, "from_file", classmethod(_raise))
+
+    # Still listed, so the skill does not silently disappear from the UI...
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False)]
+    # ...and never handed to the model as usable.
+    assert storage.load_skills(enabled_only=True) == []

backend/tests/test_user_scoped_skills_fail_closed.py

@@ -0,0 +1,91 @@
+"""A broken extensions config must not take down a user-scoped skill listing.
+
+``UserScopedSkillStorage._load_skills_unlocked`` re-reads the extensions config to AND the per-user state
+with the global default. That second read is unguarded, so a config that cannot be read raised out of
+``load_skills()`` for user-scoped deployments even after the local/global path learned to degrade.
+"""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+
+import pytest
+
+from deerflow.config.extensions_config import ExtensionsConfig, reset_extensions_config
+from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
+
+_SKILL_MD = """---
+name: {name}
+description: {name} description
+---
+body
+"""
+
+
+def _raise_unreadable_config(*_args: object, **_kwargs: object) -> None:
+    # What one schema-invalid ``mcpServers`` entry raises from
+    # ``McpServerConfig._validate_header_names``; a malformed JSON file raises
+    # ``ValueError`` from the same call site.
+    raise ValueError("headers maps the same HTTP header under two spellings ('X-Api-Key' and 'x-api-key')")
+
+
+@pytest.fixture()
+def user_skills_root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
+    """A skills root with one public skill and a readable extensions config."""
+    root = tmp_path / "skills"
+    public = root / "public" / "demo"
+    public.mkdir(parents=True)
+    (public / "SKILL.md").write_text(_SKILL_MD.format(name="demo"), encoding="utf-8")
+    config_path = tmp_path / "extensions_config.json"
+    config_path.write_text(json.dumps({"mcpServers": {}, "skills": {}}), encoding="utf-8")
+    monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(config_path))
+    reset_extensions_config()
+    return root
+
+
+def test_user_scoped_listing_degrades_when_the_config_cannot_be_read(user_skills_root: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    storage = UserScopedSkillStorage("owner", host_path=str(user_skills_root))
+
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", True)]
+
+    monkeypatch.setattr(ExtensionsConfig, "from_file", classmethod(_raise_unreadable_config))
+
+    # The listing survives: no exception, every skill present and disabled.
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False)]
+    assert storage.load_skills(enabled_only=True) == []
+
+
+def test_user_scoped_custom_skill_with_per_user_state_degrades_when_the_config_cannot_be_read(
+    user_skills_root: Path,
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """CUSTOM/LEGACY skills are the second read's own responsibility.
+
+    Their per-user state exists only to be ANDed with the global default, so
+    the degraded branch has to cover them too: with a per-user state present,
+    an unreadable config still lists the custom skill — disabled — instead of
+    raising out of ``load_skills()``. Dropping the guard turns this red: the
+    second read raises out of the listing.
+    """
+    from deerflow.config.paths import Paths
+
+    # Route the user custom root (and the per-user states file) into tmp.
+    monkeypatch.setattr("deerflow.config.paths._paths", Paths(base_dir=tmp_path / "home"))
+    custom_dir = tmp_path / "home" / "users" / "owner" / "skills" / "custom" / "my-tool"
+    custom_dir.mkdir(parents=True)
+    (custom_dir / "SKILL.md").write_text(_SKILL_MD.format(name="my-tool"), encoding="utf-8")
+    states_file = tmp_path / "home" / "users" / "owner" / "skills" / "_skill_states.json"
+    states_file.write_text(json.dumps({"my-tool": {"enabled": True}}), encoding="utf-8")
+
+    storage = UserScopedSkillStorage("owner", host_path=str(user_skills_root))
+
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", True), ("my-tool", True)]
+
+    monkeypatch.setattr(ExtensionsConfig, "from_file", classmethod(_raise_unreadable_config))
+
+    # The listing survives: every skill present (per-user state on disk for
+    # the custom one), none enabled, no exception.
+    assert [(skill.name, skill.enabled) for skill in storage.load_skills()] == [("demo", False), ("my-tool", False)]
+    assert storage.load_skills(enabled_only=True) == []