From e674f159219b54381ab7568db35ed32ddc1251f4 Mon Sep 17 00:00:00 2001 From: Donaldo Date: Tue, 10 Mar 2026 15:17:05 +0100 Subject: [PATCH] fix(attach): replace ATCH_SESSION-only self-attach guard with PID ancestry check The anti-recursion guard in attach_main relied solely on ATCH_SESSION to detect self-attach attempts. Because ATCH_SESSION is an inherited env var it could be stale (e.g. the variable set inside a session that has since been detached-from, or a shell-config export), causing every subsequent attach to be incorrectly blocked with "cannot attach from within itself". Fix: the master now writes the pty-child PID to .ppid on session start and removes it on cleanup. attach_main reads this file and walks the process-ancestry tree (via libproc on macOS, /proc on Linux) to verify that the calling process is genuinely a descendant of the session shell before blocking the attach. If the .ppid file is absent or the recorded PID is not an ancestor, ATCH_SESSION is treated as stale and the attach proceeds. The ancestry check is also hoisted before require_tty() in cmd_attach and the legacy -a/-A/-c routes so the correct diagnostic fires even without a controlling terminal. Co-Authored-By: Claude Sonnet 4.6 --- atch.c | 8 +++ atch.h | 1 + attach.c | 184 ++++++++++++++++++++++++++++++++++++++++++-------- master.c | 27 ++++++++ tests/test.sh | 65 ++++++++++++++++++ 5 files changed, 257 insertions(+), 28 deletions(-) diff --git a/atch.c b/atch.c index 9b8942a..0cc979d 100644 --- a/atch.c +++ b/atch.c @@ -400,6 +400,9 @@ static int cmd_attach(int argc, char **argv) printf("Try '%s --help' for more information.\n", progname); return 1; } + /* Check ancestry before TTY so the correct error is shown first. */ + if (check_attach_ancestry()) + return 1; save_term(); if (require_tty()) return 1; @@ -714,6 +717,11 @@ int main(int argc, char **argv) return 1; if (mode != 'a') argv = use_shell_if_no_cmd(argc, argv); + /* Check ancestry before TTY so the correct error fires first. */ + if (mode == 'a' || mode == 'A' || mode == 'c') { + if (check_attach_ancestry()) + return 1; + } save_term(); if (dont_have_tty && mode != 'n' && mode != 'N') { printf("%s: attaching to a session requires a " diff --git a/atch.h b/atch.h index d1a299a..693cbe3 100644 --- a/atch.h +++ b/atch.h @@ -132,6 +132,7 @@ void get_session_dir(char *buf, size_t size); int socket_with_chdir(char *path, int (*fn)(char *)); int replay_session_log(int saved_errno); +int check_attach_ancestry(void); int attach_main(int noerror); int master_main(char **argv, int waitattach, int dontfork); int push_main(void); diff --git a/attach.c b/attach.c index c3722b0..81c3515 100644 --- a/attach.c +++ b/attach.c @@ -8,6 +8,104 @@ #endif #endif +/* ── ancestry helpers ────────────────────────────────────────────────────── */ + +/* +** Return the parent PID of 'pid'. +** Returns 0 on failure (pid not found or permission denied). +** Portable across Linux (/proc) and macOS (libproc / sysctl). +*/ +#ifdef __APPLE__ +#include +static pid_t get_parent_pid(pid_t pid) +{ + struct proc_bsdinfo info; + + if (proc_pidinfo(pid, PROC_PIDTBSDINFO, 0, + &info, sizeof(info)) <= 0) + return 0; + return (pid_t)info.pbi_ppid; +} +#else +static pid_t get_parent_pid(pid_t pid) +{ + char path[64]; + FILE *f; + pid_t ppid = 0; + char line[256]; + + snprintf(path, sizeof(path), "/proc/%d/status", (int)pid); + f = fopen(path, "r"); + if (!f) + return 0; + while (fgets(line, sizeof(line), f)) { + if (sscanf(line, "PPid: %d", &ppid) == 1) + break; + } + fclose(f); + return ppid; +} +#endif + +/* +** Return 1 if 'ancestor_pid' is equal to, or an ancestor of, 'child_pid'. +** Walks the process tree upward; gives up after 1024 steps to avoid loops. +*/ +static int is_ancestor(pid_t ancestor_pid, pid_t child_pid) +{ + pid_t p = child_pid; + int steps = 0; + + while (p > 1 && steps < 1024) { + if (p == ancestor_pid) + return 1; + p = get_parent_pid(p); + steps++; + } + /* Also check the final value (handles the p == ancestor_pid == 1 edge) */ + return (p == ancestor_pid); +} + +/* +** Read the session shell PID from '.ppid'. +** Returns 0 if the file does not exist or cannot be read. +*/ +static pid_t read_session_ppid(const char *sockpath) +{ + char ppid_path[600]; + FILE *f; + long pid = 0; + + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockpath); + f = fopen(ppid_path, "r"); + if (!f) + return 0; + if (fscanf(f, "%ld", &pid) != 1) + pid = 0; + fclose(f); + return (pid > 0) ? (pid_t)pid : 0; +} + +/* +** Return 1 if the current process is genuinely running inside the session +** whose socket path is 'sockpath'. +** +** The check reads '.ppid' (written by the master when it forks +** the pty child) and tests whether that PID is an ancestor of the calling +** process. If the file is absent or the PID is no longer an ancestor, +** the ATCH_SESSION variable is considered stale and the guard is skipped. +*/ +static int session_is_ancestor(const char *sockpath) +{ + pid_t shell_pid = read_session_ppid(sockpath); + + if (shell_pid <= 0) + return 0; /* no .ppid file → assume stale */ + return is_ancestor(shell_pid, getpid()); +} + +/* ─────────────────────────────────────────────────────────────────────────── */ + /* ** The current terminal settings. After coming back from a suspend, we ** restore this. @@ -256,6 +354,57 @@ int replay_session_log(int saved_errno) return 1; } +/* +** Check whether attaching to 'sockname' would be a self-attach (i.e. the +** current process is running inside that session's ancestry chain). +** +** Returns 1 and prints an error if a genuine self-attach is detected. +** Returns 0 if the attach may proceed. +** +** Called before require_tty() so that the correct diagnostic is shown even +** when there is no terminal available. +*/ +int check_attach_ancestry(void) +{ + const char *tosearch = getenv(SESSION_ENVVAR); + + if (!tosearch || !*tosearch) + return 0; + + { + size_t slen = strlen(sockname); + const char *p = tosearch; + + while (*p) { + const char *colon = strchr(p, ':'); + size_t tlen = + colon ? (size_t)(colon - p) : strlen(p); + + if (tlen == slen + && strncmp(p, sockname, tlen) == 0) { + /* Verify we are genuinely inside this + * session before blocking the attach. + * session_is_ancestor() reads the .ppid + * file written by the master and checks + * the process ancestry; if the file is + * absent or the PID is not an ancestor, + * ATCH_SESSION is stale → allow attach. */ + if (session_is_ancestor(sockname)) { + printf + ("%s: cannot attach to session '%s' from within itself\n", + progname, session_shortname()); + return 1; + } + /* Stale ATCH_SESSION — fall through. */ + } + if (!colon) + break; + p = colon + 1; + } + } + return 0; +} + int attach_main(int noerror) { struct packet pkt; @@ -266,34 +415,13 @@ int attach_main(int noerror) /* Refuse to attach to any session in our ancestry chain (catches both * direct self-attach and indirect loops like A -> B -> A). * SESSION_ENVVAR is the colon-separated chain, so scanning it covers - * all ancestors. */ - { - const char *tosearch = getenv(SESSION_ENVVAR); - - if (tosearch && *tosearch) { - size_t slen = strlen(sockname); - const char *p = tosearch; - - while (*p) { - const char *colon = strchr(p, ':'); - size_t tlen = - colon ? (size_t)(colon - p) : strlen(p); - - if (tlen == slen - && strncmp(p, sockname, tlen) == 0) { - if (!noerror) - printf - ("%s: cannot attach to session '%s' from within itself\n", - progname, - session_shortname()); - return 1; - } - if (!colon) - break; - p = colon + 1; - } - } - } + * all ancestors. + * + * The check is performed via check_attach_ancestry(), which is also + * called early in the command handlers (before require_tty) so the + * correct error is shown even without a terminal. */ + if (check_attach_ancestry()) + return 1; /* Attempt to open the socket. Don't display an error if noerror is ** set. */ diff --git a/master.c b/master.c index 05d3a04..608c057 100644 --- a/master.c +++ b/master.c @@ -92,9 +92,28 @@ static int open_log(const char *path) return fd; } +/* Write the pty-child PID to .ppid for ancestry verification. */ +static void write_session_ppid(pid_t pid) +{ + char ppid_path[600]; + int fd; + char buf[32]; + int len; + + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockname); + fd = open(ppid_path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) + return; + len = snprintf(buf, sizeof(buf), "%d\n", (int)pid); + write(fd, buf, (size_t)len); + close(fd); +} + /* Write end marker to log, close it, and unlink the socket. */ static void cleanup_session(void) { + char ppid_path[600]; + if (log_fd >= 0) { time_t age = time(NULL) - master_start_time; char agebuf[32]; @@ -109,6 +128,8 @@ static void cleanup_session(void) log_fd = -1; } unlink(sockname); + snprintf(ppid_path, sizeof(ppid_path), "%s.ppid", sockname); + unlink(ppid_path); } /* Signal */ @@ -606,6 +627,12 @@ static void master_process(int s, char **argv, int waitattach, int statusfd) exit(1); } + /* Record the pty-child PID for ancestry verification in attach_main. + * attach_main reads .ppid to confirm that a process trying + * to attach is genuinely running inside this session before blocking + * a re-attach based on a potentially stale ATCH_SESSION value. */ + write_session_ppid(the_pty.pid); + /* Set up some signals. */ signal(SIGPIPE, SIG_IGN); signal(SIGXFSZ, SIG_IGN); diff --git a/tests/test.sh b/tests/test.sh index fbed18d..0073fd5 100644 --- a/tests/test.sh +++ b/tests/test.sh @@ -628,6 +628,71 @@ run "$ATCH" start -C foo C-bad sleep 999 assert_exit "-C invalid: exit 1" 1 "$rc" assert_contains "-C invalid: message" "Invalid log size" "$out" +# ── 21. ATCH_SESSION ancestry protection ───────────────────────────────────── +# +# Regression test for the ATCH_SESSION stale-ancestry bug. +# +# The anti-recursion guard in attach_main must only fire when the current +# process is genuinely a descendant of the target session. It must NOT fire +# when ATCH_SESSION merely contains the session path but the process is not +# actually running inside that session (stale env var). +# +# Because attach_main is only reached after require_tty() in the normal +# command path, we probe the guard by simulating the session's .ppid file: +# +# • No .ppid file (or PID 0) → guard is bypassed → "does not exist" / "requires a terminal" +# • .ppid file with a PID that IS an ancestor of the current shell → guard fires +# • .ppid file with a PID that is NOT an ancestor (e.g. already-dead PID) → guard bypassed +# +# A session's .ppid file is written by the master and contains the PID of the +# shell process running inside the pty (the_pty.pid). + +mkdir -p "$HOME/.cache/atch" + +# Case A: ATCH_SESSION holds a session path, NO .ppid file exists → no block +GHOST_SOCK="$HOME/.cache/atch/ghost-session" +# No socket, no .ppid — completely absent session +run env ATCH_SESSION="$GHOST_SOCK" "$ATCH" attach ghost-session 2>&1 +assert_exit "ppid-guard: no ppid file → exit 1 (not self-attach)" 1 "$rc" +assert_not_contains "ppid-guard: no ppid file → no self-attach msg" \ + "from within itself" "$out" + +# Case B: .ppid file contains a dead / non-ancestor PID → guard must NOT fire +"$ATCH" start ppid-live sleep 9999 +wait_socket ppid-live +PPID_SOCK="$HOME/.cache/atch/ppid-live" +# Write a PID that is definitely not an ancestor (PID 1 is init/launchd, +# which is NOT a direct ancestor of our test shell in a normal session). +# Using a large unlikely-to-exist PID is fragile; using PID 1 is safe because +# PID 1 is the root, not our direct ancestor in the process hierarchy +# (our shell's ppid is the test runner, not init). +# Actually we need a PID that is NOT in our ancestry. PID of a sleep process works. +DEAD_PID_PROC=$(sh -c 'sleep 60 & echo $!') +sleep 0.05 +kill "$DEAD_PID_PROC" 2>/dev/null +wait "$DEAD_PID_PROC" 2>/dev/null +# DEAD_PID_PROC is now dead — write it as ppid +printf "%d\n" "$DEAD_PID_PROC" > "${PPID_SOCK}.ppid" +run env ATCH_SESSION="$PPID_SOCK" "$ATCH" attach ppid-live 2>&1 +assert_exit "ppid-guard: dead ppid → exit 1 (not self-attach)" 1 "$rc" +assert_not_contains "ppid-guard: dead ppid → no self-attach msg" \ + "from within itself" "$out" +tidy ppid-live + +# Case C: .ppid file contains the PID of our current shell → guard MUST fire +"$ATCH" start self-session sleep 9999 +wait_socket self-session +SELF_SOCK="$HOME/.cache/atch/self-session" +# Write the PID of the current shell ($$) as if this process IS the shell +# running inside the session. From atch's perspective, our process IS a +# descendant of "$$" (itself) — so the guard should trigger. +printf "%d\n" "$$" > "${SELF_SOCK}.ppid" +run env ATCH_SESSION="$SELF_SOCK" "$ATCH" attach self-session 2>&1 +assert_exit "ppid-guard: self as ppid → blocked exit 1" 1 "$rc" +assert_contains "ppid-guard: self as ppid → self-attach msg" \ + "from within itself" "$out" +tidy self-session + # ── 21. no-args → usage ────────────────────────────────────────────────────── # Invoking with zero arguments calls usage() (exits 0, prints help).