Skip to content

Commit c5cc184

Browse files
committed
Tell participants about a busy dashboard port instead of letting ssh fail
sfbox dashboard already took --port, but nothing checked whether the local port was free before exec ssh -L, so a participant whose machine already had that port hit ssh's bare "bind: Address already in use" with no mention that --port exists. A busy default now moves to the next free port and says which. A port the participant named is never moved: that case stops and names a free one to try. The probe is a bash /dev/tcp connect, so it needs no lsof, ss or netstat. City: city · Agent: local-core.builder-2
1 parent dd1fcda commit c5cc184

4 files changed

Lines changed: 183 additions & 3 deletions

File tree

‎participant-box-cli/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ sfbox dashboard
9494

9595
The dashboard's built into the `gc` binary and served by the supervisor, so there's nothing to deploy or start. `sfbox` forwards it over SSH and prints you a local URL. Leave the command running; Ctrl-C closes the tunnel.
9696

97+
If the default port is already busy on your laptop, `sfbox` moves the tunnel to the next free one and tells you which it picked, so the URL it prints is always the one to open. Pass `--port <local-port>` to choose for yourself. A port you named is never moved: if it's taken the command stops and names a free one to try, because choosing a port usually means something else of yours expects the dashboard there.
98+
9799
Tunnelling is the whole point. Your security group opens `:22` and nothing else, and because you're reaching the dashboard same-origin through the tunnel, it stays fully read-write. Bind it to a public interface instead and you'd leave reads open to anyone who found the address, plus the API would drop to read-only unless you'd explicitly switched mutations on.
98100

99101
## Where your state lives

‎participant-box-cli/SKILL.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ sfbox dashboard
8989

9090
This just opens an SSH tunnel and prints a `127.0.0.1` URL. The dashboard's embedded in the `gc` binary and served by the supervisor, so nothing needs starting on the box. It'll run in the foreground until Ctrl-C.
9191

92+
A busy port on the user's own laptop is the common snag here, and the command handles the two cases differently. If the default port is taken it moves to the next free one and says which, so read the port back off its output rather than assuming. If they passed `--port` and that port is taken, the command stops and names a free one to try. That's deliberate: ask them which port they want instead of choosing one for them.
93+
9294
Never suggest binding the API port publicly, or opening it up in the security group. Only `:22` belongs there, really. Binding it non-loopback also drops the API to read-only unless mutations are explicitly enabled, and it'll leave reads open to anyone who finds the address.
9395

9496
## Restarting

‎participant-box-cli/sfbox‎

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ SFBOX_PROTECTED_IMPORTS="core bd"
3232
SFBOX_DEFAULT_USER="ubuntu"
3333
SFBOX_DEFAULT_PORT="22"
3434
SFBOX_DEFAULT_DASHBOARD_PORT="8372"
35+
36+
# How far above a busy default the dashboard looks for a free local port before
37+
# it gives up and says so. Small on purpose: a participant reading a port number
38+
# off their own screen should not have to check it is the one in the room's
39+
# instructions plus fifty.
40+
SFBOX_DASHBOARD_PORT_SCAN=20
41+
3542
SFBOX_SERVICE="gas-city.service"
3643

3744
# ---------------------------------------------------------------- output ----
@@ -756,12 +763,77 @@ restart_factory() { # box wait_secs
756763
return 1
757764
}
758765

766+
# ------------------------------------------------------------- dashboard ----
767+
768+
# Is something already listening on this local port?
769+
#
770+
# /dev/tcp is a bash builtin rather than a device, which is what makes this the
771+
# portable check. lsof, ss and netstat are each absent from some stock macOS or
772+
# slim Linux, and this runs on whatever laptop a participant walked in with.
773+
#
774+
# A completed connect is the only answer trusted here. Refused, unreachable, and
775+
# a bash built without /dev/tcp all report free, so the worst this can do is hand
776+
# the participant the ssh bind error they would have got anyway. Guessing the
777+
# other way is the costlier mistake: it would refuse to tunnel over a port
778+
# nothing was holding.
779+
port_in_use() { # port -> 0 in use, 1 free or undetectable
780+
( exec 3<>"/dev/tcp/127.0.0.1/$1" ) >/dev/null 2>&1
781+
}
782+
783+
# The first free port at or above the one given, within SFBOX_DASHBOARD_PORT_SCAN
784+
# tries. Prints nothing when they are all taken, which the caller reports rather
785+
# than searching further: someone holding that many consecutive ports is better
786+
# told than moved somewhere they would never think to look.
787+
dashboard_free_port() { # start -> prints a free port, or nothing
788+
local p="$1" tried=0
789+
while [ "$tried" -lt "$SFBOX_DASHBOARD_PORT_SCAN" ] && [ "$p" -le 65535 ]; do
790+
port_in_use "$p" || { printf '%s' "$p"; return 0; }
791+
p=$((p + 1))
792+
tried=$((tried + 1))
793+
done
794+
return 1
795+
}
796+
797+
# Settle which local port the tunnel binds.
798+
#
799+
# An explicit --port is never moved. The participant named that port, usually
800+
# because something else of theirs expects the dashboard there, so a collision is
801+
# theirs to see; moving them quietly would hide both the collision and the port
802+
# they ended up on. The default is moved, because someone who never chose a port
803+
# has no stake in which one they get, and the alternative is ssh's own "bind:
804+
# Address already in use", which names neither the port nor the flag that fixes it.
805+
dashboard_resolve_port() { # port explicit -> prints the port to bind
806+
local port="$1" explicit="$2" free=""
807+
808+
port_in_use "$port" || { printf '%s' "$port"; return 0; }
809+
free="$(dashboard_free_port "$((port + 1))")"
810+
811+
if [ -n "$explicit" ]; then
812+
err "dashboard: local port $port is already in use on this machine."
813+
if [ -n "$free" ]; then
814+
err "Stop whatever is listening there, or use a free one: sfbox dashboard --port $free"
815+
else
816+
err "Stop whatever is listening there, or pass --port with a free port."
817+
fi
818+
return 1
819+
fi
820+
821+
if [ -z "$free" ]; then
822+
err "dashboard: local port $port is in use, and so are the next $SFBOX_DASHBOARD_PORT_SCAN."
823+
err "Free one of them, or pass --port with a port you know is free."
824+
return 1
825+
fi
826+
827+
warn "local port $port is in use, so this tunnel uses $free instead."
828+
printf '%s' "$free"
829+
}
830+
759831
cmd_dashboard() {
760-
local box_arg="" port="$SFBOX_DEFAULT_DASHBOARD_PORT"
832+
local box_arg="" port="$SFBOX_DEFAULT_DASHBOARD_PORT" port_explicit=""
761833
while [ $# -gt 0 ]; do
762834
case "$1" in
763835
--box) box_arg="${2:-}"; shift 2 || die "'$1' needs a value" ;;
764-
--port) port="${2:-}"; shift 2 || die "'$1' needs a value" ;;
836+
--port) port="${2:-}"; port_explicit=yes; shift 2 || die "'$1' needs a value" ;;
765837
-h|--help) echo "usage: sfbox dashboard [--box <boxId>] [--port <local-port>]"; return 0 ;;
766838
*) die "dashboard: unknown argument '$1'" ;;
767839
esac
@@ -771,6 +843,7 @@ cmd_dashboard() {
771843
esac
772844
state_init
773845
local box; box="$(resolve_box "$box_arg")" || exit $?
846+
port="$(dashboard_resolve_port "$port" "$port_explicit")" || exit 1
774847

775848
say "Tunnelling the dashboard from '$box' to http://127.0.0.1:$port"
776849
say "The SPA is embedded in the gc binary and served by the supervisor, so there"

‎participant-box-cli/tests/run-tests.sh‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ trap 'rm -rf "$SFBOX_TEST_ROOT"' EXIT
2323

2424
PASS=0
2525
FAIL=0
26+
SKIP=0
2627

2728
ok() { PASS=$((PASS + 1)); printf ' ok %s\n' "$1"; }
2829
bad() { FAIL=$((FAIL + 1)); printf ' FAIL %s\n' "$1"; printf ' %s\n' "$2"; }
30+
# A test that could not run is neither a pass nor a failure, and counting it as
31+
# either hides it. The summary reports these separately for the same reason.
32+
skip() { SKIP=$((SKIP + 1)); printf ' SKIP %s\n' "$1"; }
2933

3034
is() { # actual expected label
3135
if [ "$1" = "$2" ]; then ok "$3"; else bad "$3" "want [$2], got [$1]"; fi
@@ -144,6 +148,103 @@ rc_is 1 "get-box rejects a non-numeric --lines" cmd_get_box --lines abc
144148
rc_is 1 "restart-factory rejects a non-numeric --wait" cmd_restart_factory --wait soon
145149
rc_is 1 "dashboard rejects a non-numeric --port" cmd_dashboard --port http
146150

151+
# -------------------------------------------------- dashboard local port ---
152+
153+
# Two halves, tested apart. The policy is what the command decides once it knows
154+
# a port is busy, and a stubbed probe drives it here so the answers do not depend
155+
# on what happens to be listening on the machine running the suite. The probe is
156+
# the other half, and it goes against a real listener below.
157+
158+
real_port_in_use="$(declare -f port_in_use)"
159+
busy_ports=""
160+
port_in_use() { case " $busy_ports " in *" $1 "*) return 0 ;; *) return 1 ;; esac; }
161+
162+
echo "dashboard: a free port is used as asked"
163+
busy_ports=""
164+
is "$(dashboard_resolve_port 8372 '')" "8372" "the default is left alone"
165+
is "$(dashboard_resolve_port 9999 yes)" "9999" "an explicit port is left alone"
166+
167+
echo "dashboard: a busy default moves, and says where"
168+
busy_ports="8372"
169+
is "$(dashboard_resolve_port 8372 '' 2>/dev/null)" "8373" "moves to the next free port"
170+
out="$(dashboard_resolve_port 8372 '' 2>&1 >/dev/null)"
171+
contains "$out" "8372" "names the port that was busy"
172+
contains "$out" "8373" "names the port it moved to"
173+
174+
busy_ports="8372 8373 8374"
175+
is "$(dashboard_resolve_port 8372 '' 2>/dev/null)" "8375" "walks past a run of busy ports"
176+
177+
# The half of the ask that is easy to get backwards. Someone who typed a port
178+
# wants to hear it is taken, not to be moved off it without being told.
179+
echo "dashboard: an explicit port is never moved"
180+
busy_ports="8372"
181+
rc_is 1 "refuses rather than relocating" dashboard_resolve_port 8372 yes
182+
is "$(dashboard_resolve_port 8372 yes 2>/dev/null)" "" "offers no port to bind"
183+
out="$(dashboard_resolve_port 8372 yes 2>&1 >/dev/null)"
184+
contains "$out" "already in use" "says what is wrong"
185+
contains "$out" "--port 8373" "points at the flag, with a port that is free"
186+
187+
echo "dashboard: every port in the scan window is busy"
188+
saved_scan="$SFBOX_DASHBOARD_PORT_SCAN"
189+
SFBOX_DASHBOARD_PORT_SCAN=2
190+
busy_ports="8372 8373 8374"
191+
rc_is 1 "gives up rather than scanning on" dashboard_resolve_port 8372 ''
192+
contains "$(dashboard_resolve_port 8372 '' 2>&1 >/dev/null)" "8372" \
193+
"names the port it started from"
194+
contains "$(dashboard_resolve_port 8372 yes 2>&1 >/dev/null)" "pass --port" \
195+
"still gives advice when it has no port to suggest"
196+
SFBOX_DASHBOARD_PORT_SCAN="$saved_scan"
197+
198+
echo "dashboard: the scan stays inside the port range"
199+
busy_ports="65535"
200+
is "$(dashboard_free_port 65535)" "" "never suggests a port above 65535"
201+
202+
eval "$real_port_in_use"
203+
unset busy_ports
204+
205+
# ---- the probe, against a real listener -------------------------------------
206+
#
207+
# The stubs above prove the decisions. This proves the one thing they cannot:
208+
# that /dev/tcp really does see a held port, which is the assumption the rest of
209+
# the feature rests on. python3 holds the port because the repo already depends
210+
# on it for scripts/check_links.py, and because nc's flags differ across the BSD
211+
# and GNU builds this suite has to run on.
212+
213+
echo "dashboard: the probe against a real listener"
214+
if command -v python3 >/dev/null 2>&1; then
215+
probe_port="$(dashboard_free_port 18372)"
216+
spare_port=""
217+
[ -n "$probe_port" ] && spare_port="$(dashboard_free_port "$((probe_port + 1))")"
218+
if [ -n "$probe_port" ] && [ -n "$spare_port" ]; then
219+
python3 -c 'import socket, sys, time
220+
s = socket.socket()
221+
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
222+
s.bind(("127.0.0.1", int(sys.argv[1])))
223+
s.listen(8)
224+
sys.stdout.write("up\n")
225+
sys.stdout.flush()
226+
time.sleep(30)' "$probe_port" >"$SFBOX_TEST_ROOT/listener" 2>/dev/null &
227+
listener_pid=$!
228+
waited=0
229+
while [ "$waited" -lt 50 ] && ! grep -q up "$SFBOX_TEST_ROOT/listener" 2>/dev/null; do
230+
sleep 0.1
231+
waited=$((waited + 1))
232+
done
233+
if grep -q up "$SFBOX_TEST_ROOT/listener" 2>/dev/null; then
234+
rc_is 0 "sees a port a listener is holding" port_in_use "$probe_port"
235+
rc_is 1 "sees a port nothing is holding" port_in_use "$spare_port"
236+
else
237+
skip "the test listener never came up"
238+
fi
239+
kill "$listener_pid" 2>/dev/null
240+
wait "$listener_pid" 2>/dev/null
241+
else
242+
skip "no free local port to hold for the probe test"
243+
fi
244+
else
245+
skip "no python3, so the probe never met a real listener"
246+
fi
247+
147248
# ------------------------------------------------------ size guardrail -----
148249

149250
echo "prompt-size guardrail"
@@ -1058,5 +1159,7 @@ is "$(wc -c <"$PROBE_LOG" | tr -d ' ')" "0" "never offers the key to a box that
10581159
# ------------------------------------------------------------------ done ---
10591160

10601161
echo
1061-
printf '%s passed, %s failed\n' "$PASS" "$FAIL"
1162+
printf '%s passed, %s failed' "$PASS" "$FAIL"
1163+
[ "$SKIP" = 0 ] || printf ', %s skipped' "$SKIP"
1164+
printf '\n'
10621165
[ "$FAIL" = 0 ] || exit 1

0 commit comments

Comments
 (0)