diff --git a/SECURITY.md b/SECURITY.md index bda5689f..160e5314 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -92,6 +92,22 @@ single-worker deployment (see the Deployment Note below). | `AUTH_IP_WINDOW_MINUTES` | `15` | Rolling window for counting failures. | | `AUTH_IP_BLOCK_MINUTES` | `15` | How long a blocked IP stays blocked. | +## Frontend HTML Sinks (XSS) + +Raw-HTML sinks (`dangerouslySetInnerHTML`, `innerHTML =`, `insertAdjacentHTML`, +`new Function`, `eval`) must sit behind a sanitizer/escaper, and template output +on the backend (`|safe`, `Markup(`, `render_template_string`) is avoided in favor +of Jinja's default autoescaping. Today every sink is safe by construction: +extension icons pass through `sanitizeSvgInner`, assistant markdown through the +escape-then-allowlist `renderMarkdownToHtml`, and the SQL/file syntax tinting +escapes input before inserting its own token spans. + +To keep the sweep swept, each sink must reference an allowlisted sanitizer **or** +carry a `sink-safe: — ` comment. `scripts/check-html-sinks.mjs` +(run in `npm run lint`, and mirrored by `backend/tests/test_html_sink_sweep.py`) +fails CI on any new unannotated sink, so a raw-HTML injection point can't land +silently. + ## Deployment Note The agent gateway keeps all connected-agent state in-memory in a single process. diff --git a/backend/tests/test_html_sink_sweep.py b/backend/tests/test_html_sink_sweep.py new file mode 100644 index 00000000..88f16377 --- /dev/null +++ b/backend/tests/test_html_sink_sweep.py @@ -0,0 +1,61 @@ +"""HTML-sink sweep guard — proving test (plan 55 Phase 1, D4). + +Mirrors scripts/check-html-sinks.mjs in Python so the sweep stays swept inside +the backend suite (the authoritative gate) even where node isn't available: every +raw-HTML sink in frontend/src or backend/app must reference an allowlisted +sanitizer/escaper or carry a `sink-safe:` annotation. A new unannotated sink +fails this test. +""" +import os +import re + +_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +FRONTEND_SINK = re.compile( + r'dangerouslySetInnerHTML|\.innerHTML\s*=[^=]|insertAdjacentHTML' + r'|new\s+Function\s*\(|\beval\s*\(') +BACKEND_SINK = re.compile(r'\|\s*safe\b|\bMarkup\s*\(|render_template_string') +ALLOWLISTED_SANITIZERS = re.compile(r'sanitizeSvgInner|renderMarkdownToHtml|highlightLine|hlSql') +ANNOTATION = re.compile(r'sink-safe:') +_COMMENT_PREFIXES = ('//', '*', '/*', '#') + + +def _walk(root, exts): + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames + if d not in ('node_modules', 'dist', '__pycache__')] + for name in filenames: + if name.endswith(exts): + yield os.path.join(dirpath, name) + + +def _scan(path, sink_re, allow_sanitizer): + with open(path, 'r', encoding='utf-8', errors='replace') as f: + lines = f.read().split('\n') + violations = [] + for i, line in enumerate(lines): + if not sink_re.search(line): + continue + if line.strip().startswith(_COMMENT_PREFIXES): + continue # a comment that merely mentions a sink keyword + window = '\n'.join(lines[max(0, i - 3):i + 1]) + cleared = (allow_sanitizer and ALLOWLISTED_SANITIZERS.search(line)) \ + or ANNOTATION.search(window) + if not cleared: + violations.append(f'{os.path.relpath(path, _REPO_ROOT)}:{i + 1} {line.strip()}') + return violations + + +def test_no_unannotated_html_sinks(): + violations = [] + fe = os.path.join(_REPO_ROOT, 'frontend', 'src') + for path in _walk(fe, ('.js', '.jsx', '.ts', '.tsx')): + violations += _scan(path, FRONTEND_SINK, allow_sanitizer=True) + be = os.path.join(_REPO_ROOT, 'backend', 'app') + for path in _walk(be, ('.py', '.html')): + violations += _scan(path, BACKEND_SINK, allow_sanitizer=False) + + assert not violations, ( + 'Unannotated raw-HTML sink(s) — each must reference a sanitizer/escaper ' + 'or carry a `sink-safe:` comment (see scripts/check-html-sinks.mjs):\n' + + '\n'.join(violations)) diff --git a/frontend/package.json b/frontend/package.json index ef11b398..6f32a060 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -6,8 +6,9 @@ "scripts": { "dev": "vite", "build": "vite build", - "lint": "eslint . && node scripts/check-settings-index.mjs", + "lint": "eslint . && node scripts/check-settings-index.mjs && node ../scripts/check-html-sinks.mjs", "lint:settings-index": "node scripts/check-settings-index.mjs", + "lint:html-sinks": "node ../scripts/check-html-sinks.mjs", "capture:skeletons": "node scripts/capture-skeletons.mjs", "preview": "vite preview" }, diff --git a/frontend/src/components/ai/Markdown.jsx b/frontend/src/components/ai/Markdown.jsx index 726d2862..125aeedd 100644 --- a/frontend/src/components/ai/Markdown.jsx +++ b/frontend/src/components/ai/Markdown.jsx @@ -3,6 +3,7 @@ import { renderMarkdownToHtml } from '../../lib/ai/markdown'; // Renders assistant text. The renderer escapes all input before injecting its // own allowlisted tags, so dangerouslySetInnerHTML is safe here. +// sink-safe: renderMarkdownToHtml (lib/ai/markdown.js) — escapes then allowlists. const Markdown = ({ text }) => { const html = useMemo(() => renderMarkdownToHtml(text || ''), [text]); return
; diff --git a/frontend/src/components/databases/SqlEditor.jsx b/frontend/src/components/databases/SqlEditor.jsx index 094e63bc..4505487f 100644 --- a/frontend/src/components/databases/SqlEditor.jsx +++ b/frontend/src/components/databases/SqlEditor.jsx @@ -46,6 +46,7 @@ const SqlEditor = forwardRef(function SqlEditor({ value, onChange, onKeyDown, pl className="dbx-sqled__hl" ref={preRef} aria-hidden="true" + /* sink-safe: hlSql() esc()-escapes each line before tinting */ dangerouslySetInnerHTML={{ __html: html }} />