From 938491df1ed45e2754710911ab0e5e96752e9383 Mon Sep 17 00:00:00 2001 From: veg Date: Sat, 4 Jul 2026 14:03:53 +0000 Subject: [PATCH] fix: classify unauthorized by stderr presence, not denial wording Review flagged the 'access not allowed' substring: if tmux's denial text drifts while keeping exit 0, unauthorized connections would classify as ok and the false-joined status/leave misreporting would silently return. A clean authorized list-clients writes nothing to stderr (verified 3.3a/3.5a/3.6-SunOS, incl. invited read-only clients), so rc 0 plus any stderr output now classifies as unauthorized: no message text consulted. Drift in any direction degrades to a hidden party, never to a false ok. --- party | 33 ++++++++++++++++++++------------- tests/20-helpers.bats | 10 +++++++--- tests/96-unauthorized.bats | 18 ++++++++++++++++++ 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/party b/party index 1c26b0e..4389855 100755 --- a/party +++ b/party @@ -343,26 +343,33 @@ is_party_alive() { # 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 +# unauthorized — live tmux server, but it refused 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, +# 3.5a, and 3.6/SunOS), for every command — including # has-session, which then reports any target as # existing. Exit codes alone cannot distinguish -# authorized from unauthorized; the message can. +# authorized from unauthorized. We deliberately do NOT +# match the denial text either: wording could drift +# across tmux versions and silently reopen the +# false-authorized paths. A clean authorized +# list-clients writes nothing to stderr, so rc 0 plus +# ANY stderr output classifies as unauthorized. Should +# tmux ever warn on an authorized call, that degrades +# to a hidden party — never to a false "ok". # 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 + if [ "$_cs_rc" -ne 0 ]; then + printf 'dead\n' + elif [ -n "$_cs_err" ]; then + printf 'unauthorized\n' + else + printf 'ok\n' + fi } # FS perms diff --git a/tests/20-helpers.bats b/tests/20-helpers.bats index 85b27f9..c89b5b6 100644 --- a/tests/20-helpers.bats +++ b/tests/20-helpers.bats @@ -142,9 +142,10 @@ setup() { 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 ;; + ok) exit 0 ;; + deny) echo "access not allowed" >&2; exit 0 ;; + deny2) echo "connection declined by policy (hypothetical future wording)" >&2; exit 0 ;; + *) echo "error connecting to /x (No such file or directory)" >&2; exit 1 ;; esac S chmod +x "$stub" @@ -153,5 +154,8 @@ S # 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" ] + # Classification must not depend on tmux's denial wording (adversarial + # review finding): rc 0 with ANY stderr output is a refusal. + export MODE=deny2; [ "$(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 index 42970d6..f7c3f68 100644 --- a/tests/96-unauthorized.bats +++ b/tests/96-unauthorized.bats @@ -129,3 +129,21 @@ STUB [[ "$output" == *"read-only"* ]] [[ "$output" == *"attach-session -r -t fiesta"* ]] } + +@test "unauthorized classification does not depend on tmux's denial wording" { + # Adversarial-review finding: matching the English "access not + # allowed" would silently reopen the false-authorized paths if the + # wording ever drifts. The classifier keys on rc 0 + any stderr + # output instead; this stub proves a drifted message still counts. + cat > "$PARTY_TMP/tmux-denied-drift" <<'STUB' +#!/bin/sh +echo "connection declined by policy" >&2 +exit 0 +STUB + chmod +x "$PARTY_TMP/tmux-denied-drift" + export PARTY_TMUX="$PARTY_TMP/tmux-denied-drift" + run "$PARTY_BIN" list + [ "$status" -eq 0 ] + [[ "$output" == *"invite-only"* ]] + [[ "$output" != *"0 attendee"* ]] +}