Skip to content

chore(data-warehouse): allow incremental_fields during read-only impersonation#60423

Merged
sakce merged 2 commits into
masterfrom
posthog-code/allowlist-incremental-fields-during-impersonation
May 28, 2026
Merged

chore(data-warehouse): allow incremental_fields during read-only impersonation#60423
sakce merged 2 commits into
masterfrom
posthog-code/allowlist-incremental-fields-during-impersonation

Conversation

@sakce
Copy link
Copy Markdown
Contributor

@sakce sakce commented May 28, 2026

Problem

When support engineers impersonate a customer in read-only mode, the
POST /api/environments/:project/external_data_schemas/:id/incremental_fields/
endpoint is blocked by ImpersonationReadOnlyMiddleware. The block makes it
impossible to inspect what the UI would receive when picking incremental sync
fields for a customer — which is exactly the kind of question support sessions
are for.

The endpoint is POST but doesn't write anything PostHog-side: it loads the
schema, validates the source's credentials, calls new_source.get_schemas()
against the customer's external source, and returns metadata (available
incremental fields, supported sync types, columns).

Changes

  • Add the external_data_schemas/<id>/incremental_fields/ regex to
    READ_ONLY_IMPERSONATION_ALLOWLISTED_PATHS in posthog/middleware.py,
    alongside the other "POST but read-only" entries.
  • Add a parameterized test case in TestImpersonationReadOnlyMiddleware that
    asserts the path is not blocked with impersonation_read_only during a
    read-only impersonation session.

The endpoint does fire an outbound credential-validation call to the
customer's source; that's still "read" from PostHog's perspective, but worth
knowing if rate limits on the customer's side are a concern during a support
session.

How did you test this code?

I'm an agent. I added an automated test (the new parameterized row in
test_read_only_impersonation_allows_allowlisted_post) and verified the new
regex pattern matches the intended URL shape (with [0-9]+ and @current
project segments, with and without a trailing slash) and rejects unrelated
paths in a local Python check. I did not run the test suite locally (no
Python venv available in the sandbox) — relying on CI to run it.

Publish to changelog?

no

🤖 Agent context

Authored by Claude Opus 4.7 via PostHog Code. The user asked whether
incremental_fields needed to be gated by the impersonation block or could
be allowlisted; I confirmed the endpoint body has no PostHog-side writes
(only validates external credentials and lists schemas) so it matches the
existing "POST but read-only" allowlist pattern used for query/,
persons/batch_by_distinct_ids/, and error_tracking/stack_frames/batch_get/.
The user then asked me to ship the change.

Decisions:

  • Allowlist via a regex matching both environments and projects variants
    of the URL, mirroring neighbouring entries.
  • Add a parameterized test row rather than a new test method to keep the test
    surface tight.
  • Did not change scope_object_write_actions on the viewset — that's a
    permissions/scope concern, separate from the impersonation block, and out
    of scope for this change.

…rsonation

The external_data_schemas `incremental_fields` action is a POST but performs
no PostHog-side mutations — it validates the source's credentials and lists
schemas against the customer's external source to return metadata about
available incremental fields and columns.

Allowlist its path in `READ_ONLY_IMPERSONATION_ALLOWLISTED_PATHS`, alongside
the other "POST but read-only" entries (query, batch_by_distinct_ids,
error_tracking/stack_frames/batch_get), so support engineers can inspect
the response while impersonating a customer read-only.

Generated-By: PostHog Code
Task-Id: 2c660a12-b953-466c-8f9d-c9f11b912554
@sakce sakce added the stamphog Request AI review from stamphog label May 28, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Reviews (1): Last reviewed commit: "chore(data-warehouse): allow incremental..." | Re-trigger Greptile

github-actions[bot]
github-actions Bot previously approved these changes May 28, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Trivial allowlist addition for a read-only POST endpoint; pattern follows established conventions and a test is included.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Combine two series with a formula and verify computed total (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this comment? Help fix flakies and failures and it'll disappear!

Collapse the new incremental_fields allowlist regex to a single line so
`ruff format` is happy. Ruff prefers a single-line `re.compile(...)` for
regex literals at this width, matching the surrounding entries.

Generated-By: PostHog Code
Task-Id: 2c660a12-b953-466c-8f9d-c9f11b912554
@github-actions github-actions Bot dismissed their stale review May 28, 2026 12:21

New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Straightforward allowlist addition following the existing pattern — a read-only POST endpoint added to the impersonation allowlist with a corresponding test.

@sakce sakce enabled auto-merge (squash) May 28, 2026 13:14
@sakce sakce merged commit dc62d18 into master May 28, 2026
225 of 227 checks passed
@sakce sakce deleted the posthog-code/allowlist-incremental-fields-during-impersonation branch May 28, 2026 13:23
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented May 28, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-05-28 13:58 UTC Run
prod-us ✅ Deployed 2026-05-28 14:15 UTC Run
prod-eu ✅ Deployed 2026-05-28 14:14 UTC Run

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

Labels

stamphog Request AI review from stamphog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant