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. The health check reads only SWWAF_LISTEN_ADDR and
SWWAF_UPSTREAM_URL, so no other setting or file can fail it.

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

@@ -335,14 +335,16 @@ GeoJS are kept, for 7 days each.
 
 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.
+names the file. `smallwebwaf` reads the file once, at start, and its health
+check reads only the files of `SWWAF_LISTEN_ADDR` and `SWWAF_UPSTREAM_URL`, each
+time it runs. The file's 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

SPEC.md

@@ -32,7 +32,7 @@ from a directory of hand-editable text files.
 - Defence against traffic floods that saturate the host's network link. That
   needs help upstream of the host.
 - A web UI or a configuration file. Settings are environment variables. Apart
-  from settings given as files (the `_FILE` form of any setting, such as
+  from settings given as files (the `_FILE` form of a setting, such as
   `SWWAF_ADMIN_TOKEN_FILE`, and `SWWAF_LOG_REMOTE_TLS_CA_FILE`), its own state
   files and the lookup database, the only files read are the rule files, which
   hold one regex per line and nothing more elaborate.
@@ -298,7 +298,8 @@ it.
 - A list set to an empty value is an empty list, and replaces the default.
 - Every setting may instead be given as a file holding the value, named by the
   setting's name with `_FILE` added, such as `SWWAF_ADMIN_TOKEN_FILE`, for
-  secrets and long lists.
+  secrets and long lists. `SWWAF_LOG_REMOTE_TLS_CA_FILE`, whose value names a
+  file already, has no `_FILE` form.
 - Settings, including those given as files, are read once at start; changing one
   means restarting the container. The files `smallwebwaf` watches while it runs
   are its state files, its rule files and the lookup database.

internal/config/config.go

@@ -174,6 +174,10 @@ const (
 	minTokenLength = 32
 	// masked is what the log shows for a token that is set.
 	masked = "********"
+	// defaultListenAddr and defaultUpstreamURL are the defaults of
+	// SWWAF_LISTEN_ADDR and SWWAF_UPSTREAM_URL.
+	defaultListenAddr  = ":8080"
+	defaultUpstreamURL = "http://127.0.0.1:8081"
 )
 
 var (
@@ -234,8 +238,8 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 	env := &environment{lookupEnv: lookupEnv}
 	hostname, _ := os.Hostname() // "" when the host has no name to give
 	cfg := &Config{
-		ListenAddr:           env.address("SWWAF_LISTEN_ADDR", ":8080"),
-		UpstreamURL:          env.appURL("SWWAF_UPSTREAM_URL", "http://127.0.0.1:8081"),
+		ListenAddr:           env.address("SWWAF_LISTEN_ADDR", defaultListenAddr),
+		UpstreamURL:          env.appURL("SWWAF_UPSTREAM_URL", defaultUpstreamURL),
 		InstanceName:         env.value("SWWAF_INSTANCE_NAME", hostname),
 		Observe:              env.observe("SWWAF_MODE", "enforce"),
 		TrustedProxies:       env.netblocks("SWWAF_TRUSTED_PROXIES", privateRanges),
@@ -299,6 +303,24 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 	return cfg, nil
 }
 
+// ListenAddrAndUpstreamURL reads only SWWAF_LISTEN_ADDR and
+// SWWAF_UPSTREAM_URL, either of which may be given as a file, as
+// FromEnvironment does. The health check needs no other setting, so it
+// reads no other, nor a file that another names.
+func ListenAddrAndUpstreamURL(
+	lookupEnv func(string) (string, bool),
+) (string, *url.URL, error) {
+	env := &environment{lookupEnv: lookupEnv}
+	listenAddr := env.address("SWWAF_LISTEN_ADDR", defaultListenAddr)
+	upstreamURL := env.appURL("SWWAF_UPSTREAM_URL", defaultUpstreamURL)
+
+	if env.err != nil {
+		return "", nil, env.err
+	}
+
+	return listenAddr, upstreamURL, nil
+}
+
 // privateRanges are the private address ranges, the default trusted
 // proxies.
 const privateRanges = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"

internal/smallwebwaf/healthcheck.go

@@ -23,6 +23,8 @@ var errHealthEndpoint = errors.New("smallwebwaf's health endpoint answered")
 // smallwebwaf answers its health endpoint on 127.0.0.1, at the port in
 // SWWAF_LISTEN_ADDR, and the app accepts connections at the address in
 // SWWAF_UPSTREAM_URL. Otherwise it writes why to stderr and returns 1.
+// It reads no other setting, nor a file that another names, so neither
+// can fail it.
 // args are the arguments after `healthcheck`; it takes none, and given
 // one it names it on stderr and returns 1 without checking anything.
 func HealthCheck(
@@ -50,13 +52,13 @@ func healthCheck(ctx context.Context, lookupEnv func(string) (string, bool)) err
 	ctx, cancel := context.WithTimeout(ctx, healthCheckTimeout)
 	defer cancel()
 
-	cfg, err := config.FromEnvironment(lookupEnv)
+	listenAddr, upstreamURL, err := config.ListenAddrAndUpstreamURL(lookupEnv)
 	if err != nil {
 		return fmt.Errorf("invalid setting: %w", err)
 	}
 
 	// The settings have checked that the address has a port.
-	_, port, _ := net.SplitHostPort(cfg.ListenAddr)
+	_, port, _ := net.SplitHostPort(listenAddr)
 	health := "http://" + net.JoinHostPort("127.0.0.1", port) + proxy.HealthPath
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, health, http.NoBody)
@@ -75,7 +77,7 @@ func healthCheck(ctx context.Context, lookupEnv func(string) (string, bool)) err
 		return fmt.Errorf("%w %s", errHealthEndpoint, res.Status)
 	}
 
-	conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", appAddress(cfg.UpstreamURL))
+	conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", appAddress(upstreamURL))
 	if err != nil {
 		return fmt.Errorf("connect to the app: %w", err)
 	}

internal/smallwebwaf/healthcheck_test.go

@@ -6,6 +6,8 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -43,6 +45,21 @@ func TestHealthCheck(t *testing.T) {
 
 	wantHealthCheck(t, env, 0, "")
 
+	// The health check reads those two settings alone, here given as
+	// files: a removed or invalid token file, or an invalid value of
+	// another setting, does not fail it.
+	for _, other := range []struct{ name, value string }{
+		{"SWWAF_METRICS_TOKEN_FILE", filepath.Join(t.TempDir(), "removed")},
+		{"SWWAF_METRICS_TOKEN_FILE", writeFile(t, "too short\n")},
+		{"SWWAF_MODE", "neither"},
+	} {
+		wantHealthCheck(t, map[string]string{
+			listenAddr + "_FILE":  writeFile(t, ":"+port+"\n"),
+			upstreamURL + "_FILE": writeFile(t, app.URL+"\n"),
+			other.name:            other.value,
+		}, 0, "")
+	}
+
 	app.Close()
 	wantHealthCheck(t, env, 1, "unhealthy: connect to the app: ")
 
@@ -98,3 +115,18 @@ func wantHealthCheck(t *testing.T, env map[string]string, status int, message st
 			got, wrote, status, message)
 	}
 }
+
+// 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
+}