Skip to content

fix(auth): reject ambiguous conversation widget tokens - #108161

Open
pauldambra wants to merge 1 commit into
posthog/fix-live-stream-membershipfrom
posthog/fix-widget-token-collisions
Open

pauldambra wants to merge 1 commit into
posthog/fix-live-stream-membershipfrom
posthog/fix-widget-token-collisions

Conversation

@pauldambra

Copy link
Copy Markdown
Member

[Robot] Prepared by PostHog Desktop.

Problem

A widget token that matches more than one project causes a server error.

Changes

Reject an ambiguous token with an authentication error.

This fixes existing behavior before the module split in #99248.

How did you test this code?

Parameterized authentication tests cover missing and duplicate tokens. The focused tests ran against both module layouts. The full type check covers the combined fixes.

Release status

  • No feature flag controls this change

Automatic notifications

  • Publish to changelog?

Docs update

No existing document under docs/ covers this validation detail.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Desktop / Codex, GPT-6

Tools: Git, GitHub CLI, hogli, pytest, Ruff, mypy, and CodeRabbit.
Skills: stacking-prs, improving-drf-endpoints, writing-tests, writing-code-comments, writing-user-facing-copy, setting-up-devbox, running-ci-preflight, reviewing-with-coderabbit, writing-pr-descriptions.
Each existing bug has its own layer below the refactor. No matching open fix appeared in the PR search.
The combined CodeRabbit review found two deployment tradeoffs in the membership layer. That PR records the decisions.


Created with PostHog Desktop

@pauldambra pauldambra self-assigned this Sep 29, 2026
@posthog

posthog Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time. Enjoy the moment:

A happy dog on a sunny path

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Duplication (Python) — 11 new duplicated blocks (worst 203 tokens)

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
posthog/temporal/ai_observability/run_aggregate_evaluation.py:692 posthog/temporal/ai_observability/run_trace_evaluation.py:928 38 203
products/warehouse_sources/backend/temporal/data_imports/sources/companycam/source.py:1 products/warehouse_sources/backend/temporal/data_imports/sources/wix/source.py:1 21 164
posthog/temporal/ai_observability/run_aggregate_evaluation.py:584 posthog/temporal/ai_observability/run_trace_evaluation.py:855 24 135
products/signals/backend/scout_harness/tools/report.py:1475 products/signals/backend/scout_harness/tools/report.py:1647 21 133
products/signals/backend/scout_harness/tools/report.py:1604 products/signals/backend/scout_harness/tools/report.py:1754 30 118
posthog/temporal/ai_observability/run_session_evaluation.py:466 posthog/temporal/ai_observability/run_trace_evaluation.py:664 24 115
posthog/management/commands/backfill_hogflow_billable_action_types.py:49 posthog/management/commands/refresh_hog_flows.py:69 23 112
products/batch_exports/backend/api/batch_export.py:2092 products/batch_exports/backend/api/batch_export.py:2131 24 112
products/warehouse_sources/backend/temporal/data_imports/sources/pardot/pardot.py:2 products/warehouse_sources/backend/temporal/data_imports/sources/wix/wix.py:1 12 97
products/signals/backend/scout_harness/tools/report.py:1516 products/signals/backend/scout_harness/tools/report.py:1675 12 71
products/signals/backend/scout_harness/tools/report.py:1531 products/signals/backend/scout_harness/tools/report.py:1690 16 70
⚠️ Duplication (TypeScript) — 2 new duplicated blocks (worst 158 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/business_knowledge/frontend/scenes/settings/businessKnowledgeSettingsLogic.test.ts:10 products/data_catalog/frontend/certificationsLogic.test.ts:17 23 158
products/business_knowledge/frontend/scenes/settings/businessKnowledgeSettingsLogic.test.ts:10 products/data_quality/frontend/dataQualityGateLogic.test.ts:14 23 158
✅ Playwright — all passed

All tests passed.

View test results →

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 9858eda6-3c7e-43ce-b870-724300bd26fc

📥 Commits

Reviewing files that changed from the base of the PR and between f696fcc and 258376d.

📒 Files selected for processing (2)
  • posthog/api/test/test_authentication.py
  • posthog/auth.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

WidgetAuthentication now raises AuthenticationFailed when team lookup raises Team.DoesNotExist or Team.MultipleObjectsReturned. A parameterized test checks both cases.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 25837

Ambiguous widget tokens now fail authentication instead of surfacing an unhandled lookup exception; missing-token behavior and successful authentication remain unchanged. No concrete merge-blocking risk remains in the supplied changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 25837

The change rejects ambiguous tokens instead of granting access to either matching team. No new access path was identified, but the exact HTTP response and behavior across every widget endpoint were not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Because the lookup selects a team by token rather than by a caller-supplied tenant identifier, a collision can affect authentication for the matching teams. This change rejects that ambiguous identity rather than selecting one.

Trust Boundaries and Controls

  • observed — An ambiguous token is stopped at authentication before a team is returned to the widget consumer. The inspected consumer's separate session controls are unchanged.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the problem, behavior change, testing, release status, documentation impact, and agent context. The Agent context includes the required tools and skills, but it does not provide…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@pauldambra
pauldambra added this pull request to stack #108171 September 29, 2026 08:42
@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

Generated-By: PostHog Desktop
Task-Id: 160dfc49-dceb-4887-b9f5-a93b2da017ed
@pauldambra
pauldambra force-pushed the posthog/fix-widget-token-collisions branch from 258376d to 39278a5 Compare September 29, 2026 12:43
@pauldambra
pauldambra marked this pull request as ready for review September 29, 2026 12:48
@pauldambra
pauldambra requested a review from a team as a code owner September 29, 2026 12:48
@posthog-security-review-bot posthog-security-review-bot Bot added the security-review Request a security review label Sep 29, 2026
@posthog-security-review-bot

posthog-security-review-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

✅ Security review complete — 39278a5adbd3

1 finding posted as a review, in 11 min.

Add the security-review label to run again on the latest commit.

@pauldambra

Copy link
Copy Markdown
Member Author

[Robot]

Note

🤖 Automated comment by QA Swarm — not written by a human

Verdict: APPROVE (round 1 @ 39278a5)

No actionable findings. Ambiguous widget tokens fail authentication instead of causing a server error.

Key findings

  • The parameterized test covers missing and duplicate token results. Both cases passed.

Convergence

Both reviewers found no correctness or security defects.

Reviewer summaries

Reviewer Assessment
Router (gpt-5.6-sol) Reviewed the complete patch. Authentication requires a second review.
Auth validator (gpt-6-astra) Confirmed that duplicate tokens cannot select or expose a team.

Automated by QA Swarm — not a human review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T12:51:27.475918Z 39278a5 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pauldambra pauldambra added the stamphog Request AI approval (no full review) label Sep 29, 2026
@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Sep 29, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

stamphog can't approve this pull request because two gates refused it. The deny-list gate flagged it for matching the auth category, since it changes posthog/auth.py and adds tests for authentication. The tier gate classified it as T2-never (12 lines, 2 files, single-area fix), a tier that is never eligible for automated approval. The size gate passed, so splitting the PR won't change the outcome. Please ask a human reviewer, ideally someone who owns authentication, to review it.

  • 👍 on the PR from chatgpt-codex-connector[bot].
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: auth
size ✓ 2L, 1F substantive, 12L/2F incl. docs/generated/snapshots — within ceiling
tier ✗ classified as T2-never: T2-never (12L, 2F, single-area, fix)
stamphog 2.3.0 .stamphog/policy.yml @ unknown · reviewed head 39278a5

@posthog-security-review-bot posthog-security-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agent-driven security review - findings inline.

Comment thread posthog/api/test/test_authentication.py
@posthog-security-review-bot posthog-security-review-bot Bot removed the security-review Request a security review label Sep 29, 2026

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