Skip to content

Commit d59cd18

Browse files
sellakumaranclaude
andcommitted
Address Copilot PR review comments on prompt injection hardening
- Escape rationale object keys through escapeMd in auto-triage-issues.yml; previously only values were escaped, leaving LLM-derived keys as a markdown injection vector. - Move issue_title/issue_body sanitisation into get_fix_instructions so the function owns the safety contract for its own <user_content> delimiter; callers no longer need to pre-sanitise these fields. - Update test_issue_body_truncated to use MAX_ISSUE_BODY_LENGTH boundary and remove the stale 1500-char / "..." assertions. - Update test_xml_escaped_title_appears_inside_user_content to pass a raw title (matching the new caller contract) and assert the escaped form. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f5b22cf commit d59cd18

3 files changed

Lines changed: 32 additions & 23 deletions

File tree

.github/workflows/auto-triage-issues.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ jobs:
111111
if (triageResult.rationale) {
112112
if (typeof triageResult.rationale === 'object') {
113113
rationaleText = Object.entries(triageResult.rationale)
114-
.map(([key, value]) => `**${key.replace(/_/g, ' ')}:** ${escapeMd(value)}`)
114+
.map(([key, value]) => `**${escapeMd(key.replace(/_/g, ' '))}:** ${escapeMd(value)}`)
115115
.join('\n\n');
116116
} else {
117117
rationaleText = escapeMd(triageResult.rationale);

autoTriage/services/copilot_service.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
import subprocess
1010
import json
1111
from typing import Dict, Any
12-
from utils.sanitise import MAX_SUGGESTION_LENGTH, sanitise_user_content as _sanitise_user_content
12+
from utils.sanitise import (
13+
MAX_ISSUE_BODY_LENGTH,
14+
MAX_ISSUE_TITLE_LENGTH,
15+
MAX_SUGGESTION_LENGTH,
16+
sanitise_user_content as _sanitise_user_content,
17+
)
1318

1419
logger = logging.getLogger(__name__)
1520

@@ -376,19 +381,21 @@ def get_fix_instructions(
376381
# fix_suggestions are generated by an upstream LLM from untrusted issue content
377382
# and must be treated as untrusted too — an attacker can steer that LLM to emit
378383
# malicious suggestions that would otherwise appear as trusted instructions.
384+
# Sanitise title and body here so callers do not need to pre-sanitise.
385+
# get_fix_instructions owns the <user_content> delimiter, so it must also
386+
# own the sanitisation of everything placed inside it.
387+
safe_title = _sanitise_user_content(issue_title, max_length=MAX_ISSUE_TITLE_LENGTH) if issue_title else ""
388+
safe_body = _sanitise_user_content(issue_body, max_length=MAX_ISSUE_BODY_LENGTH) if issue_body else ""
389+
379390
instructions.append("<user_content>")
380-
if issue_title:
381-
instructions.append(f"Issue: {issue_title}")
391+
if safe_title:
392+
instructions.append(f"Issue: {safe_title}")
382393
instructions.append("")
383394

384-
# Include issue body (truncated for reasonable instruction length)
385-
if issue_body:
386-
# Truncate body to 1500 chars to keep instructions manageable
387-
truncated_body = issue_body[:1500]
388-
if len(issue_body) > 1500:
389-
truncated_body += "..."
395+
# Include issue body (truncated by sanitise_user_content)
396+
if safe_body:
390397
instructions.append("Description:")
391-
instructions.append(truncated_body)
398+
instructions.append(safe_body)
392399
instructions.append("")
393400

394401
if fix_suggestions:

autoTriage/tests/services/test_copilot_service.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -714,20 +714,21 @@ def test_issue_body_included(self):
714714

715715
# Test: Long issue body is truncated
716716
def test_issue_body_truncated(self):
717-
"""Test that very long issue bodies are truncated."""
717+
"""Test that very long issue bodies are truncated to MAX_ISSUE_BODY_LENGTH."""
718718
from services.copilot_service import CopilotService
719-
719+
from utils.sanitise import MAX_ISSUE_BODY_LENGTH
720+
720721
service = CopilotService()
721-
long_body = "A" * 2000 # Longer than 1500 char limit
722+
long_body = "A" * (MAX_ISSUE_BODY_LENGTH + 500)
722723
instructions = service.get_fix_instructions(
723724
issue_title="Test",
724725
issue_body=long_body,
725726
fix_suggestions=[]
726727
)
727-
728-
assert "..." in instructions
729-
# Should have truncated content
730-
assert "A" * 1500 in instructions
728+
729+
# sanitise_user_content truncates at MAX_ISSUE_BODY_LENGTH (no "..." suffix)
730+
assert "A" * MAX_ISSUE_BODY_LENGTH in instructions
731+
assert "A" * (MAX_ISSUE_BODY_LENGTH + 1) not in instructions
731732

732733
# Test: Suggestion numbering
733734
def test_suggestions_are_numbered(self):
@@ -873,21 +874,22 @@ def test_dependency_constraint_always_present(self, suggestions):
873874
f"Dependency constraint missing when fix_suggestions={suggestions!r}"
874875

875876
def test_xml_escaped_title_appears_inside_user_content(self):
876-
"""A title with XML special chars that was sanitized upstream stays inside user_content."""
877+
"""A title with XML special chars is sanitised by get_fix_instructions and stays inside user_content."""
877878
from services.copilot_service import CopilotService
878879

879880
service = CopilotService()
880-
# Simulate a title that has already been XML-escaped by _sanitise_user_content
881-
sanitized_title = "&lt;script&gt;alert(1)&lt;/script&gt; fix typo"
881+
# Pass raw (unsanitised) title — get_fix_instructions now owns the sanitisation.
882+
raw_title = "<script>alert(1)</script> fix typo"
883+
escaped_title = "&lt;script&gt;alert(1)&lt;/script&gt; fix typo"
882884
instructions = service.get_fix_instructions(
883-
issue_title=sanitized_title,
885+
issue_title=raw_title,
884886
issue_body="",
885887
fix_suggestions=[]
886888
)
887889

888890
open_pos = instructions.index("<user_content>")
889891
close_pos = instructions.index("</user_content>")
890-
title_pos = instructions.index(sanitized_title)
892+
title_pos = instructions.index(escaped_title)
891893
assert open_pos < title_pos < close_pos, \
892894
"Escaped title must remain inside user_content block"
893895

0 commit comments

Comments
 (0)