Skip to content

Fix missing bounds check in MCP handlers#61

Merged
wesm merged 1 commit intowesm:mainfrom
hughdbrown:bug-missing-bounds-check
Feb 5, 2026
Merged

Fix missing bounds check in MCP handlers#61
wesm merged 1 commit intowesm:mainfrom
hughdbrown:bug-missing-bounds-check

Conversation

@hughdbrown
Copy link
Contributor

The getAttachment and exportAttachment MCP handlers had att.Size (database metadata) available but never checked it against maxAttachmentSize before calling readAttachmentFile. This meant:

  • Unnecessary file I/O (open + stat) for attachments already known to be too large
  • Misleading error messages ("file not available" instead of "too large")
  • If the file existed, the full stat + LimitReader pipeline would execute before rejecting

The Fix (2 lines of logic, added to 2 call sites)

Added a pre-flight att.Size > maxAttachmentSize check in both getAttachment (line 153) and exportAttachment (line 219), immediately after confirming the attachments directory is configured and before any file I/O.

Changes Made

  • internal/mcp/handlers.go: Added pre-flight size check in both handlers (2x 3 lines)
  • internal/mcp/server_test.go: Added TestGetAttachment_RejectsOversizedBeforeFileIO and TestExportAttachment_RejectsOversizedBeforeFileIO (44 lines)

  The getAttachment and exportAttachment MCP handlers had att.Size (database metadata) available but never checked it against
  maxAttachmentSize before calling readAttachmentFile. This meant:
  - Unnecessary file I/O (open + stat) for attachments already known to be too large
  - Misleading error messages ("file not available" instead of "too large")
  - If the file existed, the full stat + LimitReader pipeline would execute before rejecting

  The Fix (2 lines of logic, added to 2 call sites)

  Added a pre-flight att.Size > maxAttachmentSize check in both getAttachment (line 153) and exportAttachment (line 219), immediately
  after confirming the attachments directory is configured and before any file I/O.

  Changes Made

  - internal/mcp/handlers.go: Added pre-flight size check in both handlers (2x 3 lines)
  - internal/mcp/server_test.go: Added TestGetAttachment_RejectsOversizedBeforeFileIO and
  TestExportAttachment_RejectsOversizedBeforeFileIO (44 lines)
@wesm wesm force-pushed the bug-missing-bounds-check branch from 40b8ae6 to 9eb4f90 Compare February 5, 2026 03:12
@wesm
Copy link
Owner

wesm commented Feb 5, 2026

rebased to get windows CI

@wesm wesm merged commit 4472abd into wesm:main Feb 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants