mirror of
https://github.com/ArchiveBox/ArchiveBox.git
synced 2026-09-12 19:50:57 +05:00
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__<plugin>__<KEY> 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) <noreply@anthropic.com>
This commit is contained in:
parent
2c2215f84d
commit
00c6731e84
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -107,6 +107,7 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{% if can_select_permissions %}
|
||||
<div class="form-field permissions-field">
|
||||
<div class="field-label-with-icon">
|
||||
<span class="field-label-icon" aria-hidden="true">🔒</span>
|
||||
@ -116,8 +117,9 @@
|
||||
{% if form.permissions.errors %}
|
||||
<div class="error">{{ form.permissions.errors }}</div>
|
||||
{% endif %}
|
||||
<div class="help-text">Public lists it. Unlisted only serves direct links. Private requires admin login.</div>
|
||||
<div class="help-text">Public lists it. Unlisted only serves direct links.{% if can_override_crawl_config %} Private requires admin login.{% endif %}</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
</div>
|
||||
|
||||
<div class="settings-row">
|
||||
@ -139,6 +141,7 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{% if can_override_crawl_config %}
|
||||
<div class="crawl-limit-grid">
|
||||
<div class="crawl-limit-field">
|
||||
<div class="field-label-with-icon">
|
||||
@ -224,6 +227,7 @@
|
||||
<div class="help-text">Whole numbers, e.g. 1, 4, 12.</div>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
<div class="form-field">
|
||||
{{ form.notes.label_tag }}
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user