diff --git a/datasette/app.py b/datasette/app.py index 6683d4dc..e6d070ea 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -308,6 +308,12 @@ DEFAULT_NOT_SET = object() ResourcesSQL = collections.namedtuple("ResourcesSQL", ("sql", "params")) +# Tracks recursive view-table inheritance checks and fails closed if a +# malicious or malformed schema declares a cycle of FTS content tables. +_derived_permission_stack = contextvars.ContextVar( + "derived_permission_stack", default=() +) + def _permission_cache_key(actor, action, parent, child): # Key on the full serialized actor so actors differing in any field @@ -1746,8 +1752,118 @@ class Datasette: sql, params = await build_allowed_resources_sql( self, actor, action, parent=parent, include_is_private=include_is_private ) + if action == "view-table": + sql, params = await self._apply_derived_table_permissions_to_sql( + sql, + params, + actor=actor, + parent=parent, + include_is_private=include_is_private, + ) return ResourcesSQL(sql, params) + async def _apply_derived_table_permissions_to_sql( + self, + sql, + params, + *, + actor, + parent, + include_is_private, + ): + databases = ( + [(parent, self.databases[parent])] + if parent in self.databases + else ([] if parent is not None else list(self.databases.items())) + ) + dependency_maps = await asyncio.gather( + *(db.derived_table_dependencies() for _, db in databases) + ) + dependencies = [ + (database_name, child, source) + for (database_name, _), dependency_map in zip(databases, dependency_maps) + for child, source in dependency_map.items() + ] + if not dependencies: + return sql, params + + sources = sorted( + {(database_name, source) for database_name, _, source in dependencies} + ) + actor_verdicts = await asyncio.gather( + *( + self.allowed( + action="view-table", + resource=TableResource(database_name, source), + actor=actor, + ) + for database_name, source in sources + ) + ) + actor_allowed = dict(zip(sources, actor_verdicts)) + + anonymous_allowed = {} + if include_is_private: + anonymous_verdicts = await asyncio.gather( + *( + self.allowed( + action="view-table", + resource=TableResource(database_name, source), + actor=None, + ) + for database_name, source in sources + ) + ) + anonymous_allowed = dict(zip(sources, anonymous_verdicts)) + + wrapped_params = dict(params) + derived_rows = [ + [ + database_name, + child, + int(actor_allowed[(database_name, source)]), + *( + [int(anonymous_allowed[(database_name, source)])] + if include_is_private + else [] + ), + ] + for database_name, child, source in dependencies + ] + derived_param = "_datasette_derived_permissions" + while derived_param in wrapped_params: + derived_param += "_" + wrapped_params[derived_param] = json.dumps(derived_rows) + + derived_columns = "parent, child, source_allowed" + select_columns = "allowed.parent, allowed.child, allowed.reason" + if include_is_private: + derived_columns += ", source_anonymous_allowed" + select_columns += ( + ", CASE WHEN derived.source_anonymous_allowed = 0 " + "THEN 1 ELSE allowed.is_private END AS is_private" + ) + wrapped_sql = f""" +WITH derived_permissions({derived_columns}) AS ( + SELECT + json_extract(value, '$[0]'), + json_extract(value, '$[1]'), + json_extract(value, '$[2]') + {", json_extract(value, '$[3]')" if include_is_private else ""} + FROM json_each(:{derived_param}) +), +allowed AS ( +{sql} +) +SELECT {select_columns} +FROM allowed +LEFT JOIN derived_permissions AS derived + ON allowed.parent = derived.parent AND allowed.child = derived.child +WHERE COALESCE(derived.source_allowed, 1) = 1 +ORDER BY allowed.parent, allowed.child +""".strip() + return wrapped_sql, wrapped_params + async def allowed_resources( self, action: str, @@ -2003,6 +2119,34 @@ class Datasette: child=child, ) + # Automatically derived implementation tables cannot be more visible + # than the logical/content table they expose. Keep the requested + # table's own permission too: either side can make access private. + if ( + "view-table" in to_check + and raw.get("view-table") + and isinstance(resource, TableResource) + and parent in self.databases + ): + dependency = ( + await self.databases[parent].derived_table_dependencies() + ).get(child) + if dependency is not None: + stack = _derived_permission_stack.get() + dependency_key = (parent, dependency) + if dependency_key in stack or dependency == child: + raw["view-table"] = False + else: + token = _derived_permission_stack.set(stack + ((parent, child),)) + try: + raw["view-table"] = await self.allowed( + action="view-table", + resource=TableResource(parent, dependency), + actor=actor, + ) + finally: + _derived_permission_stack.reset(token) + def resolve(name): # final verdict = own rules AND verdict of also_requires chain if name in final: diff --git a/datasette/database.py b/datasette/database.py index 90c4e429..25523f3e 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -29,7 +29,7 @@ from .utils import ( table_columns, ) from .utils.sql_analysis import SQLAnalysis, analyze_sql_tables -from .utils.sqlite import sqlite_hidden_table_names +from .utils.sqlite import sqlite_derived_table_dependencies, sqlite_hidden_table_names connections = threading.local() @@ -85,6 +85,7 @@ class Database: self.cached_hash = None self.cached_size = None self._cached_table_counts = None + self._cached_derived_table_dependencies = None self._write_thread = None self._write_queue = None self._closed = False @@ -768,6 +769,17 @@ class Database: return hidden_tables + async def derived_table_dependencies(self): + """Return implementation tables and the tables they derive from.""" + schema_version = (await self.execute("PRAGMA schema_version")).first()[0] + if ( + self._cached_derived_table_dependencies is None + or self._cached_derived_table_dependencies[0] != schema_version + ): + dependencies = await self.execute_fn(sqlite_derived_table_dependencies) + self._cached_derived_table_dependencies = (schema_version, dependencies) + return self._cached_derived_table_dependencies[1] + async def view_names(self): results = await self.execute("select name from sqlite_master where type='view'") return [r[0] for r in results.rows] diff --git a/datasette/utils/sqlite.py b/datasette/utils/sqlite.py index d3926f6f..2082fa34 100644 --- a/datasette/utils/sqlite.py +++ b/datasette/utils/sqlite.py @@ -118,6 +118,48 @@ def sqlite_hidden_table_names(conn, *, schema: str | None = "main") -> list[str] return sorted(hidden_tables) + content_fts_tables +def sqlite_derived_table_dependencies( + conn, *, schema: str | None = "main" +) -> dict[str, str]: + """Return implementation table -> logical/content table dependencies. + + ``PRAGMA table_list`` safely identifies virtual and shadow tables, but + does not report which virtual table owns a shadow table or which table is + named by an FTS ``content=`` option. Derive those relationships from + ``sqlite_master`` DDL and the documented shadow-table suffixes. + """ + schema_table = _sqlite_schema_table(schema) + try: + rows = conn.execute( + f"select name, sql from {schema_table} where type = 'table'" + ).fetchall() + except sqlite3.DatabaseError: + return {} + + table_names = {row[0] for row in rows} + dependencies = {} + for virtual_table, sql in rows: + module = _virtual_table_module(sql) + if module is None: + continue + + # SQLite's documented shadow tables are implementation details of + # their logical virtual table. + for suffix in _VIRTUAL_TABLE_SHADOW_SUFFIXES.get(module, ()): + shadow_table = virtual_table + suffix + if shadow_table in table_names: + dependencies[shadow_table] = virtual_table + + # An external-content FTS table can expose values fetched from its + # content table, so it must also depend on that table's permission. + if module in {"fts3", "fts4", "fts5"}: + content_table = _fts_external_content_table(sql) + if content_table: + dependencies[virtual_table] = content_table + + return dependencies + + def _sqlite_table_type_from_schema( conn, table: str, @@ -190,6 +232,121 @@ def _virtual_table_module(sql: str | None) -> str | None: return match.group(1).strip("\"'[]`").lower() +def _fts_external_content_table(sql: str | None) -> str | None: + """Extract the external ``content=`` table from an FTS declaration.""" + if not sql: + return None + sql = _strip_sql_comments(sql) + match = _VIRTUAL_TABLE_MODULE_RE.search(sql) + if match is None: + return None + open_paren = sql.find("(", match.end()) + if open_paren == -1: + return None + close_paren = sql.rfind(")") + if close_paren <= open_paren: + return None + + for argument in _split_sql_arguments(sql[open_paren + 1 : close_paren]): + key, separator, value = argument.partition("=") + if not separator or key.strip().lower() != "content": + continue + return _unquote_sql_value(value.strip()) + return None + + +def _split_sql_arguments(arguments: str) -> list[str]: + """Split comma-separated SQLite arguments without splitting quoted text.""" + parts = [] + start = 0 + quote = None + closing_quote = None + index = 0 + while index < len(arguments): + char = arguments[index] + if quote is None: + if char in {"'", '"', "`", "["}: + quote = char + closing_quote = "]" if char == "[" else char + elif char == ",": + parts.append(arguments[start:index]) + start = index + 1 + elif char == closing_quote: + # Single/double/backtick quoting escapes the delimiter by + # doubling it. Square-bracket identifiers do not. + if ( + quote != "[" + and index + 1 < len(arguments) + and arguments[index + 1] == closing_quote + ): + index += 1 + else: + quote = None + closing_quote = None + index += 1 + parts.append(arguments[start:]) + return parts + + +def _strip_sql_comments(sql: str) -> str: + """Remove SQLite comments while preserving quoted strings/identifiers.""" + output = [] + quote = None + closing_quote = None + index = 0 + while index < len(sql): + char = sql[index] + next_char = sql[index + 1] if index + 1 < len(sql) else "" + if quote is None: + if char in {"'", '"', "`", "["}: + quote = char + closing_quote = "]" if char == "[" else char + output.append(char) + elif char == "-" and next_char == "-": + index += 2 + while index < len(sql) and sql[index] not in "\r\n": + index += 1 + output.append(" ") + continue + elif char == "/" and next_char == "*": + index += 2 + while index + 1 < len(sql) and sql[index : index + 2] != "*/": + index += 1 + index = min(index + 2, len(sql)) + output.append(" ") + continue + else: + output.append(char) + else: + output.append(char) + if char == closing_quote: + if ( + quote != "[" + and index + 1 < len(sql) + and sql[index + 1] == closing_quote + ): + output.append(sql[index + 1]) + index += 1 + else: + quote = None + closing_quote = None + index += 1 + return "".join(output) + + +def _unquote_sql_value(value: str) -> str: + if len(value) < 2: + return value + pairs = {"'": "'", '"': '"', "`": "`", "[": "]"} + closing = pairs.get(value[0]) + if closing is None or value[-1] != closing: + return value + unquoted = value[1:-1] + if value[0] != "[": + unquoted = unquoted.replace(closing * 2, closing) + return unquoted + + def _is_fts_content_virtual_table(sql: str | None) -> bool: return ( _virtual_table_module(sql) in {"fts3", "fts4", "fts5"} diff --git a/tests/test_fts_permissions.py b/tests/test_fts_permissions.py new file mode 100644 index 00000000..c3e4292a --- /dev/null +++ b/tests/test_fts_permissions.py @@ -0,0 +1,295 @@ +import pytest + +from datasette.app import Datasette +from datasette.resources import DatabaseResource, TableResource + + +@pytest.mark.asyncio +@pytest.mark.parametrize("fts_module", ("fts4", "fts5")) +async def test_external_content_fts_inherits_content_table_view_permission(fts_module): + actor = {"id": "reader"} + secret_marker = "ISSUE_17_EXTERNAL_CONTENT_FTS_SECRET" + ds = Datasette( + memory=True, + config={ + "permissions": { + "view-instance": {"id": "reader"}, + "view-database": {"id": "reader"}, + "view-table": {"id": "reader"}, + "execute-sql": {"id": "nobody"}, + }, + "databases": { + "data": { + "tables": { + "secret": {"permissions": {"view-table": False}}, + } + } + }, + }, + ) + db = ds.add_memory_database(f"issue_17_{fts_module}_permissions", name="data") + await db.execute_write("create table secret (id integer primary key, body text)") + await db.execute_write( + "insert into secret (body) values (?)", + [secret_marker], + ) + fts_options = "body, content='secret'" + if fts_module == "fts5": + fts_options += ", content_rowid='id'" + await db.execute_write( + f"create virtual table secret_fts using {fts_module}({fts_options})" + ) + await db.execute_write("insert into secret_fts(secret_fts) values ('rebuild')") + await ds.invoke_startup() + + try: + assert "secret_fts" in await db.hidden_table_names() + assert ( + await ds.allowed( + action="execute-sql", + resource=DatabaseResource("data"), + actor=actor, + ) + is False + ) + + direct = await ds.client.get("/data/secret.json", actor=actor) + assert direct.status_code == 403 + + companion = await ds.client.get( + "/data/secret_fts.json?_shape=array", + actor=actor, + ) + assert companion.status_code in (403, 404), ( + "An automatically hidden external-content FTS table must inherit " + "the content table's view denial or be unavailable: " + f"{companion.text}" + ) + assert secret_marker not in companion.text + finally: + ds.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("fts_module", ("fts4", "fts5")) +@pytest.mark.parametrize("contentless", (False, True), ids=("internal", "contentless")) +async def test_fts_shadow_tables_inherit_logical_table_view_permission( + fts_module, contentless +): + table_config = { + "secret_fts": {"permissions": {"view-table": False}}, + # An explicit allow on one implementation table must not override + # the logical FTS table's denial. + "secret_fts_docsize": {"permissions": {"view-table": True}}, + } + ds = Datasette( + memory=True, + config={ + "permissions": { + "view-instance": True, + "view-database": True, + "view-table": True, + "execute-sql": False, + }, + "databases": {"data": {"tables": table_config}}, + }, + ) + db = ds.add_memory_database( + f"issue_17_{fts_module}_{'contentless' if contentless else 'internal'}", + name="data", + ) + options = "body, content=''" if contentless else "body" + await db.execute_write( + f"create virtual table secret_fts using {fts_module}({options})" + ) + await db.execute_write( + "insert into secret_fts(rowid, body) values (1, 'ISSUE_17_SHADOW_SECRET')" + ) + await ds.invoke_startup() + + try: + dependencies = await db.derived_table_dependencies() + shadow_tables = sorted( + table for table, source in dependencies.items() if source == "secret_fts" + ) + assert shadow_tables + assert "secret_fts_docsize" in shadow_tables + + for shadow_table in shadow_tables: + assert ( + await ds.allowed( + action="view-table", + resource=TableResource("data", shadow_table), + ) + is False + ) + response = await ds.client.get(f"/data/{shadow_table}.json?_shape=array") + assert response.status_code == 403 + assert "ISSUE_17_SHADOW_SECRET" not in response.text + + allowed = await ds.allowed_resources("view-table", parent="data", limit=1000) + allowed_names = {resource.child for resource in allowed.resources} + assert not set(shadow_tables).intersection(allowed_names) + + database_json = await ds.client.get("/data.json") + assert database_json.status_code == 200 + for shadow_table in shadow_tables: + assert shadow_table not in database_json.text + + schema_json = await ds.client.get("/data/-/schema.json") + assert schema_json.status_code == 200 + for shadow_table in shadow_tables: + assert shadow_table not in schema_json.text + finally: + ds.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "content_allowed,companion_allowed,expected", + ( + (False, True, False), + (True, False, False), + (True, True, True), + ), +) +async def test_external_content_and_companion_permissions_are_both_required( + content_allowed, companion_allowed, expected +): + ds = Datasette( + memory=True, + default_deny=True, + config={ + "permissions": { + "view-instance": True, + "view-database": True, + }, + "databases": { + "data": { + "tables": { + "secret": {"permissions": {"view-table": content_allowed}}, + "secret_fts": { + "permissions": {"view-table": companion_allowed} + }, + } + } + }, + }, + ) + db = ds.add_memory_database( + f"issue_17_explicit_{int(content_allowed)}_{int(companion_allowed)}", + name="data", + ) + await db.execute_write("create table secret(id integer primary key, body text)") + await db.execute_write("insert into secret(body) values ('ISSUE_17_MATRIX_SECRET')") + await db.execute_write( + "create virtual table secret_fts using fts5(" + "body, content='secret', content_rowid='id')" + ) + await db.execute_write("insert into secret_fts(secret_fts) values ('rebuild')") + await ds.invoke_startup() + + try: + assert ( + await ds.allowed( + action="view-table", + resource=TableResource("data", "secret_fts"), + ) + is expected + ) + response = await ds.client.get("/data/secret_fts.json?_shape=array") + assert response.status_code == (200 if expected else 403) + if not expected: + assert "ISSUE_17_MATRIX_SECRET" not in response.text + finally: + ds.close() + + +@pytest.mark.asyncio +async def test_derived_tables_propagate_private_flag_and_route_permissions(): + actor = {"id": "reader"} + ds = Datasette( + memory=True, + config={ + "permissions": { + "view-instance": True, + "view-database": True, + "view-table": True, + }, + "databases": { + "data": { + "tables": { + "secret": {"permissions": {"view-table": {"id": "reader"}}} + } + } + }, + }, + ) + db = ds.add_memory_database("issue_17_private_flag", name="data") + await db.execute_write("create table secret(id integer primary key, body text)") + await db.execute_write("insert into secret(body) values ('PRIVATE')") + await db.execute_write( + "create virtual table secret_fts using fts5(" + "body, content='secret', content_rowid='id')" + ) + await db.execute_write("insert into secret_fts(secret_fts) values ('rebuild')") + await ds.invoke_startup() + + try: + actor_page = await ds.allowed_resources( + "view-table", actor, parent="data", include_is_private=True, limit=1000 + ) + actor_resources = { + resource.child: resource for resource in actor_page.resources + } + derived_names = set(await db.derived_table_dependencies()) + assert derived_names.issubset(actor_resources) + assert all(actor_resources[name].private for name in derived_names) + + anonymous_page = await ds.allowed_resources( + "view-table", parent="data", limit=1000 + ) + anonymous_names = {resource.child for resource in anonymous_page.resources} + assert not derived_names.intersection(anonymous_names) + + for path in ( + "/data/secret_fts.json?_facet=body", + "/data/secret_fts.csv", + "/data/secret_fts/-/autocomplete?q=PRIVATE", + "/data/secret_fts/-/schema.json", + ): + denied = await ds.client.get(path) + assert denied.status_code == 403 + allowed = await ds.client.get(path, actor=actor) + assert allowed.status_code == 200 + finally: + ds.close() + + +@pytest.mark.asyncio +async def test_cyclic_derived_table_dependencies_fail_closed(): + ds = Datasette(memory=True) + db = ds.add_memory_database("issue_17_cycle", name="data") + await db.execute_write( + "create virtual table first_fts using fts5(body, content='second_fts')" + ) + await db.execute_write( + "create virtual table second_fts using fts5(body, content='first_fts')" + ) + await ds.invoke_startup() + + try: + for table in ("first_fts", "second_fts"): + assert ( + await ds.allowed( + action="view-table", resource=TableResource("data", table) + ) + is False + ) + + page = await ds.allowed_resources("view-table", parent="data", limit=1000) + allowed_names = {resource.child for resource in page.resources} + assert "first_fts" not in allowed_names + assert "second_fts" not in allowed_names + finally: + ds.close() diff --git a/tests/test_utils.py b/tests/test_utils.py index 1808b3cf..a491b5a5 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -16,6 +16,7 @@ from datasette.app import Datasette from datasette.utils.asgi import Request from datasette.utils.sqlite import ( sqlite3, + sqlite_derived_table_dependencies, sqlite_hidden_table_names, sqlite_table_type, supports_returning, @@ -369,6 +370,46 @@ def test_sqlite_hidden_table_names_hides_multiline_content_fts_table(): conn.close() +def test_sqlite_derived_table_dependencies(): + conn = utils.sqlite3.connect(":memory:") + try: + conn.executescript(""" + create table docs(id integer primary key, body text); + create virtual table external_fts5 using fts5( + body, content='docs', content_rowid='id' + ); + create virtual table internal_fts5 using fts5(body); + create virtual table contentless_fts5 using fts5(body, content=''); + create virtual table external_fts4 using fts4(body, content="docs"); + create virtual table internal_fts4 using fts4(body); + create virtual table contentless_fts4 using fts4(body, content=""); + create table [docs, archive](body text); + create virtual table commented_fts5 using fts5( + body, tokenize='porter unicode61', + /* Comments and commas in quoted values must not confuse parsing. */ + content='docs, archive' + ); + create virtual table boxes using rtree(id, minx, maxx, miny, maxy); + """) + + dependencies = sqlite_derived_table_dependencies(conn) + + assert dependencies["external_fts5"] == "docs" + assert dependencies["external_fts4"] == "docs" + assert dependencies["commented_fts5"] == "docs, archive" + assert "contentless_fts5" not in dependencies + assert "contentless_fts4" not in dependencies + assert dependencies["internal_fts5_content"] == "internal_fts5" + assert dependencies["internal_fts4_content"] == "internal_fts4" + assert dependencies["external_fts5_data"] == "external_fts5" + assert dependencies["external_fts4_segments"] == "external_fts4" + assert dependencies["boxes_node"] == "boxes" + assert dependencies["boxes_parent"] == "boxes" + assert dependencies["boxes_rowid"] == "boxes" + finally: + conn.close() + + @pytest.mark.parametrize( "url,expected", [