From 83e3610b12cd3bd9f9387d07157875e80cff39f3 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:40:19 -0700 Subject: [PATCH 1/4] feat(hybridcloud): Propagate ViewerContext through cross-silo RPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pack ViewerContext into the RPC meta dict on the sending side and restore it via viewer_context_scope on the receiving side. This closes the gap where cross-silo RPC handlers had no ViewerContext despite the original request having one. Uses the existing reserved meta key in the RPC wire format. Backwards compatible — old callers send empty meta, receivers treat missing viewer_context as None. Co-Authored-By: Claude Opus 4.6 --- src/sentry/api/endpoints/internal/rpc.py | 13 ++- src/sentry/hybridcloud/rpc/service.py | 9 +- src/sentry/viewer_context.py | 13 +++ tests/sentry/api/endpoints/test_rpc.py | 102 +++++++++++++++++++++++ 4 files changed, 135 insertions(+), 2 deletions(-) diff --git a/src/sentry/api/endpoints/internal/rpc.py b/src/sentry/api/endpoints/internal/rpc.py index 14040dbf7a36e8..e02ae9a165caac 100644 --- a/src/sentry/api/endpoints/internal/rpc.py +++ b/src/sentry/api/endpoints/internal/rpc.py @@ -1,3 +1,5 @@ +import contextlib + import pydantic import sentry_sdk from rest_framework import status @@ -17,6 +19,7 @@ ) from sentry.hybridcloud.rpc.sig import SerializableFunctionValueException from sentry.utils.env import in_test_environment +from sentry.viewer_context import ViewerContext, viewer_context_scope @internal_all_silo_endpoint @@ -61,8 +64,16 @@ def post(self, request: Request, service_name: str, method_name: str) -> Respons sentry_sdk.capture_exception() raise ParseError from e + meta = request.data.get("meta", {}) + vc_data = meta.get("viewer_context") + vc_scope = ( + viewer_context_scope(ViewerContext.deserialize(vc_data)) + if vc_data + else contextlib.nullcontext() + ) + try: - with auth_context.applied_to_request(request): + with vc_scope, auth_context.applied_to_request(request): result = dispatch_to_local_service(service_name, method_name, arguments) except RpcValidationException as e: return Response( diff --git a/src/sentry/hybridcloud/rpc/service.py b/src/sentry/hybridcloud/rpc/service.py index 5a4bb2058ce37f..2081ab3fa55377 100644 --- a/src/sentry/hybridcloud/rpc/service.py +++ b/src/sentry/hybridcloud/rpc/service.py @@ -571,8 +571,15 @@ def get_method_timeout(self) -> float: return settings.RPC_TIMEOUT def _send_to_remote_silo(self, use_test_client: bool) -> Any: + from sentry.viewer_context import get_viewer_context + + vc = get_viewer_context() + meta: dict[str, Any] = {} + if vc is not None: + meta["viewer_context"] = vc.serialize() + request_body = { - "meta": {}, # reserved for future use + "meta": meta, "args": self.serial_arguments, } data = json.dumps(request_body).encode(_RPC_CONTENT_CHARSET) diff --git a/src/sentry/viewer_context.py b/src/sentry/viewer_context.py index b3c28788efc06d..0e1926a67c06e8 100644 --- a/src/sentry/viewer_context.py +++ b/src/sentry/viewer_context.py @@ -62,6 +62,19 @@ def serialize(self) -> dict[str, Any]: result["token"] = {"kind": self.token.kind, "scopes": list(self.token.get_scopes())} return result + @classmethod + def deserialize(cls, data: dict[str, Any]) -> ViewerContext: + """Reconstruct from a serialized dict. Token is not deserialized.""" + try: + actor_type = ActorType(data.get("actor_type", "unknown")) + except ValueError: + actor_type = ActorType.UNKNOWN + return cls( + organization_id=data.get("organization_id"), + user_id=data.get("user_id"), + actor_type=actor_type, + ) + @contextlib.contextmanager def viewer_context_scope(ctx: ViewerContext) -> Generator[None]: diff --git a/tests/sentry/api/endpoints/test_rpc.py b/tests/sentry/api/endpoints/test_rpc.py index 2cb505ea946ec9..3edbd2e87f4fba 100644 --- a/tests/sentry/api/endpoints/test_rpc.py +++ b/tests/sentry/api/endpoints/test_rpc.py @@ -1,6 +1,7 @@ from __future__ import annotations from typing import Any +from unittest.mock import patch import orjson from django.test import override_settings @@ -10,6 +11,7 @@ from sentry.hybridcloud.rpc.service import generate_request_signature from sentry.organizations.services.organization import RpcUserOrganizationContext from sentry.testutils.cases import APITestCase +from sentry.viewer_context import ActorType, ViewerContext, get_viewer_context @override_settings(RPC_SHARED_SECRET=["a-long-value-that-is-hard-to-guess"]) @@ -142,3 +144,103 @@ def test_with_invalid_arguments(self) -> None: assert response.data == { "detail": ErrorDetail(string="Malformed request.", code="parse_error") } + + def test_viewer_context_propagated_from_meta(self) -> None: + """ViewerContext in meta is set as the contextvar during dispatch.""" + organization = self.create_organization() + captured_contexts: list[ViewerContext | None] = [] + + original_dispatch = __import__( + "sentry.hybridcloud.rpc.service", fromlist=["dispatch_to_local_service"] + ).dispatch_to_local_service + + def capturing_dispatch(*args, **kwargs): + captured_contexts.append(get_viewer_context()) + return original_dispatch(*args, **kwargs) + + path = self._get_path("organization", "get_organization_by_id") + data = { + "args": {"id": organization.id}, + "meta": { + "viewer_context": { + "organization_id": organization.id, + "user_id": 42, + "actor_type": "user", + } + }, + } + + with patch( + "sentry.api.endpoints.internal.rpc.dispatch_to_local_service", + side_effect=capturing_dispatch, + ): + response = self._send_post_request(path, data) + + assert response.status_code == 200 + assert len(captured_contexts) == 1 + ctx = captured_contexts[0] + assert ctx is not None + assert ctx.organization_id == organization.id + assert ctx.user_id == 42 + assert ctx.actor_type == ActorType.USER + + def test_viewer_context_none_when_meta_empty(self) -> None: + """No ViewerContext set when meta is empty.""" + organization = self.create_organization() + captured_contexts: list[ViewerContext | None] = [] + + original_dispatch = __import__( + "sentry.hybridcloud.rpc.service", fromlist=["dispatch_to_local_service"] + ).dispatch_to_local_service + + def capturing_dispatch(*args, **kwargs): + captured_contexts.append(get_viewer_context()) + return original_dispatch(*args, **kwargs) + + path = self._get_path("organization", "get_organization_by_id") + data = {"args": {"id": organization.id}, "meta": {}} + + with patch( + "sentry.api.endpoints.internal.rpc.dispatch_to_local_service", + side_effect=capturing_dispatch, + ): + response = self._send_post_request(path, data) + + assert response.status_code == 200 + assert len(captured_contexts) == 1 + assert captured_contexts[0] is None + + def test_viewer_context_roundtrip_through_meta(self) -> None: + """ViewerContext set on the sending side arrives on the receiving side.""" + organization = self.create_organization() + captured_contexts: list[ViewerContext | None] = [] + + original_dispatch = __import__( + "sentry.hybridcloud.rpc.service", fromlist=["dispatch_to_local_service"] + ).dispatch_to_local_service + + def capturing_dispatch(*args, **kwargs): + captured_contexts.append(get_viewer_context()) + return original_dispatch(*args, **kwargs) + + # Simulate what _send_to_remote_silo builds when ViewerContext is set + ctx = ViewerContext(organization_id=organization.id, user_id=42, actor_type=ActorType.USER) + path = self._get_path("organization", "get_organization_by_id") + data = { + "args": {"id": organization.id}, + "meta": {"viewer_context": ctx.serialize()}, + } + + with patch( + "sentry.api.endpoints.internal.rpc.dispatch_to_local_service", + side_effect=capturing_dispatch, + ): + response = self._send_post_request(path, data) + + assert response.status_code == 200 + assert len(captured_contexts) == 1 + restored = captured_contexts[0] + assert restored is not None + assert restored.organization_id == ctx.organization_id + assert restored.user_id == ctx.user_id + assert restored.actor_type == ctx.actor_type From e11723d05bee0275ae7deac706938dfb07a7088e Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Tue, 7 Apr 2026 09:39:45 -0700 Subject: [PATCH 2/4] ref(hybridcloud): Harden RPC ViewerContext deserialization and fix import Wrap ViewerContext.deserialize() in try/except to return ParseError on malformed viewer_context data, matching the existing AuthenticationContext error handling pattern. Move get_viewer_context import to top-level in service.py since there is no circular dependency. Addresses PR review feedback from markstory. Co-Authored-By: Claude Opus 4.6 --- src/sentry/api/endpoints/internal/rpc.py | 12 +++++++----- src/sentry/hybridcloud/rpc/service.py | 3 +-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/sentry/api/endpoints/internal/rpc.py b/src/sentry/api/endpoints/internal/rpc.py index e02ae9a165caac..a44bb3b86caf5b 100644 --- a/src/sentry/api/endpoints/internal/rpc.py +++ b/src/sentry/api/endpoints/internal/rpc.py @@ -66,11 +66,13 @@ def post(self, request: Request, service_name: str, method_name: str) -> Respons meta = request.data.get("meta", {}) vc_data = meta.get("viewer_context") - vc_scope = ( - viewer_context_scope(ViewerContext.deserialize(vc_data)) - if vc_data - else contextlib.nullcontext() - ) + vc_scope: contextlib.AbstractContextManager[None] = contextlib.nullcontext() + if vc_data: + try: + vc_scope = viewer_context_scope(ViewerContext.deserialize(vc_data)) + except Exception as e: + sentry_sdk.capture_exception() + raise ParseError from e try: with vc_scope, auth_context.applied_to_request(request): diff --git a/src/sentry/hybridcloud/rpc/service.py b/src/sentry/hybridcloud/rpc/service.py index 2081ab3fa55377..34070c61d1c690 100644 --- a/src/sentry/hybridcloud/rpc/service.py +++ b/src/sentry/hybridcloud/rpc/service.py @@ -35,6 +35,7 @@ from sentry.types.cell import Cell, CellMappingNotFound from sentry.utils import json, metrics from sentry.utils.env import in_test_environment +from sentry.viewer_context import get_viewer_context if TYPE_CHECKING: from sentry.hybridcloud.rpc.resolvers import CellResolutionStrategy @@ -571,8 +572,6 @@ def get_method_timeout(self) -> float: return settings.RPC_TIMEOUT def _send_to_remote_silo(self, use_test_client: bool) -> Any: - from sentry.viewer_context import get_viewer_context - vc = get_viewer_context() meta: dict[str, Any] = {} if vc is not None: From 1fa13d4be47a38ecf40919d5112549cdb17156ef Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Tue, 7 Apr 2026 09:48:45 -0700 Subject: [PATCH 3/4] fix(hybridcloud): Handle null meta in RPC endpoint request.data.get("meta", {}) returns None when the payload contains "meta": null, since the default only applies for missing keys. Use `or {}` to handle both missing and null values, preventing an AttributeError on the subsequent .get() call. Co-Authored-By: Claude Opus 4.6 --- src/sentry/api/endpoints/internal/rpc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/api/endpoints/internal/rpc.py b/src/sentry/api/endpoints/internal/rpc.py index a44bb3b86caf5b..5b9eced6066ce0 100644 --- a/src/sentry/api/endpoints/internal/rpc.py +++ b/src/sentry/api/endpoints/internal/rpc.py @@ -64,7 +64,7 @@ def post(self, request: Request, service_name: str, method_name: str) -> Respons sentry_sdk.capture_exception() raise ParseError from e - meta = request.data.get("meta", {}) + meta = request.data.get("meta") or {} vc_data = meta.get("viewer_context") vc_scope: contextlib.AbstractContextManager[None] = contextlib.nullcontext() if vc_data: From d1e6d1af362f558c6fe9c32c37c749a7cbcef99a Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Tue, 7 Apr 2026 12:01:59 -0700 Subject: [PATCH 4/4] fix(hybridcloud): Always set ViewerContext in RPC endpoint The RPC endpoint ran inside ViewerContextMiddleware, which sets a ViewerContext for the transport request itself. When no viewer_context was provided in meta, nullcontext() left the middleware's context active, leaking incorrect identity to the RPC handler. Always enter viewer_context_scope with either the caller's deserialized context or an empty ViewerContext(actor_type=UNKNOWN) to override the middleware's context. Co-Authored-By: Claude Opus 4.6 --- src/sentry/api/endpoints/internal/rpc.py | 8 +++----- tests/sentry/api/endpoints/test_rpc.py | 10 +++++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/sentry/api/endpoints/internal/rpc.py b/src/sentry/api/endpoints/internal/rpc.py index 5b9eced6066ce0..aadac806d60d79 100644 --- a/src/sentry/api/endpoints/internal/rpc.py +++ b/src/sentry/api/endpoints/internal/rpc.py @@ -1,5 +1,3 @@ -import contextlib - import pydantic import sentry_sdk from rest_framework import status @@ -66,16 +64,16 @@ def post(self, request: Request, service_name: str, method_name: str) -> Respons meta = request.data.get("meta") or {} vc_data = meta.get("viewer_context") - vc_scope: contextlib.AbstractContextManager[None] = contextlib.nullcontext() + vc = ViewerContext() if vc_data: try: - vc_scope = viewer_context_scope(ViewerContext.deserialize(vc_data)) + vc = ViewerContext.deserialize(vc_data) except Exception as e: sentry_sdk.capture_exception() raise ParseError from e try: - with vc_scope, auth_context.applied_to_request(request): + with viewer_context_scope(vc), auth_context.applied_to_request(request): result = dispatch_to_local_service(service_name, method_name, arguments) except RpcValidationException as e: return Response( diff --git a/tests/sentry/api/endpoints/test_rpc.py b/tests/sentry/api/endpoints/test_rpc.py index 3edbd2e87f4fba..2890a5350f6b23 100644 --- a/tests/sentry/api/endpoints/test_rpc.py +++ b/tests/sentry/api/endpoints/test_rpc.py @@ -184,8 +184,8 @@ def capturing_dispatch(*args, **kwargs): assert ctx.user_id == 42 assert ctx.actor_type == ActorType.USER - def test_viewer_context_none_when_meta_empty(self) -> None: - """No ViewerContext set when meta is empty.""" + def test_viewer_context_unknown_when_meta_empty(self) -> None: + """Empty ViewerContext with UNKNOWN actor type when meta has no viewer_context.""" organization = self.create_organization() captured_contexts: list[ViewerContext | None] = [] @@ -208,7 +208,11 @@ def capturing_dispatch(*args, **kwargs): assert response.status_code == 200 assert len(captured_contexts) == 1 - assert captured_contexts[0] is None + ctx = captured_contexts[0] + assert ctx is not None + assert ctx.user_id is None + assert ctx.organization_id is None + assert ctx.actor_type == ActorType.UNKNOWN def test_viewer_context_roundtrip_through_meta(self) -> None: """ViewerContext set on the sending side arrives on the receiving side."""