-
Notifications
You must be signed in to change notification settings - Fork 119
Security: Add XSS sanitization to chat messages #1877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { supabase } from "@/integrations/supabase/client"; | |
| import { useAwardXP } from "@/hooks/useAwardXP"; | ||
| import { toast } from "@/hooks/use-toast"; | ||
| import { logError } from "@/utils/logger"; | ||
| import { sanitizeMessageContent } from "@/utils/sanitize"; | ||
|
|
||
| export type ProfileSummary = { | ||
| id: string; | ||
|
|
@@ -577,13 +578,14 @@ export function useMessages( | |
| } | ||
|
|
||
| try { | ||
| const sanitizedContent = sanitizeMessageContent(content); | ||
| const { data, error: insertError } = await supabase | ||
| .from("messages") | ||
| .insert({ | ||
| sender_id: currentUserId, | ||
| receiver_id: selectedUser.id, | ||
| content, | ||
| text: content, | ||
| content: sanitizedContent, | ||
| text: sanitizedContent, | ||
|
Comment on lines
+581
to
+588
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Validate the sanitized value before persisting. Raw non-empty checks allow content consisting only of removable markup to become empty messages.
📍 Affects 3 files
🤖 Prompt for AI Agents |
||
| }) | ||
| .select("id,sender_id,receiver_id,content,text,message,created_at,read_at") | ||
| .single(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import DOMPurify from 'dompurify'; | ||
|
|
||
| /** | ||
| * Configuration for DOMPurify sanitization. | ||
| * Allows basic text formatting while preventing XSS attacks. | ||
| */ | ||
| const SANITIZE_CONFIG = { | ||
| ALLOWED_TAGS: ['b', 'i', 'em', 'strong', 'p', 'br', 'ul', 'ol', 'li', 'code', 'pre', 'blockquote'], | ||
| ALLOWED_ATTR: [], | ||
| KEEP_CONTENT: true, | ||
| }; | ||
|
|
||
| /** | ||
| * Sanitizes user input to prevent XSS attacks. | ||
| * Removes potentially dangerous HTML/JavaScript while preserving basic formatting. | ||
| * | ||
| * @param input - The raw user input to sanitize | ||
| * @returns Sanitized string safe for display | ||
| */ | ||
| export const sanitizeInput = (input: string): string => { | ||
| if (!input || typeof input !== 'string') { | ||
| return ''; | ||
| } | ||
|
|
||
| // First pass: DOMPurify sanitization | ||
| const sanitized = DOMPurify.sanitize(input, SANITIZE_CONFIG); | ||
|
|
||
| // Second pass: Remove any remaining dangerous content | ||
| return sanitized.replace(/<script[^>]*>.*?<\/script>/gi, '').replace(/on\w+\s*=/gi, ''); | ||
|
Comment on lines
+20
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: rg -n 'react-markdown|remark|marked|dangerouslySetInnerHTML|innerHTML' srcRepository: durdana3105/peer-learning Length of output: 483 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | sed -n '1,120p'
printf '\n--- MarkdownRenderer.tsx ---\n'
cat -n src/components/MarkdownRenderer.tsx
printf '\n--- sanitize.ts ---\n'
cat -n src/utils/sanitize.ts
printf '\n--- usages of sanitizeInput and MarkdownRenderer ---\n'
rg -n 'sanitizeInput|MarkdownRenderer' src
printf '\n--- package react markdown versions ---\n'
if [ -f package.json ]; then cat package.json | sed -n '1,220p'; fiRepository: durdana3105/peer-learning Length of output: 254 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files | sed -n '1,160p'
echo
echo '--- MarkdownRenderer.tsx ---'
cat -n src/components/MarkdownRenderer.tsx
echo
echo '--- sanitize.ts ---'
cat -n src/utils/sanitize.ts
echo
echo '--- usages of sanitizeInput and MarkdownRenderer ---'
rg -n 'sanitizeInput|MarkdownRenderer' src
echo
echo '--- package react markdown versions ---'
if [ -f package.json ]; then cat package.json | sed -n '1,220p'; fiRepository: durdana3105/peer-learning Length of output: 13847 Sanitize after Markdown/HTML transformation.
🤖 Prompt for AI Agents🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,160p'
echo "== sanitize paths =="
fd -i 'sanitize|supabase|rls|policy|rds|function|rpc|edge|trigger' . | sed -n '1,160p'
echo "== sanitize.ts =="
if [ -f src/utils/sanitize.ts ]; then
nl -ba src/utils/sanitize.ts | sed -n '1,120p'
fi
echo "== usages of sanitizeInput and sanitizeText =="
rg -n "sanitizeInput|sanitizeText|SANITIZE_CONFIG|DOMPurify" . -g '!node_modules' -g '!dist' -g '!build' || true
echo "== supabase/client writes in tracked code =="
rg -n "supabase|from('supabase|Supabase|createClient|rpc\\(|insert\\(|upsert|update\\(|delete\\(" . -g '!node_modules' -g '!dist' -g '!build' | sed -n '1,240p'Repository: durdana3105/peer-learning Length of output: 6773 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate sanitize.ts and supabase files =="
fd -i 'sanitize\.ts|supabase\.ts(js)?|supabase' src supabase backend 2>/dev/null | sed -n '1,160p' || true
echo "== src/utils/sanitize.ts =="
if [ -f src/utils/sanitize.ts ]; then
cat -n src/utils/sanitize.ts | sed -n '1,140p'
fi
echo "== supabase migration files mentioning insert/read/update/policy/default/database function =="
python3 - <<'PY'
from pathlib import Path
terms=['insert','read','update','delete','drop policy','create policy','alter policy','security invariant','security definer','database function','set search_path','default', 'sanitize']
patterns=['*chat*','*messages*','*rls*','*supabase*','*function*','*.sql']
for p in Path('supabase/migrations').glob('*.sql'):
text=p.read_text(errors='ignore').lower()
if any(t in text for t in terms):
print(f"\n--- {p} ---")
for term in terms:
if term in text:
print(f"contains {term}")
idx=text.find(term)
start=text.rfind('\n',0,max(0,idx-200))+1
if start<0: start=0
print(text[start:idx+300].split('\n',1)[0:])
PY
echo "== supabase folder outline =="
find supabase/src -maxdepth 3 -type f 2>/dev/null | sort | sed -n '1,200p' || true
echo "== sanitize usages =="
rg -n "sanitizeInput|sanitizeText|SANITIZE_CONFIG|DOMPurify" . -g '!node_modules' -g '!dist' -g '!build' || trueRepository: durdana3105/peer-learning Length of output: 50382 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== sanitizeInput usages with adjacent code =="
while IFS= read -r f; do
line=$(rg -n "sanitizeInput" "$f" | cut -d: -f1 | head -n1 || true)
if [ -n "$line" ]; then
start=$(( line > 15 ? line - 15 : 1 ))
end=$(( line + 30 ))
echo "\n--- $f:$start-$end ---"
cat -n "$f" | sed -n "${start},$((end+5))p"
fi
done < <(rg -l "sanitizeInput" . -g '!node_modules' -g '!dist' -g '!build')
echo "== sanitizeMessageContent usages =="
while IFS= read -r f; do
echo "\n--- $f ---"
cat -n "$f" | rg -n -C 8 "sanitizeMessageContent|\sanitizeInput|content|content:|messages" || true
done < <(rg -l "sanitizeMessageContent" . -g '!node_modules' -g '!dist' -g '!build')
echo "== messages table RLS and triggers/functions focused =="
rg -n "messages|messages_|content|trunc|regex|regexp_replace|pg_catalog|create.*function|security definer|set search_path|alter table public.messages" supabase/migrations -g '*.sql' --max-count 400Repository: durdana3105/peer-learning Length of output: 262 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== sanitize file list =="
rg -l "sanitizeInput|sanitizeMessageContent|SANITIZE_CONFIG|DOMPurify" . -g '!node_modules' -g '!dist' -g '!build' || true
echo "== sanitizeInput usages with context =="
for f in $(rg -l "sanitizeInput|sanitizeMessageContent" . -g '!node_modules' -g '!dist' -g '!build'); do
echo "\n--- $f ---"
rg -n -C 12 "sanitizeInput|sanitizeMessageContent" "$f" || true
done
echo "== messages table migration snippets =="
python3 - <<'PY'
from pathlib import Path
for p in sorted(Path('supabase/migrations').glob('*.sql')):
text = p.read_text(errors='ignore')
if 'messages' in text.lower() and p.name not in ['20260722000000_add_is_pinned_to_messages.sql']:
print(f"\n--- {p.name} ---")
for i,line in enumerate(text.splitlines(), 1):
if any(kw in line.lower() for kw in ['create table','alter table','create policy','regexp_replace','truncate','pg_catalog','content', 'trigger', 'function']):
print(f"{i}: {line[:320]}")
PYRepository: durdana3105/peer-learning Length of output: 24428 Enforce sanitization on the database insert path.
🤖 Prompt for AI Agents |
||
| }; | ||
|
|
||
| /** | ||
| * Sanitizes message content for storage. | ||
| * Uses strict configuration to prevent stored XSS. | ||
| * | ||
| * @param content - Message content to sanitize | ||
| * @returns Sanitized content safe for storage and display | ||
| */ | ||
| export const sanitizeMessageContent = (content: string): string => { | ||
| return sanitizeInput(content); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: durdana3105/peer-learning
Length of output: 11807
🏁 Script executed:
Repository: durdana3105/peer-learning
Length of output: 26385
🏁 Script executed:
Repository: durdana3105/peer-learning
Length of output: 18834
Sanitize every message read before hydration or rendering.
New inserts protect the current client write path, but legacy/bypassed content can still reach the UI before it’s saved or streamed.
src/hooks/useChatbot.ts#L17: sanitize loaded chat history before state.src/hooks/useChatbot.ts#L85-L96: chunksanitizedBotReplyfor the typewriter instead of rawbotReply.src/hooks/useMessages.ts: sanitize RPC summary rows, loaded/paginated messages, and realtime rows before updatingthreadMessages/conversation summaries.src/hooks/useRoomChat.ts#L17: sanitize the fetched room message rows before injecting them into render state.📍 Affects 3 files
src/hooks/useChatbot.ts#L125-L126(this comment)src/hooks/useMessages.ts#L581-L588src/hooks/useRoomChat.ts#L34-L37🤖 Prompt for AI Agents