From e5473137371aa76762b170775fa2f726f547c70f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 23 Aug 2026 23:46:44 +0000 Subject: [PATCH 1/4] feat: resolve Tailscale hostnames at sync time and auto-rebind stale sink IPs After Tailscale re-auth, the sink may get a new 100.x IP while a leftover offline node keeps the old hostname. This breaks sync when sink.url has a frozen IP that no longer exists. Changes: 1. Source-side hostname resolution: - Add ResolvePeerIP to tsclient that resolves Tailscale hostnames via `tailscale status --json`, preferring Online peers over offline duplicates - Add ResolveSinkURL helper that rewrites hostname URLs to resolved IPs - Source now resolves sink.url hostnames at sync time before POST 2. Sink-side IP rebind: - Add maybeRebindListenAddr that detects when listen.addr IP is stale (not on any local interface) and auto-rebinds to current tailnet IP - Preserves the configured port, only swaps the IP - Localhost bindings are not subject to rebind 3. Documentation: - Update README, quickstart.md, and example configs to recommend hostnames - Add notes about auto-rebind behavior This enables sink.url: http://grok-bot:9999/sync (hostname) to keep working when the sink's 100.x changes after re-auth. Existing IP-literal configs continue to work unchanged. Tests: cover online vs offline duplicate hostname, URL rewriting, sink rebind Co-authored-by: Matt Van Horn --- README.md | 6 +- docs/quickstart.md | 4 +- examples/sink.yaml | 9 +- examples/source.yaml | 2 + internal/cli/sink.go | 88 ++++++++++ internal/cli/sink_test.go | 61 +++++++ internal/cli/source.go | 22 ++- internal/tsclient/tsclient.go | 150 ++++++++++++++++ internal/tsclient/tsclient_test.go | 270 +++++++++++++++++++++++++++++ 9 files changed, 602 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 969da90..396dfad 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,9 @@ mkdir -p ~/.config/agentcookie # 3. Write sink.yaml cat > ~/.config/agentcookie/sink.yaml << 'EOF' listen: - addr: 100.x.y.z:9999 # Your Linux box's Tailscale IP + # Use your current Tailscale IP. After Tailscale re-auth, if this IP + # becomes stale, the sink auto-rebinds to the new 100.x address. + addr: 100.x.y.z:9999 peer: hostname: your-mac.tailnet # Mac's Tailscale hostname @@ -156,7 +158,7 @@ agentcookie pair --as sink \ ``` Replace: -- `100.x.y.z` with your Linux box's Tailscale IP (`tailscale ip -4`) +- `100.x.y.z` with your current Tailscale IP (`tailscale ip -4`). If Tailscale re-auth gives the sink a new IP, the sink auto-rebinds to it on next start. - `your-mac.tailnet` with your Mac's Tailscale hostname (`tailscale status` on either machine) - The pairing code and URL with the values printed by the Mac source wizard diff --git a/docs/quickstart.md b/docs/quickstart.md index 65e91a0..ba0646e 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -27,7 +27,7 @@ cp agentcookie/examples/blocklist.yaml ~/.config/agentcookie/blocklist.yaml ``` Edit `source.yaml`: -- `sink.url`: the sink's tailnet URL, e.g. `http://my-mac-mini.tailnet.ts.net:9999/sync` +- `sink.url`: the sink's tailnet URL. **Use a hostname** (e.g., `http://my-mac-mini.tailnet.ts.net:9999/sync` or the short MagicDNS name `http://my-mac-mini:9999/sync`) rather than a frozen 100.x IP. The source resolves the hostname via `tailscale status --json` at sync time, preferring Online peers, so sync keeps working after the sink's Tailscale re-auth assigns a new IP. - `peer.hostname`: the sink's tailnet hostname Edit `blocklist.yaml` or run `agentcookie accounts off ` for sites you do not want to sync. Empty blocklist means sync everything. For a stricter agent-runtime setup, set `policy: allowlist` in `blocklist.yaml` and list only the exact hosts/subdomains you want to sync; all other cookie hosts are dropped on both source and sink. @@ -40,7 +40,7 @@ cp agentcookie/examples/blocklist.yaml ~/.config/agentcookie/blocklist.yaml ``` Edit `sink.yaml`: -- `listen.addr`: the sink's tailnet IP + port, e.g. `100.x.y.z:9999` +- `listen.addr`: the sink's current tailnet IP + port, e.g. `100.x.y.z:9999`. After Tailscale re-auth, if this IP becomes stale, the sink auto-rebinds to the new 100.x address on startup. - `peer.hostname`: the source's tailnet hostname - `cdp.enabled: true` if you want cookies to land in a running Chrome immediately (recommended) diff --git a/examples/sink.yaml b/examples/sink.yaml index be4e36d..d41b489 100644 --- a/examples/sink.yaml +++ b/examples/sink.yaml @@ -6,11 +6,12 @@ # See examples/blocklist.yaml for the sync-all configuration. listen: - # Address the sink's /sync HTTP listener binds to. MUST be your - # Tailscale 100.x address. The sink refuses to start on 0.0.0.0 or - # any non-tailnet routable address. + # Address the sink's /sync HTTP listener binds to. Use your current + # Tailscale 100.x address. After Tailscale re-auth, if this IP becomes + # stale (no longer on any local interface), the sink auto-rebinds to + # the new 100.x address on startup. # - # Find your Tailscale IP: tailscale ip -4 + # Find your current Tailscale IP: tailscale ip -4 addr: 100.x.y.z:9999 peer: diff --git a/examples/source.yaml b/examples/source.yaml index f8b23c0..474cbc3 100644 --- a/examples/source.yaml +++ b/examples/source.yaml @@ -3,6 +3,8 @@ sink: # URL of the sink machine's /sync endpoint over your tailnet. + # Use the MagicDNS hostname (not a 100.x IP) so the source follows + # the sink after Tailscale re-auth gives it a new IP. url: http://my-mac-mini.tailnet.ts.net:9999/sync chrome: diff --git a/internal/cli/sink.go b/internal/cli/sink.go index 2f69088..8d48ef9 100644 --- a/internal/cli/sink.go +++ b/internal/cli/sink.go @@ -29,6 +29,7 @@ import ( "github.com/mvanhorn/agentcookie/internal/sinkpush" "github.com/mvanhorn/agentcookie/internal/state" "github.com/mvanhorn/agentcookie/internal/transport" + "github.com/mvanhorn/agentcookie/internal/tsclient" ) var ( @@ -73,6 +74,13 @@ func runSink(cmd *cobra.Command, args []string) error { return fmt.Errorf("sink listen %q: %w", cfg.Listen.Addr, err) } + // Auto-rebind if configured listen.addr IP is stale (not on any local + // interface). This handles the case where Tailscale re-auth gave the + // machine a new 100.x IP but sink.yaml still has the old frozen IP. + // Keep the configured port, just swap the IP. Localhost bindings are + // excluded from rebind since they don't depend on Tailscale state. + cfg.Listen.Addr = maybeRebindListenAddr(cmd.Context(), cfg.Listen.Addr) + // Linux sink: require Tailscale 100.x in production. Localhost is // allowed for tests but is not the documented Linux sink path. if config.IsLinux() { @@ -758,3 +766,83 @@ func unionCookiesWithExtraProfiles(envelopeCookies []chrome.Cookie, profileDir s func cookieDedupeKey(c chrome.Cookie) string { return c.HostKey + "\x00" + c.Name + "\x00" + c.Path } + +// maybeRebindListenAddr checks if the configured listen address IP is currently +// bound on a local interface. If not (e.g., Tailscale re-auth gave the machine +// a new 100.x IP), it rebinds to the current tailnet IP while keeping the +// configured port. +// +// This allows sink.yaml to have a frozen Tailscale IP that becomes stale after +// re-auth, without requiring manual edit. The sink will automatically find and +// bind to the new IP. +// +// Returns the original addr unchanged if: +// - The IP is currently bound locally +// - The IP is localhost/loopback (not subject to Tailscale churn) +// - RequireTailnetIP fails (Tailscale not running) +// - The address parsing fails +func maybeRebindListenAddr(ctx context.Context, addr string) string { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return addr + } + + // Localhost bindings don't need rebind + switch host { + case "127.0.0.1", "::1", "localhost": + return addr + } + + // Check if the configured IP is on a local interface + if isIPBoundLocally(host) { + return addr + } + + // IP is not bound locally. If it's a tailnet IP, try to get the current one. + if !tsclient.IsTailnetIP(host) { + // Not a tailnet IP, can't auto-rebind + return addr + } + + // Get the current tailnet IP + newIP, err := tsclient.RequireTailnetIP(ctx) + if err != nil { + // Tailscale not running or no IP available + fmt.Fprintf(os.Stderr, "agentcookie sink: configured listen IP %s not on any local interface, but cannot get current tailnet IP: %v\n", host, err) + return addr + } + + if newIP == host { + // Same IP, no rebind needed (shouldn't happen since isIPBoundLocally failed) + return addr + } + + newAddr := net.JoinHostPort(newIP, port) + fmt.Fprintf(os.Stderr, "agentcookie sink: configured listen IP %s is stale (not on any local interface); rebinding to current tailnet IP %s\n", host, newIP) + return newAddr +} + +// isIPBoundLocally returns true if the given IP address is currently assigned +// to a local network interface. Used to detect stale Tailscale IPs after re-auth. +func isIPBoundLocally(ipStr string) bool { + targetIP := net.ParseIP(ipStr) + if targetIP == nil { + return false + } + + addrs, err := net.InterfaceAddrs() + if err != nil { + return false + } + + for _, a := range addrs { + ipnet, ok := a.(*net.IPNet) + if !ok { + continue + } + if ipnet.IP.Equal(targetIP) { + return true + } + } + return false +} diff --git a/internal/cli/sink_test.go b/internal/cli/sink_test.go index f71b0af..554f9b6 100644 --- a/internal/cli/sink_test.go +++ b/internal/cli/sink_test.go @@ -729,3 +729,64 @@ func TestSinkHandler_EmptyEnvelopeStillUnionsExtraProfiles(t *testing.T) { t.Errorf("empty envelope on Linux should return empty after union; got %d", len(result)) } } + +// TestIsIPBoundLocally verifies the local IP detection used by sink rebind. +func TestIsIPBoundLocally(t *testing.T) { + // 127.0.0.1 should always be bound locally on any system. + if !isIPBoundLocally("127.0.0.1") { + t.Error("127.0.0.1 should be bound locally") + } + + // An arbitrary private IP that's unlikely to be bound. + if isIPBoundLocally("10.99.99.99") { + t.Error("10.99.99.99 should not be bound locally (unless coincidentally assigned)") + } + + // Invalid IP should return false. + if isIPBoundLocally("not-an-ip") { + t.Error("invalid IP should return false") + } + + // Empty string should return false. + if isIPBoundLocally("") { + t.Error("empty string should return false") + } +} + +// TestMaybeRebindListenAddr_LocalhostPassthrough verifies that localhost +// bindings are not subject to rebind. +func TestMaybeRebindListenAddr_LocalhostPassthrough(t *testing.T) { + cases := []string{ + "127.0.0.1:9999", + "localhost:9999", + } + for _, addr := range cases { + t.Run(addr, func(t *testing.T) { + got := maybeRebindListenAddr(context.Background(), addr) + if got != addr { + t.Errorf("localhost should pass through unchanged: got %q, want %q", got, addr) + } + }) + } +} + +// TestMaybeRebindListenAddr_BoundIPPassthrough verifies that IPs currently +// bound locally are not subject to rebind. +func TestMaybeRebindListenAddr_BoundIPPassthrough(t *testing.T) { + // 127.0.0.1 is always bound locally. + addr := "127.0.0.1:9999" + got := maybeRebindListenAddr(context.Background(), addr) + if got != addr { + t.Errorf("bound IP should pass through unchanged: got %q, want %q", got, addr) + } +} + +// TestMaybeRebindListenAddr_ParseError verifies that unparseable addresses +// are returned unchanged. +func TestMaybeRebindListenAddr_ParseError(t *testing.T) { + addr := "no-port-here" + got := maybeRebindListenAddr(context.Background(), addr) + if got != addr { + t.Errorf("unparseable address should pass through unchanged: got %q, want %q", got, addr) + } +} diff --git a/internal/cli/source.go b/internal/cli/source.go index be1b699..0ab621a 100644 --- a/internal/cli/source.go +++ b/internal/cli/source.go @@ -22,6 +22,7 @@ import ( "github.com/mvanhorn/agentcookie/internal/secretsbus" "github.com/mvanhorn/agentcookie/internal/state" "github.com/mvanhorn/agentcookie/internal/transport" + "github.com/mvanhorn/agentcookie/internal/tsclient" "github.com/mvanhorn/agentcookie/internal/watcher" ) @@ -378,6 +379,23 @@ func pushOnce( return 0, dbsc, fmt.Errorf("seal payload: %w", err) } + // Resolve sink URL hostname to IP via Tailscale if needed. This allows + // sink.url to use MagicDNS hostnames (e.g., http://grok-bot:9999/sync) + // instead of frozen 100.x IPs that break after Tailscale re-auth. + // If resolution fails (Tailscale not available, peer offline), fall back + // to the original URL and let the HTTP layer report the connection error. + sinkURL := cfg.Sink.URL + if resolved, resolveErr := tsclient.ResolveSinkURL(ctx, sinkURL); resolveErr != nil { + if verbose { + fmt.Fprintf(os.Stderr, "agentcookie source: sink URL resolution failed (%v); using original %s\n", resolveErr, sinkURL) + } + } else if resolved != sinkURL { + if verbose { + fmt.Fprintf(os.Stderr, "agentcookie source: resolved sink URL %s -> %s\n", sinkURL, resolved) + } + sinkURL = resolved + } + // Bound the POST by the SyncClient profile's timeout (5 minutes // in v0.12) so a heavy LocalStorage / IndexedDB payload over a // slow tailnet link does not get cut off at the pre-v0.12 30s @@ -386,14 +404,14 @@ func pushOnce( // cancellation. postCtx, cancel := context.WithTimeout(ctx, httpserver.Defaults(httpserver.SyncClient).ClientTimeout) defer cancel() - req, err := http.NewRequestWithContext(postCtx, "POST", cfg.Sink.URL, bytes.NewReader(sealed)) + req, err := http.NewRequestWithContext(postCtx, "POST", sinkURL, bytes.NewReader(sealed)) if err != nil { return 0, dbsc, fmt.Errorf("new request: %w", err) } req.Header.Set("Content-Type", "application/octet-stream") resp, err := httpserver.Client(httpserver.SyncClient).Do(req) if err != nil { - return 0, dbsc, fmt.Errorf("POST to sink %s: %w", cfg.Sink.URL, err) + return 0, dbsc, fmt.Errorf("POST to sink %s: %w", sinkURL, err) } defer resp.Body.Close() body, _ := io.ReadAll(resp.Body) diff --git a/internal/tsclient/tsclient.go b/internal/tsclient/tsclient.go index b2de7aa..cd67a4a 100644 --- a/internal/tsclient/tsclient.go +++ b/internal/tsclient/tsclient.go @@ -9,6 +9,8 @@ import ( "encoding/json" "errors" "fmt" + "net" + "net/url" "os" "os/exec" "strings" @@ -126,3 +128,151 @@ func (s *Status) SelfHostname() string { } return s.Self.HostName } + +// ErrPeerNotFound is returned by ResolvePeerIP when no matching peer exists. +var ErrPeerNotFound = errors.New("tsclient: peer not found") + +// ErrPeerOffline is returned by ResolvePeerIP when all matching peers are offline. +var ErrPeerOffline = errors.New("tsclient: peer is offline") + +// ErrPeerNoIPv4 is returned when a peer has no IPv4 address in TailscaleIPs. +var ErrPeerNoIPv4 = errors.New("tsclient: peer has no IPv4 address") + +// ResolvePeerIP resolves a Tailscale hostname to its 100.x IPv4 address. +// When multiple peers share the same hostname (a common scenario after +// Tailscale re-auth creates a new node while the old offline node lingers), +// this function prefers Online peers over offline duplicates. +// +// The hostname can be: +// - Short MagicDNS name: "grok-bot" +// - Full MagicDNS FQDN: "grok-bot.tail-xxxx.ts.net." +// - HostName field value: "grok-bot-1" +// +// Returns ErrPeerNotFound if no peer matches, ErrPeerOffline if all matches +// are offline, or ErrPeerNoIPv4 if the peer lacks an IPv4 address. +func (s *Status) ResolvePeerIP(hostname string) (string, error) { + if s == nil { + return "", ErrPeerNotFound + } + + target := strings.ToLower(strings.TrimSpace(hostname)) + // Strip trailing dot from FQDN if present + target = strings.TrimSuffix(target, ".") + + var matches []*PeerStatus + for _, p := range s.Peer { + if p == nil { + continue + } + // Match against HostName (e.g., "grok-bot-1") + if strings.EqualFold(p.HostName, target) { + matches = append(matches, p) + continue + } + // Match against DNS name's first label (MagicDNS short name) + if label, _, ok := strings.Cut(p.DNSName, "."); ok && strings.EqualFold(label, target) { + matches = append(matches, p) + continue + } + // Match against full DNSName (with or without trailing dot) + dnsLower := strings.ToLower(strings.TrimSuffix(p.DNSName, ".")) + if dnsLower == target { + matches = append(matches, p) + continue + } + } + + if len(matches) == 0 { + return "", fmt.Errorf("%w: %q", ErrPeerNotFound, hostname) + } + + // Prefer Online peers. If multiple are online, take the first one. + // If none are online, return ErrPeerOffline. + var best *PeerStatus + for _, p := range matches { + if p.Online { + best = p + break + } + } + if best == nil { + return "", fmt.Errorf("%w: %q (all %d matching nodes are offline; check `tailscale status`)", ErrPeerOffline, hostname, len(matches)) + } + + // Extract IPv4 from TailscaleIPs + for _, ip := range best.TailscaleIPs { + if IsTailnetIP(ip) { + return ip, nil + } + } + + return "", fmt.Errorf("%w: %q", ErrPeerNoIPv4, hostname) +} + +// ResolvePeerIPWithCLI is a convenience wrapper that finds the Tailscale CLI, +// gets the current status, and resolves the hostname. Use this when you don't +// already have a Status object. +func ResolvePeerIPWithCLI(ctx context.Context, hostname string) (string, error) { + cli, err := FindCLI() + if err != nil { + return "", err + } + st, err := Get(ctx, cli) + if err != nil { + return "", err + } + return st.ResolvePeerIP(hostname) +} + +// ResolveSinkURL takes a sink URL and resolves any hostname in it to a +// Tailscale IP, preferring Online peers. If the URL already uses an IP +// address, it is returned unchanged. If the hostname cannot be resolved +// (Tailscale not available, peer not found, peer offline), the original +// URL is returned along with an error so the caller can decide whether +// to proceed with the unresolved URL. +// +// Examples: +// - "http://grok-bot:9999/sync" -> "http://100.124.19.34:9999/sync" +// - "http://grok-bot.tail-xxxx.ts.net:9999/sync" -> "http://100.124.19.34:9999/sync" +// - "http://100.87.49.2:9999/sync" -> "http://100.87.49.2:9999/sync" (unchanged) +func ResolveSinkURL(ctx context.Context, rawURL string) (string, error) { + parsed, err := parseURL(rawURL) + if err != nil { + return rawURL, fmt.Errorf("parse sink URL: %w", err) + } + + host := parsed.Hostname() + port := parsed.Port() + + // If host is already an IP, return unchanged + if isIPAddress(host) { + return rawURL, nil + } + + // Resolve hostname via Tailscale + resolvedIP, err := ResolvePeerIPWithCLI(ctx, host) + if err != nil { + return rawURL, err + } + + // Reconstruct the URL with the resolved IP + newHost := resolvedIP + if port != "" { + newHost = resolvedIP + ":" + port + } + parsed.Host = newHost + + return parsed.String(), nil +} + +// isIPAddress returns true if s is an IPv4 or IPv6 address literal. +func isIPAddress(s string) bool { + // net.ParseIP returns nil for invalid IPs and for hostnames + ip := net.ParseIP(s) + return ip != nil +} + +// parseURL is a thin wrapper around url.Parse that handles common edge cases. +func parseURL(rawURL string) (*url.URL, error) { + return url.Parse(rawURL) +} diff --git a/internal/tsclient/tsclient_test.go b/internal/tsclient/tsclient_test.go index e41503d..8c68b08 100644 --- a/internal/tsclient/tsclient_test.go +++ b/internal/tsclient/tsclient_test.go @@ -2,6 +2,8 @@ package tsclient import ( "encoding/json" + "errors" + "net/url" "testing" ) @@ -66,3 +68,271 @@ func TestFindPeer_Misses(t *testing.T) { t.Errorf("nil receiver should return nil, got %v", got) } } + +func TestResolvePeerIP(t *testing.T) { + cases := []struct { + name string + status *Status + hostname string + wantIP string + wantErr error + }{ + { + name: "online peer by hostname", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantIP: "100.124.19.34", + }, + { + name: "online peer by MagicDNS short name", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot-1", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantIP: "100.124.19.34", + }, + { + name: "online peer by full FQDN", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot-1", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot.tail-xxxx.ts.net.", + wantIP: "100.124.19.34", + }, + { + name: "online peer by FQDN without trailing dot", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot-1", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot.tail-xxxx.ts.net", + wantIP: "100.124.19.34", + }, + { + name: "duplicate hostname: prefer online over offline", + status: &Status{Peer: map[string]*PeerStatus{ + "stale": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.87.49.2"}, Online: false}, + "live": {HostName: "grok-bot-1", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantIP: "100.124.19.34", + }, + { + name: "duplicate hostname both share MagicDNS: prefer online", + status: &Status{Peer: map[string]*PeerStatus{ + "old": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.87.49.2"}, Online: false}, + "new": {HostName: "grok-bot", DNSName: "grok-bot-1.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantIP: "100.124.19.34", + }, + { + name: "all matching peers offline", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.87.49.2"}, Online: false}, + "b": {HostName: "grok-bot", DNSName: "grok-bot-2.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.87.49.3"}, Online: false}, + }}, + hostname: "grok-bot", + wantErr: ErrPeerOffline, + }, + { + name: "peer not found", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "alpha", DNSName: "alpha.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.80.229.80"}, Online: true}, + }}, + hostname: "grok-bot", + wantErr: ErrPeerNotFound, + }, + { + name: "nil status", + status: nil, + hostname: "grok-bot", + wantErr: ErrPeerNotFound, + }, + { + name: "peer online but no IPv4", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"fd7a:115c:a1e0::1"}, Online: true}, + }}, + hostname: "grok-bot", + wantErr: ErrPeerNoIPv4, + }, + { + name: "case insensitive match", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "Grok-Bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "GROK-BOT", + wantIP: "100.124.19.34", + }, + { + name: "peer with multiple IPs picks first IPv4", + status: &Status{Peer: map[string]*PeerStatus{ + "a": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"fd7a:115c:a1e0::1", "100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantIP: "100.124.19.34", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := tc.status.ResolvePeerIP(tc.hostname) + if tc.wantErr != nil { + if err == nil { + t.Fatalf("expected error %v, got nil (ip=%q)", tc.wantErr, got) + } + if !errors.Is(err, tc.wantErr) { + t.Errorf("error: got %v, want sentinel %v", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.wantIP { + t.Errorf("ip: got %q, want %q", got, tc.wantIP) + } + }) + } +} + +func TestIsIPAddress(t *testing.T) { + cases := map[string]bool{ + "100.80.229.80": true, + "192.168.1.1": true, + "127.0.0.1": true, + "0.0.0.0": true, + "::1": true, + "fd7a:115c:a1e0::1": true, + "grok-bot": false, + "grok-bot.tail-xxxx.ts.net": false, + "localhost": false, + "": false, + } + for in, want := range cases { + t.Run(in, func(t *testing.T) { + if got := isIPAddress(in); got != want { + t.Errorf("isIPAddress(%q) = %v, want %v", in, got, want) + } + }) + } +} + +func TestResolveSinkURL_IPPassthrough(t *testing.T) { + // When the URL already contains an IP, ResolveSinkURL should return it unchanged. + // This test doesn't need Tailscale CLI since IP URLs bypass resolution. + cases := []string{ + "http://100.80.229.80:9999/sync", + "http://127.0.0.1:9999/sync", + "http://192.168.1.1:8080/healthz", + } + for _, rawURL := range cases { + t.Run(rawURL, func(t *testing.T) { + // Even if Tailscale is not available, IP URLs should pass through + got, err := resolveSinkURLWithStatus(nil, rawURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != rawURL { + t.Errorf("got %q, want %q", got, rawURL) + } + }) + } +} + +func TestResolveSinkURL_HostnameResolution(t *testing.T) { + st := &Status{Peer: map[string]*PeerStatus{ + "live": {HostName: "grok-bot-1", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }} + + cases := []struct { + name string + rawURL string + wantURL string + }{ + { + name: "short hostname", + rawURL: "http://grok-bot:9999/sync", + wantURL: "http://100.124.19.34:9999/sync", + }, + { + name: "FQDN hostname", + rawURL: "http://grok-bot.tail-xxxx.ts.net:9999/sync", + wantURL: "http://100.124.19.34:9999/sync", + }, + { + name: "preserves path", + rawURL: "http://grok-bot:9999/healthz", + wantURL: "http://100.124.19.34:9999/healthz", + }, + { + name: "preserves query", + rawURL: "http://grok-bot:9999/sync?foo=bar", + wantURL: "http://100.124.19.34:9999/sync?foo=bar", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := resolveSinkURLWithStatus(st, tc.rawURL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tc.wantURL { + t.Errorf("got %q, want %q", got, tc.wantURL) + } + }) + } +} + +func TestResolveSinkURL_HostnameNotFound(t *testing.T) { + st := &Status{Peer: map[string]*PeerStatus{}} + rawURL := "http://unknown-host:9999/sync" + + got, err := resolveSinkURLWithStatus(st, rawURL) + if err == nil { + t.Fatal("expected error for unknown hostname") + } + if !errors.Is(err, ErrPeerNotFound) { + t.Errorf("error: got %v, want sentinel %v", err, ErrPeerNotFound) + } + // Should return original URL on error + if got != rawURL { + t.Errorf("on error, should return original URL: got %q, want %q", got, rawURL) + } +} + +// resolveSinkURLWithStatus is a test helper that resolves a sink URL using a +// pre-populated Status rather than calling the Tailscale CLI. +func resolveSinkURLWithStatus(st *Status, rawURL string) (string, error) { + parsed, err := url.Parse(rawURL) + if err != nil { + return rawURL, err + } + + host := parsed.Hostname() + port := parsed.Port() + + if isIPAddress(host) { + return rawURL, nil + } + + if st == nil { + return rawURL, nil + } + + resolvedIP, err := st.ResolvePeerIP(host) + if err != nil { + return rawURL, err + } + + newHost := resolvedIP + if port != "" { + newHost = resolvedIP + ":" + port + } + parsed.Host = newHost + + return parsed.String(), nil +} From d6711498766a025aa69774cb59656f75e8b1f725 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 23 Aug 2026 23:48:38 +0000 Subject: [PATCH 2/4] fix: increase SinkSync read/write timeout to 5 minutes The sink's /sync endpoint had a 60-second ReadTimeout while the source client had a 5-minute ClientTimeout. A first full sync with 16k+ cookies over a slow Tailscale link could not finish reading within 60s, causing a 400 'i/o timeout' error. Align SinkSync ReadTimeout and WriteTimeout with SyncClient's 5-minute timeout to accommodate large initial syncs. Co-authored-by: Matt Van Horn --- internal/cli/httpserver/httpserver.go | 4 ++-- internal/cli/httpserver/httpserver_test.go | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/cli/httpserver/httpserver.go b/internal/cli/httpserver/httpserver.go index 565817b..f8174b5 100644 --- a/internal/cli/httpserver/httpserver.go +++ b/internal/cli/httpserver/httpserver.go @@ -59,8 +59,8 @@ func Defaults(p Profile) Settings { case SinkSync: return Settings{ ReadHeaderTimeout: 5 * time.Second, - ReadTimeout: 60 * time.Second, - WriteTimeout: 60 * time.Second, + ReadTimeout: 5 * time.Minute, // Match SyncClient; first full sync (16k+ cookies) needs time over Tailscale + WriteTimeout: 5 * time.Minute, // Response can also be slow on congested links IdleTimeout: 120 * time.Second, MaxHeaderBytes: 16 * 1024, MaxBodyBytes: 256 * 1024 * 1024, diff --git a/internal/cli/httpserver/httpserver_test.go b/internal/cli/httpserver/httpserver_test.go index 70b2213..6700fda 100644 --- a/internal/cli/httpserver/httpserver_test.go +++ b/internal/cli/httpserver/httpserver_test.go @@ -44,11 +44,13 @@ func TestConfigure_AppliesTimeouts(t *testing.T) { if srv.ReadHeaderTimeout != 5*time.Second { t.Errorf("ReadHeaderTimeout: got %v want 5s", srv.ReadHeaderTimeout) } - if srv.ReadTimeout != 60*time.Second { - t.Errorf("ReadTimeout: got %v want 60s", srv.ReadTimeout) + // ReadTimeout and WriteTimeout are 5 minutes to match SyncClient and + // accommodate large first syncs (16k+ cookies) over slow Tailscale links. + if srv.ReadTimeout != 5*time.Minute { + t.Errorf("ReadTimeout: got %v want 5m", srv.ReadTimeout) } - if srv.WriteTimeout != 60*time.Second { - t.Errorf("WriteTimeout: got %v want 60s", srv.WriteTimeout) + if srv.WriteTimeout != 5*time.Minute { + t.Errorf("WriteTimeout: got %v want 5m", srv.WriteTimeout) } if srv.MaxHeaderBytes != 16*1024 { t.Errorf("MaxHeaderBytes: got %d want 16384", srv.MaxHeaderBytes) From 12713baf942b1a528537b224f69b01b74ecea24b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 07:12:48 +0000 Subject: [PATCH 3/4] fix: fail closed when multiple online peers share same hostname ResolvePeerIP now returns ErrAmbiguousPeer when multiple Online peers match the same hostname, rather than nondeterministically picking the first one from map iteration. The error message includes all matching IPs so the operator can either: - Pin sink.url to a specific 100.x IP - Delete the leftover node from Tailscale admin console The offline+online duplicate hostname case still correctly picks the single online peer. Co-authored-by: Matt Van Horn --- internal/tsclient/tsclient.go | 41 +++++++++++++++++++++++++----- internal/tsclient/tsclient_test.go | 9 +++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/internal/tsclient/tsclient.go b/internal/tsclient/tsclient.go index cd67a4a..9b3bbbf 100644 --- a/internal/tsclient/tsclient.go +++ b/internal/tsclient/tsclient.go @@ -138,6 +138,11 @@ var ErrPeerOffline = errors.New("tsclient: peer is offline") // ErrPeerNoIPv4 is returned when a peer has no IPv4 address in TailscaleIPs. var ErrPeerNoIPv4 = errors.New("tsclient: peer has no IPv4 address") +// ErrAmbiguousPeer is returned when multiple Online peers share the same +// hostname. The caller should pin sink.url to a specific 100.x IP or delete +// the leftover node from the Tailscale admin console. +var ErrAmbiguousPeer = errors.New("tsclient: multiple online peers match hostname") + // ResolvePeerIP resolves a Tailscale hostname to its 100.x IPv4 address. // When multiple peers share the same hostname (a common scenario after // Tailscale re-auth creates a new node while the old offline node lingers), @@ -149,7 +154,9 @@ var ErrPeerNoIPv4 = errors.New("tsclient: peer has no IPv4 address") // - HostName field value: "grok-bot-1" // // Returns ErrPeerNotFound if no peer matches, ErrPeerOffline if all matches -// are offline, or ErrPeerNoIPv4 if the peer lacks an IPv4 address. +// are offline, ErrAmbiguousPeer if multiple Online peers match (caller should +// pin sink.url to a 100.x IP or delete the leftover node), or ErrPeerNoIPv4 +// if the peer lacks an IPv4 address. func (s *Status) ResolvePeerIP(hostname string) (string, error) { if s == nil { return "", ErrPeerNotFound @@ -186,19 +193,39 @@ func (s *Status) ResolvePeerIP(hostname string) (string, error) { return "", fmt.Errorf("%w: %q", ErrPeerNotFound, hostname) } - // Prefer Online peers. If multiple are online, take the first one. - // If none are online, return ErrPeerOffline. - var best *PeerStatus + // Collect Online peers. If exactly one is online, use it. If multiple + // are online, fail closed (nondeterministic map iteration order would + // pick arbitrarily). If none are online, return ErrPeerOffline. + var onlinePeers []*PeerStatus for _, p := range matches { if p.Online { - best = p - break + onlinePeers = append(onlinePeers, p) } } - if best == nil { + + if len(onlinePeers) == 0 { return "", fmt.Errorf("%w: %q (all %d matching nodes are offline; check `tailscale status`)", ErrPeerOffline, hostname, len(matches)) } + if len(onlinePeers) > 1 { + // Multiple online peers with the same hostname. Collect their IPs + // for the error message so the operator can pick one. + var ips []string + for _, p := range onlinePeers { + for _, ip := range p.TailscaleIPs { + if IsTailnetIP(ip) { + ips = append(ips, ip) + break + } + } + } + return "", fmt.Errorf("%w: %q has %d online nodes with IPs %v; pin sink.url to one 100.x IP or delete the leftover node from Tailscale admin", + ErrAmbiguousPeer, hostname, len(onlinePeers), ips) + } + + // Exactly one online peer - use it + best := onlinePeers[0] + // Extract IPv4 from TailscaleIPs for _, ip := range best.TailscaleIPs { if IsTailnetIP(ip) { diff --git a/internal/tsclient/tsclient_test.go b/internal/tsclient/tsclient_test.go index 8c68b08..a76ead6 100644 --- a/internal/tsclient/tsclient_test.go +++ b/internal/tsclient/tsclient_test.go @@ -136,6 +136,15 @@ func TestResolvePeerIP(t *testing.T) { hostname: "grok-bot", wantErr: ErrPeerOffline, }, + { + name: "ambiguous: multiple online peers same hostname", + status: &Status{Peer: map[string]*PeerStatus{ + "node1": {HostName: "grok-bot", DNSName: "grok-bot.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.87.49.2"}, Online: true}, + "node2": {HostName: "grok-bot", DNSName: "grok-bot-2.tail-xxxx.ts.net.", TailscaleIPs: []string{"100.124.19.34"}, Online: true}, + }}, + hostname: "grok-bot", + wantErr: ErrAmbiguousPeer, + }, { name: "peer not found", status: &Status{Peer: map[string]*PeerStatus{ From 4429d14d344e7e1761405afb3cc97855da1d9269 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 07:57:02 +0000 Subject: [PATCH 4/4] fix: abort push on ErrAmbiguousPeer instead of falling back to hostname When ResolveSinkURL returns ErrAmbiguousPeer (multiple online peers share the hostname), the push now fails closed instead of falling back to the original hostname URL. Falling back would hand peer selection to MagicDNS/system DNS and undo the fail-closed behavior. Soft failures (Tailscale CLI missing, peer not found, peer offline) still fall back to the original URL so HTTP can report the connection error. Added tests: - TestSourcePushAmbiguousPeerAbortsWithoutPOST: verifies ErrAmbiguousPeer aborts and sends no HTTP request - TestSourcePushSoftResolveErrorFallsBackToHostname: verifies soft failures still POST with the original URL Co-authored-by: Matt Van Horn --- internal/cli/source.go | 27 +++++++++++++-- internal/cli/source_test.go | 67 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/internal/cli/source.go b/internal/cli/source.go index 0ab621a..e3dcbec 100644 --- a/internal/cli/source.go +++ b/internal/cli/source.go @@ -34,6 +34,19 @@ var ( sourceSkipDBSC bool ) +// resolveSinkURL is the sink URL resolver used by pushOnce. Production +// wires it to tsclient.ResolveSinkURL; tests can override it to inject +// specific resolution behaviors (e.g., ErrAmbiguousPeer). +var resolveSinkURL = tsclient.ResolveSinkURL + +// SetResolveSinkURLForTesting replaces resolveSinkURL with the given +// function and returns a restore func. Test-only seam. +func SetResolveSinkURLForTesting(f func(ctx context.Context, rawURL string) (string, error)) func() { + prev := resolveSinkURL + resolveSinkURL = f + return func() { resolveSinkURL = prev } +} + // dbscSummary carries the DBSC-suspect tally from one push back to the caller // so it can be recorded in SourceState for `doctor` / `status`. type dbscSummary struct { @@ -382,10 +395,18 @@ func pushOnce( // Resolve sink URL hostname to IP via Tailscale if needed. This allows // sink.url to use MagicDNS hostnames (e.g., http://grok-bot:9999/sync) // instead of frozen 100.x IPs that break after Tailscale re-auth. - // If resolution fails (Tailscale not available, peer offline), fall back - // to the original URL and let the HTTP layer report the connection error. + // + // Fail-closed errors (ErrAmbiguousPeer) abort the push — falling back to + // the hostname URL would hand selection to MagicDNS and undo fail-closed. + // Soft failures (Tailscale CLI missing, peer not found, peer offline) fall + // back to the original URL so HTTP can report the connection error. sinkURL := cfg.Sink.URL - if resolved, resolveErr := tsclient.ResolveSinkURL(ctx, sinkURL); resolveErr != nil { + if resolved, resolveErr := resolveSinkURL(ctx, sinkURL); resolveErr != nil { + if errors.Is(resolveErr, tsclient.ErrAmbiguousPeer) { + return 0, dbsc, fmt.Errorf("resolve sink URL: %w", resolveErr) + } + // Soft failure: Tailscale not available, peer offline, etc. + // Fall back to the original URL and let HTTP report the error. if verbose { fmt.Fprintf(os.Stderr, "agentcookie source: sink URL resolution failed (%v); using original %s\n", resolveErr, sinkURL) } diff --git a/internal/cli/source_test.go b/internal/cli/source_test.go index 86420a3..70d95fa 100644 --- a/internal/cli/source_test.go +++ b/internal/cli/source_test.go @@ -5,6 +5,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -12,6 +13,7 @@ import ( "path/filepath" "reflect" "sort" + "strings" "sync" "testing" @@ -22,6 +24,7 @@ import ( "github.com/mvanhorn/agentcookie/internal/protocol" "github.com/mvanhorn/agentcookie/internal/state" "github.com/mvanhorn/agentcookie/internal/transport" + "github.com/mvanhorn/agentcookie/internal/tsclient" ) func TestSourcePushReloadsBlocklistBetweenPushes(t *testing.T) { @@ -359,3 +362,67 @@ CREATE UNIQUE INDEX IF NOT EXISTS cookies_unique_index ON cookies( host_key, top_frame_site_key, has_cross_site_ancestor, name, path, source_scheme, source_port ); ` + +// TestSourcePushAmbiguousPeerAbortsWithoutPOST verifies that when ResolveSinkURL +// returns ErrAmbiguousPeer (multiple online peers share the hostname), the push +// fails closed and does NOT fall back to the hostname URL. Falling back would +// hand selection to MagicDNS and undo the fail-closed behavior. +func TestSourcePushAmbiguousPeerAbortsWithoutPOST(t *testing.T) { + fx := newSourcePushFixture(t, []chrome.Cookie{ + {HostKey: ".example.com", Name: "session", Value: "xyz", Path: "/"}, + }) + + // Override the resolver to return ErrAmbiguousPeer. + restore := SetResolveSinkURLForTesting(func(ctx context.Context, rawURL string) (string, error) { + return rawURL, fmt.Errorf("%w: \"grok-bot\" has 2 online nodes with IPs [100.87.49.2 100.124.19.34]; pin sink.url to one 100.x IP or delete the leftover node from Tailscale admin", + tsclient.ErrAmbiguousPeer) + }) + defer restore() + + n, err := fx.push() + if err == nil { + t.Fatal("ambiguous peer should fail closed, got nil error") + } + if !errors.Is(err, tsclient.ErrAmbiguousPeer) { + t.Errorf("error should wrap ErrAmbiguousPeer, got: %v", err) + } + if !strings.Contains(err.Error(), "resolve sink URL") { + t.Errorf("error should mention 'resolve sink URL', got: %v", err) + } + if n != 0 { + t.Errorf("ambiguous peer push count = %d, want 0", n) + } + if got := fx.batchCount(); got != 0 { + t.Fatalf("ambiguous peer should NOT send any HTTP request, got %d batches", got) + } + if fx.srcState.TotalFailures != 1 { + t.Errorf("TotalFailures = %d, want 1", fx.srcState.TotalFailures) + } +} + +// TestSourcePushSoftResolveErrorFallsBackToHostname verifies that soft failures +// (Tailscale CLI missing, peer not found, peer offline) fall back to the original +// URL and proceed with the POST, letting HTTP report the connection error. +func TestSourcePushSoftResolveErrorFallsBackToHostname(t *testing.T) { + fx := newSourcePushFixture(t, []chrome.Cookie{ + {HostKey: ".example.com", Name: "session", Value: "xyz", Path: "/"}, + }) + + // Override the resolver to return ErrPeerNotFound (a soft failure). + restore := SetResolveSinkURLForTesting(func(ctx context.Context, rawURL string) (string, error) { + return rawURL, fmt.Errorf("%w: \"unknown-host\"", tsclient.ErrPeerNotFound) + }) + defer restore() + + // Push should succeed (fall back to original URL, HTTP capture accepts it). + n, err := fx.push() + if err != nil { + t.Fatalf("soft resolve error should fall back to original URL, got: %v", err) + } + if n != 1 { + t.Errorf("push count = %d, want 1", n) + } + if got := fx.batchCount(); got != 1 { + t.Fatalf("soft resolve error should still POST, got %d batches", got) + } +}