From 00c6731e841cb6cc234570580a3a070ece5d99ee Mon Sep 17 00:00:00 2001 From: Nick Sweeting Date: Fri, 29 May 2026 17:14:27 -0700 Subject: [PATCH] security: lock down /add/ config for anon/non-staff users Extends the GHSA-3h23-7824-pj8r / CVE-2026-42601 hardening so that non-staff users cannot set ANY crawl config on /add/, closing the alternate injection path via typed plugin_config____ fields (e.g. YTDLP_ARGS_EXTRA -> yt-dlp --exec RCE). Non-staff users (anonymous when PUBLIC_ADD_VIEW=True, or authenticated non-staff) may now only set: URLs, depth, tags, notes, URL allow/deny filters, and a public persona preset. - views: gate all crawl resource-limit overrides (CRAWL_MAX_URLS/SIZE/ TIMEOUT, TIMEOUT, SNAPSHOT_MAX_SIZE, DELETE_AFTER, CRAWL_MAX_CONCURRENT_SNAPSHOTS) behind staff; non-staff inherit them from persona/server defaults at hook runtime. - views/forms: cap permissions for non-staff (anonymous -> public; authenticated non-staff -> public/unlisted only, never private). - template: hide the crawl-limit grid (staff-only) and the permissions selector (authenticated-only); plugin grid + advanced section already staff-gated. - tests: update public-submission test to assert limits are rejected, and add coverage for the permission tiers and limit-override rejection. Co-Authored-By: Claude Opus 4.8 (1M context) --- archivebox/core/forms.py | 26 +++- archivebox/core/views.py | 37 +++--- archivebox/templates/core/add.html | 6 +- archivebox/tests/test_ui_add_view.py | 179 +++++++++++++++++++++++++-- 4 files changed, 225 insertions(+), 23 deletions(-) diff --git a/archivebox/core/forms.py b/archivebox/core/forms.py index f698b74e..36975faa 100644 --- a/archivebox/core/forms.py +++ b/archivebox/core/forms.py @@ -15,7 +15,13 @@ from taggit.utils import edit_string_for_tags, parse_tags from archivebox.base_models.admin import KeyValueWidget from archivebox.crawls.schedule_utils import validate_schedule from archivebox.config.common import get_config, parse_delete_after -from archivebox.core.permissions import PERMISSIONS_CHOICES, PERMISSIONS_PUBLIC, filter_personas_by_permissions, is_admin_user +from archivebox.core.permissions import ( + PERMISSIONS_CHOICES, + PERMISSIONS_PUBLIC, + PERMISSIONS_UNLISTED, + filter_personas_by_permissions, + is_admin_user, +) from archivebox.core.widgets import TagEditorWidget, URLFiltersWidget from archivebox.hooks import get_plugins, discover_plugin_configs, get_plugin_icon from archivebox.personas.models import Persona @@ -715,6 +721,7 @@ class AddLinkForm(PluginConfigFormMixin, forms.Form): def __init__(self, *args, **kwargs): self.request = kwargs.pop("request", None) self.can_override_crawl_config = bool(self.request and is_admin_user(self.request)) + self.is_authenticated_user = bool(self.request and self.request.user.is_authenticated) super().__init__(*args, **kwargs) default_persona = Persona.get_or_create_default() @@ -728,6 +735,16 @@ class AddLinkForm(PluginConfigFormMixin, forms.Form): if selected_persona: self.fields["persona"].initial = selected_persona.name self.fields["permissions"].initial = default_config.PERMISSIONS + if not self.can_override_crawl_config: + # Non-staff users may only mark their own crawl public/unlisted (never private), + # and anonymous users have no say at all (forced public, see clean()). + self.fields["permissions"].choices = [ + (PERMISSIONS_PUBLIC, "Public"), + (PERMISSIONS_UNLISTED, "Unlisted"), + ] + self.fields["permissions"].required = self.is_authenticated_user + if str(self.fields["permissions"].initial or "").strip().lower() not in {PERMISSIONS_PUBLIC, PERMISSIONS_UNLISTED}: + self.fields["permissions"].initial = PERMISSIONS_PUBLIC self.fields["timeout"].initial = default_config.TIMEOUT self.fields["crawl_max_concurrent_snapshots"].initial = default_config.CRAWL_MAX_CONCURRENT_SNAPSHOTS self.fields["delete_after"].initial = default_config.DELETE_AFTER @@ -756,9 +773,16 @@ class AddLinkForm(PluginConfigFormMixin, forms.Form): cleaned_data = super().clean() or {} if not self.can_override_crawl_config: + # Non-staff users cannot set any crawl config (plugins, plugin_config, custom + # KEY=VALUE overrides, or resource limits). Those values are dropped here and + # the limit fields are re-derived from persona/server defaults in the view. cleaned_data["plugins"] = [] cleaned_data["plugin_config"] = {} cleaned_data["config"] = {} + permission = str(cleaned_data.get("permissions") or "").strip().lower() + if not self.is_authenticated_user or permission not in {PERMISSIONS_PUBLIC, PERMISSIONS_UNLISTED}: + permission = PERMISSIONS_PUBLIC + cleaned_data["permissions"] = permission return cleaned_data # Combine all plugin groups into single list diff --git a/archivebox/core/views.py b/archivebox/core/views.py index 7556a691..aed07408 100644 --- a/archivebox/core/views.py +++ b/archivebox/core/views.py @@ -1157,6 +1157,7 @@ class AddView(UserPassesTestMixin, FormView): "persona_config_map_json": json.dumps(persona_config_map, sort_keys=True, default=str), "recent_personas": recent_personas, "can_override_crawl_config": can_override_crawl_config, + "can_select_permissions": self.request.user.is_authenticated, "stdout": "", } @@ -1176,6 +1177,11 @@ class AddView(UserPassesTestMixin, FormView): crawl_max_concurrent_snapshots = int(form.cleaned_data["crawl_max_concurrent_snapshots"]) permissions = str(form.cleaned_data.get("permissions") or "public").strip().lower() can_override_crawl_config = self._can_override_crawl_config() + if not can_override_crawl_config: + # Authenticated non-staff users may only choose public/unlisted; anonymous users + # are always forced to public. Never honor a non-staff request for private. + if not self.request.user.is_authenticated or permissions not in {"public", "unlisted"}: + permissions = "public" plugins = ",".join(form.cleaned_data.get("plugins", [])) if can_override_crawl_config else "" schedule = form.cleaned_data.get("schedule", "").strip() if can_override_crawl_config else "" persona = form.cleaned_data.get("persona") @@ -1219,21 +1225,24 @@ class AddView(UserPassesTestMixin, FormView): if plugins: config["PLUGINS"] = plugins effective_config = get_config(persona=persona, user=self.request.user) if persona else get_config(user=self.request.user) - if crawl_max_concurrent_snapshots != int(effective_config.CRAWL_MAX_CONCURRENT_SNAPSHOTS): - config["CRAWL_MAX_CONCURRENT_SNAPSHOTS"] = crawl_max_concurrent_snapshots - if delete_after != str(effective_config.DELETE_AFTER): - config["DELETE_AFTER"] = delete_after + if can_override_crawl_config: + # Resource limits are crawl config and are only honored for staff. Non-staff + # users inherit these from the (public) persona / server defaults at hook runtime. + if crawl_max_concurrent_snapshots != int(effective_config.CRAWL_MAX_CONCURRENT_SNAPSHOTS): + config["CRAWL_MAX_CONCURRENT_SNAPSHOTS"] = crawl_max_concurrent_snapshots + if delete_after != str(effective_config.DELETE_AFTER): + config["DELETE_AFTER"] = delete_after + if max_urls: + config["CRAWL_MAX_URLS"] = max_urls + if crawl_max_size: + config["CRAWL_MAX_SIZE"] = crawl_max_size + if crawl_timeout: + config["CRAWL_TIMEOUT"] = crawl_timeout + if timeout is not None and int(timeout) != int(effective_config.TIMEOUT): + config["TIMEOUT"] = int(timeout) + if snapshot_max_size: + config["SNAPSHOT_MAX_SIZE"] = snapshot_max_size config["PERMISSIONS"] = permissions - if max_urls: - config["CRAWL_MAX_URLS"] = max_urls - if crawl_max_size: - config["CRAWL_MAX_SIZE"] = crawl_max_size - if crawl_timeout: - config["CRAWL_TIMEOUT"] = crawl_timeout - if timeout is not None and int(timeout) != int(effective_config.TIMEOUT): - config["TIMEOUT"] = int(timeout) - if snapshot_max_size: - config["SNAPSHOT_MAX_SIZE"] = snapshot_max_size # Merge custom config overrides config.update(plugin_config) diff --git a/archivebox/templates/core/add.html b/archivebox/templates/core/add.html index f61d87f6..f3d9c1a5 100644 --- a/archivebox/templates/core/add.html +++ b/archivebox/templates/core/add.html @@ -107,6 +107,7 @@ + {% if can_select_permissions %}
@@ -116,8 +117,9 @@ {% if form.permissions.errors %}
{{ form.permissions.errors }}
{% endif %} -
Public lists it. Unlisted only serves direct links. Private requires admin login.
+
Public lists it. Unlisted only serves direct links.{% if can_override_crawl_config %} Private requires admin login.{% endif %}
+ {% endif %}
@@ -139,6 +141,7 @@
+ {% if can_override_crawl_config %}
@@ -224,6 +227,7 @@
Whole numbers, e.g. 1, 4, 12.
+ {% endif %}
{{ form.notes.label_tag }} diff --git a/archivebox/tests/test_ui_add_view.py b/archivebox/tests/test_ui_add_view.py index 73609b8c..bf750e93 100644 --- a/archivebox/tests/test_ui_add_view.py +++ b/archivebox/tests/test_ui_add_view.py @@ -325,24 +325,189 @@ def test_add_view_public_submission_ignores_plugin_and_custom_config(client, adm assert response.status_code == 302, response.context["form"].errors if response.context else response.content.decode() crawl = Crawl.objects.order_by("-created_at").first() assert crawl is not None - assert crawl.config["CRAWL_MAX_URLS"] == 10 - assert crawl.config["CRAWL_MAX_SIZE"] == 45 * 1024 * 1024 - assert crawl.config["CRAWL_TIMEOUT"] == 120 - assert crawl.config["TIMEOUT"] == 90 - assert crawl.config["SNAPSHOT_MAX_SIZE"] == 5 * 1024 * 1024 - assert crawl.config["DELETE_AFTER"] == "2h" - assert crawl.config["CRAWL_MAX_CONCURRENT_SNAPSHOTS"] == 2 + # Resource limits are crawl config and may only be set by staff. Anonymous/non-staff + # submissions inherit them from persona/server defaults instead of writing overrides. + assert "CRAWL_MAX_URLS" not in crawl.config + assert "CRAWL_MAX_SIZE" not in crawl.config + assert "CRAWL_TIMEOUT" not in crawl.config + assert "TIMEOUT" not in crawl.config + assert "SNAPSHOT_MAX_SIZE" not in crawl.config + assert "DELETE_AFTER" not in crawl.config + assert "CRAWL_MAX_CONCURRENT_SNAPSHOTS" not in crawl.config + # URL allow/deny filters and permissions remain available to public submissions. assert crawl.config["URL_ALLOWLIST"] == "example.com" assert crawl.config["URL_DENYLIST"] == "cdn.example.com" + assert crawl.config["PERMISSIONS"] == "public" + assert crawl.max_depth == 0 + # Plugins, plugin config, and custom KEY=VALUE overrides are all rejected. assert "PLUGINS" not in crawl.config assert "WGET_TIMEOUT" not in crawl.config assert "NODE_BINARY" not in crawl.config assert "TWOCAPTCHA_API_KEY" not in crawl.config assert "INDEX_ONLY" not in crawl.config + # schedule + start_paused are staff-only too, so the crawl queues immediately. assert crawl.status == Crawl.StatusChoices.QUEUED assert crawl.schedule is None +# Help text that is rendered only when the matching field is actually drawn. We key the +# render assertions off these (not name="..."), because the page's JavaScript references +# the field names in querySelector() strings even when the inputs themselves are hidden. +LIMIT_GRID_MARKER = b"Whole numbers, e.g. 1, 4, 12." +DELETE_AFTER_MARKER = b"0 = keep forever. Durations: 1hr, 30d, 3mo." +PERMISSIONS_MARKER = b"only serves direct links" +ADVANCED_MARKER = b"Advanced Crawl Options" +CUSTOM_CONFIG_MARKER = b"Custom config overrides" + + +def test_add_view_anonymous_hides_limit_and_permission_fields(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + + response = client.get(reverse("add"), HTTP_HOST=WEB_HOST) + + assert response.status_code == 200 + assert response.context["can_override_crawl_config"] is False + assert response.context["can_select_permissions"] is False + # Limit grid, permissions selector, and the advanced/custom-config section are all hidden. + assert LIMIT_GRID_MARKER not in response.content + assert DELETE_AFTER_MARKER not in response.content + assert PERMISSIONS_MARKER not in response.content + assert ADVANCED_MARKER not in response.content + assert CUSTOM_CONFIG_MARKER not in response.content + # The allowed public fields stay intact. + assert b"archive just these URLs" in response.content # depth radios + assert b"Optional description for this crawl" in response.content # notes + assert b'name="persona"' in response.content + assert b'name="url_filters_only_new"' in response.content + + +def test_add_view_authenticated_non_staff_shows_permissions_without_private(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + user = User.objects.create_user(username="plainuser", email="plain@test.com", password="testpassword") + client.force_login(user) + + response = client.get(reverse("add"), HTTP_HOST=WEB_HOST) + form = response.context["form"] + + assert response.status_code == 200 + assert response.context["can_override_crawl_config"] is False + assert response.context["can_select_permissions"] is True + assert [value for value, _label in form.fields["permissions"].choices] == ["public", "unlisted"] + # Permissions selector renders, but only with public/unlisted options (no private). + assert PERMISSIONS_MARKER in response.content + assert b'value="public"' in response.content + assert b'value="unlisted"' in response.content + assert b'value="private"' not in response.content + # Limit grid, advanced section, and plugin config still hidden for non-staff. + assert LIMIT_GRID_MARKER not in response.content + assert ADVANCED_MARKER not in response.content + assert form.plugin_groups == [] + + +def test_add_view_anonymous_cannot_choose_unlisted(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + + response = client.post( + reverse("add"), + data={ + "url": "https://example.com/anon-unlisted", + "depth": "0", + "persona": "Default", + "permissions": "unlisted", + }, + HTTP_HOST=WEB_HOST, + ) + + assert response.status_code == 302, response.context["form"].errors if response.context else response.content.decode() + crawl = Crawl.objects.order_by("-created_at").first() + assert crawl is not None + # Anonymous users have no say over permissions — always forced to public. + assert crawl.config["PERMISSIONS"] == "public" + + +def test_add_view_authenticated_non_staff_can_set_unlisted(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + user = User.objects.create_user(username="unlisteduser", email="unlisted@test.com", password="testpassword") + client.force_login(user) + + response = client.post( + reverse("add"), + data={ + "url": "https://example.com/user-unlisted", + "depth": "0", + "persona": "Default", + "permissions": "unlisted", + }, + HTTP_HOST=WEB_HOST, + ) + + assert response.status_code == 302, response.context["form"].errors if response.context else response.content.decode() + crawl = Crawl.objects.order_by("-created_at").first() + assert crawl is not None + assert crawl.config["PERMISSIONS"] == "unlisted" + + +def test_add_view_non_staff_cannot_set_private(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + user = User.objects.create_user(username="privuser", email="priv@test.com", password="testpassword") + client.force_login(user) + crawl_count_before = Crawl.objects.count() + + response = client.post( + reverse("add"), + data={ + "url": "https://example.com/user-private", + "depth": "0", + "persona": "Default", + "permissions": "private", + }, + HTTP_HOST=WEB_HOST, + ) + + # 'private' is not a valid choice for non-staff, so the submission is rejected outright. + assert response.status_code == 200 + assert "permissions" in response.context["form"].errors + assert Crawl.objects.count() == crawl_count_before + + +def test_add_view_non_staff_limit_overrides_are_rejected(client, monkeypatch): + monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") + user = User.objects.create_user(username="limituser", email="limit@test.com", password="testpassword") + client.force_login(user) + + response = client.post( + reverse("add"), + data={ + "url": "https://example.com/user-limits", + "depth": "0", + "persona": "Default", + "permissions": "public", + # Even an authenticated non-staff user cannot write crawl resource limits. + "max_urls": "999", + "crawl_max_size": "45mb", + "timeout": "1.5m", + "crawl_max_concurrent_snapshots": "9", + "delete_after": "2h", + "config": '{"TIMEOUT": 99}', + "plugin_config__wget__WGET_TIMEOUT": "77", + }, + HTTP_HOST=WEB_HOST, + ) + + assert response.status_code == 302, response.context["form"].errors if response.context else response.content.decode() + crawl = Crawl.objects.order_by("-created_at").first() + assert crawl is not None + for key in ( + "CRAWL_MAX_URLS", + "CRAWL_MAX_SIZE", + "TIMEOUT", + "CRAWL_MAX_CONCURRENT_SNAPSHOTS", + "DELETE_AFTER", + "WGET_TIMEOUT", + ): + assert key not in crawl.config, key + + def test_add_view_queues_crawl_for_background_runner(client, admin_user, monkeypatch): monkeypatch.setenv("PUBLIC_ADD_VIEW", "true") client.force_login(admin_user)