Commit Graph
418 Commits
Author SHA1 Message Date
Dev Scribe 911cc08467 feat(cli): route Tailscale Split DNS and App Connector domains in macOS intercept mode 2026-10-20 17:23:47 +07:00
Cuong Manh Le 5f86f4750c test(cli): bind the explicit intercept listener test to its own ports
TestCustomListener5353Binding bound 127.0.0.x:5353, the mDNS port, so
it failed on any Linux host running an mDNS responder such as avahi,
which holds *:5353, before the code under test ran. Nothing in the
listener code treats 5353 specially.

Use ports the test holds itself, and rename the test and its file after
what they cover: an explicit intercept listener binds unchanged, and an
occupied one, by UDP, TCP or both, fails instead of falling back.
2026-10-20 17:23:47 +07:00
Cuong Manh Le bf7c264464 cli: default to dns intercept mode for new desktop installations
When no intercept mode is selected by flag or config, a new
installation on macOS or a Windows desktop edition now uses "dns".
Windows Server, iOS and other platforms keep interface DNS mode unless
intercept is selected explicitly, and so does a config that cannot run
in intercept mode.

Existing Standard installations are not switched on upgrade. ctrld
start marks the services it installs fresh, and the service applies
the default below service.intercept_mode against the config it
fetches, so a service with neither a mode nor the marker predates the
default and keeps interface DNS mode, whichever path upgrades it; the
macOS package also pins such a service to "off" explicitly. An
explicit "off" stays final and is persisted.
2026-10-20 17:23:47 +07:00
Cuong Manh Le 0aca0984ed test(cli): keep the indefinite recovery wait test off the host's DNS
TestNetworkShutdownCancelsIndefiniteRecoveryWait relied on an empty
upstream map never recovering. Since 73db7adc an empty pool checks
upstream.os instead, so the test probed the host's real OS resolver.
On a host with working DNS that check succeeds, recovery completes, and
it reads the upstream monitor the test never built.

Recover against a loopback port nothing listens on, so the wait can
never end on its own, and give the prog the upstream monitor that run
always creates before network monitoring can start a recovery. The
panic is test-only: production never reaches recovery without one.
2026-10-20 17:23:47 +07:00
Dev Scribe 78789b11bf fix(cli): accept IPv4 link-local as no usable IPv4 on IPv6-only networks
When DHCPv4 gives up, macOS self-assigns a 169.254.0.0/16 address on the
service. On the ctrld-qa macOS 26 guest (v6-only personality, IPv6 port 53
blocked so no CLAT) it appears about 22-26 s after joining: en0 holds
169.254.x.x and State:/Network/Service/<id>/IPv4 holds only that address,
with no Router and no CLAT46, while Global/IPv4 stays absent. The 15 s
IPv6-only confirmation could finish after that point and then never admit.

The IPv6-only evidence now treats a link-local-only IPv4 configuration of
the same device as no usable IPv4. Any router, CLAT, primary, routable
address, other device or unknown field still rejects. It only appears after
DHCPv4 has given up, so it does not reopen the dual-stack window.

Tests: reader cases for the captured link-local state and its unsafe
variants; the harness now keeps the scheduled confirmation and runs it to
cover admission, closed network activity and stopped intercept.
2026-10-20 17:23:47 +07:00
Alex Paguis 83ceee82fb fix(cli): stop the control server when the service stop is accepted 2026-10-20 17:23:47 +07:00
Dev Scribe 6673276c93 Send full host metadata on managed startup 2026-10-20 17:23:47 +07:00
Dev Scribe aea147fbb9 fix(cli): confirm IPv6-only evidence before setting the Intercept target
"No IPv4 yet" is also the state of a dual-stack network whose router
advertisement arrived before the DHCPv4 ACK. In that window the DHCP
read fails, configd has no IPv4 state, and the interface holds only
SLAAC IPv6, so a single IPv6-only observation could write 127.0.0.53 on
a service that provides IPv4 DNS seconds later.

The IPv6-only evidence must now hold on the same service and device for
15 seconds before it admits a target. The first observation starts the
clock and schedules one confirming reconcile. Any pass that reads the
evidence and finds a readable lease, IPv4 state, CLAT, or a different
service ends the confirmation without a write. Shutdown drops a pending
confirmation. CLAT evidence still admits immediately.

Review follow-ups: the service-name and networksetup mapping tail is a
shared reader helper, the IPv6 interface check uses an escaped tab, and
nativeIPv6OnlyAddresses has its own table test.
2026-10-20 17:23:47 +07:00
Cuong Manh Le 51411de9db test(cli): hold the intercept upgrade check line itself to no secrets
The test for #632 covered redactedArgs, but not the debug line that
uses it: putting osArgsEarly back into that Msgf would have leaked the
provision code again with every test passing.

Build the line in interceptUpgradeCheckLine and test it directly, for
--cd-org and --cd in both flag forms. Reverting the line to the raw
arguments fails the test.
2026-10-20 17:23:47 +07:00
Dev Scribe 50caf516bd fix(cli): set the Intercept DNS target on IPv6-only networks before CLAT starts
On macOS in Intercept mode, an IPv6-only network that offers no DHCPv4
and no PREF64 ended in a DNS outage. The DHCP option-6 read fails there
because no lease exists, and the native-CLAT evidence does not exist yet
because macOS starts its CLAT only after RFC 7050 prefix discovery
(ipv4only.arpa) succeeds. pf blocks IPv6 port 53 and the service has no
IPv4 DNS, so that lookup can never be sent. ctrld waited for evidence
that only DNS could produce.

A failed DHCP read now admits the target with a second kind of positive
native evidence: configd publishes no global or per-service IPv4 state,
the IPv6 primary service is this device, and the interface holds only
IPv6 addresses (at least one routable). The service mapping, unique
enabled networksetup name, two-second shared budget, tuple recheck,
validated snapshot backup and ownership rules are the same as on the
CLAT path. Unknown or changing state still admits nothing. A readable
DHCP lease never consults the new evidence, and CLAT evidence keeps
precedence. The journal line records which evidence admitted the
target.
2026-10-20 17:23:47 +07:00
Dev Scribe b8616e4ec9 feat(cli): add opt-in macOS unprivileged log-send socket 2026-10-20 17:23:47 +07:00
Dev Scribe fec2077409 fix(cli): redact the provision code in the intercept upgrade check line
`ctrld start --cd-org <code>` logged its arguments as-is in the debug line
`intercept upgrade check: args=[--cd-org <code> ...]`, so the code reached
the console with -v/-vv and any log that keeps debug lines. A provision code
enrolls endpoints into the organization until it expires or runs out.

redactedArgs formats the arguments through redactSecrets with
provisionSecrets, which covers --cd-org and both parts of --cd, in either
flag form. The line keeps the other arguments.
2026-10-20 17:17:54 +07:00
Cuong Manh Le a193ec2ce0 feat(cli): keep serving DNS through Control D API maintenance
A hard maintenance window took DNS down on endpoints that already had a
working configuration: the bootstrap preflight read the maintenance answer
as a failed fetch and exited, turning a temporary API outage into an
endpoint outage.

Startup now falls back to the configuration on disk, which keeps resolving
until the API returns. Eligibility is not file presence: a start with no
configuration writes the seeded freedns default before the API is ever
reached, so a marker records that the API produced the file, bound to both
the identity and the file's contents - an identity-only marker outlives the
file it vouched for, and would certify the default regenerated in its
place. Because that binding follows the contents, every managed write
refreshes it, or one API-driven config change would leave the host
ineligible for the fallback its own configuration is meant to provide.
Generated defaults, NextDNS configurations and failed writes certify
nothing. Without a matching marker ctrld still stops, now with
API_MAINTENANCE (exit 37) and a message naming what cannot be done rather
than reporting a fetch failure that reads as broken connectivity.

Also:

- Maintenance is never a permanent rejection or a deleted device, so it
  cannot stop the service for good or trigger a self-uninstall.
- A run that started on disk retries every 5-7 minutes until it has an
  answer. refetch_time is untouched, and the jitter keeps hosts that fell
  back together from returning as one burst.
- Recovery is acknowledged where the configuration takes effect, not where
  the fetch returns or the reload is triggered: the receiver fetches again
  and can fail. Until then the run stays pending and its answers keep
  reaching the apply path, instead of comparing equal to a response that
  was stored but never applied.
- Such a run never asked about the deactivation PIN, and the value meaning
  "no PIN" is the one it starts with, so deactivation is refused with 503
  while the PIN cannot be verified.
2026-10-20 17:17:54 +07:00
Cuong Manh Le bb78863d33 fix(cli): mark AD lookups on the bypass and OS retry paths
The previous change marked a rule-matched AD query as a LAN query, but
two paths still reach the OS resolver without that mark, so osResolver
raced the domain controller against 76.76.2.0 in plaintext:

  - the DNS-intercept recovery bypass, which forwards queries before
    rule handling runs
  - the OS-resolver retry catch-all after an explicit upstream fails
    with leak_on_upstream_failure enabled

Mark a query under the detected AD domain on both paths. Other names
keep the public fallback, and an explicit upstream is still tried first.

Build resolvers in proxy() through newResolverFn, so tests drive proxy()
and assert the LAN mark that reaches each resolver, instead of testing
the predicate alone.
2026-10-20 17:17:54 +07:00
Alex Paguis 65cd14afc5 fix(cli): keep Active Directory lookups off Control D's public resolver
The auto-added AD split rules route corp names to upstream.os, but a rule
match skipped the LAN-query classification in proxy(), so osResolver raced
the domain controller against 76.76.2.0 and sent every AD lookup (DC
hostnames, DC-locator SRV names carrying the machine name, WPAD) there in
plaintext. Mark a rule-matched query under the machine's AD domain
(p.adDomain) whose upstreams are the OS resolver alone as a LAN query, which
drops 76.76.2.0 for it. Explicit-resolver rules are unchanged.
2026-10-20 17:17:49 +07:00
Cuong Manh Le 806683d727 test(cli): take the upload time bound before reading the upload
TestOSRecoverySkipSurvivesDebugTruncation passed time.Now() as the
argument after readLogReader, and Go evaluates arguments left to right,
so the lower bound was taken after the upload header was rendered. The
header check failed whenever a second boundary fell between the two:

    header time = 11:13:44, want a time between 11:13:45.007 and now

Take the bound first, as the other splitUpload callers do.
2026-10-06 14:53:33 +07:00
Cuong Manh Le 5978ddfe35 cli: add network fallback and explicit-only Internal Domain modes
Internal Domains now support three modes. "os" (network default) is
unchanged. "resolvers" becomes explicit resolver with network fallback,
the default explicit selection: the configured resolvers are tried
first, and a timeout, unreachable resolver, SERVFAIL, NXDOMAIN, REFUSED
or NOTIMP hands the query to the network resolvers - matching VPN DNS
servers, then domain-less VPN DNS servers, then the OS resolver's LAN
nameservers. A valid empty answer stays final. "resolvers_only" keeps
the previous strict behavior.

The OS step uses a new LanOnlyQueryCtx: the OS resolver then asks only
its LAN-classified nameservers (private, loopback, link-local, CGNAT)
and sends nothing when it has none, so the private name never reaches a
public nameserver, whether DHCP supplied it or it is ctrld's own public
fallback. LanQueryCtx only dropped the latter. LAN-only queries get
their own singleflight and hot-cache key, so they are never answered
with an ordinary query's public answer.

In fallback mode a configured resolver that the upstream monitor has
marked down is skipped instead of waited on, so an endpoint off the
organization network reaches the fallback without paying the
resolver's 2s timeout on every query. Nothing else marks a generated
resolver up again, so skipping one re-checks it in the background, at
most once every 30s; an answer marks it up. Strict mode never skips.

An absent mode with resolvers gets the fallback mode. An unrecognized
mode with resolvers fails closed to explicit resolver only, so a wire
value this build does not know never sends the domain to the Control D
upstream; without resolvers it is dropped. Switching between the
explicit modes is detected on refresh.

Generated upstreams are now identified by an UpstreamConfig.InternalDomain
marker that records their mode and that no configuration file can set,
instead of by the internal_ key prefix. An upstream a local ctrld.toml or
custom config defines as internal_foo, or as a copy of a generated one,
is an ordinary upstream again: it keeps the OS-resolver catch-all,
recovery and query-health grading.

The "resolvers_only" wire value is provisional until the API agrees on
it; the dashboard does not offer strict mode yet.
2026-10-06 14:53:19 +07:00
Anthony Wong db228a86ae fix(cli): fix three log journal defects and test Windows with the race detector 2026-10-06 14:52:42 +07:00
Dev Scribe 462f912495 Harden DNS64, VPN routing and DNS recovery ownership 2026-09-28 16:56:04 +07:00
Dev Scribe 44a638d158 macOS: share network service-order parsing
Decode networksetup entries once for interface lookup and DNS-target
selection. Preserve first-enabled lookup and fail-closed DNS uniqueness
as separate policies. Treat only the (*) index as disabled, preserving
asterisks and whitespace in enabled service names.

Add shared-parser, caller-policy and incomplete-read regressions.
2026-09-28 16:55:42 +07:00
Dev Scribe d779b5374c macOS: use the networksetup service name for USB CLAT targets 2026-09-28 16:55:27 +07:00
Dev Scribe 7613c05076 macOS: gate DNS-target fallback on native CLAT and diagnose IPv6 blocking 2026-09-28 16:55:15 +07:00
Cuong Manh Le 48fa94eb7f test(cli): scope probe journal assertions to their own log lines
captureDebugMainLog swaps the process-wide mainLog, so every
mainLog.Load().Warn()/.Error() site in the package writes into the
buffer while it is installed, including goroutines left running by
other tests. retainedProbeLines returned every retained line and the
tests asserted field values over all of them, so one unrelated warn
failed the run.

Test_checkDnsLoopKeepsACustomKeyOutOfTheJournal hit this on Windows,
where the loop probe held the buffer open for two seconds:

    upstream_probe_journal_test.go: field "upstream": got , want upstream.custom

The sibling tests were exposed the same way; they index retained[0]
after a strict length check, so a stray line fails them as got 2, want 1.

Let retainedProbeLines take the messages a caller owns and return only
those lines. wantNoProbeSecret keeps scanning every retained line,
which is what that leak check is for.
2026-09-28 16:53:02 +07:00
Cuong Manh Le e34fef3167 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.
2026-09-28 16:52:51 +07:00
Dev Scribe c7048f71b1 Scope OS-only recovery and retain DNS-target diagnostics 2026-09-28 16:52:27 +07:00
Dev Scribe 72507f3e2c test(cli): verify closed log files portably on Windows
Use Write rather than Stat to verify retained file handles are closed.
Windows Stat returns ERROR_INVALID_HANDLE instead of os.ErrClosed.
Retain final-cleanup content assertions and strict closed-error matching.
2026-09-28 16:52:10 +07:00
Anthony Wong f52dfd0354 fix(cli): carry the review fixes of the master port to v1.0 2026-09-28 16:51:55 +07:00
Cuong Manh Le 5410cc5f2b test(cli): cover the DNS reset failure paths through the real code
The genuine-failure cases fed a fabricated error straight to
logIfaceLookupFailure, so they exercised only its own branching. A
restoration failure never reaches that helper, and no test reached the
enumeration classification in netInterface at all: discarding the
enumeration error, or downgrading an actual restoration failure to
debug, both kept the suite green.

The host boundaries of the reset path are now variables, in the style
the package already uses: the interface enumeration, the interface
lookup, the DNS setters and the NetworkManager restore. The tests stub
them, so a run changes no host DNS or NetworkManager state, and drive
the real netInterface and resetDNSForRunningIface to assert that a
failed enumeration keeps its cause and is not reported as a missing
interface, and that a failure to restore the saved static config or to
reset to DHCP on an interface that does exist stays at error level.
Both surviving mutations, and three more, now fail.
2026-09-28 16:50:50 +07:00
Cuong Manh Le 6eed09338d fix(cli): skip DNS restoration quietly when the interface is gone
A successful macOS upgrade printed "could not get interface
error=interface not found iface=en5" before reporting success, because
the DNS cleanup logged every interface lookup failure at error level.
An interface that is gone, an unplugged adapter or a torn down tether,
has no DNS settings left to restore, so the skip is now a debug
diagnostic naming the interface and the work it skipped.

Only that condition is quiet. netInterface returns a sentinel error for
a missing interface and no longer reports a failed enumeration as one,
so any other lookup failure and any restoration failure on an interface
that does exist stay at error level.
2026-09-28 16:50:39 +07:00
Dev ScribeandGinder Singh 3127212eca fix(cli): release runtime resources across startup and shutdown
Co-authored-by: Ginder Singh <ginder@windscribe.com>
2026-09-28 16:49:42 +07:00
Anthony Wong 8ae09ac98e feat(cli): put the network state of the host in the log journal
The journal records the network state of the host and each change of
that state. The support team reads one file to see the interfaces, the
routes, the resolvers, and the recoveries around an incident. The same
file shows the health of the query path. A change that touches only
AirDrop or virtual interfaces runs no DNS work.
2026-09-28 16:49:05 +07:00
Anthony Wong 8934e09428 feat(cli): keep a log journal on disk with file headers and an upload limit
ctrld keeps a second log file, ctrld-journal.log, next to ctrld.log. The
journal holds all warnings and errors and each event with the field
journal=true. It stays on disk after a start of the service and after a
self-upgrade. Each log file starts with a Log header line. Each log send
upload starts with the same line, made at send time, and has a limit of
about 16 MB.
2026-09-28 16:47:40 +07:00
Cuong Manh Le d1690545f9 dns_intercept: don't write a GP sibling on failed NRPT handback
A failed handback restored ctrld's rule via addNRPTCatchAllRule, which
also writes the GP-path rule whenever another GP rule exists - leaving
CtrldCatchAll beside the administrator's catch-all. Restore only the
local store while that child is still present.
2026-09-28 16:45:45 +07:00
Dev Scribe 799f14e639 macOS: repair network recovery and improve diagnostics 2026-09-28 16:45:33 +07:00
Cuong Manh Le 7da2507d19 cli: route organization Internal Domains from managed config
Consume resolver.split_dns and generate a policy rule per domain, plus its
wildcard, so a suffix and its subdomains route to the OS resolver when no
resolvers are given, or to the configured resolvers when they are.

Explicit resolvers are exclusive: when all of them fail the query returns
SERVFAIL rather than taking the OS catch-all or starting recovery, and it
skips VPN split routing, which is auto-detected. Excludes and custom config
keep precedence; a refresh regenerates the config, so removals leave no
stale rule or upstream.
2026-09-28 16:45:16 +07:00
Local Scribe c75989a6ee darwin: route Zscaler's DNS echo name through the OS resolver 2026-09-28 16:45:03 +07:00
Anthony Wong 3cd20a3eef feat(cli): port provisioning hardening to v1.0 2026-09-28 16:44:51 +07:00
Dev Scribe 9d724869e9 dns proxy: DNS64 synthesis for IPv6-only networks without CLAT 2026-09-28 16:43:43 +07:00
Cuong Manh Le ca27f60bc4 fix(darwin): probe interception after the host wakes
Rule text survives a suspend unchanged, so the watchdog reports the pf
anchor intact while nothing translates, and the probe monitor is only
armed by interface changes with a schedule frozen through the sleep.
Detect the resume from the wall-clock gap and probe from there, with a
bounded repair: at most two forced reloads across the retry schedule.

Refresh the OS resolver before every probe - the probe aims at the first
OS nameserver and passes when it has none, so a pre-sleep list makes it
meaningless. Stand down for stabilization, which verifies on completion.
2026-09-28 16:40:49 +07:00
Dev Scribe 8013bb72d2 dns intercept: handle DNS-less networks and canceled-recovery state leak
macOS intercept mode fails totally on networks that provide no usable
IPv4 DNS (e.g. IPv6-only iPhone tethering with 464XLAT): the pf ruleset
blocks all outbound IPv6 port 53, and with no IPv4 DNS configured
mDNSResponder emits no DNS packets at all, so pf has nothing to
intercept while the Control D upstream stays provably healthy.

When network-change recovery discovers no usable IPv4 DNS on the
default-route service, set 127.0.0.1 as that service DNS so macOS can
emit queries that land directly on the listener. The entry is removed
when the network regains IPv4 DNS and on intercept shutdown; networks
that provide IPv4 DNS are never modified. Runs on the already-debounced
recovery path so interface flaps do not churn networksetup.

Also fix the canceled-recovery state leak: the cancellation early
return never reset recoveryBypass/recoveryRunning, so a flap burst
ending in a canceled recovery left the daemon in bypass forever with
the DNS watchdog disabled. Cleanup is generation-gated so a superseded
recovery never clears state owned by its successor.
2026-09-01 11:08:13 +07:00
Dev Scribe d2aa155ff3 pkg: port managed DNS mode review fixes 2026-09-01 11:03:13 +07:00
Anthony Wong d78e9bcf5b fix(cli): treat explicit intercept-mode off as final in listener setup
tryUpdateListenerConfig treated an explicit --intercept-mode off the same
as an empty flag and fell back to the persisted config value. run() selects
the listener strategy before it clears the persisted mode, so the first
start after a revert to standard mode selected the intercept strategy from
a stale dns/hard value while setDNS kept interception off.

Extract the resolution into listenerInterceptMode and make an explicit off
final, the same contract as setDNS. Add a regression test that fails
without the fix.
2026-08-22 01:34:48 +07:00
Cuong Manh Le 30acb846ca Bump staticcheck-action to v1.4.1
While at it, also removing the unmatched //lint line.
2026-08-21 15:38:35 +07:00
Dev Scribe 6615e431dc Apply managed DNS mode in the macOS package 2026-08-21 15:12:32 +07:00
Anthony Wong 1f001a559a feat(cli): add stable provisioning failure codes for manual and MDM installs 2026-08-21 14:52:02 +07:00
Cuong Manh Le 8ce3b7ca6c cmd/cli: log the config error that rejected a custom config
The warning reported err, the resolver-config fetch error, which is nil on
every path that reaches it - so a rejected custom config was logged with no
reason attached. cfgErr holds the validation failure.
2026-08-21 14:50:43 +07:00
Cuong Manh Le 084c785ed5 cmd/cli: report whether ctrld finished starting up, not just what SCM thinks
"ctrld status" reported the service manager's view and nothing else, so it
printed "Service is running" and exited 0 for a process that was alive and
registered as started but had never got past startup: no control socket, no DNS
listener, no policy applied. The one command an operator reaches for first
confirmed the service was fine while the host had no working DNS.

Probe the control server's /started endpoint before reporting success. That
endpoint only answers once the onStarted hooks have completed, which is after the
listeners are up, so a successful probe means the process is serving rather than
merely alive. A service that is registered as running but cannot confirm startup
is now reported as such, with a pointer to the log, and exits 3 - distinct from
stopped (1) and unknown (2), because it needs a different response.

A probe blocked by permissions is not evidence of a broken service: an
unprivileged caller still gets "Service is running", with a note that startup was
not verified. The probe is bounded by a short timeout so status stays fast.

Document the exit codes in the command's help, and cover the probe (ready, not
finished starting, no socket, timed out) and the classification, including that
an unreadable socket is not reported as a failure.

The not-ready verdict is only reported when the probe could have found the
daemon's socket. socketDir() is caller-relative on unix - the system directory
when writable, the caller's home otherwise - so an unprivileged "ctrld status"
looks somewhere the root-owned daemon never listened and gets ENOENT, which is
"wrong path", not "not ready". Since only darwin has an elevation PreRun and the
root-level alias has none, that is the normal invocation; reporting exit 3 there
would have told a monitoring check to restart healthy daemons. Such a caller now
gets the service manager's view with startup reported as unverified. Windows and
mobile resolve the same directory for every caller, so the verdict stays fully
available on the platform the hung start was seen on. A successful probe is still
conclusive whoever ran it.
2026-08-21 14:50:27 +07:00
Cuong Manh Le 5c9d3dec4e cmd/cli: stop the replacement before rolling its binary back
Rollback ran os.Remove(bin) while the replacement service was still running from
that image. Windows locks a running executable, so the remove failed with
"Access is denied" - and it was fatal, so the os.Rename that restores the
previous binary never ran. The upgrade ended with the broken replacement still
installed and the working binary stranded at its _previous name.

Readiness failing is not evidence the process exited: the service manager can
report a started service whose process never became operational. So rollback now
stops the service and waits until the manager reports it stopped before touching
the executable, then cleans up DNS the way the restart path's Cleanup task does.

Restoring is now conditional on the previous binary reporting a version, since a
_previous file that exists but produces no version output would trade a service
that starts and hangs for one that cannot start at all. When it is unusable,
rollback keeps it for inspection, leaves the installed binary alone, and says so
instead of pressing on. The --version probe is bounded by a timeout so a binary
that hangs cannot hang the upgrade.

Remaining failures are reported rather than fatal, so each one says what state
the host was left in. os.Remove is retried while the path stays locked, since
Windows releases an image lock asynchronously after the process exits.

The helpers live in a new file rather than in commands.go, and the rollback is
extracted into rollbackToPreviousBinary() so it can be covered: the stop happens
while the executable is still present, an unusable previous binary is kept
without swapping or restarting, and a failed stop aborts before anything is
modified. Reversing the stop and the remove fails these tests.

The version probe is called through a variable so those tests do not have to
stage a runnable executable. Staging one is not portable: oldBin is
bin+"_previous", so a fixture named "ctrld" yields the extension-less
"ctrld_previous", which Windows refuses to execute, and a symlink to the test
binary needs a privilege Windows does not grant by default. The probe itself is
still covered against the real test binary. Production is unaffected: ctrld.exe
_previous does have an extension, and os/exec only appends PATHEXT entries when
a path has none at all - noted at binaryVersion so the suffix is not renamed
into something extension-less by accident.
2026-08-21 14:50:11 +07:00
Cuong Manh Le 2400f27962 all: keep the first-attempt error when the direct-ip fallback also fails
Both API requests and binary downloads retry against a hard-coded IP when the
attempt via hostname fails. Both then overwrote the first error with the
fallback's, so only the last failure was reported.

That discarded the diagnosis. When the hostname attempt is denied locally -
WSAEACCES on Windows, "An attempt was made to access a socket in a way forbidden
by its access permissions", which means the host is blocking ctrld - and the
direct-ip fallback fails with an unreachable IPv6 route, what surfaces to the
operator is "dial tcp6: no route to host": a routing problem that does not
exist, while the error naming the real cause is visible only in debug logs.

Report both failures instead, keeping the error chain intact so errors.Is still
matches either one. Also switch the final wrap in doWithRetry from %v to %w,
which had been flattening the chain even when a single error was reported.

This also changes retry classification, which is worth stating explicitly because
it is not obvious from "report both errors". processCDFlags decides whether to
keep backing off with errUrlNetworkError, which uses errors.As - and errors.As
returns the *first* match in the tree. Wrapping the hostname attempt first
therefore hands the predicate that attempt's failure, where previously only the
fallback's error survived to be classified.

The effect is intended. A locally denied socket (WSAEACCES) is not a transient
network error, so preflight now fails fast and reports instead of retrying
against a firewall that is not going to clear on its own. The case that justifies
retrying forever, a network unreachable on both attempts at boot, is unchanged.
Both classifications are pinned by tests, along with the wrap order they depend on
at each composition site, so reversing it fails loudly rather than silently
restoring the old behaviour.
2026-08-21 14:49:58 +07:00
Cuong Manh Le 4f730167d4 cmd/cli: bound API preflight by service lifetime
processCDFlags retries the resolver-config fetch indefinitely by design: a
device that has no working network at boot must eventually come up. The loop had
no cancellation, so a stop request arriving while the API is unreachable was
ignored - the process kept retrying long after the service reported itself
stopped, doing work on behalf of a service the OS considers stopped.

Thread a context through processCDFlags and derive it from p.stopCh, in both the
startup preflight and the config-reload path. The loop now returns as soon as the
context is cancelled, checked both before a retry and after backoff returns
(backoff can wake up on cancellation). A stop during preflight exits the way a
normal stop does, without Fatal, so the service manager does not treat it as a
failed start and apply its restart policy to a service the operator just asked
to stop.

Bind the two API requests themselves as well, so a stop does not have to wait out
an in-flight request. Without this the loop honours a stop only between attempts,
which leaves up to defaultTimeout (20s) of a request the service is no longer
interested in - the same "still working after Service stopped" the loop change
exists to end, one layer down.

Doing so means a context parameter on FetchResolverConfig, FetchResolverUID,
UpdateCustomLastFailed and SendLogs, since all four reach a request builder. The
callers that have no context pass context.Background(), which is what master
effectively does at those sites: its loggerCtx carries a logger, not
cancellation. doWithFallback needs no parameter, because it clones the request
with req.Context() and so inherits the binding. This also repairs
internal/controld/controld_test.go, which is behind //go:build controld and had
already been written against the context-taking signature, so it could not
compile.

Cover the cancellation paths; removing either check makes the tests hang until
timeout.

Sampling the stop state is the whole point of runAPIPreflight rather than doing
this inline. A stop and a failure need opposite handling - one exits quietly, the
other self-uninstalls a deleted device, surfaces the error to a mobile app, and
reports a failed start - so the two must not be confused. Reading it from the
context after cancelling would report "stopped" for every failure, since
CancelFunc sets ctx.Err() regardless of whether anyone asked to stop; the stop
channel is read directly instead, which also does not depend on the context
watcher goroutine having been scheduled.
2026-08-21 14:49:30 +07:00