From 01e0558825b8f7ec17d3b691aa072daf122fcc74 Mon Sep 17 00:00:00 2001
From: Simon Willison <swillison@gmail.com>
Date: Tue, 22 Aug 2023 10:10:01 -0700
Subject: [PATCH] Merge pull request from GHSA-7ch3-7pp7-7cpq

* API explorer requires view-instance permission

* Check database/table permissions on /-/api page

* Release notes for 1.0a4

Refs #2119, #2133, #2138, #2140

Refs https://github.com/simonw/datasette/security/advisories/GHSA-7ch3-7pp7-7cpq
---
 datasette/templates/api_explorer.html |  2 +-
 datasette/version.py                  |  2 +-
 datasette/views/special.py            | 19 ++++++--
 docs/changelog.rst                    | 16 +++++++
 tests/test_permissions.py             | 67 +++++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html
index ea95c023..109fb1e9 100644
--- a/datasette/templates/api_explorer.html
+++ b/datasette/templates/api_explorer.html
@@ -8,7 +8,7 @@
 
 {% block content %}
 
-<h1>API Explorer</h1>
+<h1>API Explorer{% if private %} 🔒{% endif %}</h1>
 
 <p>Use this tool to try out the
   {% if datasette_version %}
diff --git a/datasette/version.py b/datasette/version.py
index 61dee464..1d003352 100644
--- a/datasette/version.py
+++ b/datasette/version.py
@@ -1,2 +1,2 @@
-__version__ = "1.0a3"
+__version__ = "1.0a4"
 __version_info__ = tuple(__version__.split("."))
diff --git a/datasette/views/special.py b/datasette/views/special.py
index 03e085d6..c45a3eca 100644
--- a/datasette/views/special.py
+++ b/datasette/views/special.py
@@ -354,9 +354,7 @@ class ApiExplorerView(BaseView):
             if name == "_internal":
                 continue
             database_visible, _ = await self.ds.check_visibility(
-                request.actor,
-                "view-database",
-                name,
+                request.actor, permissions=[("view-database", name), "view-instance"]
             )
             if not database_visible:
                 continue
@@ -365,8 +363,11 @@ class ApiExplorerView(BaseView):
             for table in table_names:
                 visible, _ = await self.ds.check_visibility(
                     request.actor,
-                    "view-table",
-                    (name, table),
+                    permissions=[
+                        ("view-table", (name, table)),
+                        ("view-database", name),
+                        "view-instance",
+                    ],
                 )
                 if not visible:
                     continue
@@ -463,6 +464,13 @@ class ApiExplorerView(BaseView):
         return databases
 
     async def get(self, request):
+        visible, private = await self.ds.check_visibility(
+            request.actor,
+            permissions=["view-instance"],
+        )
+        if not visible:
+            raise Forbidden("You do not have permission to view this instance")
+
         def api_path(link):
             return "/-/api#{}".format(
                 urllib.parse.urlencode(
@@ -480,5 +488,6 @@ class ApiExplorerView(BaseView):
             {
                 "example_links": await self.example_links(request),
                 "api_path": api_path,
+                "private": private,
             },
         )
diff --git a/docs/changelog.rst b/docs/changelog.rst
index c497ea9f..937610bd 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -4,6 +4,22 @@
 Changelog
 =========
 
+.. _v1_0_a4:
+
+1.0a4 (2023-08-21)
+------------------
+
+This alpha fixes a security issue with the ``/-/api`` API explorer. On authenticated Datasette instances (instances protected using plugins such as `datasette-auth-passwords <https://datasette.io/plugins/datasette-auth-passwords>`__) the API explorer interface could reveal the names of databases and tables within the protected instance. The data stored in those tables was not revealed.
+
+For more information and workarounds, read `the security advisory <https://github.com/simonw/datasette/security/advisories/GHSA-7ch3-7pp7-7cpq>`__. The issue has been present in every previous alpha version of Datasette 1.0: versions 1.0a0, 1.0a1, 1.0a2 and 1.0a3.
+
+Also in this alpha:
+
+- The new ``datasette plugins --requirements`` option outputs a list of currently installed plugins in Python ``requirements.txt`` format, useful for duplicating that installation elsewhere. (:issue:`2133`)
+- :ref:`canned_queries_writable` can now define a ``on_success_message_sql`` field in their configuration, containing a SQL query that should be executed upon successful completion of the write operation in order to generate a message to be shown to the user. (:issue:`2138`)
+- The automatically generated border color for a database is now shown in more places around the application. (:issue:`2119`)
+- Every instance of example shell script code in the documentation should now include a working copy button, free from additional syntax. (:issue:`2140`)
+
 .. _v1_0_a3:
 
 1.0a3 (2023-08-09)
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index caad588c..f940d486 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -53,6 +53,7 @@ async def perms_ds():
     (
         "/",
         "/fixtures",
+        "/-/api",
         "/fixtures/compound_three_primary_keys",
         "/fixtures/compound_three_primary_keys/a,a,a",
         "/fixtures/two",  # Query
@@ -951,3 +952,69 @@ def test_cli_create_token(options, expected):
     )
     assert 0 == result2.exit_code, result2.output
     assert json.loads(result2.output) == {"actor": expected}
+
+
+_visible_tables_re = re.compile(r">\/((\w+)\/(\w+))\.json<\/a> - Get rows for")
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "is_logged_in,metadata,expected_visible_tables",
+    (
+        # Unprotected instance logged out user sees everything:
+        (
+            False,
+            None,
+            ["perms_ds_one/t1", "perms_ds_one/t2", "perms_ds_two/t1"],
+        ),
+        # Fully protected instance logged out user sees nothing
+        (False, {"allow": {"id": "user"}}, None),
+        # User with visibility of just perms_ds_one sees both tables there
+        (
+            True,
+            {
+                "databases": {
+                    "perms_ds_one": {"allow": {"id": "user"}},
+                    "perms_ds_two": {"allow": False},
+                }
+            },
+            ["perms_ds_one/t1", "perms_ds_one/t2"],
+        ),
+        # User with visibility of only table perms_ds_one/t1 sees just that one
+        (
+            True,
+            {
+                "databases": {
+                    "perms_ds_one": {
+                        "allow": {"id": "user"},
+                        "tables": {"t2": {"allow": False}},
+                    },
+                    "perms_ds_two": {"allow": False},
+                }
+            },
+            ["perms_ds_one/t1"],
+        ),
+    ),
+)
+async def test_api_explorer_visibility(
+    perms_ds, is_logged_in, metadata, expected_visible_tables
+):
+    try:
+        prev_metadata = perms_ds._metadata_local
+        perms_ds._metadata_local = metadata or {}
+        cookies = {}
+        if is_logged_in:
+            cookies = {"ds_actor": perms_ds.client.actor_cookie({"id": "user"})}
+        response = await perms_ds.client.get("/-/api", cookies=cookies)
+        if expected_visible_tables:
+            assert response.status_code == 200
+            # Search HTML for stuff matching:
+            # '>/perms_ds_one/t2.json</a> - Get rows for'
+            visible_tables = [
+                match[0] for match in _visible_tables_re.findall(response.text)
+            ]
+            assert visible_tables == expected_visible_tables
+        else:
+            assert response.status_code == 403
+    finally:
+        perms_ds._metadata_local = prev_metadata