Viewing derived table requires permission for both table and its source

Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com>

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:

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]

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"}

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()

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",
     [