Security: fix chat XSS and harden login/request-size/secret-key - #50
Merged
Conversation
XSS (client): - markdown_parser.py now HTML-escapes source text before any markdown transformation, so raw <script>/<img onerror> in AI output can never reach the innerHTML sinks; only parser-emitted tags survive - Added URL scheme allowlist (http/https/mailto + relative) for links; javascript:/data:/vbscript: and control-char tricks are neutralized to plain text - chat_ui_manager.py escapes sender/content in the error-fallback sink Server hardening: - Login rate limit keys on request.remote_addr instead of the spoofable X-Forwarded-For; added a global failed-attempt ceiling (50 / 5 min) that blocks brute force from distributed/spoofed sources - Set MAX_CONTENT_LENGTH (80 MB) with a JSON 413 handler; enforce server-side image caps (max 5, 20 MB base64 each) on both send routes - Warn when SECRET_KEY is unset in a deployed environment Tests: - 13 markdown XSS regression tests; new test_login_and_hardening.py covering login, per-IP + global rate limiting, XFF spoof resistance, and image validation - Server: 1293 passed; client: 2733 passed; mypy and ruff clean
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.
Summary
Security follow-up to the PR #49 codebase review. Fixes the one High-severity verified vulnerability (stored/reflected XSS in AI chat rendering) plus three hardening gaps found during the survey.
XSS in chat rendering (High)
AI output is attacker-influenceable via prompt injection (workspace text, tool results), and the custom markdown parser did no HTML escaping before assigning to
innerHTML.markdown_parser.pynow HTML-escapes source text before any markdown transformation, so raw<script>/<img onerror=...>can never reach theinnerHTMLsinks — only parser-emitted tags survive.http/https/mailto+ relative);javascript:,data:,vbscript:, and control-char tricks (java\tscript:) are neutralized to plain text.chat_ui_manager.pyescapessender/contentin the error-fallbackinnerHTMLpath.Login rate-limit bypass (Medium)
request.remote_addrinstead of the client-spoofableX-Forwarded-Forheader.No request size limit / client-only image caps (Medium)
MAX_CONTENT_LENGTH(80 MB) with a JSON413error handler./send_messageand/send_message_stream— previously enforced only client-side.Ephemeral SECRET_KEY in deployment (Low)
SECRET_KEYis unset in a deployed environment (sessions won't survive restarts); random fallback retained, no hard-fail.Test plan
javascript:/mixed-case/control-char/data:links neutralized, safe links & normal markdown still render, code blocks escaped.test_login_and_hardening.py— the login flow had zero tests before. Covers correct/incorrect PIN, per-IP cooldown, X-Forwarded-For spoof resistance, global ceiling lockout, image count/size rejection,MAX_CONTENT_LENGTH+ JSON 413.Not included (documented for follow-up)
Concurrency thread-safety of shared provider/conversation state, Selenium driver locking,
webdriver_managertest coverage,workspace_manager.pyprint()→logging, and friendlier streaming error messages — all latent/lower-priority items from the same survey.