Browse Source

feat(symbol-sources): Redact symbol source secrets from audit logs (#28334)

This redacts any secrets stored in symbol sources before they are logged
as an audit entry.

Co-authored-by: Floris Bruynooghe <flub@sentry.io>
relaxolotl 3 years ago
parent
commit
adb01c0a47

+ 6 - 3
src/sentry/api/endpoints/project_app_store_connect_credentials.py

@@ -70,7 +70,7 @@ from sentry.api.exceptions import (
 )
 from sentry.api.fields.secret import SecretField, validate_secret
 from sentry.lang.native import appconnect
-from sentry.lang.native.symbolicator import secret_fields
+from sentry.lang.native.symbolicator import redact_source_secrets, secret_fields
 from sentry.models import AppConnectBuild, AuditLogEntryEvent, LatestAppConnectBuildsCheck, Project
 from sentry.tasks.app_store_connect import dsym_download
 from sentry.utils import json
@@ -287,12 +287,14 @@ class AppStoreConnectCreateCredentialsEndpoint(ProjectEndpoint):  # type: ignore
             new_sources = validated_config.update_project_symbol_source(project, allow_multiple)
         except ValueError:
             raise AppConnectMultipleSourcesError
+
+        redacted_sources = redact_source_secrets(new_sources)
         self.create_audit_entry(
             request=request,
             organization=project.organization,
             target_object=project.id,
             event=AuditLogEntryEvent.PROJECT_EDIT,
-            data={appconnect.SYMBOL_SOURCES_PROP_NAME: new_sources},
+            data={appconnect.SYMBOL_SOURCES_PROP_NAME: redacted_sources},
         )
 
         dsym_download.apply_async(
@@ -403,12 +405,13 @@ class AppStoreConnectUpdateCredentialsEndpoint(ProjectEndpoint):  # type: ignore
             project, allow_multiple=True
         )
 
+        redacted_sources = redact_source_secrets(new_sources)
         self.create_audit_entry(
             request=request,
             organization=project.organization,
             target_object=project.id,
             event=AuditLogEntryEvent.PROJECT_EDIT,
-            data={appconnect.SYMBOL_SOURCES_PROP_NAME: new_sources},
+            data={appconnect.SYMBOL_SOURCES_PROP_NAME: redacted_sources},
         )
 
         dsym_download.apply_async(

+ 9 - 1
src/sentry/api/endpoints/project_details.py

@@ -26,6 +26,7 @@ from sentry.lang.native.symbolicator import (
     InvalidSourcesError,
     parse_backfill_sources,
     parse_sources,
+    redact_source_secrets,
 )
 from sentry.lang.native.utils import convert_crashreport_count
 from sentry.models import (
@@ -559,7 +560,14 @@ class ProjectDetailsEndpoint(ProjectEndpoint):
                 ]
         if result.get("symbolSources") is not None:
             if project.update_option("sentry:symbol_sources", result["symbolSources"]):
-                changed_proj_settings["sentry:symbol_sources"] = result["symbolSources"] or None
+                # Redact secrets so they don't get logged directly to the Audit Log
+                sources_json = result["symbolSources"] or None
+                try:
+                    sources = parse_sources(sources_json)
+                except Exception:
+                    sources = []
+                redacted_sources = redact_source_secrets(sources)
+                changed_proj_settings["sentry:symbol_sources"] = redacted_sources
         if "defaultEnvironment" in result:
             if result["defaultEnvironment"] is None:
                 project.delete_option("sentry:default_environment")

+ 8 - 4
src/sentry/api/serializers/models/project.py

@@ -18,7 +18,7 @@ from sentry.digests import backend as digests
 from sentry.eventstore.models import DEFAULT_SUBJECT_TEMPLATE
 from sentry.features.base import ProjectFeature
 from sentry.ingest.inbound_filters import FilterTypes
-from sentry.lang.native.symbolicator import redact_source_secrets
+from sentry.lang.native.symbolicator import parse_sources, redact_source_secrets
 from sentry.lang.native.utils import convert_crashreport_count
 from sentry.models import (
     EnvironmentProject,
@@ -41,6 +41,7 @@ from sentry.notifications.helpers import (
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
 from sentry.snuba import discover
 from sentry.snuba.sessions import check_has_health_data, get_current_and_previous_crash_free_rates
+from sentry.utils import json
 from sentry.utils.compat import zip
 
 STATUS_LABELS = {
@@ -797,16 +798,19 @@ class DetailedProjectSerializer(ProjectWithTeamSerializer):
         )
         custom_symbol_sources_json = attrs["options"].get("sentry:symbol_sources")
         try:
-            symbol_sources = redact_source_secrets(custom_symbol_sources_json)
+            sources = parse_sources(custom_symbol_sources_json, False)
         except Exception:
             # In theory sources stored on the project should be valid. If they are invalid, we don't
             # want to abort serialization just for sources, so just return an empty list instead of
             # returning sources with their secrets included.
-            symbol_sources = "[]"
+            serialized_sources = "[]"
+        else:
+            redacted_sources = redact_source_secrets(sources)
+            serialized_sources = json.dumps(redacted_sources)
 
         data.update(
             {
-                "symbolSources": symbol_sources,
+                "symbolSources": serialized_sources,
             }
         )
 

+ 8 - 6
src/sentry/lang/native/symbolicator.py

@@ -4,6 +4,7 @@ import random
 import sys
 import time
 import uuid
+from copy import deepcopy
 from urllib.parse import urljoin
 
 import jsonschema
@@ -389,20 +390,21 @@ def parse_backfill_sources(sources_json, original_sources):
     return sources
 
 
-def redact_source_secrets(config_sources):
+def redact_source_secrets(config_sources: json.JSONData) -> json.JSONData:
     """
-    Returns a json string with all of the secrets redacted from every source.
+    Returns a JSONData with all of the secrets redacted from every source.
 
-    May raise InvalidSourcesError if the provided sources are invalid.
+    The original value is not mutated in the process; A clone is created
+    and returned by this function.
     """
 
-    sources = parse_sources(config_sources, False)
-    for source in sources:
+    redacted_sources = deepcopy(config_sources)
+    for source in redacted_sources:
         for secret in secret_fields(source["type"]):
             if secret in source:
                 source[secret] = {"hidden-secret": True}
 
-    return json.dumps(sources)
+    return redacted_sources
 
 
 def get_options_for_project(project):

+ 13 - 5
tests/sentry/api/endpoints/test_project_details.py

@@ -27,7 +27,7 @@ from sentry.models import (
     ScheduledDeletion,
 )
 from sentry.testutils import APITestCase
-from sentry.testutils.helpers import Feature
+from sentry.testutils.helpers import Feature, faux
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
 from sentry.utils.compat import mock, zip
@@ -752,11 +752,12 @@ class ProjectUpdateTest(APITestCase):
         expiry = response.data["secondaryGroupingExpiry"]
         assert (now + 3600 * 24 * 90) < expiry < (now + 3600 * 24 * 92)
 
-    def test_redacted_symbol_source_secrets(self):
+    @mock.patch("sentry.api.base.create_audit_entry")
+    def test_redacted_symbol_source_secrets(self, create_audit_entry):
         with Feature(
             {"organizations:symbol-sources": True, "organizations:custom-symbol-sources": True}
         ):
-            redacted_source = {
+            config = {
                 "id": "honk",
                 "name": "honk source",
                 "layout": {
@@ -769,12 +770,19 @@ class ProjectUpdateTest(APITestCase):
                 "password": "beepbeep",
             }
             self.get_valid_response(
-                self.org_slug, self.proj_slug, symbolSources=json.dumps([redacted_source])
+                self.org_slug, self.proj_slug, symbolSources=json.dumps([config])
             )
-            assert self.project.get_option("sentry:symbol_sources") == json.dumps([redacted_source])
+            assert self.project.get_option("sentry:symbol_sources") == json.dumps([config])
 
             # redact password
+            redacted_source = config.copy()
             redacted_source["password"] = {"hidden-secret": True}
+
+            # check that audit entry was created with redacted password
+            assert create_audit_entry.called
+            call = faux.faux(create_audit_entry)
+            assert call.kwarg_equals("data", {"sentry:symbol_sources": [redacted_source]})
+
             self.get_valid_response(
                 self.org_slug, self.proj_slug, symbolSources=json.dumps([redacted_source])
             )