From 9cddc4447e11ab8eb2660b811416225f25cfa03a Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 11:01:24 +0000 Subject: [PATCH 1/4] Test that a config file pixa may not read aborts startup Of the places pixa looks for its config file on its own, a file in a directory pixa may not enter is passed over today and pixa starts without it. This test puts the config file in such a directory and expects startup to abort with an error naming the file. It is skipped when run as root, which may enter any directory. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index c269a38..8399aba 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -564,6 +564,60 @@ func TestMalformedConfigFileAbortsStartup(t *testing.T) { t.Logf("got expected error: %v", err) } +// TestConfigFileInDirectoryPixaMayNotEnterAbortsStartup checks that a +// config file pixa cannot read because it may not enter its directory +// aborts startup instead of being passed over. +func TestConfigFileInDirectoryPixaMayNotEnterAbortsStartup(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root may enter any directory") + } + + home := t.TempDir() + configDir := filepath.Join(home, ".config", "pixa-test-nonexistent-app") + configPath := filepath.Join(configDir, "config.yml") + + err := os.MkdirAll(configDir, 0o700) + if err != nil { + t.Fatalf("failed to create config directory: %v", err) + } + + err = os.WriteFile(configPath, []byte(signingKeyLine), 0o600) + if err != nil { + t.Fatalf("failed to write config: %v", err) + } + + err = os.Chmod(configDir, 0) + if err != nil { + t.Fatalf("failed to remove the config directory's permissions: %v", err) + } + + // Give the directory back its permissions so t.TempDir can remove it. + t.Cleanup(func() { + //nolint:gosec // G302: a directory needs its execute bit to be removed + _ = os.Chmod(configDir, 0o700) + }) + + // The ~/.config candidate is the only one that exists: the appname + // rules out /etc, and the working directory is empty. + t.Setenv("PIXA_CONFIG_PATH", "") + t.Setenv("HOME", home) + t.Chdir(t.TempDir()) + + log := slog.New(slog.DiscardHandler) + + sc, err := loadConfigFile(log, "pixa-test-nonexistent-app") + if err == nil { + t.Fatalf("config file pixa cannot read must abort startup, got config: %v", + sc) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), configPath) { + t.Errorf("error %q does not name the config file %s", err.Error(), configPath) + } +} + func TestEnsureStateDirCreatesDirectory(t *testing.T) { t.Parallel() -- 2.54.0 From aace8e78b345f1a1ae854bb2e934db465f7db424 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 11:02:21 +0000 Subject: [PATCH 2/4] Abort startup on a config file pixa cannot read (closes #176) Of the places pixa looks for its config file on its own, it passed over any place where os.Stat failed, so a file in a directory pixa may not enter was skipped without a word and pixa started on a later file or on the environment and defaults. Now only a file that does not exist is passed over; any other error aborts startup naming the file, as a file that does not parse already did. README.md says so where it gives the search order. Model: opus-5-5 --- README.md | 8 ++++---- TODO.md | 5 +++++ internal/config/config.go | 29 ++++++++++++++++++----------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 54de083..6e3e2d1 100644 --- a/README.md +++ b/README.md @@ -413,10 +413,10 @@ pixa finds: `/etc/pixa/config.yml`, `/etc/pixa/config.yaml`, `~/.config/pixa/config.yml`, `~/.config/pixa/config.yaml`, then `config.yml` and `config.yaml` in the working directory. A named file that does not exist, cannot be read or does not parse aborts startup. Of the files pixa looks for on -its own, one it finds but cannot read or parse aborts startup; one it cannot -find, for any reason, is passed over without a message, even when the file is -there in a directory pixa may not enter. With no file, pixa uses the environment -and the defaults. +its own, only one that does not exist is passed over, without a message. One +that pixa cannot read or parse aborts startup, naming the file. So does one in a +directory pixa may not enter, whether or not it is there, since pixa cannot +tell. With no file, pixa uses the environment and the defaults. | Variable | Config key | Meaning | | ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- | diff --git a/TODO.md b/TODO.md index 8a0673a..84e896f 100644 --- a/TODO.md +++ b/TODO.md @@ -48,6 +48,11 @@ P2: security: referer blacklist files and the eviction pass after it evicts them. No other test in `internal/imgcache` inserts a row by hand after starting the evictor. Test only. +- 2026-10-04 a config file pixa cannot read aborts startup (closes #176): of the + places pixa looks for its config file on its own, only one where the file does + not exist is passed over; any other error, such as a directory on the path + that pixa may not enter, aborts startup naming the file, as a file that does + not parse already did. - 2026-10-04 deployment guide and example Caddy config (closes #89): "Deployment" in `README.md` says what the reverse proxy in front of pixa must do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set diff --git a/internal/config/config.go b/internal/config/config.go index b8e041b..35254b3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,6 +4,7 @@ package config import ( "errors" "fmt" + "io/fs" "log/slog" "math" "net/netip" @@ -778,19 +779,25 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro for _, path := range configPaths { cleanPath := filepath.Clean(path) + // Only a config file that does not exist is skipped. One that + // cannot be read or does not parse is a fatal startup error. _, statErr := os.Stat(cleanPath) - if statErr == nil { - // A config file that exists but does not parse is a fatal - // startup error, never something to skip over. - sc, err := smartconfig.NewFromConfigPath(path) - if err != nil { - return nil, fmt.Errorf("failed to parse config file %s: %w", path, err) - } - - log.Info("loaded config file", "path", path) - - return sc, nil + if errors.Is(statErr, fs.ErrNotExist) { + continue } + + if statErr != nil { + return nil, fmt.Errorf("failed to read config file %s: %w", path, statErr) + } + + sc, err := smartconfig.NewFromConfigPath(path) + if err != nil { + return nil, fmt.Errorf("failed to parse config file %s: %w", path, err) + } + + log.Info("loaded config file", "path", path) + + return sc, nil } return nil, nil //nolint:nilnil // nil config is valid (use defaults) -- 2.54.0 From 52db3cf0c56953161441e5a3673a2bdb32c8338b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 13:21:13 +0000 Subject: [PATCH 3/4] Test a config file that links to itself and a HOME that is a file The test for a directory pixa may not enter is skipped as root, where the gate runs the tests. A config.yml in the working directory that is a symbolic link to itself fails os.Stat as root too, and must abort startup. A config path that runs through a file, such as one under a HOME of /dev/null, cannot hold a file and must be passed over like one that does not exist; today it aborts startup. Model: opus-5-5 --- .../config/config_validation_internal_test.go | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/internal/config/config_validation_internal_test.go b/internal/config/config_validation_internal_test.go index 8399aba..1800ab3 100644 --- a/internal/config/config_validation_internal_test.go +++ b/internal/config/config_validation_internal_test.go @@ -618,6 +618,62 @@ func TestConfigFileInDirectoryPixaMayNotEnterAbortsStartup(t *testing.T) { } } +// TestConfigFileLinkingToItselfAbortsStartup checks that a config file +// pixa cannot read for a reason other than not existing aborts startup, +// as root too: a symbolic link to itself fails with "too many levels of +// symbolic links". +func TestConfigFileLinkingToItselfAbortsStartup(t *testing.T) { + workDir := t.TempDir() + + err := os.Symlink("config.yml", filepath.Join(workDir, "config.yml")) + if err != nil { + t.Fatalf("failed to create symbolic link: %v", err) + } + + // Only the working directory's config.yml is there: the appname rules + // out /etc, and HOME is empty. + t.Setenv("PIXA_CONFIG_PATH", "") + t.Setenv("HOME", t.TempDir()) + t.Chdir(workDir) + + log := slog.New(slog.DiscardHandler) + + sc, err := loadConfigFile(log, "pixa-test-nonexistent-app") + if err == nil { + t.Fatalf("config file pixa cannot read must abort startup, got config: %v", + sc) + } + + t.Logf("got expected error: %v", err) + + if !strings.Contains(err.Error(), "config.yml") { + t.Errorf("error %q does not name the config file config.yml", err.Error()) + } +} + +// TestConfigPathThroughFileIsPassedOver checks that a config file path +// that runs through a file, such as one under a HOME of /dev/null, is +// passed over like one that does not exist, since no file can be there. +func TestConfigPathThroughFileIsPassedOver(t *testing.T) { + // No config file is there: the appname rules out /etc, HOME is + // /dev/null, and the working directory is empty. + t.Setenv("PIXA_CONFIG_PATH", "") + t.Setenv("HOME", os.DevNull) + t.Chdir(t.TempDir()) + + log := slog.New(slog.DiscardHandler) + + sc, err := loadConfigFile(log, "pixa-test-nonexistent-app") + if err != nil { + t.Fatalf("a config path through a file must be passed over, got error: %v", + err) + } + + if sc != nil { + t.Errorf("expected no config file, got config: %v", sc) + } +} + func TestEnsureStateDirCreatesDirectory(t *testing.T) { t.Parallel() -- 2.54.0 From c29b68e2323a1e1da5c771c640904dd7ee45e0c0 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 13:35:36 +0000 Subject: [PATCH 4/4] Pass over a config path that runs through a file Of the places pixa looks for its config file on its own, one whose path runs through a file, such as under a HOME of /dev/null, aborted startup with "not a directory", though no file can be there. It is now passed over like a file that does not exist, as README.md says. Model: opus-5-5 --- internal/config/config.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 35254b3..a9e583f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,6 +15,7 @@ import ( "sort" "strconv" "strings" + "syscall" "time" "git.eeqj.de/sneak/smartconfig" @@ -779,10 +780,12 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro for _, path := range configPaths { cleanPath := filepath.Clean(path) - // Only a config file that does not exist is skipped. One that - // cannot be read or does not parse is a fatal startup error. + // Only a config file that does not exist is skipped, including + // one whose path runs through a file, such as under a HOME of + // /dev/null. One that cannot be read or does not parse is a + // fatal startup error. _, statErr := os.Stat(cleanPath) - if errors.Is(statErr, fs.ErrNotExist) { + if errors.Is(statErr, fs.ErrNotExist) || errors.Is(statErr, syscall.ENOTDIR) { continue } -- 2.54.0