security: data exposure & privacy hardening (6 fixes)

apps/web/src/components/json-ld.tsx

@@ -1,9 +1,14 @@
+import { serializeJsonLd } from '@/lib/shared/json-ld-serialization'
+
 /**
  * JsonLd - Renders a <script type="application/ld+json"> tag for structured data.
  * Use inside component bodies since TanStack Router head() doesn't support script injection.
  */
 export function JsonLd({ data }: { data: Record<string, unknown> }) {
   return (
-    <script type="application/ld+json" dangerouslySetInnerHTML={{ __html: JSON.stringify(data) }} />
+    <script
+      type="application/ld+json"
+      dangerouslySetInnerHTML={{ __html: serializeJsonLd(data) }}
+    />
   )
 }

apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts

@@ -4,6 +4,7 @@ import type { HelpCenterArticleId } from '@opencoven-feedback/ids'
 const mockArticleFindFirst = vi.fn()
 const mockArticleFindMany = vi.fn()
 const mockCategoryFindMany = vi.fn()
+const mockCategoryFindFirst = vi.fn()
 
 vi.mock('@/lib/server/db', () => ({
   db: {
@@ -13,6 +14,7 @@ vi.mock('@/lib/server/db', () => ({
         findMany: (...args: unknown[]) => mockArticleFindMany(...args),
       },
       helpCenterCategories: {
+        findFirst: (...args: unknown[]) => mockCategoryFindFirst(...args),
         findMany: (...args: unknown[]) => mockCategoryFindMany(...args),
       },
       principal: {
@@ -82,6 +84,7 @@ let listPublicArticlesForCategory: typeof import('../help-center.article.query')
 
 beforeEach(async () => {
   vi.clearAllMocks()
+  mockCategoryFindFirst.mockResolvedValue({ id: 'category_1' })
 
   const mod = await import('../help-center.article.query')
   listArticles = mod.listArticles
@@ -115,8 +118,7 @@ describe('listPublicArticlesForCategory', () => {
     const orderByMock = vi.fn().mockResolvedValue(mockArticles)
     const whereMock = vi.fn().mockReturnValue({ orderBy: orderByMock })
     const leftJoinMock = vi.fn().mockReturnValue({ where: whereMock })
-    const innerJoinMock = vi.fn().mockReturnValue({ leftJoin: leftJoinMock })
-    const fromMock = vi.fn().mockReturnValue({ innerJoin: innerJoinMock })
+    const fromMock = vi.fn().mockReturnValue({ leftJoin: leftJoinMock })
     vi.mocked(db.select).mockReturnValueOnce({ from: fromMock } as never)
 
     const result = await listPublicArticlesForCategory('category_1')
@@ -135,8 +137,7 @@ describe('listPublicArticlesForCategory', () => {
     const orderByMock = vi.fn().mockResolvedValue([])
     const whereMock = vi.fn().mockReturnValue({ orderBy: orderByMock })
     const leftJoinMock = vi.fn().mockReturnValue({ where: whereMock })
-    const innerJoinMock = vi.fn().mockReturnValue({ leftJoin: leftJoinMock })
-    const fromMock = vi.fn().mockReturnValue({ innerJoin: innerJoinMock })
+    const fromMock = vi.fn().mockReturnValue({ leftJoin: leftJoinMock })
     vi.mocked(db.select).mockReturnValueOnce({ from: fromMock } as never)
 
     const result = await listPublicArticlesForCategory('category_1')
@@ -145,25 +146,14 @@ describe('listPublicArticlesForCategory', () => {
 })
 
 describe('listPublicArticles', () => {
-  it('returns no articles when the requested category is not public', async () => {
-    mockCategoryFindMany.mockResolvedValue([{ id: 'category_public' }])
+  it('does not return articles when no public categories exist', async () => {
+    mockCategoryFindMany.mockResolvedValueOnce([])
 
-    const result = await listPublicArticles({ categoryId: 'category_hidden' })
+    const result = await listPublicArticles({ search: 'private' })
 
     expect(result).toEqual({ items: [], nextCursor: null, hasMore: false })
     expect(mockArticleFindMany).not.toHaveBeenCalled()
   })
-
-  it('restricts public article listings to public category IDs', async () => {
-    const { inArray } = await import('@/lib/server/db')
-    mockCategoryFindMany.mockResolvedValue([{ id: 'category_public' }])
-    mockArticleFindMany.mockResolvedValue([])
-
-    await listPublicArticles({ limit: 20 })
-
-    expect(mockArticleFindMany).toHaveBeenCalled()
-    expect(inArray).toHaveBeenCalledWith('category_id', ['category_public'])
-  })
 })
 
 describe('listArticles with showDeleted option', () => {

apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts

@@ -15,7 +15,6 @@ function createUpdateChain() {
     updateWhereCalls.push(args)
     return chain
   })
-  chain.catch = vi.fn().mockResolvedValue(undefined)
   chain.returning = vi.fn().mockResolvedValue([
     {
       id: 'article_1' as HelpCenterArticleId,
@@ -128,14 +127,14 @@ vi.mock('@/lib/server/markdown-tiptap', () => ({
 }))
 
 let getArticleById: typeof import('../help-center.article.service').getArticleById
+let getPublicArticleBySlug: typeof import('../help-center.article.service').getPublicArticleBySlug
 let createArticle: typeof import('../help-center.article.service').createArticle
 let updateArticle: typeof import('../help-center.article.service').updateArticle
 let publishArticle: typeof import('../help-center.article.service').publishArticle
 let unpublishArticle: typeof import('../help-center.article.service').unpublishArticle
 let deleteArticle: typeof import('../help-center.article.service').deleteArticle
 let restoreArticle: typeof import('../help-center.article.service').restoreArticle
 let recordArticleFeedback: typeof import('../help-center.article.service').recordArticleFeedback
-let getPublicArticleBySlug: typeof import('../help-center.article.service').getPublicArticleBySlug
 
 beforeEach(async () => {
   vi.clearAllMocks()
@@ -145,14 +144,14 @@ beforeEach(async () => {
 
   const mod = await import('../help-center.article.service')
   getArticleById = mod.getArticleById
+  getPublicArticleBySlug = mod.getPublicArticleBySlug
   createArticle = mod.createArticle
   updateArticle = mod.updateArticle
   publishArticle = mod.publishArticle
   unpublishArticle = mod.unpublishArticle
   deleteArticle = mod.deleteArticle
   restoreArticle = mod.restoreArticle
   recordArticleFeedback = mod.recordArticleFeedback
-  getPublicArticleBySlug = mod.getPublicArticleBySlug
 })
 
 describe('getArticleById', () => {
@@ -201,41 +200,28 @@ describe('getArticleById', () => {
 })
 
 describe('getPublicArticleBySlug', () => {
-  const publishedArticle = {
-    id: 'article_1' as HelpCenterArticleId,
-    slug: 'how-to-start',
-    title: 'How to Start',
-    content: 'Content here',
-    contentJson: null,
-    categoryId: 'category_1',
-    principalId: null,
-    publishedAt: new Date(),
-    viewCount: 5,
-    helpfulCount: 2,
-    notHelpfulCount: 0,
-    createdAt: new Date(),
-    updatedAt: new Date(),
-  }
-
-  it('returns a published article when its category is public', async () => {
-    mockArticleFindFirst.mockResolvedValue(publishedArticle)
-    mockCategoryFindFirst
-      .mockResolvedValueOnce({ id: 'category_1' })
-      .mockResolvedValueOnce({ id: 'category_1', slug: 'getting-started', name: 'Getting Started' })
-    mockPrincipalFindFirst.mockResolvedValue(null)
-
-    const result = await getPublicArticleBySlug('how-to-start')
-
-    expect(result.title).toBe('How to Start')
-  })
-
-  it('throws NotFoundError when the article category is not public', async () => {
-    mockArticleFindFirst.mockResolvedValue(publishedArticle)
+  it('throws NotFoundError when the published article belongs to a private category', async () => {
+    mockArticleFindFirst.mockResolvedValue({
+      id: 'article_1' as HelpCenterArticleId,
+      slug: 'private-article',
+      title: 'Private Article',
+      content: 'Private content',
+      contentJson: null,
+      categoryId: 'category_private',
+      principalId: 'principal_1',
+      publishedAt: new Date(),
+      viewCount: 0,
+      helpfulCount: 0,
+      notHelpfulCount: 0,
+      createdAt: new Date(),
+      updatedAt: new Date(),
+    })
     mockCategoryFindFirst.mockResolvedValue(null)
 
-    await expect(getPublicArticleBySlug('how-to-start')).rejects.toMatchObject({
+    await expect(getPublicArticleBySlug('private-article')).rejects.toMatchObject({
       code: 'ARTICLE_NOT_FOUND',
     })
+    expect(updateSetCalls).toHaveLength(0)
   })
 })
 
@@ -361,7 +347,6 @@ describe('createArticle', () => {
     const { db } = await import('@/lib/server/db')
     const chain: Record<string, unknown> = {}
     chain.values = vi.fn(() => chain)
-    chain.catch = vi.fn().mockResolvedValue(undefined)
     chain.returning = vi.fn().mockResolvedValue([
       {
         id: 'article_new' as HelpCenterArticleId,
@@ -527,7 +512,6 @@ describe('updateArticle authorId validation', () => {
     const chain: Record<string, unknown> = {}
     chain.set = vi.fn(() => chain)
     chain.where = vi.fn(() => chain)
-    chain.catch = vi.fn().mockResolvedValue(undefined)
     chain.returning = vi.fn().mockResolvedValue([
       {
         id: 'article_1' as HelpCenterArticleId,
@@ -569,7 +553,6 @@ describe('updateArticle authorId validation', () => {
     const chain: Record<string, unknown> = {}
     chain.set = vi.fn(() => chain)
     chain.where = vi.fn(() => chain)
-    chain.catch = vi.fn().mockResolvedValue(undefined)
     chain.returning = vi.fn().mockResolvedValue([
       {
         id: 'article_1' as HelpCenterArticleId,
@@ -688,7 +671,6 @@ describe('restoreArticle', () => {
       return chain
     })
     chain.where = vi.fn().mockReturnValue(chain)
-    chain.catch = vi.fn().mockResolvedValue(undefined)
     chain.returning = vi.fn().mockResolvedValue([
       {
         id: 'article_1' as HelpCenterArticleId,

apps/web/src/lib/server/domains/help-center/help-center-search.service.ts

@@ -14,6 +14,7 @@ import {
   isNull,
   isNotNull,
   sql,
+  eq,
 } from '@/lib/server/db'
 import { generateKbEmbedding } from './help-center-embedding.service'
 
@@ -103,6 +104,7 @@ async function hybridQuery(
       and(
         isNotNull(helpCenterArticles.publishedAt),
         isNull(helpCenterArticles.deletedAt),
+        eq(helpCenterCategories.isPublic, true),
         isNull(helpCenterCategories.deletedAt),
         sql`(
           ${helpCenterArticles.searchVector} @@ ${tsQuery}
@@ -156,6 +158,7 @@ async function keywordOnlyQuery(query: string, limit: number): Promise<HybridSea
       and(
         isNotNull(helpCenterArticles.publishedAt),
         isNull(helpCenterArticles.deletedAt),
+        eq(helpCenterCategories.isPublic, true),
         isNull(helpCenterCategories.deletedAt),
         sql`${helpCenterArticles.searchVector} @@ ${tsQuery}`
       )

apps/web/src/lib/server/domains/help-center/help-center.article.query.ts

@@ -31,18 +31,26 @@ import type {
 // Article Queries
 // ============================================================================
 
-export async function listArticles(
-  params: ListArticlesParams & { categoryIds?: HelpCenterCategoryId[] }
-): Promise<ArticleListResult> {
+type InternalListArticlesParams = ListArticlesParams & { publicOnly?: boolean }
+
+async function listPublicCategoryIds(): Promise<HelpCenterCategoryId[]> {
+  const categories = await db.query.helpCenterCategories.findMany({
+    where: and(eq(helpCenterCategories.isPublic, true), isNull(helpCenterCategories.deletedAt)),
+    columns: { id: true },
+  })
+  return categories.map((cat) => cat.id as HelpCenterCategoryId)
+}
+
+export async function listArticles(params: InternalListArticlesParams): Promise<ArticleListResult> {
   const {
     categoryId,
-    categoryIds: categoryIdsFilter,
     status = 'all',
     search,
     cursor,
     limit = 20,
     showDeleted = false,
     sort = 'newest',
+    publicOnly = false,
   } = params
   const now = new Date()
 
@@ -55,12 +63,22 @@ export async function listArticles(
       ]
     : [isNull(helpCenterArticles.deletedAt)]
 
+  if (publicOnly) {
+    const publicCategoryIds = await listPublicCategoryIds()
+
+    if (categoryId && !publicCategoryIds.includes(categoryId as HelpCenterCategoryId)) {
+      return { items: [], nextCursor: null, hasMore: false }
+    }
+
+    if (publicCategoryIds.length === 0) {
+      return { items: [], nextCursor: null, hasMore: false }
+    }
+
+    conditions.push(inArray(helpCenterArticles.categoryId, publicCategoryIds))
+  }
+
   if (categoryId) {
     conditions.push(eq(helpCenterArticles.categoryId, categoryId as HelpCenterCategoryId))
-  } else if (categoryIdsFilter?.length) {
-    conditions.push(
-      inArray(helpCenterArticles.categoryId, categoryIdsFilter as HelpCenterCategoryId[])
-    )
   }
 
   if (!showDeleted) {
@@ -189,39 +207,25 @@ export async function listArticles(
   }
 }
 
-async function listPublicCategoryIds(): Promise<HelpCenterCategoryId[]> {
-  const categories = await db.query.helpCenterCategories.findMany({
-    where: and(eq(helpCenterCategories.isPublic, true), isNull(helpCenterCategories.deletedAt)),
-    columns: { id: true },
-  })
-  return categories.map((category) => category.id as HelpCenterCategoryId)
-}
-
 export async function listPublicArticles(params: {
   categoryId?: string
   search?: string
   cursor?: string
   limit?: number
 }): Promise<ArticleListResult> {
-  const publicCategoryIds = await listPublicCategoryIds()
-
-  if (params.categoryId && !publicCategoryIds.includes(params.categoryId as HelpCenterCategoryId)) {
-    return { items: [], nextCursor: null, hasMore: false }
-  }
-
-  if (!params.categoryId && publicCategoryIds.length === 0) {
-    return { items: [], nextCursor: null, hasMore: false }
-  }
-
-  return listArticles({
-    ...params,
-    categoryIds: params.categoryId ? undefined : publicCategoryIds,
-    status: 'published',
-  })
+  return listArticles({ ...params, status: 'published', publicOnly: true })
 }
 
 export async function listPublicArticlesForCategory(categoryId: string) {
-  const now = new Date()
+  const category = await db.query.helpCenterCategories.findFirst({
+    where: and(
+      eq(helpCenterCategories.id, categoryId as HelpCenterCategoryId),
+      eq(helpCenterCategories.isPublic, true),
+      isNull(helpCenterCategories.deletedAt)
+    ),
+    columns: { id: true },
+  })
+  if (!category) return []
 
   return db
     .select({
@@ -236,15 +240,11 @@ export async function listPublicArticlesForCategory(categoryId: string) {
       authorAvatarUrl: principal.avatarUrl,
     })
     .from(helpCenterArticles)
-    .innerJoin(helpCenterCategories, eq(helpCenterCategories.id, helpCenterArticles.categoryId))
     .leftJoin(principal, eq(principal.id, helpCenterArticles.principalId))
     .where(
       and(
         eq(helpCenterArticles.categoryId, categoryId as HelpCenterCategoryId),
-        eq(helpCenterCategories.isPublic, true),
-        isNull(helpCenterCategories.deletedAt),
         isNotNull(helpCenterArticles.publishedAt),
-        lte(helpCenterArticles.publishedAt, now),
         isNull(helpCenterArticles.deletedAt)
       )
     )
@@ -254,7 +254,8 @@ export async function listPublicArticlesForCategory(categoryId: string) {
 export async function listPublicCategoryEditors(): Promise<
   Record<string, Array<{ name: string; avatarUrl: string | null }>>
 > {
-  const now = new Date()
+  const publicCategoryIds = await listPublicCategoryIds()
+  if (publicCategoryIds.length === 0) return {}
 
   const rows = await db
     .select({
@@ -264,15 +265,12 @@ export async function listPublicCategoryEditors(): Promise<
       avatarUrl: principal.avatarUrl,
     })
     .from(helpCenterArticles)
-    .innerJoin(helpCenterCategories, eq(helpCenterCategories.id, helpCenterArticles.categoryId))
     .innerJoin(principal, eq(principal.id, helpCenterArticles.principalId))
     .where(
       and(
-        eq(helpCenterCategories.isPublic, true),
-        isNull(helpCenterCategories.deletedAt),
         isNotNull(helpCenterArticles.publishedAt),
-        lte(helpCenterArticles.publishedAt, now),
         isNull(helpCenterArticles.deletedAt),
+        inArray(helpCenterArticles.categoryId, publicCategoryIds),
         inArray(principal.role, ['admin', 'member'])
       )
     )