fix: llm proxy hardening and behavior consistency h-batch-7-final (#3120)

* fix: llm proxy hardening and behavior consistency

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* Update AGENTS.md

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* chore: lint docstring hardening and behavior consistency

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix: harden alembic sqlite migration compatibility

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix: tighten llm token scoping and update rbac docs

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

Author: crivetimihai

AGENTS.md

@@ -95,6 +95,22 @@ The `teams` claim in JWT tokens determines resource visibility:
 - Admin bypass requires BOTH `teams: null` AND `is_admin: true`
 - `normalize_token_teams()` in `mcpgateway/auth.py` is the single source of truth
 
+### Security Invariants (Required)
+
+- Treat `public` as platform-public scope, not internet-anonymous scope.
+- Explicit exception: when `MCP_REQUIRE_AUTH=false`, unauthenticated `/mcp` requests are allowed with public-only visibility.
+- Keep the two-layer model on every path:
+  - Layer 1: token scoping controls what a caller can see.
+  - Layer 2: RBAC controls what a caller can do.
+- Do not re-implement token team interpretation logic; always use `normalize_token_teams()` in `mcpgateway/auth.py`.
+- Do not accept inbound client auth tokens via URL query parameters.
+- Legacy `INSECURE_ALLOW_QUERYPARAM_AUTH` is interop-only for outbound peer auth and must remain opt-in and host-restricted.
+- High-risk transports must be feature-flagged and disabled by default.
+- Transport/session endpoints must authenticate before session establishment (or message forwarding) and enforce RBAC before processing actions.
+- Token-scoped route authorization must be default-deny for unmapped protected paths.
+- Never trust client-provided ownership fields (`owner_email`, `team_id`, session owner); derive authorization from authenticated identity and server-side state.
+- Security-sensitive changes must include deny-path regression tests (unauthenticated, wrong team, insufficient permissions, feature disabled).
+
 ### Built-in Roles
 
 | Role | Scope | Key Permissions |

docs/docs/manage/rbac.md

@@ -63,10 +63,10 @@ Logical groups that:
 | Role | Scope | Permissions |
 |------|-------|-------------|
 | `platform_admin` | global | `["*"]` (all permissions) |
-| `team_admin` | team | admin.dashboard, gateways.read, gateways.create, gateways.update, gateways.delete, servers.read, servers.create, servers.update, servers.delete, teams.read, teams.update, teams.join, teams.delete, teams.manage_members, tools.read, tools.create, tools.update, tools.delete, tools.execute, resources.read, resources.create, resources.update, resources.delete, prompts.read, prompts.create, prompts.update, prompts.delete, a2a.read, a2a.create, a2a.update, a2a.delete, a2a.invoke |
-| `developer` | team | admin.dashboard, gateways.read, gateways.create, gateways.update, gateways.delete, servers.read, servers.create, servers.update, servers.delete, teams.join, tools.read, tools.create, tools.update, tools.delete, tools.execute, resources.read, resources.create, resources.update, resources.delete, prompts.read, prompts.create, prompts.update, prompts.delete, a2a.read, a2a.create, a2a.update, a2a.delete, a2a.invoke |
-| `viewer` | team | admin.dashboard, gateways.read, servers.read, teams.join, tools.read, resources.read, prompts.read, a2a.read |
-| `platform_viewer` | global | admin.dashboard, gateways.read, servers.read, teams.join, tools.read, resources.read, prompts.read, a2a.read |
+| `team_admin` | team | admin.dashboard, gateways.read, gateways.create, gateways.update, gateways.delete, servers.read, servers.create, servers.update, servers.delete, teams.read, teams.update, teams.join, teams.delete, teams.manage_members, tools.read, tools.create, tools.update, tools.delete, tools.execute, resources.read, resources.create, resources.update, resources.delete, prompts.read, prompts.create, prompts.update, prompts.delete, a2a.read, a2a.create, a2a.update, a2a.delete, a2a.invoke, llm.read, llm.invoke |
+| `developer` | team | admin.dashboard, gateways.read, gateways.create, gateways.update, gateways.delete, servers.read, servers.create, servers.update, servers.delete, teams.join, tools.read, tools.create, tools.update, tools.delete, tools.execute, resources.read, resources.create, resources.update, resources.delete, prompts.read, prompts.create, prompts.update, prompts.delete, a2a.read, a2a.create, a2a.update, a2a.delete, a2a.invoke, llm.read, llm.invoke |
+| `viewer` | team | admin.dashboard, gateways.read, servers.read, teams.join, tools.read, resources.read, prompts.read, a2a.read, llm.read |
+| `platform_viewer` | global | admin.dashboard, gateways.read, servers.read, teams.join, tools.read, resources.read, prompts.read, a2a.read, llm.read |
 
 !!! info "Default Role Assignment"
     **New users automatically receive up to two roles upon creation:**

mcpgateway/alembic/versions/191a2def08d7_resource_rename_template_to_uri_template.py

@@ -25,6 +25,9 @@ def upgrade() -> None:
     conn = op.get_bind()
     inspector = sa.inspect(conn)
 
+    if "resources" not in inspector.get_table_names():
+        return
+
     columns = [c["name"] for c in inspector.get_columns("resources")]
 
     # Only rename if old column exists
@@ -38,6 +41,9 @@ def downgrade() -> None:
     conn = op.get_bind()
     inspector = sa.inspect(conn)
 
+    if "resources" not in inspector.get_table_names():
+        return
+
     columns = [c["name"] for c in inspector.get_columns("resources")]
 
     # Only rename back if current column exists

mcpgateway/alembic/versions/43c07ed25a24_add_oauth_fields_to_servers.py

@@ -24,17 +24,58 @@
 depends_on: Union[str, Sequence[str], None] = None
 
 
+def _table_exists(table_name: str) -> bool:
+    """Check whether a table exists.
+
+    Args:
+        table_name: Name of table to inspect.
+
+    Returns:
+        True if table exists, False otherwise.
+    """
+    bind = op.get_bind()
+    inspector = sa.inspect(bind)
+    return table_name in inspector.get_table_names()
+
+
+def _column_exists(table_name: str, column_name: str) -> bool:
+    """Check whether a column exists in a table.
+
+    Args:
+        table_name: Name of table to inspect.
+        column_name: Name of column to inspect.
+
+    Returns:
+        True if column exists, False otherwise.
+    """
+    if not _table_exists(table_name):
+        return False
+    bind = op.get_bind()
+    inspector = sa.inspect(bind)
+    return any(col["name"] == column_name for col in inspector.get_columns(table_name))
+
+
 def upgrade() -> None:
     """Add OAuth configuration fields to servers table."""
+    if not _table_exists("servers"):
+        return
+
     # Add oauth_enabled column with default False
     # Use sa.false() for cross-database compatibility (SQLite, PostgreSQL)
-    op.add_column("servers", sa.Column("oauth_enabled", sa.Boolean(), nullable=False, server_default=sa.false()))
+    if not _column_exists("servers", "oauth_enabled"):
+        op.add_column("servers", sa.Column("oauth_enabled", sa.Boolean(), nullable=False, server_default=sa.false()))
 
     # Add oauth_config column for storing OAuth configuration as JSON
-    op.add_column("servers", sa.Column("oauth_config", sa.JSON(), nullable=True))
+    if not _column_exists("servers", "oauth_config"):
+        op.add_column("servers", sa.Column("oauth_config", sa.JSON(), nullable=True))
 
 
 def downgrade() -> None:
     """Remove OAuth configuration fields from servers table."""
-    op.drop_column("servers", "oauth_config")
-    op.drop_column("servers", "oauth_enabled")
+    if not _table_exists("servers"):
+        return
+
+    if _column_exists("servers", "oauth_config"):
+        op.drop_column("servers", "oauth_config")
+    if _column_exists("servers", "oauth_enabled"):
+        op.drop_column("servers", "oauth_enabled")

mcpgateway/alembic/versions/4f07c116f917_add_indexes_for_pagination.py

@@ -53,6 +53,23 @@ def _index_exists(table_name: str, index_name: str) -> bool:
         return False
 
 
+def _table_exists(table_name: str) -> bool:
+    """Check if a table exists.
+
+    Args:
+        table_name: Name of the table to check.
+
+    Returns:
+        True if table exists, False otherwise.
+    """
+    conn = op.get_bind()
+    inspector = inspect(conn)
+    try:
+        return table_name in inspector.get_table_names()
+    except Exception:
+        return False
+
+
 def _create_index_safe(index_name: str, table_name: str, columns: list[str], unique: bool = False) -> bool:
     """Create an index only if it doesn't already exist.
 
@@ -65,6 +82,10 @@ def _create_index_safe(index_name: str, table_name: str, columns: list[str], uni
     Returns:
         True if index was created, False if it already existed
     """
+    if not _table_exists(table_name):
+        print(f"⚠️  Skipping {index_name}: Table '{table_name}' does not exist")
+        return False
+
     if _index_exists(table_name, index_name):
         print(f"⚠️  Skipping {index_name}: Index already exists on {table_name}")
         return False
@@ -84,6 +105,10 @@ def _drop_index_safe(index_name: str, table_name: str) -> bool:
     Returns:
         True if index was dropped, False if it didn't exist
     """
+    if not _table_exists(table_name):
+        print(f"⚠️  Skipping drop of {index_name}: Table '{table_name}' does not exist")
+        return False
+
     if not _index_exists(table_name, index_name):
         print(f"⚠️  Skipping drop of {index_name}: Index does not exist on {table_name}")
         return False

mcpgateway/alembic/versions/9f5d93ced2b3_add_llm_permissions_to_default_roles.py

@@ -0,0 +1,147 @@
+# -*- coding: utf-8 -*-
+"""Add LLM permissions to default RBAC roles.
+
+Revision ID: 9f5d93ced2b3
+Revises: y8i9j0k1l2m3
+Create Date: 2026-02-23 11:09:14.709030
+
+Backfills default role permission sets so existing deployments receive:
+- team_admin: llm.read, llm.invoke
+- developer: llm.read, llm.invoke
+- viewer: llm.read
+- platform_viewer: llm.read
+"""
+
+# Standard
+from datetime import datetime, timezone
+import json
+from typing import Sequence, Union
+
+# Third-Party
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy import text
+
+# revision identifiers, used by Alembic.
+revision: str = "9f5d93ced2b3"
+down_revision: Union[str, Sequence[str], None] = "y8i9j0k1l2m3"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+ROLE_PERMISSION_ADDITIONS = {
+    "team_admin": ["llm.read", "llm.invoke"],
+    "developer": ["llm.read", "llm.invoke"],
+    "viewer": ["llm.read"],
+    "platform_viewer": ["llm.read"],
+}
+
+
+def _load_permissions(raw_permissions: object) -> list[str]:
+    """Normalize stored role permissions into a list of strings.
+
+    Args:
+        raw_permissions: Raw permissions value from the role row.
+
+    Returns:
+        list[str]: Normalized list of permission strings.
+    """
+    if not raw_permissions:
+        return []
+
+    parsed = raw_permissions
+    if isinstance(parsed, (bytes, bytearray)):
+        parsed = parsed.decode("utf-8")
+
+    if isinstance(parsed, str):
+        try:
+            parsed = json.loads(parsed)
+        except json.JSONDecodeError:
+            return []
+
+    if isinstance(parsed, list):
+        return [perm for perm in parsed if isinstance(perm, str)]
+
+    return []
+
+
+def _update_role_permissions(conn, role_name: str, permissions: list[str], add: bool) -> None:
+    """Add or remove permissions from a role, idempotently.
+
+    Args:
+        conn: Active Alembic connection.
+        role_name: Role name to update.
+        permissions: Permission values to add or remove.
+        add: When True, add permissions; when False, remove permissions.
+    """
+    row = conn.execute(
+        text("SELECT id, permissions FROM roles WHERE name = :name LIMIT 1"),
+        {"name": role_name},
+    ).fetchone()
+
+    if not row:
+        print(f"{role_name} role not found. Skipping.")
+        return
+
+    role_id = row[0]
+    current_permissions = _load_permissions(row[1])
+
+    if add:
+        updated_permissions = list(current_permissions)
+        for permission in permissions:
+            if permission not in updated_permissions:
+                updated_permissions.append(permission)
+    else:
+        updated_permissions = [permission for permission in current_permissions if permission not in permissions]
+
+    if updated_permissions == current_permissions:
+        return
+
+    dialect_name = conn.dialect.name
+    if dialect_name == "postgresql":
+        update_query = text(
+            """
+            UPDATE roles
+            SET permissions = CAST(:permissions AS JSONB), updated_at = :updated_at
+            WHERE id = :role_id
+            """
+        )
+    else:
+        update_query = text(
+            """
+            UPDATE roles
+            SET permissions = :permissions, updated_at = :updated_at
+            WHERE id = :role_id
+            """
+        )
+
+    conn.execute(
+        update_query,
+        {
+            "permissions": json.dumps(updated_permissions),
+            "updated_at": datetime.now(timezone.utc),
+            "role_id": role_id,
+        },
+    )
+
+
+def upgrade() -> None:
+    """Backfill LLM permissions into default role records."""
+    conn = op.get_bind()
+    inspector = sa.inspect(conn)
+    if "roles" not in inspector.get_table_names():
+        print("roles table not found. Skipping migration.")
+        return
+
+    for role_name, permissions in ROLE_PERMISSION_ADDITIONS.items():
+        _update_role_permissions(conn, role_name, permissions, add=True)
+
+
+def downgrade() -> None:
+    """Remove LLM permissions from default role records."""
+    conn = op.get_bind()
+    inspector = sa.inspect(conn)
+    if "roles" not in inspector.get_table_names():
+        return
+
+    for role_name, permissions in ROLE_PERMISSION_ADDITIONS.items():
+        _update_role_permissions(conn, role_name, permissions, add=False)