Skip to content

fix(flags): enforce copy_flags target-project cap and add throttling - #70916

Merged
haacked merged 1 commit into
masterfrom
haacked/copy-flags-target-limits
Jul 16, 2026
Merged

fix(flags): enforce copy_flags target-project cap and add throttling#70916
haacked merged 1 commit into
masterfrom
haacked/copy-flags-target-limits

Conversation

@haacked

@haacked haacked commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

copy_flags declared max_length=50 on target_project_ids in CopyFlagsRequestSerializer, but the view read target_project_ids straight off request.data without validating against that serializer, so the cap was documented in the OpenAPI schema but never actually enforced. Each target project can also create cohorts and a feature flag, so an unbounded list turns one request into unbounded fan-out work, and the endpoint had no rate limiting at all.

(The bulk-copy UI and copy_flags API themselves already shipped in #69044. This PR only adds the hardening that was missing.)

Changes

  • Enforce the 50-target-project cap, backed by a shared MAX_COPY_FLAGS_TARGET_PROJECTS constant so the serializer's documented limit and the enforced limit can't drift apart.
  • Add CopyFlagsBurstRateThrottle (120/minute) and CopyFlagsSustainedRateThrottle (300/hour), both on PersonalApiKeyOrUserRateThrottle so they cover personal-API-key and session-authenticated callers alike, since the bulk-copy UI can drive up to 100 sequential calls for one copy operation and doesn't retry on 429.
  • Document the cap and throttles in docs/internal/feature-flags/django-api-endpoints.md.

This PR rebased across #67240 ("copy dependencies across projects"), which landed on master mid-review and rewrote most of copy_flags — including switching request parsing from manual request.data access to real serializer.is_valid(raise_exception=True) validation. That rewrite already fixes the cap-enforcement bug and the form/JSON body-parsing gap this PR originally patched by hand, via DRF's own validation machinery, so the manual checks were dropped as redundant rather than merged alongside the new code. What's left is genuinely new: the throttling (still absent on master) and the shared cap constant.

How did you test this code?

Automated:

  • Backend tests cover the target-project cap and throttling for session-authenticated requests (test_organization_feature_flag.py, 118 passed).
  • flagSelectionLogic.test.ts (5 passed).
  • Manually verified against a running local stack: exact cap boundary (50 accepted, 51 rejected), a real cross-project copy, and malformed target_project_ids input.

I haven't driven the UI manually in this session.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

Updated docs/internal/feature-flags/django-api-endpoints.md to note the target-project cap and throttles.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Tool: Claude Code.
  • I asked Claude to rebase this branch onto master and squash it to a single clean commit. It found the branch's history had drifted — the bulk-copy feature itself had already merged separately as feat(flags): bulk copy flags to other projects from the flags list #69044, the branch's first commit was a stray duplicate-import artifact, and an unrelated "fix the projects-tab toggle" commit had been fully reverted by later commits in the same branch. It dropped both and squashed the real changes into one commit.
  • Master moved again mid-review: feat(flags): copy dependencies across projects #67240 landed and rewrote most of copy_flags to support copying flag dependencies, conflicting with this branch across 7 hunks (one spanning ~320 lines). I asked Claude to resolve it via /resolve-conflicts. Rather than mechanically keeping both sides, it read feat(flags): copy dependencies across projects #67240's rewrite, recognized that switching to serializer.is_valid(raise_exception=True) already subsumed the cap-enforcement and body-parsing fixes this PR made by hand (DRF's ListField already understands form-encoded bodies), and dropped the now-redundant manual checks and the _check_target_project_write_access helper (superseded by feat(flags): copy dependencies across projects #67240's _validate_target_flag_for_copy) rather than keeping duplicate logic. It updated two test assertions that depended on the old manual-validation response shape and reverified everything (118 backend + 5 frontend tests, plus a fresh manual check against a running stack) before pushing.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Bundle size — no change

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 64.77 MiB · no change

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.22 MiB · 22 files no change ███░░░░░░░ 28.4% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.13 MiB · 2,978 files no change █████████░ 87.9% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
281.3 KiB ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
235.5 KiB src/taxonomy/core-filter-definitions-by-group.json
222.9 KiB ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
154.3 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB ../packages/quill/packages/quill/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

Dist folder size — 🔺 +247 B (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1312.48 MiB · 🔺 +247 B (+0.0%)

Playwright — all passed

All tests passed.

View test results →

Backend coverage — all changed backend lines covered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (43 / 43)

All changed backend lines are covered ✅

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
batch_exports ████████░░░░░░░░░░░░ 39.7% 8,414 / 21,220
demo ███████████░░░░░░░░░ 56.2% 1,497 / 2,663
warehouse_sources_queue ████████████░░░░░░░░ 59.2% 148 / 250
tasks █████████████░░░░░░░ 67.4% 25,560 / 37,895
data_tools ██████████████░░░░░░ 70.0% 63 / 90
ai_gateway ███████████████░░░░░ 75.0% 9 / 12
signals ████████████████░░░░ 79.1% 19,031 / 24,073
data_modeling ████████████████░░░░ 80.0% 4,834 / 6,045
cdp ████████████████░░░░ 80.7% 3,118 / 3,864
wizard ████████████████░░░░ 82.5% 772 / 936
notebooks █████████████████░░░ 84.3% 6,343 / 7,520
agent_platform █████████████████░░░ 84.7% 3,273 / 3,862
cohorts █████████████████░░░ 86.1% 4,022 / 4,671
actions █████████████████░░░ 86.6% 717 / 828
product_tours █████████████████░░░ 87.5% 1,266 / 1,447
exports ██████████████████░░ 88.3% 6,891 / 7,800
visual_review ██████████████████░░ 88.5% 5,565 / 6,287
business_knowledge ██████████████████░░ 88.5% 4,400 / 4,969
conversations ██████████████████░░ 88.9% 16,129 / 18,133
dashboards ██████████████████░░ 89.1% 5,719 / 6,418
mcp_analytics ██████████████████░░ 89.1% 2,502 / 2,807
error_tracking ██████████████████░░ 89.6% 9,718 / 10,852
alerts ██████████████████░░ 89.9% 3,638 / 4,046
engineering_analytics ██████████████████░░ 90.1% 5,105 / 5,665
streamlit_apps ██████████████████░░ 90.4% 2,499 / 2,764
slack_app ██████████████████░░ 90.6% 9,511 / 10,503
links ██████████████████░░ 90.6% 183 / 202
marketing_analytics ██████████████████░░ 90.8% 11,514 / 12,684
product_analytics ██████████████████░░ 91.1% 5,599 / 6,143
mcp_store ██████████████████░░ 91.8% 3,668 / 3,995
managed_migrations ██████████████████░░ 91.9% 908 / 988
data_warehouse ██████████████████░░ 92.1% 18,133 / 19,683
workflows ██████████████████░░ 92.4% 5,148 / 5,574
notifications ███████████████████░ 92.7% 1,026 / 1,107
web_analytics ███████████████████░ 92.7% 13,624 / 14,691
ai_observability ███████████████████░ 92.8% 14,868 / 16,019
surveys ███████████████████░ 92.9% 5,687 / 6,120
posthog_ai ███████████████████░ 93.2% 1,322 / 1,418
tracing ███████████████████░ 93.3% 2,435 / 2,611
approvals ███████████████████░ 93.3% 3,395 / 3,640
reminders ███████████████████░ 93.4% 468 / 501
early_access_features ███████████████████░ 93.8% 848 / 904
legal_documents ███████████████████░ 94.1% 1,568 / 1,667
endpoints ███████████████████░ 94.1% 8,606 / 9,143
skills ███████████████████░ 94.4% 2,827 / 2,995
revenue_analytics ███████████████████░ 94.5% 3,598 / 3,809
messaging ███████████████████░ 94.5% 2,530 / 2,677
review_hog ███████████████████░ 94.6% 6,532 / 6,905
logs ███████████████████░ 95.3% 9,528 / 9,994
experiments ███████████████████░ 95.6% 24,171 / 25,288
replay_vision ███████████████████░ 95.7% 13,354 / 13,952
growth ███████████████████░ 95.8% 2,837 / 2,960
annotations ███████████████████░ 96.2% 732 / 761
warehouse_sources ███████████████████░ 96.2% 230,252 / 239,282
feature_flags ███████████████████░ 96.3% 16,038 / 16,661
user_interviews ███████████████████░ 96.4% 2,242 / 2,325
access_control ███████████████████░ 96.8% 849 / 877
data_catalog ███████████████████░ 97.1% 2,034 / 2,095
customer_analytics ███████████████████░ 97.2% 7,480 / 7,698
analytics_platform ████████████████████ 98.0% 2,102 / 2,145
metrics ████████████████████ 98.2% 2,404 / 2,448
live_debugger ████████████████████ 99.2% 613 / 618
field_notes ████████████████████ 99.4% 158 / 159

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

ℹ️ Docs preview — preview build triggered

Docs from this PR will be published at posthog.com.

Project Preview Updated (UTC)
posthog.com Open preview Jul 15, 2026, 11:30 PM

The preview should be ready in about 10 minutes. Open the preview at /handbook/engineering/.

Copilot AI 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.

Pull request overview

Adds end-to-end support for bulk copying feature flags to multiple projects from the feature flags list, strengthening backend enforcement/telemetry (caps + throttles) and improving frontend UX (bulk-copy summaries + projects toggle accessibility and failure handling).

Changes:

  • Backend: enforce a shared 50-target-project cap for copy_flags, add dedicated burst/sustained throttles, and document 400 responses in the schema.
  • Backend tests: cover target-project cap, throttling for session-authenticated requests, and object-level access-control denial on overwrite.
  • Frontend: refine bulk-copy summary formatting, harden failed-project-ID handling, and improve project active-toggle accessibility + failure recovery.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
products/feature_flags/frontend/generated/api.schemas.ts Regenerates OpenAPI TS types (moves/defines ErrorResponseApi).
products/feature_flags/backend/api/test/test_organization_feature_flag.py Adds tests for cap enforcement, throttling, and object-level overwrite denial.
products/feature_flags/backend/api/organization_feature_flag.py Adds cap enforcement + structured 400 response schema + throttling + helper for target write-access checks.
posthog/rate_limit.py Introduces CopyFlagsBurstRateThrottle and CopyFlagsSustainedRateThrottle.
frontend/src/scenes/feature-flags/flagSelectionLogic.ts Improves bulk-copy result summary messaging and makes failed-project-ID collection null-safe.
frontend/src/scenes/feature-flags/FlagActiveToggleTag.tsx Adds switch semantics/labeling to the project active toggle wrapper (but currently duplicates switch roles).
frontend/src/scenes/feature-flags/featureFlagLogic.test.ts Adds frontend test ensuring toggle failure clears in-flight state and restores previous values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/scenes/feature-flags/FlagActiveToggleTag.tsx
Comment thread products/feature_flags/backend/api/organization_feature_flag.py Outdated
@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit cbcdaa4 · box box-a76d0a039897 · ready in 753s (push → usable) · build log · rebuilds on every push, torn down on close

@haacked haacked moved this to In Progress in Feature Flags Jul 15, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 15, 2026 02:39
@posthog-project-board-bot posthog-project-board-bot Bot moved this from In Progress to In Review in Feature Flags Jul 15, 2026
@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "Fix CI: remove duplicate test, narrow ta..." | Re-trigger Greptile

@haacked
haacked force-pushed the haacked/copy-flags-target-limits branch from 46553cc to f1a6a2a Compare July 15, 2026 02:52
@haacked haacked changed the title feat(flags): copy flags to other projects from the flags list fix(flags): enforce copy_flags target-project cap and add throttling Jul 15, 2026
@haacked
haacked force-pushed the haacked/copy-flags-target-limits branch from f1a6a2a to 8e5040a Compare July 15, 2026 20:04
copy_flags declared max_length=50 on target_project_ids in
CopyFlagsRequestSerializer, but the view read the field straight off
request.data without validating against that serializer, so the cap
was documented in the OpenAPI schema but never enforced. Each target
project can also create cohorts and a feature flag, so an unbounded
list turns one request into unbounded fan-out work. Add a runtime
check backed by a shared MAX_COPY_FLAGS_TARGET_PROJECTS constant so
the documented and enforced limits can't drift apart.

Also add throttling, since copy_flags had none: CopyFlagsBurstRateThrottle
(120/minute) and CopyFlagsSustainedRateThrottle (300/hour), both on
PersonalApiKeyOrUserRateThrottle so they cover personal-API-key and
session-authenticated callers alike, since the bulk-copy UI can drive
up to 100 sequential calls for one copy operation and doesn't retry
on 429.

target_project_ids now parses correctly from both form-encoded and
JSON request bodies. Form bodies parse to a QueryDict, where .get()
returns only the last raw value for a repeated key; .getlist() handles
that case as well as plain JSON bodies.

Extract the target-project write-access check into
_check_target_project_write_access for reuse and clarity.
@haacked
haacked force-pushed the haacked/copy-flags-target-limits branch from 8e5040a to cbcdaa4 Compare July 15, 2026 23:29
@haacked haacked added the stamphog Request AI approval (no full review) label Jul 15, 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.

Adds throttling and a shared cap constant to an existing feature-flag copy endpoint; author is on the owning team, tests cover the cap/throttle/access-control paths, and the two prior bot review comments are already resolved by the current code (serializer-based validation, and the flagged file isn't touched here). No unresolved concerns, holds, or in-flight reviews.

  • Author wrote 20% of the modified lines and has 203 merged PRs in these paths (familiarity MODERATE).
  • 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 55L, 3F substantive, 148L/5F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1d-complex (148L, 5F, cross-cutting, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ b92384c · reviewed head cbcdaa4

@github-project-automation github-project-automation Bot moved this from In Review to Approved in Feature Flags Jul 15, 2026
@haacked
haacked enabled auto-merge (squash) July 16, 2026 00:08
@haacked
haacked merged commit dba9319 into master Jul 16, 2026
414 of 423 checks passed
@haacked
haacked deleted the haacked/copy-flags-target-limits branch July 16, 2026 00:09
@github-project-automation github-project-automation Bot moved this from Approved to Done in Feature Flags Jul 16, 2026
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-16 00:33 UTC Run
prod-us ✅ Deployed 2026-07-16 00:56 UTC Run
prod-eu ✅ Deployed 2026-07-16 00:54 UTC Run

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

Labels

feature/feature-flags Feature Tag: Feature flags stamphog Request AI approval (no full review) team/feature-flags

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants