Skip to content

fix!: follow capture redirects only to the configured origin - #1035

Open
eli-r-ph wants to merge 1 commit into
v1-sdk-identityfrom
v1-capture-redirects
Open

eli-r-ph wants to merge 1 commit into
v1-sdk-identityfrom
v1-capture-redirects

Conversation

@eli-r-ph

@eli-r-ph eli-r-ph commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Stacked on #1034. Part of the 8.0 series (checklist in #1016).

_post_v1 used the default requests redirect handling. On a cross-origin 307 or 308 it resent the event batch to the new origin. requests drops Authorization when the host changes, but sends the body. So a response from the ingestion host could make the SDK POST event data to another host, a loopback address, or over plain HTTP.

The 7.x async client blocked this for v0 (#899, #941): it followed only same-origin 307/308, at most 5 times. #1017 made v1 the async default and #1018 removed v0, so async lost that guard. The sync client never had it.

Changes:

  • _post_v1 sends with allow_redirects=False. It follows a 307 or 308 only to the origin of host (scheme, host and port), at most 5 times, and keeps the body and headers. The target keeps a path prefix on host, as fix: preserve redirect URLs with async capture host prefixes #941 did.
  • Any other redirect returns the redirect response. _send_v1_batch already treats 3xx as terminal, so the batch fails at once and reaches on_error with the redirect status.
  • The sync and async clients share _post_v1, so both are covered.
  • Migration guide: a checklist item and a note under Endpoints. Changeset (major).
  • typings/requests: Session.post accepts allow_redirects.

Addresses the veria-ai findings on #1017 and #1018.

💚 How did you test it?

  • test_follows_same_origin_redirect_with_same_body: relative Location, a host with a path prefix, and an explicit default port. Each one checks the URL that is followed, an identical body and headers, and allow_redirects=False on every hop.
  • test_does_not_follow_other_redirects: another host, loopback over HTTP, HTTPS to HTTP on the same host, another port, a missing Location, and a 302. Each one sends exactly one request.
  • test_stops_after_max_redirects: a redirect loop stops after 6 requests.
  • test_terminal_status_raises_immediately covers 307 and 308: one attempt, then CaptureError with that status.
  • With the origin check removed, 5 of these tests fail.
  • ruff, mypy (baseline filter), make public_api_check, python -W error -c "import posthog", and the full pytest suite pass locally.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Cursor agent (Claude). The author chose to port the existing async same-origin guard into the shared v1 transport, for both clients, in 8.0 only. Disabling redirects entirely was rejected, because it would break proxies that redirect on the same origin. Other SDKs' redirect handling is tracked separately.

@eli-r-ph
eli-r-ph requested a review from a team as a code owner October 7, 2026 22:47
@eli-r-ph eli-r-ph self-assigned this Oct 7, 2026
@eli-r-ph eli-r-ph mentioned this pull request Oct 7, 2026
3 of 20 tasks
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from 9f403f2 to 530f831 Compare October 8, 2026 00:08
_post_v1 disables automatic redirects and resends the batch only on a 307 or 308 to the origin of host, at most 5 times. Any other redirect returns as a terminal failure. Covers the sync client and the async client, which lost its v0 same-origin guard when v1 became the only path.
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from 530f831 to 6eb24c5 Compare October 8, 2026 01:20

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant