fix: enforce tool and agent config authorization

backend/app/api/tools.py

@@ -131,12 +131,44 @@ async def _load_assigned_smithery_connection(
 
 
 def _resolve_target_tenant_id(current_user: User, tenant_id: str | None = None) -> uuid.UUID | None:
+    """Resolve a requested tenant and reject cross-tenant access by non-platform admins."""
     if tenant_id:
         try:
-            return uuid.UUID(tenant_id)
+            target_tenant_id = uuid.UUID(tenant_id)
         except ValueError:
             raise HTTPException(status_code=400, detail="Invalid tenant_id format")
-    return current_user.tenant_id
+    else:
+        target_tenant_id = current_user.tenant_id
+
+    if target_tenant_id != current_user.tenant_id and current_user.role != "platform_admin":
+        raise HTTPException(status_code=403, detail="No access to this tenant")
+    return target_tenant_id
+
+
+def _require_tool_manager(current_user: User) -> None:
+    """Restrict tenant tool administration to organization and platform administrators."""
+    if current_user.role not in ("platform_admin", "org_admin"):
+        raise HTTPException(status_code=403, detail="Tool management permission required")
+
+
+async def _require_agent_tool_manager(
+    db: AsyncSession,
+    current_user: User,
+    agent_id: uuid.UUID,
+):
+    """Load an agent only when the caller may manage its configuration."""
+    agent = await _load_agent_for_tool_scope(db, agent_id)
+    if current_user.role == "platform_admin":
+        return agent
+    if not await can_manage_agent(db, current_user, agent):
+        raise HTTPException(status_code=403, detail="Agent manage permission required")
+    return agent
+
+
+def _require_tool_record_access(current_user: User, tool: Tool) -> None:
+    """Ensure a tenant administrator cannot mutate another tenant's tool record."""
+    if tool.tenant_id is not None:
+        _resolve_target_tenant_id(current_user, str(tool.tenant_id))
 
 
 def _get_sensitive_keys(config_schema: dict | None = None) -> set[str]:
@@ -199,6 +231,7 @@ async def list_tools(
     db: AsyncSession = Depends(get_db),
 ):
     """List platform tools scoped by tenant (builtin + tenant-specific)."""
+    _require_tool_manager(current_user)
     query = (
         select(Tool)
         .where(Tool.source.in_(["builtin", "admin"]))
@@ -229,7 +262,7 @@ async def list_tools(
             "enabled": t.enabled,
             "is_default": t.is_default,
             "source": t.source,
-            "config": company_config,
+            "config": mask_sensitive_fields(company_config, t.config_schema),
             "config_schema": t.config_schema or {},
             "created_at": t.created_at.isoformat() if t.created_at else None,
         })
@@ -248,6 +281,7 @@ async def create_tool(
     own tenant but can be overridden via data.tenant_id. This allows platform
     admins to import MCP tools while viewing another company's settings page.
     """
+    _require_tool_manager(current_user)
     # Resolve target tenant: explicit payload value takes priority so that
     # platform admins importing tools for another company work correctly.
     target_tenant_id = _resolve_target_tenant_id(current_user, data.tenant_id)
@@ -295,12 +329,16 @@ async def update_tools_bulk(
     db: AsyncSession = Depends(get_db),
 ):
     """Bulk update the enabled status of multiple tools."""
+    _require_tool_manager(current_user)
     tool_ids = [uuid.UUID(u.tool_id) for u in updates]
     result = await db.execute(select(Tool).where(Tool.id.in_(tool_ids)))
     tools_map = {str(t.id): t for t in result.scalars().all()}
 
     for update in updates:
         if update.tool_id in tools_map:
+            _require_tool_record_access(current_user, tools_map[update.tool_id])
+            if tools_map[update.tool_id].source == "builtin" and current_user.role != "platform_admin":
+                raise HTTPException(status_code=403, detail="Platform admin permission required")
             tools_map[update.tool_id].enabled = update.enabled
 
     await db.commit()
@@ -315,10 +353,12 @@ async def update_tool(
     db: AsyncSession = Depends(get_db),
 ):
     """Update a tool."""
+    _require_tool_manager(current_user)
     result = await db.execute(select(Tool).where(Tool.id == tool_id))
     tool = result.scalar_one_or_none()
     if not tool:
         raise HTTPException(status_code=404, detail="Tool not found")
+    _require_tool_record_access(current_user, tool)
 
     update_data = data.model_dump(exclude_unset=True)
     target_tenant_id = _resolve_target_tenant_id(current_user, update_data.pop("tenant_id", None))
@@ -332,6 +372,9 @@ async def update_tool(
         else:
             update_data["config"] = _encrypt_sensitive_fields(config_value, tool.config_schema)
 
+    if tool.source == "builtin" and update_data and current_user.role != "platform_admin":
+        raise HTTPException(status_code=403, detail="Platform admin permission required")
+
     for field, value in update_data.items():
         setattr(tool, field, value)
     await db.commit()
@@ -345,10 +388,12 @@ async def delete_tool(
     db: AsyncSession = Depends(get_db),
 ):
     """Delete a tool (only non-builtin)."""
+    _require_tool_manager(current_user)
     result = await db.execute(select(Tool).where(Tool.id == tool_id))
     tool = result.scalar_one_or_none()
     if not tool:
         raise HTTPException(status_code=404, detail="Tool not found")
+    _require_tool_record_access(current_user, tool)
     if tool.type == "builtin":
         raise HTTPException(status_code=400, detail="Cannot delete builtin tools")
 
@@ -366,12 +411,11 @@ async def get_agent_tools(
     db: AsyncSession = Depends(get_db),
 ):
     """Get tools for a specific agent with their enabled status."""
-    from app.services.agent_tools import _agent_has_feishu
-    has_feishu = await _agent_has_feishu(agent_id)
-
     # Determine if this is a system agent (e.g. OKR Agent).
     # System agents can see all tools; regular agents cannot see okr_agent_only tools.
-    agent_obj = await _load_agent_for_tool_scope(db, agent_id)
+    agent_obj = await _require_agent_tool_manager(db, current_user, agent_id)
+    from app.services.agent_tools import _agent_has_feishu
+    has_feishu = await _agent_has_feishu(agent_id)
     is_system_agent = bool(agent_obj and agent_obj.is_system)
 
     # Agent-specific assignments
@@ -456,7 +500,7 @@ async def update_agent_tools(
     db: AsyncSession = Depends(get_db),
 ):
     """Update tool assignments for an agent."""
-    agent_obj = await _load_agent_for_tool_scope(db, agent_id)
+    agent_obj = await _require_agent_tool_manager(db, current_user, agent_id)
     assignments = await _load_agent_tool_assignments(db, agent_id)
     for u in updates:
         tool_id = uuid.UUID(u.tool_id)
@@ -504,12 +548,7 @@ async def get_mcp_authorization_status(
     no_store_headers = {"Cache-Control": "no-store"}
 
     try:
-        agent = await _load_agent_for_tool_scope(db, agent_id)
-        if not await can_manage_agent(db, current_user, agent):
-            raise HTTPException(
-                status_code=403,
-                detail="Agent manage permission required",
-            )
+        await _require_agent_tool_manager(db, current_user, agent_id)
 
         connection = await _load_assigned_smithery_connection(
             db,
@@ -589,6 +628,7 @@ async def test_mcp_connection(
     - URL-embedded key (e.g. ?tavilyApiKey=xxx) — include in server_url.
     - Bearer token — pass via api_key field; sent as Authorization header.
     """
+    _require_tool_manager(current_user)
     from app.services.mcp_client import MCPClient
 
     try:
@@ -626,15 +666,8 @@ async def update_mcp_server(
     2. URL query param (e.g. ?tavilyApiKey=xxx) — extracted from the URL
        and converted to Bearer by MCPClient automatically.
     """
-    # Resolve target tenant
-    target_tenant_id: uuid.UUID | None = None
-    if data.tenant_id:
-        try:
-            target_tenant_id = uuid.UUID(data.tenant_id)
-        except ValueError:
-            raise HTTPException(status_code=400, detail="Invalid tenant_id format")
-    else:
-        target_tenant_id = current_user.tenant_id
+    _require_tool_manager(current_user)
+    target_tenant_id = _resolve_target_tenant_id(current_user, data.tenant_id)
 
     # Load all tools from this server under the target tenant
     result = await db.execute(
@@ -674,6 +707,7 @@ async def list_agent_installed_tools(
     db: AsyncSession = Depends(get_db),
 ):
     """Admin endpoint: list user-installed tools scoped by tenant."""
+    _require_tool_manager(current_user)
     from app.models.agent import Agent
     query = (
         select(AgentTool, Tool, Agent)
@@ -683,7 +717,8 @@ async def list_agent_installed_tools(
         .order_by(AgentTool.created_at.desc())
     )
     # Scope by tenant: only show tools installed by agents in this tenant
-    tid = tenant_id or (str(current_user.tenant_id) if current_user.tenant_id else None)
+    target_tenant_id = _resolve_target_tenant_id(current_user, tenant_id)
+    tid = str(target_tenant_id) if target_tenant_id else None
     if tid:
         from app.models.agent import Agent as Ag
         # Some local/prod databases still have agents.tenant_id as varchar from
@@ -724,10 +759,12 @@ async def delete_agent_tool(
     db: AsyncSession = Depends(get_db),
 ):
     """Admin: remove an agent-tool assignment. Also deletes the tool record if no other agents use it."""
+    _require_tool_manager(current_user)
     at_r = await db.execute(select(AgentTool).where(AgentTool.id == agent_tool_id))
     at = at_r.scalar_one_or_none()
     if not at:
         raise HTTPException(status_code=404, detail="Agent tool assignment not found")
+    await _require_agent_tool_manager(db, current_user, at.agent_id)
     tool_id = at.tool_id
     await db.delete(at)
     await db.flush()
@@ -760,11 +797,13 @@ async def get_agent_tool_config(
     Both configs are decrypted before returning. Global sensitive fields are
     masked so the frontend can show a key is configured without exposing it.
     """
+    agent = await _require_agent_tool_manager(db, current_user, agent_id)
     tool_r = await db.execute(select(Tool).where(Tool.id == tool_id))
     tool = tool_r.scalar_one_or_none()
-    if not tool:
+    if not tool or not _tool_record_visible_to_agent(
+        tool, agent.tenant_id, await _load_agent_tool_assignments(db, agent_id)
+    ):
         raise HTTPException(status_code=404, detail="Tool not found")
-    agent = await _load_agent_for_tool_scope(db, agent_id)
     at_r = await db.execute(
         select(AgentTool).where(AgentTool.agent_id == agent_id, AgentTool.tool_id == tool_id)
     )
@@ -781,7 +820,7 @@ async def get_agent_tool_config(
     # Merged: agent overrides take precedence over global defaults.
     # Use raw (non-masked) global as the base so the agent inherits actual values
     # at runtime, but the UI will show masked_global for display hints.
-    merged = {**raw_global, **(raw_agent or {})}
+    merged = {**masked_global, **(raw_agent or {})}
     return {
         "global_config": masked_global,
         "agent_config": raw_agent or {},
@@ -799,6 +838,7 @@ async def update_agent_tool_config(
     db: AsyncSession = Depends(get_db),
 ):
     """Save per-agent config override for a tool."""
+    agent = await _require_agent_tool_manager(db, current_user, agent_id)
     # Check permission: only platform_admin and org_admin can modify allow_network
     if "allow_network" in data.config:
         if current_user.role not in ("platform_admin", "org_admin"):
@@ -810,6 +850,10 @@ async def update_agent_tool_config(
     # Encrypt sensitive fields using the tool's config_schema for field type awareness
     tool_r2 = await db.execute(select(Tool).where(Tool.id == tool_id))
     tool_for_schema = tool_r2.scalar_one_or_none()
+    if not tool_for_schema or not _tool_record_visible_to_agent(
+        tool_for_schema, agent.tenant_id, await _load_agent_tool_assignments(db, agent_id)
+    ):
+        raise HTTPException(status_code=404, detail="Tool not found")
     encrypted_config = _encrypt_sensitive_fields(data.config, tool_for_schema.config_schema if tool_for_schema else None)
 
     at_r = await db.execute(
@@ -841,11 +885,10 @@ async def get_agent_tools_with_config(
     rather than Tool.config. We resolve those as part of the global config so
     the agent-level UI can show the inherited key hint.
     """
+    # Determine if this is a system agent (e.g. OKR Agent).
+    agent_obj2 = await _require_agent_tool_manager(db, current_user, agent_id)
     from app.services.agent_tools import _agent_has_feishu
     has_feishu = await _agent_has_feishu(agent_id)
-
-    # Determine if this is a system agent (e.g. OKR Agent).
-    agent_obj2 = await _load_agent_for_tool_scope(db, agent_id)
     is_system_agent2 = bool(agent_obj2 and agent_obj2.is_system)
 
     assignments = await _load_agent_tool_assignments(db, agent_id)
@@ -982,10 +1025,9 @@ async def get_category_config(
     Sensitive fields in global_config are masked for display.
     Company-level values always take precedence at runtime.
     """
-    from app.core.permissions import check_agent_access
     from app.models.channel_config import ChannelConfig
 
-    agent, _ = await check_agent_access(db, current_user, agent_id)
+    agent = await _require_agent_tool_manager(db, current_user, agent_id)
 
     # ── 1. Load company-level (global) config from Tool.config ──────────────
     # Find a tool in this category that actually has config data.
@@ -1036,7 +1078,7 @@ async def get_category_config(
     # ── 3. Build effective config ───────────────────────────────────────────
     # Priority: Agent config > Company config > Default
     # Agent can override company values by setting their own.
-    effective_config = {**raw_global, **raw_agent}
+    effective_config = {**masked_global, **raw_agent}
 
     return {
         "id": config_id,
@@ -1060,10 +1102,10 @@ async def update_category_config(
     db: AsyncSession = Depends(get_db),
 ):
     """Update or create shared configuration for a tool category."""
-    from app.core.permissions import check_agent_access, is_agent_creator
+    from app.core.permissions import is_agent_creator
     from app.models.channel_config import ChannelConfig
 
-    agent, _ = await check_agent_access(db, current_user, agent_id)
+    agent = await _require_agent_tool_manager(db, current_user, agent_id)
     if not is_agent_creator(current_user, agent):
         raise HTTPException(status_code=403, detail="Only creator can configure category")
 
@@ -1117,10 +1159,10 @@ async def delete_category_config(
     db: AsyncSession = Depends(get_db),
 ):
     """Remove shared configuration for a tool category."""
-    from app.core.permissions import check_agent_access, is_agent_creator
+    from app.core.permissions import is_agent_creator
     from app.models.channel_config import ChannelConfig
 
-    agent, _ = await check_agent_access(db, current_user, agent_id)
+    agent = await _require_agent_tool_manager(db, current_user, agent_id)
     if not is_agent_creator(current_user, agent):
         raise HTTPException(status_code=403, detail="Only creator can remove config")
 
@@ -1141,6 +1183,7 @@ async def test_category_config(
     db: AsyncSession = Depends(get_db),
 ):
     """Test connectivity for a tool category."""
+    await _require_agent_tool_manager(db, current_user, agent_id)
     if category == "atlassian":
         from app.api.atlassian import test_atlassian_channel
         return await test_atlassian_channel(agent_id, current_user, db)

backend/tests/test_tool_tenant_scope.py

@@ -1,7 +1,15 @@
 import uuid
 from types import SimpleNamespace
 
-from app.api.tools import _tool_record_visible_to_agent
+import pytest
+from fastapi import HTTPException
+
+from app.api.tools import (
+    _require_tool_manager,
+    _require_tool_record_access,
+    _resolve_target_tenant_id,
+    _tool_record_visible_to_agent,
+)
 
 
 def make_tool(**overrides):
@@ -39,3 +47,39 @@ def test_agent_installed_tools_require_explicit_assignment():
     assert _tool_record_visible_to_agent(installed_tool, tenant_id, {}) is False
     assert _tool_record_visible_to_agent(installed_tool, tenant_id, {str(tool_id): object()}) is True
 
+
+def make_user(tenant_id: uuid.UUID, role: str = "user"):
+    return SimpleNamespace(tenant_id=tenant_id, role=role)
+
+
+def test_regular_users_cannot_access_tool_management():
+    with pytest.raises(HTTPException, match="Tool management permission required") as error:
+        _require_tool_manager(make_user(uuid.uuid4()))
+
+    assert error.value.status_code == 403
+
+
+def test_org_admin_cannot_select_another_tenant_for_tools():
+    user = make_user(uuid.uuid4(), role="org_admin")
+
+    with pytest.raises(HTTPException, match="No access to this tenant") as error:
+        _resolve_target_tenant_id(user, str(uuid.uuid4()))
+
+    assert error.value.status_code == 403
+
+
+def test_platform_admin_can_select_another_tenant_for_tools():
+    target_tenant_id = uuid.uuid4()
+    user = make_user(uuid.uuid4(), role="platform_admin")
+
+    assert _resolve_target_tenant_id(user, str(target_tenant_id)) == target_tenant_id
+
+
+def test_org_admin_cannot_mutate_a_foreign_tenant_tool():
+    user = make_user(uuid.uuid4(), role="org_admin")
+    foreign_tool = make_tool(tenant_id=uuid.uuid4())
+
+    with pytest.raises(HTTPException, match="No access to this tenant") as error:
+        _require_tool_record_access(user, foreign_tool)
+
+    assert error.value.status_code == 403