security: data exposure & privacy hardening (6 fixes)#44
Merged
Conversation
# Conflicts: # apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts # apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts
# Conflicts: # apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.query.test.ts # apps/web/src/lib/server/domains/help-center/__tests__/help-center-article.service.test.ts # apps/web/src/lib/server/domains/help-center/help-center.article.query.ts # apps/web/src/lib/server/domains/help-center/help-center.article.service.ts # apps/web/src/lib/server/functions/help-center.ts
# Conflicts: # apps/web/src/lib/server/functions/help-center.ts
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Security/privacy hardening across the web app by preventing public exposure of private help center / post merge metadata, tightening MCP authorization for help center write tools, and mitigating stored XSS via safer JSON-LD serialization.
Changes:
- Add JSON-LD serialization helper that escapes script-breaking characters and use it in the JsonLd component (with tests).
- Require admin role for MCP help center write tools and add handler tests for the new gate.
- Restrict public portal/help center queries to public categories/boards and add public-only merge helpers + tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/lib/shared/json-ld-serialization.ts | New safe JSON-LD serializer that escapes HTML-significant characters. |
| apps/web/src/lib/shared/tests/json-ld-serialization.test.ts | Adds unit coverage for JSON-LD escaping behavior. |
| apps/web/src/lib/server/mcp/tools.ts | Tightens help center write tools to require admin role (not just team member). |
| apps/web/src/lib/server/mcp/tests/handler.test.ts | Adds mocks + tests validating new admin gating behavior. |
| apps/web/src/lib/server/functions/portal.ts | Switches public portal merge reads to public-only helpers. |
| apps/web/src/lib/server/domains/posts/post.public.detail.ts | Ensures merged/related post resolution in public detail only includes public boards. |
| apps/web/src/lib/server/domains/posts/post.merge.ts | Adds merge visibility compatibility check, plus public-only merge lookup helpers. |
| apps/web/src/lib/server/domains/posts/tests/post-merge.test.ts | Adds tests for merge visibility compatibility + public-only merge helpers. |
| apps/web/src/lib/server/domains/help-center/help-center.article.service.ts | Minor refactor while enforcing public-category gating for public article access. |
| apps/web/src/lib/server/domains/help-center/help-center.article.query.ts | Refactors public listing logic to apply public-category filtering in listArticles. |
| apps/web/src/lib/server/domains/help-center/help-center-search.service.ts | Adds category public visibility constraints to help center search queries. |
| apps/web/src/lib/server/domains/help-center/tests/help-center-article.service.test.ts | Updates tests aligning with public-category gating changes. |
| apps/web/src/lib/server/domains/help-center/tests/help-center-article.query.test.ts | Updates tests for refactored public listing behavior. |
| apps/web/src/components/json-ld.tsx | Uses new serializer to prevent JSON-LD script-breakout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
244
to
250
| .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) | ||
| ) | ||
| ) |
Comment on lines
269
to
276
| .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']) | ||
| ) | ||
| ) |
Comment on lines
+369
to
+376
| .innerJoin(boards, eq(posts.boardId, boards.id)) | ||
| .where( | ||
| and( | ||
| eq(posts.canonicalPostId, canonicalPostId), | ||
| isNull(posts.deletedAt), | ||
| eq(boards.isPublic, true) | ||
| ) | ||
| ) |
Comment on lines
+363
to
+366
| authorName: sql<string | null>`( | ||
| SELECT m.display_name FROM ${principalTable} m | ||
| WHERE m.id = ${posts.principalId} | ||
| )`.as('author_name'), |
| @@ -0,0 +1,14 @@ | |||
| export function serializeJsonLd(data: Record<string, unknown>): string { | |||
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
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.
Batches 6 Codex security fixes: private feedback leak, public help center exposure, private help center articles exposure, authenticated access, private post metadata, and stored XSS in JSON-LD.