From c5e416dfdfb9430d2dd1869411bd02a396197d56 Mon Sep 17 00:00:00 2001
From: Floris Bruynooghe <flub@sentry.io>
Date: Fri, 1 Oct 2021 10:01:35 +0200
Subject: [PATCH] fix(symbolSources): Deal better with missing hidden secrets
 (#28996)

This improves handling of someone sending in missing hidden secrets.
This could still be a UI bug though, so do still capture a message but
do not break in an unexpected way that's hard to understand.

Fixes SENTRY-S3C
---
 src/sentry/lang/native/symbolicator.py        | 16 ++++----
 .../api/endpoints/test_project_details.py     | 37 ++++++++++++++++++-
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/src/sentry/lang/native/symbolicator.py b/src/sentry/lang/native/symbolicator.py
index 239ad17507c4f8..8788477c7279e4 100644
--- a/src/sentry/lang/native/symbolicator.py
+++ b/src/sentry/lang/native/symbolicator.py
@@ -19,7 +19,7 @@
 from sentry.models import Organization
 from sentry.net.http import Session
 from sentry.tasks.store import RetrySymbolication
-from sentry.utils import json, metrics
+from sentry.utils import json, metrics, safe
 
 MAX_ATTEMPTS = 3
 REQUEST_CACHE_TIMEOUT = 3600
@@ -362,7 +362,7 @@ def parse_backfill_sources(sources_json, original_sources):
     try:
         sources = json.loads(sources_json)
     except Exception as e:
-        raise InvalidSourcesError(f"{e}")
+        raise InvalidSourcesError("Sources are not valid serialised JSON") from e
 
     orig_by_id = {src["id"]: src for src in original_sources}
 
@@ -377,15 +377,17 @@ def parse_backfill_sources(sources_json, original_sources):
 
         for secret in secret_fields(source["type"]):
             if secret in source and source[secret] == {"hidden-secret": True}:
-                orig_source = orig_by_id.get(source["id"])
-                # Should just omit the source entirely if it's referencing a previously stored
-                # secret that we can't find
-                source[secret] = orig_source.get(secret, "")
+                secret_value = safe.get_path(orig_by_id, source["id"], secret)
+                if secret_value is None:
+                    sentry_sdk.capture_message("Hidden secret not present in project options")
+                    raise InvalidSourcesError("Sources contain unknown hidden secret")
+                else:
+                    source[secret] = secret_value
 
     try:
         jsonschema.validate(sources, SOURCES_SCHEMA)
     except jsonschema.ValidationError as e:
-        raise InvalidSourcesError(f"{e}")
+        raise InvalidSourcesError("Sources did not validate JSON-schema") from e
 
     return sources
 
diff --git a/tests/sentry/api/endpoints/test_project_details.py b/tests/sentry/api/endpoints/test_project_details.py
index 9f3258e1b258e3..3cdea633c16c29 100644
--- a/tests/sentry/api/endpoints/test_project_details.py
+++ b/tests/sentry/api/endpoints/test_project_details.py
@@ -816,8 +816,41 @@ def test_redacted_symbol_source_secrets(self, create_audit_entry):
                 self.org_slug, self.proj_slug, symbolSources=json.dumps([redacted_source])
             )
             # on save the magic object should be replaced with the previously set password
-            redacted_source["password"] = "beepbeep"
-            assert self.project.get_option("sentry:symbol_sources") == json.dumps([redacted_source])
+            assert self.project.get_option("sentry:symbol_sources") == json.dumps([config])
+
+    @mock.patch("sentry.api.base.create_audit_entry")
+    def test_redacted_symbol_source_secrets_unknown_secret(self, create_audit_entry):
+        with Feature(
+            {"organizations:symbol-sources": True, "organizations:custom-symbol-sources": True}
+        ):
+            config = {
+                "id": "honk",
+                "name": "honk source",
+                "layout": {
+                    "type": "native",
+                },
+                "filetypes": ["pe"],
+                "type": "http",
+                "url": "http://honk.beep",
+                "username": "honkhonk",
+                "password": "beepbeep",
+            }
+            self.get_valid_response(
+                self.org_slug, self.proj_slug, symbolSources=json.dumps([config])
+            )
+            assert self.project.get_option("sentry:symbol_sources") == json.dumps([config])
+
+            # prepare new call, this secret is not known
+            new_source = config.copy()
+            new_source["password"] = {"hidden-secret": True}
+            new_source["id"] = "oops"
+            response = self.get_response(
+                self.org_slug, self.proj_slug, symbolSources=json.dumps([new_source])
+            )
+            assert response.status_code == 400
+            assert json.loads(response.content) == {
+                "symbolSources": ["Sources contain unknown hidden secret"]
+            }
 
 
 class CopyProjectSettingsTest(APITestCase):