Merge pull request #1204 from DavidMusk93/deleted-self-exec-fix

fix(mcp): survive atomic binary replacement

Author: DeusData

src/ui/http_server.c

@@ -1052,15 +1052,50 @@ bool cbm_http_server_resolve_binary_path(const char *argv0, char *out, size_t ou
     }
 #endif
 
+#ifndef _WIN32
+    /* #1204: never hand back a self-resolved path we have not confirmed is
+     * executable. After an installer's atomic rename-over, the resolved image
+     * path no longer exists (Linux readlink reads "<path> (deleted)"), and an
+     * unvalidated return turns into a doomed worker spawn (ENOENT). */
+    if (resolve_self_executable(out, outsz)) {
+        if (is_executable_file(out)) {
+            return true;
+        }
+#if defined(__linux__)
+        /* Deleted image: the /proc/self/exe magic link still executes the
+         * in-memory OLD build — the only spawn that satisfies the worker's
+         * build-fingerprint gate (a rename-over leaves the on-disk path
+         * holding a DIFFERENT build). Hand back the link, not the stale
+         * path. Deliberately NOT the saved launch path: preferring it swaps
+         * an ENOENT for a fingerprint refusal. */
+        return copy_path(out, outsz, "/proc/self/exe");
+#else
+        /* macOS has no magic link. Fail closed: the supervisor logs
+         * index.supervisor.no_self_path and indexing resumes after a daemon
+         * restart, instead of spawning a missing or mismatched binary. */
+        out[0] = '\0';
+        return false;
+#endif
+    }
+#else
     if (resolve_self_executable(out, outsz)) {
         return true;
     }
+#endif
     return copy_path(out, outsz, argv0);
 }
 
 void cbm_http_server_set_binary_path(const char *path) {
     if (path) {
-        if (!cbm_http_server_resolve_binary_path(path, g_binary_path, sizeof(g_binary_path))) {
+        /* Resolve into a local buffer first: `path` may alias g_binary_path
+         * on a repeated call. Resolving in place first hits resolve's leading
+         * out[0]='\0', which zeroes the shared buffer and silently discards
+         * the very path we were asked to re-resolve — and the in-place write
+         * is one reset-reorder away from an overlapping snprintf. */
+        char resolved[sizeof(g_binary_path)];
+        if (cbm_http_server_resolve_binary_path(path, resolved, sizeof(resolved))) {
+            snprintf(g_binary_path, sizeof(g_binary_path), "%s", resolved);
+        } else {
             g_binary_path[0] = '\0';
         }
     }

tests/test_httpd.c

@@ -11,6 +11,9 @@
  *      RPC dispatch, transport limits, receive deadline, clean shutdown.
  */
 #include "../src/foundation/compat.h"
+#if defined(__APPLE__)
+#include <mach-o/dyld.h> /* _NSGetExecutablePath — deleted-self driver */
+#endif
 #include "../src/foundation/compat_fs.h"
 #include "../src/foundation/compat_thread.h"
 #include "../src/foundation/log.h"
@@ -457,6 +460,86 @@ TEST(httpd_resolves_bare_binary_path_from_path) {
 #endif
 }
 
+TEST(httpd_deleted_self_spawn_path_follows_platform_ruling) {
+#if defined(__linux__) || defined(__APPLE__)
+    char tmpdir[256];
+    snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_httpd_deleted_self_XXXXXX");
+    ASSERT_NOT_NULL(cbm_mkdtemp(tmpdir));
+
+    char current[1024];
+#if defined(__linux__)
+    ssize_t current_len = readlink("/proc/self/exe", current, sizeof(current) - 1);
+    ASSERT_GT(current_len, 0);
+    ASSERT_LT(current_len, (ssize_t)(sizeof(current) - 1));
+    current[current_len] = '\0';
+#else
+    uint32_t current_sz = sizeof(current);
+    ASSERT_EQ(_NSGetExecutablePath(current, &current_sz), 0);
+#endif
+
+    char launch_path[512];
+    char replacement_path[512];
+    snprintf(launch_path, sizeof(launch_path), "%s/test-runner", tmpdir);
+    snprintf(replacement_path, sizeof(replacement_path), "%s/test-runner.new", tmpdir);
+    ASSERT_EQ(cbm_copy_file(current, launch_path), 0);
+    ASSERT_EQ(chmod(launch_path, 0755), 0);
+
+    int ready_pipe[2];
+    int continue_pipe[2];
+    ASSERT_EQ(pipe(ready_pipe), 0);
+    ASSERT_EQ(pipe(continue_pipe), 0);
+
+    pid_t child = fork();
+    if (child == 0) {
+        close(ready_pipe[0]);
+        close(continue_pipe[1]);
+        char ready_fd[32];
+        char continue_fd[32];
+        snprintf(ready_fd, sizeof(ready_fd), "%d", ready_pipe[1]);
+        snprintf(continue_fd, sizeof(continue_fd), "%d", continue_pipe[0]);
+        execl(launch_path, launch_path, "__cbm_deleted_self_probe", ready_fd, continue_fd,
+              launch_path, (char *)NULL);
+        _exit(127);
+    }
+    ASSERT_GT(child, 0);
+
+    close(ready_pipe[1]);
+    close(continue_pipe[0]);
+    char ready = '\0';
+    ASSERT_EQ(read(ready_pipe[0], &ready, 1), 1);
+    ASSERT_EQ(ready, 'R');
+
+#if defined(__linux__)
+    /* Atomic rename-over: the installer's real move. /proc/self/exe now reads
+     * "(deleted)" in the child, which is the state the magic-link contract
+     * covers. */
+    ASSERT_EQ(cbm_copy_file(current, replacement_path), 0);
+    ASSERT_EQ(chmod(replacement_path, 0755), 0);
+    ASSERT_EQ(rename(replacement_path, launch_path), 0);
+#else
+    /* macOS: a rename-over leaves a VALID executable at the launch path, which
+     * resolve is right to return (the worker's build-fingerprint gate is the
+     * layer that refuses a mismatched build — covered by its own tests). The
+     * fail-closed branch guards the image-GONE case, so remove the image. */
+    (void)replacement_path;
+    ASSERT_EQ(unlink(launch_path), 0);
+#endif
+    ASSERT_EQ(write(continue_pipe[1], "G", 1), 1);
+    close(ready_pipe[0]);
+    close(continue_pipe[1]);
+
+    int status = 0;
+    ASSERT_EQ(waitpid(child, &status, 0), child);
+    ASSERT_TRUE(WIFEXITED(status));
+    ASSERT_EQ(WEXITSTATUS(status), 0);
+    unlink(launch_path);
+    rmdir(tmpdir);
+    PASS();
+#else
+    PASS();
+#endif
+}
+
 /* ── Transport integration (listener only) ────────────────────── */
 
 TEST(httpd_listen_ephemeral_port) {
@@ -2278,6 +2361,7 @@ SUITE(httpd) {
     RUN_TEST(httpd_query_param_edge_cases);
     RUN_TEST(httpd_path_match_matrix);
     RUN_TEST(httpd_resolves_bare_binary_path_from_path);
+    RUN_TEST(httpd_deleted_self_spawn_path_follows_platform_ruling);
     RUN_TEST(repo_info_web_base_normalizes_to_https);
     RUN_TEST(repo_info_strips_credentials_from_remote);
 

tests/test_main.c

@@ -22,6 +22,7 @@ int tf_skip_count = 0;
 #include "daemon/version_cohort.h" /* Windows crash-turnover re-exec probe */
 #include "mcp/index_supervisor.h"  /* cbm_index_set_worker_role */
 #include "mcp/mcp.h"               /* cbm_mcp_handle_tool — act as a real worker */
+#include "ui/http_server.h"       /* deleted-self executable probe */
 #include <sqlite3.h>
 #include <errno.h>
 #include <stdbool.h>
@@ -617,6 +618,55 @@ static int tf_maybe_run_mcp_idxfailclosed_probe(int argc, char **argv) {
 #endif
 }
 
+static int tf_maybe_run_deleted_self_probe(int argc, char **argv) {
+#if defined(__linux__) || defined(__APPLE__)
+    if (argc != 5 || strcmp(argv[1], "__cbm_deleted_self_probe") != 0) {
+        return -1;
+    }
+    int ready_fd = atoi(argv[2]);
+    int continue_fd = atoi(argv[3]);
+    cbm_http_server_set_binary_path(argv[4]);
+    if (write(ready_fd, "R", 1) != 1) {
+        return 41;
+    }
+    char go = '\0';
+    if (read(continue_fd, &go, 1) != 1) {
+        return 42;
+    }
+    char resolved[1024];
+    bool ok = cbm_http_server_resolve_binary_path(NULL, resolved, sizeof(resolved));
+#if defined(__linux__)
+    /* Contract (#1204 strategy ruling): after a rename-over, the resolver
+     * hands back the /proc/self/exe magic link — the in-memory OLD build,
+     * the only spawn the worker's build-fingerprint gate accepts. First
+     * prove we really are in the deleted state, or the assertions below
+     * would pass vacuously on an intact image. */
+    char link_target[1024];
+    ssize_t n = readlink("/proc/self/exe", link_target, sizeof(link_target) - 1);
+    if (n <= 0) {
+        return 45;
+    }
+    link_target[n] = '\0';
+    if (strstr(link_target, " (deleted)") == NULL) {
+        return 46;
+    }
+    if (!ok) {
+        return 43;
+    }
+    return strcmp(resolved, "/proc/self/exe") == 0 && access(resolved, X_OK) == 0 ? 0 : 44;
+#else
+    /* macOS has no magic link: the ruling is fail-closed. Success here is
+     * the resolver REFUSING, so the supervisor logs no_self_path instead of
+     * spawning a missing or mismatched binary. */
+    return ok ? 44 : 0;
+#endif
+#else
+    (void)argc;
+    (void)argv;
+    return -1;
+#endif
+}
+
 static int g_suite_argc = 0;
 static char **g_suite_argv = NULL;
 static bool *g_suite_arg_matched = NULL;
@@ -869,6 +919,10 @@ int main(int argc, char **argv) {
     if (daemon_ipc_probe_rc >= 0) {
         return daemon_ipc_probe_rc;
     }
+    int deleted_self_rc = tf_maybe_run_deleted_self_probe(argc, argv);
+    if (deleted_self_rc >= 0) {
+        return deleted_self_rc;
+    }
 
     /* #798 follow-up: if spawned as the socket-isolation probe, report whether an
      * inheritable socket handle crossed into this child and exit before any suite. */