From 368094e15d9144172ee9bca96cdacc37cd3ec0d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Colomb?= <src@andre.colomb.de>
Date: Fri, 7 Jan 2022 11:02:12 +0100
Subject: [PATCH] cmd/syncthing: Always update config.xml with generate
 subcommand (ref #8090) (#8098)

* cmd/syncthing: Remove unnecessary function arguments.

The openGUI() function does not need a device ID to work, and there is
only one caller anyway which uses EmptyDeviceID.

The loadOrDefaultConfig() function is always called with the same
dummy values.

* cmd/syncthing: Avoid misleading info messages from monitor process.

In order to check whether panic reporting is enabled, the monitor
process utilizes the loadOrDefaultConfig() function.  In case there is
no config file yet, info messages may be logged during creation if the
config Wrapper, which is discarded immediately after.

Stop using the DefaultConfig() utility function from lib/syncthing and
directly generate a minimal config instead to avoid these.

Add comments to loadOrDefaultConfig() explaining its limited purpose.

* cmd/syncthing/generate: Always write updated config file.

Previously, an existing config file was left untouched unless either
of the --gui-user or --gui-password options was given.  Remove that
condition and simplify the checking code.
---
 cmd/syncthing/generate/generate.go | 15 ++++-----------
 cmd/syncthing/main.go              | 19 +++++++++++--------
 cmd/syncthing/monitor.go           |  4 +---
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/cmd/syncthing/generate/generate.go b/cmd/syncthing/generate/generate.go
index 8c5bd1323..4f5c2fa14 100644
--- a/cmd/syncthing/generate/generate.go
+++ b/cmd/syncthing/generate/generate.go
@@ -90,20 +90,13 @@ func Generate(confDir, guiUser, guiPassword string, noDefaultFolder bool) error
 	log.Println("Device ID:", myID)
 
 	cfgFile := locations.Get(locations.ConfigFile)
-	var cfg config.Wrapper
-	if _, err := os.Stat(cfgFile); err == nil {
-		if guiUser == "" && guiPassword == "" {
-			log.Println("WARNING: Config exists; will not overwrite.")
-			return nil
-		}
-
-		if cfg, _, err = config.Load(cfgFile, myID, events.NoopLogger); err != nil {
-			return fmt.Errorf("load config: %w", err)
-		}
-	} else {
+	cfg, _, err := config.Load(cfgFile, myID, events.NoopLogger)
+	if fs.IsNotExist(err) {
 		if cfg, err = syncthing.DefaultConfig(cfgFile, myID, events.NoopLogger, noDefaultFolder); err != nil {
 			return fmt.Errorf("create config: %w", err)
 		}
+	} else if err != nil {
+		return fmt.Errorf("load config: %w", err)
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go
index 3e3604d60..07deb9be8 100644
--- a/cmd/syncthing/main.go
+++ b/cmd/syncthing/main.go
@@ -329,7 +329,7 @@ func (options serveOptions) Run() error {
 	}
 
 	if options.BrowserOnly {
-		if err := openGUI(protocol.EmptyDeviceID); err != nil {
+		if err := openGUI(); err != nil {
 			l.Warnln("Failed to open web UI:", err)
 			os.Exit(svcutil.ExitError.AsInt())
 		}
@@ -406,8 +406,8 @@ func (options serveOptions) Run() error {
 	return nil
 }
 
-func openGUI(myID protocol.DeviceID) error {
-	cfg, err := loadOrDefaultConfig(myID, events.NoopLogger)
+func openGUI() error {
+	cfg, err := loadOrDefaultConfig()
 	if err != nil {
 		return err
 	}
@@ -452,7 +452,7 @@ func (e *errNoUpgrade) Error() string {
 }
 
 func checkUpgrade() (upgrade.Release, error) {
-	cfg, err := loadOrDefaultConfig(protocol.EmptyDeviceID, events.NoopLogger)
+	cfg, err := loadOrDefaultConfig()
 	if err != nil {
 		return upgrade.Release{}, err
 	}
@@ -471,7 +471,7 @@ func checkUpgrade() (upgrade.Release, error) {
 }
 
 func upgradeViaRest() error {
-	cfg, err := loadOrDefaultConfig(protocol.EmptyDeviceID, events.NoopLogger)
+	cfg, err := loadOrDefaultConfig()
 	if err != nil {
 		return err
 	}
@@ -711,12 +711,15 @@ func setupSignalHandling(app *syncthing.App) {
 	}()
 }
 
-func loadOrDefaultConfig(myID protocol.DeviceID, evLogger events.Logger) (config.Wrapper, error) {
+// loadOrDefaultConfig creates a temporary, minimal configuration wrapper if no file
+// exists.  As it disregards some command-line options, that should never be persisted.
+func loadOrDefaultConfig() (config.Wrapper, error) {
 	cfgFile := locations.Get(locations.ConfigFile)
-	cfg, _, err := config.Load(cfgFile, myID, evLogger)
+	cfg, _, err := config.Load(cfgFile, protocol.EmptyDeviceID, events.NoopLogger)
 
 	if err != nil {
-		cfg, err = syncthing.DefaultConfig(cfgFile, myID, evLogger, true)
+		newCfg := config.New(protocol.EmptyDeviceID)
+		return config.Wrap(cfgFile, newCfg, protocol.EmptyDeviceID, events.NoopLogger), nil
 	}
 
 	return cfg, err
diff --git a/cmd/syncthing/monitor.go b/cmd/syncthing/monitor.go
index 61f335e21..b30ece39b 100644
--- a/cmd/syncthing/monitor.go
+++ b/cmd/syncthing/monitor.go
@@ -20,11 +20,9 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/locations"
 	"github.com/syncthing/syncthing/lib/osutil"
-	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/svcutil"
 	"github.com/syncthing/syncthing/lib/sync"
 )
@@ -564,7 +562,7 @@ func childEnv() []string {
 // panicUploadMaxWait uploading panics...
 func maybeReportPanics() {
 	// Try to get a config to see if/where panics should be reported.
-	cfg, err := loadOrDefaultConfig(protocol.EmptyDeviceID, events.NoopLogger)
+	cfg, err := loadOrDefaultConfig()
 	if err != nil {
 		l.Warnln("Couldn't load config; not reporting crash")
 		return