Settings given as files: the _FILE form of every setting (closes #87)

Every setting X may instead be given as a file that X_FILE names, read
once at start: its contents, less one trailing newline, are the value,
checked as X would be. X and X_FILE both set, or a file that cannot be
read, stops the start with a message naming the variable. The logged
settings name the file, and mask a token read from one.
SWWAF_LOG_REMOTE_TLS_CA_FILE, whose value is a file already, has no
_FILE form. README.md says so, with the token file example from
SPEC.md's Deployment.

Judgement call: an invalid value read from a file is named as X, not X_FILE.
Rule suppressed: gosec G304 on reading the named file, as for the CA file.

Model: opus-5-5

README.md

@@ -181,9 +181,10 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set, and the default rule file of
 
 ## Settings
 
-Each setting is an environment variable, and each has a default, so none has to
-be set. A setting that is set but invalid stops the start with a message naming
-it, and the effective settings are logged at start.
+Each setting is an environment variable, or a file one names (see "Settings
+given as files" below), and each has a default, so none has to be set. A setting
+that is set but invalid stops the start with a message naming it, and the
+effective settings are logged at start.
 
 - `SWWAF_LISTEN_ADDR` (default `:8080`): where `smallwebwaf` listens.
 - `SWWAF_UPSTREAM_URL` (default `http://127.0.0.1:8081`): the app, as `http` or
@@ -288,7 +289,8 @@ it, and the effective settings are logged at start.
 - `SWWAF_METRICS_TOKEN` (default unset): the token a scraper sends for the
   metrics, a long random value. While it is unset the metrics are off; one
   shorter than 32 characters stops the start. The settings logged at start show
-  `********` in its place.
+  `********` in its place. Given as a file, it can be kept out of the app's
+  reach (see "Settings given as files" below).
 - `SWWAF_METRICS_TOP_N` (default `50`): how many countries get series of their
   own in the metrics by country; the others are counted as `other`.
 - `SWWAF_RULES_DIR` (default `/etc/smallwebwaf/rules.d`): the directory of the
@@ -329,6 +331,36 @@ with their counters and history, and an IPv6 client is counted by its /64. A new
 client waits at most a second for its country, and at most 100,000 answers from
 GeoJS are kept, for 7 days each.
 
+### Settings given as files
+
+Any setting may instead be given as a file that holds its value: the variable
+named like the setting with `_FILE` added, such as `SWWAF_METRICS_TOKEN_FILE`,
+names the file. `smallwebwaf` reads the file once, at start. Its contents are
+the value, less one newline at their end so that a file written with `echo` or
+an editor works, and are checked as the setting's own value would be. Setting
+both the setting and its `_FILE` form, or naming a file that cannot be read,
+stops the start with a message naming the variable. The settings logged at start
+name the file, and show a token given in one as `********`, as they show one
+given directly. `SWWAF_LOG_REMOTE_TLS_CA_FILE`, whose value names a file
+already, has no `_FILE` form.
+
+The app starts with the same environment variables as `smallwebwaf`, so it can
+read a token given as one. A token given as a file is out of the app's reach
+only while the `smallwebwaf` user alone can read the file: make it on the host,
+owned by uid 65532, the `smallwebwaf` user, with mode `0400`, and mount the
+directory that holds it into the container read-only; the container sees the
+same owner and mode. For example, on the host:
+
+```sh
+mkdir -p /srv/app/tokens
+openssl rand -hex 32 > /srv/app/tokens/metrics
+chown 65532:65532 /srv/app/tokens/metrics
+chmod 0400 /srv/app/tokens/metrics
+```
+
+and for the container, `-v /srv/app/tokens:/etc/smallwebwaf/tokens:ro` and
+`-e SWWAF_METRICS_TOKEN_FILE=/etc/smallwebwaf/tokens/metrics`.
+
 ## Request log
 
 `smallwebwaf` writes one JSON object per line on stdout for every request,

internal/config/config.go

@@ -1,6 +1,7 @@
 // Package config reads smallwebwaf's settings. Every setting is an
-// environment variable whose name starts with SWWAF_, every setting has a
-// default, and this package is the one place they are read.
+// environment variable whose name starts with SWWAF_, or a file such a
+// variable names, every setting has a default, and this package is the
+// one place they are read.
 package config
 
 import (
@@ -154,8 +155,8 @@ type Config struct {
 	LogRemoteFacility int
 	LogRemoteAppName  string
 
-	// settings are the values read, as given or by default, for the
-	// log line at start.
+	// settings are the values read, as given or by default, and the
+	// files they were read from, for the log line at start.
 	settings []slog.Attr
 }
 
@@ -221,11 +222,14 @@ var (
 	errNotFacility   = errors.New("is not a syslog facility such as local0 or daemon")
 	errNotAppName    = errors.New(
 		"is not 1 to 48 printable ASCII characters without a space, such as gitea")
+	errSetTwice = errors.New("set only one of them")
 )
 
 // FromEnvironment reads the settings with lookupEnv, normally
-// os.LookupEnv. A setting that is not set takes its default. A setting
-// that is set but invalid is an error that names it.
+// os.LookupEnv. A setting may instead be given as a file: the variable
+// named by the setting's name with _FILE added names the file, which is
+// read now (see lookup). A setting that is not set takes its default. A
+// setting that is set but invalid is an error that names it.
 func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 	env := &environment{lookupEnv: lookupEnv}
 	hostname, _ := os.Hostname() // "" when the host has no name to give
@@ -316,8 +320,8 @@ type environment struct {
 // value returns a setting's value, or its default when it is not set,
 // and notes it for the log.
 func (e *environment) value(name, defaultValue string) string {
-	value, ok := e.lookupEnv(name)
-	if !ok {
+	value, set := e.lookup(name)
+	if !set {
 		value = defaultValue
 	}
 
@@ -326,6 +330,37 @@ func (e *environment) value(name, defaultValue string) string {
 	return value
 }
 
+// lookup returns a setting's value and whether it is set: the value of the
+// variable name, or the contents of the file that the variable name_FILE
+// names, less one newline at their end. It notes that file's path for the
+// log. Both variables set, or a file that cannot be read, is an error.
+func (e *environment) lookup(name string) (string, bool) {
+	value, set := e.lookupEnv(name)
+	fileName := name + "_FILE"
+
+	path, inFile := e.lookupEnv(fileName)
+	if !inFile {
+		return value, set
+	}
+
+	if set {
+		e.check(name, fmt.Errorf("is set, and so is %s; %w", fileName, errSetTwice))
+
+		return value, set
+	}
+
+	e.settings = append(e.settings, slog.String(fileName, path))
+
+	contents, err := os.ReadFile(path) //nolint:gosec // a file the admin names
+	if err != nil {
+		e.check(fileName, fmt.Errorf("cannot be read: %w", err))
+
+		return "", false
+	}
+
+	return strings.TrimSuffix(string(contents), "\n"), true
+}
+
 // check keeps the first error, naming the setting it is about.
 func (e *environment) check(name string, err error) {
 	if err != nil && e.err == nil {
@@ -484,7 +519,7 @@ func (e *environment) absolutePath(name, defaultValue string) string {
 // switches off what it guards; set, it must be at least minTokenLength
 // characters. Neither the log nor an error shows its value.
 func (e *environment) token(name string) string {
-	value, set := e.lookupEnv(name)
+	value, set := e.lookup(name)
 	if !set {
 		e.settings = append(e.settings, slog.String(name, ""))
 
@@ -515,9 +550,12 @@ func (e *environment) logRemoteURL(name string) *url.URL {
 }
 
 // certificates reads a setting that is the path of a file of PEM
-// certificates. Unset or empty, it is nil.
+// certificates. Unset or empty, it is nil. Its value names a file
+// already, so, unlike the other settings, it has no _FILE form.
 func (e *environment) certificates(name string) *x509.CertPool {
-	path := e.value(name, "")
+	path, _ := e.lookupEnv(name)
+	e.settings = append(e.settings, slog.String(name, path))
+
 	if path == "" {
 		return nil
 	}
@@ -552,9 +590,12 @@ func (e *environment) facility(name, defaultValue string) int {
 // lines are sent in, by default the instance name. Its value is checked
 // when it is set, and, while lines are sent, when it is the instance name.
 func (e *environment) appName(name, instanceName string, sending bool) string {
-	_, set := e.lookupEnv(name)
+	value, set := e.lookup(name)
+	if !set {
+		value = instanceName
+	}
 
-	value := e.value(name, instanceName)
+	e.settings = append(e.settings, slog.String(name, value))
 
 	switch {
 	case isAppName(value):

internal/config/config_test.go

@@ -722,6 +722,133 @@ func TestTokenIsLoggedMasked(t *testing.T) {
 	}
 }
 
+func TestSettingFromFileLosesOneNewlineAndNoMore(t *testing.T) {
+	t.Parallel()
+
+	for contents, want := range map[string]string{
+		token:          token,
+		token + "\n":   token,
+		token + "\n\n": token + "\n",
+		token + " \n":  token + " ",
+	} {
+		cfg := fromEnvironment(t, environment{
+			metricsToken + "_FILE": writeFile(t, contents),
+		})
+		if cfg.MetricsToken != want {
+			t.Errorf("file holding %q gave %s %q, want %q", contents, metricsToken,
+				cfg.MetricsToken, want)
+		}
+	}
+}
+
+func TestSettingFromFileIsCheckedAsTheSettingItself(t *testing.T) {
+	t.Parallel()
+
+	_, err := config.FromEnvironment(environment{
+		requestMaxBytes + "_FILE": writeFile(t, "lots\n"),
+	}.lookupEnv)
+
+	want := requestMaxBytes + `: "lots" is not a size such as 512K, 100M or 5G, or off`
+	if err == nil || err.Error() != want {
+		t.Errorf("error %v, want %s", err, want)
+	}
+
+	_, err = config.FromEnvironment(environment{
+		logRemoteURL:               remoteURL,
+		instanceName:               instance,
+		logRemoteAppName + "_FILE": writeFile(t, "my app\n"),
+	}.lookupEnv)
+
+	want = logRemoteAppName + `: "my app" is not 1 to 48 printable ASCII ` +
+		`characters without a space, such as gitea`
+	if err == nil || err.Error() != want {
+		t.Errorf("error %v, want %s", err, want)
+	}
+}
+
+func TestSettingAndItsFileBothSetStopsTheStart(t *testing.T) {
+	t.Parallel()
+
+	_, err := config.FromEnvironment(environment{
+		metricsToken:           token,
+		metricsToken + "_FILE": writeFile(t, token),
+	}.lookupEnv)
+
+	want := metricsToken + ": is set, and so is " + metricsToken +
+		"_FILE; set only one of them"
+	if err == nil || err.Error() != want {
+		t.Errorf("error %v, want %s", err, want)
+	}
+}
+
+func TestUnreadableSettingFileStopsTheStart(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	for _, path := range []string{filepath.Join(dir, "missing"), dir} {
+		_, err := config.FromEnvironment(environment{metricsToken + "_FILE": path}.lookupEnv)
+
+		want := metricsToken + "_FILE: cannot be read: "
+		if err == nil || !strings.HasPrefix(err.Error(), want) {
+			t.Errorf("%s: error %v, want one starting %s", path, err, want)
+		}
+	}
+}
+
+func TestTokenFromFileIsLoggedMaskedWithTheFile(t *testing.T) {
+	t.Parallel()
+
+	path := writeFile(t, token+"\n")
+	cfg := fromEnvironment(t, environment{metricsToken + "_FILE": path})
+
+	var out bytes.Buffer
+
+	slog.New(slog.NewJSONHandler(&out, nil)).Info("starting", "settings", cfg)
+
+	var line struct {
+		Settings map[string]string `json:"settings"`
+	}
+
+	err := json.Unmarshal(out.Bytes(), &line)
+	if err != nil {
+		t.Fatalf("decode %s: %v", out.Bytes(), err)
+	}
+
+	if strings.Contains(out.String(), token) ||
+		line.Settings[metricsToken] != "********" ||
+		line.Settings[metricsToken+"_FILE"] != path {
+		t.Errorf("the token is not logged masked, with its file %s: %s", path,
+			out.String())
+	}
+}
+
+func TestRemoteLogCAFileIsNotReadAsAFileInItsTurn(t *testing.T) {
+	t.Parallel()
+
+	cfg := fromEnvironment(t, environment{
+		logRemoteTLSCAFile + "_FILE": writeFile(t, "/nonexistent/ca.pem\n"),
+	})
+	if cfg.LogRemoteTLSCAs != nil {
+		t.Errorf("%s_FILE gave certificates", logRemoteTLSCAFile)
+	}
+}
+
+// writeFile writes contents to a file in a directory of its own, removed
+// when the test ends, and returns the file's path.
+func writeFile(t *testing.T, contents string) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "setting")
+
+	err := os.WriteFile(path, []byte(contents), 0o600)
+	if err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+
+	return path
+}
+
 func TestLogsEachSettingWithItsValue(t *testing.T) {
 	t.Parallel()