Skip to content

fix(redteam): keep $ sequences literal when datamarking an injection - #11381

Merged
mldangelo-oai merged 4 commits into
promptfoo:mainfrom
chrikrah:chrikrah/datamark-dollar-sequences
Oct 8, 2026
Merged

mldangelo-oai merged 4 commits into
promptfoo:mainfrom
chrikrah:chrikrah/datamark-dollar-sequences

Conversation

@chrikrah

@chrikrah chrikrah commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

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.

`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

codecov Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.42%. Comparing base (2041ef9) to head (de595d3).
⚠️ Report is 1 commits behind head on main.

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     
Flag Coverage Δ
backend 88.69% <100.00%> (+<0.01%) ⬆️
site 26.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@promptfoo-scanner promptfoo-scanner Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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


Was this helpful?  👍 Yes  |  👎 No 

Copy link
Copy Markdown
Contributor

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.

@promptfoo-scanner promptfoo-scanner Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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


Was this helpful?  👍 Yes  |  👎 No 

@promptfoo-scanner promptfoo-scanner Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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


Was this helpful?  👍 Yes  |  👎 No 

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

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.

@promptfoo-scanner promptfoo-scanner Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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


Was this helpful?  👍 Yes  |  👎 No 

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mldangelo-oai

Copy link
Copy Markdown
Contributor

Thanks!

@mldangelo-oai
mldangelo-oai merged commit 667ebe3 into promptfoo:main Oct 8, 2026
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants