From 8f97e180bb2c418983f762d20a34c5ebfd160c34 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 6 Oct 2026 21:19:43 +0000 Subject: [PATCH] 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 | 40 +++++++++-- internal/config/config.go | 67 +++++++++++++---- internal/config/config_test.go | 127 +++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index a545085..43e6225 100644 --- a/README.md +++ b/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, diff --git a/internal/config/config.go b/internal/config/config.go index 0f0ddc0..4dabe54 100644 --- a/internal/config/config.go +++ b/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): diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 30a97e1..00cf272 100644 --- a/internal/config/config_test.go +++ b/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()