-
Notifications
You must be signed in to change notification settings - Fork 141
Add template syntax sanitization to prevent injection bypass (T24) #15015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Add neutralizeTemplateDelimiters function to sanitize_content_core.cjs
- Detects and escapes Jinja2/Liquid ({{), ERB (<%=), JS template literals (${), Jinja2 comments ({#), and Jekyll directives ({%)
- Logs info messages for each detected template pattern
- Logs warning message summarizing defense-in-depth approach
- Integrates template neutralization into sanitizeContentCore pipeline
- Add comprehensive test suite covering all template types and edge cases
- All 233 tests passing
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Create comprehensive documentation in scratchpad/template-syntax-sanitization.md - Explains the T24 security concern and solution - Documents all template patterns detected and their escaping strategy - Includes defense-in-depth rationale and test coverage summary - Manual testing confirms all T24 test payloads are properly escaped Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Keep PR focused on template sanitization only
- Fix comment in sanitize_content_core.cjs to accurately describe ERB escaping - Revert unrelated lock file recompilation changes to keep PR focused Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Use explicit newline characters instead of template literal
- Makes test clearer that we're testing ${ not \$
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🧪 Smoke Project is now testing project operations... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
✅ Changeset Generator completed successfully! |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements template syntax sanitization to prevent potential template injection attacks as a defense-in-depth security measure. While GitHub's markdown rendering doesn't currently evaluate template syntax, this change protects against scenarios where content might be processed by downstream template engines (Jinja2, Liquid, ERB, JavaScript template literals, Jekyll).
Changes:
- Added
neutralizeTemplateDelimiters()function that escapes template delimiters using backslash escaping (e.g.,{{→\{\{) - Integrated the function into the core sanitization pipeline between bot trigger neutralization and markdown code region balancing
- Added 17 comprehensive test cases covering all template types and edge cases
- Created detailed documentation explaining the security rationale, implementation details, and usage
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scratchpad/template-syntax-sanitization.md |
Comprehensive documentation describing the template injection defense, patterns detected, implementation details, and security rationale |
actions/setup/js/sanitize_content_core.cjs |
Implementation of neutralizeTemplateDelimiters() function with backslash escaping for 5 template types, integration into sanitization pipeline, and proper exports |
actions/setup/js/sanitize_content.test.cjs |
17 new test cases covering individual template types, multiple occurrences, mixed patterns, edge cases (double-escaping, code blocks, GitHub Actions expressions, nested patterns) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Agent Container Tool Check
Result: 12/12 tools available ✅ All required development tools are present and functional in the agent container environment.
|
|
✅ Smoke Project completed successfully. All project operations validated. |
|
PRs: #15018 Sort GH_AW_RATE_LIMIT_EVENTS alphabetically
|
Smoke Test ResultsStatus: ✅ PASS Tests:
Run: §21926186000 cc @pelikhan
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Template delimiters (Jinja2, Liquid, ERB, JavaScript, Jekyll) weren't being sanitized. Defense-in-depth gap if content flows to downstream template engines.
Changes
Added
neutralizeTemplateDelimiters()tosanitize_content_core.cjs{{→\{\{,<%=→\<%=,${→\$\{,{#→\{#,{%→\{%Test coverage for all patterns, edge cases (nested, already-escaped, in code blocks)
Documentation in
scratchpad/template-syntax-sanitization.mdExample
Context
GitHub markdown doesn't evaluate template syntax (no current risk), but this prevents issues if content is later processed by template engines - standard defense-in-depth.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Changeset
sanitize_content_core.cjsto prevent downstream template injection bypasses.