Harden note and owner isolation, public fetchers, and IRC resource bounds

bot/bot.go

@@ -208,7 +208,13 @@ func (b *Bot) sendNow(target, text string) {
 	c := b.client
 	b.mu.RUnlock()
 	if c != nil {
-		c.WriteMessage(&irc.Message{Command: "PRIVMSG", Params: []string{target, text}})
+		if err := c.WriteMessage(&irc.Message{Command: "PRIVMSG", Params: []string{target, text}}); err != nil {
+			b.recordDroppedOutput()
+			if b.Log != nil {
+				b.Log.Warn("IRC message write failed")
+			}
+			return
+		}
 		b.Stats.sent.Add(1)
 		if b.networkStats != nil {
 			b.networkStats.sent.Add(1)
@@ -260,14 +266,15 @@ func (b *Bot) connect(ctx context.Context) error {
 		}
 	}
 	var conn net.Conn
+	dialer := &net.Dialer{Timeout: 15 * time.Second, KeepAlive: 30 * time.Second}
 	if b.Config.Server.TLS {
-		tlsConfig := &tls.Config{ServerName: b.Config.Server.Host, InsecureSkipVerify: !b.Config.Server.VerifyCert}
+		tlsConfig := &tls.Config{ServerName: b.Config.Server.Host, MinVersion: tls.VersionTLS12, InsecureSkipVerify: !b.Config.Server.VerifyCert}
 		if b.Config.Server.ClientCert != "" {
 			tlsConfig.Certificates = []tls.Certificate{clientCertificate}
 		}
-		conn, err = tls.Dial("tcp", server, tlsConfig)
+		conn, err = (&tls.Dialer{NetDialer: dialer, Config: tlsConfig}).DialContext(ctx, "tcp", server)
 	} else {
-		conn, err = net.Dial("tcp", server)
+		conn, err = dialer.DialContext(ctx, "tcp", server)
 	}
 	if err != nil {
 		return err
@@ -535,7 +542,29 @@ func (b *Bot) IsOwner(m Message) bool {
 		return false
 	}
 	for _, owner := range b.Config.OwnerAccounts {
-		if strings.EqualFold(strings.TrimSpace(owner), account) {
+		owner = strings.TrimSpace(owner)
+		if network, name, qualified := strings.Cut(owner, "/"); qualified {
+			network, name = strings.TrimSpace(network), strings.TrimSpace(name)
+			if network != "" && name != "" && strings.EqualFold(network, strings.TrimSpace(b.Config.NetworkName)) && strings.EqualFold(name, account) {
+				return true
+			}
+			continue
+		}
+		// Bare legacy account names grant access only on the first configured
+		// network. A same-named account elsewhere is a different principal.
+		primary := true
+		if len(b.Config.Networks) > 1 {
+			first := b.Config.Networks[0]
+			name := strings.TrimSpace(first.Name)
+			if name == "" {
+				name = strings.TrimSpace(first.Server.Host)
+			}
+			if name == "" {
+				name = "network-1"
+			}
+			primary = strings.EqualFold(name, strings.TrimSpace(b.Config.NetworkName))
+		}
+		if primary && strings.EqualFold(owner, account) {
 			return true
 		}
 	}
@@ -570,6 +599,11 @@ func (b *Bot) AllowCommand(m Message) bool {
 	if exists && now.Sub(last) < cooldown {
 		lastWarning, warned := b.lastWarnings[key]
 		if !warned || now.Sub(lastWarning) >= warningCooldown {
+			pruneCommandHistory(b.lastWarnings, now, warningCooldown)
+			if _, known := b.lastWarnings[key]; !known && len(b.lastWarnings) >= maxCommandIdentities {
+				b.commandMu.Unlock()
+				return false
+			}
 			b.lastWarnings[key] = now
 			b.commandMu.Unlock()
 			b.Send(m.ReplyTarget(), "command cooldown—please wait a moment")
@@ -578,11 +612,31 @@ func (b *Bot) AllowCommand(m Message) bool {
 		b.commandMu.Unlock()
 		return false
 	}
+	if !exists {
+		pruneCommandHistory(b.lastCommands, now, cooldown)
+		if len(b.lastCommands) >= maxCommandIdentities {
+			b.commandMu.Unlock()
+			return false
+		}
+	}
 	b.lastCommands[key] = now
 	b.commandMu.Unlock()
 	return true
 }
 
+const maxCommandIdentities = 10000
+
+func pruneCommandHistory(history map[string]time.Time, now time.Time, lifetime time.Duration) {
+	if len(history) < maxCommandIdentities {
+		return
+	}
+	for key, last := range history {
+		if now.Sub(last) >= lifetime {
+			delete(history, key)
+		}
+	}
+}
+
 func commandBypassesCooldown(m Message, prefix string) bool {
 	cmd, arg, ok := IsCommand(m, prefix)
 	if !ok {

bot/security_test.go

@@ -0,0 +1,89 @@
+package bot
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	"testing"
+	"time"
+
+	"gopkg.in/irc.v3"
+)
+
+func TestOwnerPrivilegesAreNetworkScoped(t *testing.T) {
+	cfg := Config{Networks: []NetworkConfig{{Name: "alpha"}, {Name: "beta"}}, OwnerAccounts: []string{"alice", "beta/bob"}}
+	for _, test := range []struct {
+		network, account string
+		want             bool
+	}{
+		{"alpha", "ALICE", true}, {"beta", "alice", false}, {"beta", "bob", true}, {"alpha", "bob", false}, {"beta", "*", false},
+	} {
+		cfg.NetworkName = test.network
+		b := &Bot{Config: cfg}
+		if got := b.IsOwner(Message{Account: test.account, Nick: "alice"}); got != test.want {
+			t.Errorf("owner check on %s/%s = %t, want %t", test.network, test.account, got, test.want)
+		}
+	}
+	cfg.NetworkName = "beta"
+	called := false
+	b := &Bot{Config: cfg, reloadHandler: func(Message) { called = true }}
+	if b.handlePrivateReload(Message{Command: "PRIVMSG", Account: "alice", Text: "reload"}) || called {
+		t.Fatal("same-named account on a secondary network triggered owner reload")
+	}
+	cfg.Networks[0] = NetworkConfig{Server: ServerConfig{Host: "irc.example"}}
+	cfg.NetworkName = "irc.example"
+	if !(&Bot{Config: cfg}).IsOwner(Message{Account: "alice"}) {
+		t.Fatal("unnamed primary network lost its legacy owner")
+	}
+}
+
+func TestCommandIdentityHistoriesAreBoundedAndExpire(t *testing.T) {
+	b := &Bot{lastCommands: make(map[string]time.Time), lastWarnings: make(map[string]time.Time)}
+	now := time.Now()
+	for i := 0; i < maxCommandIdentities; i++ {
+		b.lastCommands[fmt.Sprint(i)] = now
+	}
+	message := Message{Command: "PRIVMSG", Nick: "new-user", Text: "!status"}
+	b.Config.CommandPrefix = "!"
+	if b.AllowCommand(message) || len(b.lastCommands) > maxCommandIdentities {
+		t.Fatal("new identities bypassed the command-history bound")
+	}
+	for key := range b.lastCommands {
+		b.lastCommands[key] = now.Add(-time.Hour)
+	}
+	if !b.AllowCommand(message) || len(b.lastCommands) != 1 {
+		t.Fatal("expired command identities did not free capacity")
+	}
+	for i := 0; i < maxCommandIdentities; i++ {
+		b.lastWarnings[fmt.Sprint(i)] = now
+	}
+	if b.AllowCommand(message) || len(b.lastWarnings) > maxCommandIdentities {
+		t.Fatal("warning history exceeded its bound")
+	}
+}
+
+func TestFailedIRCWriteCountsAsDroppedNotSent(t *testing.T) {
+	left, right := net.Pipe()
+	defer left.Close()
+	right.Close()
+	stats := NewStats()
+	b := &Bot{Stats: stats, client: irc.NewClient(left, irc.ClientConfig{})}
+	b.networkStats = stats.registerNetwork("test", 0, nil)
+	b.sendNow("#test", "synthetic test message")
+	if stats.sent.Load() != 0 || stats.dropped.Load() != 1 || b.networkStats.sent.Load() != 0 || b.networkStats.dropped.Load() != 1 {
+		t.Fatal("failed IRC write was counted as a successful send")
+	}
+}
+
+func TestIRCConnectHonorsCancellation(t *testing.T) {
+	for _, useTLS := range []bool{false, true} {
+		ctx, cancel := context.WithCancel(t.Context())
+		cancel()
+		b := &Bot{Config: Config{Server: ServerConfig{Host: "example.invalid", Port: 6697, TLS: useTLS, VerifyCert: true}}}
+		started := time.Now()
+		if err := b.connect(ctx); !errors.Is(err, context.Canceled) || time.Since(started) > time.Second {
+			t.Fatalf("IRC connection ignored cancellation: %v", err)
+		}
+	}
+}

docs/configuration.md

@@ -288,12 +288,19 @@ alone are never treated as proof of ownership:
 ```yaml
 owner_accounts:
   - "your-account"
-  - "another-owner"
+  - "secondary-network/another-owner"
 ```
 
 GoBot records the IRCv3 `account` tag when the network provides it. There is
 intentionally no ownership-claim command.
 
+Account names are local to their IRC network. With multiple networks, bare
+legacy names such as `your-account` grant owner access only on the **first**
+configured network. Use an explicit `network-name/account-name` entry for each
+additional grant. A same-named account on another network is not an owner by
+default. Single-network bare-account configurations retain their existing
+behavior. Owner configuration changes require a restart.
+
 ### Owner-only private reload
 
 An owner can reload file-backed plugin settings without disconnecting GoBot:

docs/plugins.md

@@ -341,15 +341,28 @@ The `note` plugin stores small, user-invoked notes by authenticated IRC account:
 !note clear
 ~~~
 
-`!notes` is an alias for `!note`. Notes are not announced automatically and are
-stored separately for each user. A note response sent in a channel is visible
-there, so use a private message when the note is sensitive. When account tags
-are unavailable, GoBot falls back to the network and nickname. Notes are limited
+`!notes` is an alias for `!note`. Send all note commands in a private message
+to GoBot while identified to an IRC account. The network must provide account
+tags; unverified nicknames cannot read or modify notes. Channel commands only
+receive a private usage notice and do not read, write, or expose notes.
+Records are scoped to both network and account, so a same-named account on
+another network cannot claim them. Legacy unscoped/nickname records remain in
+the old `notes` bucket, untouched and inaccessible to commands; new records use
+`notes_v2`. An operator must verify legacy ownership before any offline recovery;
+there is deliberately no automatic ownership claim or destructive migration.
+Notes are limited
 to 50 per user by default, with 400 characters per note, and unused notes expire
 after 180 days. Operators can change these limits with `plugins.note.max_notes`,
 `plugins.note.max_note_length`, and `plugins.note.expiry_days`; set
 `expiry_days: 0` to disable expiry.
 
+User-controlled HTTP fetchers (`urltitle`, paste-source fetches, Ask excerpts,
+and website status checks) share a strict public-IP policy. Shared/CGNAT,
+benchmark, documentation, reserved, local, and transition addresses are not
+valid targets. Redirects and response headers are bounded, proxies cannot
+bypass destination validation, and URL previews use at most four background
+workers across networks. URL-preview logs omit query strings and paths.
+
 ## Join greetings
 
 The optional `welcome` plugin posts a short, playful line when another user
@@ -1417,6 +1430,12 @@ with an existing snippet gets at most 600 ms to refresh from its origin before
 returning the attributed snippet. Sources without snippets retain their normal
 bounded fetch budget.
 
+For explicit `X vs Y` / `X versus Y` comparisons, a search-result fallback must
+mention both distinct subjects; a fast one-sided result cannot preempt an
+in-flight Assist answer. If an origin refresh loses that context, a complete
+attributed snippet is retained instead. This lexical safeguard applies to
+search excerpts, not to generated Assist paraphrases.
+
 Lightweight attempts still retain the 300 ms normalized retry and a final
 original-question recovery request after 500 ms. Their original budget reserve
 is confined to that lane; it no longer delays independent providers. All waits

plugins/ask.go

@@ -1098,6 +1098,9 @@ func selectAskSearchResult(question string, results []askRenderedSearchResult) (
 		if result.Title == "" || !validPublicHTTPURL(result.URL) {
 			continue
 		}
+		if !askComparisonRelevant(question, result.Title+" "+result.Snippet) {
+			continue
+		}
 		parsedURL, _ := url.Parse(result.URL)
 		if parsedURL == nil || isDuckDuckGoHost(parsedURL.Hostname()) {
 			continue
@@ -1309,12 +1312,15 @@ func fetchAskSearchResult(ctx context.Context, result askRenderedSearchResult, q
 			body, readErr := io.ReadAll(io.LimitReader(res.Body, 512<<10))
 			if readErr == nil {
 				if excerpt := extractAskHTMLExcerpt(body, result.Snippet, result.Title, strings.Fields(askFocusedTerm(question))); excerpt != "" {
-					return askSource{Title: result.Title, Summary: formatAskSearchResultSummary(result.Title, excerpt), URL: result.URL, Provider: "search_result"}, true
+					summary := formatAskSearchResultSummary(result.Title, excerpt)
+					if askComparisonRelevant(question, summary) {
+						return askSource{Title: result.Title, Summary: summary, URL: result.URL, Provider: "search_result"}, true
+					}
 				}
 			}
 		}
 	}
-	if result.Snippet != "" {
+	if result.Snippet != "" && askComparisonRelevant(question, result.Title+" "+result.Snippet) {
 		return askSource{Title: result.Title, Summary: formatAskSearchResultSummary(result.Title, result.Snippet), URL: result.URL, Provider: "search_result"}, true
 	}
 	return askSource{}, false
@@ -1979,11 +1985,13 @@ func isDuckDuckGoHost(host string) bool {
 }
 
 func isPublicIP(address netip.Addr) bool {
-	return address.IsGlobalUnicast() && !address.IsPrivate() && !address.IsLoopback() && !address.IsLinkLocalUnicast() && !address.IsLinkLocalMulticast() && !address.IsMulticast() && !address.IsUnspecified()
+	return publicInternetIP(address)
 }
 
 func newAskWebHTTPClient() *http.Client {
 	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.Proxy = nil
+	transport.MaxResponseHeaderBytes = 64 << 10
 	transport.DialContext = askPublicDialContext
 	return &http.Client{
 		Timeout:   6 * time.Second,
@@ -2016,11 +2024,15 @@ func askPublicDialContext(ctx context.Context, network, address string) (net.Con
 		return nil, err
 	}
 	dialer := &net.Dialer{Timeout: 3 * time.Second}
+	// Reject mixed public/private answers before dialing any member.
 	for _, ip := range ips {
 		parsed, parseErr := netip.ParseAddr(ip.String())
 		if parseErr != nil || !isPublicIP(parsed) {
-			continue
+			return nil, fmt.Errorf("refusing non-public address")
 		}
+	}
+	for _, ip := range ips {
+		parsed, _ := netip.ParseAddr(ip.String())
 		conn, dialErr := dialer.DialContext(ctx, network, net.JoinHostPort(parsed.String(), port))
 		if dialErr == nil {
 			return conn, nil

plugins/ask_relevance.go

@@ -0,0 +1,62 @@
+package plugins
+
+import (
+	"strings"
+	"unicode"
+)
+
+// A search excerpt about only one side must not win a comparison query over
+// an in-flight Assist answer. This is a conservative lexical check for weak
+// search-result fallbacks, not a filter on generated Assist paraphrases.
+func askComparisonRelevant(question, excerpt string) bool {
+	fields := strings.Fields(strings.ToLower(question))
+	for i, field := range fields {
+		if token := strings.Trim(field, ".,?!:;"); token != "vs" && token != "versus" {
+			continue
+		}
+		left := askComparisonWords(strings.Join(fields[:i], " "))
+		right := askComparisonWords(strings.Join(fields[i+1:], " "))
+		for word := range left {
+			if _, shared := right[word]; shared {
+				delete(left, word)
+				delete(right, word)
+			}
+		}
+		if len(left) == 0 || len(right) == 0 {
+			return true // Too little subject information to reject a source.
+		}
+		compact := strings.Map(func(r rune) rune {
+			if unicode.IsLetter(r) || unicode.IsDigit(r) {
+				return unicode.ToLower(r)
+			}
+			return -1
+		}, excerpt)
+		matches := func(words map[string]struct{}) bool {
+			for word := range words {
+				if strings.Contains(compact, word) {
+					return true
+				}
+			}
+			return false
+		}
+		return matches(left) && matches(right)
+	}
+	return true
+}
+
+func askComparisonWords(text string) map[string]struct{} {
+	words := make(map[string]struct{})
+	for _, word := range strings.FieldsFunc(text, func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsDigit(r) }) {
+		switch word {
+		case "how", "much", "do", "does", "what", "which", "is", "are", "the", "a", "an", "of", "to", "in", "for", "at", "cost", "costs", "price", "prices", "compare", "comparison", "between", "new", "used":
+			continue
+		}
+		if len(word) > 3 {
+			word = strings.TrimSuffix(word, "s")
+		}
+		if len(word) >= 2 {
+			words[word] = struct{}{}
+		}
+	}
+	return words
+}