fix(skills): keep load error text out of user-facing skill read 500s (#7155)

* fix(skills): keep load error text out of user-facing skill read 500s

GET /api/skills, /api/skills/custom and /api/skills/{name} are open to
every signed-in user, but their 500 detail echoed str(e). A malformed or
unreadable extensions_config.json (re-read on every load by the
user-scoped storage) made all three return the file's absolute host path,
plus the [Errno 13] text for a permission error.

Return a fixed detail like the diagnostics and reload routes already do,
chain the cause, and keep the full error in the Gateway log.

* docs(changelog): reference #7155 for the skill read 500 detail fix

* test(skills): pin capability skill discovery 500 through the real Gateway app

GET /api/capabilities/installations/skills reads the same user-scoped
storage as the skill routes but has no handler of its own. Its 500 body
is TraceMiddleware's fixed fallback, so drive create_app() with a
malformed extensions_config.json and assert no host path reaches the
body on any of the four reads.
This commit is contained in:
Hyeonsang Cho 2026-10-20 23:51:02 +09:00 committed by GitHub
parent 5cef1a11ef
commit 19e861d0c3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 100 additions and 5 deletions

View File

@ -6111,6 +6111,14 @@ This release closes that milestone with **621 merged pull requests**.
### Security
- **skills:** `GET /api/skills`, `GET /api/skills/custom` and
`GET /api/skills/{name}`, which every signed-in user can call, no longer put
the raw exception text in their `500` detail. A malformed or unreadable
`extensions_config.json` made all three answer with the file's absolute host
path (and, for a permission error, the `[Errno 13]` message). They now return
a fixed `Failed to load skills.` / `Failed to list custom skills.` /
`Failed to get skill.`, and the full error stays in the Gateway log. ([#7155])
- **skillscan:** The pre-LLM static gate now matches fork bombs by shape
instead of only the canonical `:(){ :|:& };:` spelling. `resource-fork-bomb`
and the fork-bomb branch of `shell-destructive-command` matched just that
@ -11911,3 +11919,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
[#7099]: https://github.com/bytedance/deer-flow/pull/7099
[#7104]: https://github.com/bytedance/deer-flow/pull/7104
[#7123]: https://github.com/bytedance/deer-flow/pull/7123
[#7155]: https://github.com/bytedance/deer-flow/pull/7155

View File

@ -4606,6 +4606,13 @@
### 安全
- **技能:** 所有已登录用户都能调用的 `GET /api/skills`、`GET /api/skills/custom`
和 `GET /api/skills/{name}`,其 `500` 响应的 detail 不再包含原始异常文本。此前
`extensions_config.json` 格式错误或不可读时,这三个接口都会在响应中返回该文件的
宿主机绝对路径(权限错误时还会附带 `[Errno 13]` 信息)。现在它们分别返回固定的
`Failed to load skills.` / `Failed to list custom skills.` /
`Failed to get skill.`,完整错误只记录在 Gateway 日志中。 ([#7155])
- **skillscan:** LLM 之前的静态门禁现在按形状匹配 fork 炸弹,而不再只匹配
`:(){ :|:& };:` 这一规范写法。`resource-fork-bomb` 和 `shell-destructive-command`
的 fork 炸弹分支此前只匹配那一种写法,因此 #6651 报告的改名或改空格的载荷——
@ -9463,3 +9470,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
[#7099]: https://github.com/bytedance/deer-flow/pull/7099
[#7104]: https://github.com/bytedance/deer-flow/pull/7104
[#7123]: https://github.com/bytedance/deer-flow/pull/7123
[#7155]: https://github.com/bytedance/deer-flow/pull/7155

View File

@ -16,6 +16,11 @@ create-only: collisions return 409, never overwrite or upsert. Ordinary creation
and import share `_persist_new_agent`, which uses `run_drained_write` so a
cancelled request still drains the owned persistence worker.
`GET /api/skills`, `/api/skills/custom` and `/api/skills/{name}` serve every
signed-in user, so their unexpected-failure 500s return a fixed detail. Load
errors (e.g. a malformed `extensions_config.json`) carry host paths and stay in
the server log.
Skill mutation operation lists return `next_cursor: null` when `has_more` is false, including empty pages after a supplied `after_id`. The next cursor is the final returned operation only when a further page exists.
`GET /api/agent-tool-groups` projects only configured group names for the

View File

@ -508,7 +508,7 @@ async def list_skills(request: Request, config: AppConfig = Depends(get_config))
skills = await asyncio.to_thread(_load_user_skills, config)
except Exception as e:
logger.error(f"Failed to load skills: {e}", exc_info=True)
raise HTTPException(status_code=500, detail=f"Failed to load skills: {str(e)}")
raise HTTPException(status_code=500, detail="Failed to load skills.") from e
visible_skills = await _filter_visible_skills(request, config, skills)
@ -655,7 +655,7 @@ async def list_custom_skills(request: Request, config: AppConfig = Depends(get_c
return SkillsListResponse(skills=[_skill_to_response(skill) for skill in visible_skills])
except Exception as e:
logger.error("Failed to list custom skills: %s", e, exc_info=True)
raise HTTPException(status_code=500, detail=f"Failed to list custom skills: {str(e)}")
raise HTTPException(status_code=500, detail="Failed to list custom skills.") from e
@router.get("/skills/diagnostics/custom", response_model=SkillDiagnosticsResponse, summary="List Owned Custom Skill YAML Errors")
@ -1027,7 +1027,7 @@ async def get_skill(skill_name: str, request: Request, config: AppConfig = Depen
raise
except Exception as e:
logger.error(f"Failed to get skill {skill_name}: {e}", exc_info=True)
raise HTTPException(status_code=500, detail=f"Failed to get skill: {str(e)}")
raise HTTPException(status_code=500, detail="Failed to get skill.") from e
def _write_extensions_skill_state(

View File

@ -31,6 +31,12 @@ def _make_admin_user() -> User:
return User(email="admin-test@example.com", password_hash="x", system_role="admin", id=uuid4())
def _make_member_user() -> User:
from uuid import uuid4
return User(email="member-test@example.com", password_hash="x", system_role="user", id=uuid4())
def _skill_content(name: str, description: str = "Demo skill") -> str:
return f"---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n"
@ -778,7 +784,7 @@ def test_custom_skill_delete_preserves_history_and_allows_restore(monkeypatch, t
assert refresh_calls == [("refresh", "default"), ("refresh", "default")]
def _make_demo_custom_skill_app(monkeypatch, tmp_path: Path) -> FastAPI:
def _patch_demo_custom_skill_config(monkeypatch, tmp_path: Path) -> SimpleNamespace:
from deerflow.config.paths import Paths
custom_dir = _user_custom_dir(tmp_path, "default") / "demo-skill"
@ -793,7 +799,11 @@ def _make_demo_custom_skill_app(monkeypatch, tmp_path: Path) -> FastAPI:
monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
monkeypatch.setattr("deerflow.config.paths._paths", None)
monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
return _make_test_app(config)
return config
def _make_demo_custom_skill_app(monkeypatch, tmp_path: Path, *, user_factory=None) -> FastAPI:
return _make_test_app(_patch_demo_custom_skill_config(monkeypatch, tmp_path), user_factory=user_factory)
@pytest.mark.parametrize(
@ -859,6 +869,69 @@ def test_custom_skill_corrupt_history_record_stays_server_error(monkeypatch, tmp
assert response.json()["detail"].startswith(detail)
@pytest.mark.parametrize(
("path", "detail"),
[
("/api/skills", "Failed to load skills."),
("/api/skills/custom", "Failed to list custom skills."),
("/api/skills/demo-skill", "Failed to get skill."),
],
)
def test_member_skill_reads_keep_load_error_text_out_of_500(monkeypatch, tmp_path, caplog, path, detail):
"""Any signed-in user reaches these reads, so a load failure's host path stays in the server log."""
import logging
extensions_config = tmp_path / "extensions_config.json"
extensions_config.write_text("{not json", encoding="utf-8")
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_config))
app = _make_demo_custom_skill_app(monkeypatch, tmp_path, user_factory=_make_member_user)
with TestClient(app) as client, caplog.at_level(logging.ERROR, logger=skills_router.__name__):
response = client.get(path)
assert response.status_code == 500
assert response.json() == {"detail": detail}
assert str(extensions_config) in caplog.text
@pytest.mark.parametrize(
"path",
[
"/api/skills",
"/api/skills/custom",
"/api/skills/demo-skill",
"/api/capabilities/installations/skills",
],
)
def test_gateway_skill_reads_keep_load_error_text_out_of_500(monkeypatch, tmp_path, path):
"""Drive the real Gateway app: capability discovery has no handler of its own, so its 500 body is TraceMiddleware's fixed fallback until a handler for the raised class or a changed fallback echoes the exception."""
import app.gateway.app as app_module
from deerflow.extensions import reset_loaded_extensions, reset_runtime_diagnostics
extensions_config = tmp_path / "extensions_config.json"
extensions_config.write_text("{not json", encoding="utf-8")
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_config))
monkeypatch.setenv("DEER_FLOW_AUTH_DISABLED", "1")
config = _patch_demo_custom_skill_config(monkeypatch, tmp_path)
config.authorization = AuthorizationConfig(enabled=False)
def _no_config_yaml():
raise FileNotFoundError("config.yaml")
# create_app() reads only the plugin list from config.yaml; keep a local one out of the test.
monkeypatch.setattr(app_module, "get_app_config", _no_config_yaml)
try:
gateway = app_module.create_app()
gateway.dependency_overrides[get_config] = lambda: config
response = TestClient(gateway, raise_server_exceptions=False).get(path)
finally:
reset_loaded_extensions()
reset_runtime_diagnostics()
assert response.status_code == 500
assert str(tmp_path) not in response.text
def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatch, tmp_path):
skills_root = tmp_path / "skills"
from deerflow.config.paths import Paths