From 91e016d2e4def8252d61d6aa1512f3c0393237a8 Mon Sep 17 00:00:00 2001 From: mountain Date: Thu, 9 Jul 2026 19:12:56 +0800 Subject: [PATCH] fix: reject trailing newline in collection-name validation Python's $ anchor matches just before a final newline, so a $-anchored re.match(r'^[a-zA-Z0-9_-]{1,128}$', name) accepted "papers\n". In local mode get_or_create_collection() then hit SQLite's CHECK via INSERT OR IGNORE, silently created no row, and returned a Collection that failed later on add(). Switch all three duplicated validators (local, cloud, sqlite backends) to re.fullmatch() so the whole string must match. --- pageindex/backend/cloud.py | 4 +++- pageindex/backend/local.py | 6 ++++-- pageindex/storage/sqlite.py | 6 ++++-- tests/test_local_backend.py | 11 +++++++++++ tests/test_sqlite_storage.py | 4 ++++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pageindex/backend/cloud.py b/pageindex/backend/cloud.py index 0dfdcfe..69ce703 100644 --- a/pageindex/backend/cloud.py +++ b/pageindex/backend/cloud.py @@ -89,7 +89,9 @@ class CloudBackend: @staticmethod def _validate_collection_name(name: str) -> None: - if not re.match(r'^[a-zA-Z0-9_-]{1,128}$', name): + # .fullmatch() (not .match()): a $-anchored .match() would accept a + # trailing newline ("papers\n") because $ matches just before a final \n. + if not re.fullmatch(r'[a-zA-Z0-9_-]{1,128}', name): raise PageIndexError( f"Invalid collection name: {name!r}. " "Must be 1-128 chars of [a-zA-Z0-9_-]." diff --git a/pageindex/backend/local.py b/pageindex/backend/local.py index c7d7200..ec68943 100644 --- a/pageindex/backend/local.py +++ b/pageindex/backend/local.py @@ -17,7 +17,9 @@ from ..backend.protocol import AgentTools from ..errors import (FileTypeError, DocumentNotFoundError, CollectionNotFoundError, IndexingError, PageIndexError) -_COLLECTION_NAME_RE = re.compile(r'^[a-zA-Z0-9_-]{1,128}$') +# Matched with .fullmatch() (not .match()): a $-anchored .match() would accept a +# trailing newline ("papers\n") because $ matches just before a final \n. +_COLLECTION_NAME_RE = re.compile(r'[a-zA-Z0-9_-]{1,128}') class LocalBackend: @@ -45,7 +47,7 @@ class LocalBackend: # Collection management def _validate_collection_name(self, name: str) -> None: - if not _COLLECTION_NAME_RE.match(name): + if not _COLLECTION_NAME_RE.fullmatch(name): raise PageIndexError(f"Invalid collection name: {name!r}. Must be 1-128 chars of [a-zA-Z0-9_-].") def create_collection(self, name: str) -> None: diff --git a/pageindex/storage/sqlite.py b/pageindex/storage/sqlite.py index bb0a461..20d5997 100644 --- a/pageindex/storage/sqlite.py +++ b/pageindex/storage/sqlite.py @@ -10,11 +10,13 @@ from ..errors import CollectionAlreadyExistsError, PageIndexError # itself (not just relying on LocalBackend's pre-check or the schema's CHECK # constraint below) because it's a public StorageEngine that can be used # directly, bypassing LocalBackend entirely. -_COLLECTION_NAME_RE = re.compile(r'^[a-zA-Z0-9_-]{1,128}$') +# Matched with .fullmatch() (not .match()): a $-anchored .match() would accept a +# trailing newline ("papers\n") because $ matches just before a final \n. +_COLLECTION_NAME_RE = re.compile(r'[a-zA-Z0-9_-]{1,128}') def _validate_collection_name(name: str) -> None: - if not _COLLECTION_NAME_RE.match(name): + if not _COLLECTION_NAME_RE.fullmatch(name): raise PageIndexError(f"Invalid collection name: {name!r}. Must be 1-128 chars of [a-zA-Z0-9_-].") diff --git a/tests/test_local_backend.py b/tests/test_local_backend.py index 72b4129..ccc0f23 100644 --- a/tests/test_local_backend.py +++ b/tests/test_local_backend.py @@ -188,6 +188,17 @@ def test_delete_collection_rejects_path_traversal(backend, tmp_path): assert canary.exists() +@pytest.mark.parametrize("bad_name", ["papers\n", "\npapers", "papers\n\n"]) +def test_get_or_create_collection_rejects_trailing_newline(backend, bad_name): + # Regression: Python's $ matches just before a final \n, so a $-anchored + # .match() accepted "papers\n"; get_or_create_collection then hit the SQL + # CHECK via INSERT OR IGNORE, silently created no row, and handed back a + # Collection that failed later on add(). .fullmatch() rejects it up front. + from pageindex.errors import PageIndexError + with pytest.raises(PageIndexError, match="Invalid collection name"): + backend.get_or_create_collection(bad_name) + + def test_add_document_missing_file_raises_file_not_found(backend, tmp_path): backend.get_or_create_collection("papers") with pytest.raises(FileNotFoundError): diff --git a/tests/test_sqlite_storage.py b/tests/test_sqlite_storage.py index 3fa2a43..aa8f757 100644 --- a/tests/test_sqlite_storage.py +++ b/tests/test_sqlite_storage.py @@ -36,6 +36,10 @@ def test_create_duplicate_collection_raises_pageindex_error(storage): @pytest.mark.parametrize("bad_name", [ "a/../../etc/passwd", "/etc/passwd", "a$(whoami)", ".hidden", "a b", "válid", "", "a" * 129, + # A trailing newline must be rejected: Python's $ matches just before a + # final \n, so a $-anchored .match() would let "papers\n" slip through + # (then INSERT OR IGNORE silently no-ops on the SQL CHECK). + "papers\n", "\npapers", "papers\n\n", ]) def test_create_collection_rejects_invalid_names_at_the_python_layer(storage, bad_name): """SQLiteStorage must validate collection names itself — it's a public