Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions src/seer/json_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
}
148 changes: 148 additions & 0 deletions tests/test_json_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Loading