Work with a copy of request, vars in the event (#2125)

* Work with a copy of request, vars in the event

In some cases we were attaching parts of the original request to the event with live references on them and
ending up modifying the underlying headers or request data when we scrubbed the event. Now we make sure to only attach a copy of the request to the event. We also do the same for frame vars.

Author: sentrivana

sentry_sdk/integrations/_wsgi_common.py

@@ -1,4 +1,5 @@
 import json
+from copy import deepcopy
 
 from sentry_sdk.hub import Hub, _should_send_default_pii
 from sentry_sdk.utils import AnnotatedValue
@@ -77,7 +78,7 @@ def extract_into_event(self, event):
         if data is not None:
             request_info["data"] = data
 
-        event["request"] = request_info
+        event["request"] = deepcopy(request_info)
 
     def content_length(self):
         # type: () -> int

sentry_sdk/integrations/asgi.py

@@ -7,6 +7,7 @@
 import asyncio
 import inspect
 import urllib
+from copy import deepcopy
 
 from sentry_sdk._functools import partial
 from sentry_sdk._types import TYPE_CHECKING
@@ -211,7 +212,7 @@ def event_processor(self, event, hint, asgi_scope):
 
         self._set_transaction_name_and_source(event, self.transaction_style, asgi_scope)
 
-        event["request"] = request_info
+        event["request"] = deepcopy(request_info)
 
         return event
 

sentry_sdk/integrations/aws_lambda.py

@@ -1,8 +1,9 @@
+import sys
+from copy import deepcopy
 from datetime import datetime, timedelta
 from os import environ
-import sys
-from sentry_sdk.consts import OP
 
+from sentry_sdk.consts import OP
 from sentry_sdk.hub import Hub, _should_send_default_pii
 from sentry_sdk.tracing import TRANSACTION_SOURCE_COMPONENT, Transaction
 from sentry_sdk._compat import reraise
@@ -380,7 +381,7 @@ def event_processor(sentry_event, hint, start_time=start_time):
                 # event. Meaning every body is unstructured to us.
                 request["data"] = AnnotatedValue.removed_because_raw_data()
 
-        sentry_event["request"] = request
+        sentry_event["request"] = deepcopy(request)
 
         return sentry_event
 

sentry_sdk/integrations/fastapi.py

@@ -1,4 +1,5 @@
 import asyncio
+from copy import deepcopy
 
 from sentry_sdk._types import TYPE_CHECKING
 from sentry_sdk.hub import Hub, _should_send_default_pii
@@ -116,7 +117,7 @@ def event_processor(event, hint):
                                 request_info["cookies"] = info["cookies"]
                             if "data" in info:
                                 request_info["data"] = info["data"]
-                        event["request"] = request_info
+                        event["request"] = deepcopy(request_info)
 
                         return event
 

sentry_sdk/integrations/gcp.py

@@ -1,8 +1,9 @@
+import sys
+from copy import deepcopy
 from datetime import datetime, timedelta
 from os import environ
-import sys
-from sentry_sdk.consts import OP
 
+from sentry_sdk.consts import OP
 from sentry_sdk.hub import Hub, _should_send_default_pii
 from sentry_sdk.tracing import TRANSACTION_SOURCE_COMPONENT, Transaction
 from sentry_sdk._compat import reraise
@@ -193,7 +194,7 @@ def event_processor(event, hint):
                 # event. Meaning every body is unstructured to us.
                 request["data"] = AnnotatedValue.removed_because_raw_data()
 
-        event["request"] = request
+        event["request"] = deepcopy(request)
 
         return event
 

sentry_sdk/integrations/starlette.py

@@ -2,6 +2,7 @@
 
 import asyncio
 import functools
+from copy import deepcopy
 
 from sentry_sdk._compat import iteritems
 from sentry_sdk._types import TYPE_CHECKING
@@ -389,7 +390,7 @@ def event_processor(event, hint):
                                     request_info["cookies"] = info["cookies"]
                                 if "data" in info:
                                     request_info["data"] = info["data"]
-                            event["request"] = request_info
+                            event["request"] = deepcopy(request_info)
 
                             return event
 
@@ -435,7 +436,7 @@ def event_processor(event, hint):
                             if cookies:
                                 request_info["cookies"] = cookies
 
-                            event["request"] = request_info
+                            event["request"] = deepcopy(request_info)
 
                             return event
 

sentry_sdk/utils.py

@@ -10,6 +10,7 @@
 import threading
 import time
 from collections import namedtuple
+from copy import copy
 from decimal import Decimal
 from numbers import Real
 
@@ -627,7 +628,7 @@ def serialize_frame(
         )
 
     if include_local_variables:
-        rv["vars"] = frame.f_locals
+        rv["vars"] = copy(frame.f_locals)
 
     return rv
 

tests/integrations/fastapi/test_fastapi.py

@@ -1,12 +1,13 @@
 import json
+import logging
 import threading
 
 import pytest
 from sentry_sdk.integrations.fastapi import FastApiIntegration
 
 fastapi = pytest.importorskip("fastapi")
 
-from fastapi import FastAPI
+from fastapi import FastAPI, Request
 from fastapi.testclient import TestClient
 from sentry_sdk import capture_message
 from sentry_sdk.integrations.starlette import StarletteIntegration
@@ -187,3 +188,33 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en
         transactions = profile.payload.json["transactions"]
         assert len(transactions) == 1
         assert str(data["active"]) == transactions[0]["active_thread_id"]
+
+
+@pytest.mark.asyncio
+async def test_original_request_not_scrubbed(sentry_init, capture_events):
+    sentry_init(
+        integrations=[StarletteIntegration(), FastApiIntegration()],
+        traces_sample_rate=1.0,
+        debug=True,
+    )
+
+    app = FastAPI()
+
+    @app.post("/error")
+    async def _error(request: Request):
+        logging.critical("Oh no!")
+        assert request.headers["Authorization"] == "Bearer ohno"
+        assert await request.json() == {"password": "secret"}
+
+        return {"error": "Oh no!"}
+
+    events = capture_events()
+
+    client = TestClient(app)
+    client.post(
+        "/error", json={"password": "secret"}, headers={"Authorization": "Bearer ohno"}
+    )
+
+    event = events[0]
+    assert event["request"]["data"] == {"password": "[Filtered]"}
+    assert event["request"]["headers"]["authorization"] == "[Filtered]"

tests/integrations/flask/test_flask.py

@@ -816,3 +816,26 @@ def index():
         response = client.get("/")
         assert response.status_code == 200
         assert response.data == b"hi"
+
+
+def test_request_not_modified_by_reference(sentry_init, capture_events, app):
+    sentry_init(integrations=[flask_sentry.FlaskIntegration()])
+
+    @app.route("/", methods=["POST"])
+    def index():
+        logging.critical("oops")
+        assert request.get_json() == {"password": "ohno"}
+        assert request.headers["Authorization"] == "Bearer ohno"
+        return "ok"
+
+    events = capture_events()
+
+    client = app.test_client()
+    client.post(
+        "/", json={"password": "ohno"}, headers={"Authorization": "Bearer ohno"}
+    )
+
+    (event,) = events
+
+    assert event["request"]["data"]["password"] == "[Filtered]"
+    assert event["request"]["headers"]["Authorization"] == "[Filtered]"

tests/integrations/starlette/test_starlette.py

@@ -2,6 +2,7 @@
 import base64
 import functools
 import json
+import logging
 import os
 import threading
 
@@ -873,3 +874,32 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en
         transactions = profile.payload.json["transactions"]
         assert len(transactions) == 1
         assert str(data["active"]) == transactions[0]["active_thread_id"]
+
+
+def test_original_request_not_scrubbed(sentry_init, capture_events):
+    sentry_init(integrations=[StarletteIntegration()])
+
+    events = capture_events()
+
+    async def _error(request):
+        logging.critical("Oh no!")
+        assert request.headers["Authorization"] == "Bearer ohno"
+        assert await request.json() == {"password": "ohno"}
+        return starlette.responses.JSONResponse({"status": "Oh no!"})
+
+    app = starlette.applications.Starlette(
+        routes=[
+            starlette.routing.Route("/error", _error, methods=["POST"]),
+        ],
+    )
+
+    client = TestClient(app)
+    client.post(
+        "/error",
+        json={"password": "ohno"},
+        headers={"Authorization": "Bearer ohno"},
+    )
+
+    event = events[0]
+    assert event["request"]["data"] == {"password": "[Filtered]"}
+    assert event["request"]["headers"]["authorization"] == "[Filtered]"

tests/test_scrubber.py

@@ -153,3 +153,21 @@ def test_custom_denylist(sentry_init, capture_events):
     assert meta == {
         "my_sensitive_var": {"": {"rem": [["!config", "s"]]}},
     }
+
+
+def test_scrubbing_doesnt_affect_local_vars(sentry_init, capture_events):
+    sentry_init()
+    events = capture_events()
+
+    try:
+        password = "cat123"
+        1 / 0
+    except ZeroDivisionError:
+        capture_exception()
+
+    (event,) = events
+
+    frames = event["exception"]["values"][0]["stacktrace"]["frames"]
+    (frame,) = frames
+    assert frame["vars"]["password"] == "[Filtered]"
+    assert password == "cat123"