Conversation
Lets a customer's server opt recipients in to or out of messages with a project secret API key instead of a user-owned personal API key. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
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 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds messaging preference read and write scopes for project secret keys across the scope catalogs and generated MCP API definitions. The messaging preference API accepts project secret keys for mapped actions, applies scope checks and throttles, and handles non-user credentials. Bulk opt-out saving now creates missing rows, locks the batch, and updates preferences. Tests cover scoped API access and concurrent recipient creation. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Project secret keys can manage messaging preferences as intended; the scope checks match the platform's existing read/write rules. One small API documentation gap remains: the API schema lists only "201 Created" for opt-out updates, although updating an existing recipient returns "200 OK". Generated clients may therefore mis-type that response. This is a quick fix and does not block safe use of the feature. Security Architecture ReviewSecurity architecture risk: 🟠 High · up to A credential intended only to update messaging preferences can also list and export recipient contact data. Individual updates additionally reveal stored preferences and whether the recipient already existed. Project isolation and rate limits contain the exposure, but the promised write-without-read boundary is not enforced. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…read Project secret keys now get a dedicated messaging_preference scope instead of hog_flow:write. Its write half does not cover read, so a key that sets preferences cannot list or export every recipient's opt-outs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Write responses show a write-only caller only the preference it wrote, always with 200, so they no longer reveal stored preferences or whether the recipient existed. - messaging_preference is OAuth-hidden: the consent screen assumes write covers read. - Project secret key system-table access follows read coverage. - The project secret key picker offers read and write together. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The stored record id is a UUIDT that encodes creation time, so returning it told a write-only caller whether the recipient existed. Write-only callers now get only the identifier and the preference they set. The project secret key picker only offers read and write together for messaging preferences, so other scopes holding both halves still show their last action. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: efff078f-5393-45f6-9315-3d984c619ef8
⛔ Files ignored due to path filters (5)
products/access_control/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/messaging/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/messaging/frontend/generated/api.tsis excluded by!**/generated/**services/mcp/src/lib/oauth-scopes.generated.tsis excluded by!**/*.generated.*services/mcp/src/tools/generated/messaging.tsis excluded by!**/generated/**
📒 Files selected for processing (14)
frontend/src/lib/components/ScopeAccessRow/ScopeAccessRow.tsxfrontend/src/lib/scopes.tsxfrontend/src/scenes/settings/project/ProjectSecretAPIKeys.tsxfrontend/src/scenes/settings/project/projectSecretAPIKeysLogic.test.tsfrontend/src/scenes/settings/project/projectSecretAPIKeysLogic.tsxposthog/api/test/test_authentication.pyposthog/auth.pyposthog/scopes.pyposthog/test/test_scopes.pyproducts/messaging/backend/api/message_preferences.pyproducts/messaging/backend/api/test/test_message_preferences.pyproducts/messaging/backend/api/test/test_message_preferences_project_secret_key.pyproducts/workflows/frontend/OptOuts/optOutListLogic.tsservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
A full preference response also matches the write-only shape, so oneOf rejected valid responses. One write result serializer with optional id and updated_at covers both. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ipients Two requests that both create the same recipient no longer race on the unique constraint. The loser raised a 500, which also told a write-only key the recipient had not existed before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rence write as read The MCP scope gate now reads WRITE_EXCLUDES_READ_SCOPE_OBJECTS from the scopes projection, and the 'Read and write' option is disabled when either half is. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A scope whose write excluded read made the project secret key picker confusing: every other row's Write covers Read. Since the key lives on a customer's server, the clearer picker is worth the extra read access. Write responses return the stored record again. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Declare the HTTP 200 response for existing recipients. · message_preferences.py:321
products/messaging/backend/api/message_preferences.py:321
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeclare the HTTP 200 response for existing recipients.
When
add_opt_outorremove_opt_outfinds an existing record, it returns HTTP 200. Bothextend_schemadeclarations currently document only HTTP 201. Add HTTP 200 with theMessagePreferencesSerializerresponse shape so generated clients can describe successful updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 7948ec4c-ead5-443a-b1e4-26de7c67f475
⛔ Files ignored due to path filters (1)
services/mcp/src/lib/oauth-scopes.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (6)
frontend/src/lib/scopes.tsxposthog/scopes.pyproducts/messaging/backend/api/message_preferences.pyproducts/messaging/backend/api/test/test_message_preferences.pyproducts/messaging/backend/api/test/test_message_preferences_project_secret_key.pyservices/mcp/src/api/generated.ts
💤 Files with no reviewable changes (1)
- services/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
|
Risk: No findings The change since the last review is confined to the project-secret-key test file: it pins the SDK's exact form-urlencoded request shape and changes the cross-team expectations to 403. I traced both rejection paths end-to-end (team resolution from the body/query token in posthog/api/routing.py:666-680 and the psak team equality checks in posthog/permissions.py:870-902 and 220-223) and confirmed production already enforces exactly what the new tests assert; no production code changed and no new risk is introduced. Sentinel reviewed |
add_opt_out and remove_opt_out return 200 when the recipient already exists, which the schema did not declare. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Problem
hog_flow:readorhog_flow:write.hog_flow:writealso lets it edit workflows.posthog.messaging.setPreferences()in PostHog/posthog-js#5173, which needs a project-scoped service credential.Changes
phs_) can now set recipients' messaging preferences.messaging_preferencescope covers the opt-out endpoints and nothing else in workflows.POST /api/projects/@current/messaging_preferences/{add_opt_out|remove_opt_out}/?token=phc_...withAuthorization: Bearer phs_....hog_flow:*keep working unchanged.messaging_preferenceobjectscope_objecthog_flowhog_flowkeys working. The narrow scope is accepted per action.add_opt_out,remove_opt_out,bulk_add_opt_outs(write);opt_outs,export_opt_outs_csv(read)generate_link,webhook_urlmessaging_preferenceis OAuth-hiddenhog_flow.hog_flow:*; the API still accepts the narrow scope.created_byNonefor project secret keysNote
The secret key alone decides the project. The
?token=query parameter is not read on POST, so a mismatched public token cannot redirect a write.The "after" screenshot replaces the earlier one, which showed a fourth "Read and write" option. The tooltip screenshot is gone, because the row no longer has a tooltip.
How did you test this code?
test_message_preferences_project_secret_key.pysends the SDK's exact request with a project secret key. It failed with 401 before the change.hog_flow:writekey cannot.hog_flow:writekey, a read-only key, another project's key, a deleted key, aphc_Bearer token, and no credentials cannot write.generate_linkandwebhook_urlrefuse project secret keys.?token=, and the shared per-project rate limit.test_personal_api_key_accessgains rows for personal keys with the narrow scope.test_bulk_opt_out_concurrency.pyholds another connection's uncommitted insert of the same recipient while a bulk opt-out runs. It returned 500 before the fix.Test rationale: The existing personal key test covers
hog_flowkeys only. Project secret keys use a different authenticator, user type, scope and throttles, so their cases live in their own file. The main test file would otherwise pass 1,000 lines.Release status
Automatic notifications
Docs update
messaging_preference:write.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5, 1M context)/adding-project-secret-api-key-auth,/adding-api-scopes,/writing-tests,/improving-drf-endpoints,/writing-user-facing-copy,/reviewing-with-coderabbit,/writing-pr-descriptions.hog_flow:writefor project secret keys.ON CONFLICT DO NOTHING, then lock and update the rows.hog_flow:writecannot mint a project secret key with the narrow scope. This is by design.hogli build:openapiandhogli build:projectionsregenerated the scope enum and the MCP OAuth hidden list.🤖 Generated with Claude Code