From e34fef31673744ccf1a3887c97078a1cac554847 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le <cuong@windscribe.com>
Date: Wed, 23 Sep 2026 15:31:38 +0700
Subject: [PATCH] fix(cli): bound the DNS loop check probe by upstream timeout

checkDnsLoop resolved with context.Background() and no dns.Client
timeout, so the DNS client fell back to its own 2s default and the
configured upstream timeout was ignored. The probes run serially on a
one-minute ticker, so each unreachable local upstream stalled the loop
for a fixed 2s no matter what the config asked for.

This is most visible on Windows, where UDP to a closed loopback port
does not surface an immediate refusal the way it does on Linux, so the
probe waits out the full default.

Apply the upstream timeout the way checkUpstreamOnce already does, and
keep 2s for an upstream that configures none so the unconfigured case
is unchanged.

The probe context deliberately carries no logger. A resolver logs its
own failure at error level with the endpoint in the message, which
would place an Internal Domain resolver address in the retained
journal; logUpstreamProbeFailure remains the reporter and bounds what
the line may hold. TestInternalDomainsLoopCheckHidesResolverAddress
covers this.
---
 cmd/cli/loop.go | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/cmd/cli/loop.go b/cmd/cli/loop.go
index b4adb8f..0580be9 100644
--- a/cmd/cli/loop.go
+++ b/cmd/cli/loop.go
@@ -14,6 +14,11 @@ import (
 const (
 	loopTestDomain = ".test"
 	loopTestQtype  = dns.TypeTXT
+
+	// defaultLoopCheckTimeout bounds one loop probe when the upstream sets no
+	// timeout. It matches the DNS client's own default, so the unconfigured
+	// case keeps its previous behavior.
+	defaultLoopCheckTimeout = 2 * time.Second
 )
 
 // newLoopGuard returns new loopGuard.
@@ -118,13 +123,34 @@ func (p *prog) checkDnsLoop() {
 			logUpstreamProbeFailure(reference[uid], uc, err, mainLog.Load().Warn, "could not perform loop check")
 			continue
 		}
-		if _, err := resolver.Resolve(context.Background(), msg); err != nil {
+		// Bound the probe, like checkUpstreamOnce does. Without a deadline the
+		// DNS client falls back to its own default, so one unreachable local
+		// upstream stalls this serial loop far past the configured timeout.
+		timeout := defaultLoopCheckTimeout
+		if uc.Timeout > 0 {
+			timeout = time.Millisecond * time.Duration(uc.Timeout)
+		}
+		if err := resolveLoopTestMsg(resolver, msg, timeout); err != nil {
 			logUpstreamProbeFailure(reference[uid], uc, err, mainLog.Load().Warn, "could not send DNS loop check query")
 		}
 	}
 	mainLog.Load().Debug().Msg("end checking DNS loop")
 }
 
+// resolveLoopTestMsg sends one loop test query under its own deadline. The
+// cancel runs before the next upstream, so a long loop leaks no contexts.
+//
+// The context deliberately carries no logger. A resolver logs its own failure
+// at error level with the endpoint in the message, which would put an Internal
+// Domain resolver address in the retained journal; logUpstreamProbeFailure is
+// what reports this failure, and it bounds what the line may hold.
+func resolveLoopTestMsg(resolver ctrld.Resolver, msg *dns.Msg, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	_, err := resolver.Resolve(ctx, msg)
+	return err
+}
+
 // checkDnsLoopTicker performs p.checkDnsLoop every minute.
 func (p *prog) checkDnsLoopTicker(ctx context.Context) {
 	timer := time.NewTicker(time.Minute)