From 56478d7a7d14e71f32f5ca4bd445a8bdc9d2953a Mon Sep 17 00:00:00 2001 From: claude-bot-andresmgsl Date: Sat, 25 Jul 2026 16:02:54 +0000 Subject: [PATCH] fix: scope the netmap tag read to Self MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit json_string_array took the first "Tags" array anywhere in the document, justified by Self-before-Peer field order. An untagged Self omits the key entirely (Go omitempty), so the match fell through into Peer and returned a peer's tag — false-refusing every login join and false-verifying untagged authkey joins on any tailnet with a tagged node. Extract Self by brace counting (Location nests an object, so slicing to the next key would truncate) and read the array inside it. Refs #160 Co-Authored-By: Claude Fable 5 --- changelog.d/160.md | 3 ++ commands/lib/runner-config.sh | 53 +++++++++++++++++++++--------- test/cli.sh | 61 +++++++++++++++++++++++++++++++++-- 3 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 changelog.d/160.md diff --git a/changelog.d/160.md b/changelog.d/160.md new file mode 100644 index 0000000..3a0aeb0 --- /dev/null +++ b/changelog.d/160.md @@ -0,0 +1,3 @@ +### Fixed + +- The netmap tag read is scoped to `Self`: an untagged node next to tagged peers no longer reads a peer's tag, false-refusing `--join login` and false-verifying untagged authkey joins (#160) diff --git a/commands/lib/runner-config.sh b/commands/lib/runner-config.sh index cef94d6..6f69327 100644 --- a/commands/lib/runner-config.sh +++ b/commands/lib/runner-config.sh @@ -14,8 +14,8 @@ json_field() { | head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//' || true } -# json_string_array — the elements of the FIRST array named , -# one per line, empty when the key is absent or the array is empty. +# json_string_array — the elements of the array named inside +# the netmap's `Self` object, one per line; empty when Self or the key is absent. # # json_field's sibling for the one shape it cannot read: `.Self.Tags` from # `tailscale status --json` is a JSON array, and bootstrap must assert on it to @@ -23,21 +23,44 @@ json_field() { # not the tag rig requested. Same grep/sed spirit, same jq-free reason: a # rig-bootstrapped box has no jq and we will not install one to read one field. # -# `tr -d '\n'` first, because tailscale pretty-prints its JSON and an array -# spans lines — grep is line-oriented and would never see `[ ... ]` whole -# otherwise. `\[[^]]*\]` then captures the first flat array body for -# (tag strings never contain `]`, so this is safe); the inner `grep -o` pulls -# every quoted token out of it, and `sed 1d` drops the key's own name — which -# `"key":[...]` leads with — leaving just the elements. +# Scoped to Self, NOT document-global. The previous body took the first "Tags" +# array anywhere in the file and justified it with Self-before-Peer field order. +# That holds only when Self HAS tags: an untagged Self omits the key entirely +# (Go omitempty), so the match fell through into Peer and returned a PEER's tag +# — silently inverting both callers on any tailnet with a tagged node (#160). # -# FIRST array wins by design, and the caller leans on it: `tailscale status -# --json` emits Self before Peer (Go struct field order, stable), so the first -# "Tags" is the node's OWN, never a peer's. An absent key omits itself entirely -# (Go's omitempty) rather than emitting `[]` — which is exactly the untagged, -# user-owned node bootstrap must catch. Never fails under `set -e`+pipefail: a -# non-match is a fact to test for, like json_field, not a reason to die. +# Self is brace-counted rather than sliced to the next key: PeerStatus carries a +# nested object (Location, a pointer with omitempty), which would end a naive +# slice early whenever it is present. Known limit of staying jq-free: a `{` or +# `}` inside a STRING value within Self would miscount — no PeerStatus string +# field (hostnames, DNS names, OS, key strings) can contain one, so this is +# sound in practice, but it is a real assumption, written down on purpose. +# +# `tr -d '\n'` first, because tailscale pretty-prints its JSON and the object +# spans lines — awk and grep are line-oriented and would never see it whole +# otherwise. `\[[^]]*\]` then captures the flat array body for (tag +# strings never contain `]`, so this is safe); the inner `grep -o` pulls every +# quoted token out of it, and `sed 1d` drops the key's own name — which +# `"key":[...]` leads with — leaving just the elements. Never fails under +# `set -e`+pipefail: a non-match is a fact to test for, not a reason to die. json_string_array() { - tr -d '\n' < "$1" 2>/dev/null \ + local self + self="$(tr -d '\n' < "$1" 2>/dev/null | awk ' + { + i = index($0, "\"Self\"") + if (i == 0) exit + s = substr($0, i) + j = index(s, "{") + if (j == 0) exit + depth = 0 + for (k = j; k <= length(s); k++) { + c = substr(s, k, 1) + if (c == "{") depth++ + else if (c == "}") { depth--; if (depth == 0) { print substr(s, j, k - j + 1); exit } } + } + }')" || true + [ -n "$self" ] || return 0 + printf '%s' "$self" \ | grep -o "\"$2\"[[:space:]]*:[[:space:]]*\[[^]]*\]" \ | head -n1 | grep -o '"[^"]*"' | sed '1d; s/^"//; s/"$//' || true } diff --git a/test/cli.sh b/test/cli.sh index fff0f8c..eb1ac34 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -1214,6 +1214,7 @@ tags_empty() { # tags_empty — exit 0 iff the reader prints NOTHING } FIX_TAGGED="$(mktemp)" # Self carries two tags; a peer carries a third FIX_UNTAGGED="$(mktemp)" # Self has no Tags key at all — the untagged hazard +FIX_NESTED="$(mktemp)" # tagged Self carrying a nested Location object cat > "$FIX_TAGGED" <<'JSON' { "BackendState": "Running", @@ -1234,23 +1235,77 @@ cat > "$FIX_TAGGED" <<'JSON' } } JSON +# The peers are the point (#160): an untagged Self OMITS its Tags key (Go +# omitempty), and the old document-global reader then fell through into Peer and +# returned tag:server here. Every real tailnet has this shape — untagged Self +# next to tagged peers — which the peerless fixture this replaces never covered. cat > "$FIX_UNTAGGED" <<'JSON' { "BackendState": "Running", "Self": { "HostName": "user-owned-box" + }, + "Peer": { + "nodekey:aaa": { + "HostName": "coolify-box", + "Tags": [ + "tag:server" + ] + }, + "nodekey:bbb": { + "HostName": "ci-box", + "Tags": [ + "tag:ci" + ] + } + } +} +JSON +# Location is a nested object INSIDE Self (a pointer with omitempty in the real +# netmap): a reader that sliced Self to the next key would end early at its +# closing brace and drop the Tags that follow — the brace counter must not. +cat > "$FIX_NESTED" <<'JSON' +{ + "BackendState": "Running", + "Self": { + "HostName": "coolify-box", + "Location": { + "Country": "Croatia", + "CountryCode": "HR" + }, + "Tags": [ + "tag:server", + "tag:prod" + ] + }, + "Peer": { + "nodekey:abc": { + "HostName": "ci-box", + "Tags": [ + "tag:ci" + ] + } } } JSON check "json_string_array: reads the first array element" 0 "tag:ci" tags "$FIX_TAGGED" check "json_string_array: reads a later array element" 0 "tag:build" tags "$FIX_TAGGED" -# Self precedes Peer in the netmap, so the FIRST "Tags" is the node's own: exactly -# two elements read proves the peer's tag:server did not leak into Self's tags. +# The reader is scoped to the Self object: exactly two elements read proves the +# peer's tag:server did not leak into Self's tags. check "json_string_array: reads Self's array, not a peer's" 0 "2" tags_count "$FIX_TAGGED" # An absent key omits itself (Go omitempty), never emits []: empty is the signal # bootstrap turns into a hard untagged-key refusal, so it must read as empty here. check "json_string_array: absent Tags key prints nothing" 0 "" tags_empty "$FIX_UNTAGGED" -rm -f "$FIX_TAGGED" "$FIX_UNTAGGED" +# Regression, #160: with tagged peers present, an untagged Self must STILL read +# empty — pre-fix this returned the peer's tag:server, false-refusing every +# login join and false-verifying untagged authkey joins as tagged. +check "json_string_array: untagged Self + tagged peers reads empty (#160)" \ + 0 "" tags_empty "$FIX_UNTAGGED" +check "json_string_array: nested Location does not truncate Self's tags" \ + 0 "2" tags_count "$FIX_NESTED" +check "json_string_array: reads past a nested object to a later element" \ + 0 "tag:prod" tags "$FIX_NESTED" +rm -f "$FIX_TAGGED" "$FIX_UNTAGGED" "$FIX_NESTED" # The guard is only worth something if it runs BEFORE the box is touched: the # token prompt, the download, configure and svc.sh start all come after it.