Skip to content

Commit 9feed00

Browse files
committed
fix: Three-part prompt split to prevent guardrail blocking PR diffs containing security strings
1 parent 14325d6 commit 9feed00

2 files changed

Lines changed: 103 additions & 8 deletions

File tree

scripts/issue_bot/bedrock_client.py

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,22 +100,42 @@ def _split_prompt(self, prompt):
100100
"""Split rendered prompt into trusted system prompt and untrusted user content.
101101
102102
Everything before <knowledge_base> is trusted instructions (system prompt).
103-
Everything from <knowledge_base> onward contains user data (guarded).
103+
Content between <knowledge_base> and <user_input> is context (not guarded).
104+
Content from <user_input> onward is untrusted user data (guarded).
105+
106+
This prevents guardrail false positives on PR diffs and source code
107+
that may contain security-related strings (e.g., "ignore previous instructions"
108+
appearing as literal code in a sanitizer module).
104109
"""
105-
marker = "<knowledge_base>"
106-
idx = prompt.find(marker)
107-
if idx == -1:
110+
kb_marker = "<knowledge_base>"
111+
user_marker = "<user_input>"
112+
113+
kb_idx = prompt.find(kb_marker)
114+
if kb_idx == -1:
108115
# No marker — guard everything as untrusted
109116
if self._guardrail_id:
110117
return None, [{"guardContent": {"text": {"text": prompt}}}]
111118
return None, [{"text": prompt}]
112119

113-
system = prompt[:idx].strip()
114-
user_data = prompt[idx:].strip()
120+
system = prompt[:kb_idx].strip()
121+
122+
user_idx = prompt.find(user_marker, kb_idx)
123+
if user_idx == -1:
124+
# Backward compat: no <user_input> marker means everything after
125+
# <knowledge_base> may contain untrusted content — guard it all.
126+
after_kb = prompt[kb_idx:].strip()
127+
if self._guardrail_id:
128+
return system, [{"guardContent": {"text": {"text": after_kb}}}]
129+
return system, [{"text": after_kb}]
130+
131+
# Three-part split: system | context (not guarded) | user input (guarded)
132+
context_data = prompt[kb_idx:user_idx].strip()
133+
user_data = prompt[user_idx:].strip()
115134

135+
user_content = [{"text": context_data}]
116136
if self._guardrail_id:
117-
user_content = [{"guardContent": {"text": {"text": user_data}}}]
137+
user_content.append({"guardContent": {"text": {"text": user_data}}})
118138
else:
119-
user_content = [{"text": user_data}]
139+
user_content.append({"text": user_data})
120140

121141
return system, user_content

tests/test_bot.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,3 +279,78 @@ def test_empty_url_preserved(self):
279279
url = ""
280280
result = "" if url and not url.startswith("https://github.com/") else url
281281
assert result == ""
282+
283+
284+
# ---------------------------------------------------------------------------
285+
# _split_prompt (BedrockClient prompt splitting)
286+
# ---------------------------------------------------------------------------
287+
288+
class TestSplitPrompt:
289+
"""Test the three-part prompt split: system | context (unguarded) | user (guarded)."""
290+
291+
def _make_client(self, guardrail_id=""):
292+
class FakeCfg:
293+
bedrock_model_id = "test"
294+
bedrock_timeout = 10
295+
guardrail_id = ""
296+
guardrail_version = "DRAFT"
297+
cfg = FakeCfg()
298+
cfg.guardrail_id = guardrail_id
299+
from issue_bot.bedrock_client import BedrockClient
300+
# Avoid actual boto3 client creation
301+
import unittest.mock as mock
302+
with mock.patch("boto3.client"):
303+
client = BedrockClient(cfg)
304+
return client
305+
306+
def test_no_markers_no_guardrail(self):
307+
client = self._make_client()
308+
system, user = client._split_prompt("just text")
309+
assert system is None
310+
assert user == [{"text": "just text"}]
311+
312+
def test_no_markers_with_guardrail(self):
313+
client = self._make_client(guardrail_id="gr-123")
314+
system, user = client._split_prompt("just text")
315+
assert system is None
316+
assert user == [{"guardContent": {"text": {"text": "just text"}}}]
317+
318+
def test_kb_only_no_user_marker(self):
319+
prompt = "instructions\n<knowledge_base>\ncontext and diff"
320+
client = self._make_client(guardrail_id="gr-123")
321+
system, user = client._split_prompt(prompt)
322+
assert system == "instructions"
323+
assert len(user) == 1
324+
assert "guardContent" in user[0] # Backward compat: guarded when no <user_input>
325+
326+
def test_three_part_split_with_guardrail(self):
327+
prompt = "instructions\n<knowledge_base>\nKB + diff here\n<user_input>\nuser title and body"
328+
client = self._make_client(guardrail_id="gr-123")
329+
system, user = client._split_prompt(prompt)
330+
assert system == "instructions"
331+
assert len(user) == 2
332+
assert "text" in user[0] # context block — unguarded
333+
assert "guardContent" in user[1] # user input — guarded
334+
assert "KB + diff" in user[0]["text"]
335+
assert "user title" in user[1]["guardContent"]["text"]["text"]
336+
337+
def test_three_part_split_no_guardrail(self):
338+
prompt = "instructions\n<knowledge_base>\ncontext\n<user_input>\nuser data"
339+
client = self._make_client()
340+
system, user = client._split_prompt(prompt)
341+
assert system == "instructions"
342+
assert len(user) == 2
343+
assert all("text" in block for block in user) # both unguarded without guardrail
344+
345+
def test_diff_with_injection_strings_not_guarded(self):
346+
"""PR diffs containing 'ignore previous instructions' as source code
347+
should NOT be guarded, preventing guardrail false positives."""
348+
diff = 'sanitizer has: "ignore previous instructions"'
349+
prompt = f"review this PR\n<knowledge_base>\n{diff}\n<user_input>\nPR title: Spark4 support"
350+
client = self._make_client(guardrail_id="gr-123")
351+
system, user = client._split_prompt(prompt)
352+
# The diff is in the unguarded context block
353+
assert "ignore previous instructions" in user[0]["text"]
354+
assert "text" in user[0] # NOT guardContent
355+
# The user input IS guarded
356+
assert "guardContent" in user[1]

0 commit comments

Comments
 (0)