Skip to content

Commit 245ea8d

Browse files
committed
fix(tooling): stop inventory.sh from answering wrongly on bad input
Six ways the selector could hand a caller the wrong host list, all of them silent, which is the failure mode the script exists to prevent: - `--tag ""` matched nothing, so the filter was dropped and the ENTIRE fleet came back at exit 0. An unset `$DEVNET` in `--tag "$DEVNET"` would act on hosts on other chains, the mirror image of the typo case already guarded. - The unknown-argument path printed help to stdout, and the documented idiom captures stdout: a mistyped flag fed 1.4 KB of usage text into a host loop. - `--not-tag T` hard-failed when no host carried T, contradicting the script's own `--tag validator` == `--tag <devnet> --not-tag aggregator` equivalence. An exclusion matching nothing is well-defined; only `--tag` is checked now. - Nothing validated nodes/subnets, so one space in the tags column shifted every later field: a role landed in `nodes`, `--tag validator` returned the aggregator, and a word reached a caller expecting a count. The file is now refused with the row named, rather than answered from. - Tag arguments were glob-expanded against the cwd, so `--tag 'devnet-*'` could silently resolve to a different group than typed. - `usage()` sliced help from a hardcoded line range; it now ends at the first non-comment line, so editing the header cannot truncate `--help`. The documented recipes were also unrunnable as written: `inventory.sh` is not on PATH, and the `| tr` pipeline discarded the exit code the script sets so carefully, turning any error into an empty-but-successful host list.
1 parent 79b2aeb commit 245ea8d

3 files changed

Lines changed: 68 additions & 21 deletions

File tree

.claude/skills/multi-server-devnet/SKILL.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,20 @@ file. It is the one place a deployment's urls and Grafana ids live.
5858
than reading it by hand:
5959

6060
```bash
61-
inventory.sh --tag devnet-ab --field ip # ips, one per line, for a loop
62-
inventory.sh --tag devnet-ab --tag aggregator # AND across tags
63-
inventory.sh --tag validator # derived, see below
64-
SERVERS=$(inventory.sh --field name | tr '\n' ' ')
61+
bash scripts/inventory.sh --tag devnet-ab --field ip # ips, for a loop
62+
bash scripts/inventory.sh --tag devnet-ab --tag aggregator # AND across tags
63+
bash scripts/inventory.sh --tag validator # derived, see below
64+
SERVERS=$(bash scripts/inventory.sh --field name) || exit # exit 2 = typo'd tag
65+
SERVERS=${SERVERS//$'\n'/ } # newlines -> spaces
6566
```
6667

6768
Tag conventions: a `devnet-*` tag names the chain a host's nodes belong to (two
6869
hosts sharing one means the split-chain model, *not* two like-named devnets), and
6970
`aggregator` is a whole-server role. `validator` is **derived** — "has a `devnet-*`
7071
tag and is not tagged `aggregator`" — so it can never disagree with the aggregator
71-
tag. An unknown tag exits 2 with the known-tag list rather than returning nothing,
72-
because a typo that yields an empty loop reports success while doing nothing.
72+
tag. An unknown or empty `--tag` exits 2 rather than returning nothing (or, worse,
73+
everything): a typo that yields an empty loop reports success while doing nothing,
74+
and `--tag "$UNSET"` would act on hosts the caller never named.
7375

7476
`NODES`/`SUBNETS` there are still the operator's inventory: the scripts take them
7577
as positional args, and the authority on a running devnet is always its own

.claude/skills/multi-server-devnet/scripts/devnet.inventory.example

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99
#
1010
# Read it with inventory.sh rather than by hand:
1111
#
12-
# inventory.sh --tag devnet-ab --field ip # ips, one per line, for a loop
13-
# inventory.sh --tag devnet-ab --tag aggregator # AND: both tags must be present
14-
# inventory.sh --tag validator # derived, see TAGS below
15-
# SERVERS=$(inventory.sh --field name | tr '\n' ' ')
12+
# bash scripts/inventory.sh --tag devnet-ab --field ip # ips, for a loop
13+
# bash scripts/inventory.sh --tag devnet-ab --tag aggregator # AND: both needed
14+
# bash scripts/inventory.sh --tag validator # derived, see TAGS
15+
# SERVERS=$(bash scripts/inventory.sh --field name) || exit # exit 2 = typo'd tag
16+
# SERVERS=${SERVERS//$'\n'/ } # newlines -> spaces
1617
#
1718
# FORMAT — whitespace-separated columns, aligned however you like. Blank lines and
1819
# lines whose first non-space character is '#' are ignored. Five columns:
@@ -22,7 +23,8 @@
2223
# resolves it, so a stale name here misroutes every ssh that uses it.
2324
# ip address. The one field worth re-deriving from the fleet rather than
2425
# trusting after a rebuild.
25-
# tags COMMA-separated, NO spaces (a space would start the next column).
26+
# tags COMMA-separated, NO spaces (a space would start the next column;
27+
# inventory.sh rejects the file rather than answer from shifted rows).
2628
# nodes NODES on this server, or '-' if it runs no devnet.
2729
# subnets ATTESTATION_COMMITTEE_COUNT, or '-'.
2830
#

.claude/skills/multi-server-devnet/scripts/inventory.sh

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,44 @@
1818
# It is computed, not read, so it can never disagree with the aggregator tag.
1919
# `--tag validator` is exactly `--tag <that devnet> --not-tag aggregator`.
2020
#
21-
# An unknown tag is an ERROR, not an empty result. A typo ('devnet5' for
21+
# An unknown `--tag` is an ERROR, not an empty result. A typo ('devnet5' for
2222
# 'devnet-5') that quietly returns no hosts turns `for h in $(inventory.sh ...)`
2323
# into a loop that does nothing and reports success, which is the same class of
2424
# failure devnet-env.sh guards against: acting on the wrong deployment, silently.
25-
# A tag that IS known but matches nothing is likewise exit 1 -- a real fleet has
26-
# no empty groups, so an empty match means the inventory is stale.
25+
# An EMPTY --tag value is an error for the mirror-image reason: an unset variable
26+
# in `--tag "$DEVNET"` would drop the filter and return the whole fleet, exit 0,
27+
# including hosts on other chains. A tag that IS known but matches nothing is
28+
# exit 1 -- a real fleet has no empty groups, so that means a stale inventory.
29+
#
30+
# `--not-tag` is deliberately NOT checked against the file: an exclusion that
31+
# matches nothing is well-defined, and demanding the tag exist would break the
32+
# documented `--tag <devnet> --not-tag aggregator` on any fleet whose aggregator
33+
# role is currently unassigned.
2734
set -u
2835

29-
usage() { sed -n '2,26p' "$0" | sed 's/^# \{0,1\}//'; exit "${1:-0}"; }
36+
# Sliced from the header block above, ending at the first non-comment line, so
37+
# editing the comment can't silently truncate --help.
38+
help_text() { awk 'NR == 1 { next } !/^#/ { exit } { sub(/^# ?/, ""); print }' "$0"; }
39+
40+
# Help goes to stdout for -h, but to stderr on the error path: callers capture
41+
# this script's stdout ($(inventory.sh --field name)), so help text printed there
42+
# becomes a list of "hosts" to ssh into.
43+
usage() {
44+
code=${1:-0}
45+
if [ "$code" = 0 ]; then help_text; else help_text >&2; fi
46+
exit "$code"
47+
}
48+
49+
# Whitespace-split a tag argument into SPLIT with globbing OFF: a tag is data, and
50+
# `--tag 'devnet-*'` must not expand against whatever happens to sit in the cwd.
51+
split_tags() {
52+
case $2 in
53+
*[![:space:]]*) ;;
54+
*) echo "$1 needs a non-empty value (an empty tag would match every host)" >&2
55+
exit 2 ;;
56+
esac
57+
set -f; SPLIT=($2); set +f
58+
}
3059

3160
file=""; field="default"; count=0
3261
tags=(); nottags=()
@@ -35,9 +64,9 @@ while [ $# -gt 0 ]; do
3564
case $1 in
3665
--tag) [ $# -ge 2 ] || { echo "$1 needs a value" >&2; exit 2; }
3766
# Split on whitespace so --tag 'devnet-5 aggregator' is two tags.
38-
for t in $2; do tags+=("$t"); done; shift 2 ;;
67+
split_tags "$1" "$2"; tags+=("${SPLIT[@]}"); shift 2 ;;
3968
--not-tag) [ $# -ge 2 ] || { echo "$1 needs a value" >&2; exit 2; }
40-
for t in $2; do nottags+=("$t"); done; shift 2 ;;
69+
split_tags "$1" "$2"; nottags+=("${SPLIT[@]}"); shift 2 ;;
4170
--field) [ $# -ge 2 ] || { echo "$1 needs a value" >&2; exit 2; }
4271
field=$2; shift 2 ;;
4372
--file) [ $# -ge 2 ] || { echo "$1 needs a value" >&2; exit 2; }
@@ -95,6 +124,17 @@ awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" \
95124
name = $1; ip = $2; tags = $3
96125
nodes = (NF >= 4 ? $4 : "-")
97126
subnets = (NF >= 5 ? $5 : "-")
127+
128+
# A space inside the tags column (or a trailing comment on a data row) shifts
129+
# every later field: $3 loses tags, and a ROLE lands in nodes. Unlike a short
130+
# row, which only drops one host, that silently reassigns roles and hands a
131+
# word to a caller expecting a count -- so refuse the whole file instead.
132+
if (nodes !~ /^([0-9]+|-)$/ || subnets !~ /^([0-9]+|-)$/) {
133+
printf "%s:%d: nodes/subnets must be a number or '\''-'\'', got '\''%s'\'' '\''%s'\''", \
134+
src, FNR, nodes, subnets > "/dev/stderr"
135+
printf " -- a space in the tags column shifts the columns right\n" > "/dev/stderr"
136+
malformed = 1; next
137+
}
98138
t = "," tags ","
99139
100140
# Every literal tag in the file, so a typo can be told from a real absence.
@@ -113,15 +153,18 @@ awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" \
113153
}
114154
115155
END {
156+
# Answering from a file whose columns do not line up would mean answering
157+
# about tags that were never written; the row numbers are already on stderr.
158+
if (malformed) {
159+
printf "%s: refusing to answer from a file that does not parse\n", src > "/dev/stderr"
160+
exit 2
161+
}
162+
116163
bad = 0
117164
for (i = 1; i <= nw; i++)
118165
if (W[i] != "validator" && !(W[i] in seen)) {
119166
printf "no such tag in %s: %s\n", src, W[i] > "/dev/stderr"; bad = 1
120167
}
121-
for (i = 1; i <= nn; i++)
122-
if (NW[i] != "validator" && !(NW[i] in seen)) {
123-
printf "no such tag in %s: %s\n", src, NW[i] > "/dev/stderr"; bad = 1
124-
}
125168
if (bad) {
126169
printf "known tags: " > "/dev/stderr"
127170
for (k in seen) printf "%s ", k > "/dev/stderr"

0 commit comments

Comments
 (0)