From d811143b5272624cb8cf1b0f97b0ee028b9f8bd6 Mon Sep 17 00:00:00 2001
From: hataa <79907651+hata33@users.noreply.github.com>
Date: Fri, 18 Sep 2026 08:02:52 +0800
Subject: [PATCH] feat(authz): filter per-caller skill visibility on the skill
 listing surfaces (#4063 Phase 4) (#5489)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* feat(authz): filter per-caller skill visibility on the listing surfaces (#4063 Phase 4)

GET /api/skills, GET /api/skills/custom, and GET /api/skills/{name} now
filter the user-scoped catalog through filter_resources(principal,
"skill", ...) — mirroring list_models. Anonymous callers are unfiltered;
provider errors follow authorization.fail_closed (fail-closed -> empty
listing / 404, fail-open -> full listing). An invisible skill on the
detail surface returns the standard 404 so the endpoint cannot become an
existence oracle the filtered list closed. Management endpoints stay
require_admin_user-gated; runtime activation is #4541's layer.

resolve_skill_authorization joins resolve_model_authorization as a thin
sibling over a shared _resolve_route_scoped_authorization core.

* docs(authz): reflect per-caller skill visibility in OpenAPI metadata and implementation notes (#5489)

Address the two non-blocking review findings on #5489:

- The three user-facing GET routes (/skills, /skills/custom,
  /skills/{name}) now say in their /docs-visible descriptions that
  authorization filters the response (hidden skills 404 on detail).
- Add the dated Phase 4 decision-log entry to the authorization
  implementation notes, per the convention of every prior merged
  authz PR: listing-visibility semantics, the 404-vs-403
  existence-oracle rationale, anonymous-caller behavior, and the
  #4541 rebase reconciliation points (config.example.yaml roles
  comment + this file's decision log).

* docs(authz): move route guidance into Gateway module guide

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
---
 backend/app/gateway/AGENTS.md                 |   6 +
 backend/app/gateway/authz.py                  |  34 +-
 backend/app/gateway/routers/skills.py         |  94 +++-
 .../deerflow/agents/middlewares/AGENTS.md     |   2 +-
 backend/tests/test_skills_custom_router.py    |  16 +
 .../test_skills_listing_authorization.py      | 471 ++++++++++++++++++
 backend/tests/test_skills_router_authz.py     |   6 +-
 config.example.yaml                           |   7 +
 ...able-authorization-implementation-notes.md |  45 ++
 9 files changed, 662 insertions(+), 19 deletions(-)
 create mode 100644 backend/tests/test_skills_listing_authorization.py

diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md
index 186cff6e1..a75d093da 100644
--- a/backend/app/gateway/AGENTS.md
+++ b/backend/app/gateway/AGENTS.md
@@ -168,3 +168,9 @@ Proxied through nginx: `/api/langgraph/*` → Gateway LangGraph-compatible runti
 archive/search behavior, read [Thread lifecycle invariants](../../docs/THREAD_LIFECYCLE.md).
 It owns lineage and settled-checkpoint rules, legacy fallback boundaries, archive
 filtering before pagination, owner isolation, and activity-time preservation.
+
+### Route and skill-listing authorization
+
+Gateway route authorization uses `authz.py::resolve_route_permissions()` as the single provider integration point for both `AuthMiddleware` and decorator-only authentication. When enabled, it evaluates the six registered `threads:*` / `runs:*` permissions as `resource="route"` requests whose targets are the full `resource:action` strings. Decisions use the async provider API and are cached for the request in `AuthContext`; decorators do not call the provider again. Provider resolution or decision errors follow `authorization.fail_closed`, scoped per permission for decision errors. When authorization is disabled, the legacy complete permission set is returned without resolving a provider. Existing `owner_check` enforcement and `require_admin_user()` management gates remain independent and unchanged. Tests: `tests/test_authorization_route_permissions.py`, `tests/test_auth.py`, and `tests/test_auth_middleware.py`.
+
+Skill listing authorization mirrors the models pattern: `routers/skills.py` routes resolve `(provider, principal)` through `authz.py::resolve_skill_authorization()` — a thin sibling of `resolve_model_authorization`, both delegating to the shared `_resolve_route_scoped_authorization` core — and the shared `_filter_visible_skills` helper filters the user-scoped catalog (public + caller's custom skills) through `filter_resources(principal, "skill", ...)` by name. Three user-facing surfaces apply it: `GET /api/skills` (the frontend skill list / slash-command autocomplete), `GET /api/skills/custom`, and `GET /api/skills/{name}` — the detail endpoint returns the standard 404 for an invisible skill so it cannot become an existence oracle the filtered list closed (`get_model`'s 403 is an execution decision via `authorize("model", "use")`, which has no skills equivalent in this layer). Anonymous callers are unfiltered (mirroring `list_models`), and provider resolution/decision errors follow `authorization.fail_closed` (fail-closed → empty listing / 404, fail-open → full listing). Skill management endpoints (`install`/`upload`/`reload`/custom-skill CRUD) stay `require_admin_user`-gated, and runtime activation authorization is a separate layer. The built-in RBAC provider maps this to the per-role `skills` policy key. Tests: `tests/test_skills_listing_authorization.py`.
diff --git a/backend/app/gateway/authz.py b/backend/app/gateway/authz.py
index aed9944f8..cefbfa5a3 100644
--- a/backend/app/gateway/authz.py
+++ b/backend/app/gateway/authz.py
@@ -317,11 +317,11 @@ class _AuthorizationUnavailable(Exception):
         self.fail_closed = fail_closed
 
 
-def resolve_model_authorization(user: User, *, is_internal: bool) -> tuple[AuthorizationProvider | None, Principal | None]:
-    """Return ``(provider, principal)`` for model-route authorization.
+def _resolve_route_scoped_authorization(user: User, *, is_internal: bool) -> tuple[AuthorizationProvider | None, Principal | None]:
+    """Return ``(provider, principal)`` for route-level resource authorization.
 
     When authorization is disabled, returns ``(None, None)`` so callers can
-    short-circuit to legacy behavior (all models visible). When enabled,
+    short-circuit to legacy behavior (resource visible). When enabled,
     resolves the cached provider and builds a Principal identical to
     ``resolve_route_permissions`` (including the ``INTERNAL_SYSTEM_ROLE``
     → ``None`` pop so internal callers fall under ``default_role``).
@@ -339,7 +339,7 @@ def resolve_model_authorization(user: User, *, is_internal: bool) -> tuple[Autho
         if provider is None:
             raise ValueError("authorization is enabled but provider resolution returned None")
     except Exception:
-        logger.warning("Failed to resolve authorization provider for model routes", exc_info=True)
+        logger.warning("Failed to resolve authorization provider for route-level resources", exc_info=True)
         raise _AuthorizationUnavailable(fail_closed=config.fail_closed)
 
     principal = build_principal_from_context(
@@ -349,13 +349,35 @@ def resolve_model_authorization(user: User, *, is_internal: bool) -> tuple[Autho
     return provider, principal
 
 
+def resolve_model_authorization(user: User, *, is_internal: bool) -> tuple[AuthorizationProvider | None, Principal | None]:
+    """Return ``(provider, principal)`` for model-route authorization.
+
+    Delegates to ``_resolve_route_scoped_authorization``: disabled →
+    ``(None, None)``; provider-resolution failure raises
+    ``_AuthorizationUnavailable`` (carrying ``fail_closed``).
+    """
+    return _resolve_route_scoped_authorization(user, is_internal=is_internal)
+
+
+def resolve_skill_authorization(user: User, *, is_internal: bool) -> tuple[AuthorizationProvider | None, Principal | None]:
+    """Return ``(provider, principal)`` for skill-route authorization.
+
+    Same resolution and Principal construction as ``resolve_model_authorization``
+    (disabled → ``(None, None)``; provider-resolution failure raises
+    ``_AuthorizationUnavailable`` carrying ``fail_closed``). Consumers translate
+    that into the appropriate deny response (empty skill listing).
+    """
+    return _resolve_route_scoped_authorization(user, is_internal=is_internal)
+
+
 def _route_authz_context(user: User, *, is_internal: bool) -> dict:
     """Build the shared Principal context dict for a request-scoped user.
 
     Applies the ``INTERNAL_SYSTEM_ROLE → None`` pop so internal callers fall
     under ``default_role`` (mirrors ``inject_authenticated_user_context``).
-    Used by ``resolve_model_authorization`` and ``authorize_sandbox_for_request``
-    so every route-level authorization path builds the identity the same way.
+    Used by ``_resolve_route_scoped_authorization`` (model/skill routes) and
+    ``authorize_sandbox_for_request`` so every route-level authorization path
+    builds the identity the same way.
     """
     from app.gateway.internal_auth import INTERNAL_SYSTEM_ROLE
 
diff --git a/backend/app/gateway/routers/skills.py b/backend/app/gateway/routers/skills.py
index f54fd22ba..9febc1b23 100644
--- a/backend/app/gateway/routers/skills.py
+++ b/backend/app/gateway/routers/skills.py
@@ -10,7 +10,12 @@ from pydantic import BaseModel, Field
 from starlette.datastructures import FormData, Headers, UploadFile
 from starlette.formparsers import MultiPartException, MultiPartParser
 
-from app.gateway.deps import get_config, require_admin_user
+from app.gateway.authz import (
+    _AuthorizationUnavailable,
+    _is_internal_caller,
+    resolve_skill_authorization,
+)
+from app.gateway.deps import get_config, get_optional_user_from_request, require_admin_user
 from app.gateway.path_utils import resolve_thread_virtual_path
 from app.gateway.skill_export import ExportClientDisconnected, SkillExportManifestResponse, SkillExportResponse, export_http_error, run_export_work
 from deerflow.agents.lead_agent.prompt import clear_skills_system_prompt_cache, refresh_skills_system_prompt_cache_async, refresh_user_skills_system_prompt_cache_async
@@ -268,21 +273,69 @@ async def _install_skill_archive(archive_path: Path, config: AppConfig) -> Skill
         raise HTTPException(status_code=500, detail=f"Failed to install skill: {str(e)}") from e
 
 
+async def _filter_visible_skills(
+    request: Request,
+    config: AppConfig,
+    skills: list[Skill],
+) -> list[Skill]:
+    """Apply the per-caller skill visibility filter (mirrors ``list_models``).
+
+    Anonymous callers are not filtered. Provider resolution or decision
+    errors follow ``authorization.fail_closed``: fail-closed returns an
+    empty list (nothing visible), fail-open returns the unfiltered input.
+    """
+    fail_closed = config.authorization.fail_closed
+
+    user = await get_optional_user_from_request(request)
+    if user is None:
+        return skills
+
+    try:
+        provider, principal = resolve_skill_authorization(user, is_internal=_is_internal_caller(request, user))
+    except _AuthorizationUnavailable as exc:
+        return [] if exc.fail_closed else skills
+
+    if provider is None or principal is None:
+        return skills
+
+    try:
+        allowed_names = provider.filter_resources(principal, "skill", [skill.name for skill in skills])
+        if not isinstance(allowed_names, list) or any(not isinstance(name, str) for name in allowed_names):
+            raise TypeError("AuthorizationProvider.filter_resources must return list[str]")
+        allowed_set = set(allowed_names)
+        return [skill for skill in skills if skill.name in allowed_set]
+    except Exception:
+        logger.warning("Authorization provider failed while filtering skills", exc_info=True)
+        return [] if fail_closed else skills
+
+
 @router.get(
     "/skills",
     response_model=SkillsListResponse,
     summary="List All Skills",
-    description="Retrieve a list of all available skills from both public and custom directories.",
+    description=("Retrieve a list of all available skills from both public and custom directories. When authorization is enabled, only skills visible to the caller's role are returned."),
 )
-async def list_skills(config: AppConfig = Depends(get_config)) -> SkillsListResponse:
+async def list_skills(request: Request, config: AppConfig = Depends(get_config)) -> SkillsListResponse:
+    """List all skills visible to the caller.
+
+    Uses user-scoped storage: loads public (global) + custom (user-level +
+    fallback) skills.
+
+    When ``authorization.enabled`` is true, only skills the caller's role may
+    see are returned (filtered via ``provider.filter_resources`` with
+    ``resource_type="skill"``, mirroring ``list_models``). A provider error
+    yields an empty list (fail-closed) or all skills (fail-open).
+    """
     try:
-        # Use user-scoped storage: loads public (global) + custom (user-level + fallback)
         skills = _get_user_skill_storage(config).load_skills(enabled_only=False)
-        return SkillsListResponse(skills=[_skill_to_response(skill) for skill in skills])
     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)}")
 
+    visible_skills = await _filter_visible_skills(request, config, skills)
+
+    return SkillsListResponse(skills=[_skill_to_response(skill) for skill in visible_skills])
+
 
 @router.post(
     "/skills/install",
@@ -375,18 +428,28 @@ async def reload_skills(request: Request) -> SkillReloadResponse:
     )
 
 
-@router.get("/skills/custom", response_model=SkillsListResponse, summary="List Custom Skills")
-async def list_custom_skills(config: AppConfig = Depends(get_config)) -> SkillsListResponse:
+@router.get(
+    "/skills/custom",
+    response_model=SkillsListResponse,
+    summary="List Custom Skills",
+    description=("Retrieve the caller's user-owned custom skills. When authorization is enabled, only skills visible to the caller's role are returned."),
+)
+async def list_custom_skills(request: Request, config: AppConfig = Depends(get_config)) -> SkillsListResponse:
     """List only user-owned custom skills (SkillCategory.CUSTOM).
 
     Legacy shared skills (SkillCategory.LEGACY) are NOT included here —
     they are read-only and appear in the full ``list_skills`` endpoint.
     The frontend should use ``list_skills`` to display all available
     skills including legacy ones.
+
+    When ``authorization.enabled`` is true, the same per-caller visibility
+    filter as ``list_skills`` applies — without it this endpoint would
+    surface names the main listing hides.
     """
     try:
         skills = [skill for skill in _get_user_skill_storage(config).load_skills(enabled_only=False) if skill.category == SkillCategory.CUSTOM]
-        return SkillsListResponse(skills=[_skill_to_response(skill) for skill in skills])
+        visible_skills = await _filter_visible_skills(request, config, skills)
+        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)}")
@@ -604,9 +667,9 @@ async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, req
     "/skills/{skill_name}",
     response_model=SkillResponse,
     summary="Get Skill Details",
-    description="Retrieve detailed information about a specific skill by its name.",
+    description=("Retrieve detailed information about a specific skill by its name. When authorization is enabled, a skill hidden from the caller's role returns 404, indistinguishable from a missing skill."),
 )
-async def get_skill(skill_name: str, config: AppConfig = Depends(get_config)) -> SkillResponse:
+async def get_skill(skill_name: str, request: Request, config: AppConfig = Depends(get_config)) -> SkillResponse:
     try:
         skill_name = skill_name.replace("\r\n", "").replace("\n", "")
         skills = _get_user_skill_storage(config).load_skills(enabled_only=False)
@@ -615,7 +678,16 @@ async def get_skill(skill_name: str, config: AppConfig = Depends(get_config)) ->
         if skill is None:
             raise HTTPException(status_code=404, detail=f"Skill '{skill_name}' not found")
 
-        return _skill_to_response(skill)
+        # Visibility filter: a skill the caller's role may not see is
+        # indistinguishable from a nonexistent one. Unlike ``get_model``
+        # (which enforces ``model:use`` and 403s on an execution decision),
+        # this layer is listing visibility only — 404 keeps the detail
+        # surface from becoming an existence oracle the filtered list closed.
+        visible_skills = await _filter_visible_skills(request, config, [skill])
+        if not visible_skills:
+            raise HTTPException(status_code=404, detail=f"Skill '{skill_name}' not found")
+
+        return _skill_to_response(visible_skills[0])
     except HTTPException:
         raise
     except Exception as e:
diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md
index c19e4e1e4..d4927cd27 100644
--- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md
+++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md
@@ -86,7 +86,7 @@ strict providers reject.
 
 Authorization identity is independent of enforcement. Gateway strips client identity overrides: only the server auth source sets `is_internal`, and only authenticated IM `body.context` supplies `channel_user_id` (never `body.config`). `build_principal_from_context` applies role defaults, strict provenance, and copied attributes; RBAC rejects unknown defaults. Delegation and `GuardrailMiddleware` share this identity. Layer 1 precedes deferred assembly across agent paths and its provider is reused for Layer 2; framework skill/memory ordering stays stable. Trusted `DeerFlowClient.stream()` accepts identity overrides. Its graph key always includes effective storage `user_id` and, when enforced, the full Principal; nested attributes are copied so mutation cannot hide stale cache state.
 
-Gateway route authorization uses `authz.py::resolve_route_permissions()` as the single provider integration point for both `AuthMiddleware` and decorator-only authentication. When enabled, it evaluates the six registered `threads:*` / `runs:*` permissions as `resource="route"` requests whose targets are the full `resource:action` strings. Decisions use the async provider API and are cached for the request in `AuthContext`; decorators do not call the provider again. Provider resolution or decision errors follow `authorization.fail_closed`, scoped per permission for decision errors. When authorization is disabled, the legacy complete permission set is returned without resolving a provider. Existing `owner_check` enforcement and `require_admin_user()` management gates remain independent and unchanged. Tests: `tests/test_authorization_route_permissions.py`, `tests/test_auth.py`, and `tests/test_auth_middleware.py`.
+Gateway route permissions and per-caller skill visibility are documented in the [Gateway guide](../../../../../app/gateway/AGENTS.md#route-and-skill-listing-authorization). Runtime skill activation remains a separate layer.
 
 Model authorization uses `authz.py::resolve_model_authorization()` (same cached-provider, internal-role, and principal-building path as route authorization) as the Gateway integration point for the `models` router: `list_models` filters names through `filter_resources(principal, "model", ...)`, and `get_model` enforces `authorize(resource="model", action="use")` with a deny surfacing as 403; provider errors follow `authorization.fail_closed` (fail-open returns the unfiltered list / proceeds). At runtime, `lead_agent/agent.py::_authorize_model_name` — called from `_make_lead_agent` and from `DeerFlowClient._ensure_agent` — applies the same `model:use` check to the resolved model name. On deny it scans the `filter_resources`-visible names (excluding the denied model), re-verifying each candidate with `authorize("model", "use")` before falling back, because a custom provider may allow `list` while denying `use`; no usable fallback raises under `fail_closed` and keeps the original model under fail-open. The built-in RBAC provider maps this to the per-role `models` policy key. Tests: `tests/test_models_authorization.py`.
 
diff --git a/backend/tests/test_skills_custom_router.py b/backend/tests/test_skills_custom_router.py
index 3d1f5568e..8240c6e40 100644
--- a/backend/tests/test_skills_custom_router.py
+++ b/backend/tests/test_skills_custom_router.py
@@ -15,6 +15,7 @@ from app.gateway.auth.models import User
 from app.gateway.deps import get_config
 from app.gateway.routers import skills as skills_router
 from app.gateway.routers import uploads as uploads_router
+from deerflow.config.authorization_config import AuthorizationConfig
 from deerflow.skills.security_static_scanner import StaticScannerError
 from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
 from deerflow.skills.types import Skill
@@ -51,6 +52,13 @@ def _make_skill(name: str, *, enabled: bool) -> Skill:
 
 
 def _make_test_app(config) -> FastAPI:
+    # The listing/detail routes read config.authorization.fail_closed even
+    # when authorization is disabled (mirroring list_models). Many tests here
+    # build minimal SimpleNamespace configs; backfill the real disabled
+    # shape once so each of them exercises the unfiltered path instead of
+    # crashing on a missing attribute.
+    if not hasattr(config, "authorization"):
+        config.authorization = AuthorizationConfig(enabled=False)
     app = make_authed_test_app(user_factory=_make_admin_user)
     app.state.config = config  # kept for any startup-style reads
     app.dependency_overrides[get_config] = lambda: config
@@ -1122,6 +1130,11 @@ class TestMultiUserSkillIsolation:
                 use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
             ),
             skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+            # list_skills reads config.authorization.fail_closed even when
+            # authorization is disabled (mirroring list_models); give the
+            # fake the real disabled shape so the isolation path (no
+            # provider filtering) stays exercised.
+            authorization=AuthorizationConfig(enabled=False),
         )
         return alice_storage, bob_storage, config
 
@@ -1322,6 +1335,9 @@ class TestMultiUserSkillIsolation:
                 use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage",
             ),
             skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+            # Same shape note as _setup_two_user_env: list_skills reads
+            # config.authorization.fail_closed even when disabled.
+            authorization=AuthorizationConfig(enabled=False),
         )
         monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: alice_storage)
         monkeypatch.setattr(skills_router, "get_effective_user_id", lambda: "alice")
diff --git a/backend/tests/test_skills_listing_authorization.py b/backend/tests/test_skills_listing_authorization.py
new file mode 100644
index 000000000..dfc1e1397
--- /dev/null
+++ b/backend/tests/test_skills_listing_authorization.py
@@ -0,0 +1,471 @@
+"""Phase 4 skill-listing authorization tests.
+
+Covers the Gateway route layer only (``list_skills``): the request-scoped
+Principal and ``filter_resources(principal, "skill", ...)`` visibility filter,
+mirroring Phase 3's ``list_models`` tests (``resolve_skill_authorization`` is
+the ``resolve_model_authorization`` twin). Runtime skill authorization —
+assembly filtering and slash-activation — is the #4541 layer and is
+deliberately out of scope here; this file pins only which skills a caller
+may *see* on the user-facing listing surface.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
+
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+from app.gateway.routers import skills as skills_router
+from deerflow.authz.provider import AuthzDecision, AuthzReason
+from deerflow.authz.rbac import RbacAuthorizationProvider
+from deerflow.config.app_config import AppConfig
+from deerflow.config.authorization_config import AuthorizationConfig
+from deerflow.config.sandbox_config import SandboxConfig
+from deerflow.skills import Skill
+from deerflow.skills.types import SkillCategory
+
+# ── Helpers ────────────────────────────────────────────────────────────
+
+
+def _user(**overrides):
+    values = {
+        "id": "user-123",
+        "system_role": "user",
+        "oauth_provider": "github",
+        "oauth_id": "oauth-456",
+    }
+    values.update(overrides)
+    return SimpleNamespace(**values)
+
+
+def _skill(name: str, *, category: SkillCategory = SkillCategory.PUBLIC, enabled: bool = True) -> Skill:
+    """Build a minimal Skill with dummy paths; only name/category/enabled matter here."""
+    return Skill(
+        name=name,
+        description=f"Skill {name}",
+        license=None,
+        skill_dir=Path(f"/skills/{category}/{name}"),
+        skill_file=Path(f"/skills/{category}/{name}/SKILL.md"),
+        relative_path=Path(name),
+        category=category,
+        enabled=enabled,
+    )
+
+
+class _FakeStorage:
+    """Stand-in for user-scoped SkillStorage: returns preloaded skills."""
+
+    def __init__(self, skills: list[Skill]) -> None:
+        self._skills = skills
+        self.load_calls: list[bool] = []
+
+    def load_skills(self, *, enabled_only: bool = False) -> list[Skill]:
+        self.load_calls.append(enabled_only)
+        return list(self._skills)
+
+
+def _make_app_config() -> AppConfig:
+    return AppConfig(
+        sandbox=SandboxConfig(use="deerflow.sandbox.local:LocalSandboxProvider"),
+        authorization=AuthorizationConfig(),
+    )
+
+
+def _make_skills_app(app_config: AppConfig) -> FastAPI:
+    """Build a FastAPI app with the skills router and a pinned config."""
+    app = FastAPI()
+    app.include_router(skills_router.router)
+    app.dependency_overrides[skills_router.get_config] = lambda: app_config
+    return app
+
+
+def _stub_storage(monkeypatch, storage: _FakeStorage) -> None:
+    # list_skills resolves the candidate universe through user-scoped storage
+    # (a plain module-level call, not a FastAPI dependency); pin it so tests
+    # control the catalog without touching the filesystem.
+    monkeypatch.setattr("app.gateway.routers.skills._get_user_skill_storage", lambda config: storage)
+
+
+def _enable_authorization(monkeypatch, provider, *, fail_closed: bool = True, default_role: str = "user") -> None:
+    config = AuthorizationConfig(
+        enabled=True,
+        fail_closed=fail_closed,
+        default_role=default_role,
+    )
+    monkeypatch.setattr("app.gateway.authz._get_route_authorization_config", lambda: config)
+    monkeypatch.setattr("app.gateway.authz._get_cached_route_provider", lambda c: provider)
+
+
+class _RecordingProvider:
+    """Provider that records all requests and can deny/error specific targets."""
+
+    name = "recording"
+
+    def __init__(self, *, denied: set[str] | None = None, errors: set[str] | None = None) -> None:
+        self.denied = denied or set()
+        self.errors = errors or set()
+        self.authorize_requests: list = []
+        self.filter_requests: list = []
+
+    def authorize(self, request):
+        self.authorize_requests.append(request)
+        if request.target in self.errors:
+            raise RuntimeError(f"provider failed for {request.target}")
+        allowed = request.target not in self.denied
+        return AuthzDecision(
+            allow=allowed,
+            reasons=[AuthzReason(code="authz.allowed" if allowed else "authz.denied")],
+        )
+
+    async def aauthorize(self, request):
+        return self.authorize(request)
+
+    def filter_resources(self, principal, resource_type, candidates):
+        self.filter_requests.append((resource_type, list(candidates)))
+        if resource_type in self.errors:
+            raise RuntimeError(f"provider failed for {resource_type}")
+        return [c for c in candidates if c not in self.denied]
+
+
+def _stub_user(monkeypatch, user) -> None:
+    monkeypatch.setattr(
+        "app.gateway.routers.skills.get_optional_user_from_request",
+        AsyncMock(return_value=user),
+    )
+
+
+# ── list_skills tests ──────────────────────────────────────────────────
+
+
+def test_list_skills_disabled_returns_all(monkeypatch):
+    """When authorization is disabled, all skills are visible."""
+    config = AuthorizationConfig(enabled=False)
+    monkeypatch.setattr("app.gateway.authz._get_route_authorization_config", lambda: config)
+    cached = AsyncMock(side_effect=AssertionError("disabled must not resolve provider"))
+    monkeypatch.setattr("app.gateway.authz._get_cached_route_provider", cached)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "web-research"]
+    cached.assert_not_called()
+
+
+def test_list_skills_anonymous_user_returns_all(monkeypatch):
+    """Anonymous requests (user=None) are not filtered."""
+    provider = _RecordingProvider()
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, None)
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "web-research"]
+    assert provider.filter_requests == []
+
+
+def test_list_skills_rbac_filters_by_allow(monkeypatch):
+    """Role with a skill allowlist sees only allowed skills, in storage order."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": ["pdf-export"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research"), _skill("data-viz")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export"]
+
+
+def test_list_skills_rbac_filters_by_deny(monkeypatch):
+    """Role with a skill denylist no longer sees denied skills."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": "*", "deny": ["web-research"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research"), _skill("data-viz")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "data-viz"]
+
+
+def test_list_skills_wildcard_returns_all(monkeypatch):
+    """Role with skills allow: '*' sees all skills."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": "*"}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "web-research"]
+
+
+def test_list_skills_no_role_policy_returns_all(monkeypatch):
+    """A role with no ``skills`` policy is unrestricted for skills (mirrors the
+    permissive-on-absent default; policies for other resources don't leak in)."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"models": {"allow": ["gpt-4"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "web-research"]
+
+
+def test_list_skills_filters_custom_and_public_uniformly(monkeypatch):
+    """The visibility filter applies by name across categories: a denied
+    custom skill disappears even while other custom skills remain."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": "*", "deny": ["my-private-skill"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage(
+        [
+            _skill("pdf-export"),
+            _skill("my-private-skill", category=SkillCategory.CUSTOM),
+            _skill("team-playbook", category=SkillCategory.CUSTOM),
+        ]
+    )
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["pdf-export", "team-playbook"]
+
+
+@pytest.mark.parametrize(
+    ("fail_closed", "expected_count"),
+    [(True, 0), (False, 3)],
+)
+def test_list_skills_provider_error_fail_closed_vs_open(monkeypatch, fail_closed, expected_count):
+    """Provider error → empty (fail-closed) or all (fail-open)."""
+    provider = _RecordingProvider(errors={"skill"})
+    _enable_authorization(monkeypatch, provider, fail_closed=fail_closed)
+
+    app_config = _make_app_config()
+    app_config.authorization.fail_closed = fail_closed
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research"), _skill("data-viz")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(app_config)) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    assert len(response.json()["skills"]) == expected_count
+
+
+@pytest.mark.parametrize(
+    ("fail_closed", "expected_count"),
+    [(True, 0), (False, 2)],
+)
+def test_list_skills_provider_unavailable_fail_closed_vs_open(monkeypatch, fail_closed, expected_count):
+    """Provider cannot be resolved → empty (fail-closed) or all (fail-open)."""
+    config = AuthorizationConfig(enabled=True, fail_closed=fail_closed, default_role="user")
+    monkeypatch.setattr("app.gateway.authz._get_route_authorization_config", lambda: config)
+
+    def _raise_resolution_error(c):
+        raise RuntimeError("provider resolution failed")
+
+    monkeypatch.setattr("app.gateway.authz._get_cached_route_provider", _raise_resolution_error)
+
+    app_config = _make_app_config()
+    app_config.authorization.fail_closed = fail_closed
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(app_config)) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    assert len(response.json()["skills"]) == expected_count
+
+
+@pytest.mark.parametrize(
+    ("fail_closed", "expected_count"),
+    [(True, 0), (False, 2)],
+)
+def test_list_skills_provider_bad_filter_return_type(monkeypatch, fail_closed, expected_count):
+    """A provider whose filter_resources returns non-list[str] is treated as a
+    provider failure (TypeError guard), not an unfiltered passthrough."""
+    provider = SimpleNamespace(
+        filter_resources=lambda principal, resource_type, candidates: "pdf-export",
+    )
+    _enable_authorization(monkeypatch, provider, fail_closed=fail_closed)
+
+    app_config = _make_app_config()
+    app_config.authorization.fail_closed = fail_closed
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(app_config)) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    assert len(response.json()["skills"]) == expected_count
+
+
+def test_list_skills_requests_skill_resource_type(monkeypatch):
+    """The route filters through resource_type "skill" with the full catalog
+    as candidates (pins the provider contract the RBAC ``skills`` key maps to)."""
+    provider = _RecordingProvider()
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills")
+
+    assert response.status_code == 200
+    assert provider.filter_requests == [
+        ("skill", ["pdf-export", "web-research"]),
+    ]
+
+
+# ── list_custom_skills / get_skill: the remaining visibility surfaces ──
+
+
+def test_list_custom_skills_rbac_filters_by_deny(monkeypatch):
+    """The custom-only listing surface applies the same visibility filter:
+    without it, a denied name hidden from GET /api/skills would remain
+    visible on GET /api/skills/custom."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": "*", "deny": ["my-private-skill"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage(
+        [
+            _skill("pdf-export"),
+            _skill("my-private-skill", category=SkillCategory.CUSTOM),
+            _skill("team-playbook", category=SkillCategory.CUSTOM),
+        ]
+    )
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills/custom")
+
+    assert response.status_code == 200
+    names = [s["name"] for s in response.json()["skills"]]
+    assert names == ["team-playbook"]
+
+
+def test_get_skill_denied_is_indistinguishable_from_missing(monkeypatch):
+    """A denied skill returns the same 404 as a nonexistent one — the detail
+    surface must not become an existence oracle that the filtered list
+    closed (403 would leak that the name exists)."""
+    provider = RbacAuthorizationProvider(
+        roles={"user": {"skills": {"allow": ["pdf-export"]}}},
+    )
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export"), _skill("web-research")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        denied = client.get("/api/skills/web-research")
+        missing = client.get("/api/skills/does-not-exist")
+        allowed = client.get("/api/skills/pdf-export")
+
+    assert denied.status_code == 404
+    assert missing.status_code == 404
+    # The detail echoes the caller-supplied name through the standard
+    # not-found template — byte-identical to a genuine miss of that name,
+    # so the response carries no extra information.
+    assert denied.json()["detail"] == "Skill 'web-research' not found"
+    assert missing.json()["detail"] == "Skill 'does-not-exist' not found"
+    assert allowed.status_code == 200
+    assert allowed.json()["name"] == "pdf-export"
+
+
+def test_get_skill_anonymous_returns_200(monkeypatch):
+    """Anonymous requests (user=None) are not filtered, mirroring list_skills."""
+    provider = _RecordingProvider()
+    _enable_authorization(monkeypatch, provider)
+
+    storage = _FakeStorage([_skill("pdf-export")])
+    _stub_user(monkeypatch, None)
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(_make_app_config())) as client:
+        response = client.get("/api/skills/pdf-export")
+
+    assert response.status_code == 200
+    assert provider.filter_requests == []
+
+
+@pytest.mark.parametrize(
+    ("fail_closed", "expected_status"),
+    [(True, 404), (False, 200)],
+)
+def test_get_skill_provider_error_fail_closed_vs_open(monkeypatch, fail_closed, expected_status):
+    """Provider error → invisible (fail-closed, 404) or visible (fail-open, 200)."""
+    provider = _RecordingProvider(errors={"skill"})
+    _enable_authorization(monkeypatch, provider, fail_closed=fail_closed)
+
+    app_config = _make_app_config()
+    app_config.authorization.fail_closed = fail_closed
+    storage = _FakeStorage([_skill("pdf-export")])
+    _stub_user(monkeypatch, _user())
+    _stub_storage(monkeypatch, storage)
+
+    with TestClient(_make_skills_app(app_config)) as client:
+        response = client.get("/api/skills/pdf-export")
+
+    assert response.status_code == expected_status
diff --git a/backend/tests/test_skills_router_authz.py b/backend/tests/test_skills_router_authz.py
index 7a1b79304..6146a1a55 100644
--- a/backend/tests/test_skills_router_authz.py
+++ b/backend/tests/test_skills_router_authz.py
@@ -30,6 +30,7 @@ from fastapi.testclient import TestClient
 from app.gateway.auth.models import User
 from app.gateway.deps import get_config
 from app.gateway.routers import skills as skills_router
+from deerflow.config.authorization_config import AuthorizationConfig
 
 
 def _make_user(system_role: str) -> User:
@@ -129,7 +130,10 @@ def test_basic_skill_listing_stays_open_to_normal_users(monkeypatch):
         ]
 
     app = _make_app(system_role="user")
-    app.dependency_overrides[get_config] = lambda: SimpleNamespace()
+    # list_skills reads config.authorization.fail_closed even when
+    # authorization is disabled (mirroring list_models); give the fake the
+    # real disabled shape so the open-to-normal-users path stays exercised.
+    app.dependency_overrides[get_config] = lambda: SimpleNamespace(authorization=AuthorizationConfig(enabled=False))
     monkeypatch.setattr(skills_router, "_get_user_skill_storage", lambda cfg: SimpleNamespace(load_skills=_load_skills))
     with TestClient(app) as client:
         assert client.get("/api/skills").status_code == 200
diff --git a/config.example.yaml b/config.example.yaml
index db423f97d..05e1ec087 100644
--- a/config.example.yaml
+++ b/config.example.yaml
@@ -2805,17 +2805,23 @@ run_ownership:
 #     config:
 #       # A known role with no policy for a resource is unrestricted for it.
 #       # Define both `tools` and `routes` wherever access should be constrained.
+#       # `models` / `skills` govern the per-caller listing/detail surfaces
+#       # (which models/skills the caller sees on GET /api/models and the
+#       # GET /api/skills[.../custom, /{name}] skill routes); skill
+#       # management endpoints (install/edit/export) stay admin-gated.
 #       roles:
 #         admin:
 #           tools: {allow: "*"}
 #           routes: {allow: "*"}
 #           models: {allow: "*"}
 #           sandbox: {allow: "*"}
+#           skills: {allow: "*"}
 #         user:
 #           tools: {allow: "*", deny: ["update_agent"]}
 #           routes: {allow: "*"}
 #           models: {allow: "*"}
 #           sandbox: {allow: "*"}
+#           skills: {allow: "*"}
 #         guest:
 #           # web-only role: sandbox-dependent tools (read_file, bash, glob,
 #           # grep, write_file, ...) are omitted — with sandbox:execute denied
@@ -2825,6 +2831,7 @@ run_ownership:
 #           routes: {allow: ["threads:read", "runs:read"]}
 #           models: {allow: ["gpt-4o-mini"]}
 #           sandbox: {allow: false}  # deny sandbox execution
+#           skills: {allow: []}      # no skills visible in the listing
 authorization:
   enabled: false
 
diff --git a/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md b/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md
index d85d9d7f3..05cc0f295 100644
--- a/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md
+++ b/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md
@@ -438,6 +438,51 @@ Phase 1 最低验证要求：
   event loop；loop-affine provider 在 `__init__` 调用 `asyncio.get_running_loop()` 并在
   `aauthorize()` 验证仍是同一个 loop。
 
+### 2026-09-17 — Phase 4 / PR #5489 Skills listing visibility (list / detail)
+
+- **背景：** Phase 4 PR 1（#5228 `/me` route permissions）与 PR 2（#5294 前端权限门控）
+  合并后，模型已有 per-caller listing/use 授权（#4540），sandbox 已有 execute 授权
+  （#4911），但 skill 列表表面仍对全部已认证用户开放——`GET /api/skills`、
+  `GET /api/skills/custom`、`GET /api/skills/{name}` 不检查角色，是 Phase 3 资源类型
+  清单里最后未收口的 listing 面。
+- **决策（表面清单）：** 恰好三个非管理 GET 表面接入 per-caller 可见过滤：`list_skills`、
+  `list_custom_skills`、`get_skill`，共享 `_filter_visible_skills(request, config, skills)`
+  helper，语义镜像 `list_models`：`provider.filter_resources(principal, "skill", names)`
+  批量过滤；provider 解析失败 → `_AuthorizationUnavailable`（携带 `fail_closed` 标志）；
+  provider 抛错或返回非 `list[str]` → 空（fail-closed）或全量（fail-open）。skills.py
+  其余全部路由维持 `require_admin_user` 门控，不在本层重复过滤。
+- **决策（detail 404 而非 403）：** `get_skill` 对被过滤 skill 返回与真实缺失逐字一致的
+  404，而非 `get_model` 的 403。理由：`get_model` 执行的是 `authorize("model", "use")`
+  使用决策（模型存在但角色无权使用 → 403 合理）；本层只有 listing visibility，没有
+  skill 执行决策的对应物，403 会让 detail 端点变成过滤清单刚关掉的 existence oracle。
+- **决策（resolver 结构）：** 从 `resolve_model_authorization` 提取共享核心
+  `_resolve_route_scoped_authorization(user, *, is_internal)`，
+  `resolve_skill_authorization` 与 model 版本互为薄封装（含 `INTERNAL_SYSTEM_ROLE → None`
+  pop 与 internal-caller 语义）。RBAC `_RESOURCE_POLICY_KEYS` 已含 `"skill": "skills"`
+  （rbac.py），roles 的 `skills: {allow: [...]}` 直接生效，无 schema 变更。
+- **否决方案：** 不为 skill 引入 `authorize("skill", "read")` 逐名授权——listing 表面
+  用批量 `filter_resources` 一次往返即可，逐名决策增加配置面且与 `list_models` 不对称。
+  不只过滤 `/skills` 主列表——自审发现 `/skills/custom` 与 `/skills/{name}` 会原样
+  泄露主列表隐藏的名字，三个表面必须同批收口。
+- **兼容性：** `authorization.enabled: false` 时三表面均 no-op（返回全量）。匿名请求
+  （user=None）不过滤——生产 auth 开启时 `AuthMiddleware` 先行 401，该分支实际只覆盖
+  auth-disabled 本地模式，与 `list_models` 对齐。skill 管理端点（install/edit/export/
+  delete 等）保持 `require_admin_user`，不受本过滤影响。RBAC 缺 `skills` 键 = 放行，
+  `allow: []` = 全拒（与 `models` 键同语义）。
+- **证据：** `tests/test_skills_listing_authorization.py` 覆盖 disabled/anonymous/RBAC
+  allow/deny/wildcard/absent-policy、custom+public 一致过滤、provider error/unavailable/
+  坏返回类型 × fail-closed/fail-open、`("skill", [...])` 契约、custom 列表绕过封闭、
+  detail 404 与真实缺失逐字一致；`test_skills_router_authz.py` 与
+  `test_skills_custom_router.py` 的 fake config 补 `AuthorizationConfig`（含
+  `_make_test_app` 回填 shim）。review（willem-bd）在 head tree 执行验证：4 套件
+  78 tests 通过，且移除 custom-listing 过滤的突变使
+  `test_list_custom_skills_rbac_filters_by_deny` 变红，守护测试真实。
+- **延期：** #4541（Phase 3 执行层：assembly 过滤 + slash-activation 授权）与本 PR
+  互补（本 PR 管 listing visibility，#4541 管 runtime use），其 rebase 时需双向调和：
+  `config.example.yaml` roles 注释段两 PR 均改；本文件决策日志两 PR 也在同一插入点
+  各追加条目。前端 effective-permissions 展示剩余项；management route 的 provider
+  迁移（沿袭前阶段延期项）。
+
 ### 新记录模板
 
 ```markdown