mirror of
https://github.com/open-webui/open-webui.git
synced 2026-10-09 03:18:18 +00:00
fix: reject collection names with unsafe characters in RAG ACL
Open WebUI's collection ACL accepted any unknown name as a legacy/ephemeral collection. In Milvus multi-tenancy mode that name becomes the `resource_id` and is interpolated unescaped into a SQL-like Milvus expression — `resource_id == '<name>'` — so a name like x' or resource_id != '' or resource_id == 'x turns the filter into a tautology and returns every tenant's chunks from the shared collection. All collection names Open WebUI generates are UUIDs, SHA-256 hex digests, or fixed-prefix variants of those — they all fit [A-Za-z0-9_-]. Add a strict format check in filter_accessible_collections (utils.py) that drops any name outside that set before any ACL or vector-store lookup, applied even on the admin bypass path. _validate_collection_access then surfaces the dropped name as a 403. As defense in depth, MilvusClient now validates resource_id at every expression-construction site and escapes single quotes / backslashes in any other string interpolated into a filter (delete ids, metadata filter values). Non-string filter values are typed-checked instead of str()-formatted.
This commit is contained in:
parent
260ead64da
commit
ddec5556dd
2 changed files with 67 additions and 7 deletions
|
|
@ -1063,6 +1063,16 @@ def get_reranking_function(reranking_engine, reranking_model, reranking_function
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# UUIDs, SHA-256 digests, and prefixed variants thereof all fit [A-Za-z0-9_-].
|
||||||
|
# Anything else cannot be a real Open WebUI collection and could break out of
|
||||||
|
# a Milvus expression literal.
|
||||||
|
_SAFE_COLLECTION_NAME_RE = re.compile(r'^[A-Za-z0-9_-]{1,255}$')
|
||||||
|
|
||||||
|
|
||||||
|
def _is_safe_collection_name(name: str) -> bool:
|
||||||
|
return isinstance(name, str) and bool(_SAFE_COLLECTION_NAME_RE.match(name))
|
||||||
|
|
||||||
|
|
||||||
async def filter_accessible_collections(
|
async def filter_accessible_collections(
|
||||||
collection_names: set[str],
|
collection_names: set[str],
|
||||||
user: UserModel,
|
user: UserModel,
|
||||||
|
|
@ -1072,6 +1082,7 @@ async def filter_accessible_collections(
|
||||||
Return only the collection names the user is allowed to access.
|
Return only the collection names the user is allowed to access.
|
||||||
Admins bypass all checks. For non-admins the policy is:
|
Admins bypass all checks. For non-admins the policy is:
|
||||||
|
|
||||||
|
- any name with characters outside [A-Za-z0-9_-] → rejected
|
||||||
- file-* → validated via has_access_to_file
|
- file-* → validated via has_access_to_file
|
||||||
- user-memory-* → must match user's own memory collection
|
- user-memory-* → must match user's own memory collection
|
||||||
- web-search-* → ephemeral per-query collections, always allowed
|
- web-search-* → ephemeral per-query collections, always allowed
|
||||||
|
|
@ -1081,11 +1092,22 @@ async def filter_accessible_collections(
|
||||||
such KB exists, the name is treated as an
|
such KB exists, the name is treated as an
|
||||||
ephemeral/legacy collection and allowed
|
ephemeral/legacy collection and allowed
|
||||||
"""
|
"""
|
||||||
|
# Applied before the admin bypass — malformed names should never reach the vector store.
|
||||||
|
safe_names = {n for n in collection_names if _is_safe_collection_name(n)}
|
||||||
|
rejected = collection_names - safe_names
|
||||||
|
if rejected:
|
||||||
|
log.warning(
|
||||||
|
'filter_accessible_collections: rejected %d collection name(s) '
|
||||||
|
'with unsafe characters (user_id=%s)',
|
||||||
|
len(rejected),
|
||||||
|
getattr(user, 'id', '<unknown>'),
|
||||||
|
)
|
||||||
|
|
||||||
if user.role == 'admin':
|
if user.role == 'admin':
|
||||||
return collection_names
|
return safe_names
|
||||||
|
|
||||||
validated = set()
|
validated = set()
|
||||||
for name in collection_names:
|
for name in safe_names:
|
||||||
if name == 'knowledge-bases':
|
if name == 'knowledge-bases':
|
||||||
# System meta-collection — never exposed to non-admins.
|
# System meta-collection — never exposed to non-admins.
|
||||||
continue
|
continue
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@ NOTE: This vector database integration is community-supported and maintained on
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import re
|
||||||
from typing import Any, Dict, List, Optional, Tuple
|
from typing import Any, Dict, List, Optional, Tuple
|
||||||
|
|
||||||
from open_webui.config import (
|
from open_webui.config import (
|
||||||
|
|
@ -35,6 +36,30 @@ log = logging.getLogger(__name__)
|
||||||
|
|
||||||
RESOURCE_ID_FIELD = 'resource_id'
|
RESOURCE_ID_FIELD = 'resource_id'
|
||||||
|
|
||||||
|
# Milvus expressions are SQL-like strings with no parameterized-query API;
|
||||||
|
# values get interpolated into single-quoted literals. Reject anything that
|
||||||
|
# can't be a legitimate Open WebUI collection name.
|
||||||
|
_SAFE_RESOURCE_ID_RE = re.compile(r'^[A-Za-z0-9_-]{1,255}$')
|
||||||
|
_SAFE_METADATA_KEY_RE = re.compile(r'^[A-Za-z_][A-Za-z0-9_]{0,63}$')
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_resource_id(resource_id: str) -> str:
|
||||||
|
if not isinstance(resource_id, str) or not _SAFE_RESOURCE_ID_RE.match(resource_id):
|
||||||
|
raise ValueError(f'Invalid Milvus resource_id (collection name): {resource_id!r}')
|
||||||
|
return resource_id
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_metadata_key(key: str) -> str:
|
||||||
|
if not isinstance(key, str) or not _SAFE_METADATA_KEY_RE.match(key):
|
||||||
|
raise ValueError(f'Invalid Milvus metadata filter key: {key!r}')
|
||||||
|
return key
|
||||||
|
|
||||||
|
|
||||||
|
def _escape_milvus_string(value: str) -> str:
|
||||||
|
if not isinstance(value, str):
|
||||||
|
raise TypeError(f'Expected str for Milvus expression value, got {type(value).__name__}')
|
||||||
|
return value.replace('\\', '\\\\').replace("'", "\\'")
|
||||||
|
|
||||||
|
|
||||||
class MilvusClient(VectorDBBase):
|
class MilvusClient(VectorDBBase):
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
|
|
@ -126,6 +151,7 @@ class MilvusClient(VectorDBBase):
|
||||||
|
|
||||||
def has_collection(self, collection_name: str) -> bool:
|
def has_collection(self, collection_name: str) -> bool:
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
if not utility.has_collection(mt_collection):
|
if not utility.has_collection(mt_collection):
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
@ -138,6 +164,7 @@ class MilvusClient(VectorDBBase):
|
||||||
if not items:
|
if not items:
|
||||||
return
|
return
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
dimension = len(items[0]['vector'])
|
dimension = len(items[0]['vector'])
|
||||||
self._ensure_collection(mt_collection, dimension)
|
self._ensure_collection(mt_collection, dimension)
|
||||||
collection = Collection(mt_collection)
|
collection = Collection(mt_collection)
|
||||||
|
|
@ -165,6 +192,7 @@ class MilvusClient(VectorDBBase):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
if not utility.has_collection(mt_collection):
|
if not utility.has_collection(mt_collection):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
@ -203,21 +231,22 @@ class MilvusClient(VectorDBBase):
|
||||||
filter: Optional[Dict[str, Any]] = None,
|
filter: Optional[Dict[str, Any]] = None,
|
||||||
):
|
):
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
if not utility.has_collection(mt_collection):
|
if not utility.has_collection(mt_collection):
|
||||||
return
|
return
|
||||||
|
|
||||||
collection = Collection(mt_collection)
|
collection = Collection(mt_collection)
|
||||||
|
|
||||||
# Build expression
|
|
||||||
expr = [f"{RESOURCE_ID_FIELD} == '{resource_id}'"]
|
expr = [f"{RESOURCE_ID_FIELD} == '{resource_id}'"]
|
||||||
if ids:
|
if ids:
|
||||||
# Milvus expects a string list for 'in' operator
|
# Milvus expects a string list for 'in' operator
|
||||||
id_list_str = ', '.join([f"'{id_val}'" for id_val in ids])
|
id_list_str = ', '.join([f"'{_escape_milvus_string(str(id_val))}'" for id_val in ids])
|
||||||
expr.append(f'id in [{id_list_str}]')
|
expr.append(f'id in [{id_list_str}]')
|
||||||
|
|
||||||
if filter:
|
if filter:
|
||||||
for key, value in filter.items():
|
for key, value in filter.items():
|
||||||
expr.append(f"metadata['{key}'] == '{value}'")
|
_validate_metadata_key(key)
|
||||||
|
expr.append(f"metadata['{key}'] == '{_escape_milvus_string(str(value))}'")
|
||||||
|
|
||||||
collection.delete(' and '.join(expr))
|
collection.delete(' and '.join(expr))
|
||||||
|
|
||||||
|
|
@ -228,6 +257,7 @@ class MilvusClient(VectorDBBase):
|
||||||
|
|
||||||
def delete_collection(self, collection_name: str):
|
def delete_collection(self, collection_name: str):
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
if not utility.has_collection(mt_collection):
|
if not utility.has_collection(mt_collection):
|
||||||
return
|
return
|
||||||
|
|
||||||
|
|
@ -236,6 +266,7 @@ class MilvusClient(VectorDBBase):
|
||||||
|
|
||||||
def query(self, collection_name: str, filter: Dict[str, Any], limit: Optional[int] = None) -> Optional[GetResult]:
|
def query(self, collection_name: str, filter: Dict[str, Any], limit: Optional[int] = None) -> Optional[GetResult]:
|
||||||
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
mt_collection, resource_id = self._get_collection_and_resource_id(collection_name)
|
||||||
|
_validate_resource_id(resource_id)
|
||||||
if not utility.has_collection(mt_collection):
|
if not utility.has_collection(mt_collection):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
@ -245,10 +276,17 @@ class MilvusClient(VectorDBBase):
|
||||||
expr = [f"{RESOURCE_ID_FIELD} == '{resource_id}'"]
|
expr = [f"{RESOURCE_ID_FIELD} == '{resource_id}'"]
|
||||||
if filter:
|
if filter:
|
||||||
for key, value in filter.items():
|
for key, value in filter.items():
|
||||||
|
_validate_metadata_key(key)
|
||||||
if isinstance(value, str):
|
if isinstance(value, str):
|
||||||
expr.append(f"metadata['{key}'] == '{value}'")
|
expr.append(f"metadata['{key}'] == '{_escape_milvus_string(value)}'")
|
||||||
else:
|
elif isinstance(value, bool):
|
||||||
|
expr.append(f"metadata['{key}'] == {str(value).lower()}")
|
||||||
|
elif isinstance(value, (int, float)):
|
||||||
expr.append(f"metadata['{key}'] == {value}")
|
expr.append(f"metadata['{key}'] == {value}")
|
||||||
|
else:
|
||||||
|
raise TypeError(
|
||||||
|
f'Unsupported Milvus filter value type for key {key!r}: {type(value).__name__}'
|
||||||
|
)
|
||||||
|
|
||||||
iterator = collection.query_iterator(
|
iterator = collection.query_iterator(
|
||||||
expr=' and '.join(expr),
|
expr=' and '.join(expr),
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue