From da9f430cae619c362f03074c116f8bd6c0bff1e5 Mon Sep 17 00:00:00 2001 From: yaojin Date: Fri, 21 Aug 2026 04:58:46 -0700 Subject: [PATCH] fix(workstation): hide tools the requester lacks view_tool on (IKABQ0) The end-user workspace chat toolbar (GET /api/v1/workstation/config) was returning every tool the admin had configured for daily chat, with no intersection against the caller's view_tool / use_tool permission. Any logged-in end user could therefore see the full set of API and MCP tools the admin had pinned for the daily-chat agent, regardless of whether they had been granted access. This was a leak of the admin workbench surface into the end-user chat surface. Mirror the platform tool list path (tool.domain.services.tool): pipe the projected tool list through ToolPermissionService.filter_tool_ids_by_permission_async(user_id, ids, "view_tool") before returning. Wire login_user through aget_config / get_daily_chat_config / get_daily_chat_config_with_meta / get_linsight_config / get_linsight_config_with_meta and update the endpoint handlers (config, apps) and the chat service to pass it. The filter is fail-closed on permission-probe errors (no leak), bypassed for admins (config page still echoes every configured tool), and a no-op when login_user is None (legacy/test paths keep prior behaviour). Regression coverage in test/workstation/test_workstation_tool_permission_filter.py covers the non-admin-without-permission, admin-bypass, no-login-user, fail-closed, empty-list, and missing-id cases. Closes IKABQ0. --- .../bisheng/workstation/api/endpoints/apps.py | 2 +- .../workstation/api/endpoints/config.py | 20 +-- .../domain/services/chat_service.py | 4 +- .../domain/services/workstation_service.py | 105 +++++++++++-- ...test_workstation_tool_permission_filter.py | 142 ++++++++++++++++++ 5 files changed, 247 insertions(+), 26 deletions(-) create mode 100644 src/backend/test/workstation/test_workstation_tool_permission_filter.py diff --git a/src/backend/bisheng/workstation/api/endpoints/apps.py b/src/backend/bisheng/workstation/api/endpoints/apps.py index 94d8c05f66..8c8b15b856 100644 --- a/src/backend/bisheng/workstation/api/endpoints/apps.py +++ b/src/backend/bisheng/workstation/api/endpoints/apps.py @@ -27,7 +27,7 @@ async def get_recommended_apps(login_user=LoginUserDep): even if an app later went offline. - Regular users (chat landing): filter to online apps the user can access. """ - config = await WorkStationService.aget_config() + config = await WorkStationService.aget_config(login_user=login_user) if not config or not config.recommendedApps: return resp_200(data=[]) diff --git a/src/backend/bisheng/workstation/api/endpoints/config.py b/src/backend/bisheng/workstation/api/endpoints/config.py index 32305f2457..f96cdbdce0 100644 --- a/src/backend/bisheng/workstation/api/endpoints/config.py +++ b/src/backend/bisheng/workstation/api/endpoints/config.py @@ -19,8 +19,8 @@ @router.get("/config", summary="Get workbench configuration", response_model=UnifiedResponseModel) async def get_config(request: Request, login_user=LoginUserDep): - ret = await WorkStationService.get_daily_chat_config() - linsight_config = await WorkStationService.get_linsight_config() + ret = await WorkStationService.get_daily_chat_config(login_user=login_user) + linsight_config = await WorkStationService.get_linsight_config(login_user=login_user) # `enable_etl4lm` historically gated the frontend on `etl4lm.url` alone, but the # parse pipeline now supports mineru / paddle_ocr as alternative providers that # also handle images. Use the unified image-parsing capability flag so the flag @@ -60,18 +60,16 @@ async def get_config(request: Request, login_user=LoginUserDep): ret["subscription"] = {"assistant_name": (sub_assistant_cfg.assistant_name or "") if sub_assistant_cfg else ""} # Sidebar entry names for the knowledge-space / subscription modules; the home and # app-center ones ride along in the daily config dump above. Empty => client i18n default. - ret["knowledge_space"]["menu_display_name"] = ( - (ks_assistant_cfg.menu_display_name or "") if ks_assistant_cfg else "" - ) - ret["subscription"]["menu_display_name"] = ( - (sub_assistant_cfg.menu_display_name or "") if sub_assistant_cfg else "" - ) + ret["knowledge_space"]["menu_display_name"] = (ks_assistant_cfg.menu_display_name or "") if ks_assistant_cfg else "" + ret["subscription"]["menu_display_name"] = (sub_assistant_cfg.menu_display_name or "") if sub_assistant_cfg else "" return resp_200(data=ret) @router.get("/config/daily", summary="Get daily workbench configuration", response_model=UnifiedResponseModel) async def get_daily_config(request: Request, login_user=LoginUserDep): - ret, inherited, source_tenant_id, has_override = await WorkStationService.get_daily_chat_config_with_meta() + ret, inherited, source_tenant_id, has_override = await WorkStationService.get_daily_chat_config_with_meta( + login_user=login_user + ) return resp_200( data={ "data": ret.model_dump(exclude_unset=True) if ret else None, @@ -94,7 +92,9 @@ async def update_daily_config( @router.get("/config/linsight", summary="Get linsight configuration", response_model=UnifiedResponseModel) async def get_linsight_config(request: Request, login_user=LoginUserDep): - ret, inherited, source_tenant_id, has_override = await WorkStationService.get_linsight_config_with_meta() + ret, inherited, source_tenant_id, has_override = await WorkStationService.get_linsight_config_with_meta( + login_user=login_user + ) return resp_200( data={ "data": ret.model_dump(exclude_unset=True) if ret else None, diff --git a/src/backend/bisheng/workstation/domain/services/chat_service.py b/src/backend/bisheng/workstation/domain/services/chat_service.py index 568125e5f1..e041c538e0 100644 --- a/src/backend/bisheng/workstation/domain/services/chat_service.py +++ b/src/backend/bisheng/workstation/domain/services/chat_service.py @@ -84,7 +84,7 @@ async def get_file_content(filepath_local: str, file_name: str, invoke_user_id: async def initialize_chat(data: APIChatCompletion, login_user: UserPayload): """Initialize chat session, message, and llm.""" - ws_config = await WorkStationService.aget_config() + ws_config = await WorkStationService.aget_config(login_user=login_user) model_info = next((model for model in ws_config.models if model.id == data.model), None) if not model_info: raise ValueError(f"Model with id '{data.model}' not found.") @@ -1169,7 +1169,7 @@ async def _agent_initialize_chat(data: APIChatCompletion, login_user: UserPayloa - `extra` is '{}' (no parentMessageId — new data is linear, no tree) - `overrideParentMessageId` is ignored (regenerate UI removed) """ - ws_config = await WorkStationService.aget_config() + ws_config = await WorkStationService.aget_config(login_user=login_user) model_info = next((m for m in ws_config.models if m.id == data.model), None) if not model_info: raise ValueError(f"Model with id '{data.model}' not found.") diff --git a/src/backend/bisheng/workstation/domain/services/workstation_service.py b/src/backend/bisheng/workstation/domain/services/workstation_service.py index cf7e235457..dc3fa0fd9c 100644 --- a/src/backend/bisheng/workstation/domain/services/workstation_service.py +++ b/src/backend/bisheng/workstation/domain/services/workstation_service.py @@ -39,6 +39,7 @@ from bisheng.knowledge.domain.services.knowledge_service import KnowledgeService from bisheng.llm.domain.schemas import WorkbenchModelConfig from bisheng.llm.domain.services import LLMService +from bisheng.permission.domain.services.tool_permission_service import ToolPermissionService from bisheng.tool.domain.const import ToolPresetType from bisheng.tool.domain.langchain.knowledge import KnowledgeRetrieverTool from bisheng.tool.domain.models.gpts_tools import GptsTools, GptsToolsDao, GptsToolsType @@ -404,11 +405,52 @@ async def _ahydrate_tools_from_source_tenant( ) return hydrated + @classmethod + async def _afilter_tools_by_view_permission( + cls, + grouped: list[dict], + login_user: UserPayload | None, + ) -> list[dict]: + """Drop tool groups the requester cannot see. + + End users without ``view_tool`` on a configured tool must not see it in + the chat toolbar / agent tool selector. We filter at the parent + (tool_type) id — every child of that group shares the same access + because permissions are bound to the parent type in OpenFGA. + + Admins (super / tenant / child) bypass the filter so the config + page echoes every configured tool. Callers without an injected + user (legacy code paths, tests that stub the method) keep the + pre-fix behaviour — no filter — so we do not regress the + pre-permission-visible config surface. + """ + if not grouped or login_user is None or login_user.is_admin(): + return grouped + + candidate_ids = [int(group["id"]) for group in grouped if group.get("id") is not None] + if not candidate_ids: + return grouped + try: + allowed = await ToolPermissionService.filter_tool_ids_by_permission_async( + login_user, + candidate_ids, + "view_tool", + ) + except Exception: + # Fail closed: if the permission probe errors, hide every tool + # rather than leak. Logged once; the chat toolbar then renders + # empty (existing UX for an empty config). + logger.exception("workstation: tool permission filter failed; hiding all configured tools") + return [] + allowed_set = {str(a) for a in allowed} + return [group for group in grouped if group.get("id") is not None and str(int(group["id"])) in allowed_set] + @classmethod async def _aproject_tools_for_current_tenant( cls, tools: list | None, source_tenant_id: int = DEFAULT_TENANT_ID, + login_user: UserPayload | None = None, ) -> list[dict]: if not tools: return [] @@ -479,7 +521,11 @@ async def _load_rows() -> tuple[list[GptsTools], list[GptsToolsType]]: for group in grouped: if group["id"] in default_checked_types: group["default_checked"] = True - return grouped + # Apply the per-user view_tool filter last so a user who lacks + # permission on a configured tool does not see it in the chat + # toolbar (IKABQ0: "user without API/MCP tool permission still sees + # the tool in the workspace"). + return await cls._afilter_tools_by_view_permission(grouped, login_user) @classmethod async def _afilter_org_kbs_for_current_tenant(cls, org_kbs: list | None) -> list[dict]: @@ -516,11 +562,14 @@ async def _aproject_daily_config_for_current_tenant( cls, config: WorkstationConfig | None, source_tenant_id: int = DEFAULT_TENANT_ID, + login_user: UserPayload | None = None, ) -> WorkstationConfig | None: if config is None: return None updates = { - "tools": await cls._aproject_tools_for_current_tenant(config.tools, source_tenant_id), + "tools": await cls._aproject_tools_for_current_tenant( + config.tools, source_tenant_id, login_user=login_user + ), "orgKbs": await cls._afilter_org_kbs_for_current_tenant(config.orgKbs), "recommendedApps": await cls._afilter_recommended_apps_for_current_tenant(config.recommendedApps), } @@ -531,10 +580,11 @@ async def _aproject_linsight_config_for_current_tenant( cls, config: LinsightConfig | None, source_tenant_id: int = DEFAULT_TENANT_ID, + login_user: UserPayload | None = None, ) -> LinsightConfig | None: if config is None: return None - tools = await cls._aproject_tools_for_current_tenant(config.tools, source_tenant_id) + tools = await cls._aproject_tools_for_current_tenant(config.tools, source_tenant_id, login_user=login_user) return config.model_copy(update={"tools": tools}) @classmethod @@ -653,7 +703,10 @@ def get_config(cls) -> WorkstationConfig | None: return cls._apply_workbench_models(ret, LLMService.get_workbench_llm_sync()) @classmethod - async def aget_config(cls) -> WorkstationConfig | None: + async def aget_config( + cls, + login_user: UserPayload | None = None, + ) -> WorkstationConfig | None: """Get the default workstation configuration asynchronously.""" value, inherited, _, _ = await cls._aresolve_tenant_config(ConfigKeyEnum.WORKSTATION) config = type("TenantConfigValue", (), {"value": value}) if value else None @@ -663,11 +716,20 @@ async def aget_config(cls) -> WorkstationConfig | None: if ret and not inherited: ret.tools = cls.sync_tool_info(ret.tools) if inherited: - ret = await cls._aproject_daily_config_for_current_tenant(ret, DEFAULT_TENANT_ID) + ret = await cls._aproject_daily_config_for_current_tenant(ret, DEFAULT_TENANT_ID, login_user=login_user) + elif login_user is not None: + # When the config is not inherited, the synced tool list still + # has to be filtered by the caller's permissions — otherwise + # the chat toolbar shows every tool the admin configured for + # this tenant, even ones the requester cannot see (IKABQ0). + ret.tools = await cls._afilter_tools_by_view_permission(ret.tools, login_user) return cls._apply_workbench_models(ret, await LLMService.get_workbench_llm()) @classmethod - async def get_daily_chat_config(cls) -> WorkstationConfig | None: + async def get_daily_chat_config( + cls, + login_user: UserPayload | None = None, + ) -> WorkstationConfig | None: """Get the default workstation configuration for daily chat.""" value, inherited, _, _ = await cls._aresolve_tenant_config(ConfigKeyEnum.WORKSTATION) config = type("TenantConfigValue", (), {"value": value}) if value else None @@ -677,7 +739,9 @@ async def get_daily_chat_config(cls) -> WorkstationConfig | None: if ret and not inherited: ret.tools = cls.sync_tool_info(ret.tools) if inherited: - ret = await cls._aproject_daily_config_for_current_tenant(ret, DEFAULT_TENANT_ID) + ret = await cls._aproject_daily_config_for_current_tenant(ret, DEFAULT_TENANT_ID, login_user=login_user) + elif login_user is not None: + ret.tools = await cls._afilter_tools_by_view_permission(ret.tools, login_user) return cls._apply_workbench_models(ret, await LLMService.get_workbench_llm()) @classmethod @@ -691,7 +755,10 @@ async def update_daily_chat_config(cls, data: WorkstationConfig) -> WorkstationC return await cls.get_daily_chat_config() @classmethod - async def get_daily_chat_config_with_meta(cls) -> tuple[WorkstationConfig | None, bool, int, bool]: + async def get_daily_chat_config_with_meta( + cls, + login_user: UserPayload | None = None, + ) -> tuple[WorkstationConfig | None, bool, int, bool]: value, inherited, source_tenant_id, has_override = await cls._aresolve_tenant_config(ConfigKeyEnum.WORKSTATION) config = type("TenantConfigValue", (), {"value": value}) if value else None ret = cls.parse_config(config) @@ -700,21 +767,28 @@ async def get_daily_chat_config_with_meta(cls) -> tuple[WorkstationConfig | None if ret and not inherited: ret.tools = cls.sync_tool_info(ret.tools) if inherited: - ret = await cls._aproject_daily_config_for_current_tenant(ret, source_tenant_id) + ret = await cls._aproject_daily_config_for_current_tenant(ret, source_tenant_id, login_user=login_user) + elif login_user is not None: + ret.tools = await cls._afilter_tools_by_view_permission(ret.tools, login_user) ret = cls._apply_workbench_models(ret, await LLMService.get_workbench_llm()) return ret, inherited, source_tenant_id, has_override @classmethod - async def get_linsight_config(cls) -> LinsightConfig | None: + async def get_linsight_config( + cls, + login_user: UserPayload | None = None, + ) -> LinsightConfig | None: """Get Linsight configuration.""" value, inherited, _, _ = await cls._aresolve_tenant_config(ConfigKeyEnum.WORKSTATION_LINSIGHT) if not value: return None ret = LinsightConfig(**json.loads(value)) if inherited: - ret = await cls._aproject_linsight_config_for_current_tenant(ret, DEFAULT_TENANT_ID) + ret = await cls._aproject_linsight_config_for_current_tenant(ret, DEFAULT_TENANT_ID, login_user=login_user) else: ret.tools = cls.sync_tool_info(ret.tools) + if login_user is not None: + ret.tools = await cls._afilter_tools_by_view_permission(ret.tools, login_user) return ret @classmethod @@ -727,7 +801,10 @@ async def update_linsight_config(cls, data: LinsightConfig) -> LinsightConfig: return data @classmethod - async def get_linsight_config_with_meta(cls) -> tuple[LinsightConfig | None, bool, int, bool]: + async def get_linsight_config_with_meta( + cls, + login_user: UserPayload | None = None, + ) -> tuple[LinsightConfig | None, bool, int, bool]: value, inherited, source_tenant_id, has_override = await cls._aresolve_tenant_config( ConfigKeyEnum.WORKSTATION_LINSIGHT ) @@ -735,9 +812,11 @@ async def get_linsight_config_with_meta(cls) -> tuple[LinsightConfig | None, boo return None, inherited, source_tenant_id, has_override ret = LinsightConfig(**json.loads(value)) if inherited: - ret = await cls._aproject_linsight_config_for_current_tenant(ret, source_tenant_id) + ret = await cls._aproject_linsight_config_for_current_tenant(ret, source_tenant_id, login_user=login_user) else: ret.tools = cls.sync_tool_info(ret.tools) + if login_user is not None: + ret.tools = await cls._afilter_tools_by_view_permission(ret.tools, login_user) return ret, inherited, source_tenant_id, has_override @classmethod diff --git a/src/backend/test/workstation/test_workstation_tool_permission_filter.py b/src/backend/test/workstation/test_workstation_tool_permission_filter.py new file mode 100644 index 0000000000..1bd92d434a --- /dev/null +++ b/src/backend/test/workstation/test_workstation_tool_permission_filter.py @@ -0,0 +1,142 @@ +"""Regression test for IKABQ0 — workstation tool list is filtered by the +requester's ``view_tool`` permission. + +The bug: an end user without API/MCP tool permission could see the +admin-configured tools in the workspace chat toolbar. The fix +(`WorkStationService._afilter_tools_by_view_permission`) drops tool +groups the requester has no ``view_tool`` permission on before the +config is returned to the client. Admins (super / tenant / child) keep +the unfiltered view so the config page can still echo every tool. +""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest + +from bisheng.workstation.domain.services.workstation_service import WorkStationService + + +def _make_user(is_admin: bool) -> SimpleNamespace: + return SimpleNamespace(is_admin=lambda: is_admin) + + +def _make_groups(*type_ids: int) -> list[dict]: + return [ + { + "id": tid, + "name": f"tool-type-{tid}", + "is_preset": 0, + "description": "", + "default_checked": False, + "children": [ + { + "id": tid * 100, + "name": f"tool-{tid}", + "tool_key": f"tool_key_{tid}", + "desc": "", + } + ], + } + for tid in type_ids + ] + + +@pytest.mark.asyncio +async def test_non_admin_without_view_permission_loses_tool(): + """A non-admin user without `view_tool` on a tool must not see it.""" + groups = _make_groups(1, 2, 3) + user = _make_user(is_admin=False) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(return_value=["2"]), # user only has permission for type 2 + ) as mock_filter: + result = await WorkStationService._afilter_tools_by_view_permission(groups, user) + + assert [g["id"] for g in result] == [2] + mock_filter.assert_awaited_once() + args, _ = mock_filter.call_args + assert args[0] is user + assert sorted(args[1]) == [1, 2, 3] + assert args[2] == "view_tool" + + +@pytest.mark.asyncio +async def test_admin_bypasses_view_permission_filter(): + """Admins see every configured tool regardless of permission bindings.""" + groups = _make_groups(1, 2, 3) + user = _make_user(is_admin=True) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(return_value=[]), + ) as mock_filter: + result = await WorkStationService._afilter_tools_by_view_permission(groups, user) + + # Admin short-circuit must not consult the permission service at all. + mock_filter.assert_not_called() + assert sorted(g["id"] for g in result) == [1, 2, 3] + + +@pytest.mark.asyncio +async def test_no_login_user_keeps_unfiltered_behavior(): + """Without a login_user (legacy/test paths) no filtering is applied.""" + groups = _make_groups(1, 2) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(return_value=[]), + ) as mock_filter: + result = await WorkStationService._afilter_tools_by_view_permission(groups, None) + + mock_filter.assert_not_called() + assert sorted(g["id"] for g in result) == [1, 2] + + +@pytest.mark.asyncio +async def test_filter_fails_closed_on_permission_probe_error(): + """A failing permission probe hides every tool (fail closed, no leak).""" + groups = _make_groups(1, 2, 3) + user = _make_user(is_admin=False) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(side_effect=RuntimeError("OpenFGA down")), + ): + result = await WorkStationService._afilter_tools_by_view_permission(groups, user) + + assert result == [] + + +@pytest.mark.asyncio +async def test_empty_tool_list_is_returned_as_is(): + """Empty input short-circuits — no permission probe at all.""" + user = _make_user(is_admin=False) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(return_value=[]), + ) as mock_filter: + result = await WorkStationService._afilter_tools_by_view_permission([], user) + + mock_filter.assert_not_called() + assert result == [] + + +@pytest.mark.asyncio +async def test_groups_with_missing_id_pass_through_dropped(): + """A group without an ``id`` is dropped (it cannot be permission-checked).""" + groups = [ + {"id": 1, "name": "ok", "is_preset": 0, "description": "", "default_checked": False, "children": []}, + {"id": None, "name": "broken", "is_preset": 0, "description": "", "default_checked": False, "children": []}, + ] + user = _make_user(is_admin=False) + + with patch( + "bisheng.workstation.domain.services.workstation_service.ToolPermissionService.filter_tool_ids_by_permission_async", + new=AsyncMock(return_value=["1"]), + ): + result = await WorkStationService._afilter_tools_by_view_permission(groups, user) + + assert [g["id"] for g in result] == [1]