fix: resolve 9 bugs across agent, memory compressor, and handler#9
fix: resolve 9 bugs across agent, memory compressor, and handler#9cybersecua merged 1 commit intomainfrom
Conversation
cybersecua
commented
Mar 6, 2026
cybersecua
commented
- isRetryableError: fix "EOF" case mismatch with strings.ToLower (was never matching)
- getAvailableTools: propagate caller context instead of context.Background() so task cancellation correctly stops external MCP queries
- convertToOpenAIType: map "int"/"integer" to "integer" (not "number") to conform to JSON Schema/OpenAI spec
- attachmentContentToBytes: use MimeType field to determine encoding instead of opportunistic try-and-see base64 decode that could corrupt plain-text files
- summarizeChunk: fallback on empty summary now preserves all chunk messages instead of silently discarding all but the first
- countTotalTokens: include ToolCalls JSON when counting message tokens so compression triggers at the correct threshold
- AgentLoopWithProgress: remove duplicate comment
- AgentLoopStream: consolidate two separate role-config lookups into one, ensuring roleSkills is captured alongside roleTools
- saveAttachmentsToDateAndConversationDir: enforce 50 MB per-file size limit to prevent disk/memory exhaustion
- isRetryableError: fix "EOF" case mismatch with strings.ToLower (was never matching) - getAvailableTools: propagate caller context instead of context.Background() so task cancellation correctly stops external MCP queries - convertToOpenAIType: map "int"/"integer" to "integer" (not "number") to conform to JSON Schema/OpenAI spec - attachmentContentToBytes: use MimeType field to determine encoding instead of opportunistic try-and-see base64 decode that could corrupt plain-text files - summarizeChunk: fallback on empty summary now preserves all chunk messages instead of silently discarding all but the first - countTotalTokens: include ToolCalls JSON when counting message tokens so compression triggers at the correct threshold - AgentLoopWithProgress: remove duplicate comment - AgentLoopStream: consolidate two separate role-config lookups into one, ensuring roleSkills is captured alongside roleTools - saveAttachmentsToDateAndConversationDir: enforce 50 MB per-file size limit to prevent disk/memory exhaustion https://claude.ai/code/session_017MtB57yFAgBeBhNyo7vfkp
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses nine distinct bugs and improvements across the agent, memory compressor, and handler components. The changes focus on enhancing the robustness, correctness, and resource management of the application, including better error handling, accurate token counting, improved attachment processing, and streamlined agent configuration. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of bug fixes and improvements across the agent, memory compressor, and handler components. However, a security audit identified two critical vulnerabilities in the attachment handling logic: a path traversal vulnerability due to insufficient filename sanitization and a potential Denial of Service (DoS) via memory exhaustion during base64 decoding of large attachments. These must be addressed to ensure the robustness of the file upload functionality. The PR also includes fixes for incorrect context propagation to external MCP queries, more robust attachment decoding using MIME types, and enhancements to the memory compressor for tool call tokens and summarization failures. A further improvement would be to expand the list of recognized MIME types for binary attachments.
| if len(raw) > maxAttachmentSizeBytes { | ||
| return nil, fmt.Errorf("attachment %s exceeds maximum allowed size of %d MB", a.FileName, maxAttachmentSizeBytes/(1024*1024)) | ||
| } |
There was a problem hiding this comment.
The function saveAttachmentsToDateAndConversationDir is vulnerable to path traversal. While it uses filepath.Base to extract the filename from a.FileName, this is insufficient on Unix-like systems if the filename contains backslashes (\). An attacker can provide a filename like ..\..\..\etc\passwd, which filepath.Base will return unchanged on Linux. When this is subsequently used in filepath.Join to construct the fullPath, it allows writing files outside the intended targetDir. Although this PR adds a size check, it does not address the underlying path traversal vulnerability in the filename handling logic.
| if a.MimeType != "" && (strings.HasPrefix(a.MimeType, "image/") || | ||
| a.MimeType == "application/octet-stream" || | ||
| a.MimeType == "application/pdf") { | ||
| decoded, err := base64.StdEncoding.DecodeString(content) |
There was a problem hiding this comment.
The attachmentContentToBytes function decodes base64-encoded content from the user request before any size validation is performed. An attacker can send a request with a very large base64 string and a binary MimeType, causing the server to attempt to allocate a large amount of memory for the decoded bytes. This can lead to memory exhaustion and a Denial of Service (DoS) condition. The size check added in saveAttachmentsToDateAndConversationDir occurs after the decoding process, which is too late to prevent the initial memory allocation.
| decoded, err := base64.StdEncoding.DecodeString(content) | |
| if len(content) > maxAttachmentSizeBytes * 4 / 3 { | |
| return nil, fmt.Errorf("attachment content exceeds maximum allowed size") | |
| } | |
| decoded, err := base64.StdEncoding.DecodeString(content) |
| if a.MimeType != "" && (strings.HasPrefix(a.MimeType, "image/") || | ||
| a.MimeType == "application/octet-stream" || | ||
| a.MimeType == "application/pdf") { |
There was a problem hiding this comment.
The list of MIME types that are treated as base64-encoded seems incomplete. It currently covers images, PDFs, and generic binary streams. However, other common binary file types like audio, video, and various document/archive formats (e.g., zip) would be incorrectly treated as plain text, potentially leading to data corruption when they are saved.
Consider expanding this list to include more common binary MIME type prefixes and types.
| if a.MimeType != "" && (strings.HasPrefix(a.MimeType, "image/") || | |
| a.MimeType == "application/octet-stream" || | |
| a.MimeType == "application/pdf") { | |
| if a.MimeType != "" && (strings.HasPrefix(a.MimeType, "image/") || | |
| strings.HasPrefix(a.MimeType, "audio/") || | |
| strings.HasPrefix(a.MimeType, "video/") || | |
| a.MimeType == "application/octet-stream" || | |
| a.MimeType == "application/pdf" || | |
| a.MimeType == "application/zip") { |
2 participants
