From 7bdc2f215e931b3bd18a6230cbd99f190fc0aa77 Mon Sep 17 00:00:00 2001 From: Francis Cao Date: Mon, 6 Jul 2026 10:58:43 -0700 Subject: [PATCH] harden report references in boards --- .../(reports)/funnels/BoardFunnel.tsx | 7 +- .../[websiteId]/(reports)/goals/BoardGoal.tsx | 9 +- src/app/api/boards/[boardId]/clone/route.ts | 10 +- src/app/api/boards/[boardId]/route.ts | 12 +- src/app/api/boards/route.ts | 11 +- src/app/api/dashboard/route.ts | 7 +- src/permissions/board.test.ts | 176 +++++++++++++++++- src/permissions/board.ts | 124 +++++++++++- 8 files changed, 346 insertions(+), 10 deletions(-) diff --git a/src/app/(main)/websites/[websiteId]/(reports)/funnels/BoardFunnel.tsx b/src/app/(main)/websites/[websiteId]/(reports)/funnels/BoardFunnel.tsx index 8818cf381..63876fe8d 100644 --- a/src/app/(main)/websites/[websiteId]/(reports)/funnels/BoardFunnel.tsx +++ b/src/app/(main)/websites/[websiteId]/(reports)/funnels/BoardFunnel.tsx @@ -1,6 +1,7 @@ import { Text } from '@umami/react-zen'; import { EmptyPlaceholder } from '@/components/common/EmptyPlaceholder'; import { LoadingPanel } from '@/components/common/LoadingPanel'; +import { Link2Off } from '@/components/icons'; import { useReportQuery } from '@/components/hooks/queries/useReportQuery'; import { Funnel } from './Funnel'; @@ -9,7 +10,11 @@ export function BoardFunnel({ websiteId, reportId }: { websiteId: string; report if (!reportId) { return ( - + } + title="Reconnect funnel" + description="Choose a funnel for this website." + /> ); } diff --git a/src/app/(main)/websites/[websiteId]/(reports)/goals/BoardGoal.tsx b/src/app/(main)/websites/[websiteId]/(reports)/goals/BoardGoal.tsx index 1211adfd8..7e68181e7 100644 --- a/src/app/(main)/websites/[websiteId]/(reports)/goals/BoardGoal.tsx +++ b/src/app/(main)/websites/[websiteId]/(reports)/goals/BoardGoal.tsx @@ -2,6 +2,7 @@ import { Text } from '@umami/react-zen'; import { EmptyPlaceholder } from '@/components/common/EmptyPlaceholder'; import { LoadingPanel } from '@/components/common/LoadingPanel'; import { useDateRange } from '@/components/hooks'; +import { Link2Off } from '@/components/icons'; import { useReportQuery } from '@/components/hooks/queries/useReportQuery'; import { Goal } from './Goal'; @@ -12,7 +13,13 @@ export function BoardGoal({ websiteId, reportId }: { websiteId: string; reportId const { data, isLoading, error, isFetching } = useReportQuery(reportId || ''); if (!reportId) { - return ; + return ( + } + title="Reconnect goal" + description="Choose a goal for this website." + /> + ); } if (data && (data.type !== 'goal' || data.websiteId !== websiteId)) { diff --git a/src/app/api/boards/[boardId]/clone/route.ts b/src/app/api/boards/[boardId]/clone/route.ts index 7431cdaa6..b127f7fcb 100644 --- a/src/app/api/boards/[boardId]/clone/route.ts +++ b/src/app/api/boards/[boardId]/clone/route.ts @@ -10,6 +10,8 @@ import { canCreateWebsite, canUpdateBoard, canViewBoardEntities, + hasValidBoardReports, + stripInvalidBoardReports, } from '@/permissions'; import { createBoard, getBoard } from '@/queries/prisma'; @@ -57,12 +59,18 @@ export async function POST(request: Request, { params }: { params: Promise<{ boa return badRequest({ message: 'Board contains inaccessible entities.' }); } + const sanitizedParameters = await stripInvalidBoardReports(sourceType, parameters); + + if (!(await hasValidBoardReports(sourceType, sanitizedParameters))) { + return badRequest({ message: 'Board contains invalid saved reports.' }); + } + const result = await createBoard({ id: uuid(), type: sourceType, name: body.name ?? sourceBoard.name, description: body.description ?? sourceBoard.description, - parameters: parameters as Prisma.InputJsonValue, + parameters: sanitizedParameters as Prisma.InputJsonValue, teamId: sourceBoard.teamId, userId: sourceBoard.teamId ? undefined : sourceBoard.userId ?? auth.user.id, }); diff --git a/src/app/api/boards/[boardId]/route.ts b/src/app/api/boards/[boardId]/route.ts index 4ce188411..a923adcf0 100644 --- a/src/app/api/boards/[boardId]/route.ts +++ b/src/app/api/boards/[boardId]/route.ts @@ -3,7 +3,13 @@ import { BOARD_TYPES, normalizeBoardType } from '@/lib/boards'; import type { BoardParameters } from '@/lib/types'; import { parseRequest } from '@/lib/request'; import { badRequest, json, ok, serverError, unauthorized } from '@/lib/response'; -import { canDeleteBoard, canUpdateBoard, canViewBoard, canViewBoardEntities } from '@/permissions'; +import { + canDeleteBoard, + canUpdateBoard, + canViewBoard, + canViewBoardEntities, + hasValidBoardReports, +} from '@/permissions'; import { deleteBoard, getBoard, updateBoard } from '@/queries/prisma'; export async function GET(request: Request, { params }: { params: Promise<{ boardId: string }> }) { @@ -68,6 +74,10 @@ export async function POST(request: Request, { params }: { params: Promise<{ boa if (!(await canViewBoardEntities(auth, nextType, nextParameters))) { return badRequest({ message: 'Board contains inaccessible entities.' }); } + + if (!(await hasValidBoardReports(nextType, nextParameters))) { + return badRequest({ message: 'Board contains invalid saved reports.' }); + } } try { diff --git a/src/app/api/boards/route.ts b/src/app/api/boards/route.ts index fd78f0029..252b6d697 100644 --- a/src/app/api/boards/route.ts +++ b/src/app/api/boards/route.ts @@ -4,7 +4,12 @@ import { uuid } from '@/lib/crypto'; import { getQueryFilters, parseRequest } from '@/lib/request'; import { badRequest, json, unauthorized } from '@/lib/response'; import { pagingParams, searchParams, sortingParams } from '@/lib/schema'; -import { canCreateTeamWebsite, canCreateWebsite, canViewBoardEntities } from '@/permissions'; +import { + canCreateTeamWebsite, + canCreateWebsite, + canViewBoardEntities, + hasValidBoardReports, +} from '@/permissions'; import { createBoard, getUserBoards } from '@/queries/prisma'; export async function GET(request: Request) { @@ -62,6 +67,10 @@ export async function POST(request: Request) { return badRequest({ message: 'Board contains inaccessible entities.' }); } + if (!(await hasValidBoardReports(body.type, body.parameters))) { + return badRequest({ message: 'Board contains invalid saved reports.' }); + } + const data = { ...body, type: normalizeBoardType(body.type), diff --git a/src/app/api/dashboard/route.ts b/src/app/api/dashboard/route.ts index 868c39d6b..71a7dc92d 100644 --- a/src/app/api/dashboard/route.ts +++ b/src/app/api/dashboard/route.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; +import { hasValidBoardReports } from '@/permissions'; import { parseRequest } from '@/lib/request'; -import { json, unauthorized } from '@/lib/response'; +import { badRequest, json, unauthorized } from '@/lib/response'; import { createBoard, getBoard, updateBoard } from '@/queries/prisma'; export async function GET(request: Request) { @@ -45,6 +46,10 @@ export async function POST(request: Request) { parameters: body.parameters ?? {}, }; + if (!(await hasValidBoardReports(existing?.type ?? 'dashboard', data.parameters))) { + return badRequest({ message: 'Board contains invalid saved reports.' }); + } + if (existing) { const result = await updateBoard(userId, data); diff --git a/src/permissions/board.test.ts b/src/permissions/board.test.ts index afae58084..97344b57c 100644 --- a/src/permissions/board.test.ts +++ b/src/permissions/board.test.ts @@ -1,12 +1,18 @@ import { beforeEach, expect, test, vi } from 'vitest'; import { BOARD_TYPES } from '@/lib/boards'; -import { canViewBoardEntities } from './board'; +import { getReport } from '@/queries/prisma'; +import { + canViewBoardEntities, + hasValidBoardReports, + stripInvalidBoardReports, +} from './board'; import { canViewLink } from './link'; import { canViewPixel } from './pixel'; import { canViewWebsite } from './website'; vi.mock('@/queries/prisma', () => ({ getBoard: vi.fn(), + getReport: vi.fn(), getTeamUser: vi.fn(), })); @@ -42,6 +48,7 @@ beforeEach(() => { vi.mocked(canViewWebsite).mockReset(); vi.mocked(canViewPixel).mockReset(); vi.mocked(canViewLink).mockReset(); + vi.mocked(getReport).mockReset(); }); test('canViewBoardEntities validates board IDs with user auth only', async () => { @@ -82,3 +89,170 @@ test('canViewBoardEntities rejects IDs not accessible to the user', async () => }), ).resolves.toBe(false); }); + +test('hasValidBoardReports allows saved reports for the board website', async () => { + vi.mocked(getReport).mockResolvedValue({ + id: 'report-1', + websiteId: 'website-1', + type: 'goal', + } as any); + + await expect( + hasValidBoardReports(BOARD_TYPES.website, { + websiteId: 'website-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + props: { + reportId: 'report-1', + }, + }, + }, + ], + }, + ], + }), + ).resolves.toBe(true); +}); + +test('hasValidBoardReports rejects saved reports from a different website', async () => { + vi.mocked(getReport).mockResolvedValue({ + id: 'report-1', + websiteId: 'website-2', + type: 'goal', + } as any); + + await expect( + hasValidBoardReports(BOARD_TYPES.website, { + websiteId: 'website-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + props: { + reportId: 'report-1', + }, + }, + }, + ], + }, + ], + }), + ).resolves.toBe(false); +}); + +test('hasValidBoardReports rejects saved reports with the wrong type', async () => { + vi.mocked(getReport).mockResolvedValue({ + id: 'report-1', + websiteId: 'website-1', + type: 'funnel', + } as any); + + await expect( + hasValidBoardReports(BOARD_TYPES.website, { + websiteId: 'website-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + props: { + reportId: 'report-1', + }, + }, + }, + ], + }, + ], + }), + ).resolves.toBe(false); +}); + +test('hasValidBoardReports rejects report widgets without a website context', async () => { + await expect( + hasValidBoardReports(BOARD_TYPES.pixel, { + pixelId: 'pixel-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + props: { + reportId: 'report-1', + }, + }, + }, + ], + }, + ], + }), + ).resolves.toBe(false); +}); + +test('stripInvalidBoardReports removes only invalid report IDs', async () => { + vi.mocked(getReport).mockResolvedValue({ + id: 'report-1', + websiteId: 'website-2', + type: 'goal', + } as any); + + await expect( + stripInvalidBoardReports(BOARD_TYPES.website, { + websiteId: 'website-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + title: 'Saved signup goal', + description: 'Should stay on the board', + props: { + reportId: 'report-1', + customValue: 'keep-me', + }, + }, + }, + ], + }, + ], + }), + ).resolves.toEqual({ + websiteId: 'website-1', + rows: [ + { + id: 'row-1', + columns: [ + { + id: 'column-1', + component: { + type: 'Goal', + title: 'Saved signup goal', + description: 'Should stay on the board', + props: { + customValue: 'keep-me', + }, + }, + }, + ], + }, + ], + }); +}); diff --git a/src/permissions/board.ts b/src/permissions/board.ts index d63b4a495..01082ce8e 100644 --- a/src/permissions/board.ts +++ b/src/permissions/board.ts @@ -1,12 +1,58 @@ import { hasPermission } from '@/lib/auth'; -import { getBoardEntityIds } from '@/lib/boards'; +import { BOARD_ENTITY_TYPES, getBoardEntityIds, getResolvedComponentEntity } from '@/lib/boards'; import { PERMISSIONS } from '@/lib/constants'; -import type { Auth, BoardParameters } from '@/lib/types'; -import { getBoard, getTeamUser } from '@/queries/prisma'; +import type { Auth, BoardComponentConfig, BoardParameters } from '@/lib/types'; +import { getBoard, getReport, getTeamUser } from '@/queries/prisma'; import { canViewLink } from './link'; import { canViewPixel } from './pixel'; import { canViewWebsite } from './website'; +const BOARD_COMPONENT_REPORT_TYPES = { + Funnel: 'funnel', + Goal: 'goal', +} as const; + +function getExpectedBoardReportType(componentType?: string) { + return componentType && componentType in BOARD_COMPONENT_REPORT_TYPES + ? BOARD_COMPONENT_REPORT_TYPES[componentType as keyof typeof BOARD_COMPONENT_REPORT_TYPES] + : null; +} + +async function isValidBoardReportReference( + board: { type?: string; parameters?: BoardParameters }, + component: BoardComponentConfig, + reportCache: Map>, +) { + const expectedReportType = getExpectedBoardReportType(component.type); + + if (!expectedReportType) { + return true; + } + + const reportId = component.props?.reportId; + + if (reportId == null || reportId === '') { + return true; + } + + if (typeof reportId !== 'string') { + return false; + } + + const entity = getResolvedComponentEntity(board, component); + + if (entity.entityType !== BOARD_ENTITY_TYPES.website || !entity.entityId) { + return false; + } + + const reportPromise = reportCache.get(reportId) ?? getReport(reportId); + reportCache.set(reportId, reportPromise); + + const report = await reportPromise; + + return !!report && report.type === expectedReportType && report.websiteId === entity.entityId; +} + async function checkBoardEntityAccess(check: Promise) { try { return await check; @@ -33,6 +79,78 @@ export async function canViewBoardEntities( return results.every(Boolean); } +export async function hasValidBoardReports( + type: string | undefined, + parameters: BoardParameters = {}, +) { + const board = { type, parameters }; + const reportCache = new Map>(); + + for (const row of parameters.rows ?? []) { + for (const column of row.columns ?? []) { + const component = column.component; + if (!component) { + continue; + } + + if (!(await isValidBoardReportReference(board, component, reportCache))) { + return false; + } + } + } + + return true; +} + +export async function stripInvalidBoardReports( + type: string | undefined, + parameters: BoardParameters = {}, +) { + const board = { type, parameters }; + const reportCache = new Map>(); + let hasChanges = false; + + const rows = await Promise.all( + (parameters.rows ?? []).map(async row => { + const columns = await Promise.all( + row.columns.map(async column => { + const component = column.component; + + if (!component || (await isValidBoardReportReference(board, component, reportCache))) { + return column; + } + + hasChanges = true; + + const { reportId: _reportId, ...nextProps } = component.props ?? {}; + + return { + ...column, + component: { + ...component, + props: Object.keys(nextProps).length > 0 ? nextProps : undefined, + }, + }; + }), + ); + + return { + ...row, + columns, + }; + }), + ); + + if (!hasChanges) { + return parameters; + } + + return { + ...parameters, + rows, + }; +} + export async function canViewBoard({ user, shareToken }: Auth, boardId: string) { if (user?.isAdmin) { return true;