feat: grant legacy pro enterprise feature access - #5912
Conversation
|
|
Running ultrareview automatically — Adds a new database backfill granting enterprise entitlements to all Pro organizations, touching core entitlement logic and the shared seeder used in Stripe checkout; a bug here could mass-grant or mass-revoke feature access.. I'll post findings when complete. |
This comment has been minimized.
This comment has been minimized.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ultrareview completed in 12m 9s
4 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/cmd/tools/migrations/pro_entitlements_cmd.go">
<violation number="1" location="server/cmd/tools/migrations/pro_entitlements_cmd.go:90">
P2: For a large Pro population, `ListProOrganizations` materializes every ID before processing any row, so memory usage grows with the full table and an interruption forces a full rescan. Page the source with a keyset cursor and process each page.</violation>
<violation number="2" location="server/cmd/tools/migrations/pro_entitlements_cmd.go:96">
P2: After this command commits an entitlement, existing Redis feature entries are never refreshed, so gated requests can continue seeing `Enabled=false` for up to 15 minutes; the raw transaction also bypasses the advisory lock that prevents a cache fill from republishing stale state. Acquire the canonical per-feature locks before `Begin` and refresh every inserted feature after commit.
(Based on your team's feedback about serializing product-feature cache updates.)</violation>
<violation number="3" location="server/cmd/tools/migrations/pro_entitlements_cmd.go:152">
P1: Custom agent: **Flag Security Vulnerabilities**
When `GRAM_DATABASE_URL` sets `sslmode=disable` or permits a plaintext fallback, this call opens the privileged PostgreSQL connection without TLS. It can expose database credentials and entitlement writes to a network attacker even when `-confirm-target` matches. Parse the pool config and require TLS with plaintext fallbacks removed before connecting.</violation>
</file>
<file name="server/cmd/tools/migrations/pro_entitlements_cmd_test.go">
<violation number="1" location="server/cmd/tools/migrations/pro_entitlements_cmd_test.go:76">
P2: The successful apply transaction is not exercised here: the apply test only checks that the orchestration callback receives `true`. Add a Pro `migrateProOrganization(..., true)` test that asserts `Commit` and not `Rollback`, so writes cannot silently stop being committed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| } | ||
| ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) | ||
| defer stop() | ||
| pool, err := pgxpool.New(ctx, cfg.dbURL) |
There was a problem hiding this comment.
P1: Custom agent: Flag Security Vulnerabilities
When GRAM_DATABASE_URL sets sslmode=disable or permits a plaintext fallback, this call opens the privileged PostgreSQL connection without TLS. It can expose database credentials and entitlement writes to a network attacker even when -confirm-target matches. Parse the pool config and require TLS with plaintext fallbacks removed before connecting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/cmd/tools/migrations/pro_entitlements_cmd.go, line 152:
<comment>When `GRAM_DATABASE_URL` sets `sslmode=disable` or permits a plaintext fallback, this call opens the privileged PostgreSQL connection without TLS. It can expose database credentials and entitlement writes to a network attacker even when `-confirm-target` matches. Parse the pool config and require TLS with plaintext fallbacks removed before connecting.</comment>
<file context>
@@ -0,0 +1,169 @@
+ }
+ ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
+ defer stop()
+ pool, err := pgxpool.New(ctx, cfg.dbURL)
+ if err != nil {
+ fmt.Fprintln(stdout, "migration failed: connect postgres")
</file context>
| } | ||
|
|
||
| func backfillProEntitlements(ctx context.Context, db proEntitlementsDB, apply bool) (proEntitlementsReport, error) { | ||
| organizationIDs, err := featurerepo.New(db).ListProOrganizations(ctx) |
There was a problem hiding this comment.
P2: For a large Pro population, ListProOrganizations materializes every ID before processing any row, so memory usage grows with the full table and an interruption forces a full rescan. Page the source with a keyset cursor and process each page.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/cmd/tools/migrations/pro_entitlements_cmd.go, line 90:
<comment>For a large Pro population, `ListProOrganizations` materializes every ID before processing any row, so memory usage grows with the full table and an interruption forces a full rescan. Page the source with a keyset cursor and process each page.</comment>
<file context>
@@ -0,0 +1,169 @@
+}
+
+func backfillProEntitlements(ctx context.Context, db proEntitlementsDB, apply bool) (proEntitlementsReport, error) {
+ organizationIDs, err := featurerepo.New(db).ListProOrganizations(ctx)
+ if err != nil {
+ return proEntitlementsReport{}, fmt.Errorf("list pro organizations: %w", err)
</file context>
| } | ||
|
|
||
| return backfillProOrganizations(organizationIDs, apply, func(organizationID string, apply bool) (int, error) { | ||
| tx, err := db.Begin(ctx) |
There was a problem hiding this comment.
P2: After this command commits an entitlement, existing Redis feature entries are never refreshed, so gated requests can continue seeing Enabled=false for up to 15 minutes; the raw transaction also bypasses the advisory lock that prevents a cache fill from republishing stale state. Acquire the canonical per-feature locks before Begin and refresh every inserted feature after commit.
(Based on your team's feedback about serializing product-feature cache updates.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/cmd/tools/migrations/pro_entitlements_cmd.go, line 96:
<comment>After this command commits an entitlement, existing Redis feature entries are never refreshed, so gated requests can continue seeing `Enabled=false` for up to 15 minutes; the raw transaction also bypasses the advisory lock that prevents a cache fill from republishing stale state. Acquire the canonical per-feature locks before `Begin` and refresh every inserted feature after commit.
(Based on your team's feedback about serializing product-feature cache updates.) </comment>
<file context>
@@ -0,0 +1,169 @@
+ }
+
+ return backfillProOrganizations(organizationIDs, apply, func(organizationID string, apply bool) (int, error) {
+ tx, err := db.Begin(ctx)
+ if err != nil {
+ return 0, fmt.Errorf("begin organization transaction: %w", err)
</file context>
| require.Equal(t, proEntitlementsReport{Organizations: 2, FeaturesAdded: 4}, report) | ||
| } | ||
|
|
||
| func TestBackfillProOrganizationsAppliesEachOrganization(t *testing.T) { |
There was a problem hiding this comment.
P2: The successful apply transaction is not exercised here: the apply test only checks that the orchestration callback receives true. Add a Pro migrateProOrganization(..., true) test that asserts Commit and not Rollback, so writes cannot silently stop being committed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/cmd/tools/migrations/pro_entitlements_cmd_test.go, line 76:
<comment>The successful apply transaction is not exercised here: the apply test only checks that the orchestration callback receives `true`. Add a Pro `migrateProOrganization(..., true)` test that asserts `Commit` and not `Rollback`, so writes cannot silently stop being committed.</comment>
<file context>
@@ -0,0 +1,105 @@
+ require.Equal(t, proEntitlementsReport{Organizations: 2, FeaturesAdded: 4}, report)
+}
+
+func TestBackfillProOrganizationsAppliesEachOrganization(t *testing.T) {
+ t.Parallel()
+ var seeded []string
</file context>
2 participants
Summary
Motivation
Legacy Pro organizations should receive enterprise-level feature access without changing their billing behavior or resource limits. An application data backfill follows the repository's migration rules while preserving administrator choices represented by soft-deleted feature rows.
Summary by cubic
Adds a dry-run-by-default backfill that grants Legacy Pro organizations the shared enterprise-access entitlement bundle without changing their billing or resource limits.
New Features
pro-entitlementscommand underserver/cmd/tools/migrations; it defaults to a non-writing preview and requires explicit environment, target, and subcommand confirmations to apply.gram_account_typebefore writing, and preserves soft-deleted features as administrator disables so reruns stay idempotent.GRAM_DATABASE_URLtarget and error-and-rerun recovery, inPRO_ENTITLEMENTS_BACKFILL.md.Refactors
SeedPaygEntitlementsTxandEnterpriseTrialBundletoSeedEnterpriseAccessEntitlementsTxandEnterpriseAccessBundleso enterprise trials, PAYG activation, and the Pro backfill share one seeder.Written for commit 4c5bf32. Summary will update on new commits.