[UPDATE PRIMITIVE] Fix markdown injection and platform-dependent path tests in prompt handlers#162
Conversation
…ndlers
- Add sanitizeForInlineCode() helper to escape backticks and newlines in user-supplied values embedded in markdown code spans
- Apply sanitizer to resolvePromptFilePath 'does not exist' warning (filePath and absolutePath)
- Apply sanitizer to formatValidationError issue.received display
- Fix POSIX path separator assumptions in tests: use basename only ('mydb', 'database')
- Rename createSafePromptHandler tests to clarify they validate the handler wrapper, not MCP SDK validation
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/2660567b-5822-4505-91c2-37fe3ef00f4e
There was a problem hiding this comment.
Pull request overview
Fixes markdown formatting corruption in prompt warnings/errors by sanitizing user-supplied values embedded in inline code spans, and makes path-related tests platform-agnostic (Windows-safe).
Changes:
- Add
sanitizeForInlineCode()and apply it to interpolated file paths and enum “received” values. - Update tests to assert on basename tokens instead of POSIX path fragments.
- Update built
server/distbundle with the new sanitization logic.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server/src/prompts/workflow-prompts.ts | Adds and uses inline-code sanitization for markdown output. |
| server/test/src/prompts/workflow-prompts.test.ts | Adjusts assertions to be platform-agnostic and clarifies test names. |
| server/dist/codeql-development-mcp-server.js | Regenerated build output reflecting the sanitization changes. |
…eCode - markdownInlineCode() uses a fence length = maxRun+1 per CommonMark spec, preserving the original string (no information loss from backtick→apostrophe) - Normalises CR/CRLF to LF before wrapping (inline spans can't span lines) - Export markdownInlineCode for testability - Add 6 unit tests for markdownInlineCode (plain text, single/double backtick, CRLF normalisation, backtick-only values) - Add regression test for formatValidationError with backtick in received value - Add regression test for resolvePromptFilePath warning with backtick in path Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com> Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/ec7c534b-93ac-40e5-bcb6-023bc7496940
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
server/src/prompts/workflow-prompts.ts:154
- resolvePromptFilePath() now uses markdownInlineCode() for the "does not exist" warning, but the earlier "is not a valid file URI" warning still wraps the raw filePath in a single-backtick code span. That path can also contain backticks/newlines, so the same markdown-injection issue remains for invalid file:// inputs. Apply markdownInlineCode() there as well for consistent sanitization.
return {
resolvedPath: absolutePath,
warning: `⚠ **File path** ${markdownInlineCode(filePath)} **does not exist.** Resolved to: ${markdownInlineCode(absolutePath)}`,
};
|
@copilot apply changes based on the comments in this thread |
…e output Replace \r\n, \r, and \n with a space (not just normalize CRLF to LF) so the returned inline code span never contains a literal newline character. Update docstring and test to reflect space-replacement behavior. Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com> Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/87cfd54e-9d66-4871-a581-601aff3c6c8d
Done in commit d49aff0. |
User-supplied path values and Zod validation error details were embedded raw into markdown code spans, allowing backticks or newlines in inputs to corrupt prompt output formatting. Test assertions used POSIX-specific path fragments that would fail on Windows after relative-to-absolute path resolution.
📝 Update Information
Primitive Details
workflow-prompts(resolvePromptFilePath,formatValidationError,markdownInlineCode)✅ ALLOWED FILES:
server/src/**/*.ts)server/test/**/*.ts)🚫 FORBIDDEN FILES: None included.
🛑 MANDATORY PR VALIDATION CHECKLIST
Update Metadata
markdownInlineCodeexport added)🎯 Changes Description
Current Behavior
resolvePromptFilePathembeds the rawfilePathandabsolutePathstrings in backtick code spans.formatValidationErrorembedsissue.receivedraw. Any value containing a backtick prematurely closes the code span; newlines break the markdown line. Test assertions liketoContain('path/to/mydb')pass on POSIX but fail on Windows once paths are resolved to OS-native absolute form.Updated Behavior
A
markdownInlineCode()helper wraps user-supplied strings in a CommonMark-compliant backtick fence whose length exceeds the longest backtick run in the value, so the original string is preserved without mutation. All newline sequences (CR, LF, CRLF) are replaced with a single space so the returned span is always a single line. Path-fragment test assertions use basename-only tokens.createSafePromptHandlererror-path test names explicitly state what they are validating.Motivation
Malformed user input (e.g. a filename containing backticks or newlines) could silently corrupt the structured markdown returned to the LLM. Replacing backticks with apostrophes (a prior approach) caused information loss. Normalising CRLF/CR to LF (an intermediate approach) still allowed literal
\nto break surrounding markdown formatting. Test suite was implicitly POSIX-only.🔄 Before vs. After Comparison
Functionality Changes
API Changes
No schema changes.
markdownInlineCodeis exported as a new testable utility.Output Format Changes
Warning and validation-error messages are semantically unchanged; values containing backticks are now displayed using a longer fence (e.g.
a`b) rather than corrupting the markdown structure or mutating the value. Values containing newlines have the newlines replaced with spaces so the inline code span is always single-line.🧪 Testing & Validation
Test Coverage Updates
markdownInlineCode(plain text, single backtick, double-backtick run, CRLF/CR/LF→space, backtick-only value)formatValidationErrorwith a backtick inissue.received; added regression test forresolvePromptFilePathwarning with a backtick in the path; path-fragment assertions made platform-agnosticcreateSafePromptHandlerwrapper behavior from MCP SDK protocol-layer validationValidation Scenarios
resolvedPath.markdownInlineCodecovers single-backtick, multi-backtick runs, and all newline variants (\n,\r\n,\r) — replacing newlines with spaces and preserving all other content.formatValidationErrornow safe for anyissue.receivedvalue.Test Results
codeql_pack_installfailure unrelated to these changes📋 Implementation Details
Files Modified
server/src/prompts/workflow-prompts.tsserver/test/src/prompts/workflow-prompts.test.tsCode Changes Summary
markdownInlineCodeapplied at interpolation sitesDependencies
🔍 Quality Improvements
Bug Fixes (if applicable)
\nin the output which could break surrounding markdown formattingmarkdownInlineCode()uses a CommonMark-compliant fence whose length exceeds any internal backtick run, and replaces all newline sequences with a space, guaranteeing single-line output while preserving the original contentCode Quality Enhancements
createSafePromptHandlerwrapper vs SDK validation); newline test updated to assert space-replacement rather than LF-normalisationmarkdownInlineCodeis exported and independently tested🔗 References
Related Issues/PRs
🚀 Compatibility & Migration
Backward Compatibility
API Evolution
markdownInlineCodeutility👥 Review Guidelines
For Reviewers
Please verify:
markdownInlineCodecorrectly handles backtick runs of all lengths and replaces all newline variants with a spaceTesting Instructions
📊 Impact Assessment
Server Impact
AI Assistant Impact
🔄 Deployment Strategy
Rollout Considerations
Update Methodology: This update follows best practices:
⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.