fix(mcp): survive atomic binary replacement#1204
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens Linux subprocess spawning against atomic replacement of the running binary by preferring a previously saved launch path when it remains executable and refusing to use a deleted /proc/self/exe target before spawning index workers. It also adds a Linux regression test that exercises the “deleted self” scenario by forking, atomically replacing the on-disk executable, and validating that subprocess resolution still uses the saved launch path.
Changes:
- Update
cbm_http_server_resolve_binary_pathto preferg_binary_pathwhen it still points to an executable, and to reject non-executable self paths on Linux. - Add a Linux-only “deleted-self” probe mode to the test runner so a fork/exec’d child can validate resolution behavior after atomic replacement.
- Add a Linux integration regression test covering atomic replacement while the child is running.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_main.c | Adds a Linux-only probe entrypoint used by the new integration test to validate binary-path resolution in a fork/exec child. |
| tests/test_httpd.c | Adds a Linux regression test that forks, atomically replaces the running executable on disk, and asserts resolution prefers the saved launch path. |
| src/ui/http_server.c | Adjusts binary-path resolution to prefer the saved executable path when valid and to avoid returning a deleted /proc/self/exe target as spawnable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ssize_t current_len = readlink("/proc/self/exe", current, sizeof(current) - 1); | ||
| ASSERT_GT(current_len, 0); | ||
| current[current_len] = '\0'; |
e161726 to
9c5e912
Compare
Prefer the saved executable launch path when it still points to an executable, and reject a deleted /proc/self/exe path before spawning index workers. Add a Linux integration regression that forks, atomically replaces the running test binary on disk, and verifies that subprocess resolution uses the saved launch path instead of the now-deleted self path. Signed-off-by: David Musk <davidmusk93@users.noreply.github.com> Signed-off-by: sunmingqiang <sunmingqiang@bytedance.com>
9c5e912 to
f891b2c
Compare
DeusData
added
stability/performance
|
Thanks for this, and sorry for the slow acknowledgement. Queued for review. Two things to clear first: the branch is CONFLICTING against Surviving atomic binary replacement is a real scenario — self-update and package-manager upgrades both hit it — so this is a welcome area to harden. |
Summary
/proc/self/exepath before spawning index workersTests
build/c/test-runner httpd: 44 passed, 1 Windows-only skipscripts/run-tests-parallel.sh build/c/test-runner 16: 6386 passed, 1 unrelated baseline failure, 1 Windows-only skipThe baseline failure is
tests/test_cli.c:6781(installer must not follow symlinked agent roots outside the selected home). It is unrelated to the three files in this PR and is fixed by a separate local change, intentionally excluded here.Runtime verification
A static build carrying this patch was loaded by both host and container MCP instances. A real
index_repositorycall completed with 17,237 nodes, 99,775 edges,skipped_count=0, andparse_partial_count=0.