listDrivePeers() returned every peer holding PeerCapabilityTaildriveSharer. That cap is an ACL grant usually given to a whole group or tag, so the result was most of the tailnet — including offline peers and peers with no reachable peerAPI — despite the doc comment claiming it mirrored LocalBackend.driveRemotesFromPeers.
The wasm bridge now applies the same conjunction as upstream (ipn/ipnlocal/drive.go): online, non-empty peerAPI base, sharer cap. The peerAPI URL was already computed and then discarded, so the check is free.
tailscale/cmd/tsconnect/wasm/drive.go (submodule, branch fix/drive-peers-reachability on webnet/tailscale, based on webnet)
IPN.listDrivePeers and IPNDrivePeer doc comments corrected: the result is peers allowed to share, not peers that expose a share
headscale integration test now asserts every returned peer is online and has a non-empty peerAPIURL
Submodule
The pointer moves from 15a70243e to the new commit on top of the fork's webnet branch, so it also picks up cmd/tsconnect: remove obsolete wasm bridge APIs (drops the ssh, fetch and setExitNodeEnabled bridge methods). Neither @webnet/tsconnect nor @webnet/tsconnect-worker references those.
The submodule commit sits on a feature branch, opened as webnet/tailscale#18. Merge that (fast-forward preferred, so webnet lands exactly on the pinned commit) alongside this.
Verification
CI green on ab987f5
npm run build-go --workspace=packages/tsconnect — wasm builds clean; gofmt clean
Note that a bare GOOS=js GOARCH=wasm go build ./cmd/tsconnect/wasm fails on feature/ace at both the old and new submodule commits — pre-existing and unrelated; the tagged build path used by build.sh is fine
npm run test --workspace=packages/tsconnect against the headscale test tailnet: 78 pass, 0 fail, 1 skipped (setExitNode, needs TSCONNECT_TEST_EXIT_NODE_ID). The suite skips entirely in CI, which has no TSCONNECT_TEST_CONTROL_URL/TSCONNECT_TEST_AUTH_KEY
Caveat on that run: listDrivePeers returns [] on that tailnet, because no node there carries PeerCapabilityTaildriveSharer (headscale grants no such cap), so the new per-peer assertions never execute. They are guards against a regression, not evidence the filter works — that would need an ACL granting the sharer cap to an offline or peerAPI-less node. The filter itself was checked by reading it against upstream's Available closure, which the review independently repeated
Reviewed autonomously by Claude Sonnet 5 (review #37): no correctness issues; one test-strictness nit applied in ab987f5
Left open
The issue's "Beyond that" question — whether listDrivePeers should stay cheap-and-permissive (and be renamed) or gain an opt-in flag that does N peerAPI round-trips to filter on a non-empty share list — is not addressed here and needs a decision.
Fixes #143.
## What
`listDrivePeers()` returned every peer holding `PeerCapabilityTaildriveSharer`. That cap is an ACL grant usually given to a whole group or tag, so the result was most of the tailnet — including offline peers and peers with no reachable peerAPI — despite the doc comment claiming it mirrored `LocalBackend.driveRemotesFromPeers`.
The wasm bridge now applies the same conjunction as upstream (`ipn/ipnlocal/drive.go`): online, non-empty peerAPI base, sharer cap. The peerAPI URL was already computed and then discarded, so the check is free.
- `tailscale/cmd/tsconnect/wasm/drive.go` (submodule, branch `fix/drive-peers-reachability` on `webnet/tailscale`, based on `webnet`)
- `IPN.listDrivePeers` and `IPNDrivePeer` doc comments corrected: the result is peers *allowed* to share, not peers that expose a share
- headscale integration test now asserts every returned peer is online and has a non-empty `peerAPIURL`
## Submodule
The pointer moves from `15a70243e` to the new commit on top of the fork's `webnet` branch, so it also picks up `cmd/tsconnect: remove obsolete wasm bridge APIs` (drops the `ssh`, `fetch` and `setExitNodeEnabled` bridge methods). Neither `@webnet/tsconnect` nor `@webnet/tsconnect-worker` references those.
The submodule commit sits on a feature branch, opened as webnet/tailscale#18. Merge that (fast-forward preferred, so `webnet` lands exactly on the pinned commit) alongside this.
## Verification
- CI green on `ab987f5`
- `npm run build-go --workspace=packages/tsconnect` — wasm builds clean; `gofmt` clean
- Note that a bare `GOOS=js GOARCH=wasm go build ./cmd/tsconnect/wasm` fails on `feature/ace` at both the old and new submodule commits — pre-existing and unrelated; the tagged build path used by `build.sh` is fine
- `npm run test --workspace=packages/tsconnect` against the headscale test tailnet: 78 pass, 0 fail, 1 skipped (`setExitNode`, needs `TSCONNECT_TEST_EXIT_NODE_ID`). The suite skips entirely in CI, which has no `TSCONNECT_TEST_CONTROL_URL`/`TSCONNECT_TEST_AUTH_KEY`
- Caveat on that run: `listDrivePeers` returns `[]` on that tailnet, because no node there carries `PeerCapabilityTaildriveSharer` (headscale grants no such cap), so the new per-peer assertions never execute. They are guards against a regression, not evidence the filter works — that would need an ACL granting the sharer cap to an offline or peerAPI-less node. The filter itself was checked by reading it against upstream's `Available` closure, which the review independently repeated
- Reviewed autonomously by Claude Sonnet 5 (review #37): no correctness issues; one test-strictness nit applied in `ab987f5`
## Left open
The issue's "Beyond that" question — whether `listDrivePeers` should stay cheap-and-permissive (and be renamed) or gain an opt-in flag that does N peerAPI round-trips to filter on a non-empty share list — is not addressed here and needs a decision.
listDrivePeers returned every peer holding PeerCapabilityTaildriveSharer,
which is an ACL grant usually given to a whole group or tag, so offline
peers and peers with no reachable peerAPI were included. Bump the
tailscale submodule to the fix and correct the TS doc comments, which
claimed the result was peers exposing a share.
The submodule bump also picks up the removal of the obsolete ssh, fetch
and setExitNodeEnabled wasm bridge APIs, already on the fork's webnet
branch and unused here.
Closes#143
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correctness of the Go filter (tailscale/cmd/tsconnect/wasm/drive.go @ 7e9868f50)
Verified against upstream driveRemoteSource.Remotes's Available closure in ipn/ipnlocal/drive.go (~L376-386):
p.Online().Get(): NodeView.Online() returns views.ValuePointer[bool] wrapping Node.Online *bool (tailcfg/tailcfg_view.go:267). ValuePointer.Get() returns the zero value (false) when the pointer is nil (types/views/views.go:872-878) — no panic, and a nil/unknown online status is correctly treated as "not online", identical to upstream's own peer.Online().Get() call on the same type.
buildPeerAPIURL(p, selfHave4, selfHave6) vs upstream cn.PeerAPIBase(peer): same Hostinfo-services-based reachability logic and same self-address-family gating; the one difference (upstream short-circuits on !peer.Hostinfo().Valid(), the wasm helper doesn't) is a no-op in practice since an invalid Hostinfo yields an empty Services() iterator either way. Behaviorally equivalent.
i.lb.PeerCaps(a.Addr()).HasCapability(cap) looped over p.Addresses().All() vs upstream cn.PeerHasCap(peer, cap): upstream's own PeerHasCap (node_backend.go:530-540) does exactly the same per-address iteration internally, keyed by address rather than node identity in both cases. No divergence.
Check ordering (online -> peerAPI -> cap) matches upstream's own order and is a pure fail-fast optimization over three independent ANDed predicates; reordering wouldn't change the result set.
The fix is a correct, faithful mirror of driveRemotesFromPeers's filtering, and closes the gap described in #143 (offline/unreachable peers holding the sharer ACL cap no longer leak through).
TypeScript doc-comment and test changes (packages/tsconnect/src/ipn.ts, types.ts, ipn.test.ts)
Accurate: the corrected doc comments now say "peers allowed to share" rather than "peers that expose a share", matching the cap's real ACL-grant semantics (also correctly called out in the issue's "Beyond that" section, left unaddressed here as scoped). The new test assertions (p.peerAPIURL !== "", p.online !== false) are consistent with the Go side always populating a non-empty online: true for peers that pass the new filter; using notEqual(..., false) rather than asserting === true is slightly loose but harmless given the field is optional (online?: boolean) in the type. npm run typecheck --workspace=packages/tsconnect passes clean locally.
Submodule bump side effects
Confirmed via grep across packages/tsconnect/src and packages/tsconnect-worker/src: no references to the removed ssh, fetch, or setExitNodeEnabled wasm bridge methods (the fetch(...) hits in index.ts are all calls to the browser/global fetch() API for loading the wasm binary, unrelated to the removed bridge method). The unrelated cmd/tsconnect: remove obsolete wasm bridge APIs commit riding along on the submodule bump is inert for this repo.
Nothing else broke. No other findings — this is a small, well-scoped, correct fix.
Autonomous review (Claude Sonnet 5).
**Correctness of the Go filter (`tailscale/cmd/tsconnect/wasm/drive.go` @ 7e9868f50)**
Verified against upstream `driveRemoteSource.Remotes`'s `Available` closure in `ipn/ipnlocal/drive.go` (~L376-386):
- `p.Online().Get()`: `NodeView.Online()` returns `views.ValuePointer[bool]` wrapping `Node.Online *bool` (`tailcfg/tailcfg_view.go:267`). `ValuePointer.Get()` returns the zero value (`false`) when the pointer is nil (`types/views/views.go:872-878`) — no panic, and a nil/unknown online status is correctly treated as "not online", identical to upstream's own `peer.Online().Get()` call on the same type.
- `buildPeerAPIURL(p, selfHave4, selfHave6)` vs upstream `cn.PeerAPIBase(peer)`: same Hostinfo-services-based reachability logic and same self-address-family gating; the one difference (upstream short-circuits on `!peer.Hostinfo().Valid()`, the wasm helper doesn't) is a no-op in practice since an invalid Hostinfo yields an empty Services() iterator either way. Behaviorally equivalent.
- `i.lb.PeerCaps(a.Addr()).HasCapability(cap)` looped over `p.Addresses().All()` vs upstream `cn.PeerHasCap(peer, cap)`: upstream's own `PeerHasCap` (`node_backend.go:530-540`) does exactly the same per-address iteration internally, keyed by address rather than node identity in both cases. No divergence.
- Check ordering (online -> peerAPI -> cap) matches upstream's own order and is a pure fail-fast optimization over three independent ANDed predicates; reordering wouldn't change the result set.
The fix is a correct, faithful mirror of `driveRemotesFromPeers`'s filtering, and closes the gap described in #143 (offline/unreachable peers holding the sharer ACL cap no longer leak through).
**TypeScript doc-comment and test changes** (`packages/tsconnect/src/ipn.ts`, `types.ts`, `ipn.test.ts`)
Accurate: the corrected doc comments now say "peers allowed to share" rather than "peers that expose a share", matching the cap's real ACL-grant semantics (also correctly called out in the issue's "Beyond that" section, left unaddressed here as scoped). The new test assertions (`p.peerAPIURL !== ""`, `p.online !== false`) are consistent with the Go side always populating a non-empty `online: true` for peers that pass the new filter; using `notEqual(..., false)` rather than asserting `=== true` is slightly loose but harmless given the field is optional (`online?: boolean`) in the type. `npm run typecheck --workspace=packages/tsconnect` passes clean locally.
**Submodule bump side effects**
Confirmed via grep across `packages/tsconnect/src` and `packages/tsconnect-worker/src`: no references to the removed `ssh`, `fetch`, or `setExitNodeEnabled` wasm bridge methods (the `fetch(...)` hits in `index.ts` are all calls to the browser/global `fetch()` API for loading the wasm binary, unrelated to the removed bridge method). The unrelated `cmd/tsconnect: remove obsolete wasm bridge APIs` commit riding along on the submodule bump is inert for this repo.
**Nothing else broke.** No other findings — this is a small, well-scoped, correct fix.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
codinget
Fixes #143.
What
listDrivePeers()returned every peer holdingPeerCapabilityTaildriveSharer. That cap is an ACL grant usually given to a whole group or tag, so the result was most of the tailnet — including offline peers and peers with no reachable peerAPI — despite the doc comment claiming it mirroredLocalBackend.driveRemotesFromPeers.The wasm bridge now applies the same conjunction as upstream (
ipn/ipnlocal/drive.go): online, non-empty peerAPI base, sharer cap. The peerAPI URL was already computed and then discarded, so the check is free.tailscale/cmd/tsconnect/wasm/drive.go(submodule, branchfix/drive-peers-reachabilityonwebnet/tailscale, based onwebnet)IPN.listDrivePeersandIPNDrivePeerdoc comments corrected: the result is peers allowed to share, not peers that expose a sharepeerAPIURLSubmodule
The pointer moves from
15a70243eto the new commit on top of the fork'swebnetbranch, so it also picks upcmd/tsconnect: remove obsolete wasm bridge APIs(drops thessh,fetchandsetExitNodeEnabledbridge methods). Neither@webnet/tsconnectnor@webnet/tsconnect-workerreferences those.The submodule commit sits on a feature branch, opened as webnet/tailscale#18. Merge that (fast-forward preferred, so
webnetlands exactly on the pinned commit) alongside this.Verification
ab987f5npm run build-go --workspace=packages/tsconnect— wasm builds clean;gofmtcleanGOOS=js GOARCH=wasm go build ./cmd/tsconnect/wasmfails onfeature/aceat both the old and new submodule commits — pre-existing and unrelated; the tagged build path used bybuild.shis finenpm run test --workspace=packages/tsconnectagainst the headscale test tailnet: 78 pass, 0 fail, 1 skipped (setExitNode, needsTSCONNECT_TEST_EXIT_NODE_ID). The suite skips entirely in CI, which has noTSCONNECT_TEST_CONTROL_URL/TSCONNECT_TEST_AUTH_KEYlistDrivePeersreturns[]on that tailnet, because no node there carriesPeerCapabilityTaildriveSharer(headscale grants no such cap), so the new per-peer assertions never execute. They are guards against a regression, not evidence the filter works — that would need an ACL granting the sharer cap to an offline or peerAPI-less node. The filter itself was checked by reading it against upstream'sAvailableclosure, which the review independently repeatedab987f5Left open
The issue's "Beyond that" question — whether
listDrivePeersshould stay cheap-and-permissive (and be renamed) or gain an opt-in flag that does N peerAPI round-trips to filter on a non-empty share list — is not addressed here and needs a decision.Autonomous review (Claude Sonnet 5).
Correctness of the Go filter (
tailscale/cmd/tsconnect/wasm/drive.go@ 7e9868f50)Verified against upstream
driveRemoteSource.Remotes'sAvailableclosure inipn/ipnlocal/drive.go(~L376-386):p.Online().Get():NodeView.Online()returnsviews.ValuePointer[bool]wrappingNode.Online *bool(tailcfg/tailcfg_view.go:267).ValuePointer.Get()returns the zero value (false) when the pointer is nil (types/views/views.go:872-878) — no panic, and a nil/unknown online status is correctly treated as "not online", identical to upstream's ownpeer.Online().Get()call on the same type.buildPeerAPIURL(p, selfHave4, selfHave6)vs upstreamcn.PeerAPIBase(peer): same Hostinfo-services-based reachability logic and same self-address-family gating; the one difference (upstream short-circuits on!peer.Hostinfo().Valid(), the wasm helper doesn't) is a no-op in practice since an invalid Hostinfo yields an empty Services() iterator either way. Behaviorally equivalent.i.lb.PeerCaps(a.Addr()).HasCapability(cap)looped overp.Addresses().All()vs upstreamcn.PeerHasCap(peer, cap): upstream's ownPeerHasCap(node_backend.go:530-540) does exactly the same per-address iteration internally, keyed by address rather than node identity in both cases. No divergence.The fix is a correct, faithful mirror of
driveRemotesFromPeers's filtering, and closes the gap described in #143 (offline/unreachable peers holding the sharer ACL cap no longer leak through).TypeScript doc-comment and test changes (
packages/tsconnect/src/ipn.ts,types.ts,ipn.test.ts)Accurate: the corrected doc comments now say "peers allowed to share" rather than "peers that expose a share", matching the cap's real ACL-grant semantics (also correctly called out in the issue's "Beyond that" section, left unaddressed here as scoped). The new test assertions (
p.peerAPIURL !== "",p.online !== false) are consistent with the Go side always populating a non-emptyonline: truefor peers that pass the new filter; usingnotEqual(..., false)rather than asserting=== trueis slightly loose but harmless given the field is optional (online?: boolean) in the type.npm run typecheck --workspace=packages/tsconnectpasses clean locally.Submodule bump side effects
Confirmed via grep across
packages/tsconnect/srcandpackages/tsconnect-worker/src: no references to the removedssh,fetch, orsetExitNodeEnabledwasm bridge methods (thefetch(...)hits inindex.tsare all calls to the browser/globalfetch()API for loading the wasm binary, unrelated to the removed bridge method). The unrelatedcmd/tsconnect: remove obsolete wasm bridge APIscommit riding along on the submodule bump is inert for this repo.Nothing else broke. No other findings — this is a small, well-scoped, correct fix.