Skip to content

Refuse MCP registry writes through a planted path.tmp symlink - #364

Merged
tylerreckart merged 8 commits into
mainfrom
fix/mcp-registry-tmp-nofollow
Sep 21, 2026
Merged

tylerreckart merged 8 commits into
mainfrom
fix/mcp-registry-tmp-nofollow

Conversation

@cursor

@cursor cursor Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Problem

save_server_registry staged through open(<path>.tmp, O_WRONLY|O_CREAT|O_TRUNC). That follows a symlink at the staging name. --setup-tools persists ~/.arbiter/mcp_servers.json this way, and registry env blocks may hold secrets. A planted <path>.tmp → elsewhere link was truncated with the registry JSON; rename then replaced only the symlink, so the victim file stayed clobbered. Reproduced: mcp_servers.json.tmp → secret.txt left the victim as session-body-with-TOKEN=super-secret.

rename over a dest symlink already replaced the link (not its target). The hole was the staging name.

Fix

unlink the staging name (does not follow), then open with O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW|O_CLOEXEC and keep the existing fchmod(0600) / dest chmod(0600). Short writes retry on EINTR. Isolated src/mcp/manager.cpp (same helper header comment + tests).

Tests

unit_mcp save_server_registry* — existing round-trip and empty-argv cases, leftover regular .tmp, planted .tmp symlink does not clobber the target (victim stays do-not-clobber, dest is 0600 and loads TOKEN=super-secret), dest symlink is replaced rather than written through. 5/5 locally (33 assertions, + ASan ×3, + UBSan, + TSan).

Independently mergeable against main (0456350). Does not overlap #344 (subprocess/env) or #363 (atomic_file). git merge-tree --write-tree vs main, #337, #343, #344, #360, #362, #363 CLEAN. CHANGELOG [Unreleased] is the usual sibling note.

Open in Web View Automation 

Note

Medium Risk
Changes secret-bearing MCP registry persistence on disk; behavior is safer but could fail writes if a hostile .tmp cannot be unlinked, and symlink edge cases deserve regression coverage.

Overview
Hardens atomic writes for ~/.arbiter/mcp_servers.json so a planted <path>.tmp symlink cannot redirect registry JSON (including env secrets) into an attacker-chosen file. save_server_registry now unlinks the staging name first, then opens it with O_CREAT|O_EXCL|O_NOFOLLOW|O_CLOEXEC (same pattern as atomic_write_file), keeps fchmod(0600) / final chmod, and retries write on EINTR.

Documents the guarantee in include/mcp/manager.h, adds an [Unreleased] CHANGELOG entry, and unit_mcp tests for planted .tmp symlinks, stale regular .tmp files, and destination symlinks replaced by rename without clobbering link targets.

Reviewed by Cursor Bugbot for commit e0a37e1. Bugbot is set up for automated code reviews on this repo. Configure here.

save_server_registry opened <path>.tmp with O_CREAT|O_TRUNC, so a
symlink at that staging name redirected the write (including registry
env secrets) into the link target before rename replaced only the
symlink. unlink first (does not follow), then open with
O_NOFOLLOW|O_EXCL. Dest-symlink rename already replaced the link.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
@tylerreckart
tylerreckart marked this pull request as ready for review September 21, 2026 12:58
@tylerreckart
tylerreckart enabled auto-merge (squash) September 21, 2026 12:58
cursoragent and others added 6 commits September 21, 2026 12:59
Keep both Unreleased fixes after #366 landed: MCP registry tmp-symlink
nofollow and remote --connect base URL query/userinfo.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
Keep the MCP registry tmp-symlink Unreleased note alongside #363's
atomic_write_file staging-name fix after that PR landed on main.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
Keep the MCP registry tmp-symlink note under Unreleased after the
v0.13.7 changelog cut moved sibling fixes into that release section.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
@tylerreckart
tylerreckart merged commit 296800f into main Sep 21, 2026
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.

3 participants