Skip to content

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

Merged
eli-r-ph merged 1 commit into
v1from
v1-capture-redirects
Oct 20, 2026
Merged

eli-r-ph merged 1 commit into
v1from
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. The v1 send path followed any redirect, so a 307 or 308 from the ingestion host could make the SDK resend event data to another host or over plain HTTP. The 7.x async client had a guard for this, which was lost when v0 was removed. This PR restores it for both clients.

Changes

  • Capture follows a 307 or 308 only to the origin of host, at most 5 times, with the same body and headers.
  • Any other redirect fails the batch and reaches on_error with the redirect status.
  • Migration guide and changeset updated.

💚 How did you test it?

  • Same-origin redirects are followed with an identical body.
  • Redirects to another host, port or scheme, or a 302, send only one request.
  • A redirect loop stops after 6 requests.
  • Removing the origin check fails 5 tests.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Cursor (Claude) under the direction of the assignee. Turning redirects off entirely was rejected because it would break same-origin proxies.

🤖 Generated with Claude Code

@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 3 times, most recently from 6eb24c5 to da670e8 Compare October 8, 2026 16:25
@eli-r-ph
eli-r-ph force-pushed the v1-sdk-identity branch 2 times, most recently from a1ba868 to 6e21be6 Compare October 8, 2026 18:13
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from da670e8 to 638eb11 Compare October 8, 2026 18:13
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from 638eb11 to 5e6c45a Compare October 8, 2026 21:12
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch 2 times, most recently from 19064bc to bcd36eb Compare October 8, 2026 23:40
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch 2 times, most recently from be748d9 to ea73def Compare October 10, 2026 03:19
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from ea73def to f7f2b74 Compare October 10, 2026 23:35
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch from f7f2b74 to c107c49 Compare October 11, 2026 20:17
@eli-r-ph eli-r-ph changed the title fix!: follow capture redirects only to the configured origin fix!: follow capture redirects only to the same origin Oct 13, 2026
@eli-r-ph
eli-r-ph force-pushed the v1-capture-redirects branch 2 times, most recently from 6e37d49 to 11143ad Compare October 14, 2026 03:48

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-assisted review.

The shared sender now restricts capture redirect replay to the configured origin across both clients and capture lanes. Current CI and compliance passed, and the major changeset documents the intentional restriction. Two optional comments cover IDNA hostname equivalence and redirect logging.

Comment thread posthog/capture_send.py
Comment thread posthog/capture_send.py
Base automatically changed from v1-sdk-identity to v1 October 20, 2026 17:02
_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 merged commit 2343064 into v1 Oct 20, 2026
40 checks passed
@eli-r-ph
eli-r-ph deleted the v1-capture-redirects branch October 20, 2026 17:03
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.

2 participants