From f4eefdf19330bcd2e6867875a93a948c96912213 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 19 May 2019 13:41:09 -0700 Subject: [PATCH] Do not allow downloads of mutable databases - closes #474 --- datasette/templates/database.html | 2 +- datasette/views/database.py | 7 +++-- tests/fixtures.py | 5 ---- tests/test_html.py | 46 +++++++++++++++++++------------ 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/datasette/templates/database.html b/datasette/templates/database.html index 81c4ffd3..9fb4d6eb 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -56,7 +56,7 @@ {% endif %} -{% if config.allow_download and database != ":memory:" %} +{% if allow_download %}

Download SQLite DB: {{ database }}.db {{ format_bytes(size) }}

{% endif %} diff --git a/datasette/views/database.py b/datasette/views/database.py index 5065b90f..c5c00bf4 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -67,6 +67,9 @@ class DatabaseView(BaseView): "show_hidden": request.args.get("_show_hidden"), "editable": True, "metadata": metadata, + "allow_download": self.ds.config("allow_download") + and not db.is_mutable + and database != ":memory:", }, ("database-{}.html".format(to_css_class(database)), "database.html"), ) @@ -76,13 +79,13 @@ class DatabaseDownload(BaseView): name = "database_download" async def view_get(self, request, database, hash, correct_hash_present, **kwargs): - if not self.ds.config("allow_download"): - raise DatasetteError("Database download is forbidden", status=403) if database not in self.ds.databases: raise DatasetteError("Invalid database", status=404) db = self.ds.databases[database] if db.is_memory: raise DatasetteError("Cannot download :memory: database", status=404) + if not self.ds.config("allow_download") or db.is_mutable: + raise DatasetteError("Database download is forbidden", status=403) if not db.path: raise DatasetteError("Cannot download database", status=404) filepath = db.path diff --git a/tests/fixtures.py b/tests/fixtures.py index 4667ba89..7b9c09f1 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -107,11 +107,6 @@ def app_client_two_attached_databases_one_immutable(): ) -@pytest.fixture(scope="session") -def app_client_with_memory(): - yield from make_app_client(memory=True) - - @pytest.fixture(scope="session") def app_client_with_hash(): yield from make_app_client(config={"hash_urls": True}, is_immutable=True) diff --git a/tests/test_html.py b/tests/test_html.py index 0b9b9178..6b673c13 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -4,7 +4,6 @@ from .fixtures import ( # noqa app_client_shorter_time_limit, app_client_two_attached_databases, app_client_with_hash, - app_client_with_memory, make_app_client, METADATA, ) @@ -45,9 +44,10 @@ def test_homepage(app_client_two_attached_databases): ] == table_links -def test_memory_database_page(app_client_with_memory): - response = app_client_with_memory.get("/:memory:") - assert response.status == 200 +def test_memory_database_page(): + for client in make_app_client(memory=True): + response = client.get("/:memory:") + assert response.status == 200 def test_database_page_redirects_with_url_hash(app_client_with_hash): @@ -724,23 +724,35 @@ def test_table_metadata(app_client): assert_footer_links(soup) -def test_database_download(app_client_with_memory): - # Regular page should have a download link - response = app_client_with_memory.get("/fixtures") +def test_database_download_allowed_for_immutable(): + for client in make_app_client(is_immutable=True): + assert not client.ds.databases["fixtures"].is_mutable + # Regular page should have a download link + response = client.get("/fixtures") + soup = Soup(response.body, "html.parser") + assert len(soup.findAll("a", {"href": re.compile(r"\.db$")})) + # Check we can actually download it + assert 200 == client.get("/fixtures.db").status + + +def test_database_download_disallowed_for_mutable(app_client): + response = app_client.get("/fixtures") soup = Soup(response.body, "html.parser") - assert len(soup.findAll("a", {"href": re.compile(r"\.db$")})) - # Check we can actually download it - assert 200 == app_client_with_memory.get("/fixtures.db").status - # Memory page should NOT have a download link - response2 = app_client_with_memory.get("/:memory:") - soup2 = Soup(response2.body, "html.parser") - assert 0 == len(soup2.findAll("a", {"href": re.compile(r"\.db$")})) - # The URL itself should 404 - assert 404 == app_client_with_memory.get("/:memory:.db").status + assert 0 == len(soup.findAll("a", {"href": re.compile(r"\.db$")})) + assert 403 == app_client.get("/fixtures.db").status + + +def test_database_download_disallowed_for_memory(): + for client in make_app_client(memory=True): + # Memory page should NOT have a download link + response = client.get("/:memory:") + soup = Soup(response.body, "html.parser") + assert 0 == len(soup.findAll("a", {"href": re.compile(r"\.db$")})) + assert 404 == client.get("/:memory:.db").status def test_allow_download_off(): - for client in make_app_client(config={"allow_download": False}): + for client in make_app_client(is_immutable=True, config={"allow_download": False}): response = client.get("/fixtures") soup = Soup(response.body, "html.parser") assert not len(soup.findAll("a", {"href": re.compile(r"\.db$")}))