diff --git a/src/seer/json_api.py b/src/seer/json_api.py index 3d5826e7..dc36a250 100644 --- a/src/seer/json_api.py +++ b/src/seer/json_api.py @@ -174,7 +174,12 @@ def compare_signature(*, body: bytes, signature: str, config: AppConfig = inject secrets = config.JSON_API_SHARED_SECRETS if not signature.startswith("rpc0:"): - sentry_sdk.capture_message("Signature did not start with rpc0:") + _capture_signature_failure( + "Signature did not start with rpc0:", + body=body, + signature=signature, + fingerprint="json_api.signature_bad_prefix", + ) return False _, signature_data = signature.split(":", 2) @@ -183,5 +188,59 @@ def compare_signature(*, body: bytes, signature: str, config: AppConfig = inject if is_valid(body, key, signature_data): return True - sentry_sdk.capture_message("No signature matches found.") + # The compose-internal caller (post-process-forwarder auto-summary) + # used to fire this 8×/day with no context, generating a noisy + # "No signature matches found" Sentry issue we couldn't diagnose. The + # enriched request context now carries url + source IP + body shape + + # signature prefix so a future investigator can correlate against the + # calling code path. Distinct fingerprints keep the two failure modes + # (bad-prefix vs no-match) as separate Sentry issues but each dedups + # to one event regardless of body shape — a recurring auth bug stays + # one fire instead of fanning out. + _capture_signature_failure( + "No signature matches found.", + body=body, + signature=signature, + fingerprint="json_api.signature_mismatch", + ) return False + + +def _capture_signature_failure( + message: str, *, body: bytes, signature: str, fingerprint: str +) -> None: + """Send a warning-level signature-failure capture with enriched request + context, scoped to the given fingerprint for dedup. + + Both failure modes (bad-prefix + no-match) funnel through here so they + share the diagnostic context shape and dedup semantics. The caller + supplies a distinct fingerprint per failure mode so they surface as + separate Sentry issues even though the wire format is identical. + """ + with sentry_sdk.push_scope() as scope: + scope.set_context("request", _signature_failure_context(body, signature)) + scope.fingerprint = [fingerprint] + sentry_sdk.capture_message(message, level="warning") + + +def _signature_failure_context(body: bytes, signature: str) -> dict[str, object]: + """Build the diagnostic context dict attached to signature-failure events. + + Captures everything the next investigator will want to correlate against + the calling code (url, source IP, body length + prefix, sig prefix) and + nothing that could exfiltrate secrets (only the first 16 chars of the + signature, body prefix capped at 200 bytes). + """ + req = request # flask thread-local + return { + "url": req.url, + "method": req.method, + "remote_addr": req.remote_addr or "", + "x_forwarded_for": req.headers.get("X-Forwarded-For", ""), + "user_agent": req.headers.get("User-Agent", ""), + "content_type": req.headers.get("Content-Type", ""), + "content_length": req.headers.get("Content-Length", ""), + "body_len": len(body), + "body_prefix": body[:200].decode("utf-8", errors="replace"), + "signature_prefix": signature[:16] + "..." if len(signature) > 16 else signature, + } diff --git a/tests/test_json_api.py b/tests/test_json_api.py index 06b4375f..fe808d59 100644 --- a/tests/test_json_api.py +++ b/tests/test_json_api.py @@ -231,3 +231,151 @@ def my_endpoint(request: DummyRequest) -> DummyResponse: ).hexdigest() headers["Authorization"] = f"Rpcsignature rpc0:{signature}" assert changed.to_value(200) + + +class TestSignatureFailureDiagnostics: + """Pin the diagnostic context that lands on a failed-signature event. + + Background: ledoent/seer Sentry issue #76 fired 118 events / 14d as + "No signature matches found" with zero context — culprit URL, source + IP, and body shape were all invisible. Diagnosis took live SSH + + clickhouse queries to identify the caller (compose-internal + post-process-forwarder auto-summary). Pinning the enriched context + + fingerprint here so future investigators can read it off the event. + """ + + def _make_app(self): + app = Flask(__name__) + blueprint = Blueprint("blueprint", __name__) + + @json_api(blueprint, "/v0/some/url") + def my_endpoint(request: DummyRequest) -> DummyResponse: + return DummyResponse(blah="ok") + + app.register_blueprint(blueprint) + return app + + @patch("seer.json_api.sentry_sdk") + def test_bad_prefix_capture_enriches_via_scope_and_fingerprint(self, mock_sdk): + """Bad-prefix branch funnels through the same `_capture_signature_failure` + helper as the no-match branch — pin that it gets the enriched + request context AND its own distinct fingerprint so the two + failure modes surface as separate Sentry issues. + """ + app = self._make_app() + test_client = app.test_client() + + with Module() as injector: + injector.get(AppConfig).JSON_API_SHARED_SECRETS = ["secret-one"] + test_client.post( + "/v0/some/url", + json={"thing": "x", "b": 1}, + headers={"Authorization": "Rpcsignature notrpc0:xyz"}, + ) + + # push_scope was used; context carries the enriched request dict. + scope = mock_sdk.push_scope.return_value.__enter__.return_value + ctx_call = next(c for c in scope.set_context.call_args_list if c.args[0] == "request") + ctx = ctx_call.args[1] + assert ctx["url"].endswith("/v0/some/url") + assert ctx["method"] == "POST" + assert ctx["body_len"] > 0 + + # Distinct fingerprint per failure mode (vs the no-match case). + assert scope.fingerprint == ["json_api.signature_bad_prefix"] + + # And the capture_message is at warning level + capture_calls = [ + c + for c in mock_sdk.capture_message.call_args_list + if "did not start with rpc0:" in c.args[0] + ] + assert capture_calls + assert capture_calls[0].kwargs.get("level") == "warning" + + @patch("seer.json_api.sentry_sdk") + def test_no_match_capture_enriches_via_scope_and_fingerprint(self, mock_sdk): + app = self._make_app() + test_client = app.test_client() + + # Build a request with VALID rpc0: prefix but wrong signature hex + with Module() as injector: + injector.get(AppConfig).JSON_API_SHARED_SECRETS = ["secret-one"] + test_client.post( + "/v0/some/url", + json={"thing": "x", "b": 1}, + headers={"Authorization": "Rpcsignature rpc0:deadbeef"}, + ) + + # push_scope was used so subsequent capture_message picks up the + # scope's context + fingerprint + assert mock_sdk.push_scope.called + scope = mock_sdk.push_scope.return_value.__enter__.return_value + + # The set_context call carried the enriched request dict + ctx_call = next(c for c in scope.set_context.call_args_list if c.args[0] == "request") + ctx = ctx_call.args[1] + assert ctx["url"].endswith("/v0/some/url") + assert "body_prefix" in ctx and ctx["body_prefix"] # non-empty + assert ctx["signature_prefix"].startswith("rpc0:deadbeef"[:16]) + + # The fingerprint groups all such failures into one Sentry issue + # regardless of body shape — so a storm stays a single fire. + assert scope.fingerprint == ["json_api.signature_mismatch"] + + # And the capture_message itself is at warning level + capture_calls = [ + c + for c in mock_sdk.capture_message.call_args_list + if "No signature matches found" in c.args[0] + ] + assert capture_calls + assert capture_calls[0].kwargs.get("level") == "warning" + + @patch("seer.json_api.sentry_sdk") + def test_signature_failure_context_does_not_leak_full_signature(self, mock_sdk): + """Context dict caps the signature at 16 chars to avoid leaking + anything sensitive even though the signature itself is a public-ish + hex digest. + """ + full_sig_hex = "deadbeef" * 8 # 64 chars + app = self._make_app() + test_client = app.test_client() + with Module() as injector: + injector.get(AppConfig).JSON_API_SHARED_SECRETS = ["secret-one"] + test_client.post( + "/v0/some/url", + json={"thing": "x", "b": 1}, + headers={"Authorization": f"Rpcsignature rpc0:{full_sig_hex}"}, + ) + + scope = mock_sdk.push_scope.return_value.__enter__.return_value + ctx_call = next(c for c in scope.set_context.call_args_list if c.args[0] == "request") + ctx = ctx_call.args[1] + # Truncated representation only + assert full_sig_hex not in ctx["signature_prefix"] + assert len(ctx["signature_prefix"]) <= 20 + + @patch("seer.json_api.sentry_sdk") + def test_short_signature_no_ellipsis(self, mock_sdk): + """A signature <= 16 chars is short enough that we don't need the + truncation marker. Pin that the `signature_prefix` field returns + the value verbatim in that case. + """ + short_sig_hex = "abc123" # 6 chars after rpc0: + app = self._make_app() + test_client = app.test_client() + with Module() as injector: + injector.get(AppConfig).JSON_API_SHARED_SECRETS = ["secret-one"] + test_client.post( + "/v0/some/url", + json={"thing": "x", "b": 1}, + headers={"Authorization": f"Rpcsignature rpc0:{short_sig_hex}"}, + ) + + scope = mock_sdk.push_scope.return_value.__enter__.return_value + ctx_call = next(c for c in scope.set_context.call_args_list if c.args[0] == "request") + ctx = ctx_call.args[1] + # The full "rpc0:abc123" (11 chars) fits inside the 16-char cap → no ellipsis + assert ctx["signature_prefix"] == f"rpc0:{short_sig_hex}" + assert "..." not in ctx["signature_prefix"]