Skip to content

Commit e647909

Browse files
kovtcharovclaude
andcommitted
fix(495): CI lint + CodeQL XSS follow-ups
- tests: black-format test_file_write_guardrails.py + test_security_edge_cases.py (previous commit reordered mocks in a way black wanted normalized). - chat-ui.js: route 'error' / 'system' messages through textContent instead of sanitizeHTML + innerHTML. Closes xss-through-exception / xss-through-dom alerts on addMessage — markdown rendering on an error banner is pure risk. - renderer.js: replace two innerHTML template interpolations (AI response, error fallback) with DOM-based construction via an appendAiMessage helper. Matches the innerHTML-removal pattern the PR already applied elsewhere. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4309aae commit e647909

4 files changed

Lines changed: 84 additions & 55 deletions

File tree

src/gaia/apps/jira/webui/public/js/modules/chat-ui.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,21 @@ export class ChatUI {
1919
const contentEl = document.createElement('div');
2020
contentEl.className = 'message-content';
2121

22-
// Handle different content types
22+
// Handle different content types.
23+
//
24+
// For 'error' / 'system' messages we MUST NOT pass through
25+
// formatMessage + sanitizeHTML: those flows include arbitrary
26+
// exception strings (`Error: ${error.message}`) which CodeQL
27+
// correctly flags as xss-through-exception / xss-through-dom
28+
// sinks. Even though sanitizeHTML strips <script>, forcing these
29+
// system-facing messages through textContent is the categorically
30+
// safe option — we don't need markdown in an error banner.
2331
if (typeof content === 'string') {
24-
contentEl.innerHTML = this.sanitizeHTML(this.formatMessage(content));
32+
if (type === 'error' || type === 'system') {
33+
contentEl.textContent = content;
34+
} else {
35+
contentEl.innerHTML = this.sanitizeHTML(this.formatMessage(content));
36+
}
2537
} else if (content instanceof HTMLElement) {
2638
contentEl.appendChild(content);
2739
} else {

src/gaia/apps/jira/webui/public/renderer.js

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -384,14 +384,27 @@ class JaxWebUIRenderer {
384384
chatInput.value = '';
385385
chatMessages.scrollTop = chatMessages.scrollHeight;
386386

387+
// Helper to build an AI message bubble via DOM APIs so we never
388+
// interpolate untrusted data into innerHTML. Closes the CodeQL
389+
// xss-through-dom / xss-through-exception alerts on this file.
390+
const appendAiMessage = (bodyText, { idAttr = null, extraClass = '' } = {}) => {
391+
const wrap = document.createElement('div');
392+
wrap.className = `chat-message ai-message${extraClass ? ' ' + extraClass : ''}`;
393+
if (idAttr) wrap.id = idAttr;
394+
const avatar = document.createElement('div');
395+
avatar.className = 'message-avatar';
396+
avatar.textContent = '\uD83E\uDD16';
397+
const body = document.createElement('div');
398+
body.className = 'message-content';
399+
body.textContent = bodyText;
400+
wrap.appendChild(avatar);
401+
wrap.appendChild(body);
402+
chatMessages.appendChild(wrap);
403+
return wrap;
404+
};
405+
387406
// Show typing indicator
388-
const typingIndicator = `
389-
<div class="chat-message ai-message typing" id="typing-indicator">
390-
<div class="message-avatar">🤖</div>
391-
<div class="message-content">Thinking...</div>
392-
</div>
393-
`;
394-
chatMessages.innerHTML += typingIndicator;
407+
appendAiMessage('Thinking...', { idAttr: 'typing-indicator', extraClass: 'typing' });
395408
chatMessages.scrollTop = chatMessages.scrollHeight;
396409

397410
try {
@@ -406,25 +419,14 @@ class JaxWebUIRenderer {
406419

407420
// Add AI response
408421
const aiResponse = response.result?.response || 'I encountered an error processing your request.';
409-
chatMessages.innerHTML += `
410-
<div class="chat-message ai-message">
411-
<div class="message-avatar">🤖</div>
412-
<div class="message-content">${aiResponse}</div>
413-
</div>
414-
`;
415-
422+
appendAiMessage(aiResponse);
416423
chatMessages.scrollTop = chatMessages.scrollHeight;
417424
} catch (error) {
418425
// Remove typing indicator
419426
const indicator = document.getElementById('typing-indicator');
420427
if (indicator) indicator.remove();
421428

422-
chatMessages.innerHTML += `
423-
<div class="chat-message ai-message">
424-
<div class="message-avatar">🤖</div>
425-
<div class="message-content">Error: ${error.message}</div>
426-
</div>
427-
`;
429+
appendAiMessage(`Error: ${error && error.message ? error.message : String(error)}`);
428430
chatMessages.scrollTop = chatMessages.scrollHeight;
429431
}
430432
}

tests/unit/test_file_write_guardrails.py

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,26 +1127,29 @@ def test_prompt_user_for_access_yes(self, validator, tmp_path):
11271127
outside = tmp_path.parent / "outside_test_prompt.txt"
11281128
# Force interactive mode so the non-TTY guard added in #495 doesn't
11291129
# short-circuit the input() prompt.
1130-
with patch("gaia.security._is_interactive", return_value=True), patch(
1131-
"builtins.input", return_value="y"
1130+
with (
1131+
patch("gaia.security._is_interactive", return_value=True),
1132+
patch("builtins.input", return_value="y"),
11321133
):
11331134
result = validator._prompt_user_for_access(Path(outside))
11341135
assert result is True
11351136

11361137
def test_prompt_user_for_access_no(self, validator, tmp_path):
11371138
"""Verify _prompt_user_for_access with 'n' denies access."""
11381139
outside = tmp_path.parent / "outside_denied.txt"
1139-
with patch("gaia.security._is_interactive", return_value=True), patch(
1140-
"builtins.input", return_value="n"
1140+
with (
1141+
patch("gaia.security._is_interactive", return_value=True),
1142+
patch("builtins.input", return_value="n"),
11411143
):
11421144
result = validator._prompt_user_for_access(Path(outside))
11431145
assert result is False
11441146

11451147
def test_prompt_user_for_access_always(self, validator, tmp_path):
11461148
"""Verify _prompt_user_for_access with 'a' grants and persists access."""
11471149
outside = tmp_path.parent / "outside_always.txt"
1148-
with patch("gaia.security._is_interactive", return_value=True), patch(
1149-
"builtins.input", return_value="a"
1150+
with (
1151+
patch("gaia.security._is_interactive", return_value=True),
1152+
patch("builtins.input", return_value="a"),
11501153
):
11511154
with patch.object(validator, "_save_persisted_path") as mock_save:
11521155
result = validator._prompt_user_for_access(Path(outside))
@@ -1156,9 +1159,10 @@ def test_prompt_user_for_access_always(self, validator, tmp_path):
11561159
def test_prompt_user_for_access_non_interactive_denies(self, validator, tmp_path):
11571160
"""Non-TTY contexts auto-deny without ever calling input()."""
11581161
outside = tmp_path.parent / "outside_non_tty.txt"
1159-
with patch("gaia.security._is_interactive", return_value=False), patch(
1160-
"builtins.input"
1161-
) as mock_input:
1162+
with (
1163+
patch("gaia.security._is_interactive", return_value=False),
1164+
patch("builtins.input") as mock_input,
1165+
):
11621166
result = validator._prompt_user_for_access(Path(outside))
11631167
assert result is False
11641168
mock_input.assert_not_called()
@@ -1167,8 +1171,9 @@ def test_prompt_overwrite_yes(self, validator, tmp_path):
11671171
"""Verify _prompt_overwrite with 'y' returns True."""
11681172
existing = tmp_path / "overwrite_prompt.txt"
11691173
existing.write_text("data")
1170-
with patch("gaia.security._is_interactive", return_value=True), patch(
1171-
"builtins.input", return_value="y"
1174+
with (
1175+
patch("gaia.security._is_interactive", return_value=True),
1176+
patch("builtins.input", return_value="y"),
11721177
):
11731178
result = validator._prompt_overwrite(existing, existing.stat().st_size)
11741179
assert result is True
@@ -1177,8 +1182,9 @@ def test_prompt_overwrite_no(self, validator, tmp_path):
11771182
"""Verify _prompt_overwrite with 'n' returns False."""
11781183
existing = tmp_path / "overwrite_no.txt"
11791184
existing.write_text("data")
1180-
with patch("gaia.security._is_interactive", return_value=True), patch(
1181-
"builtins.input", return_value="n"
1185+
with (
1186+
patch("gaia.security._is_interactive", return_value=True),
1187+
patch("builtins.input", return_value="n"),
11821188
):
11831189
result = validator._prompt_overwrite(existing, existing.stat().st_size)
11841190
assert result is False
@@ -1187,9 +1193,10 @@ def test_prompt_overwrite_non_interactive_approves(self, validator, tmp_path):
11871193
"""Non-TTY contexts auto-approve overwrite (relies on backup)."""
11881194
existing = tmp_path / "overwrite_non_tty.txt"
11891195
existing.write_text("data")
1190-
with patch("gaia.security._is_interactive", return_value=False), patch(
1191-
"builtins.input"
1192-
) as mock_input:
1196+
with (
1197+
patch("gaia.security._is_interactive", return_value=False),
1198+
patch("builtins.input") as mock_input,
1199+
):
11931200
result = validator._prompt_overwrite(existing, existing.stat().st_size)
11941201
assert result is True
11951202
mock_input.assert_not_called()

tests/unit/test_security_edge_cases.py

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,9 @@ def test_prompt_overwrite_yes(self, validator, tmp_path):
209209
target = tmp_path / "file.txt"
210210
target.write_text("data")
211211

212-
with patch("gaia.security._is_interactive", return_value=True), patch(
213-
"builtins.input", return_value="y"
212+
with (
213+
patch("gaia.security._is_interactive", return_value=True),
214+
patch("builtins.input", return_value="y"),
214215
):
215216
result = validator._prompt_overwrite(target, 100)
216217

@@ -221,8 +222,9 @@ def test_prompt_overwrite_no(self, validator, tmp_path):
221222
target = tmp_path / "file.txt"
222223
target.write_text("data")
223224

224-
with patch("gaia.security._is_interactive", return_value=True), patch(
225-
"builtins.input", return_value="n"
225+
with (
226+
patch("gaia.security._is_interactive", return_value=True),
227+
patch("builtins.input", return_value="n"),
226228
):
227229
result = validator._prompt_overwrite(target, 100)
228230

@@ -233,8 +235,9 @@ def test_prompt_overwrite_yes_full_word(self, validator, tmp_path):
233235
target = tmp_path / "file.txt"
234236
target.write_text("data")
235237

236-
with patch("gaia.security._is_interactive", return_value=True), patch(
237-
"builtins.input", return_value="yes"
238+
with (
239+
patch("gaia.security._is_interactive", return_value=True),
240+
patch("builtins.input", return_value="yes"),
238241
):
239242
result = validator._prompt_overwrite(target, 100)
240243

@@ -245,8 +248,9 @@ def test_prompt_overwrite_no_full_word(self, validator, tmp_path):
245248
target = tmp_path / "file.txt"
246249
target.write_text("data")
247250

248-
with patch("gaia.security._is_interactive", return_value=True), patch(
249-
"builtins.input", return_value="no"
251+
with (
252+
patch("gaia.security._is_interactive", return_value=True),
253+
patch("builtins.input", return_value="no"),
250254
):
251255
result = validator._prompt_overwrite(target, 100)
252256

@@ -258,8 +262,9 @@ def test_prompt_overwrite_invalid_then_yes(self, validator, tmp_path):
258262
target.write_text("data")
259263

260264
# Simulate: "maybe" -> "xxx" -> "y"
261-
with patch("gaia.security._is_interactive", return_value=True), patch(
262-
"builtins.input", side_effect=["maybe", "xxx", "y"]
265+
with (
266+
patch("gaia.security._is_interactive", return_value=True),
267+
patch("builtins.input", side_effect=["maybe", "xxx", "y"]),
263268
):
264269
result = validator._prompt_overwrite(target, 200)
265270

@@ -271,8 +276,9 @@ def test_prompt_overwrite_invalid_then_no(self, validator, tmp_path):
271276
target.write_text("data")
272277

273278
# Simulate: "" -> "asdf" -> "n"
274-
with patch("gaia.security._is_interactive", return_value=True), patch(
275-
"builtins.input", side_effect=["", "asdf", "n"]
279+
with (
280+
patch("gaia.security._is_interactive", return_value=True),
281+
patch("builtins.input", side_effect=["", "asdf", "n"]),
276282
):
277283
result = validator._prompt_overwrite(target, 50)
278284

@@ -291,8 +297,9 @@ def test_prompt_overwrite_prints_file_info(self, validator, tmp_path):
291297
" ".join(str(x) for x in a)
292298
),
293299
):
294-
with patch("gaia.security._is_interactive", return_value=True), patch(
295-
"builtins.input", return_value="y"
300+
with (
301+
patch("gaia.security._is_interactive", return_value=True),
302+
patch("builtins.input", return_value="y"),
296303
):
297304
validator._prompt_overwrite(target, 2048)
298305

@@ -307,9 +314,10 @@ def test_prompt_overwrite_non_interactive_approves_with_backup(
307314
target = tmp_path / "file.txt"
308315
target.write_text("data")
309316

310-
with patch("gaia.security._is_interactive", return_value=False), patch(
311-
"builtins.input"
312-
) as mock_input:
317+
with (
318+
patch("gaia.security._is_interactive", return_value=False),
319+
patch("builtins.input") as mock_input,
320+
):
313321
result = validator._prompt_overwrite(target, 100)
314322

315323
assert result is True

0 commit comments

Comments
 (0)