Abort startup on an unknown PIXA_ environment variable (closes #133) #136

Merged
clawbot merged 2 commits from issue-133-unknown-pixa-variables into next 2026-09-28 16:12:58 +02:00
4 changed files with 169 additions and 1 deletions
+5 -1
View File
@@ -161,7 +161,11 @@ process was started with and the file's own key. A variable's value is
parsed as the same text in the file would be. The three lists take
comma-separated entries, with the spaces around each trimmed; an empty
variable is an empty list. A value that does not parse or is invalid aborts
startup, naming the variable.
startup, naming the variable. A variable whose name starts with `PIXA_` but
is not in the table below, such as a misspelled one or `PIXA_PORT`, aborts
startup naming it, as an unknown config key does. The one other accepted
name is `PIXA_CONFIG_PATH`, the config file's path (like `--config`). The
variables set by the file's `env:` section are checked the same way.
| Variable | Config key | Meaning |
| ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- |
+7
View File
@@ -30,6 +30,13 @@ exhaustion
# Completed Steps
- 2026-09-28 unknown `PIXA_` environment variables abort startup (closes
#133): a variable whose name starts with `PIXA_` but is neither a
setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
an unknown config key does, and `PIXA_PORT` is named with a pointer to
`PORT`; the check runs after the config file loads, so the variables
the file's `env:` section sets are checked too; documented in
`README.md`.
- 2026-09-28 start on a fresh upaas volume (closes #129): the image
starts as root only to give `/var/lib/pixa` to `pixad` when `pixad`
does not own it (`deploy/docker-entrypoint.sh`), then runs the server
+45
View File
@@ -58,6 +58,7 @@ var (
errValueRequired = errors.New("a value is required")
errValueEmpty = errors.New("value must not be empty")
errUnknownConfigKeys = errors.New("unknown config keys")
errUnknownEnvVars = errors.New("unknown environment variables")
errNotAString = errors.New("not a string")
errNotAnInteger = errors.New("not an integer")
errNotABoolean = errors.New("not a boolean")
@@ -154,6 +155,13 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
return nil, err
}
// Loading the config file sets the variables in its env section,
// so this also checks their names.
err = validateKnownEnvVars()
if err != nil {
return nil, err
}
if sc == nil {
log.Info("no config file found, using environment variables and defaults")
}
@@ -377,6 +385,43 @@ func envVarNames() map[string]string {
}
}
// validateKnownEnvVars rejects environment variables whose names start
// with PIXA_ but that are neither a setting's variable nor
// PIXA_CONFIG_PATH, so a misspelled variable fails at startup instead of
// being silently ignored, as validateKnownKeys does for config file keys.
// New calls it after loading the config file, so the variables the
// file's env section sets are checked too.
func validateKnownEnvVars() error {
known := map[string]bool{"PIXA_CONFIG_PATH": true}
for _, name := range envVarNames() {
known[name] = true
}
var unknown []string
for _, entry := range os.Environ() {
name, _, _ := strings.Cut(entry, "=")
switch {
case !strings.HasPrefix(name, "PIXA_") || known[name]:
continue
case name == "PIXA_PORT":
unknown = append(unknown, name+" (use PORT for the port)")
default:
unknown = append(unknown, name)
}
}
if len(unknown) > 0 {
sort.Strings(unknown)
return fmt.Errorf("%w: %s", errUnknownEnvVars, strings.Join(unknown, ", "))
}
return nil
}
// lookupValue returns the value set for key and whether one is set. The
// key's environment variable wins when it is present, even when empty;
// its value is a string, read exactly as the same text quoted in the
+112
View File
@@ -3,10 +3,14 @@ package config
import (
"net/netip"
"os"
"path/filepath"
"reflect"
"slices"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger"
)
// TestMain unsets PORT and every PIXA_ environment variable before the
@@ -95,6 +99,114 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
}
}
// TestUnknownPixaVariableAbortsStartup checks that a PIXA_ variable that
// is not a setting's variable, such as a misspelled one, aborts startup
// naming it, as an unknown config key does, instead of being ignored.
func TestUnknownPixaVariableAbortsStartup(t *testing.T) {
t.Setenv("PIXA_TRUSTED_PROXY", "192.0.2.0/24")
t.Setenv("PIXA_SIGNINGKEY", validTestSigningKey)
err := validateKnownEnvVars()
wantStartupError(t, err, "PIXA_TRUSTED_PROXY", "PIXA_SIGNINGKEY")
}
// TestPixaPortAbortsStartupPointingToPort checks that PIXA_PORT aborts
// startup with a message saying to use PORT, which sets the port.
func TestPixaPortAbortsStartupPointingToPort(t *testing.T) {
t.Setenv("PIXA_PORT", "9090")
err := validateKnownEnvVars()
wantStartupError(t, err, "PIXA_PORT", "use PORT")
}
// TestSettingVariablesAndConfigPathAreAccepted checks that every
// setting's variable and PIXA_CONFIG_PATH pass the check for unknown
// PIXA_ variables. TestEnvironmentSetsEveryKey pins the names in the list.
func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
// A config file's env section loaded by another test can leave a
// PIXA_ variable set for the whole process, so every one is unset
// here first; t.Setenv restores each when the test ends.
for _, entry := range os.Environ() {
name, _, _ := strings.Cut(entry, "=")
if !strings.HasPrefix(name, "PIXA_") {
continue
}
t.Setenv(name, "")
err := os.Unsetenv(name)
if err != nil {
t.Fatalf("failed to unset %s: %v", name, err)
}
}
t.Setenv("PIXA_CONFIG_PATH", "/etc/pixa/config.yml")
for _, name := range envVarNames() {
t.Setenv(name, "")
}
err := validateKnownEnvVars()
if err != nil {
t.Fatalf("PIXA_CONFIG_PATH and every setting's variable "+
"must be accepted: %v", err)
}
}
// configFromNew writes yamlContent to a temporary config file, points
// PIXA_CONFIG_PATH at it, and runs New, as the server does at startup.
// The state directory is a temporary one and the disk cache is off, so
// New succeeds unless something in the test is wrong.
func configFromNew(t *testing.T, yamlContent string) (*Config, error) {
t.Helper()
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "config.yml")
err := os.WriteFile(configPath, []byte(yamlContent), 0o600)
if err != nil {
t.Fatalf("failed to write test config: %v", err)
}
t.Setenv("PIXA_CONFIG_PATH", configPath)
t.Setenv("PIXA_STATE_DIR", filepath.Join(tmpDir, "state"))
t.Setenv("PIXA_CACHE_MAX_BYTES", "0")
testLogger, err := logger.New(nil, logger.Params{Globals: &globals.Globals{}})
if err != nil {
t.Fatalf("failed to create logger: %v", err)
}
return New(nil, Params{Logger: testLogger})
}
// TestUnknownPixaVariableAbortsNew checks that New, which the server
// calls at startup, aborts on a misspelled PIXA_ variable.
func TestUnknownPixaVariableAbortsNew(t *testing.T) {
t.Setenv("PIXA_TRUSTED_PROXY", "192.0.2.0/24")
_, err := configFromNew(t, signingKeyLine)
wantStartupError(t, err, "PIXA_TRUSTED_PROXY")
}
// TestUnknownPixaVariableInEnvSectionAbortsNew checks that New aborts
// on a misspelled PIXA_ name in the config file's env section, which
// loading the file sets as an environment variable.
func TestUnknownPixaVariableInEnvSectionAbortsNew(t *testing.T) {
// The variable must be absent until the file loads. t.Setenv makes
// sure the one the file sets is removed when the test ends.
t.Setenv("PIXA_TRUSTED_PROXY", "")
err := os.Unsetenv("PIXA_TRUSTED_PROXY")
if err != nil {
t.Fatalf("failed to unset PIXA_TRUSTED_PROXY: %v", err)
}
_, err = configFromNew(t, signingKeyLine+
"env:\n PIXA_TRUSTED_PROXY: 192.0.2.0/24\n")
wantStartupError(t, err, "PIXA_TRUSTED_PROXY")
}
// TestPortFromEnvironmentOverridesConfigFile checks that PORT wins over
// the port in the config file.
func TestPortFromEnvironmentOverridesConfigFile(t *testing.T) {