mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-19 11:06:18 +00:00
feat(authz): filter per-caller skill visibility on the skill listing surfaces (#4063 Phase 4) (#5489)
* 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>
This commit is contained in:
hataa
GitHub
parent
aa7f616734
commit
d811143b52
@ -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`.
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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`.
|
||||
|
||||
|
||||
@ -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")
|
||||
|
||||
471
backend/tests/test_skills_listing_authorization.py
Normal file
471
backend/tests/test_skills_listing_authorization.py
Normal file
@ -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
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user