From 19451649fa284b940fbca6b6d4b986d2ba572762 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 26 May 2026 10:53:26 -0400 Subject: [PATCH] Fixes #22283: Restrict ScriptResultView queryset to current user --- netbox/extras/tests/test_views.py | 92 ++++++++++++++++++++++++++++++- netbox/extras/views.py | 5 +- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index e1306a93a..74c88cea8 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -1,18 +1,19 @@ +import uuid from unittest.mock import PropertyMock, patch from django.contrib.contenttypes.models import ContentType from django.test import tag from django.urls import reverse -from core.choices import ManagedFileRootPathChoices +from core.choices import JobStatusChoices, ManagedFileRootPathChoices from core.events import * -from core.models import ObjectType +from core.models import Job, ObjectType from dcim.models import DeviceType, Manufacturer, Site from extras.choices import * from extras.models import * from extras.scripts import BooleanVar, IntegerVar from extras.scripts import Script as PythonClass -from users.models import Group, User +from users.models import Group, ObjectPermission, User from utilities.testing import TestCase, ViewTestCases @@ -1165,3 +1166,88 @@ class ScriptDefaultValuesTestCase(TestCase): self.assertEqual(call_kwargs['data']['bool_default_false'], False) self.assertEqual(call_kwargs['data']['int_with_default'], 0) self.assertIsNone(call_kwargs['data']['int_without_default']) + + +class ScriptResultViewTestCase(TestCase): + SECRET_OUTPUT = 'my secret script output' + + @classmethod + def setUpTestData(cls): + with patch.object(ScriptModule, 'sync_classes'): + module = ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path='test_script.py', + ) + cls.allowed_script = Script.objects.create( + module=module, name='Allowed script', is_executable=True + ) + cls.secret_script = Script.objects.create( + module=module, name='Secret script', is_executable=True + ) + + script_type = ObjectType.objects.get_for_model(Script) + cls.allowed_job = Job.objects.create( + object_type=script_type, + object_id=cls.allowed_script.pk, + name='allowed-job', + job_id=uuid.uuid4(), + status=JobStatusChoices.STATUS_COMPLETED, + data={'log': [], 'output': 'benign output'}, + ) + cls.secret_job = Job.objects.create( + object_type=script_type, + object_id=cls.secret_script.pk, + name='secret-job', + job_id=uuid.uuid4(), + status=JobStatusChoices.STATUS_COMPLETED, + data={'log': [], 'output': cls.SECRET_OUTPUT}, + ) + + def test_get_without_view_job_permission_returns_404(self): + """ + A user with extras.view_script but no core.view_job must not retrieve any job result + via ScriptResultView, even for the script whose object-level permission they hold. + """ + self.add_permissions('extras.view_script') + + url = reverse('extras:script_result', kwargs={'job_pk': self.allowed_job.pk}) + self.assertHttpStatus(self.client.get(url), 404) + + url = reverse('extras:script_result', kwargs={'job_pk': self.secret_job.pk}) + self.assertHttpStatus(self.client.get(url), 404) + + def test_get_export_output_without_view_job_permission_returns_404(self): + """ + Regression for the PoC: the ?export=output path must not leak job.data['output'] + when the user lacks core.view_job. + """ + self.add_permissions('extras.view_script') + + url = reverse('extras:script_result', kwargs={'job_pk': self.secret_job.pk}) + response = self.client.get(url, {'export': 'output'}) + + self.assertHttpStatus(response, 404) + self.assertNotIn(self.SECRET_OUTPUT.encode(), response.content) + + def test_get_with_constrained_view_job_permission(self): + """ + With core.view_job constrained to the allowed job only, the user can fetch the allowed + result but the secret result is hidden (404). + """ + self.add_permissions('extras.view_script') + obj_perm = ObjectPermission( + name='View allowed job only', + constraints={'pk': self.allowed_job.pk}, + actions=['view'], + ) + obj_perm.save() + obj_perm.users.add(self.user) + obj_perm.object_types.add(ObjectType.objects.get_for_model(Job)) + + url = reverse('extras:script_result', kwargs={'job_pk': self.allowed_job.pk}) + self.assertHttpStatus(self.client.get(url), 200) + + url = reverse('extras:script_result', kwargs={'job_pk': self.secret_job.pk}) + response = self.client.get(url, {'export': 'output'}) + self.assertHttpStatus(response, 404) + self.assertNotIn(self.SECRET_OUTPUT.encode(), response.content) diff --git a/netbox/extras/views.py b/netbox/extras/views.py index f57aa6fcf..344830e67 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -1803,6 +1803,9 @@ class ScriptResultView(TableMixin, generic.ObjectView): def get_required_permission(self): return 'extras.view_script' + def get_queryset(self, request): + return Job.objects.restrict(request.user, 'view') + def get_table(self, job, request, bulk_actions=True): data = [] tests = None @@ -1863,7 +1866,7 @@ class ScriptResultView(TableMixin, generic.ObjectView): def get(self, request, **kwargs): table = None - job = get_object_or_404(Job.objects.all(), pk=kwargs.get('job_pk')) + job = get_object_or_404(self.queryset, pk=kwargs.get('job_pk')) # If a direct export output has been requested, return the job data content as a # downloadable file.