Skip to content

MCP server: allow absolute attachment paths via operator-configured roots - #952

Open
chesterXalan wants to merge 1 commit into
mattermost:masterfrom
chesterXalan:feat/mcp-attachment-roots
Open

MCP server: allow absolute attachment paths via operator-configured roots#952
chesterXalan wants to merge 1 commit into
mattermost:masterfrom
chesterXalan:feat/mcp-attachment-roots

Conversation

@chesterXalan

@chesterXalan chesterXalan commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Split from #888 to keep reviews small and focused. This part adds operator-configured roots for absolute attachment paths in local access mode:

  • New environment variable MM_MCP_ATTACHMENT_ROOTS (OS path-list of directories, e.g. /srv/exports:/var/reports) allows local-mode attachments to reference absolute paths. Each root is opened through os.Root, so symlinks and path traversal cannot escape an allowed directory.
  • Relative paths still resolve inside the data directory. Absolute paths are rejected when no roots are configured, so default behavior is unchanged.

QA test steps:

  1. Without the environment variable, pass an absolute path as an attachment → the tool call fails with guidance mentioning MM_MCP_ATTACHMENT_ROOTS.
  2. With MM_MCP_ATTACHMENT_ROOTS=/tmp/exports, attach /tmp/exports/report.pdf → the file uploads. Attach a symlink under /tmp/exports pointing outside it → rejected.

Ticket Link

None. Supersedes part of #888.

Release Note

Added the MM_MCP_ATTACHMENT_ROOTS environment variable to the standalone MCP server, allowing local-mode attachments to reference absolute paths under operator-configured directories.

Summary by CodeRabbit

  • New Features

    • Added support for accessing local attachments through configured absolute-path roots.
    • Supports multiple attachment roots while continuing to support relative paths.
    • Restricts access to approved locations and rejects traversal, symlink escapes, and files outside configured roots.
    • Maintains the existing 100 MiB file-size limit.
  • Bug Fixes

    • Improved validation for attachment paths and configured root directories.

- MM_MCP_ATTACHMENT_ROOTS (OS path-list of directories) lets local-mode
  attachments use absolute paths; each root is opened through os.Root
  so symlinks and path traversal cannot escape the allowed directory
- relative paths still resolve inside the data directory; absolute
  paths are rejected when no roots are configured
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 73f84525-e5f9-497f-9948-0df1627eba51

📥 Commits

Reviewing files that changed from the base of the PR and between f4e52d2 and 10d0b7e.

📒 Files selected for processing (2)
  • mcpserver/tools/file_utils.go
  • mcpserver/tools/file_utils_test.go

📝 Walkthrough

Walkthrough

The file utilities now support absolute local attachment paths through configured MM_MCP_ATTACHMENT_ROOTS values. The implementation validates roots, blocks path escapes, enforces the 100 MiB limit, and preserves relative-path handling. Tests cover permitted and rejected paths.

Changes

Local attachment root support

Layer / File(s) Summary
Attachment-root resolution and enforcement
mcpserver/tools/file_utils.go, mcpserver/tools/file_utils_test.go
AttachmentRootsEnvVar and root-based reading support absolute attachment paths. Root validation, traversal protection, symlink protection, and the 100 MiB limit are enforced. Relative paths continue to use the MCP data directory. Table-driven tests cover valid roots and rejected paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: Setup Cloud Test Server

Sequence Diagram(s)

sequenceDiagram
  participant FetchFileDataForLocal
  participant readLocalFileFromAllowedRoots
  participant os.Root
  participant Filesystem
  FetchFileDataForLocal->>readLocalFileFromAllowedRoots: absolute attachment path
  readLocalFileFromAllowedRoots->>os.Root: open configured root
  os.Root->>Filesystem: read validated file
  Filesystem-->>readLocalFileFromAllowedRoots: file data
  readLocalFileFromAllowedRoots-->>FetchFileDataForLocal: size-limited data or error
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: operator-configured roots for absolute attachment paths in the MCP server.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant