Partner viewer role and program/link scoping for partner member#3676
Open
Partner viewer role and program/link scoping for partner member#3676
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
📝 WalkthroughWalkthroughThis pull request implements a comprehensive role-based access control (RBAC) system for partner users, introducing granular permission management at program and link levels. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
actor User as Partner User (any role)
participant Client as API Client
participant Handler as withPartnerProfile Handler
participant Lookup as Prisma Lookups
participant Auth as Access Checks
participant Query as Scoped Queries
participant Response as Response
User->>Client: Request to /api/partner-profile/programs/[id]
Client->>Handler: Route request
Handler->>Lookup: Find partnerUser by (userId, partnerId)
Lookup-->>Handler: partnerUser with assignedPrograms/assignedLinks
Handler->>Auth: throwIfNoProgramAccess(programId, partnerUser)
alt Access Denied
Auth-->>Handler: DubApiError (forbidden)
Handler-->>Client: 403 Forbidden
else Access Granted
Handler->>Query: fetchProgram with linkIncludeFilter(assignedLinks)
Query-->>Handler: Program with scoped links
Handler->>Response: Build response
Response-->>Client: 200 OK with program data
end
sequenceDiagram
actor Owner as Partner Owner
participant UI as Members Page UI
participant Sheet as Programs Sheet
participant API as PUT /users/[id]/programs
participant DB as Database
participant Response as Response
Owner->>UI: Click "Edit programs" on member row
UI->>Sheet: Open sheet for member
Sheet->>Sheet: Load programEnrollments
Sheet->>Sheet: Initialize editableState from member.programs
Owner->>Sheet: Select programs and assign links
Sheet->>Sheet: Build programIds and linkIds from selections
Owner->>Sheet: Click Save
Sheet->>API: PUT request with programAccess, programIds, linkIds
API->>DB: Validate programs enrolled, fetch links
DB-->>API: Program and link records
alt Validation Error
API-->>Sheet: 400 Bad Request
else Success
API->>DB: Update partnerUserProgram and partnerUserLink
DB-->>API: Updated rows
API-->>Sheet: 200 OK with array of assigned programs
Sheet->>Sheet: Invalidate SWR cache
Sheet-->>UI: Close sheet, show success toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…count, and payout queries, ensuring access control based on program assignments.
…Ds and link IDs from partnerUser, enhancing code clarity and consistency across message and payout queries.
Add links.write and referrals.submit permissions, enforce them on partner link CRUD routes, bounty file upload, and referral submission to prevent viewer/member role bypass.
…ssigned links Add payout_settings.read permission to gate payout settings endpoint. Scope customers and earnings routes by assignedLinkIds for restricted partner users.
Gate sidebar nav items by permissions: Payouts requires payouts.read, Messages requires messages.send, Postbacks requires postbacks.read. Also hides PayoutStats widget from unauthorized roles.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/playwright/partners/rbac.spec.ts (1)
6-6:⚠️ Potential issue | 🟠 MajorIsolate the link-creation case from the seeded read assertions.
Lines 150-166 assert the exact seeded link set, but the same matrix later creates new links and the suite is configured to run in parallel at both scopes. Once any owner/member
POST /programs/acme/linkssucceeds, later owner/viewer reads become order-dependent. Move the mutating case into a separate serial/isolated describe, or clean up the created link before the fixed-list assertions run.#!/bin/bash FILE="apps/web/playwright/partners/rbac.spec.ts" printf '\nParallel configuration and link-matrix entries:\n' nl -ba "$FILE" | sed -n '1,340p' | rg -n 'describe\.configure|endpoint: "/programs/acme/links"|PARTNER_LINKS\.acme|method: "POST"' printf '\nExpected result:\n' printf '%s\n' \ '- parallel mode is enabled near the top of the file and inside runRbacSuite' \ '- the same RBAC matrix contains exact seeded-link assertions and a mutating POST /programs/acme/links case'Also applies to: 145-169, 208-215, 273-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` at line 6, The test enables parallel mode via describe.configure({ mode: "parallel" }) and the RBAC matrix in rbac.spec.ts mixes a mutating POST case (the owner/member POST /programs/acme/links entries that use PARTNER_LINKS.acme) with later assertions that expect the exact seeded link set; to fix, move the mutating POST /programs/acme/links case into its own describe configured as serial (or a separate test file) or ensure the POST-created link is deleted/rolled-back before the fixed-list assertions run so reads remain deterministic; locate the RBAC matrix and the seeded-link assertions in runRbacSuite / the matrix that references PARTNER_LINKS.acme and either extract the POST case into a serial describe or add cleanup logic that removes the created link prior to the seeded-list assertions.
🧹 Nitpick comments (1)
apps/web/playwright/partners/rbac.spec.ts (1)
259-268: Drop the unusedapi()wrapper or route requests through it.Right now the helper is dead code, and the method branch below re-implements the same wrapper logic directly. Keeping both makes future matrix changes easier to drift.
Also applies to: 307-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` around lines 259 - 268, Remove the dead helper or unify usage: either delete the unused api(request: APIRequestContext) factory, or refactor the later direct request calls to use it; specifically, ensure all request calls that currently inline `${API_BASE_URL}${path}` (including the block duplicated around the other request helpers) instead call api(request).get/post/patch/delete so the single api function (which builds URLs and forwards to request.get/post/patch/delete) is the single source of truth for API requests; update any tests that reference the inline behavior to use the api(...) wrapper or remove the wrapper if you prefer the inline form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/playwright/partners/rbac.spec.ts`:
- Line 6: The test enables parallel mode via describe.configure({ mode:
"parallel" }) and the RBAC matrix in rbac.spec.ts mixes a mutating POST case
(the owner/member POST /programs/acme/links entries that use PARTNER_LINKS.acme)
with later assertions that expect the exact seeded link set; to fix, move the
mutating POST /programs/acme/links case into its own describe configured as
serial (or a separate test file) or ensure the POST-created link is
deleted/rolled-back before the fixed-list assertions run so reads remain
deterministic; locate the RBAC matrix and the seeded-link assertions in
runRbacSuite / the matrix that references PARTNER_LINKS.acme and either extract
the POST case into a serial describe or add cleanup logic that removes the
created link prior to the seeded-list assertions.
---
Nitpick comments:
In `@apps/web/playwright/partners/rbac.spec.ts`:
- Around line 259-268: Remove the dead helper or unify usage: either delete the
unused api(request: APIRequestContext) factory, or refactor the later direct
request calls to use it; specifically, ensure all request calls that currently
inline `${API_BASE_URL}${path}` (including the block duplicated around the other
request helpers) instead call api(request).get/post/patch/delete so the single
api function (which builds URLs and forwards to request.get/post/patch/delete)
is the single source of truth for API requests; update any tests that reference
the inline behavior to use the api(...) wrapper or remove the wrapper if you
prefer the inline form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38a1da65-c5d0-492e-853d-a5c85dc00fde
📒 Files selected for processing (1)
apps/web/playwright/partners/rbac.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/playwright/seed.ts (1)
72-81:⚠️ Potential issue | 🟠 MajorFail fast when a required program fixture is missing.
Lines 72-81 can return a partial set, but later code assumes every slug in
PARTNER_PROGRAMSexists and that"acme"is present. If one is missing, the seed either skips data silently or blows up later onprogramsBySlug.get("acme")!.Suggested fix
const programs = await prisma.program.findMany({ where: { slug: { in: PARTNER_PROGRAMS, }, }, include: { groups: true, }, }); + + const programsBySlug = new Map( + programs.map((program) => [program.slug, program]), + ); + const missingPrograms = PARTNER_PROGRAMS.filter( + (slug) => !programsBySlug.has(slug), + ); + + if (missingPrograms.length > 0) { + throw new Error( + `Missing seeded programs: ${missingPrograms.join(", ")}`, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 72 - 81, The query using prisma.program.findMany may return a subset, but later code assumes every slug in PARTNER_PROGRAMS exists (and specifically "acme") — modify the seeding to fail fast: after fetching programs, build programsBySlug from programs and verify that every slug in PARTNER_PROGRAMS is present (and that programsBySlug.get("acme") exists); if any are missing, throw a clear error listing the missing slugs so the seed stops immediately instead of proceeding with incomplete data. Ensure you reference PARTNER_PROGRAMS, prisma.program.findMany, programs, and programsBySlug in the check and error message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/playwright/seed.ts`:
- Around line 54-67: The upsert for prisma.partnerUser currently only sets role
and programAccess on create, so rerunning seed doesn't update existing rows;
update the prisma.partnerUser.upsert call used to populate partnerUsers[role] to
include an update object that sets role and programAccess (e.g., update: { role:
role as PartnerRole, programAccess }) so existing partnerUser rows are updated
when reseeding.
- Around line 153-182: The current code upserts a single partnerUserProgram and
partnerUserLink for partnerUsers.member but does not remove any existing
assignments, leaving stale access; before creating the scoped assignments,
delete existing rows for that user: call prisma.partnerUserProgram.deleteMany({
where: { partnerUserId: partnerUsers.member.id } }) and
prisma.partnerUserLink.deleteMany({ where: { partnerUserId:
partnerUsers.member.id } }), then create the intended partnerUserProgram (using
programsBySlug.get("acme")) and partnerUserLink (using links["acme-link-1"].id
and programId) so the member ends up with only the desired scoped assignments.
---
Duplicate comments:
In `@apps/web/playwright/seed.ts`:
- Around line 72-81: The query using prisma.program.findMany may return a
subset, but later code assumes every slug in PARTNER_PROGRAMS exists (and
specifically "acme") — modify the seeding to fail fast: after fetching programs,
build programsBySlug from programs and verify that every slug in
PARTNER_PROGRAMS is present (and that programsBySlug.get("acme") exists); if any
are missing, throw a clear error listing the missing slugs so the seed stops
immediately instead of proceeding with incomplete data. Ensure you reference
PARTNER_PROGRAMS, prisma.program.findMany, programs, and programsBySlug in the
check and error message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07700dbb-2d29-4ebf-99a3-494d8773c078
📒 Files selected for processing (8)
.github/workflows/playwright.yamlapps/web/global-setup.tsapps/web/playwright/partners/auth.setup.tsapps/web/playwright/partners/rbac.spec.tsapps/web/playwright/seed.tsapps/web/playwright/workspaces/auth.setup.tsapps/web/scripts/dev/acme-workspace.jsonapps/web/scripts/dev/example-workspace.json
💤 Files with no reviewable changes (1)
- .github/workflows/playwright.yaml
✅ Files skipped from review due to trivial changes (3)
- apps/web/playwright/partners/auth.setup.ts
- apps/web/playwright/partners/rbac.spec.ts
- apps/web/scripts/dev/example-workspace.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/scripts/dev/acme-workspace.json
- apps/web/global-setup.ts
…boarding test for visibility checks
Collaborator
Author
|
@coderabbitai full review please |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
apps/web/playwright.config.ts (1)
35-35:⚠️ Potential issue | 🟠 Major
testIgnorestill does not excluderbac.spec.ts.
/(auth\.setup|rbac)\.ts/won’t matchrbac.spec.ts, so RBAC specs can still run underpartnersin addition topartner-rbac.✅ Suggested regex fix
- testIgnore: /(auth\.setup|rbac)\.ts/, + testIgnore: /(auth\.setup|rbac-auth\.setup|rbac\.spec)\.ts$/,#!/bin/bash python - <<'PY' import re pat = re.compile(r'(auth\.setup|rbac)\.ts') files = [ "partners/auth.setup.ts", "partners/rbac-auth.setup.ts", "partners/rbac.spec.ts", ] for f in files: print(f"{f}: {'MATCH' if pat.search(f) else 'NO MATCH'}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright.config.ts` at line 35, The current testIgnore regex only matches filenames ending with "rbac.ts" and misses "rbac.spec.ts"; update the testIgnore pattern used in the playwright config (testIgnore) to also match spec variants (e.g., change the pattern to allow an optional ".spec" after "rbac" such as using something like /(auth\.setup|rbac)(\.spec)?\.ts/ so "rbac.spec.ts" is excluded as intended).apps/web/lib/actions/partners/message-program.ts (1)
19-66:⚠️ Potential issue | 🔴 CriticalMissing member program-scope enforcement in server action.
messages.writerole permission is enforced, but this action still authorizes bypartner.idonly. A restricted partner member can target a program outside their assigned programs if the partner account is enrolled there.🔒 Suggested fix (constrain by member-assigned programs)
const { partner, user, partnerUser } = ctx; const { programSlug, text } = parsedInput; throwIfNoPermission({ role: partnerUser.role, permission: "messages.write", }); + // Enforce member program scope (for restricted partner users) + if ( + partnerUser.programAccess !== "all" && + !partnerUser.assignedPrograms?.some( + (p) => p.id === programSlug || p.slug === programSlug, + ) + ) { + throw new Error("Program not found"); // replace with DubApiError not_found + } + const program = await prisma.program.findFirstOrThrow({Based on learnings,
withPartnerProfileauto-enforces single-program scope for route handlers only; this server action needs its own program-scope check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/message-program.ts` around lines 19 - 66, The query currently authorizes by partner.id only and misses enforcing the partner member's program scope; modify the prisma.program.findFirstOrThrow call (used with programSlug) to also constrain programs to the member's assigned program(s): if partnerUser has a single-assigned program field (e.g., partnerUser.programId or partnerUser.assignedProgramId) add where: { id: partnerUser.programId, slug: programSlug, ... } (or include id: { in: partnerUser.programIds } if it's an array), and only omit this extra filter when partnerUser has org-wide scope; keep the existing messaging/partners conditions and the throwIfNoPermission check.apps/web/playwright/partners/rbac.spec.ts (1)
6-6:⚠️ Potential issue | 🟠 MajorRun this RBAC matrix serially.
This suite still mixes fixed-count reads for
/programs/acme/linkswithPOST /programs/acme/links, so parallel mode will race the seeded assertions and make the file flaky.Suggested fix
-test.describe.configure({ mode: "parallel" }); +test.describe.configure({ mode: "serial" }); @@ function runRbacSuite(role: PartnerRole) { - test.describe.configure({ mode: "parallel" }); + test.describe.configure({ mode: "serial" });Also applies to: 273-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` at line 6, The suite is configured to run in parallel via the call test.describe.configure({ mode: "parallel" }), which causes race conditions between seeded reads and POSTs; change the configuration to run serially by setting the mode to "serial" (replace any test.describe.configure({ mode: "parallel" }) occurrences in this file, including the duplicate instance later in the file, with test.describe.configure({ mode: "serial" }) so the RBAC matrix tests run deterministically).apps/web/playwright/seed.ts (3)
71-81:⚠️ Potential issue | 🟠 MajorValidate the required program fixtures before using them.
findMany()can return a partial set, but the rest of the seed assumes allPARTNER_PROGRAMSslugs exist (including"acme"). Fail fast if any are missing.🔧 Suggested fix
const programs = await prisma.program.findMany({ where: { slug: { in: PARTNER_PROGRAMS, }, }, include: { groups: true, }, }); + const foundSlugs = new Set(programs.map((p) => p.slug)); + const missingSlugs = PARTNER_PROGRAMS.filter((s) => !foundSlugs.has(s)); + if (missingSlugs.length > 0) { + throw new Error(`Missing seeded programs: ${missingSlugs.join(", ")}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 71 - 81, The seed uses prisma.program.findMany to load programs for PARTNER_PROGRAMS but doesn't verify all slugs were found; add a validation after the call (checking the programs array against PARTNER_PROGRAMS) and throw/exit with a clear error if any slugs are missing (include missing slug names in the message) so the seeding fails fast; update the code surrounding prisma.program.findMany and the programs variable to perform this check before further use.
54-68:⚠️ Potential issue | 🟠 MajorUpdate existing
partnerUserrows when reseeding.The
update: {}is empty, so rerunning the seed won't apply role or programAccess changes to existing rows.🔧 Suggested fix
partnerUsers[role] = await prisma.partnerUser.upsert({ where: { userId_partnerId: { userId: user.id, partnerId: partner.id, }, }, - update: {}, + update: { + role: role as PartnerRole, + programAccess, + }, create: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 54 - 68, The upsert call for partnerUsers uses an empty update block so reseeding won't change existing rows; modify the prisma.partnerUser.upsert (the partnerUsers[role] assignment) to set the existing record's role and programAccess in the update clause (use the role and programAccess variables tied to user.id and partner.id) so rerunning seed will apply role/programAccess changes to existing partnerUser rows.
153-182:⚠️ Potential issue | 🟠 MajorReplace the member's scoped assignments instead of only upserting.
The upserts ensure the allowed rows exist but don't remove older
partnerUserProgram/partnerUserLinkassignments. On reseed, the member can retain stale access.🔧 Suggested fix
+ // Clear stale assignments before upserting + await prisma.partnerUserProgram.deleteMany({ + where: { + partnerUserId: partnerUsers.member.id, + programId: { not: programsBySlug.get("acme")! }, + }, + }); + await prisma.partnerUserLink.deleteMany({ + where: { + partnerUserId: partnerUsers.member.id, + linkId: { not: links["acme-link-1"].id }, + }, + }); + // Assign Acme program to member (restricted access) await prisma.partnerUserProgram.upsert({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 153 - 182, Instead of upserting (which only ensures rows exist and can leave stale rows), remove the member's existing scoped assignments and then recreate the intended ones: use prisma.partnerUserProgram.deleteMany({ where: { partnerUserId: partnerUsers.member.id } }) and prisma.partnerUserLink.deleteMany({ where: { partnerUserId: partnerUsers.member.id } }) (or wrap in a transaction) to clear old assignments, then create the specific partnerUserProgram and partnerUserLink rows for partnerUsers.member.id using programsBySlug.get("acme") and links["acme-link-1"].id so the member's assignments are replaced rather than only upserted.apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx (1)
170-185:⚠️ Potential issue | 🟠 MajorGuard the Programs cell with the same editability rule as the menu.
The cell's
onClickis always provided, butRowMenuButtononly shows "Edit programs" for owners editing active, non-owner members. Invites, owner rows, and non-owner viewers can still reach the sheet by clicking the cell.🔧 Suggested fix
{ id: "programs", header: "Programs", minSize: 80, maxSize: 80, meta: { disableTruncate: true }, - cell: ({ row }) => ( - <PartnerMemberProgramsCell - partnerUser={row.original} - onClick={() => { - setSelectedUserForPrograms(row.original); - setShowProgramsSheet(true); - }} - /> - ), + cell: ({ row }) => { + const canEditPrograms = + isCurrentUserOwner && + status !== "invited" && + row.original.role !== "owner"; + + return ( + <PartnerMemberProgramsCell + partnerUser={row.original} + onClick={ + canEditPrograms + ? () => { + setSelectedUserForPrograms(row.original); + setShowProgramsSheet(true); + } + : undefined + } + /> + ); + }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx around lines 170 - 185, The Programs cell currently always supplies an onClick that opens the programs sheet (via setSelectedUserForPrograms and setShowProgramsSheet) while the RowMenuButton only exposes "Edit programs" when the same editability rule applies; update the cell to guard its onClick with the identical condition used by RowMenuButton (the owner/editing/active/non-owner check) so clicking PartnerMemberProgramsCell only triggers setSelectedUserForPrograms and setShowProgramsSheet when the user is allowed to edit programs.
🧹 Nitpick comments (3)
apps/web/tsconfig.json (1)
53-55: Confirm whether excludingplaywrightfrom tsconfig was intentional.The
"playwright"entry was previously in the exclude list (commit d691fda) but has been removed. Since the include pattern uses"**/*.ts"and"**/*.tsx", Playwright test files (apps/web/playwright/**/*.spec.ts) are now included in the app's TypeScript program.While tests run via separate commands (
test:e2efor Playwright,testfor Vitest) and maintain their own configs, including test files in the main tsconfig can introduce unnecessary type checking overhead. If typed E2E checks are desired, consider a dedicatedtsconfig.playwright.jsonand exclude theplaywrightdirectory from the main config for cleaner separation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tsconfig.json` around lines 53 - 55, The tsconfig.json change removed "playwright" from the "exclude" array which causes apps/web/playwright/**/*.spec.ts to be picked up by the main TypeScript program (because "include" covers "**/*.ts"/"**/*.tsx"); decide whether to keep Playwright tests out of the main build by re-adding "playwright" to the "exclude" list or, if you want typed E2E checks, create a dedicated tsconfig.playwright.json for the Playwright suite and leave the main tsconfig.json excluding "playwright" to avoid extra typechecking overhead..github/workflows/playwright.yaml (1)
106-111: Consider adding a health check for the Tinybird service.Unlike
mysqlandredis, thetinybirdservice lacks a health check. The "Setup Tinybird Local" step (line 140) assumes the container is ready when fetching the token. This could cause intermittent CI failures if the container isn't fully started.🔧 Suggested: Add a health check or retry logic
Option 1 — Add a health check to the service:
tinybird: image: tinybirdco/tinybird-local:latest env: COMPATIBILITY_MODE: "1" + options: >- + --health-cmd="curl -sf http://localhost:7181/tokens || exit 1" + --health-interval=5s + --health-timeout=5s + --health-retries=10 ports: - 7181:7181Option 2 — Add a wait loop before token fetch:
- name: Setup Tinybird Local working-directory: packages/tinybird run: | pip install tinybird-cli + # Wait for Tinybird to be ready + for i in {1..30}; do + curl -sf http://localhost:7181/tokens && break || sleep 2 + done TINYBIRD_TOKEN=$(curl -s http://localhost:7181/tokens | jq -r ".workspace_admin_token")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/playwright.yaml around lines 106 - 111, Add a readiness mechanism for the Tinybird service used by the Playwright workflow: either add a Docker healthcheck to the tinybird service definition (service name "tinybird" in the workflow) that probes the HTTP port 7181 until it returns a healthy response, or update the "Setup Tinybird Local" step to implement retry/wait logic that polls the Tinybird token endpoint (port 7181) with exponential backoff until it succeeds; ensure the change targets the tinybird service block and/or the Setup Tinybird Local step so token fetch only runs after the container is healthy.apps/web/scripts/dev/seed.ts (1)
138-160: Reject unknown CLI args instead of silently defaulting toacme.Any unrecognized argument is currently ignored, so a typo like
--workpsace examplewill quietly seed the default workspace. For a script that can also truncate data, fail-fast behavior is much safer.Suggested fix
for (let i = 0; i < args.length; i++) { const a = args[i]; if (a === "--truncate") { shouldTruncate = true; continue; @@ workspaceSlug = next; i++; continue; } + + throw new Error( + `Unknown argument: ${a}. Usage: [--truncate] [-w <slug> | --workspace <slug>]`, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/scripts/dev/seed.ts` around lines 138 - 160, The CLI parsing loop currently ignores any unknown arguments causing typos like `--workpsace` to be silently accepted; update the parsing logic that iterates over args to validate flags: keep handling for "--truncate" and "--workspace"/"-w" (using variables args, shouldTruncate, workspaceSlug), but add a branch that if a startsWith("-") and is not one of the handled flags, throw a descriptive Error (e.g. "Unknown argument: <arg>") so the script fails fast; ensure you still increment i when consuming the workspace value and return workspaceSlug default "acme" only after validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/partner-profile/payouts/count/route.ts:
- Around line 20-24: The composed `where` object currently spreads
programScopeFilter over the explicit programId, causing the exact filter to be
overwritten; update the construction of the Prisma.PayoutWhereInput (`where`) so
both constraints are applied: either wrap them in an AND array combining {
programId } and programScopeFilter(partnerUser.assignedProgramIds) when
programId is present, or skip spreading programScopeFilter when an exact
programId is provided (since throwIfNoProgramAccess already validated access);
modify the code around the where declaration (reference: where,
Prisma.PayoutWhereInput, programScopeFilter, programId, throwIfNoProgramAccess,
partnerUser.assignedProgramIds) accordingly.
In
`@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/partner-links-selector.tsx:
- Around line 45-54: The reconciliation useEffect is missing selectedLinkIds
from its dependency array; update the effect that references loading,
isAllLinks, selectedLinkIds, validLinkIds, and calls setSelectedLinkIds so that
selectedLinkIds is included in the dependency list (so the effect re-runs when
the parent updates selectedLinkIds and can filter stale IDs against
validLinkIds).
In
`@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-cell.tsx:
- Around line 67-79: The loading skeleton branch (condition enrollmentsLoading
&& programAccess === "all") runs before the owner check and causes owners to
briefly see the pulse UI; change the logic to not gate owner rows by either
moving the isOwner check above the loading branch (so the owner path renders the
"All" badge first) or modify the loading condition to include && !isOwner so
owners always render the owner-specific UI; refer to enrollmentsLoading,
programAccess, isOwner and the hook useProgramEnrollments() to locate the code
to update.
---
Duplicate comments:
In
`@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx:
- Around line 170-185: The Programs cell currently always supplies an onClick
that opens the programs sheet (via setSelectedUserForPrograms and
setShowProgramsSheet) while the RowMenuButton only exposes "Edit programs" when
the same editability rule applies; update the cell to guard its onClick with the
identical condition used by RowMenuButton (the owner/editing/active/non-owner
check) so clicking PartnerMemberProgramsCell only triggers
setSelectedUserForPrograms and setShowProgramsSheet when the user is allowed to
edit programs.
In `@apps/web/lib/actions/partners/message-program.ts`:
- Around line 19-66: The query currently authorizes by partner.id only and
misses enforcing the partner member's program scope; modify the
prisma.program.findFirstOrThrow call (used with programSlug) to also constrain
programs to the member's assigned program(s): if partnerUser has a
single-assigned program field (e.g., partnerUser.programId or
partnerUser.assignedProgramId) add where: { id: partnerUser.programId, slug:
programSlug, ... } (or include id: { in: partnerUser.programIds } if it's an
array), and only omit this extra filter when partnerUser has org-wide scope;
keep the existing messaging/partners conditions and the throwIfNoPermission
check.
In `@apps/web/playwright.config.ts`:
- Line 35: The current testIgnore regex only matches filenames ending with
"rbac.ts" and misses "rbac.spec.ts"; update the testIgnore pattern used in the
playwright config (testIgnore) to also match spec variants (e.g., change the
pattern to allow an optional ".spec" after "rbac" such as using something like
/(auth\.setup|rbac)(\.spec)?\.ts/ so "rbac.spec.ts" is excluded as intended).
In `@apps/web/playwright/partners/rbac.spec.ts`:
- Line 6: The suite is configured to run in parallel via the call
test.describe.configure({ mode: "parallel" }), which causes race conditions
between seeded reads and POSTs; change the configuration to run serially by
setting the mode to "serial" (replace any test.describe.configure({ mode:
"parallel" }) occurrences in this file, including the duplicate instance later
in the file, with test.describe.configure({ mode: "serial" }) so the RBAC matrix
tests run deterministically).
In `@apps/web/playwright/seed.ts`:
- Around line 71-81: The seed uses prisma.program.findMany to load programs for
PARTNER_PROGRAMS but doesn't verify all slugs were found; add a validation after
the call (checking the programs array against PARTNER_PROGRAMS) and throw/exit
with a clear error if any slugs are missing (include missing slug names in the
message) so the seeding fails fast; update the code surrounding
prisma.program.findMany and the programs variable to perform this check before
further use.
- Around line 54-68: The upsert call for partnerUsers uses an empty update block
so reseeding won't change existing rows; modify the prisma.partnerUser.upsert
(the partnerUsers[role] assignment) to set the existing record's role and
programAccess in the update clause (use the role and programAccess variables
tied to user.id and partner.id) so rerunning seed will apply role/programAccess
changes to existing partnerUser rows.
- Around line 153-182: Instead of upserting (which only ensures rows exist and
can leave stale rows), remove the member's existing scoped assignments and then
recreate the intended ones: use prisma.partnerUserProgram.deleteMany({ where: {
partnerUserId: partnerUsers.member.id } }) and
prisma.partnerUserLink.deleteMany({ where: { partnerUserId:
partnerUsers.member.id } }) (or wrap in a transaction) to clear old assignments,
then create the specific partnerUserProgram and partnerUserLink rows for
partnerUsers.member.id using programsBySlug.get("acme") and
links["acme-link-1"].id so the member's assignments are replaced rather than
only upserted.
---
Nitpick comments:
In @.github/workflows/playwright.yaml:
- Around line 106-111: Add a readiness mechanism for the Tinybird service used
by the Playwright workflow: either add a Docker healthcheck to the tinybird
service definition (service name "tinybird" in the workflow) that probes the
HTTP port 7181 until it returns a healthy response, or update the "Setup
Tinybird Local" step to implement retry/wait logic that polls the Tinybird token
endpoint (port 7181) with exponential backoff until it succeeds; ensure the
change targets the tinybird service block and/or the Setup Tinybird Local step
so token fetch only runs after the container is healthy.
In `@apps/web/scripts/dev/seed.ts`:
- Around line 138-160: The CLI parsing loop currently ignores any unknown
arguments causing typos like `--workpsace` to be silently accepted; update the
parsing logic that iterates over args to validate flags: keep handling for
"--truncate" and "--workspace"/"-w" (using variables args, shouldTruncate,
workspaceSlug), but add a branch that if a startsWith("-") and is not one of the
handled flags, throw a descriptive Error (e.g. "Unknown argument: <arg>") so the
script fails fast; ensure you still increment i when consuming the workspace
value and return workspaceSlug default "acme" only after validation.
In `@apps/web/tsconfig.json`:
- Around line 53-55: The tsconfig.json change removed "playwright" from the
"exclude" array which causes apps/web/playwright/**/*.spec.ts to be picked up by
the main TypeScript program (because "include" covers "**/*.ts"/"**/*.tsx");
decide whether to keep Playwright tests out of the main build by re-adding
"playwright" to the "exclude" list or, if you want typed E2E checks, create a
dedicated tsconfig.playwright.json for the Playwright suite and leave the main
tsconfig.json excluding "playwright" to avoid extra typechecking overhead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b30bab4e-e510-4e0e-a479-ae66fbaea692
📒 Files selected for processing (79)
.github/workflows/playwright.yamlapps/web/app/(ee)/api/cron/export/events/partner/route.tsapps/web/app/(ee)/api/partner-profile/invites/route.tsapps/web/app/(ee)/api/partner-profile/messages/count/route.tsapps/web/app/(ee)/api/partner-profile/messages/route.tsapps/web/app/(ee)/api/partner-profile/payouts/count/route.tsapps/web/app/(ee)/api/partner-profile/payouts/route.tsapps/web/app/(ee)/api/partner-profile/payouts/settings/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/export/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/[customerId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/timeseries/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/events/export/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/events/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/[linkId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/route.tsapps/web/app/(ee)/api/partner-profile/users/[userId]/programs/route.tsapps/web/app/(ee)/api/partner-profile/users/route.tsapps/web/app/(ee)/api/stripe/integration/webhook/checkout-session-completed.tsapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-links-selector.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-cell.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-sheet.tsxapps/web/global-setup.tsapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/create-bounty-submission.tsapps/web/lib/actions/partners/mark-program-messages-read.tsapps/web/lib/actions/partners/merge-partner-accounts.tsapps/web/lib/actions/partners/message-program.tsapps/web/lib/actions/partners/start-partner-platform-verification.tsapps/web/lib/actions/partners/update-partner-platforms.tsapps/web/lib/actions/partners/upload-bounty-submission-file.tsapps/web/lib/actions/partners/verify-partner-website.tsapps/web/lib/actions/partners/verify-social-account-by-code.tsapps/web/lib/actions/partners/withdraw-partner-application.tsapps/web/lib/actions/referrals/submit-referral.tsapps/web/lib/api/partner-profile/client.tsapps/web/lib/api/partner-profile/get-earnings-for-partner.tsapps/web/lib/api/partner-profile/get-partner-earnings-timeseries.tsapps/web/lib/api/programs/get-program-enrollment-or-throw.tsapps/web/lib/auth/options.tsapps/web/lib/auth/partner-users/link-scope-filter.tsapps/web/lib/auth/partner-users/partner-user-permissions.tsapps/web/lib/auth/partner-users/program-scope-filter.tsapps/web/lib/auth/partner-users/throw-if-no-access.tsapps/web/lib/auth/partner.tsapps/web/lib/integrations/appsflyer/schema.tsapps/web/lib/swr/use-program-messages-count.tsapps/web/lib/swr/use-program-messages.tsapps/web/lib/zod/schemas/partner-profile.tsapps/web/playwright.config.tsapps/web/playwright/partners/auth.setup.tsapps/web/playwright/partners/constants.tsapps/web/playwright/partners/rbac-auth.setup.tsapps/web/playwright/partners/rbac.spec.tsapps/web/playwright/seed.tsapps/web/playwright/workspaces/auth.setup.tsapps/web/playwright/workspaces/onboarding.spec.tsapps/web/scripts/dev/acme-workspace.jsonapps/web/scripts/dev/example-workspace.jsonapps/web/scripts/dev/seed.tsapps/web/tests/commissions/bulk-updates.test.tsapps/web/tsconfig.jsonapps/web/ui/layout/sidebar/partners-sidebar-nav.tsxapps/web/ui/modals/invite-partner-user-modal.tsxapps/web/ui/modals/reject-partner-application-modal.tsxapps/web/ui/partners/partner-application-details.tsxpackages/prisma/schema/link.prismapackages/prisma/schema/partner.prismapackages/prisma/schema/program.prismapackages/ui/src/hooks/use-router-stuff.tspackages/ui/src/icons/index.tsx
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-links-selector.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-cell.tsx
Outdated
Show resolved
Hide resolved
…right from TypeScript compilation, and improve conditional logic in partner profile API route. Refactor PartnerLinksSelector dependencies and adjust loading state rendering in PartnerMemberProgramsCell.
Collaborator
Author
|
@coderabbitai full review please |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/count/route.ts (1)
46-70:⚠️ Potential issue | 🟠 MajorDon't let the scope predicate erase the requested
linkIdfilter.When
partnerUser.assignedLinksis set, Line 69 replaces the earlierlinkIdequality from Lines 56-59 with{ in: assignedIds }. A restricted member requesting one allowed link will get counts for all assigned links instead of that one link.🔧 Suggested fix
+ throwIfNoLinkAccess({ + linkId, + partnerUser, + }); + + const scopedLinkWhere = linkScopeFilter(partnerUser.assignedLinks); + const commonWhere: Prisma.CustomerWhereInput = { partnerId: partner.id, programId: program.id, projectId: program.workspaceId, // Only filter by country if not grouping by country ...(country && groupBy !== "country" && { country, }), // Only filter by linkId if not grouping by linkId ...(linkId && groupBy !== "linkId" && { linkId, }), // Only allow search if customer data sharing is enabled ...(search && customerDataSharingEnabledAt ? search.includes("@") ? { email: search } : { email: { search: sanitizeFullTextSearch(search) }, name: { search: sanitizeFullTextSearch(search) }, } : {}), - ...linkScopeFilter(partnerUser.assignedLinks), + ...(scopedLinkWhere.linkId ? { AND: [scopedLinkWhere] } : {}), };Also add:
import { throwIfNoLinkAccess } from "@/lib/auth/partner-users/throw-if-no-access";Based on learnings: route-level validation of a requested
linkIdagainst assigned links is the intended pattern before downstream scoped queries rely on the Prismawhereclause.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/customers/count/route.ts around lines 46 - 70, The current scope filter can overwrite an explicit linkId: before building commonWhere, validate the requested linkId against the user's assigned links by calling throwIfNoLinkAccess(partnerUser.assignedLinks, linkId) (import throwIfNoLinkAccess), then change the spread of linkScopeFilter to only apply when linkId is not provided (e.g. use ...(linkId ? {} : linkScopeFilter(partnerUser.assignedLinks))) so an explicit linkId stays as an equality in commonWhere while still enforcing access for unrestricted queries.apps/web/app/(ee)/api/partner-profile/programs/[programId]/events/export/route.ts (1)
176-189:⚠️ Potential issue | 🟠 MajorQueue the normalized filters, not raw
searchParams.This handler rewrites filters before querying:
NOT INgets normalized,domain+keybecomes a concretelinkId, and restricted members may get an implicit scopedlinkIdfallback. The QStash payload still sends the originalsearchParams, so exports overMAX_EVENTS_TO_EXPORTcan return a different dataset than the sync path and can drop the computed link scope.Pass the resolved filter state you actually queried with to the background job instead of the raw request params.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/events/export/route.ts around lines 176 - 189, The QStash payload is sending the raw searchParams instead of the normalized filter state used to run the query, so background exports can return a different dataset; update the qstash.publishJSON call to send the resolved/normalized filter object (the variable you compute before querying — e.g., resolvedFilters, normalizedSearchParams, or similar) in place of searchParams, preserving columns: columns.join(","), partnerId, programId (params.programId), userId (session.user.id), and dataAvailableFrom ((program.startedAt ?? program.createdAt).toISOString()); ensure you reference the exact variable name for the normalized filters used by the querying logic rather than the original searchParams..github/workflows/playwright.yaml (1)
106-148:⚠️ Potential issue | 🟠 MajorWait for Tinybird to become ready before requesting a token.
curl http://localhost:7181/tokensruns immediately after the container starts, but the new service has no health check or retry loop. That makes the E2E job flaky whenever Tinybird Local needs a few extra seconds to boot.♻️ Suggested hardening
tinybird: image: tinybirdco/tinybird-local:latest env: COMPATIBILITY_MODE: "1" + options: >- + --health-cmd="curl -fsS http://127.0.0.1:7181/tokens >/dev/null || exit 1" + --health-interval=5s + --health-timeout=5s + --health-retries=20 ports: - 7181:7181 ... - name: Setup Tinybird Local working-directory: packages/tinybird run: | pip install tinybird-cli - TINYBIRD_TOKEN=$(curl -s http://localhost:7181/tokens | jq -r ".workspace_admin_token") + for i in {1..30}; do + TINYBIRD_TOKEN=$(curl -fsS http://localhost:7181/tokens | jq -r '.workspace_admin_token // empty') || true + [ -n "${TINYBIRD_TOKEN}" ] && break + sleep 2 + done + [ -n "${TINYBIRD_TOKEN}" ] echo "TINYBIRD_API_KEY=${TINYBIRD_TOKEN}" >> $GITHUB_ENV echo "TINYBIRD_API_URL=http://localhost:7181" >> $GITHUB_ENV tb auth --host http://localhost:7181 --token $TINYBIRD_TOKEN tb push🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/playwright.yaml around lines 106 - 148, The Setup Tinybird Local step currently calls curl http://localhost:7181/tokens immediately after starting the tinybird container, causing flakes; update the "Setup Tinybird Local" job to wait for Tinybird to be ready before requesting a token by polling the service (e.g., loop calling curl/http GET against http://localhost:7181/health or /tokens with --fail and sleeping/retrying) and only proceed to set TINYBIRD_TOKEN, export env vars, run tb auth, and tb push once the health check succeeds; reference the "Setup Tinybird Local" step and the curl http://localhost:7181/tokens invocation to locate where to add the retry/polling logic.
♻️ Duplicate comments (7)
apps/web/global-setup.ts (1)
7-24:⚠️ Potential issue | 🟠 MajorSeed the workspace fixtures for local Playwright runs too.
playwright/seed.tsstill runs unconditionally, butacme/exampleare only created behind the CI guard. On a clean local DB, global setup will fail before the tests start.🔧 Suggested change
- // Seed workspaces + programs (from dev seed JSON files) — CI only; local dev assumes seeded DB - if (process.env.GITHUB_ACTION) { - execSync("pnpm exec tsx scripts/dev/seed.ts -w acme", { - stdio: "inherit", - cwd: __dirname, - }); - - execSync("pnpm exec tsx scripts/dev/seed.ts -w example", { - stdio: "inherit", - cwd: __dirname, - }); - } + execSync("pnpm exec tsx scripts/dev/seed.ts -w acme", { + stdio: "inherit", + cwd: __dirname, + }); + + execSync("pnpm exec tsx scripts/dev/seed.ts -w example", { + stdio: "inherit", + cwd: __dirname, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/global-setup.ts` around lines 7 - 24, The global setup seeds 'acme' and 'example' only when process.env.GITHUB_ACTION is set but always runs playwright/seed.ts, causing local runs to fail on a clean DB; update the logic around the execSync calls so the workspace seed commands (the two execSync calls invoking "scripts/dev/seed.ts -w acme" and "-w example") also run for local development (i.e., remove or relax the CI-only guard using process.env.GITHUB_ACTION) or add a fallback that ensures those workspaces are created before running the unconditional execSync that calls "playwright/seed.ts", keeping the existing execSync usage and cwd/__dirname handling intact.apps/web/lib/actions/partners/withdraw-partner-application.ts (1)
19-27:⚠️ Potential issue | 🔴 CriticalAdd the per-program scope check in this server action.
throwIfNoPermissiononly validates role permissions. A restricted member withprogram_enrollments.withdrawcan still target another program on the same partner account because the lookup only checkspartnerId/programId, not the acting member’s assigned-program scope.Based on learnings,
withPartnerProfileonly applies this authorization to single-program API routes, so this action still needs its own explicit assigned-program validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/actions/partners/withdraw-partner-application.ts` around lines 19 - 27, The action currently calls throwIfNoPermission({ role: partnerUser.role, permission: "program_enrollments.withdraw" }) but doesn't enforce the member's assigned-program scope, so add an explicit check right after that call and before getProgramEnrollmentOrThrow: verify the acting member's assigned program(s) (e.g., partnerUser.assignedProgramId or partnerUser.assignedProgramIds) includes the target programId and throw a forbidden/authorization error if not; keep existing partner-level permission check and then proceed to getProgramEnrollmentOrThrow only when the program-scope check passes.apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx (1)
170-185:⚠️ Potential issue | 🟡 MinorGuard the Programs cell click with the same editability rule as the menu.
The
PartnerMemberProgramsCellalways receives anonClickhandler, butRowMenuButtononly shows "Edit programs" for owners editing active, non-owner members (line 435). This allows invites, owner rows, and non-owner viewers to open the programs sheet by clicking the cell.🛡️ Suggested fix
{ id: "programs", header: "Programs", minSize: 80, maxSize: 80, meta: { disableTruncate: true }, - cell: ({ row }) => ( - <PartnerMemberProgramsCell - partnerUser={row.original} - onClick={() => { - setSelectedUserForPrograms(row.original); - setShowProgramsSheet(true); - }} - /> - ), + cell: ({ row }) => { + const canEditPrograms = + isCurrentUserOwner && + status !== "invited" && + row.original.role !== "owner"; + + return ( + <PartnerMemberProgramsCell + partnerUser={row.original} + onClick={ + canEditPrograms + ? () => { + setSelectedUserForPrograms(row.original); + setShowProgramsSheet(true); + } + : undefined + } + /> + ); + }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx around lines 170 - 185, The Programs cell currently always invokes the programs sheet via the onClick passed into PartnerMemberProgramsCell, allowing non-editable rows (invites, owners, non-owner viewers) to open it; update the cell to apply the same editability guard used by RowMenuButton (the ownership/active/non-owner viewer check used around "Edit programs") before calling setSelectedUserForPrograms and setShowProgramsSheet so the programs sheet only opens for rows that are allowed to be edited.apps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-sheet.tsx (1)
114-163:⚠️ Potential issue | 🟡 MinorWrap save logic in try/finally to ensure
isSavingresets on unexpected errors.If
partnerProfileFetchthrows an unexpected exception (e.g., network failure before the onError callback),setIsSaving(false)won't execute, leaving the button in a perpetual loading state.Suggested fix
const handleSave = async () => { if (!user.id) return; const programIds = programAccess === "all" ? [] : Object.entries(accessState) .filter(([, hasAccess]) => hasAccess) .map(([id]) => id); // Build linkIds map for accessible programs const linkIds: Record<string, string[] | undefined> = {}; if (programAccess === "restricted") { for (const programId of programIds) { linkIds[programId] = linkState[programId]; } } setIsSaving(true); - await partnerProfileFetch( - "@put/api/partner-profile/users/:userId/programs", - { - params: { - userId: user.id, - }, - body: { - programAccess, - programIds, - linkIds, - }, - onSuccess: async () => { - toast.success("Program assignments updated!"); - await mutate( - (key) => - typeof key === "string" && - key.startsWith("/api/partner-profile/users"), - ); - setShowSheet(false); - }, - onError: (ctx) => { - toast.error( - ctx.error.message ?? "Failed to update program assignments.", - ); - }, - }, - ); - - setIsSaving(false); + try { + await partnerProfileFetch( + "@put/api/partner-profile/users/:userId/programs", + { + params: { + userId: user.id, + }, + body: { + programAccess, + programIds, + linkIds, + }, + onSuccess: async () => { + toast.success("Program assignments updated!"); + await mutate( + (key) => + typeof key === "string" && + key.startsWith("/api/partner-profile/users"), + ); + setShowSheet(false); + }, + onError: (ctx) => { + toast.error( + ctx.error.message ?? "Failed to update program assignments.", + ); + }, + }, + ); + } finally { + setIsSaving(false); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-sheet.tsx around lines 114 - 163, The save flow in handleSave can leave isSaving true if partnerProfileFetch throws before its callbacks run; wrap the partnerProfileFetch await in a try/finally so setIsSaving(false) is always executed on exit. Specifically, surround the await partnerProfileFetch(...) call with try { await partnerProfileFetch(...) } finally { setIsSaving(false) } (keeping the existing onSuccess/onError handlers intact) so setIsSaving is reset even on unexpected exceptions.apps/web/playwright/seed.ts (2)
54-68:⚠️ Potential issue | 🟠 MajorUpdate existing
partnerUserrows when reseeding.The upsert has an empty
updateobject, so rerunning the seed leaves existing users with their oldroleandprogramAccessvalues instead of applying the new fixture values.Suggested fix
partnerUsers[role] = await prisma.partnerUser.upsert({ where: { userId_partnerId: { userId: user.id, partnerId: partner.id, }, }, - update: {}, + update: { + role: role as PartnerRole, + programAccess, + }, create: { userId: user.id, partnerId: partner.id, role: role as PartnerRole, programAccess, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 54 - 68, The upsert for partnerUsers uses an empty update so existing rows keep old values; modify the prisma.partnerUser.upsert call to populate the update object with the same fields you set in create (assign role and programAccess using the current role variable and programAccess value, cast role as PartnerRole if needed) so reseeding updates existing partnerUser rows with the fixture role and programAccess values.
71-81:⚠️ Potential issue | 🟠 MajorValidate the required program fixtures before using them.
findMany()can return a partial set, but the code assumes allPARTNER_PROGRAMSslugs exist. The non-null assertions at lines 158, 164, and 180 will cause runtime errors if any program is missing.Suggested fix
const programs = await prisma.program.findMany({ where: { slug: { in: PARTNER_PROGRAMS, }, }, include: { groups: true, }, }); + + const missingSlugs = PARTNER_PROGRAMS.filter( + (slug) => !programs.some((p) => p.slug === slug), + ); + + if (missingSlugs.length > 0) { + throw new Error(`Missing seeded programs: ${missingSlugs.join(", ")}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/seed.ts` around lines 71 - 81, The code calls prisma.program.findMany(...) to load PARTNER_PROGRAMS but assumes all slugs exist (used later with non-null assertions), which can lead to runtime crashes; update the seed logic around programs to map results by slug, verify that every slug in PARTNER_PROGRAMS has a matching program (e.g., using a lookup from the returned programs), and if any are missing either create the missing fixtures or throw a clear error that stops the seed run; reference the variables and functions programs, PARTNER_PROGRAMS, and prisma.program.findMany when making the change so you replace the unsafe non-null assertions with an explicit presence check and a descriptive failure/creation path.apps/web/playwright/partners/rbac.spec.ts (1)
6-6:⚠️ Potential issue | 🟠 MajorDon't run the mutating RBAC matrix in parallel.
This file asserts fixed seeded counts for
/programs/acme/links, but the same matrix also creates links viaPOST /programs/acme/links. With file-level parallel mode, those writes race the read assertions across roles and make the suite flaky.Suggested fix
-test.describe.configure({ mode: "parallel" }); +test.describe.configure({ mode: "serial" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` at line 6, The test file currently configures Playwright to run in parallel via test.describe.configure({ mode: "parallel" }), which causes races for the mutating RBAC matrix; change that configuration so the suite runs serially (e.g., set mode to "serial" or remove the parallel configuration) to prevent concurrent POST /programs/acme/links from racing read assertions; update the top-level test.describe.configure call in rbac.spec.ts (the one using test.describe.configure) so the RBAC matrix tests run non-parallel.
🧹 Nitpick comments (3)
apps/web/playwright/partners/constants.ts (1)
50-50: Addas constfor type consistency.
PARTNER_PROGRAMSlacksas const, making itstring[]instead of a readonly tuple like other exports. This prevents TypeScript from inferring the literal types.Suggested fix
-export const PARTNER_PROGRAMS = ["acme", "example"]; +export const PARTNER_PROGRAMS = ["acme", "example"] as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/constants.ts` at line 50, PARTNER_PROGRAMS is declared as a mutable string[]; change its declaration to a readonly tuple by appending as const to the initializer so TypeScript infers string literal types (i.e., modify the export of PARTNER_PROGRAMS to use the same readonly tuple pattern as other exports, preserving the values "acme" and "example" and adding as const).apps/web/playwright/partners/rbac.spec.ts (2)
275-283: Potential race condition inbeforeAllacross parallel describes.With parallel execution, multiple role-specific describe blocks may execute
beforeAllconcurrently. The sharedinaccessibleLinkIdvariable could be set multiple times, though the value should be identical. Consider moving the lookup outsiderunRbacSuiteor using a module-level async initialization.Alternative: use a global setup or top-level beforeAll
let inaccessibleLinkId: string; +test.beforeAll(async () => { + const link = await prisma.link.findFirstOrThrow({ + where: { key: "acme-link-2" }, + select: { id: true }, + }); + inaccessibleLinkId = link.id; +}); + function runRbacSuite(role: PartnerRole) { test.describe.configure({ mode: "parallel" }); - test.beforeAll(async () => { - if (!inaccessibleLinkId) { - const link = await prisma.link.findFirstOrThrow({ - where: { key: "acme-link-2" }, - select: { id: true }, - }); - inaccessibleLinkId = link.id; - } - }); - for (const entry of RBAC_MATRIX) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` around lines 275 - 283, The shared lookup for inaccessibleLinkId inside test.beforeAll can race when multiple describe blocks run in parallel; move the prisma.lookup out of runRbacSuite and into a single module-level/top-level initialization (e.g., a top-level test.beforeAll or global setup) so it runs once and sets inaccessibleLinkId before any role-specific describes run; specifically relocate the prisma.link.findFirstOrThrow call that currently lives inside test.beforeAll to a single centralized initializer that guards against repeated execution and assigns the module-level inaccessibleLinkId.
259-268: Remove unusedapihelper function.The
apifunction is defined but never used—all tests userequestdirectly instead.Suggested fix
-function api(request: APIRequestContext) { - return { - get: (path: string) => request.get(`${API_BASE_URL}${path}`), - post: (path: string, data?: object) => - request.post(`${API_BASE_URL}${path}`, { data }), - patch: (path: string, data?: object) => - request.patch(`${API_BASE_URL}${path}`, { data }), - delete: (path: string) => request.delete(`${API_BASE_URL}${path}`), - }; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/playwright/partners/rbac.spec.ts` around lines 259 - 268, Remove the unused helper function `api` from the test file: delete the `function api(request: APIRequestContext) { ... }` definition since all tests use `request` directly; ensure there are no remaining references to `api` (if any, replace them with direct `request.get/post/patch/delete` calls) and run tests to confirm nothing else depends on that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/lib/actions/partners/mark-program-messages-read.ts`:
- Around line 16-23: The action currently checks throwIfNoPermission for
partnerUser.role but authorizes the target program via authPartnerActionClient
using partner.id, allowing restricted partner members to act on programs outside
their scope; update the handler to enforce per-user program scoping by resolving
the partner profile with withPartnerProfile (or otherwise validate partnerUser's
allowed program against parsedInput.programSlug) before calling
authPartnerActionClient so that programSlug is validated against the
partnerUser's assigned program(s) (ensure checks reference partnerUser,
programSlug, withPartnerProfile and authPartnerActionClient and refuse the
operation if the user's profile does not include the requested program).
In `@apps/web/lib/auth/partner.ts`:
- Around line 301-309: The access check currently assumes params.programId is an
ID when it starts with "prog_" which misclassifies valid slugs; update the logic
in apps/web/lib/auth/partner.ts where hasAccess is set so it checks both fields
instead of branching by prefix—use assignedPrograms.some(({ id, slug }) => id
=== params.programId || slug === params.programId) (referencing
params.programId, assignedPrograms, and hasAccess) to allow matching by either
id or slug.
In `@apps/web/ui/layout/sidebar/partners-sidebar-nav.tsx`:
- Around line 86-99: The unread-messages fetch is still invoked unconditionally
via useProgramMessagesCount even when the Messages nav is hidden; update the
hook call (useProgramMessagesCount) to only run when the viewer has the
messages.read permission by passing enabled: Boolean(partnerRole &&
hasPermission(partnerRole, "messages.read")) (or similarly gating with
partnerRole + hasPermission) so the fetch is skipped for users without
permission; apply the same change for the other occurrences referenced (lines
showing the Messages entry and any other useProgramMessagesCount calls around
218-228 and 390-402).
---
Outside diff comments:
In @.github/workflows/playwright.yaml:
- Around line 106-148: The Setup Tinybird Local step currently calls curl
http://localhost:7181/tokens immediately after starting the tinybird container,
causing flakes; update the "Setup Tinybird Local" job to wait for Tinybird to be
ready before requesting a token by polling the service (e.g., loop calling
curl/http GET against http://localhost:7181/health or /tokens with --fail and
sleeping/retrying) and only proceed to set TINYBIRD_TOKEN, export env vars, run
tb auth, and tb push once the health check succeeds; reference the "Setup
Tinybird Local" step and the curl http://localhost:7181/tokens invocation to
locate where to add the retry/polling logic.
In
`@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/customers/count/route.ts:
- Around line 46-70: The current scope filter can overwrite an explicit linkId:
before building commonWhere, validate the requested linkId against the user's
assigned links by calling throwIfNoLinkAccess(partnerUser.assignedLinks, linkId)
(import throwIfNoLinkAccess), then change the spread of linkScopeFilter to only
apply when linkId is not provided (e.g. use ...(linkId ? {} :
linkScopeFilter(partnerUser.assignedLinks))) so an explicit linkId stays as an
equality in commonWhere while still enforcing access for unrestricted queries.
In
`@apps/web/app/`(ee)/api/partner-profile/programs/[programId]/events/export/route.ts:
- Around line 176-189: The QStash payload is sending the raw searchParams
instead of the normalized filter state used to run the query, so background
exports can return a different dataset; update the qstash.publishJSON call to
send the resolved/normalized filter object (the variable you compute before
querying — e.g., resolvedFilters, normalizedSearchParams, or similar) in place
of searchParams, preserving columns: columns.join(","), partnerId, programId
(params.programId), userId (session.user.id), and dataAvailableFrom
((program.startedAt ?? program.createdAt).toISOString()); ensure you reference
the exact variable name for the normalized filters used by the querying logic
rather than the original searchParams.
---
Duplicate comments:
In
`@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsx:
- Around line 170-185: The Programs cell currently always invokes the programs
sheet via the onClick passed into PartnerMemberProgramsCell, allowing
non-editable rows (invites, owners, non-owner viewers) to open it; update the
cell to apply the same editability guard used by RowMenuButton (the
ownership/active/non-owner viewer check used around "Edit programs") before
calling setSelectedUserForPrograms and setShowProgramsSheet so the programs
sheet only opens for rows that are allowed to be edited.
In
`@apps/web/app/`(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-sheet.tsx:
- Around line 114-163: The save flow in handleSave can leave isSaving true if
partnerProfileFetch throws before its callbacks run; wrap the
partnerProfileFetch await in a try/finally so setIsSaving(false) is always
executed on exit. Specifically, surround the await partnerProfileFetch(...) call
with try { await partnerProfileFetch(...) } finally { setIsSaving(false) }
(keeping the existing onSuccess/onError handlers intact) so setIsSaving is reset
even on unexpected exceptions.
In `@apps/web/global-setup.ts`:
- Around line 7-24: The global setup seeds 'acme' and 'example' only when
process.env.GITHUB_ACTION is set but always runs playwright/seed.ts, causing
local runs to fail on a clean DB; update the logic around the execSync calls so
the workspace seed commands (the two execSync calls invoking
"scripts/dev/seed.ts -w acme" and "-w example") also run for local development
(i.e., remove or relax the CI-only guard using process.env.GITHUB_ACTION) or add
a fallback that ensures those workspaces are created before running the
unconditional execSync that calls "playwright/seed.ts", keeping the existing
execSync usage and cwd/__dirname handling intact.
In `@apps/web/lib/actions/partners/withdraw-partner-application.ts`:
- Around line 19-27: The action currently calls throwIfNoPermission({ role:
partnerUser.role, permission: "program_enrollments.withdraw" }) but doesn't
enforce the member's assigned-program scope, so add an explicit check right
after that call and before getProgramEnrollmentOrThrow: verify the acting
member's assigned program(s) (e.g., partnerUser.assignedProgramId or
partnerUser.assignedProgramIds) includes the target programId and throw a
forbidden/authorization error if not; keep existing partner-level permission
check and then proceed to getProgramEnrollmentOrThrow only when the
program-scope check passes.
In `@apps/web/playwright/partners/rbac.spec.ts`:
- Line 6: The test file currently configures Playwright to run in parallel via
test.describe.configure({ mode: "parallel" }), which causes races for the
mutating RBAC matrix; change that configuration so the suite runs serially
(e.g., set mode to "serial" or remove the parallel configuration) to prevent
concurrent POST /programs/acme/links from racing read assertions; update the
top-level test.describe.configure call in rbac.spec.ts (the one using
test.describe.configure) so the RBAC matrix tests run non-parallel.
In `@apps/web/playwright/seed.ts`:
- Around line 54-68: The upsert for partnerUsers uses an empty update so
existing rows keep old values; modify the prisma.partnerUser.upsert call to
populate the update object with the same fields you set in create (assign role
and programAccess using the current role variable and programAccess value, cast
role as PartnerRole if needed) so reseeding updates existing partnerUser rows
with the fixture role and programAccess values.
- Around line 71-81: The code calls prisma.program.findMany(...) to load
PARTNER_PROGRAMS but assumes all slugs exist (used later with non-null
assertions), which can lead to runtime crashes; update the seed logic around
programs to map results by slug, verify that every slug in PARTNER_PROGRAMS has
a matching program (e.g., using a lookup from the returned programs), and if any
are missing either create the missing fixtures or throw a clear error that stops
the seed run; reference the variables and functions programs, PARTNER_PROGRAMS,
and prisma.program.findMany when making the change so you replace the unsafe
non-null assertions with an explicit presence check and a descriptive
failure/creation path.
---
Nitpick comments:
In `@apps/web/playwright/partners/constants.ts`:
- Line 50: PARTNER_PROGRAMS is declared as a mutable string[]; change its
declaration to a readonly tuple by appending as const to the initializer so
TypeScript infers string literal types (i.e., modify the export of
PARTNER_PROGRAMS to use the same readonly tuple pattern as other exports,
preserving the values "acme" and "example" and adding as const).
In `@apps/web/playwright/partners/rbac.spec.ts`:
- Around line 275-283: The shared lookup for inaccessibleLinkId inside
test.beforeAll can race when multiple describe blocks run in parallel; move the
prisma.lookup out of runRbacSuite and into a single module-level/top-level
initialization (e.g., a top-level test.beforeAll or global setup) so it runs
once and sets inaccessibleLinkId before any role-specific describes run;
specifically relocate the prisma.link.findFirstOrThrow call that currently lives
inside test.beforeAll to a single centralized initializer that guards against
repeated execution and assigns the module-level inaccessibleLinkId.
- Around line 259-268: Remove the unused helper function `api` from the test
file: delete the `function api(request: APIRequestContext) { ... }` definition
since all tests use `request` directly; ensure there are no remaining references
to `api` (if any, replace them with direct `request.get/post/patch/delete`
calls) and run tests to confirm nothing else depends on that helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5409a840-9ec6-44d1-9e43-6e0e41f03693
📒 Files selected for processing (75)
.github/workflows/playwright.yamlapps/web/app/(ee)/api/cron/export/events/partner/route.tsapps/web/app/(ee)/api/partner-profile/invites/route.tsapps/web/app/(ee)/api/partner-profile/messages/count/route.tsapps/web/app/(ee)/api/partner-profile/messages/route.tsapps/web/app/(ee)/api/partner-profile/payouts/count/route.tsapps/web/app/(ee)/api/partner-profile/payouts/route.tsapps/web/app/(ee)/api/partner-profile/payouts/settings/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/export/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/[customerId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/customers/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/timeseries/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/events/export/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/events/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/[linkId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/links/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/count/route.tsapps/web/app/(ee)/api/partner-profile/programs/route.tsapps/web/app/(ee)/api/partner-profile/users/[userId]/programs/route.tsapps/web/app/(ee)/api/partner-profile/users/route.tsapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/page-client.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-links-selector.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-cell.tsxapps/web/app/(ee)/partners.dub.co/(dashboard)/profile/members/partner-member-programs-sheet.tsxapps/web/global-setup.tsapps/web/lib/actions/partners/accept-program-invite.tsapps/web/lib/actions/partners/create-bounty-submission.tsapps/web/lib/actions/partners/mark-program-messages-read.tsapps/web/lib/actions/partners/merge-partner-accounts.tsapps/web/lib/actions/partners/message-program.tsapps/web/lib/actions/partners/start-partner-platform-verification.tsapps/web/lib/actions/partners/update-partner-platforms.tsapps/web/lib/actions/partners/upload-bounty-submission-file.tsapps/web/lib/actions/partners/verify-partner-website.tsapps/web/lib/actions/partners/verify-social-account-by-code.tsapps/web/lib/actions/partners/withdraw-partner-application.tsapps/web/lib/actions/referrals/submit-referral.tsapps/web/lib/api/partner-profile/client.tsapps/web/lib/api/partner-profile/get-earnings-for-partner.tsapps/web/lib/api/partner-profile/get-partner-earnings-timeseries.tsapps/web/lib/api/payouts/get-eligible-payouts.tsapps/web/lib/api/programs/get-program-enrollment-or-throw.tsapps/web/lib/auth/options.tsapps/web/lib/auth/partner-users/link-scope-filter.tsapps/web/lib/auth/partner-users/partner-user-permissions.tsapps/web/lib/auth/partner-users/program-scope-filter.tsapps/web/lib/auth/partner-users/throw-if-no-access.tsapps/web/lib/auth/partner.tsapps/web/lib/swr/use-program-messages-count.tsapps/web/lib/swr/use-program-messages.tsapps/web/lib/zod/schemas/partner-profile.tsapps/web/playwright.config.tsapps/web/playwright/partners/auth.setup.tsapps/web/playwright/partners/constants.tsapps/web/playwright/partners/rbac-auth.setup.tsapps/web/playwright/partners/rbac.spec.tsapps/web/playwright/seed.tsapps/web/playwright/workspaces/auth.setup.tsapps/web/playwright/workspaces/onboarding.spec.tsapps/web/scripts/dev/acme-workspace.jsonapps/web/scripts/dev/example-workspace.jsonapps/web/scripts/dev/seed.tsapps/web/tsconfig.jsonapps/web/ui/layout/sidebar/partners-sidebar-nav.tsxapps/web/ui/modals/invite-partner-user-modal.tsxapps/web/ui/partners/confirm-payouts-sheet.tsxapps/web/ui/shared/inline-badge-popover.tsxpackages/prisma/schema/link.prismapackages/prisma/schema/partner.prismapackages/prisma/schema/program.prisma
Comment on lines
+16
to
+23
| const { partner, partnerUser } = ctx; | ||
| const { programSlug } = parsedInput; | ||
|
|
||
| throwIfNoPermission({ | ||
| role: partnerUser.role, | ||
| permission: "messages.mark_as_read", | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
Enforce partner-user program scope before the update.
This action now checks role permission, but it still authorizes the target program with partner.id only. Because it uses authPartnerActionClient rather than withPartnerProfile, restricted members can still mark messages as read for partner programs outside their assigned scope by passing another programSlug. The downstream impact is that they can clear unread state for conversations they should not control.
🔒 Proposed fix
+"use server";
+
+import { DubApiError } from "@/lib/api/errors";
import { throwIfNoPermission } from "@/lib/auth/partner-users/throw-if-no-permission";
import { prisma } from "@dub/prisma";
import * as z from "zod/v4";
import { authPartnerActionClient } from "../safe-action";
@@
throwIfNoPermission({
role: partnerUser.role,
permission: "messages.mark_as_read",
});
+
+ if (
+ partnerUser.assignedPrograms &&
+ !partnerUser.assignedPrograms.some(({ slug }) => slug === programSlug)
+ ) {
+ throw new DubApiError({
+ code: "not_found",
+ message: "Program not found.",
+ });
+ }
const program = await prisma.program.findFirstOrThrow({Also applies to: 24-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/actions/partners/mark-program-messages-read.ts` around lines 16
- 23, The action currently checks throwIfNoPermission for partnerUser.role but
authorizes the target program via authPartnerActionClient using partner.id,
allowing restricted partner members to act on programs outside their scope;
update the handler to enforce per-user program scoping by resolving the partner
profile with withPartnerProfile (or otherwise validate partnerUser's allowed
program against parsedInput.programSlug) before calling authPartnerActionClient
so that programSlug is validated against the partnerUser's assigned program(s)
(ensure checks reference partnerUser, programSlug, withPartnerProfile and
authPartnerActionClient and refuse the operation if the user's profile does not
include the requested program).
Comment on lines
+301
to
+309
| if (params.programId.startsWith("prog_")) { | ||
| hasAccess = assignedPrograms.some( | ||
| ({ id }) => id === params.programId, | ||
| ); | ||
| } else { | ||
| hasAccess = assignedPrograms.some( | ||
| ({ slug }) => slug === params.programId, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any program slugs in the codebase start with "prog_"
rg -n "slug.*prog_" --type ts -g '!*.spec.ts' -g '!*.test.ts' | head -20
# Check program slug validation/generation patterns
ast-grep --pattern 'slug: $$$'Repository: dubinc/dub
Length of output: 180
🏁 Script executed:
# Find the program schema definition
fd -e prisma | xargs grep -l "model Program"Repository: dubinc/dub
Length of output: 129
🏁 Script executed:
# Search for program slug validation or generation
rg -n "slug" --type ts -A 2 -B 2 | grep -i "program" | head -30Repository: dubinc/dub
Length of output: 2452
🏁 Script executed:
# Look for program creation patterns and slug usage
rg -n "createProgram|program.*slug|slug.*program" --type ts -A 1 | head -40Repository: dubinc/dub
Length of output: 3186
🏁 Script executed:
cat packages/prisma/schema/program.prismaRepository: dubinc/dub
Length of output: 9227
🏁 Script executed:
# Search for slug validation patterns, especially for programs
rg -n "slug.*validate|validate.*slug|slug.*format|slug.*pattern" --type ts -A 2 -B 2 | head -50Repository: dubinc/dub
Length of output: 3469
🏁 Script executed:
# Search for where programs are created and how slugs are generated
rg -n "createProgram|program.*create|prisma\.program\.create" --type ts -A 5 | head -60Repository: dubinc/dub
Length of output: 4848
🏁 Script executed:
# Check if there's explicit slug validation in the codebase
rg -n "slug" packages/prisma/schema/ -A 1Repository: dubinc/dub
Length of output: 1884
🏁 Script executed:
# Check the programs.ts schema file for slug validation
cat apps/web/lib/zod/schemas/programs.ts | head -100Repository: dubinc/dub
Length of output: 3274
🏁 Script executed:
# Also search for where programs are actually created in the API
rg -n "prisma\.program\.create|createProgram" --type ts -A 10 | head -80Repository: dubinc/dub
Length of output: 5422
🏁 Script executed:
# Check what actual program IDs look like
rg -n "id.*prog_|prog_.*id" --type ts | head -20Repository: dubinc/dub
Length of output: 2154
🏁 Script executed:
# Look for how program IDs are generated and used
rg -n "@default\(cuid\(\)" packages/prisma/schema/program.prisma -B 3 -A 3Repository: dubinc/dub
Length of output: 637
🏁 Script executed:
# Check if there are any example program IDs in the codebase
rg -n 'programId.*=|id.*=.*"prog_' --type ts | head -30Repository: dubinc/dub
Length of output: 3901
🏁 Script executed:
# Check if there's any validation preventing slugs from starting with "prog_"
rg -n "validSlugRegex|slug.*validate|slug.*pattern" --type ts -B 2 -A 5 | head -50Repository: dubinc/dub
Length of output: 3171
🏁 Script executed:
# Check the validSlugRegex constant definition
rg -n "validSlugRegex" --type ts -B 5 -A 5Repository: dubinc/dub
Length of output: 4056
Update program access validation to check both ID and slug without relying on prefix.
The current logic at lines 301–309 assumes that params.programId starting with prog_ indicates an ID lookup, otherwise it's a slug. However, validSlugRegex (/^[a-zA-Z0-9\-]+$/) does not prevent slugs from starting with prog_, unlike group slugs which explicitly exclude the grp_ prefix (see apps/web/lib/zod/schemas/groups.ts:122). A slug starting with prog_ would be incorrectly treated as an ID and rejected.
Check both collections instead:
Suggested fix
- if (params.programId.startsWith("prog_")) {
- hasAccess = assignedPrograms.some(
- ({ id }) => id === params.programId,
- );
- } else {
- hasAccess = assignedPrograms.some(
- ({ slug }) => slug === params.programId,
- );
- }
+ hasAccess = assignedPrograms.some(
+ ({ id, slug }) =>
+ id === params.programId || slug === params.programId,
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/auth/partner.ts` around lines 301 - 309, The access check
currently assumes params.programId is an ID when it starts with "prog_" which
misclassifies valid slugs; update the logic in apps/web/lib/auth/partner.ts
where hasAccess is set so it checks both fields instead of branching by
prefix—use assignedPrograms.some(({ id, slug }) => id === params.programId ||
slug === params.programId) (referencing params.programId, assignedPrograms, and
hasAccess) to allow matching by either id or slug.
Comment on lines
+86
to
+99
| ...(partnerRole && hasPermission(partnerRole, "messages.read") | ||
| ? [ | ||
| { | ||
| name: "Messages", | ||
| description: "Chat with programs you're enrolled in", | ||
| icon: Msgs, | ||
| href: "/messages" as `/${string}`, | ||
| active: pathname.startsWith("/messages"), | ||
| badge: unreadMessagesCount | ||
| ? Math.min(9, unreadMessagesCount) | ||
| : undefined, | ||
| }, | ||
| ] | ||
| : []), |
Contributor
There was a problem hiding this comment.
Gate the unread-messages fetch off the same permission check.
These changes only hide the Messages entry. useProgramMessagesCount({ enabled: true }) still runs unconditionally, so viewer users will keep hitting the now-forbidden unread-count endpoint on every page load.
♻️ Suggested follow-up
+ const canReadMessages =
+ !!partner?.role && hasPermission(partner.role, "messages.read");
+
const { count: unreadMessagesCount } = useProgramMessagesCount({
- enabled: true,
+ enabled: canReadMessages,
query: {
unread: true,
},
});Also applies to: 218-228, 390-402
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/ui/layout/sidebar/partners-sidebar-nav.tsx` around lines 86 - 99,
The unread-messages fetch is still invoked unconditionally via
useProgramMessagesCount even when the Messages nav is hidden; update the hook
call (useProgramMessagesCount) to only run when the viewer has the messages.read
permission by passing enabled: Boolean(partnerRole && hasPermission(partnerRole,
"messages.read")) (or similarly gating with partnerRole + hasPermission) so the
fetch is skipped for users without permission; apply the same change for the
other occurrences referenced (lines showing the Messages entry and any other
useProgramMessagesCount calls around 218-228 and 390-402).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores