Skip to content

Commit 5b96b06

Browse files
DeusDatasunmingqiang
andcommitted
fix(mcp): resolve deleted self-image per platform ruling
Maintainer follow-up on top of the contributor's rebased commit, implementing the Half-B strategy decision from the 2026-08-21 review: - Linux: when the self-resolved path fails the executable check after an atomic rename-over, hand back the /proc/self/exe magic link. The link still executes the in-memory OLD build, which is the only spawn the worker's build-fingerprint gate accepts. Deliberately NOT the saved launch path: after a replacement that path holds a DIFFERENT build, so preferring it swaps an ENOENT for a fingerprint refusal. - macOS: no magic link exists; fail closed. The supervisor logs index.supervisor.no_self_path and indexing resumes on daemon restart instead of spawning a missing binary. A rename-over that leaves a VALID binary still resolves (and the fingerprint gate refuses downstream, with its own coverage). - The prefer-saved-launch-path branch from the original head is dropped per the same ruling. Also fixes both Half-A review items: - Aliasing in cbm_http_server_set_binary_path: `path` may alias g_binary_path on a repeated call. The overlapping snprintf cannot actually fire today — resolve's leading out[0]='\0' zeroes the shared buffer first — but that same reset means the call silently DISCARDS the stored path it was asked to re-resolve, and the in-place write is one reset-reorder away from real UB. Resolve into a local first: correct input, no aliasing trap. - Coverage asymmetry: the probe and driver ran only on Linux while the production edit covered all of POSIX. Both now run on macOS too, and the Linux probe first proves the "(deleted)" state via readlink so its assertions cannot pass vacuously on an intact image. Revert-check: with main's http_server.c restored, the driver fails at WEXITSTATUS 44 (resolver returned a dead path) on macOS; with the guard it passes 63/63. Co-authored-by: sunmingqiang <sunmingqiang@bytedance.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 11bbf0d commit 5b96b06

3 files changed

Lines changed: 88 additions & 12 deletions

File tree

‎src/ui/http_server.c‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,11 +1053,29 @@ bool cbm_http_server_resolve_binary_path(const char *argv0, char *out, size_t ou
10531053
#endif
10541054

10551055
#ifndef _WIN32
1056-
if (g_binary_path[0] && is_executable_file(g_binary_path)) {
1057-
return copy_path(out, outsz, g_binary_path);
1058-
}
1059-
if (resolve_self_executable(out, outsz) && is_executable_file(out)) {
1060-
return true;
1056+
/* #1204: never hand back a self-resolved path we have not confirmed is
1057+
* executable. After an installer's atomic rename-over, the resolved image
1058+
* path no longer exists (Linux readlink reads "<path> (deleted)"), and an
1059+
* unvalidated return turns into a doomed worker spawn (ENOENT). */
1060+
if (resolve_self_executable(out, outsz)) {
1061+
if (is_executable_file(out)) {
1062+
return true;
1063+
}
1064+
#if defined(__linux__)
1065+
/* Deleted image: the /proc/self/exe magic link still executes the
1066+
* in-memory OLD build — the only spawn that satisfies the worker's
1067+
* build-fingerprint gate (a rename-over leaves the on-disk path
1068+
* holding a DIFFERENT build). Hand back the link, not the stale
1069+
* path. Deliberately NOT the saved launch path: preferring it swaps
1070+
* an ENOENT for a fingerprint refusal. */
1071+
return copy_path(out, outsz, "/proc/self/exe");
1072+
#else
1073+
/* macOS has no magic link. Fail closed: the supervisor logs
1074+
* index.supervisor.no_self_path and indexing resumes after a daemon
1075+
* restart, instead of spawning a missing or mismatched binary. */
1076+
out[0] = '\0';
1077+
return false;
1078+
#endif
10611079
}
10621080
#else
10631081
if (resolve_self_executable(out, outsz)) {
@@ -1069,7 +1087,15 @@ bool cbm_http_server_resolve_binary_path(const char *argv0, char *out, size_t ou
10691087

10701088
void cbm_http_server_set_binary_path(const char *path) {
10711089
if (path) {
1072-
if (!cbm_http_server_resolve_binary_path(path, g_binary_path, sizeof(g_binary_path))) {
1090+
/* Resolve into a local buffer first: `path` may alias g_binary_path
1091+
* on a repeated call. Resolving in place first hits resolve's leading
1092+
* out[0]='\0', which zeroes the shared buffer and silently discards
1093+
* the very path we were asked to re-resolve — and the in-place write
1094+
* is one reset-reorder away from an overlapping snprintf. */
1095+
char resolved[sizeof(g_binary_path)];
1096+
if (cbm_http_server_resolve_binary_path(path, resolved, sizeof(resolved))) {
1097+
snprintf(g_binary_path, sizeof(g_binary_path), "%s", resolved);
1098+
} else {
10731099
g_binary_path[0] = '\0';
10741100
}
10751101
}

‎tests/test_httpd.c‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
* RPC dispatch, transport limits, receive deadline, clean shutdown.
1212
*/
1313
#include "../src/foundation/compat.h"
14+
#if defined(__APPLE__)
15+
#include <mach-o/dyld.h> /* _NSGetExecutablePath — deleted-self driver */
16+
#endif
1417
#include "../src/foundation/compat_fs.h"
1518
#include "../src/foundation/compat_thread.h"
1619
#include "../src/foundation/log.h"
@@ -457,17 +460,22 @@ TEST(httpd_resolves_bare_binary_path_from_path) {
457460
#endif
458461
}
459462

460-
TEST(httpd_resolves_deleted_self_to_replaced_launch_path) {
461-
#if defined(__linux__)
463+
TEST(httpd_deleted_self_spawn_path_follows_platform_ruling) {
464+
#if defined(__linux__) || defined(__APPLE__)
462465
char tmpdir[256];
463466
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_httpd_deleted_self_XXXXXX");
464467
ASSERT_NOT_NULL(cbm_mkdtemp(tmpdir));
465468

466469
char current[1024];
470+
#if defined(__linux__)
467471
ssize_t current_len = readlink("/proc/self/exe", current, sizeof(current) - 1);
468472
ASSERT_GT(current_len, 0);
469473
ASSERT_LT(current_len, (ssize_t)(sizeof(current) - 1));
470474
current[current_len] = '\0';
475+
#else
476+
uint32_t current_sz = sizeof(current);
477+
ASSERT_EQ(_NSGetExecutablePath(current, &current_sz), 0);
478+
#endif
471479

472480
char launch_path[512];
473481
char replacement_path[512];
@@ -501,9 +509,21 @@ TEST(httpd_resolves_deleted_self_to_replaced_launch_path) {
501509
ASSERT_EQ(read(ready_pipe[0], &ready, 1), 1);
502510
ASSERT_EQ(ready, 'R');
503511

512+
#if defined(__linux__)
513+
/* Atomic rename-over: the installer's real move. /proc/self/exe now reads
514+
* "(deleted)" in the child, which is the state the magic-link contract
515+
* covers. */
504516
ASSERT_EQ(cbm_copy_file(current, replacement_path), 0);
505517
ASSERT_EQ(chmod(replacement_path, 0755), 0);
506518
ASSERT_EQ(rename(replacement_path, launch_path), 0);
519+
#else
520+
/* macOS: a rename-over leaves a VALID executable at the launch path, which
521+
* resolve is right to return (the worker's build-fingerprint gate is the
522+
* layer that refuses a mismatched build — covered by its own tests). The
523+
* fail-closed branch guards the image-GONE case, so remove the image. */
524+
(void)replacement_path;
525+
ASSERT_EQ(unlink(launch_path), 0);
526+
#endif
507527
ASSERT_EQ(write(continue_pipe[1], "G", 1), 1);
508528
close(ready_pipe[0]);
509529
close(continue_pipe[1]);
@@ -2341,7 +2361,7 @@ SUITE(httpd) {
23412361
RUN_TEST(httpd_query_param_edge_cases);
23422362
RUN_TEST(httpd_path_match_matrix);
23432363
RUN_TEST(httpd_resolves_bare_binary_path_from_path);
2344-
RUN_TEST(httpd_resolves_deleted_self_to_replaced_launch_path);
2364+
RUN_TEST(httpd_deleted_self_spawn_path_follows_platform_ruling);
23452365
RUN_TEST(repo_info_web_base_normalizes_to_https);
23462366
RUN_TEST(repo_info_strips_credentials_from_remote);
23472367

‎tests/test_main.c‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,15 @@ static int tf_maybe_run_mcp_idxfailclosed_probe(int argc, char **argv) {
609609
const char *cache_dir);
610610
alarm(20);
611611
return mcp_test_idxfailclosed_supervisor_start_check(argv[2], argv[3]);
612+
#else
613+
(void)argc;
614+
(void)argv;
615+
return -1;
616+
#endif
617+
}
618+
612619
static int tf_maybe_run_deleted_self_probe(int argc, char **argv) {
613-
#if defined(__linux__)
620+
#if defined(__linux__) || defined(__APPLE__)
614621
if (argc != 5 || strcmp(argv[1], "__cbm_deleted_self_probe") != 0) {
615622
return -1;
616623
}
@@ -625,10 +632,32 @@ static int tf_maybe_run_deleted_self_probe(int argc, char **argv) {
625632
return 42;
626633
}
627634
char resolved[1024];
628-
if (!cbm_http_server_resolve_binary_path(NULL, resolved, sizeof(resolved))) {
635+
bool ok = cbm_http_server_resolve_binary_path(NULL, resolved, sizeof(resolved));
636+
#if defined(__linux__)
637+
/* Contract (#1204 strategy ruling): after a rename-over, the resolver
638+
* hands back the /proc/self/exe magic link — the in-memory OLD build,
639+
* the only spawn the worker's build-fingerprint gate accepts. First
640+
* prove we really are in the deleted state, or the assertions below
641+
* would pass vacuously on an intact image. */
642+
char link_target[1024];
643+
ssize_t n = readlink("/proc/self/exe", link_target, sizeof(link_target) - 1);
644+
if (n <= 0) {
645+
return 45;
646+
}
647+
link_target[n] = '\0';
648+
if (strstr(link_target, " (deleted)") == NULL) {
649+
return 46;
650+
}
651+
if (!ok) {
629652
return 43;
630653
}
631-
return strcmp(resolved, argv[4]) == 0 && access(resolved, X_OK) == 0 ? 0 : 44;
654+
return strcmp(resolved, "/proc/self/exe") == 0 && access(resolved, X_OK) == 0 ? 0 : 44;
655+
#else
656+
/* macOS has no magic link: the ruling is fail-closed. Success here is
657+
* the resolver REFUSING, so the supervisor logs no_self_path instead of
658+
* spawning a missing or mismatched binary. */
659+
return ok ? 44 : 0;
660+
#endif
632661
#else
633662
(void)argc;
634663
(void)argv;
@@ -887,6 +916,7 @@ int main(int argc, char **argv) {
887916
int daemon_ipc_probe_rc = tf_maybe_run_daemon_ipc_lock_probe(argc, argv);
888917
if (daemon_ipc_probe_rc >= 0) {
889918
return daemon_ipc_probe_rc;
919+
}
890920
int deleted_self_rc = tf_maybe_run_deleted_self_probe(argc, argv);
891921
if (deleted_self_rc >= 0) {
892922
return deleted_self_rc;

0 commit comments

Comments
 (0)