From 81608420496a4b8a72781dd21fd88c85173e7447 Mon Sep 17 00:00:00 2001
From: Val Alexander <bunsthedev@gmail.com>
Date: Fri, 5 Jun 2026 16:42:24 -0700
Subject: [PATCH 1/2] fix: harden help center MCP and JSON-LD

---
 apps/web/src/components/json-ld.tsx           |  4 +++-
 apps/web/src/lib/server/mcp/tools.ts          | 20 ++++++++++++++++---
 .../__tests__/json-ld-serialization.test.ts   | 16 +++++++++++++++
 .../src/lib/shared/json-ld-serialization.ts   | 14 +++++++++++++
 4 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 apps/web/src/lib/shared/__tests__/json-ld-serialization.test.ts
 create mode 100644 apps/web/src/lib/shared/json-ld-serialization.ts

diff --git a/apps/web/src/components/json-ld.tsx b/apps/web/src/components/json-ld.tsx
index a74cbea86..9db28b5ac 100644
--- a/apps/web/src/components/json-ld.tsx
+++ b/apps/web/src/components/json-ld.tsx
@@ -1,9 +1,11 @@
+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) }} />
   )
 }
diff --git a/apps/web/src/lib/server/mcp/tools.ts b/apps/web/src/lib/server/mcp/tools.ts
index bf5205da9..0f87464a3 100644
--- a/apps/web/src/lib/server/mcp/tools.ts
+++ b/apps/web/src/lib/server/mcp/tools.ts
@@ -68,7 +68,7 @@ import {
   removePostFromRoadmap,
 } from '@/lib/server/domains/roadmaps/roadmap.service'
 import { getTypeIdPrefix, isTypeId, isValidTypeId } from '@opencoven-feedback/ids'
-import { isTeamMember } from '@/lib/shared/roles'
+import { isAdmin, isTeamMember } from '@/lib/shared/roles'
 import { truncate } from '@/lib/shared/utils/string'
 import {
   listArticles,
@@ -189,10 +189,24 @@ async function requireHelpCenter(): Promise<CallToolResult | null> {
   }
 }
 
-/** Combined gate: feature flag + scope + team role for help center write tools. */
+/** Return an error if the user doesn't have an admin role. */
+function requireAdminRole(auth: McpAuthContext): CallToolResult | null {
+  if (isAdmin(auth.role)) return null
+  return {
+    isError: true,
+    content: [
+      {
+        type: 'text',
+        text: 'Error: This operation requires an admin role.',
+      },
+    ],
+  }
+}
+
+/** Combined gate: feature flag + scope + admin role for help center write tools. */
 async function requireHelpCenterWrite(auth: McpAuthContext): Promise<CallToolResult | null> {
   return (
-    (await requireHelpCenter()) ?? requireScope(auth, 'write:help-center') ?? requireTeamRole(auth)
+    (await requireHelpCenter()) ?? requireScope(auth, 'write:help-center') ?? requireAdminRole(auth)
   )
 }
 
diff --git a/apps/web/src/lib/shared/__tests__/json-ld-serialization.test.ts b/apps/web/src/lib/shared/__tests__/json-ld-serialization.test.ts
new file mode 100644
index 000000000..31234f7f7
--- /dev/null
+++ b/apps/web/src/lib/shared/__tests__/json-ld-serialization.test.ts
@@ -0,0 +1,16 @@
+import { describe, expect, it } from 'vitest'
+import { serializeJsonLd } from '../json-ld-serialization'
+
+describe('serializeJsonLd', () => {
+  it('escapes script-breaking characters while preserving JSON values', () => {
+    const payload = '</script><script>alert(1)</script>&'
+
+    const result = serializeJsonLd({ headline: payload })
+
+    expect(result).not.toContain('</script>')
+    expect(result).not.toContain('<script>')
+    expect(result).toContain('\\u003c/script\\u003e\\u003cscript\\u003e')
+    expect(result).toContain('\\u0026')
+    expect(JSON.parse(result).headline).toBe(payload)
+  })
+})
diff --git a/apps/web/src/lib/shared/json-ld-serialization.ts b/apps/web/src/lib/shared/json-ld-serialization.ts
new file mode 100644
index 000000000..20349d163
--- /dev/null
+++ b/apps/web/src/lib/shared/json-ld-serialization.ts
@@ -0,0 +1,14 @@
+export function serializeJsonLd(data: Record<string, unknown>): string {
+  return JSON.stringify(data).replace(/[<>&]/g, (character) => {
+    switch (character) {
+      case '<':
+        return '\\u003c'
+      case '>':
+        return '\\u003e'
+      case '&':
+        return '\\u0026'
+      default:
+        return character
+    }
+  })
+}

From cbf61937083a1d28b7012dbce7d9701f088e93d8 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 6 Jun 2026 19:09:08 +0000
Subject: [PATCH 2/2] test: add MCP help-center admin gate regression coverage

---
 apps/web/src/components/json-ld.tsx           |   5 +-
 .../lib/server/mcp/__tests__/handler.test.ts  | 200 ++++++++++++++++++
 2 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/apps/web/src/components/json-ld.tsx b/apps/web/src/components/json-ld.tsx
index 9db28b5ac..8beeab1c1 100644
--- a/apps/web/src/components/json-ld.tsx
+++ b/apps/web/src/components/json-ld.tsx
@@ -6,6 +6,9 @@ import { serializeJsonLd } from '@/lib/shared/json-ld-serialization'
  */
 export function JsonLd({ data }: { data: Record<string, unknown> }) {
   return (
-    <script type="application/ld+json" dangerouslySetInnerHTML={{ __html: serializeJsonLd(data) }} />
+    <script
+      type="application/ld+json"
+      dangerouslySetInnerHTML={{ __html: serializeJsonLd(data) }}
+    />
   )
 }
diff --git a/apps/web/src/lib/server/mcp/__tests__/handler.test.ts b/apps/web/src/lib/server/mcp/__tests__/handler.test.ts
index 54efdba70..0b53b9576 100644
--- a/apps/web/src/lib/server/mcp/__tests__/handler.test.ts
+++ b/apps/web/src/lib/server/mcp/__tests__/handler.test.ts
@@ -52,6 +52,7 @@ vi.mock('@/lib/server/domains/settings/settings.service', () => ({
     .fn()
     .mockResolvedValue({ mcpEnabled: true, mcpPortalAccessEnabled: false }),
   getTenantSettings: vi.fn().mockResolvedValue(null),
+  isFeatureEnabled: vi.fn().mockResolvedValue(true),
 }))
 
 // Mock config so baseUrl is available (used in WWW-Authenticate header)
@@ -213,6 +214,128 @@ vi.mock('@/lib/server/domains/changelog/changelog.query', () => ({
   }),
 }))
 
+vi.mock('@/lib/server/domains/help-center/help-center.service', () => ({
+  listArticles: vi.fn().mockResolvedValue({ items: [], nextCursor: null, hasMore: false }),
+  getArticleById: vi.fn().mockResolvedValue({
+    id: 'article_test',
+    slug: 'getting-started',
+    title: 'Getting Started',
+    content: 'Welcome',
+    description: null,
+    position: 1,
+    category: { id: 'category_test', slug: 'guides', name: 'Guides' },
+    author: { id: 'principal_test', name: 'Jane Admin', avatarUrl: null },
+    publishedAt: null,
+    viewCount: 0,
+    helpfulCount: 0,
+    notHelpfulCount: 0,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-01'),
+  }),
+  getCategoryById: vi.fn().mockResolvedValue({
+    id: 'category_test',
+    slug: 'guides',
+    name: 'Guides',
+    description: null,
+    icon: '📚',
+    parentId: null,
+    isPublic: true,
+    position: 1,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-01'),
+  }),
+  createArticle: vi.fn().mockResolvedValue({
+    id: 'article_test',
+    slug: 'getting-started',
+    title: 'Getting Started',
+    content: 'Welcome',
+    description: null,
+    position: 1,
+    category: { id: 'category_test', slug: 'guides', name: 'Guides' },
+    author: { id: 'principal_test', name: 'Jane Admin', avatarUrl: null },
+    publishedAt: null,
+    viewCount: 0,
+    helpfulCount: 0,
+    notHelpfulCount: 0,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-01'),
+  }),
+  updateArticle: vi.fn().mockResolvedValue({
+    id: 'article_test',
+    slug: 'getting-started',
+    title: 'Updated',
+    content: 'Updated content',
+    description: null,
+    position: 1,
+    category: { id: 'category_test', slug: 'guides', name: 'Guides' },
+    author: { id: 'principal_test', name: 'Jane Admin', avatarUrl: null },
+    publishedAt: null,
+    viewCount: 0,
+    helpfulCount: 0,
+    notHelpfulCount: 0,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-02'),
+  }),
+  publishArticle: vi.fn().mockResolvedValue({
+    id: 'article_test',
+    slug: 'getting-started',
+    title: 'Getting Started',
+    content: 'Welcome',
+    description: null,
+    position: 1,
+    category: { id: 'category_test', slug: 'guides', name: 'Guides' },
+    author: { id: 'principal_test', name: 'Jane Admin', avatarUrl: null },
+    publishedAt: new Date('2026-01-02'),
+    viewCount: 0,
+    helpfulCount: 0,
+    notHelpfulCount: 0,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-02'),
+  }),
+  unpublishArticle: vi.fn().mockResolvedValue({
+    id: 'article_test',
+    slug: 'getting-started',
+    title: 'Getting Started',
+    content: 'Welcome',
+    description: null,
+    position: 1,
+    category: { id: 'category_test', slug: 'guides', name: 'Guides' },
+    author: { id: 'principal_test', name: 'Jane Admin', avatarUrl: null },
+    publishedAt: null,
+    viewCount: 0,
+    helpfulCount: 0,
+    notHelpfulCount: 0,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-02'),
+  }),
+  deleteArticle: vi.fn().mockResolvedValue(undefined),
+  createCategory: vi.fn().mockResolvedValue({
+    id: 'category_test',
+    slug: 'guides',
+    name: 'Guides',
+    description: null,
+    icon: '📚',
+    parentId: null,
+    isPublic: true,
+    position: 1,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-01'),
+  }),
+  updateCategory: vi.fn().mockResolvedValue({
+    id: 'category_test',
+    slug: 'guides',
+    name: 'Updated Guides',
+    description: null,
+    icon: '📚',
+    parentId: null,
+    isPublic: true,
+    position: 1,
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-02'),
+  }),
+  deleteCategory: vi.fn().mockResolvedValue(undefined),
+}))
+
 vi.mock('@/lib/server/domains/changelog/changelog.service', () => ({
   createChangelog: vi.fn().mockResolvedValue({
     id: 'changelog_new',
@@ -1677,6 +1800,83 @@ describe('MCP HTTP Handler', () => {
       expect(body.result.content[0].text).toContain('team member')
     })
 
+    it('should deny help center write tools for non-admin OAuth user with write:help-center', async () => {
+      const handleMcpRequest = await initializeOAuthSession(['write:help-center'])
+      await setupValidOAuth({
+        role: 'member',
+        scopes: ['write:help-center'],
+      })
+
+      const requests = [
+        {
+          name: 'create_article',
+          arguments: { categoryId: 'category_test', title: 'Getting Started', content: 'Welcome' },
+        },
+        {
+          name: 'update_article',
+          arguments: { articleId: 'article_test', title: 'Updated' },
+        },
+        {
+          name: 'manage_category',
+          arguments: { action: 'create', name: 'Guides' },
+        },
+      ] as const
+
+      for (const request of requests) {
+        const response = await handleMcpRequest(
+          oauthRequest(
+            jsonRpcRequest('tools/call', {
+              name: request.name,
+              arguments: request.arguments,
+            })
+          )
+        )
+
+        expect(response.status).toBe(200)
+        const body = (await response.json()) as {
+          result: { isError: boolean; content: Array<{ text: string }> }
+        }
+        expect(body.result.isError).toBe(true)
+        expect(body.result.content[0].text).toContain('admin role')
+      }
+    })
+
+    it('should allow help center write tools for admin OAuth user with write:help-center', async () => {
+      const handleMcpRequest = await initializeOAuthSession(['write:help-center'])
+
+      const requests = [
+        {
+          name: 'create_article',
+          arguments: { categoryId: 'category_test', title: 'Getting Started', content: 'Welcome' },
+        },
+        {
+          name: 'update_article',
+          arguments: { articleId: 'article_test', title: 'Updated' },
+        },
+        {
+          name: 'manage_category',
+          arguments: { action: 'create', name: 'Guides' },
+        },
+      ] as const
+
+      for (const request of requests) {
+        const response = await handleMcpRequest(
+          oauthRequest(
+            jsonRpcRequest('tools/call', {
+              name: request.name,
+              arguments: request.arguments,
+            })
+          )
+        )
+
+        expect(response.status).toBe(200)
+        const body = (await response.json()) as {
+          result: { isError?: boolean; content: Array<{ text: string }> }
+        }
+        expect(body.result.isError).toBeUndefined()
+      }
+    })
+
     it('should deny delete_post when write:feedback scope missing', async () => {
       const handleMcpRequest = await initializeOAuthSession(['read:feedback'])