fix(json_api): enrich + dedupe signature-mismatch capture#21
Merged
dnplkndll merged 2 commits intoMay 22, 2026
Merged
Conversation
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
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ledoent/seer Sentry issue #76 ("No signature matches found") fired 118 events / 14d with zero context — no URL, no source IP, no body shape, no signature hint. Diagnosing required live SSH + clickhouse queries to identify the caller (compose-internal post-process-forwarder-errors at 172.18.0.56, hitting /v1/automation/summarize/issue with empty UA).
Future investigators shouldn't have to do that. This PR:
Enriches the capture_message with a `request` context dict (url, method, remote_addr, X-Forwarded-For, User-Agent, Content-Type, Content-Length, body_len, first 200 bytes of body, first 16 chars of signature for eyeball compare).
Dedupes via `scope.fingerprint = ["json_api.signature_mismatch"]` — all signature failures group into one Sentry issue regardless of body shape, so a recurring auth bug stays one fire instead of fanning out.
Demotes both capture_message calls to `level="warning"`. The failure isn't critical — Sentry's caller gets a 401 and skips that auto-summary. No data loss.
Defensive caps — `body_prefix` at 200 bytes, `signature_prefix` at 16 chars. Nothing leaks.
Doesn't change
The auth logic itself is bit-identical. Only diagnostics around failures change.
Tests (3 new cases in `TestSignatureFailureDiagnostics`)
All existing signature tests stay green.
Out of scope (separate investigation)
This PR doesn't fix why the post-process-forwarder occasionally produces an unsignable request (8/day average, sporadic clusters). With the enriched context, the next investigator can correlate body_len + body_prefix across events to pin the root cause — likely urllib3 connection-pool reuse or a sporadic body-mutation. The single-fingerprint dedup means the storm collapses to one fire so the noise stops in production immediately.