mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-10-23 03:27:43 +09:00
feat(security): HTML-sink sweep guard (D4) — keep XSS sinks swept
Plan 55 Phase 1. The sweep found zero vulnerabilities: every raw-HTML sink is safe by construction (escape-first SQL/file highlighters, sanitizeSvgInner for extension icons, the escape-then-allowlist markdown renderer) and the backend has no |safe/Markup(/render_template_string. To keep it that way: scripts/check-html-sinks.mjs (wired into npm run lint) and a Python mirror test (test_html_sink_sweep.py, runs in the backend suite) fail on any raw-HTML sink that neither references an allowlisted sanitizer nor carries a 'sink-safe:' annotation. Standardized annotations on the three variable-fed sinks (markdown, SQL editor, marketplace icon) and documented the rule in SECURITY.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
cf9cc5d662
commit
28cece380f
7 files changed
+194
-1
No files matched your search
+16
@@ -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: <sanitizer> — <why>` 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.
|
||||
|
||||
@@ -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))
|
||||
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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 <div className="sk-ai-markdown" dangerouslySetInnerHTML={{ __html: html }} />;
|
||||
|
||||
@@ -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 }}
|
||||
/>
|
||||
<textarea
|
||||
|
||||
@@ -586,6 +586,7 @@ const ExtensionCover = ({ entry, category, brandSize = 34 }) => {
|
||||
aria-hidden="true"
|
||||
focusable="false"
|
||||
className="extension-card__glyph"
|
||||
/* sink-safe: iconSvg = sanitizeSvgInner(entry.icon) above */
|
||||
dangerouslySetInnerHTML={{ __html: iconSvg }}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
/**
|
||||
* HTML-sink guard (plan 55 Phase 1, Decision D4).
|
||||
*
|
||||
* Keeps the XSS-sink sweep swept: every raw-HTML sink must sit behind a known
|
||||
* sanitizer/escaper OR carry an explicit `sink-safe:` annotation naming why it's
|
||||
* safe. A new unannotated sink fails this check, so the sweep can't silently
|
||||
* regress (the plan-41 completeness-script pattern).
|
||||
*
|
||||
* node scripts/check-html-sinks.mjs # exit 1 on any uncleared sink
|
||||
*
|
||||
* Scans:
|
||||
* - frontend/src for dangerouslySetInnerHTML | .innerHTML= | insertAdjacentHTML
|
||||
* | new Function( | eval(
|
||||
* - backend/app for |safe | Markup( | render_template_string
|
||||
*
|
||||
* A sink LINE is cleared when it either references an allowlisted sanitizer
|
||||
* (frontend only) or a `sink-safe:` comment appears on it or within the 3 lines
|
||||
* above it. Allowlisted sanitizers live in frontend/src/utils + lib + the
|
||||
* escape-first highlighters; extend ALLOWLISTED_SANITIZERS only alongside a real
|
||||
* escaper/sanitizer.
|
||||
*/
|
||||
import { promises as fs } from 'fs';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = path.resolve(__dirname, '..');
|
||||
|
||||
const FRONTEND_SINK = /dangerouslySetInnerHTML|\.innerHTML\s*=[^=]|insertAdjacentHTML|new\s+Function\s*\(|\beval\s*\(/;
|
||||
const BACKEND_SINK = /\|\s*safe\b|\bMarkup\s*\(|render_template_string/;
|
||||
// Escapers/sanitizers that make a raw-HTML sink safe by construction.
|
||||
const ALLOWLISTED_SANITIZERS = /sanitizeSvgInner|renderMarkdownToHtml|highlightLine|hlSql/;
|
||||
const ANNOTATION = /sink-safe:/;
|
||||
|
||||
async function walk(dir, exts) {
|
||||
const out = [];
|
||||
let entries;
|
||||
try {
|
||||
entries = await fs.readdir(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return out;
|
||||
}
|
||||
for (const e of entries) {
|
||||
if (e.name === 'node_modules' || e.name === 'dist' || e.name === '__pycache__') continue;
|
||||
const full = path.join(dir, e.name);
|
||||
if (e.isDirectory()) {
|
||||
out.push(...await walk(full, exts));
|
||||
} else if (exts.some((x) => e.name.endsWith(x))) {
|
||||
out.push(full);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function scanFile(rel, text, sinkRe, allowSanitizer) {
|
||||
const lines = text.split('\n');
|
||||
const violations = [];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (!sinkRe.test(lines[i])) continue;
|
||||
// Skip comment-only lines: a doc comment that merely *mentions* a sink
|
||||
// keyword isn't a sink (e.g. "// dangerouslySetInnerHTML is safe here").
|
||||
const trimmed = lines[i].trim();
|
||||
if (trimmed.startsWith('//') || trimmed.startsWith('*')
|
||||
|| trimmed.startsWith('/*') || trimmed.startsWith('#')) continue;
|
||||
const window = lines.slice(Math.max(0, i - 3), i + 1).join('\n');
|
||||
const cleared = (allowSanitizer && ALLOWLISTED_SANITIZERS.test(lines[i]))
|
||||
|| ANNOTATION.test(window);
|
||||
if (!cleared) {
|
||||
violations.push({ file: rel, line: i + 1, text: lines[i].trim() });
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const violations = [];
|
||||
let sinkCount = 0;
|
||||
|
||||
const feFiles = await walk(path.join(ROOT, 'frontend', 'src'), ['.js', '.jsx', '.ts', '.tsx']);
|
||||
for (const f of feFiles) {
|
||||
const text = await fs.readFile(f, 'utf8');
|
||||
const rel = path.relative(ROOT, f);
|
||||
for (const line of text.split('\n')) if (FRONTEND_SINK.test(line)) sinkCount++;
|
||||
violations.push(...scanFile(rel, text, FRONTEND_SINK, true));
|
||||
}
|
||||
|
||||
const beFiles = await walk(path.join(ROOT, 'backend', 'app'), ['.py', '.html']);
|
||||
for (const f of beFiles) {
|
||||
const text = await fs.readFile(f, 'utf8');
|
||||
const rel = path.relative(ROOT, f);
|
||||
for (const line of text.split('\n')) if (BACKEND_SINK.test(line)) sinkCount++;
|
||||
violations.push(...scanFile(rel, text, BACKEND_SINK, false));
|
||||
}
|
||||
|
||||
if (violations.length) {
|
||||
console.error('\n✖ HTML-sink guard: unannotated raw-HTML sink(s) found.\n');
|
||||
for (const v of violations) {
|
||||
console.error(` ${v.file}:${v.line} ${v.text}`);
|
||||
}
|
||||
console.error('\nEach raw-HTML sink must reference a sanitizer/escaper or carry a');
|
||||
console.error('`sink-safe: <sanitizer> — <why>` comment (on the line or up to 3 above).');
|
||||
console.error('See scripts/check-html-sinks.mjs and SECURITY.md.\n');
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
console.log(`✓ html-sinks: ${sinkCount} raw-HTML sink(s), all sanitized or annotated.`);
|
||||
}
|
||||
|
||||
main().catch((e) => {
|
||||
console.error(e);
|
||||
process.exit(2);
|
||||
});
|
||||
Reference in new issue
Block a user