Repository navigation
fix(redteam): keep $ sequences literal when datamarking an injection - #11381
Conversation
`getDatamarkingSuggestion` passes the datamarked text as the replacement string of `String.prototype.replace`, so `$&`, `` $` ``, `$'` and `$$` inside the injection payload expand instead of landing as themselves. For `$&` the expansion is the whole match, which puts the original, undatamarked instruction back into the prompt the suggestion exists to mark up. A replacer function inserts the value literally.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11381 +/- ##
=======================================
Coverage 87.41% 87.42%
=======================================
Files 979 979
Lines 90213 90214 +1
Branches 31486 31486
=======================================
+ Hits 78860 78868 +8
+ Misses 11353 11346 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
👍 All Clear
Reviewed changes to the redteam indirect prompt injection grader. The update switches to a function-based replace to ensure $ sequences in attacker input are treated literally. No new LLM calls, tools, or execution sinks were introduced. Overall, this hardens prompt handling and introduces no LLM security risks in scope.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
|
Thanks for catching this subtle case. I synced with main and kept the one-line callback fix, then added coverage for every special dollar sequence and repeated combinations. All 11 tests pass. I also ran the actual local eval path and checked both suggestions for each of six inputs; the five problematic inputs reproduce the corruption on main. The branch is pushed and remains open while CI runs. |
There was a problem hiding this comment.
👍 All Clear
Reviewed changes to the indirect prompt injection redteam plugin and its tests. The update uses a replacer function to ensure datamarked text is inserted literally, which hardens against replacement-sequence re-expansion. No LLM security vulnerabilities were introduced by this PR.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
There was a problem hiding this comment.
👍 All Clear
Reviewed changes to the redteam indirect prompt injection grader and its tests. The update switches to a safer replacement method that preserves literal $ sequences during datamarking. No LLM security vulnerabilities were identified in these changes.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
mldangelo-oai
left a comment
There was a problem hiding this comment.
The literal callback fix is sound, with all special dollar sequences and ordinary suggestions verified through tests and the local eval path. Independent review is clean. All reported checks are now green, including the retried macOS package job; its first attempt never acquired a runner.
Code review is approved. Normal merging still waits on the required CodeQL context, which this repository's default setup excludes for fork PRs: https://docs.github.com/en/code-security/concepts/code-scanning/setup-types. Leaving this open.
|
I trimmed the test setup further: ordinary input now uses the same exact-output table as the dollar-pattern cases, and the overlapping assertions are gone. This removes 16 lines while keeping the runtime fix unchanged. All 11 tests pass, and the local eval still produces the expected suggestions for all six inputs. |
There was a problem hiding this comment.
👍 All Clear
Reviewed changes to the indirect prompt injection grader and its tests. The update switches to a literal replacer function for datamarking, preventing special $-sequence expansion, which strengthens defenses. Based on the diff, no new LLM security risks were introduced.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
mldangelo-oai
left a comment
There was a problem hiding this comment.
The consolidated table keeps the ordinary case and every active dollar-pattern regression with clearer exact-output assertions. Runtime behavior is unchanged. Independent review is clean and all reported CI checks pass on this head. Approved; leaving the PR open.
|
Thanks! |
Datamarking could interpret dollar patterns inside an injection as replacement syntax, reintroducing unmarked text or duplicating surrounding text. Insert the marked text with a replacement callback.
One exact-output table covers ordinary input, all four active dollar patterns, and repeated combinations. Validation: 11 focused tests pass; a fresh local eval verified both suggestions for six inputs.