feat(exceptions): opt-in Ed25519 signing of $exception events#657
feat(exceptions): opt-in Ed25519 signing of $exception events#657Gilbert09 wants to merge 2 commits into
Conversation
Backend services can configure an Ed25519 private key; the SDK then signs every captured $exception over a canonical, length-prefixed projection of its $exception_list and attaches $exception_signature / _key_id / _version. PostHog error tracking verifies this against the project's registered public key to prove the exception genuinely came from the customer's backend (not forged through the public ingest key). Signing happens after before_send so it covers the final content. Crypto is an optional [exception-signing] extra to keep the base SDK lean.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
posthog/client.py:384-390
**Silent misconfiguration when key is omitted**
If a user sets `enable_exception_signing=True` but forgets (or fails to inject) `exception_signing_private_key`, the guard `if enable_exception_signing and exception_signing_private_key:` silently skips signer creation — no warning is ever logged, `_exception_signer` stays `None`, and every `$exception` event is sent unsigned. For a security feature the user deliberately opted into, this silent no-op is indistinguishable from normal operation and would be very difficult to detect in production.
### Issue 2 of 3
posthog/test/test_exception_signing.py:101-104
**Non-parametrised tests**
`test_tolerates_missing_and_malformed` packs three independent inputs (`[]`, `None`, `[{}]`) into a single assertion chain — a failure reveals only the first failing case. `test_sign_event_attaches_props_only_for_exceptions` similarly tests two event types together. Both should be written as parametrised tests so each case fails independently and the coverage is explicit.
### Issue 3 of 3
posthog/exception_signing.py:63-64
The explicit `str(lineno)` conversion is superfluous: `_lp` already calls `str(value).encode()` on any non-`None` input, so `_lp(lineno if lineno is None else str(lineno))` produces exactly the same bytes as `_lp(lineno)`. Removing the outer conversion is cleaner and consistent with how the other fields are encoded.
```suggestion
lineno = frame.get("lineno")
out += _lp(lineno)
```
Reviews (1): Last reviewed commit: "feat(exceptions): opt-in Ed25519 signing..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if enable_exception_signing and exception_signing_private_key: | ||
| from posthog.exception_signing import make_signer | ||
|
|
||
| try: | ||
| self._exception_signer = make_signer(exception_signing_private_key) | ||
| except Exception as e: | ||
| self.log.error("Failed to initialise exception signing: %s", e) |
There was a problem hiding this comment.
If a user sets enable_exception_signing=True but forgets (or fails to inject) exception_signing_private_key, the guard if enable_exception_signing and exception_signing_private_key: silently skips signer creation — no warning is ever logged, _exception_signer stays None, and every $exception event is sent unsigned. For a security feature the user deliberately opted into, this silent no-op is indistinguishable from normal operation and would be very difficult to detect in production.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/client.py
Line: 384-390
Comment:
**Silent misconfiguration when key is omitted**
If a user sets `enable_exception_signing=True` but forgets (or fails to inject) `exception_signing_private_key`, the guard `if enable_exception_signing and exception_signing_private_key:` silently skips signer creation — no warning is ever logged, `_exception_signer` stays `None`, and every `$exception` event is sent unsigned. For a security feature the user deliberately opted into, this silent no-op is indistinguishable from normal operation and would be very difficult to detect in production.
How can I resolve this? If you propose a fix, please make it concise.| def test_tolerates_missing_and_malformed(self): | ||
| self.assertTrue(build_canonical([]).startswith(b"PHEXC1\n")) | ||
| self.assertTrue(build_canonical(None).startswith(b"PHEXC1\n")) | ||
| self.assertTrue(build_canonical([{}]).startswith(b"PHEXC1\n")) |
There was a problem hiding this comment.
test_tolerates_missing_and_malformed packs three independent inputs ([], None, [{}]) into a single assertion chain — a failure reveals only the first failing case. test_sign_event_attaches_props_only_for_exceptions similarly tests two event types together. Both should be written as parametrised tests so each case fails independently and the coverage is explicit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/test_exception_signing.py
Line: 101-104
Comment:
**Non-parametrised tests**
`test_tolerates_missing_and_malformed` packs three independent inputs (`[]`, `None`, `[{}]`) into a single assertion chain — a failure reveals only the first failing case. `test_sign_event_attaches_props_only_for_exceptions` similarly tests two event types together. Both should be written as parametrised tests so each case fails independently and the coverage is explicit.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| lineno = frame.get("lineno") | ||
| out += _lp(lineno if lineno is None else str(lineno)) |
There was a problem hiding this comment.
str(lineno) conversion is superfluous: _lp already calls str(value).encode() on any non-None input, so _lp(lineno if lineno is None else str(lineno)) produces exactly the same bytes as _lp(lineno). Removing the outer conversion is cleaner and consistent with how the other fields are encoded.
| lineno = frame.get("lineno") | |
| out += _lp(lineno if lineno is None else str(lineno)) | |
| lineno = frame.get("lineno") | |
| out += _lp(lineno) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/exception_signing.py
Line: 63-64
Comment:
The explicit `str(lineno)` conversion is superfluous: `_lp` already calls `str(value).encode()` on any non-`None` input, so `_lp(lineno if lineno is None else str(lineno))` produces exactly the same bytes as `_lp(lineno)`. Removing the outer conversion is cleaner and consistent with how the other fields are encoded.
```suggestion
lineno = frame.get("lineno")
out += _lp(lineno)
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
posthog-python Compliance ReportDate: 2026-06-10 18:01:52 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
- Log a warning when enable_exception_signing=True but no key is provided, instead of silently sending unsigned (review). - Apply ruff format (CI code-quality). - Parametrise/split the two multi-case tests so each case fails independently.
1 participant
Problem
Exceptions captured by a backend can be forwarded to systems that act on them (e.g. an internal app that auto-triages data-import failures with an unattended agent). Because the PostHog ingest key is public, anyone can capture a forged
$exceptionthat's indistinguishable from a genuine one — there's no way for a consumer to prove an exception actually came from the customer's backend. That's an injection vector for anything downstream that takes the exception content as input.Changes
Adds opt-in Ed25519 signing of
$exceptionevents. A backend service configures an Ed25519 private key; the SDK then signs every captured exception over a canonical projection of its$exception_listand attaches:$exception_signature— base64 Ed25519 signature$exception_signature_key_id— short key fingerprint (base64url(sha256(raw_pubkey))[:16]) for lookup/rotation$exception_signature_versionPostHog error-tracking ingestion verifies this against the project's registered public key and stamps a trusted
$exception_verifiedflag (separate changes in PostHog/posthog). PostHog never holds a key that can forge.posthog/exception_signing.py: a deterministic length-prefixed canonical encoding of$exception_list(type + message + each frame's function/filename/lineno/module — excludes everything ingestion mutates and anything float-valued), Ed25519 signing, and key-id derivation. JSON canonicalisation was deliberately avoided in favour of explicit length-prefixing to keep the bytes identical across the SDK and the Rust verifier.enable_exception_signing+exception_signing_private_key(PEM), wired throughClient.__init__, the module proxies, andsetup()._enqueueafterbefore_send, so it covers the final sent content and abefore_sendcallback can't strip it. Failures leave the event unsigned, never dropped.cryptography) is an optional[exception-signing]extra so the base SDK stays lean.Backend use only — never ship a private key in a browser/mobile app.
How tested
Agent-assisted.
pytest posthog/test/test_exception_signing.py(14 tests) + the existing before_send/exception-capture suites pass. Includes a cross-language parity vector — a fixed keypair signing a fixed exception list, asserting the exact canonical bytes + signature — that the Rust verifier (PostHog/posthog) reproduces, guarding against any drift between the two implementations. Verified sign→verify round-trips with a generated keypair; confirmed signing happens after before_send.