Merge pull request #161 from claude-bot-andresmgsl/build/160-self-scoped-tags

fix: scope the netmap tag read to Self, not the whole document

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)

commands/lib/runner-config.sh

@@ -14,8 +14,8 @@ json_field() {
     | head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//' || true
 }
 
-# json_string_array <file> <key> — the elements of the FIRST array named <key>,
-# one per line, empty when the key is absent or the array is empty.
+# json_string_array <file> <key> — the elements of the array named <key> 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 <key>
-# (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 <key> (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
 }

test/cli.sh

@@ -1253,6 +1253,7 @@ tags_empty() { # tags_empty <file> — 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",
@@ -1273,23 +1274,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.