From 5804ee1a7e1d0246c242fcf39da5c3013db5064e Mon Sep 17 00:00:00 2001 From: AochenShen99 Date: Thu, 24 Sep 2026 16:11:46 +0800 Subject: [PATCH] fix(suggestions): enforce model use authorization (#5816) --- README.md | 2 + backend/app/gateway/AGENTS.md | 6 + backend/app/gateway/authz.py | 31 ++++ backend/app/gateway/routers/models.py | 25 +--- backend/app/gateway/routers/suggestions.py | 10 +- .../test_suggestions_model_authorization.py | 134 ++++++++++++++++++ backend/tests/test_suggestions_router.py | 4 + 7 files changed, 187 insertions(+), 25 deletions(-) create mode 100644 backend/tests/test_suggestions_model_authorization.py diff --git a/README.md b/README.md index 2c9f562949..48298036c1 100644 --- a/README.md +++ b/README.md @@ -1278,6 +1278,8 @@ Each message can still select up to 1000 documents. When more than 100 documents Advanced deployments can enable pluggable authorization with `authorization.enabled` in `config.yaml`. A configured `AuthorizationProvider` filters denied tools before they reach the model or deferred-tool catalog, then the same provider is checked again before every business-tool execution through the existing guardrail middleware. Gateway `threads:*` and `runs:*` route permissions are derived from the same provider, while existing owner checks and admin-only management gates remain in force. Every HTTP route that starts or enables a future Agent run requires `runs:create`: this includes the stateless `POST /api/runs/stream` and `POST /api/runs/wait` endpoints plus scheduled-task create, update, resume, and manual-trigger mutations. Scheduled-task mutations retain their existing `threads:write` requirement, and the stateless routes separately enforce ownership when the optional thread ID is supplied in the request body. A generated `tool_search` may bypass the second tool check only when it fronts the current build's already-filtered deferred catalog. Model access follows the same provider: the Gateway `models` list is filtered per principal, `model:use` is enforced on model detail requests and again when the runtime resolves the agent's model, and a denied default model falls back to the first remaining candidate that also passes `model:use`. The built-in RBAC provider supports per-role `tools`, `routes`, `models`, `skills`, and `sandbox` allow/deny policies and validates that `default_role` names a configured role; authorization is disabled by default. See `config.example.yaml` and the [authorization RFC](docs/plans/2026-07-10-pluggable-authorization-rfc.md). +Follow-up suggestions also check `model:use` before calling the selected model, including the default model when no name is supplied. A denied model returns HTTP 403 without an LLM call; authorization-provider failures follow the configured `fail_closed` policy. + For vision-capable agents, `view_image` and the subsequent model-context image read also require `sandbox:execute`. Allowing the tool name alone does not grant image-file access; a role denied sandbox execution cannot reread previously recorded image metadata after its permissions change. Advanced deployments can also extend the agent runtime itself by declaring `AgentMiddleware` classes under `extensions.middlewares` in `config.yaml` or `extensions_config.json`. Each entry is a `module.path:ClassName` string (zero-argument constructor) or an object `{class, kwargs}` whose `kwargs` are passed to the constructor. `kwargs` values must be JSON types (object, array, string, number, boolean, or null); YAML dates and timestamps are coerced to ISO strings so they match JSON. DeerFlow loads the same configured list into the lead-agent and subagent pipelines after their built-in runtime middlewares and loop/token guards, but before the terminal-response/safety/clarification tail, so enterprise forks can add domain guardrails, tool-call governance, or observability hooks without patching the built-in middleware builders. Missing packages, invalid classes, broken modules, and constructor errors fail loudly at agent creation. Treat `config.yaml` and `extensions_config.json` as trusted operator-controlled files: middleware paths are code execution, just like custom tool, model, sandbox, guardrail, MCP server, and MCP interceptor declarations. Gateway skill/MCP toggle endpoints preserve this field but do not expose an API write path for `extensions.middlewares`. Separate lead-only/subagent-only middleware lists are not supported yet. diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index 5e70a12284..1ecc9a05c2 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -212,6 +212,12 @@ failure policy rather than exposing the unfiltered user-scoped catalog. ### Route and skill-listing authorization +`authz.authorize_model_use` shares the `model:use` decision between model details +and follow-up suggestions, including the factory's first configured model when +the caller omits `model_name`. Suggestions check admission before their +best-effort LLM error handler: explicit denies return 403 without invoking the +model, while provider failures honor `authorization.fail_closed`. + `authz.py::resolve_route_permissions()` is shared by HTTP middleware, decorator-only auth, and Live Browser WebSocket admission. It evaluates registered permissions asynchronously as `resource="route"`, targeting full `resource:action` strings. HTTP caches decisions in `AuthContext`; provider errors follow `authorization.fail_closed` (decision errors are per-permission). Disabled authorization returns all permissions without a provider. Owner and admin checks remain independent. Live requires `threads:write` with `is_internal=False` after login/Origin checks but before acceptance, owner lookup, or session acquisition; denial closes 4403, unexpected setup errors close 4501, cancellation propagates. Checks are admission-only; restart Gateway to close old connections. Tests: `test_authorization_route_permissions.py`, `test_browser_readonly_security.py`, `test_auth.py`, `test_auth_middleware.py` in `tests/`. 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 545559844e..273c2784da 100644 --- a/backend/app/gateway/authz.py +++ b/backend/app/gateway/authz.py @@ -375,6 +375,37 @@ def resolve_model_authorization(user: User, *, is_internal: bool) -> tuple[Autho return _resolve_route_scoped_authorization(user, is_internal=is_internal) +def authorize_model_use(user: User, model_name: str | None, *, is_internal: bool, app_config: AppConfig) -> None: + """Enforce model:use for an explicit model or the factory's default model. + + Shared by model details and caller-selected one-shot requests. Explicit + denies always reject; provider failures honor the configured failure policy. + """ + if model_name is None: + if not app_config.models: + return # The caller's existing no-model handling remains authoritative. + model_name = app_config.models[0].name + detail = f"Model '{model_name}' is not available for your role" + try: + provider, principal = resolve_model_authorization(user, is_internal=is_internal) + except _AuthorizationUnavailable: + if app_config.authorization.fail_closed: + raise HTTPException(status_code=403, detail=detail) from None + return + if provider is None or principal is None: + return + try: + decision = provider.authorize(AuthzRequest(principal=principal, resource="model", action="use", target=model_name)) + if not isinstance(decision, AuthzDecision): + raise TypeError("AuthorizationProvider.authorize must return AuthzDecision") + allowed = decision.allow + except Exception: + logger.warning("Authorization provider failed while checking model:use for %s", model_name, exc_info=True) + allowed = not app_config.authorization.fail_closed + if not allowed: + raise HTTPException(status_code=403, detail=detail) + + def resolve_skill_authorization(user: User, *, is_internal: bool) -> tuple[AuthorizationProvider | None, Principal | None]: """Return ``(provider, principal)`` for skill-route authorization. diff --git a/backend/app/gateway/routers/models.py b/backend/app/gateway/routers/models.py index da779e8687..8fec748c96 100644 --- a/backend/app/gateway/routers/models.py +++ b/backend/app/gateway/routers/models.py @@ -6,10 +6,10 @@ from pydantic import BaseModel, Field from app.gateway.authz import ( _AuthorizationUnavailable, _is_internal_caller, + authorize_model_use, resolve_model_authorization, ) from app.gateway.deps import get_config, get_optional_user_from_request -from deerflow.authz.provider import AuthzDecision, AuthzRequest from deerflow.config.app_config import AppConfig logger = logging.getLogger(__name__) @@ -170,30 +170,9 @@ async def get_model( # Phase 3: enforce model:use authorization (deny → 403, not 404, since the # model exists but the role lacks permission to use it). - fail_closed = config.authorization.fail_closed user = await get_optional_user_from_request(request) if user is not None: - try: - provider, principal = resolve_model_authorization(user, is_internal=_is_internal_caller(request, user)) - except _AuthorizationUnavailable: - if fail_closed: - raise HTTPException(status_code=403, detail=f"Model '{model_name}' is not available for your role") - else: - if provider is not None and principal is not None: - try: - decision = provider.authorize(AuthzRequest(principal=principal, resource="model", action="use", target=model_name)) - if not isinstance(decision, AuthzDecision): - raise TypeError("AuthorizationProvider.authorize must return AuthzDecision") - allowed = decision.allow - except Exception: - logger.warning( - "Authorization provider failed while checking model:use for %s", - model_name, - exc_info=True, - ) - allowed = not fail_closed - if not allowed: - raise HTTPException(status_code=403, detail=f"Model '{model_name}' is not available for your role") + authorize_model_use(user, model_name, is_internal=_is_internal_caller(request, user), app_config=config) return ModelResponse( name=model.name, diff --git a/backend/app/gateway/routers/suggestions.py b/backend/app/gateway/routers/suggestions.py index 59dc2cb3f2..9a3fd582cc 100644 --- a/backend/app/gateway/routers/suggestions.py +++ b/backend/app/gateway/routers/suggestions.py @@ -5,8 +5,8 @@ from fastapi import APIRouter, Depends, Request from pydantic import BaseModel, Field import deerflow.utils.llm_text as llm_text -from app.gateway.authz import require_permission -from app.gateway.deps import get_config +from app.gateway.authz import _is_internal_caller, authorize_model_use, require_permission +from app.gateway.deps import get_config, get_current_user_from_request from deerflow.config.app_config import AppConfig from deerflow.config.suggestions_config import DEFAULT_MAX_SUGGESTIONS, MAX_SUGGESTIONS_LIMIT from deerflow.utils.oneshot_llm import run_oneshot_llm @@ -118,6 +118,12 @@ async def generate_suggestions( if not conversation: return SuggestionsResponse(suggestions=[]) + # Check the same effective model the factory will use, including its + # default when the caller omits model_name. Keep permission failures out + # of the best-effort LLM error handler below so a denial remains a 403. + user = await get_current_user_from_request(request) + authorize_model_use(user, body.model_name, is_internal=_is_internal_caller(request, user), app_config=config) + system_instruction = ( "You are generating follow-up questions to help the user continue the conversation.\n" f"Based on the conversation below, produce EXACTLY {n} short questions the user might ask next.\n" diff --git a/backend/tests/test_suggestions_model_authorization.py b/backend/tests/test_suggestions_model_authorization.py new file mode 100644 index 0000000000..35f52fe916 --- /dev/null +++ b/backend/tests/test_suggestions_model_authorization.py @@ -0,0 +1,134 @@ +"""The suggestions route must enforce the same model policy as model details.""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest +from fastapi import FastAPI, Request +from fastapi.testclient import TestClient + +from app.gateway import authz +from app.gateway.deps import get_config +from app.gateway.routers import models, suggestions +from deerflow.authz.provider import AuthzDecision +from deerflow.authz.rbac import RbacAuthorizationProvider +from deerflow.config.app_config import AppConfig +from deerflow.config.authorization_config import AuthorizationConfig +from deerflow.config.model_config import ModelConfig +from deerflow.config.sandbox_config import SandboxConfig +from deerflow.utils import oneshot_llm + + +@pytest.fixture +def route_env(monkeypatch): + user = SimpleNamespace(id="user-123", system_role="user", oauth_provider=None, oauth_id=None) + config = AppConfig( + models=[ModelConfig(name=name, model=name, use="langchain_openai:ChatOpenAI") for name in ["restricted", "allowed"]], + sandbox=SandboxConfig(use="deerflow.sandbox.local:LocalSandboxProvider"), + authorization=AuthorizationConfig(enabled=True, fail_closed=True), + ) + provider = RbacAuthorizationProvider(roles={"user": {"models": {"allow": ["allowed"]}}}) + monkeypatch.setattr(authz, "_get_route_authorization_config", lambda: config.authorization) + monkeypatch.setattr(authz, "_get_cached_route_provider", lambda _config: provider) + monkeypatch.setattr(models, "get_optional_user_from_request", AsyncMock(return_value=user)) + invoked = [] + model = SimpleNamespace(ainvoke=AsyncMock(return_value=SimpleNamespace(content='["What next?"]'))) + + def create_model(**kwargs): + invoked.append(kwargs["name"] if kwargs["name"] is not None else config.models[0].name) + return model + + monkeypatch.setattr(oneshot_llm, "create_chat_model", create_model) + app = FastAPI() + + @app.middleware("http") + async def authenticate(request: Request, call_next): + request.state.user = user + request.state.auth_source = "session" + request.state.auth = authz.AuthContext(user, ["threads:read", "runs:create"]) + return await call_next(request) + + app.state.thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) + app.dependency_overrides[get_config] = lambda: config + app.include_router(models.router) + app.include_router(suggestions.router) + with TestClient(app) as client: + yield SimpleNamespace(client=client, config=config, provider=provider, invoked=invoked, model=model, app=app) + + +def _suggest(env, **body): + return env.client.post( + "/api/threads/thread-1/suggestions", + json={"messages": [{"role": "user", "content": "Hello"}], **body}, + ) + + +@pytest.mark.parametrize("body", [{"model_name": "restricted"}, {}, {"model_name": None}]) +def test_denied_explicit_and_default_models_are_not_invoked(route_env, body): + assert route_env.client.get("/api/models/restricted").status_code == 403 + response = _suggest(route_env, **body) + assert response.status_code == 403 + assert route_env.invoked == [] + route_env.model.ainvoke.assert_not_awaited() + + +@pytest.mark.parametrize("use_default", [False, True]) +def test_allowed_model_still_generates_suggestions(route_env, use_default): + if use_default: + route_env.config.models.reverse() + response = _suggest(route_env, **({} if use_default else {"model_name": "allowed"})) + assert response.status_code == 200 + assert response.json() == {"suggestions": ["What next?"]} + assert route_env.invoked == ["allowed"] + route_env.model.ainvoke.assert_awaited_once() + + +def test_disabled_authorization_preserves_existing_behavior(route_env): + route_env.config.authorization.enabled = False + assert _suggest(route_env, model_name="restricted").status_code == 200 + assert route_env.invoked == ["restricted"] + + +@pytest.mark.parametrize("fail_closed", [False, True]) +@pytest.mark.parametrize("failure", ["resolution", "decision", "invalid_decision"]) +def test_provider_failure_honors_configured_policy(route_env, monkeypatch, fail_closed, failure): + route_env.config.authorization.fail_closed = fail_closed + + def broken(*_args, **_kwargs): + if failure == "invalid_decision": + return None + raise RuntimeError("provider unavailable") + + if failure == "resolution": + monkeypatch.setattr(authz, "_get_cached_route_provider", broken) + else: + monkeypatch.setattr(route_env.provider, "authorize", broken) + response = _suggest(route_env, model_name="allowed") + assert response.status_code == (403 if fail_closed else 200) + assert route_env.invoked == ([] if fail_closed else ["allowed"]) + + +def test_explicit_deny_is_not_treated_as_a_fail_open_error(route_env): + route_env.config.authorization.fail_closed = False + assert _suggest(route_env, model_name="restricted").status_code == 403 + assert route_env.invoked == [] + + +def test_checks_use_permission_even_when_model_is_visible(route_env, monkeypatch): + checked = [] + + def authorize(request): + checked.append((request.resource, request.action, request.target)) + return AuthzDecision(allow=request.action != "use") + + monkeypatch.setattr(route_env.provider, "authorize", authorize) + response = _suggest(route_env, model_name="allowed") + assert response.status_code == 403 + assert checked == [("model", "use", "allowed")] + assert route_env.invoked == [] + + +def test_foreign_thread_remains_denied_before_model_invocation(route_env): + route_env.app.state.thread_store.check_access.return_value = False + assert _suggest(route_env, model_name="allowed").status_code == 404 + assert route_env.invoked == [] diff --git a/backend/tests/test_suggestions_router.py b/backend/tests/test_suggestions_router.py index 30a69a7a67..ef2df09875 100644 --- a/backend/tests/test_suggestions_router.py +++ b/backend/tests/test_suggestions_router.py @@ -16,6 +16,10 @@ def _clear_langfuse_env(monkeypatch): for name in ("LANGFUSE_TRACING", "LANGFUSE_PUBLIC_KEY", "LANGFUSE_SECRET_KEY", "LANGFUSE_BASE_URL", "DEER_FLOW_ENV", "ENVIRONMENT"): monkeypatch.delenv(name, raising=False) reset_tracing_config() + # These tests bypass route auth to exercise parsing and tracing. Model + # admission is covered through HTTP in test_suggestions_model_authorization. + monkeypatch.setattr(suggestions, "get_current_user_from_request", AsyncMock(return_value=SimpleNamespace())) + monkeypatch.setattr(suggestions, "authorize_model_use", lambda *_args, **_kwargs: None) yield reset_tracing_config()