From f3209e39d1a1ac2283f7e3f7f14b5ecce932fa8b Mon Sep 17 00:00:00 2001 From: veg Date: Sat, 4 Jul 2026 09:55:47 +0000 Subject: [PATCH] fix: classify unauthorized tmux connections; stop misreporting membership tmux answers non-allowlisted users with 'access not allowed' on stderr and exit 0 for every command (verified 3.3a/3.5a, two users), so has-session reports any target as existing. party status claimed uninvited group members were joined, party leave silently 'succeeded', and party list printed bogus '0 attendee(s)' rows. New party_conn_state (ok/unauthorized/dead) classifies by message content; list now shows 'invite-only (ask )'. --- party | 65 ++++++++++++++++++++++------ tests/20-helpers.bats | 19 +++++++++ tests/96-unauthorized.bats | 87 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 tests/96-unauthorized.bats diff --git a/party b/party index a21c1fd..062c9f1 100755 --- a/party +++ b/party @@ -334,6 +334,31 @@ is_party_alive() { "$PARTY_TMUX" -S "$_pa_sock" list-clients >/dev/null 2>&1 } +# Classify the caller's relationship to a party socket. Prints exactly +# one of: +# ok — live tmux server, the caller is authorized. +# unauthorized — live tmux server, but server-access rejected us. +# CAUTION, the reason this function exists: tmux +# answers a non-allowlisted user with "access not +# allowed" on stderr and EXIT STATUS 0 (verified live +# on 3.3a and 3.5a), for every command — including +# has-session, which then reports any target as +# existing. Exit codes alone cannot distinguish +# authorized from unauthorized; the message can. +# dead — nothing speaking the tmux protocol at that socket +# (stale roster, killed server, AF_UNIX impostor). +# Unknown error text with nonzero rc falls through to "dead", so string +# drift in a future tmux degrades to hidden-party behavior, never to a +# false "ok". +party_conn_state() { + _cs_err=$("$PARTY_TMUX" -S "$1" list-clients 2>&1 >/dev/null) \ + && _cs_rc=0 || _cs_rc=$? + case "$_cs_err" in + *"access not allowed"*) printf 'unauthorized\n'; return 0 ;; + esac + if [ "$_cs_rc" -eq 0 ]; then printf 'ok\n'; else printf 'dead\n'; fi +} + # FS perms # -------- # @@ -516,7 +541,7 @@ find_party_by_name() { set -- for rec in $(roster_list); do roster_read "$rec" || continue - is_party_alive "$RR_SERVER_PID" "$RR_SOCKET" || continue + [ "$(party_conn_state "$RR_SOCKET")" = dead ] && continue [ "$RR_PARTY_NAME" = "$target" ] && set -- "$@" "$rec" done case $# in @@ -532,7 +557,7 @@ pick_live_party() { set -- for rec in $(roster_list); do roster_read "$rec" || continue - is_party_alive "$RR_SERVER_PID" "$RR_SOCKET" || continue + [ "$(party_conn_state "$RR_SOCKET")" = dead ] && continue set -- "$@" "$rec" done case $# in @@ -981,7 +1006,11 @@ cmd_leave() { found=0 for rec in $(roster_list); do roster_read "$rec" || continue - is_party_alive "$RR_SERVER_PID" "$RR_SOCKET" || continue + # Presence checks below (has-session, list-clients) are only + # meaningful on a server that accepted our connection: for an + # unauthorized caller has-session answers rc 0 for ANY target, + # which made leave "succeed" against parties we never joined. + [ "$(party_conn_state "$RR_SOCKET")" = ok ] || continue attached=0 "$PARTY_TMUX" -S "$RR_SOCKET" has-session -t "__party_guest_$USER" 2>/dev/null \ && attached=1 @@ -1148,7 +1177,8 @@ cmd_list() { found=0 for rec in $(roster_list); do roster_read "$rec" || continue - is_party_alive "$RR_SERVER_PID" "$RR_SOCKET" || continue + state=$(party_conn_state "$RR_SOCKET") + [ "$state" = dead ] && continue # Output hygiene, NOT a confidentiality control. If inherited ACLs # widened the per-party dir past its 0750 / group=RR_GROUP mode bits, # roster_read above has ALREADY parsed host/name/group into vars, @@ -1159,21 +1189,24 @@ cmd_list() { # the FS perimeter alone. list_group="${RR_GROUP:-$TMUX_PARTY_GROUP}" id -nG "$USER" 2>/dev/null | tr ' ' '\n' | grep -qx "$list_group" || continue - # Stale records (dead PID, dead socket, planted daemon) are - # filtered out by is_party_alive. They're not auto-cleaned here: - # cmd_host's mkdir-as-lock refuses to take over leftover dirs - # (auto-cleanup races with concurrent in-flight hosts). Recovery - # for a genuine crash leftover is a one-shot - # manual `rm -rf` per the host-side error message. - attendees=$("$PARTY_TMUX" -S "$RR_SOCKET" list-clients 2>/dev/null | wc -l | tr -d ' ') # Show the group only when it differs from the env/default, to # keep the common case uncluttered. group_tag= if [ -n "$RR_GROUP" ] && [ "$RR_GROUP" != "$TMUX_PARTY_GROUP" ]; then group_tag=" [group=$RR_GROUP]" fi - printf '%-12s %-30s %s attendee(s)%s\n' \ - "$RR_HOST_USER" "$RR_PARTY_NAME" "$attendees" "$group_tag" + if [ "$state" = ok ]; then + attendees=$("$PARTY_TMUX" -S "$RR_SOCKET" list-clients 2>/dev/null | wc -l | tr -d ' ') + printf '%-12s %-30s %s attendee(s)%s\n' \ + "$RR_HOST_USER" "$RR_PARTY_NAME" "$attendees" "$group_tag" + else + # Live server, connection refused: we can't count attendees + # (list-clients is behind the auth gate — its empty stdout + # used to render here as a bogus "0 attendee(s)"), but the + # party is real and the caller can ask for an invite. + printf '%-12s %-30s invite-only (ask %s)%s\n' \ + "$RR_HOST_USER" "$RR_PARTY_NAME" "$RR_HOST_USER" "$group_tag" + fi found=$((found+1)) done @@ -1257,10 +1290,14 @@ cmd_status() { hosting='' joined='' for rec in $(roster_list); do roster_read "$rec" || continue - is_party_alive "$RR_SERVER_PID" "$RR_SOCKET" || continue + state=$(party_conn_state "$RR_SOCKET") + [ "$state" = dead ] && continue if [ "$RR_HOST_USER" = "$USER" ]; then hosting="$hosting $RR_PARTY_NAME" fi + # The joined checks below trust has-session, which lies (rc 0 + # for any target) when the server refused our connection. + [ "$state" = ok ] || continue # Joined covers two cases. Passive: we have a client attached to # the host's session, list-clients sees it. Active: we have our # own __party_guest_$USER session, which survives a client detach diff --git a/tests/20-helpers.bats b/tests/20-helpers.bats index 8f37501..85b27f9 100644 --- a/tests/20-helpers.bats +++ b/tests/20-helpers.bats @@ -136,3 +136,22 @@ setup() { [ "$status" -ne 0 ] [[ "$output" == *"pass a name"* ]] } + +@test "party_conn_state classifies ok / unauthorized / dead" { + stub="$PARTY_TMP/conn-stub" + cat > "$stub" <<'S' +#!/bin/sh +case "${MODE:-}" in + ok) exit 0 ;; + deny) echo "access not allowed" >&2; exit 0 ;; + *) echo "error connecting to /x (No such file or directory)" >&2; exit 1 ;; +esac +S + chmod +x "$stub" + export PARTY_TMUX="$stub" + export MODE=ok; [ "$(party_conn_state /x)" = "ok" ] + # The trap this function exists for: tmux answers unauthorized users + # with EXIT 0 + a stderr message, so rc alone says "authorized". + export MODE=deny; [ "$(party_conn_state /x)" = "unauthorized" ] + export MODE=dead; [ "$(party_conn_state /x)" = "dead" ] +} diff --git a/tests/96-unauthorized.bats b/tests/96-unauthorized.bats new file mode 100644 index 0000000..65ba671 --- /dev/null +++ b/tests/96-unauthorized.bats @@ -0,0 +1,87 @@ +#!/usr/bin/env bats +# +# Uninvited-guest semantics. tmux >= 3.3 answers a non-allowlisted +# user's connection with "access not allowed" on stderr and EXIT 0 +# (verified live on 3.3a and 3.5a with two real users), for every +# command — including has-session, which then reports any target as +# existing. Exit-code-based checks therefore misclassified +# "unauthorized" as "authorized": party status reported uninvited +# members as joined, party leave silently "succeeded", and party list +# printed bogus "0 attendee(s)" rows. These tests pin the corrected +# classification. + +load 'helpers' + +setup() { + setup_party_sandbox + export TMUX_PARTY_GROUP="$(id -gn)" + + # Stub tmux that answers every command the way a real server answers + # a non-allowlisted user: message on stderr, exit 0. + cat > "$PARTY_TMP/tmux-denied" <<'STUB' +#!/bin/sh +echo "access not allowed" >&2 +exit 0 +STUB + chmod +x "$PARTY_TMP/tmux-denied" + export PARTY_TMUX="$PARTY_TMP/tmux-denied" + + # Plant a live-looking party dir + roster. Ownership is ours (bats + # can't fake a foreign uid without root), so cmd_status will also see + # this fixture as "hosting" — that's orthogonal to the joined/leave + # misreporting under test. + ensure_party_dir "$USER" fiesta + d="$PARTY_SOCKET_DIR/party-$USER:fiesta.d" + cat > "$d/roster" < "$PARTY_TMP/tmux-dead" <<'STUB' +#!/bin/sh +echo "no server running" >&2 +exit 1 +STUB + chmod +x "$PARTY_TMP/tmux-dead" + export PARTY_TMUX="$PARTY_TMP/tmux-dead" + run "$PARTY_BIN" list + [ "$status" -eq 0 ] + [[ "$output" == *"no parties found"* ]] +}