Skip to content

tsrelay/handler: don't panic on nil CurrentTailnet or Self - #358

Open
ZAG23 wants to merge 1 commit into
tailscale-dev:mainfrom
ZAG23:fix/tsrelay-nil-currenttailnet
Open

ZAG23 wants to merge 1 commit into
tailscale-dev:mainfrom
ZAG23:fix/tsrelay-nil-currenttailnet

Conversation

@ZAG23

@ZAG23 ZAG23 commented Sep 24, 2026

Copy link
Copy Markdown

getPeers checks st.CurrentTailnet for nil before copying it into the response, but then reads st.CurrentTailnet.MagicDNSSuffix unconditionally for every peer (get_peers.go:120). When tailscaled returns a status that lists peers but has no CurrentTailnet, each /peers poll panics and the connection is reset:

[tsrelay] http: panic serving 127.0.0.1:56614: runtime error: invalid memory address or nil pointer dereference
[tsrelay] github.com/tailscale-dev/vscode-tailscale/tsrelay/handler.(*handler).getPeers(...)
[tsrelay] 	/home/runner/work/vscode-tailscale/vscode-tailscale/tsrelay/handler/get_peers.go:120 +0x4d0
[error] could not poll for updates: d: request to http://127.0.0.1:56270/peers failed, reason: socket hang up

I hit this with v1.1.0 in VSCodium on macOS (standalone Tailscale app 1.102.4): 10 panics, one on its own and then nine consecutive 5-second polls about a minute and a half later, after which they stopped. I didn't determine why tailscaled briefly omitted CurrentTailnet.

Fix

  • getPeers: when CurrentTailnet is nil, return the offline error if there is one (the logged-out case, unchanged), and otherwise return an error so the request fails like other status errors. Returning "CurrentTailnet": null alongside peers would just move the crash: the extension reads status.CurrentTailnet.Name without a null check in src/node-explorer-provider.ts. With a failed request, the poll throws before currentStatus is replaced, the same path a reset connection took before, so the Node Explorer keeps its last good tree and picks up changes on the next good poll.
  • Defensive nil checks for Self in getPeers, getServe (which already checks Self a few lines earlier) and serveConfigDNS. tailscaled always sets Self, but -mockfile profiles don't have to.

Testing

  • New tsrelay/handler/nil_status_test.go. Without the fix, four of its five tests panic, at get_peers.go:120, get_peers.go:140, get_serve.go:167 and create_serve.go:96; the logged-out test passes before and after. With the fix all five pass, along with go vet ./tsrelay/... and go test ./tsrelay/....
  • Against a live tailnet, the full /peers and /serve JSON responses are identical to the v1.1.0 binary's.
  • Run standalone with a -mockfile status that is running, lists peers, and has no CurrentTailnet: v1.1.0 panics, and this returns a plain-text 500 that the extension's resp.json() rejects, so the poll counts as failed.
  • Built with Go 1.27 and GOEXPERIMENT=nojsonv2: 1.27 enables jsonv2 by default, and the pinned go-json-experiment/json doesn't compile against it. Not tested with ./tool/go.

🤖 Generated with Claude Code

getPeers checks CurrentTailnet before copying it into the response, but
then dereferences st.CurrentTailnet.MagicDNSSuffix for every peer. If
tailscaled returns a status that lists peers but has no CurrentTailnet,
every /peers request panics and the connection is reset.

The extension expects CurrentTailnet to be set whenever there is no
error, so returning it as null would just move the failure into the
Node Explorer. Instead, return the offline error when there is one (the
logged-out case, unchanged), and otherwise fail the request like other
status errors, so the extension keeps its last good tree and retries.

Also add nil checks for Self in getPeers, getServe and serveConfigDNS.
tailscaled always sets Self, but these handlers can be pointed at a
-mockfile profile, and getServe already checks Self a few lines earlier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: ZAG23 <zacharygruenberg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant