fix: Prevent context attributes from influencing judge template parsing#1258
fix: Prevent context attributes from influencing judge template parsing#1258jsonbailey wants to merge 6 commits intomainfrom
Conversation
Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
@cursor review |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
…t rule Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>
Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fcc72ae. Configure here.
… injection Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>
Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>
| */ | ||
| private _interpolateMessage(content: string, variables: Record<string, string>): string { | ||
| return Mustache.render(content, variables, undefined, { escape: (item: any) => item }); | ||
| return content.replace(/\{\{(\w+)\}\}/g, (match, key) => |
There was a problem hiding this comment.
Just want to make sure that we are okay with this matching since \w+ will only match alphanumerics and _ (?). So whitespace and some common special characters will not pass.
There was a problem hiding this comment.
Correct — \w+ matches [a-zA-Z0-9_]. The only two placeholders used in pass 2 are message_history and response_to_evaluate, which are purely alphanumeric + underscores. Anything that doesn't match a key in the variables map is left as-is (the callback returns match for unknown keys), so non-matching patterns like {{> partial}} or {{=[ ]=}} pass through untouched.
Co-Authored-By: jbailey@launchdarkly.com <accounts@sidewaysgravity.com>

Requirements
Related issues
Addresses SEC-8020. Mirrors the fix applied in the Go server SDK (
go-server-sdk@3317871). A matching Python SDK PR has also been opened inlaunchdarkly/python-server-sdk-ai.Describe the solution you've provided
The Judge's
_interpolateMessagepreviously usedMustache.render()to substitute{{message_history}}and{{response_to_evaluate}}placeholders (pass 2). Because pass 1 (LDAIClientImpl) already resolves user/context attributes via Mustache, an attacker who controls a context attribute could inject Mustache control sequences (e.g.{{=[ ]=}}delimiter-change tags) that corrupt pass 2, effectively blinding the judge to the actual conversation content.This PR replaces the Mustache call in
_interpolateMessagewith a literalObject.entries().reduce()usingsplit().join()for string replacement. Since the judge only ever substitutes two known placeholders, a full template engine is unnecessary. Themustachedependency remains becauseLDAIClientImplstill uses it for general config interpolation (pass 1).Key changes:
Judge.ts: RemoveMustacheimport; replaceMustache.render()withreduce+split().join()over the known placeholder keys.Judge.test.ts: Addtemplate injection preventiondescribe block with regression tests covering delimiter changes, partials, comments, triple-stache, sections, inverted sections, injection via response, multiple placeholder occurrences, and preservation of Mustache-like syntax inside substituted values.Describe alternatives you've considered
String.prototype.replaceAll()— not available with thees2020target;split().join()is the standard workaround.for...ofloop — disallowed by the repo's ESLintno-restricted-syntaxrule;reduceis the idiomatic alternative.Additional context
Reviewers should verify:
split().join()pattern correctly handles all occurrences of each placeholder (it does —splitwith a string literal is equivalent to a global literal replace).mustachepackage remains independenciessinceLDAIClientImpl.tsstill imports it for pass 1._constructEvaluationMessagesvia(judge as any)cast — this is the simplest way to exercise the interpolation logic end-to-end without mocking the full provider invocation.Link to Devin session: https://app.devin.ai/sessions/651e799b906748a4834bafefb4a3e3e5
Requested by: @jsonbailey
Note
Medium Risk
Touches judge prompt construction logic to mitigate a template-injection vulnerability; while localized, it changes how placeholders are detected/replaced and could affect existing judge message templates that relied on Mustache behavior.
Overview
Hardens
Judgeprompt construction against template injection.Judge._interpolateMessageno longer usesMustache.render; it now performs a literal{{key}}string replacement for known variables so attacker-controlled strings like{{=[ ]=}}cannot alter parsing in the second interpolation pass.Adds a new
template injection preventiontest suite covering delimiter changes, sections/partials/comments, injection via response/history, multiple placeholder occurrences, and ensuring Mustache-like syntax in substituted values is preserved as literal text.Reviewed by Cursor Bugbot for commit f24a371. Bugbot is set up for automated code reviews on this repo. Configure here.