From 3f8d8417f6acccfba217774a24ec331afd91aa0b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 9 Sep 2026 12:27:16 -0700 Subject: [PATCH] Inherit source permissions for FTS vocabulary tables --- datasette/app.py | 15 +++- datasette/utils/sqlite.py | 56 +++++++++++++- docs/authentication.rst | 5 ++ tests/test_pr76_fts_policy.py | 138 ++++++++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 7 deletions(-) create mode 100644 tests/test_pr76_fts_policy.py diff --git a/datasette/app.py b/datasette/app.py index acce7a4a..76c48825 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1858,7 +1858,7 @@ allowed AS ( SELECT {select_columns} FROM allowed LEFT JOIN derived_permissions AS derived - ON allowed.parent = derived.parent AND allowed.child = derived.child + ON allowed.parent = derived.parent AND allowed.child = derived.child COLLATE NOCASE WHERE COALESCE(derived.source_allowed, 1) = 1 ORDER BY allowed.parent, allowed.child """.strip() @@ -2128,9 +2128,16 @@ ORDER BY allowed.parent, allowed.child and isinstance(resource, TableResource) and parent in self.databases ): - dependency = ( - await self.databases[parent].derived_table_dependencies() - ).get(child) + dependency = await self.databases[parent].derived_table_dependencies() + dependency = next( + ( + source + for table, source in dependency.items() + if TableResource.normalize_child(table) + == TableResource.normalize_child(child) + ), + None, + ) if dependency is not None: stack = _derived_permission_stack.get() dependency_key = (parent, dependency) diff --git a/datasette/utils/sqlite.py b/datasette/utils/sqlite.py index 11a6a926..27d8aebb 100644 --- a/datasette/utils/sqlite.py +++ b/datasette/utils/sqlite.py @@ -15,8 +15,17 @@ if hasattr(sqlite3, "enable_callback_tracebacks"): _cached_sqlite_version = None _cached_supports_returning = None SQLiteTableType = Literal["table", "view", "virtual", "shadow"] +_SQLITE_IDENTIFIER_RE = ( + r"""(?:"(?:[^"]|"")*"|'(?:[^']|'')*'|`(?:[^`]|``)*`|\[[^\]]*\]|[^\s.()'"`\[\]]+)""" +) _VIRTUAL_TABLE_MODULE_RE = re.compile( - r"\bCREATE\s+VIRTUAL\s+TABLE\b.*?\bUSING\s+([^\s(]+)", + r"^\s*CREATE\s+VIRTUAL\s+TABLE\b\s*(?:IF\s+NOT\s+EXISTS\s+)?" + + _SQLITE_IDENTIFIER_RE + + r"(?:\s*\.\s*" + + _SQLITE_IDENTIFIER_RE + + r")?\s*\bUSING\b\s*(" + + _SQLITE_IDENTIFIER_RE + + r")", re.IGNORECASE | re.DOTALL, ) _VIRTUAL_TABLE_SHADOW_SUFFIXES = { @@ -153,6 +162,11 @@ def sqlite_derived_table_dependencies( return {} table_names = {row[0] for row in rows} + # SQLite identifiers fold ASCII letters only. + identifier_case = str.maketrans( + "ABCDEFGHIJKLMNOPQRSTUVWXYZ", "abcdefghijklmnopqrstuvwxyz" + ) + canonical_names = {name.translate(identifier_case): name for name in table_names} dependencies = {} for virtual_table, sql in rows: module = _virtual_table_module(sql) @@ -173,6 +187,16 @@ def sqlite_derived_table_dependencies( if content_table: dependencies[virtual_table] = content_table + if module in {"fts5vocab", "fts4aux"}: + source = _fts_vocabulary_source(sql, module, schema or "main") + source = ( + canonical_names.get(source.translate(identifier_case)) + if source + else None + ) + # An unresolved source uses the existing cycle guard to deny access. + dependencies[virtual_table] = source or virtual_table + return dependencies @@ -242,10 +266,10 @@ def _quote_identifier(value: str) -> str: def _virtual_table_module(sql: str | None) -> str | None: if not sql: return None - match = _VIRTUAL_TABLE_MODULE_RE.search(sql) + match = _VIRTUAL_TABLE_MODULE_RE.search(_strip_sql_comments(sql)) if match is None: return None - return match.group(1).strip("\"'[]`").lower() + return _unquote_sql_value(match.group(1)).lower() def _fts_external_content_table(sql: str | None) -> str | None: @@ -271,6 +295,32 @@ def _fts_external_content_table(sql: str | None) -> str | None: return None +def _fts_vocabulary_source(sql: str, module: str, schema: str) -> str | None: + """Resolve a vocabulary source within the current SQLite schema. + + Cross-schema sources cannot be represented by the dependency map and + are conservatively left unresolved. + """ + sql = _strip_sql_comments(sql) + match = _VIRTUAL_TABLE_MODULE_RE.search(sql) + if match is None: + return None + start = sql.find("(", match.end()) + end = sql.rfind(")") + if start < 0 or end <= start: + return None + arguments = [ + _unquote_sql_value(arg.strip()) + for arg in _split_sql_arguments(sql[start + 1 : end]) + ] + expected = 2 if module == "fts5vocab" else 1 + if len(arguments) == expected: + return arguments[0] + if len(arguments) == expected + 1 and arguments[0].lower() == schema.lower(): + return arguments[1] + return None + + def _split_sql_arguments(arguments: str) -> list[str]: """Split comma-separated SQLite arguments without splitting quoted text.""" parts = [] diff --git a/docs/authentication.rst b/docs/authentication.rst index b559d7ac..aea055e3 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -1382,6 +1382,11 @@ view-table Actor is allowed to view a table (or view) page, e.g. https://latest.datasette.io/fixtures/complex_foreign_keys +FTS vocabulary tables (``fts5vocab`` and ``fts4aux``) also require access to +their source FTS table and, for an external-content index, its content table. +The vocabulary table's own permission rules still apply. Vocabulary tables +whose source cannot be resolved within the same SQLite schema are denied. + ``resource`` - ``datasette.resources.TableResource(database, table)`` ``database`` is the name of the database (string) diff --git a/tests/test_pr76_fts_policy.py b/tests/test_pr76_fts_policy.py new file mode 100644 index 00000000..848a4a8c --- /dev/null +++ b/tests/test_pr76_fts_policy.py @@ -0,0 +1,138 @@ +"""Policy and compatibility coverage for PR #76, run against the fixed checkout.""" + +import uuid + +import pytest + +from datasette.app import Datasette +from datasette.resources import TableResource +from datasette.utils.sqlite import sqlite3, sqlite_derived_table_dependencies + + +@pytest.mark.parametrize("vocab_name", ["words", "name USING fts4aux", 'quoted"name']) +@pytest.mark.parametrize( + "module,arguments", + [ + ("fts5vocab", "'Search,Index', 'row'"), + ("fts5vocab", "'SEARCH,INDEX', 'col'"), + ("fts5vocab", "'Search,Index', 'instance'"), + ("fts4aux", "'Search,Index'"), + ], +) +def test_vocabulary_dependency_identity(module, arguments, vocab_name): + conn = sqlite3.connect(":memory:") + try: + fts = "fts5" if module == "fts5vocab" else "fts4" + conn.execute(f'create virtual table "Search,Index" using {fts}(body)') + quoted_name = '"' + vocab_name.replace('"', '""') + '"' + conn.execute( + f"create virtual table {quoted_name} USING /* module */ {module}({arguments})" + ) + assert sqlite_derived_table_dependencies(conn)[vocab_name] == "Search,Index" + finally: + conn.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("module", ["fts5", "fts4"]) +@pytest.mark.parametrize( + "source_allowed,vocab_allowed", [(False, True), (True, False), (True, True)] +) +async def test_vocabulary_transitive_permissions(module, source_allowed, vocab_allowed): + ds = Datasette( + memory=True, + config={ + "databases": { + "data": { + "tables": { + "documents": { + "permissions": { + "view-table": ( + {"id": "reader"} if source_allowed else False + ) + } + }, + "words": {"permissions": {"view-table": vocab_allowed}}, + } + } + } + }, + ) + db = ds.add_memory_database(uuid.uuid4().hex, name="data") + await db.execute_write("create table documents(body text)") + await db.execute_write( + f"create virtual table search using {module}(body, content='documents')" + ) + definition = ( + "fts5vocab('SEARCH', 'row')" if module == "fts5" else "fts4aux('SEARCH')" + ) + await db.execute_write(f"create virtual table words using {definition}") + await ds.invoke_startup() + try: + actor = {"id": "reader"} + expected = source_allowed and vocab_allowed + for name in ("words", "WORDS"): + assert ( + await ds.allowed( + action="view-table", + resource=TableResource("data", name), + actor=actor, + ) + is expected + ) + resources = await ds.allowed_resources( + "view-table", parent="data", actor=actor, include_is_private=True + ) + words = [r for r in resources.resources if r.child == "words"] + assert bool(words) is expected + if expected: + assert words[0].private + assert not await ds.allowed( + action="view-table", resource=TableResource("data", "words") + ) + # Dropping the source invalidates dependency metadata and remains denied. + await db.execute_write("drop table search") + assert not await ds.allowed( + action="view-table", resource=TableResource("data", "words"), actor=actor + ) + finally: + ds.close() + + +@pytest.mark.parametrize( + "module,definition", + [ + ("fts5", "fts5vocab('main', 'search', 'row')"), + ("fts4", "fts4aux('main', 'search')"), + ], +) +def test_cross_schema_vocabulary_is_unresolved(module, definition): + conn = sqlite3.connect(":memory:") + try: + conn.execute(f"create virtual table search using {module}(body)") + conn.execute(f"create virtual table temp.words using {definition}") + # Cross-schema ownership is not representable by the current map. + # The self-dependency invokes the permission layer's cycle denial. + assert ( + sqlite_derived_table_dependencies(conn, schema="temp")["words"] == "words" + ) + finally: + conn.close() + + +@pytest.mark.parametrize( + "definition", + [ + """CREATE VIRTUAL TABLE"words"USING"fts5vocab"('search', 'row')""", + """CREATE VIRTUAL TABLE[words]USING[fts5vocab]('search', 'row')""", + """CREATE VIRTUAL TABLE`words`USING`fts5vocab`('search', 'row')""", + ], +) +def test_vocabulary_quoted_token_boundaries(definition): + conn = sqlite3.connect(":memory:") + try: + conn.execute("create virtual table search using fts5(body)") + conn.execute(definition) + assert sqlite_derived_table_dependencies(conn)["words"] == "search" + finally: + conn.close()