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

A variable whose name starts with PIXA_ but is neither a setting's
variable, from the list pairing each config key with its variable, nor
PIXA_CONFIG_PATH now aborts startup naming it, as an unknown config key
does. PIXA_PORT is named with a pointer to PORT. The check runs before
the config file loads, so the variables the file's env section sets are
not checked. README.md says so under Configuration.

Model: opus-5-5

README.md

@@ -165,7 +165,7 @@ 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.
+variables set by the file's `env:` section are not checked.
 
 | Variable                             | Config key                      | Meaning                                                                      |
 | ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- |

TODO.md

@@ -34,8 +34,8 @@ exhaustion
   #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
+  `PORT`; the check runs before the config file loads, so the variables
-  the file's `env:` section sets are checked too; documented in
+  the file's `env:` section sets are not checked; 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`

internal/config/config.go

@@ -147,17 +147,15 @@ type Config struct {
 func New(_ fx.Lifecycle, params Params) (*Config, error) {
 	log := params.Logger.Get()
 
-	// Look for the config file under the project name (/etc/pixa/,
+	err := validateKnownEnvVars()
-	// ~/.config/pixa/), matching the /var/lib/pixa state directory,
-	// not under the daemon name pixad.
-	sc, err := loadConfigFile(log, "pixa")
 	if err != nil {
 		return nil, err
 	}
 
-	// Loading the config file sets the variables in its env section,
+	// Look for the config file under the project name (/etc/pixa/,
-	// so this also checks their names.
+	// ~/.config/pixa/), matching the /var/lib/pixa state directory,
-	err = validateKnownEnvVars()
+	// not under the daemon name pixad.
+	sc, err := loadConfigFile(log, "pixa")
 	if err != nil {
 		return nil, err
 	}
@@ -389,8 +387,9 @@ func envVarNames() map[string]string {
 // 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
+// New calls it before loading the config file, so the variables the
-// file's env section sets are checked too.
+// file's env section sets are not checked, just as validateKnownKeys
+// leaves that section's contents alone.
 func validateKnownEnvVars() error {
 	known := map[string]bool{"PIXA_CONFIG_PATH": true}
 

internal/config/env_internal_test.go

@@ -3,14 +3,10 @@ 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
@@ -123,23 +119,6 @@ func TestPixaPortAbortsStartupPointingToPort(t *testing.T) {
 // 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() {
@@ -153,60 +132,6 @@ func TestSettingVariablesAndConfigPathAreAccepted(t *testing.T) {
 	}
 }
 
-// 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) {