Conversation
A carriage return or newline is valid ASCII, so it passed the existing character guard, but requests still rejects it as an invalid header value when the key is validated. That exception is a RequestException subclass, so it was swallowed by the generic network-error handler and reported as a transient failure instead of explaining the malformed key. Generated-By: PostHog Desktop Task-Id: dddbb8df-805b-4cb2-8d5e-53a568522829
Gilbert09
added
the
stamphog
Talyn App
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
Trivial, well-tested validation fix by an owning-team author with strong familiarity; no risky territory touched.
- Author wrote 100% of the modified lines and has 6 merged PRs in these paths (familiarity STRONG).
- copilot-pull-request-reviewer[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 6L, 1F substantive, 19L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1a-trivial (19L, 2F, single-area, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 3006b51 · reviewed head 3006b51 |
🤖 CI report
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to Clerk keys containing pasted line breaks are rejected before a request, preventing invalid headers from being reported as transient connectivity failures. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Problem
Pasting a secret key that contains a carriage return or newline (for example, a whole
.envsnippet pasted into the secret key field instead of just the key) crashes credential validation for the Clerk data warehouse source, reported via error tracking.A return character is valid ASCII, so it passes the existing
isascii()guard, butrequestsstill refuses to send it as a header value (InvalidHeader). That exception is aRequestExceptionsubclass, so it was swallowed by the generic network-error handler and reported back as "Couldn't reach Clerk to validate your secret key. Please try again in a moment." — a message that tells the user to retry a key that will never validate.Changes
validate_credentialsnow rejects a secret key containing\ror\nbefore making any request, alongside the existing non-ASCII check, and explains it with the same "unsupported character" message.How did you test this code?
Added
test_key_with_return_character_is_rejected_before_any_request, which asserts the key is rejected before any request is sent and the session mock is never called — the same pattern as the existing non-ASCII test.Ran
pytest products/warehouse_sources/backend/temporal/data_imports/sources/clerk/locally: 174 passed. Not run: the full CI matrix (relies on this PR's checks).Release status
Automatic notifications
Docs update
None — this is an internal validation fix with no documented behavior change.
🤖 Agent context
Autonomy: Fully autonomous
Agent: Claude Code, Sonnet 5
Triaged from a live error tracking issue for the Clerk data warehouse source. Confirmed the failure originates in
validate_credentials(not just referenced in serialized context), read the requests library's header-validation regex to identify the exact class of character it rejects, and scoped the fix to that guard rather than wideningNonRetryableErrors(this needed a code fix, not a retry-policy change, since retrying the same malformed key would never succeed and the existing UX message actively told users to retry). Searched open PRs (by keyword and by this account's own PR list) for a duplicate; none matched.🤖 Generated with Claude Code