Pull request: home: imp code

Merge in DNS/adguard-home from home-imp-code to master

Squashed commit of the following:

commit 459297e189c55393bf0340dd51ec9608d3475e55
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 11:42:34 2023 +0300

    home: imp code

commit ab38e1e80fed7b24fe57d4afdc57b70608f65d73
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 11:01:23 2023 +0300

    all: lint script

commit 7df68b128bf32172ef2e3bf7116f4f72a97baa2b
Merge: bcb482714 db52f7a3a
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 10 10:59:40 2023 +0300

    Merge remote-tracking branch 'origin/master' into home-imp-code

commit bcb482714780da882e69c261be08511ea4f36f3b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 13:48:27 2023 +0300

    all: lint script

commit 1c017f27715202ec1f40881f069a96f11f9822e8
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 13:45:25 2023 +0300

    all: lint script

commit ee3d427a7d6ee7e377e67c5eb99eebc7fb1e6acc
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 13:44:53 2023 +0300

    home: imp code

commit bc50430469123415216e60e178bd8e30fc229300
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 13:12:10 2023 +0300

    home: imp code

commit fc07e416aeab2612e68cf0e3f933aaed95931115
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 11:42:32 2023 +0300

    aghos: service precheck

commit a68480fd9c4cd6f3c89210bee6917c53074f7a82
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Thu May 4 11:07:05 2023 +0300

    home: imp code

commit 61b743a340ac1564c48212452c7a9acd1808d352
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 17:17:21 2023 +0300

    all: lint script

commit c6fe620510c4af5b65456e90cb3424831334e004
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 17:16:37 2023 +0300

    home: imp code

commit 4b2fb47ea9c932054ccc72b1fd1d11793c93e39c
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 16:55:44 2023 +0300

    home: imp code

commit 63df3e2ab58482920a074cfd5f4188e49a0f8448
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 16:25:38 2023 +0300

    home: imp code

commit c7f1502f976482c2891e0c64426218b549585e83
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 15:54:30 2023 +0300

    home: imp code

commit c64cdaf1c82495bb70d9cdcaf7be9eeee9a7c773
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 14:35:04 2023 +0300

    home: imp code

commit a50436e040b3a064ef51d5f936b879fe8de72d41
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 14:24:02 2023 +0300

    home: imp code

commit 2b66464f472df732ea27cbbe5ac5c673a13bc14b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 14:11:53 2023 +0300

    home: imp code

commit 713ce2963c210887faa0a06e41e01e4ebbf96894
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Wed May 3 14:10:54 2023 +0300

    home: imp code

internal/aghos/service.go

@@ -0,0 +1,6 @@
+package aghos
+
+// PreCheckActionStart performs the service start action pre-check.
+func PreCheckActionStart() (err error) {
+	return preCheckActionStart()
+}

internal/aghos/service_darwin.go

@@ -0,0 +1,32 @@
+//go:build darwin
+
+package aghos
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/AdguardTeam/golibs/log"
+)
+
+// preCheckActionStart performs the service start action pre-check.  It warns
+// user that the service should be installed into Applications directory.
+func preCheckActionStart() (err error) {
+	exe, err := os.Executable()
+	if err != nil {
+		return fmt.Errorf("getting executable path: %v", err)
+	}
+
+	exe, err = filepath.EvalSymlinks(exe)
+	if err != nil {
+		return fmt.Errorf("evaluating executable symlinks: %v", err)
+	}
+
+	if !strings.HasPrefix(exe, "/Applications/") {
+		log.Info("warning: service must be started from within the /Applications directory")
+	}
+
+	return err
+}

internal/aghos/service_others.go

@@ -0,0 +1,8 @@
+//go:build !darwin
+
+package aghos
+
+// preCheckActionStart performs the service start action pre-check.
+func preCheckActionStart() (err error) {
+	return nil
+}

internal/home/config.go

@@ -399,19 +399,23 @@ func (c *configuration) getConfigFilename() string {
 	return configFile
 }
 
-// getLogSettings reads logging settings from the config file.
+// readLogSettings reads logging settings from the config file.  We do it in a
-// we do it in a separate method in order to configure logger before the actual configuration is parsed and applied.
+// separate method in order to configure logger before the actual configuration
-func getLogSettings() logSettings {
+// is parsed and applied.
-	l := logSettings{}
+func readLogSettings() (ls *logSettings) {
+	ls = &logSettings{}
+
 	yamlFile, err := readConfigFile()
 	if err != nil {
-		return l
+		return ls
 	}
-	err = yaml.Unmarshal(yamlFile, &l)
+
+	err = yaml.Unmarshal(yamlFile, ls)
 	if err != nil {
 		log.Error("Couldn't get logging settings from the configuration: %s", err)
 	}
-	return l
+
+	return ls
 }
 
 // validateBindHosts returns error if any of binding hosts from configuration is

internal/home/home.go

@@ -37,6 +37,7 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/AdguardTeam/golibs/stringutil"
 	"golang.org/x/exp/slices"
 	"gopkg.in/natefinch/lumberjack.v2"
 )
@@ -145,7 +146,9 @@ func Main(clientBuildFS fs.FS) {
 	run(opts, clientBuildFS)
 }
 
-func setupContext(opts options) {
+// setupContext initializes [Context] fields.  It also reads and upgrades
+// config file if necessary.
+func setupContext(opts options) (err error) {
 	setupContextFlags(opts)
 
 	Context.tlsRoots = aghtls.SystemRootCAs()
@@ -162,10 +165,15 @@ func setupContext(opts options) {
 		},
 	}
 
+	Context.mux = http.NewServeMux()
+
 	if !Context.firstRun {
 		// Do the upgrade if necessary.
-		err := upgradeConfig()
+		err = upgradeConfig()
-		fatalOnError(err)
+		if err != nil {
+			// Don't wrap the error, because it's informative enough as is.
+			return err
+		}
 
 		if err = parseConfig(); err != nil {
 			log.Error("parsing configuration file: %s", err)
@@ -181,11 +189,14 @@ func setupContext(opts options) {
 
 		if !opts.noEtcHosts && config.Clients.Sources.HostsFile {
 			err = setupHostsContainer()
-			fatalOnError(err)
+			if err != nil {
+				// Don't wrap the error, because it's informative enough as is.
+				return err
+			}
 		}
 	}
 
-	Context.mux = http.NewServeMux()
+	return nil
 }
 
 // setupContextFlags sets global flags and prints their status to the log.
@@ -287,78 +298,27 @@ func setupHostsContainer() (err error) {
 	return nil
 }
 
-func setupConfig(opts options) (err error) {
+// setupOpts sets up command-line options.
-	config.DNS.DnsfilterConf.EtcHosts = Context.etcHosts
+func setupOpts(opts options) (err error) {
-	config.DNS.DnsfilterConf.ConfigModified = onConfigModified
+	err = setupBindOpts(opts)
-	config.DNS.DnsfilterConf.HTTPRegister = httpRegister
+	if err != nil {
-	config.DNS.DnsfilterConf.DataDir = Context.getDataDir()
+		// Don't wrap the error, because it's informative enough as is.
-	config.DNS.DnsfilterConf.Filters = slices.Clone(config.Filters)
+		return err
-	config.DNS.DnsfilterConf.WhitelistFilters = slices.Clone(config.WhitelistFilters)
-	config.DNS.DnsfilterConf.UserRules = slices.Clone(config.UserRules)
-	config.DNS.DnsfilterConf.HTTPClient = Context.client
-
-	const (
-		dnsTimeout = 3 * time.Second
-
-		sbService                 = "safe browsing"
-		defaultSafeBrowsingServer = `https://family.adguard-dns.com/dns-query`
-		sbTXTSuffix               = `sb.dns.adguard.com.`
-
-		pcService             = "parental control"
-		defaultParentalServer = `https://family.adguard-dns.com/dns-query`
-		pcTXTSuffix           = `pc.dns.adguard.com.`
-	)
-
-	cacheTime := time.Duration(config.DNS.DnsfilterConf.CacheTime) * time.Minute
-
-	upsOpts := &upstream.Options{
-		Timeout: dnsTimeout,
-		ServerIPAddrs: []net.IP{
-			{94, 140, 14, 15},
-			{94, 140, 15, 16},
-			net.ParseIP("2a10:50c0::bad1:ff"),
-			net.ParseIP("2a10:50c0::bad2:ff"),
-		},
 	}
 
-	sbUps, err := upstream.AddressToUpstream(defaultSafeBrowsingServer, upsOpts)
+	if len(opts.pidFile) != 0 && writePIDFile(opts.pidFile) {
-	if err != nil {
+		Context.pidFileName = opts.pidFile
-		return fmt.Errorf("converting safe browsing server: %w", err)
 	}
 
-	safeBrowsing := hashprefix.New(&hashprefix.Config{
+	return nil
-		Upstream:    sbUps,
+}
-		ServiceName: sbService,
-		TXTSuffix:   sbTXTSuffix,
-		CacheTime:   cacheTime,
-		CacheSize:   config.DNS.DnsfilterConf.SafeBrowsingCacheSize,
-	})
 
-	parUps, err := upstream.AddressToUpstream(defaultParentalServer, upsOpts)
+// initContextClients initializes Context clients and related fields.
+func initContextClients() (err error) {
+	err = setupDNSFilteringConf(config.DNS.DnsfilterConf)
 	if err != nil {
-		return fmt.Errorf("converting parental server: %w", err)
+		// Don't wrap the error, because it's informative enough as is.
-	}
+		return err
-
-	parentalControl := hashprefix.New(&hashprefix.Config{
-		Upstream:    parUps,
-		ServiceName: pcService,
-		TXTSuffix:   pcTXTSuffix,
-		CacheTime:   cacheTime,
-		CacheSize:   config.DNS.DnsfilterConf.SafeBrowsingCacheSize,
-	})
-
-	config.DNS.DnsfilterConf.SafeBrowsingChecker = safeBrowsing
-	config.DNS.DnsfilterConf.ParentalControlChecker = parentalControl
-
-	config.DNS.DnsfilterConf.SafeSearchConf.CustomResolver = safeSearchResolver{}
-	config.DNS.DnsfilterConf.SafeSearch, err = safesearch.NewDefault(
-		config.DNS.DnsfilterConf.SafeSearchConf,
-		"default",
-		config.DNS.DnsfilterConf.SafeSearchCacheSize,
-		time.Minute*time.Duration(config.DNS.DnsfilterConf.CacheTime),
-	)
-	if err != nil {
-		return fmt.Errorf("initializing safesearch: %w", err)
 	}
 
 	//lint:ignore SA1019  Migration is not over.
@@ -393,8 +353,19 @@ func setupConfig(opts options) (err error) {
 		arpdb = aghnet.NewARPDB()
 	}
 
-	Context.clients.Init(config.Clients.Persistent, Context.dhcpServer, Context.etcHosts, arpdb, config.DNS.DnsfilterConf)
+	Context.clients.Init(
+		config.Clients.Persistent,
+		Context.dhcpServer,
+		Context.etcHosts,
+		arpdb,
+		config.DNS.DnsfilterConf,
+	)
 
+	return nil
+}
+
+// setupBindOpts overrides bind host/port from the opts.
+func setupBindOpts(opts options) (err error) {
 	if opts.bindPort != 0 {
 		config.BindPort = opts.bindPort
 
@@ -405,12 +376,83 @@ func setupConfig(opts options) (err error) {
 		}
 	}
 
-	// override bind host/port from the console
 	if opts.bindHost.IsValid() {
 		config.BindHost = opts.bindHost
 	}
-	if len(opts.pidFile) != 0 && writePIDFile(opts.pidFile) {
+
-		Context.pidFileName = opts.pidFile
+	return nil
+}
+
+// setupDNSFilteringConf sets up DNS filtering configuration settings.
+func setupDNSFilteringConf(conf *filtering.Config) (err error) {
+	const (
+		dnsTimeout = 3 * time.Second
+
+		sbService                 = "safe browsing"
+		defaultSafeBrowsingServer = `https://family.adguard-dns.com/dns-query`
+		sbTXTSuffix               = `sb.dns.adguard.com.`
+
+		pcService             = "parental control"
+		defaultParentalServer = `https://family.adguard-dns.com/dns-query`
+		pcTXTSuffix           = `pc.dns.adguard.com.`
+	)
+
+	conf.EtcHosts = Context.etcHosts
+	conf.ConfigModified = onConfigModified
+	conf.HTTPRegister = httpRegister
+	conf.DataDir = Context.getDataDir()
+	conf.Filters = slices.Clone(config.Filters)
+	conf.WhitelistFilters = slices.Clone(config.WhitelistFilters)
+	conf.UserRules = slices.Clone(config.UserRules)
+	conf.HTTPClient = Context.client
+
+	cacheTime := time.Duration(conf.CacheTime) * time.Minute
+
+	upsOpts := &upstream.Options{
+		Timeout: dnsTimeout,
+		ServerIPAddrs: []net.IP{
+			{94, 140, 14, 15},
+			{94, 140, 15, 16},
+			net.ParseIP("2a10:50c0::bad1:ff"),
+			net.ParseIP("2a10:50c0::bad2:ff"),
+		},
+	}
+
+	sbUps, err := upstream.AddressToUpstream(defaultSafeBrowsingServer, upsOpts)
+	if err != nil {
+		return fmt.Errorf("converting safe browsing server: %w", err)
+	}
+
+	conf.SafeBrowsingChecker = hashprefix.New(&hashprefix.Config{
+		Upstream:    sbUps,
+		ServiceName: sbService,
+		TXTSuffix:   sbTXTSuffix,
+		CacheTime:   cacheTime,
+		CacheSize:   conf.SafeBrowsingCacheSize,
+	})
+
+	parUps, err := upstream.AddressToUpstream(defaultParentalServer, upsOpts)
+	if err != nil {
+		return fmt.Errorf("converting parental server: %w", err)
+	}
+
+	conf.ParentalControlChecker = hashprefix.New(&hashprefix.Config{
+		Upstream:    parUps,
+		ServiceName: pcService,
+		TXTSuffix:   pcTXTSuffix,
+		CacheTime:   cacheTime,
+		CacheSize:   conf.SafeBrowsingCacheSize,
+	})
+
+	conf.SafeSearchConf.CustomResolver = safeSearchResolver{}
+	conf.SafeSearch, err = safesearch.NewDefault(
+		conf.SafeSearchConf,
+		"default",
+		conf.SafeSearchCacheSize,
+		cacheTime,
+	)
+	if err != nil {
+		return fmt.Errorf("initializing safesearch: %w", err)
 	}
 
 	return nil
@@ -487,14 +529,16 @@ func fatalOnError(err error) {
 
 // run configures and starts AdGuard Home.
 func run(opts options, clientBuildFS fs.FS) {
-	// configure config filename
+	// Configure config filename.
 	initConfigFilename(opts)
 
-	// configure working dir and config path
+	// Configure working dir and config path.
-	initWorkingDir(opts)
+	err := initWorkingDir(opts)
+	fatalOnError(err)
 
-	// configure log level and output
+	// Configure log level and output.
-	configureLogger(opts)
+	err = configureLogger(opts)
+	fatalOnError(err)
 
 	// Print the first message after logger is configured.
 	log.Info(version.Full())
@@ -503,25 +547,29 @@ func run(opts options, clientBuildFS fs.FS) {
 		log.Info("AdGuard Home is running as a service")
 	}
 
-	setupContext(opts)
+	err = setupContext(opts)
-
-	err := configureOS(config)
 	fatalOnError(err)
 
-	// clients package uses filtering package's static data (filtering.BlockedSvcKnown()),
+	err = configureOS(config)
-	//  so we have to initialize filtering's static data first,
+	fatalOnError(err)
-	//  but also avoid relying on automatic Go init() function
+
+	// Clients package uses filtering package's static data
+	// (filtering.BlockedSvcKnown()), so we have to initialize filtering static
+	// data first, but also to avoid relying on automatic Go init() function.
 	filtering.InitModule()
 
-	err = setupConfig(opts)
+	err = initContextClients()
 	fatalOnError(err)
 
-	// TODO(e.burkov):  This could be made earlier, probably as the option's
+	err = setupOpts(opts)
+	fatalOnError(err)
+
+	// TODO(e.burkov): This could be made earlier, probably as the option's
 	// effect.
 	cmdlineUpdate(opts)
 
 	if !Context.firstRun {
-		// Save the updated config
+		// Save the updated config.
 		err = config.write()
 		fatalOnError(err)
 
@@ -531,33 +579,15 @@ func run(opts options, clientBuildFS fs.FS) {
 		}
 	}
 
-	err = os.MkdirAll(Context.getDataDir(), 0o755)
+	dir := Context.getDataDir()
-	if err != nil {
+	err = os.MkdirAll(dir, 0o755)
-		log.Fatalf("Cannot create DNS data dir at %s: %s", Context.getDataDir(), err)
+	fatalOnError(errors.Annotate(err, "creating DNS data dir at %s: %w", dir))
-	}
 
-	sessFilename := filepath.Join(Context.getDataDir(), "sessions.db")
 	GLMode = opts.glinetMode
-	var rateLimiter *authRateLimiter
-	if config.AuthAttempts > 0 && config.AuthBlockMin > 0 {
-		rateLimiter = newAuthRateLimiter(
-			time.Duration(config.AuthBlockMin)*time.Minute,
-			config.AuthAttempts,
-		)
-	} else {
-		log.Info("authratelimiter is disabled")
-	}
 
-	Context.auth = InitAuth(
+	// Init auth module.
-		sessFilename,
+	Context.auth, err = initUsers()
-		config.Users,
+	fatalOnError(err)
-		config.WebSessionTTLHours*60*60,
-		rateLimiter,
-	)
-	if Context.auth == nil {
-		log.Fatalf("Couldn't initialize Auth module")
-	}
-	config.Users = nil
 
 	Context.tls, err = newTLSManager(config.TLS)
 	if err != nil {
@@ -575,10 +605,10 @@ func run(opts options, clientBuildFS fs.FS) {
 		Context.tls.start()
 
 		go func() {
-			serr := startDNSServer()
+			sErr := startDNSServer()
-			if serr != nil {
+			if sErr != nil {
 				closeDNSServer()
-				fatalOnError(serr)
+				fatalOnError(sErr)
 			}
 		}()
 
@@ -592,10 +622,33 @@ func run(opts options, clientBuildFS fs.FS) {
 
 	Context.web.start()
 
-	// wait indefinitely for other go-routines to complete their job
+	// Wait indefinitely for other goroutines to complete their job.
 	select {}
 }
 
+// initUsers initializes context auth module.  Clears config users field.
+func initUsers() (auth *Auth, err error) {
+	sessFilename := filepath.Join(Context.getDataDir(), "sessions.db")
+
+	var rateLimiter *authRateLimiter
+	if config.AuthAttempts > 0 && config.AuthBlockMin > 0 {
+		blockDur := time.Duration(config.AuthBlockMin) * time.Minute
+		rateLimiter = newAuthRateLimiter(blockDur, config.AuthAttempts)
+	} else {
+		log.Info("authratelimiter is disabled")
+	}
+
+	sessionTTL := config.WebSessionTTLHours * 60 * 60
+	auth = InitAuth(sessFilename, config.Users, sessionTTL, rateLimiter)
+	if auth == nil {
+		return nil, errors.Error("initializing auth module failed")
+	}
+
+	config.Users = nil
+
+	return auth, nil
+}
+
 func (c *configuration) anonymizer() (ipmut *aghnet.IPMut) {
 	var anonFunc aghnet.IPMutFunc
 	if c.DNS.AnonymizeClientIP {
@@ -668,22 +721,19 @@ func writePIDFile(fn string) bool {
 	return true
 }
 
+// initConfigFilename sets up context config file path.  This file path can be
+// overridden by command-line arguments, or is set to default.
 func initConfigFilename(opts options) {
-	// config file path can be overridden by command-line arguments:
+	Context.configFilename = stringutil.Coalesce(opts.confFilename, "AdGuardHome.yaml")
-	if opts.confFilename != "" {
-		Context.configFilename = opts.confFilename
-	} else {
-		// Default config file name
-		Context.configFilename = "AdGuardHome.yaml"
-	}
 }
 
-// initWorkingDir initializes the workDir
+// initWorkingDir initializes the workDir.  If no command-line arguments are
-// if no command-line arguments specified, we use the directory where our binary file is located
+// specified, the directory with the binary file is used.
-func initWorkingDir(opts options) {
+func initWorkingDir(opts options) (err error) {
 	execPath, err := os.Executable()
 	if err != nil {
-		panic(err)
+		// Don't wrap the error, because it's informative enough as is.
+		return err
 	}
 
 	if opts.workDir != "" {
@@ -695,34 +745,20 @@ func initWorkingDir(opts options) {
 
 	workDir, err := filepath.EvalSymlinks(Context.workDir)
 	if err != nil {
-		panic(err)
+		// Don't wrap the error, because it's informative enough as is.
+		return err
 	}
 
 	Context.workDir = workDir
+
+	return nil
 }
 
-// configureLogger configures logger level and output
+// configureLogger configures logger level and output.
-func configureLogger(opts options) {
+func configureLogger(opts options) (err error) {
-	ls := getLogSettings()
+	ls := getLogSettings(opts)
 
-	// command-line arguments can override config settings
+	// Configure logger level.
-	if opts.verbose || config.Verbose {
-		ls.Verbose = true
-	}
-	if opts.logFile != "" {
-		ls.File = opts.logFile
-	} else if config.File != "" {
-		ls.File = config.File
-	}
-
-	// Handle default log settings overrides
-	ls.Compress = config.Compress
-	ls.LocalTime = config.LocalTime
-	ls.MaxBackups = config.MaxBackups
-	ls.MaxSize = config.MaxSize
-	ls.MaxAge = config.MaxAge
-
-	// log.SetLevel(log.INFO) - default
 	if ls.Verbose {
 		log.SetLevel(log.DEBUG)
 	}
@@ -731,38 +767,63 @@ func configureLogger(opts options) {
 	// happen pretty quickly.
 	log.SetFlags(log.LstdFlags | log.Lmicroseconds)
 
-	if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" {
+	// Write logs to stdout by default.
-		// When running as a Windows service, use eventlog by default if nothing
-		// else is configured.  Otherwise, we'll simply lose the log output.
-		ls.File = configSyslog
-	}
-
-	// logs are written to stdout (default)
 	if ls.File == "" {
-		return
+		return nil
 	}
 
 	if ls.File == configSyslog {
-		// Use syslog where it is possible and eventlog on Windows
+		// Use syslog where it is possible and eventlog on Windows.
-		err := aghos.ConfigureSyslog(serviceName)
+		err = aghos.ConfigureSyslog(serviceName)
 		if err != nil {
-			log.Fatalf("cannot initialize syslog: %s", err)
+			return fmt.Errorf("cannot initialize syslog: %w", err)
-		}
-	} else {
-		logFilePath := ls.File
-		if !filepath.IsAbs(logFilePath) {
-			logFilePath = filepath.Join(Context.workDir, logFilePath)
 		}
 
-		log.SetOutput(&lumberjack.Logger{
+		return nil
-			Filename:   logFilePath,
-			Compress:   ls.Compress, // disabled by default
-			LocalTime:  ls.LocalTime,
-			MaxBackups: ls.MaxBackups,
-			MaxSize:    ls.MaxSize, // megabytes
-			MaxAge:     ls.MaxAge,  // days
-		})
 	}
+
+	logFilePath := ls.File
+	if !filepath.IsAbs(logFilePath) {
+		logFilePath = filepath.Join(Context.workDir, logFilePath)
+	}
+
+	log.SetOutput(&lumberjack.Logger{
+		Filename:   logFilePath,
+		Compress:   ls.Compress,
+		LocalTime:  ls.LocalTime,
+		MaxBackups: ls.MaxBackups,
+		MaxSize:    ls.MaxSize,
+		MaxAge:     ls.MaxAge,
+	})
+
+	return nil
+}
+
+// getLogSettings returns a log settings object properly initialized from opts.
+func getLogSettings(opts options) (ls *logSettings) {
+	ls = readLogSettings()
+
+	// Command-line arguments can override config settings.
+	if opts.verbose || config.Verbose {
+		ls.Verbose = true
+	}
+
+	ls.File = stringutil.Coalesce(opts.logFile, config.File, ls.File)
+
+	// Handle default log settings overrides.
+	ls.Compress = config.Compress
+	ls.LocalTime = config.LocalTime
+	ls.MaxBackups = config.MaxBackups
+	ls.MaxSize = config.MaxSize
+	ls.MaxAge = config.MaxAge
+
+	if opts.runningAsService && ls.File == "" && runtime.GOOS == "windows" {
+		// When running as a Windows service, use eventlog by default if
+		// nothing else is configured.  Otherwise, we'll lose the log output.
+		ls.File = configSyslog
+	}
+
+	return ls
 }
 
 // cleanup stops and resets all the modules.

internal/home/service.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
@@ -84,14 +83,9 @@ func svcStatus(s service.Service) (status service.Status, err error) {
 // On OpenWrt, the service utility may not exist.  We use our service script
 // directly in this case.
 func svcAction(s service.Service, action string) (err error) {
-	if runtime.GOOS == "darwin" && action == "start" {
+	if action == "start" {
-		var exe string
+		if err = aghos.PreCheckActionStart(); err != nil {
-		if exe, err = os.Executable(); err != nil {
+			log.Error("starting service: %s", err)
-			log.Error("starting service: getting executable path: %s", err)
-		} else if exe, err = filepath.EvalSymlinks(exe); err != nil {
-			log.Error("starting service: evaluating executable symlinks: %s", err)
-		} else if !strings.HasPrefix(exe, "/Applications/") {
-			log.Info("warning: service must be started from within the /Applications directory")
 		}
 	}
 
@@ -99,8 +93,6 @@ func svcAction(s service.Service, action string) (err error) {
 	if err != nil && service.Platform() == "unix-systemv" &&
 		(action == "start" || action == "stop" || action == "restart") {
 		_, err = runInitdCommand(action)
-
-		return err
 	}
 
 	return err
@@ -224,6 +216,7 @@ func handleServiceControlAction(opts options, clientBuildFS fs.FS) {
 
 	runOpts := opts
 	runOpts.serviceControlAction = "run"
+
 	svcConfig := &service.Config{
 		Name:             serviceName,
 		DisplayName:      serviceDisplayName,
@@ -233,35 +226,48 @@ func handleServiceControlAction(opts options, clientBuildFS fs.FS) {
 	}
 	configureService(svcConfig)
 
-	prg := &program{
+	s, err := service.New(&program{clientBuildFS: clientBuildFS, opts: runOpts}, svcConfig)
-		clientBuildFS: clientBuildFS,
+	if err != nil {
-		opts:          runOpts,
-	}
-	var s service.Service
-	if s, err = service.New(prg, svcConfig); err != nil {
 		log.Fatalf("service: initializing service: %s", err)
 	}
 
+	err = handleServiceCommand(s, action, opts)
+	if err != nil {
+		log.Fatalf("service: %s", err)
+	}
+
+	log.Printf(
+		"service: action %s has been done successfully on %s",
+		action,
+		service.ChosenSystem(),
+	)
+}
+
+// handleServiceCommand handles service command.
+func handleServiceCommand(s service.Service, action string, opts options) (err error) {
 	switch action {
 	case "status":
 		handleServiceStatusCommand(s)
 	case "run":
 		if err = s.Run(); err != nil {
-			log.Fatalf("service: failed to run service: %s", err)
+			return fmt.Errorf("failed to run service: %w", err)
 		}
 	case "install":
 		initConfigFilename(opts)
-		initWorkingDir(opts)
+		if err = initWorkingDir(opts); err != nil {
+			return fmt.Errorf("failed to init working dir: %w", err)
+		}
+
 		handleServiceInstallCommand(s)
 	case "uninstall":
 		handleServiceUninstallCommand(s)
 	default:
 		if err = svcAction(s, action); err != nil {
-			log.Fatalf("service: executing action %q: %s", action, err)
+			return fmt.Errorf("executing action %q: %w", action, err)
 		}
 	}
 
-	log.Printf("service: action %s has been done successfully on %s", action, service.ChosenSystem())
+	return nil
 }
 
 // handleServiceStatusCommand handles service "status" command.

internal/home/tls.go

@@ -172,9 +172,32 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 		}
 	}()
 
-	tlsConf.CertificateChainData = []byte(tlsConf.CertificateChain)
+	err = loadCertificateChainData(tlsConf, status)
-	tlsConf.PrivateKeyData = []byte(tlsConf.PrivateKey)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
 
+	err = loadPrivateKeyData(tlsConf, status)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
+
+	err = validateCertificates(
+		status,
+		tlsConf.CertificateChainData,
+		tlsConf.PrivateKeyData,
+		tlsConf.ServerName,
+	)
+
+	return errors.Annotate(err, "validating certificate pair: %w")
+}
+
+// loadCertificateChainData loads PEM-encoded certificates chain data to the
+// TLS configuration.
+func loadCertificateChainData(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error) {
+	tlsConf.CertificateChainData = []byte(tlsConf.CertificateChain)
 	if tlsConf.CertificatePath != "" {
 		if tlsConf.CertificateChain != "" {
 			return errors.Error("certificate data and file can't be set together")
@@ -190,6 +213,13 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 		status.ValidCert = true
 	}
 
+	return nil
+}
+
+// loadPrivateKeyData loads PEM-encoded private key data to the TLS
+// configuration.
+func loadPrivateKeyData(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error) {
+	tlsConf.PrivateKeyData = []byte(tlsConf.PrivateKey)
 	if tlsConf.PrivateKeyPath != "" {
 		if tlsConf.PrivateKey != "" {
 			return errors.Error("private key data and file can't be set together")
@@ -203,16 +233,6 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 		status.ValidKey = true
 	}
 
-	err = validateCertificates(
-		status,
-		tlsConf.CertificateChainData,
-		tlsConf.PrivateKeyData,
-		tlsConf.ServerName,
-	)
-	if err != nil {
-		return fmt.Errorf("validating certificate pair: %w", err)
-	}
-
 	return nil
 }
 

internal/home/upgrade.go

@@ -294,71 +294,61 @@ func upgradeSchema4to5(diskConf yobj) error {
 	return nil
 }
 
-// clients:
+// upgradeSchema5to6 performs the following changes:
-// ...
 //
-//	ip: 127.0.0.1
+//	# BEFORE:
-//	mac: ...
+//	  'clients':
+//	    ...
+//	    'ip': 127.0.0.1
+//	    'mac': ...
 //
-// ->
+//	# AFTER:
-//
+//	  'clients':
-// clients:
+//	    ...
-// ...
+//	    'ids':
-//
+//		  - 127.0.0.1
-//	ids:
+//		  - ...
-//	- 127.0.0.1
-//	- ...
 func upgradeSchema5to6(diskConf yobj) error {
-	log.Printf("%s(): called", funcName())
+	log.Printf("Upgrade yaml: 5 to 6")
-
 	diskConf["schema_version"] = 6
 
-	clients, ok := diskConf["clients"]
+	clientsVal, ok := diskConf["clients"]
 	if !ok {
 		return nil
 	}
 
-	switch arr := clients.(type) {
+	clients, ok := clientsVal.([]yobj)
-	case []any:
+	if !ok {
-		for i := range arr {
+		return fmt.Errorf("unexpected type of clients: %T", clientsVal)
-			switch c := arr[i].(type) {
+	}
-			case map[any]any:
-				var ipVal any
-				ipVal, ok = c["ip"]
-				ids := []string{}
-				if ok {
-					var ip string
-					ip, ok = ipVal.(string)
-					if !ok {
-						log.Fatalf("client.ip is not a string: %v", ipVal)
-						return nil
-					}
-					if len(ip) != 0 {
-						ids = append(ids, ip)
-					}
-				}
 
-				var macVal any
+	for i := range clients {
-				macVal, ok = c["mac"]
+		c := clients[i]
-				if ok {
+		var ids []string
-					var mac string
-					mac, ok = macVal.(string)
-					if !ok {
-						log.Fatalf("client.mac is not a string: %v", macVal)
-						return nil
-					}
-					if len(mac) != 0 {
-						ids = append(ids, mac)
-					}
-				}
 
-				c["ids"] = ids
+		if ipVal, hasIP := c["ip"]; hasIP {
-			default:
+			var ip string
-				continue
+			if ip, ok = ipVal.(string); !ok {
+				return fmt.Errorf("client.ip is not a string: %v", ipVal)
+			}
+
+			if ip != "" {
+				ids = append(ids, ip)
 			}
 		}
-	default:
+
-		return nil
+		if macVal, hasMac := c["mac"]; hasMac {
+			var mac string
+			if mac, ok = macVal.(string); !ok {
+				return fmt.Errorf("client.mac is not a string: %v", macVal)
+			}
+
+			if mac != "" {
+				ids = append(ids, mac)
+			}
+		}
+
+		c["ids"] = ids
 	}
 
 	return nil

internal/home/upgrade_test.go

@@ -68,6 +68,95 @@ func TestUpgradeSchema2to3(t *testing.T) {
 	assertEqualExcept(t, oldDiskConf, diskConf, excludedEntries, excludedEntries)
 }
 
+func TestUpgradeSchema5to6(t *testing.T) {
+	const newSchemaVer = 6
+
+	testCases := []struct {
+		in      yobj
+		want    yobj
+		wantErr string
+		name    string
+	}{{
+		in: yobj{
+			"clients": []yobj{},
+		},
+		want: yobj{
+			"clients":        []yobj{},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "",
+		name:    "no_clients",
+	}, {
+		in: yobj{
+			"clients": []yobj{{"ip": "127.0.0.1"}},
+		},
+		want: yobj{
+			"clients": []yobj{{
+				"ids": []string{"127.0.0.1"},
+				"ip":  "127.0.0.1",
+			}},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "",
+		name:    "client_ip",
+	}, {
+		in: yobj{
+			"clients": []yobj{{"mac": "mac"}},
+		},
+		want: yobj{
+			"clients": []yobj{{
+				"ids": []string{"mac"},
+				"mac": "mac",
+			}},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "",
+		name:    "client_mac",
+	}, {
+		in: yobj{
+			"clients": []yobj{{"ip": "127.0.0.1", "mac": "mac"}},
+		},
+		want: yobj{
+			"clients": []yobj{{
+				"ids": []string{"127.0.0.1", "mac"},
+				"ip":  "127.0.0.1",
+				"mac": "mac",
+			}},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "",
+		name:    "client_ip_mac",
+	}, {
+		in: yobj{
+			"clients": []yobj{{"ip": 1, "mac": "mac"}},
+		},
+		want: yobj{
+			"clients":        []yobj{{"ip": 1, "mac": "mac"}},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "client.ip is not a string: 1",
+		name:    "inv_client_ip",
+	}, {
+		in: yobj{
+			"clients": []yobj{{"ip": "127.0.0.1", "mac": 1}},
+		},
+		want: yobj{
+			"clients":        []yobj{{"ip": "127.0.0.1", "mac": 1}},
+			"schema_version": newSchemaVer,
+		},
+		wantErr: "client.mac is not a string: 1",
+		name:    "inv_client_mac",
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := upgradeSchema5to6(tc.in)
+			testutil.AssertErrorMsg(t, tc.wantErr, err)
+			assert.Equal(t, tc.want, tc.in)
+		})
+	}
+}
+
 func TestUpgradeSchema7to8(t *testing.T) {
 	const host = "1.2.3.4"
 	oldConf := yobj{

scripts/make/go-lint.sh

@@ -161,11 +161,8 @@ run_linter "$GO" vet ./...
 run_linter govulncheck ./...
 
 # Apply more lax standards to the code we haven't properly refactored yet.
-run_linter gocyclo --over 13\
+run_linter gocyclo --over 13 ./internal/querylog
-	./internal/dhcpd\
+run_linter gocyclo --over 12 ./internal/dhcpd
-	./internal/home/\
-	./internal/querylog/\
-	;
 
 # Apply the normal standards to new or somewhat refactored code.
 run_linter gocyclo --over 10\
@@ -175,6 +172,7 @@ run_linter gocyclo --over 10\
 	./internal/aghtest/\
 	./internal/dnsforward/\
 	./internal/filtering/\
+	./internal/home/\
 	./internal/stats/\
 	./internal/tools/\
 	./internal/updater/\