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

@@ -335,16 +335,14 @@ 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, and its health
+names the file. `smallwebwaf` reads the file once, at start. Its contents are
-check reads only the files of `SWWAF_LISTEN_ADDR` and `SWWAF_UPSTREAM_URL`, each
+the value, less one newline at their end so that a file written with `echo` or
-time it runs. The file's contents are the value, less one newline at their end
+an editor works, and are checked as the setting's own value would be. Setting
-so that a file written with `echo` or an editor works, and are checked as the
+both the setting and its `_FILE` form, or naming a file that cannot be read,
-setting's own value would be. Setting both the setting and its `_FILE` form, or
+stops the start with a message naming the variable. The settings logged at start
-naming a file that cannot be read, stops the start with a message naming the
+name the file, and show a token given in one as `********`, as they show one
-variable. The settings logged at start name the file, and show a token given in
+given directly. `SWWAF_LOG_REMOTE_TLS_CA_FILE`, whose value names a file
-one as `********`, as they show one given directly.
+already, has no `_FILE` form.
-`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 a setting, such as
+  from settings given as files (the `_FILE` form of any 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,8 +298,7 @@ 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. `SWWAF_LOG_REMOTE_TLS_CA_FILE`, whose value names a
+  secrets and long lists.
-  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,10 +174,6 @@ 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 (
@@ -238,8 +234,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", defaultListenAddr),
+		ListenAddr:           env.address("SWWAF_LISTEN_ADDR", ":8080"),
-		UpstreamURL:          env.appURL("SWWAF_UPSTREAM_URL", defaultUpstreamURL),
+		UpstreamURL:          env.appURL("SWWAF_UPSTREAM_URL", "http://127.0.0.1:8081"),
 		InstanceName:         env.value("SWWAF_INSTANCE_NAME", hostname),
 		Observe:              env.observe("SWWAF_MODE", "enforce"),
 		TrustedProxies:       env.netblocks("SWWAF_TRUSTED_PROXIES", privateRanges),
@@ -303,24 +299,6 @@ 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,8 +23,6 @@ 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(
@@ -52,13 +50,13 @@ func healthCheck(ctx context.Context, lookupEnv func(string) (string, bool)) err
 	ctx, cancel := context.WithTimeout(ctx, healthCheckTimeout)
 	defer cancel()
 
-	listenAddr, upstreamURL, err := config.ListenAddrAndUpstreamURL(lookupEnv)
+	cfg, err := config.FromEnvironment(lookupEnv)
 	if err != nil {
 		return fmt.Errorf("invalid setting: %w", err)
 	}
 
 	// The settings have checked that the address has a port.
-	_, port, _ := net.SplitHostPort(listenAddr)
+	_, port, _ := net.SplitHostPort(cfg.ListenAddr)
 	health := "http://" + net.JoinHostPort("127.0.0.1", port) + proxy.HealthPath
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, health, http.NoBody)
@@ -77,7 +75,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(upstreamURL))
+	conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", appAddress(cfg.UpstreamURL))
 	if err != nil {
 		return fmt.Errorf("connect to the app: %w", err)
 	}

internal/smallwebwaf/healthcheck_test.go

@@ -6,8 +6,6 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -45,21 +43,6 @@ 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: ")
 
@@ -115,18 +98,3 @@ 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
-}