From d8280e69c4c9cc1053ca49db81de24523a30c8d8 Mon Sep 17 00:00:00 2001 From: Nimish Date: Sat, 14 Mar 2026 13:25:42 +0800 Subject: [PATCH] fix: scope secret PUT/DELETE operations to authenticated environment --- backend/api/views/secrets.py | 28 ++- .../tests/api/views/test_secrets_env_scope.py | 189 ++++++++++++++++++ 2 files changed, 207 insertions(+), 10 deletions(-) create mode 100644 backend/tests/api/views/test_secrets_env_scope.py diff --git a/backend/api/views/secrets.py b/backend/api/views/secrets.py index 4a9981632..216911568 100644 --- a/backend/api/views/secrets.py +++ b/backend/api/views/secrets.py @@ -375,6 +375,13 @@ def put(self, request, *args, **kwargs): secret_obj = Secret.objects.get(id=secret["id"]) + # Verify the secret belongs to the authenticated environment + if secret_obj.environment_id != env.id: + return JsonResponse( + {"error": "Secret does not belong to the authenticated environment"}, + status=403, + ) + tags = SecretTag.objects.filter( name__in=secret["tags"], organisation=env.app.organisation ) @@ -406,7 +413,6 @@ def put(self, request, *args, **kwargs): ) secret_data = { - "environment": env, "key": secret["key"], "key_digest": secret["keyDigest"], "value": secret["value"], @@ -459,17 +465,17 @@ def put(self, request, *args, **kwargs): def delete(self, request, *args, **kwargs): + env = request.auth["environment"] + request_body = json.loads(request.body) ip_address, user_agent = get_resolver_request_meta(request) - secrets_to_delete = Secret.objects.filter(id__in=request_body["secrets"]) + secrets_to_delete = Secret.objects.filter(id__in=request_body["secrets"], environment=env) if not secrets_to_delete.exists(): return Response(status=status.HTTP_200_OK) - env = secrets_to_delete[0].environment - for secret in secrets_to_delete: secret.updated_at = timezone.now() secret.deleted_at = timezone.now() @@ -873,6 +879,13 @@ def put(self, request, *args, **kwargs): secret_obj = Secret.objects.get(id=secret["id"]) + # Verify the secret belongs to the authenticated environment + if secret_obj.environment_id != env.id: + return Response( + {"error": "Secret does not belong to the authenticated environment"}, + status=403, + ) + if "key" not in secret: secret["key"] = secret_obj.key secret["keyDigest"] = secret_obj.key_digest @@ -888,7 +901,6 @@ def put(self, request, *args, **kwargs): secret["comment"] = secret_obj.comment secret_data = { - "environment": env, "key": secret["key"], "key_digest": secret["keyDigest"], "value": secret["value"], @@ -976,11 +988,7 @@ def delete(self, request, *args, **kwargs): ip_address, user_agent = get_resolver_request_meta(request) - secrets_to_delete = Secret.objects.filter(id__in=request_body["secrets"]) - - for secret in secrets_to_delete: - if not Secret.objects.filter(id=secret.id).exists(): - return JsonResponse({"error": "Secret does not exist"}, status=404) + secrets_to_delete = Secret.objects.filter(id__in=request_body["secrets"], environment=env) for secret in secrets_to_delete: secret.updated_at = timezone.now() diff --git a/backend/tests/api/views/test_secrets_env_scope.py b/backend/tests/api/views/test_secrets_env_scope.py new file mode 100644 index 000000000..1140af5ab --- /dev/null +++ b/backend/tests/api/views/test_secrets_env_scope.py @@ -0,0 +1,189 @@ +import pytest +import json +from unittest.mock import MagicMock, patch, PropertyMock +from django.http import JsonResponse +from rest_framework.test import APIRequestFactory +from api.views.secrets import E2EESecretsView + + +class TestE2EESecretsViewEnvironmentScoping: + """Tests that secret PUT/DELETE operations are scoped to the authenticated environment.""" + + def _make_authed_request(self, method, env_id, body=None, auth_env=None): + """Helper to create a request with mocked auth.""" + factory = APIRequestFactory() + if method == "PUT": + request = factory.put( + "/secrets/", + data=json.dumps(body), + content_type="application/json", + ) + elif method == "DELETE": + request = factory.delete( + "/secrets/", + data=json.dumps(body), + content_type="application/json", + ) + else: + request = factory.get("/secrets/") + + request.META["HTTP_ENVIRONMENT"] = str(env_id) + request.META["HTTP_HOST"] = "localhost" + + # Mock auth + mock_env = MagicMock() + mock_env.id = auth_env or env_id + mock_env.app.organisation = MagicMock() + mock_env.app.sse_enabled = False + + request.auth = { + "auth_type": "User", + "environment": mock_env, + "org_member": MagicMock(), + "service_token": None, + "service_account_token": None, + "service_account": None, + } + request.user = MagicMock() + return request, mock_env + + @patch("api.views.secrets.log_secret_event") + @patch("api.views.secrets.SecretTag") + @patch("api.views.secrets.validate_encrypted_string", return_value=True) + @patch("api.views.secrets.check_for_duplicates_blind", return_value=False) + @patch("api.views.secrets.Environment") + @patch("api.views.secrets.Secret") + def test_put_rejects_secret_from_different_environment( + self, MockSecret, MockEnv, mock_dup, mock_validate, MockTag, mock_log + ): + """PUT must reject secrets that belong to a different environment.""" + import uuid + + auth_env_id = uuid.uuid4() + other_env_id = uuid.uuid4() + secret_id = uuid.uuid4() + + # Secret belongs to other_env_id, not auth_env_id + mock_secret_obj = MagicMock() + mock_secret_obj.environment_id = other_env_id + mock_secret_obj.version = 1 + MockSecret.objects.get.return_value = mock_secret_obj + + mock_env = MagicMock() + mock_env.id = auth_env_id + MockEnv.objects.get.return_value = mock_env + + request, _ = self._make_authed_request( + "PUT", + auth_env_id, + body={ + "secrets": [ + { + "id": str(secret_id), + "key": "ph:v1:" + "a" * 64 + ":test", + "keyDigest": "digest", + "value": "ph:v1:" + "b" * 64 + ":val", + "comment": "", + "tags": [], + "path": "/", + } + ] + }, + ) + request.auth["environment"] = mock_env + + view = E2EESecretsView() + view.request = request + response = view.put(request) + + assert response.status_code == 403 + + @patch("api.views.secrets.log_secret_event") + @patch("api.views.secrets.Secret") + def test_delete_only_deletes_secrets_in_authenticated_environment( + self, MockSecret, mock_log + ): + """DELETE must filter secrets by the authenticated environment.""" + import uuid + + auth_env_id = uuid.uuid4() + secret_id = uuid.uuid4() + + mock_env = MagicMock() + mock_env.id = auth_env_id + + # Return empty queryset (secret doesn't belong to this env) + mock_qs = MagicMock() + mock_qs.exists.return_value = False + MockSecret.objects.filter.return_value = mock_qs + + request, _ = self._make_authed_request( + "DELETE", + auth_env_id, + body={"secrets": [str(secret_id)]}, + ) + request.auth["environment"] = mock_env + + view = E2EESecretsView() + view.request = request + response = view.delete(request) + + # Verify the filter included environment scoping + MockSecret.objects.filter.assert_called_once_with( + id__in=[str(secret_id)], environment=mock_env + ) + + @patch("api.views.secrets.log_secret_event") + @patch("api.views.secrets.SecretTag") + @patch("api.views.secrets.validate_encrypted_string", return_value=True) + @patch("api.views.secrets.check_for_duplicates_blind", return_value=False) + @patch("api.views.secrets.Environment") + @patch("api.views.secrets.Secret") + def test_put_allows_secret_from_same_environment( + self, MockSecret, MockEnv, mock_dup, mock_validate, MockTag, mock_log + ): + """PUT must allow updates to secrets that belong to the authenticated environment.""" + import uuid + + env_id = uuid.uuid4() + secret_id = uuid.uuid4() + + mock_secret_obj = MagicMock() + mock_secret_obj.environment_id = env_id + mock_secret_obj.version = 1 + mock_secret_obj.key = "ph:v1:" + "a" * 64 + ":test" + mock_secret_obj.key_digest = "digest" + mock_secret_obj.value = "ph:v1:" + "b" * 64 + ":val" + mock_secret_obj.comment = "" + MockSecret.objects.get.return_value = mock_secret_obj + + mock_env = MagicMock() + mock_env.id = env_id + mock_env.app.organisation = MagicMock() + MockEnv.objects.get.return_value = mock_env + MockTag.objects.filter.return_value = [] + + request, _ = self._make_authed_request( + "PUT", + env_id, + body={ + "secrets": [ + { + "id": str(secret_id), + "key": "ph:v1:" + "a" * 64 + ":test", + "keyDigest": "digest", + "value": "ph:v1:" + "b" * 64 + ":val", + "comment": "", + "tags": [], + "path": "/", + } + ] + }, + ) + request.auth["environment"] = mock_env + + view = E2EESecretsView() + view.request = request + response = view.put(request) + + assert response.status_code == 200