Repository navigation
fix(security): close three untrusted-content gaps in AI and MCP paths - #405
Merged
Merged
Conversation
Conflict region text bypassed the boundary-marker wrapping applied to the surrounding file content, so a crafted branch could inject instructions into the resolution prompt (CWE-1427). Regions now go through SanitizeExternalContent like everything else, which also defuses any embedded terminator. Chat tool results were boundary-marked but never secret-redacted before reaching the provider. The filename denylist only covers names, so a docker-compose.yml or terraform.tfvars holding an inline credential sailed through. Tool results now redact fail-closed, matching how user input is handled in sendMessage (CWE-201). file_list did not apply IsSensitivePath, so it leaked the names and sizes of .env files, SSH keys, and credential stores even though file_read blocks reading them (CWE-200). Benign dot-files such as .gitignore still list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c5a9c45a-0341-46e5-8458-f4060ddd58c7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the three untrusted-content findings from the release MQ pass. None were release blockers, all are cheap to fix.
Conflict regions bypassed prompt sanitization (CWE-1427)
buildConflictUserPromptwrapped full file content in boundary markers but wrote the individual conflict regions raw. Since region text comes from the branches being merged, a crafted branch could inject instructions into the resolution prompt. Regions now go throughSanitizeExternalContent, which also defuses any embedded terminator. Per-region human approval was already required, which is why this stayed medium rather than high.Tool results were never redacted (CWE-201)
sendMessageredacts user input fail-closed, buthandleToolResultsonly boundary-marked. Tool results carry file contents and git output, and the redactor's filename denylist only covers names, so adocker-compose.ymlorterraform.tfvarswith an inline credential reached the provider intact. Tool results now redact the same way, falling back to the placeholder on error.file_list leaked sensitive filenames (CWE-200)
file_readandfile_writeboth callIsSensitivePath;file_listdid not. Reading was blocked but listing still exposed the names and sizes of.envfiles, SSH keys, and credential stores. Benign dot-files like.gitignorestill list, since the denylist already allowlists them.Testing
go test ./internal/mcp/... ./internal/chat/... ./internal/ai/...passes. Each fix has a regression test, including one asserting an injected[EXTERNAL_DATA_END]in conflict text gets defused rather than passed through.