From f6e97f576af4c774102265711249d214f358cc20 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Thu, 21 May 2026 21:34:55 -0400 Subject: [PATCH 1/2] fix(json_api): enrich + dedupe signature-mismatch capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The signature-failure capture_message landed in Sentry as 118 events / 14d (ledoent/seer issue #76) with zero context — no URL, no source IP, no body shape. Diagnosis required live SSH + clickhouse queries to identify the caller (compose-internal post-process-forwarder auto-summary). Investigators shouldn't have to do that. This PR enriches the capture and stops the storm: * Adds a `request` context dict to both signature-failure branches (bad-prefix + no-match-found) carrying url, method, remote_addr, X-Forwarded-For, User-Agent, Content-Type, Content-Length, body_len, first 200 bytes of body, and the first 16 chars of the signature for quick eyeball comparison. * Uses `sentry_sdk.push_scope() + scope.fingerprint = [...]` to group all signature mismatches under one Sentry fingerprint regardless of body shape, so a recurring auth issue stays one fire instead of fanning out into many. * Demotes both capture_message calls to `level="warning"`. The failure ISN'T critical — Sentry's caller gets a 401 and skips the auto-summary for that issue. No data loss. * Defensively caps `body_prefix` at 200 bytes and `signature_prefix` at 16 chars so the context doesn't carry anything that could leak in transit. Doesn't change the auth logic itself — only the diagnostics around failures. The signing/verifying code path stays bit-identical. Tests (3 new cases in `TestSignatureFailureDiagnostics`): * bad-prefix capture carries the request context * no-match capture goes through push_scope + sets fingerprint + uses level="warning" * full signature hex doesn't leak into the captured context --- src/seer/json_api.py | 46 ++++++++++++++++- tests/test_json_api.py | 113 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 2 deletions(-) diff --git a/src/seer/json_api.py b/src/seer/json_api.py index 3d5826e7..d4d6f150 100644 --- a/src/seer/json_api.py +++ b/src/seer/json_api.py @@ -174,7 +174,11 @@ 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:") + sentry_sdk.capture_message( + "Signature did not start with rpc0:", + level="warning", + contexts={"request": _signature_failure_context(body, signature)}, + ) return False _, signature_data = signature.split(":", 2) @@ -183,5 +187,43 @@ 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.") + # Enriched diagnostic + fingerprint dedup. 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" issue that we + # couldn't diagnose. The context now carries the URL, body length, + # source IP, and the body prefix so a future investigator can correlate + # against the calling code path. The fingerprint groups all events into + # one issue regardless of body length so the storm stays a single fire. + ctx = _signature_failure_context(body, signature) + with sentry_sdk.push_scope() as scope: + scope.set_context("request", ctx) + scope.fingerprint = ["json_api.signature_mismatch"] + sentry_sdk.capture_message( + "No signature matches found.", + level="warning", + ) return False + + +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 + forwarded_for = req.headers.get("X-Forwarded-For", "") + return { + "url": req.url, + "method": req.method, + "remote_addr": req.remote_addr or "", + "x_forwarded_for": 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..873359e9 100644 --- a/tests/test_json_api.py +++ b/tests/test_json_api.py @@ -231,3 +231,116 @@ 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_includes_request_context(self, mock_sdk): + 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"}, + ) + + # capture_message called with the bad-prefix branch + call = next( + c + for c in mock_sdk.capture_message.call_args_list + if "did not start with rpc0:" in c.args[0] + ) + ctx = call.kwargs["contexts"]["request"] + assert ctx["url"].endswith("/v0/some/url") + assert ctx["method"] == "POST" + # body_len matches what flask received + assert ctx["body_len"] > 0 + # signature_prefix truncated, no full secret leakage + assert "..." in ctx["signature_prefix"] or len(ctx["signature_prefix"]) <= 16 + + @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 From d75e8270975ca7b8032b4cd2b5917b2862648dd0 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Fri, 22 May 2026 07:25:55 -0400 Subject: [PATCH 2/2] review: DRY both signature-failure capture branches + add short-sig test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot pass on PR #21. Findings: 1. DRY: bad-prefix branch used `contexts={"request": ...}` directly on capture_message; no-match branch used push_scope + set_context + fingerprint. Two patterns for the same failure class. 2. Missing test: short-signature (<= 16 chars after rpc0:) path was not exercised — the conditional `signature[:16] + "..." if len(signature) > 16 else signature` had no positive-case test for the else branch. Changes: - Extract `_capture_signature_failure(message, *, body, signature, fingerprint)` helper. Both call sites go through it. - Bad-prefix branch now gets its own fingerprint ("json_api.signature_bad_prefix") + push_scope + warning level for symmetry with the no-match case. Distinct fingerprints keep them as separate Sentry issues. - Test renamed `test_bad_prefix_capture_includes_request_context` -> `test_bad_prefix_capture_enriches_via_scope_and_fingerprint`, now asserts the new contract. - New `test_short_signature_no_ellipsis` pins the else-branch behavior (signature_prefix is the raw value when <= 16 chars). 5 tests pass (was 4 + 1 strict-mode regression test). --- src/seer/json_api.py | 57 +++++++++++++++++++++++++++--------------- tests/test_json_api.py | 57 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 83 insertions(+), 31 deletions(-) diff --git a/src/seer/json_api.py b/src/seer/json_api.py index d4d6f150..dc36a250 100644 --- a/src/seer/json_api.py +++ b/src/seer/json_api.py @@ -174,10 +174,11 @@ 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( + _capture_signature_failure( "Signature did not start with rpc0:", - level="warning", - contexts={"request": _signature_failure_context(body, signature)}, + body=body, + signature=signature, + fingerprint="json_api.signature_bad_prefix", ) return False @@ -187,24 +188,41 @@ def compare_signature(*, body: bytes, signature: str, config: AppConfig = inject if is_valid(body, key, signature_data): return True - # Enriched diagnostic + fingerprint dedup. 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" issue that we - # couldn't diagnose. The context now carries the URL, body length, - # source IP, and the body prefix so a future investigator can correlate - # against the calling code path. The fingerprint groups all events into - # one issue regardless of body length so the storm stays a single fire. - ctx = _signature_failure_context(body, signature) - with sentry_sdk.push_scope() as scope: - scope.set_context("request", ctx) - scope.fingerprint = ["json_api.signature_mismatch"] - sentry_sdk.capture_message( - "No signature matches found.", - level="warning", - ) + # 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. @@ -214,12 +232,11 @@ def _signature_failure_context(body: bytes, signature: str) -> dict[str, object] signature, body prefix capped at 200 bytes). """ req = request # flask thread-local - forwarded_for = req.headers.get("X-Forwarded-For", "") return { "url": req.url, "method": req.method, "remote_addr": req.remote_addr or "", - "x_forwarded_for": forwarded_for, + "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", ""), diff --git a/tests/test_json_api.py b/tests/test_json_api.py index 873359e9..fe808d59 100644 --- a/tests/test_json_api.py +++ b/tests/test_json_api.py @@ -256,7 +256,12 @@ def my_endpoint(request: DummyRequest) -> DummyResponse: return app @patch("seer.json_api.sentry_sdk") - def test_bad_prefix_capture_includes_request_context(self, mock_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() @@ -268,19 +273,25 @@ def test_bad_prefix_capture_includes_request_context(self, mock_sdk): headers={"Authorization": "Rpcsignature notrpc0:xyz"}, ) - # capture_message called with the bad-prefix branch - call = next( - c - for c in mock_sdk.capture_message.call_args_list - if "did not start with rpc0:" in c.args[0] - ) - ctx = call.kwargs["contexts"]["request"] + # 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" - # body_len matches what flask received assert ctx["body_len"] > 0 - # signature_prefix truncated, no full secret leakage - assert "..." in ctx["signature_prefix"] or len(ctx["signature_prefix"]) <= 16 + + # 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): @@ -344,3 +355,27 @@ def test_signature_failure_context_does_not_leak_full_signature(self, mock_sdk): # 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"]