Add PIMONITOR_API_KEY env var and warn about -api-key exposure

README.md

@@ -396,6 +396,14 @@ or CLI flags (`-listen`, `-log-level`, `-api-key`, `-config`, `-version`).
 Flags take precedence over the config file, which takes precedence over
 built-in defaults.
 
+The API key additionally reads the `PIMONITOR_API_KEY` environment variable,
+which sits between the config file and the flags in precedence. Set the key
+via `api_key` in the config file (kept at mode `640 root:pimonitor` by the
+installer), or via `PIMONITOR_API_KEY` from a systemd `EnvironmentFile=` with
+the same restricted permissions. **The `-api-key` flag is for local
+development only** — command lines are world-readable through
+`/proc/<pid>/cmdline`, so any local user could read the key.
+
 ## Development
 
 See [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) for how the system is put together

SECURITY.md

@@ -54,6 +54,13 @@ reporters and reviewers have context:
   per browser and persists it in `localStorage` (an accepted trade-off —
   anyone with access to the browser profile can read it, and without TLS
   the key is visible on the wire either way).
+- **Supply the API key through the config file or the environment, not the
+  command line.** `/etc/pimonitor/config.yaml` is kept at mode `640
+  root:pimonitor` by `install.sh`; alternatively set `PIMONITOR_API_KEY`,
+  which systemd can load from an `EnvironmentFile=` with equally restricted
+  permissions. The `-api-key` flag is a development convenience only: a
+  process's command line is world-readable via `/proc/<pid>/cmdline`, so a
+  key passed that way is exposed to every local user on the machine.
 - Shell-outs (`apt list --upgradable`, optional `vcgencmd measure_temp`) are
   invoked with fixed argument lists (no user input is interpolated into
   shell commands), to avoid command injection.

docs/API.md

@@ -12,11 +12,21 @@ existing integrations against `/api/v1/...` keep working.
 
 By default, no authentication is required — PiMonitor is meant to run on a
 trusted local network. If you set `api_key` in `config.yaml` (or the
-`-api-key` flag), every `/api/v1/...` request must include one of:
+`PIMONITOR_API_KEY` environment variable), every `/api/v1/...` request must
+include one of:
 
 - `Authorization: Bearer <api_key>`
 - `X-Api-Key: <api_key>`
 
+The key can be supplied three ways, in increasing precedence: `api_key` in
+the config file, the `PIMONITOR_API_KEY` environment variable, and the
+`-api-key` flag. **Prefer the config file** — `install.sh` restricts it to
+mode `640 root:pimonitor`. `PIMONITOR_API_KEY` is the deployment-friendly
+alternative (systemd `EnvironmentFile=` pointing at a file with the same
+restricted permissions). The `-api-key` flag is for local development only:
+command lines are world-readable via `/proc/<pid>/cmdline`, so every local
+user on the machine can read a key passed that way.
+
 Requests without a valid key receive `401 Unauthorized`. `GET /healthz` is
 never gated by the API key, so external health checks keep working
 regardless of authentication configuration.

docs/ARCHITECTURE.md

@@ -264,8 +264,16 @@ same warn/crit cutoffs the server-side alert engine evaluates against (`>=`), an
 ## Configuration (`internal/config`)
 
 `config.Load` resolves `Config` in strictly increasing precedence: **built-in defaults**
-(`Default()`) → **optional YAML file** (`-config`) → **CLI flags** (`-listen`,
-`-log-level`, `-api-key`). YAML decoding uses `KnownFields(true)`, so an unrecognized key
+(`Default()`) → **optional YAML file** (`-config`) → **environment**
+(`PIMONITOR_API_KEY`) → **CLI flags** (`-listen`, `-log-level`, `-api-key`). The
+environment layer exists only for the API key: the `-api-key` flag leaks the secret into
+the process list (`/proc/<pid>/cmdline` is world-readable), whereas `PIMONITOR_API_KEY`
+can be delivered by systemd's `EnvironmentFile=` from a root-only file, and the config
+file itself is kept at mode `640 root:pimonitor` by `install.sh`. An unset *or empty*
+`PIMONITOR_API_KEY` changes nothing, mirroring the empty-flag default, so exporting it
+blank cannot accidentally turn off an `api_key` set in the file. `Load` delegates to an
+unexported `load(args, lookupEnv)` so the precedence rules are testable without mutating
+the process environment. YAML decoding uses `KnownFields(true)`, so an unrecognized key
 (e.g. a typo like `api_kay`) fails config loading outright at startup instead of silently
 falling back to a default that could be security-relevant (e.g. no authentication because
 the intended `api_key` was never actually applied).

internal/config/config.go

@@ -338,16 +338,30 @@ type Result struct {
 	VersionRequested bool
 }
 
+// APIKeyEnvVar is the environment variable that overrides the REST API key.
+// It exists because the -api-key flag leaks the secret into the process
+// list (any local user can read /proc/<pid>/cmdline), while an environment
+// variable can be supplied by systemd via EnvironmentFile= from a
+// root-readable file. The config file remains the primary mechanism.
+const APIKeyEnvVar = "PIMONITOR_API_KEY"
+
 // Load resolves configuration from defaults, an optional YAML file
-// (-config), and flag overrides, in that order of increasing precedence.
+// (-config), the PIMONITOR_API_KEY environment variable, and flag
+// overrides, in that order of increasing precedence.
 func Load(args []string) (Result, error) {
+	return load(args, os.LookupEnv)
+}
+
+// load is Load with the environment injected, so tests can exercise the
+// precedence rules without mutating the process environment.
+func load(args []string, lookupEnv func(string) (string, bool)) (Result, error) {
 	cfg := Default()
 
 	fs := flag.NewFlagSet("pimonitor", flag.ContinueOnError)
 	configPath := fs.String("config", "", "path to YAML config file")
 	listenAddr := fs.String("listen", "", "override listen address, e.g. :8080")
 	logLevel := fs.String("log-level", "", "override log level (debug, info, warn, error)")
-	apiKey := fs.String("api-key", "", "override REST API key")
+	apiKey := fs.String("api-key", "", "override REST API key (development only: the value is visible in the process list to every local user; use api_key in the config file or "+APIKeyEnvVar+" instead)")
 	showVersion := fs.Bool("version", false, "print version and exit")
 
 	if err := fs.Parse(args); err != nil {
@@ -360,6 +374,16 @@ func Load(args []string) (Result, error) {
 		}
 	}
 
+	// The environment sits between the config file and the flags: it is the
+	// recommended override for deployments (systemd EnvironmentFile=), while
+	// an explicit -api-key on the command line still wins. An empty or unset
+	// variable changes nothing, mirroring how the empty flag default is
+	// treated, so exporting PIMONITOR_API_KEY="" cannot accidentally disable
+	// an api_key configured in the file.
+	if v, ok := lookupEnv(APIKeyEnvVar); ok && v != "" {
+		cfg.APIKey = v
+	}
+
 	if *listenAddr != "" {
 		cfg.ListenAddr = *listenAddr
 	}

internal/config/config_test.go

@@ -207,6 +207,78 @@ func TestLoad_APIKeyOverride(t *testing.T) {
 	}
 }
 
+// envFunc builds a lookupEnv stub for load() from a fixed environment.
+func envFunc(env map[string]string) func(string) (string, bool) {
+	return func(key string) (string, bool) {
+		v, ok := env[key]
+		return v, ok
+	}
+}
+
+func TestLoad_APIKeyFromEnv(t *testing.T) {
+	result, err := load(nil, envFunc(map[string]string{APIKeyEnvVar: "env-secret"}))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if result.Config.APIKey != "env-secret" {
+		t.Fatalf("APIKey = %q, want env-secret", result.Config.APIKey)
+	}
+}
+
+func TestLoad_APIKeyEnvOverridesConfigFile(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.yaml")
+	writeFile(t, path, "api_key: \"file-secret\"\n")
+
+	result, err := load([]string{"-config", path}, envFunc(map[string]string{APIKeyEnvVar: "env-secret"}))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if result.Config.APIKey != "env-secret" {
+		t.Fatalf("APIKey = %q, want env-secret (env must override the config file)", result.Config.APIKey)
+	}
+}
+
+func TestLoad_APIKeyFlagOverridesEnv(t *testing.T) {
+	result, err := load([]string{"-api-key", "flag-secret"}, envFunc(map[string]string{APIKeyEnvVar: "env-secret"}))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if result.Config.APIKey != "flag-secret" {
+		t.Fatalf("APIKey = %q, want flag-secret (flag must override the env)", result.Config.APIKey)
+	}
+}
+
+func TestLoad_APIKeyEmptyEnvKeepsConfigFileValue(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "config.yaml")
+	writeFile(t, path, "api_key: \"file-secret\"\n")
+
+	// An exported-but-empty PIMONITOR_API_KEY must not silently disable the
+	// authentication configured in the file.
+	result, err := load([]string{"-config", path}, envFunc(map[string]string{APIKeyEnvVar: ""}))
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if result.Config.APIKey != "file-secret" {
+		t.Fatalf("APIKey = %q, want file-secret", result.Config.APIKey)
+	}
+}
+
+// TestLoad_ReadsAPIKeyEnvVar covers the wiring of the exported Load to the
+// real process environment, which the load() tests above deliberately bypass.
+func TestLoad_ReadsAPIKeyEnvVar(t *testing.T) {
+	t.Setenv(APIKeyEnvVar, "env-secret")
+
+	result, err := Load(nil)
+	if err != nil {
+		t.Fatalf("Load: %v", err)
+	}
+	if result.Config.APIKey != "env-secret" {
+		t.Fatalf("APIKey = %q, want env-secret", result.Config.APIKey)
+	}
+}
+
 func TestValidate_DefaultIsValid(t *testing.T) {
 	if err := Default().Validate(); err != nil {
 		t.Fatalf("Default() must pass Validate(): %v", err)

packaging/pimonitor.example.yaml

@@ -57,6 +57,12 @@ pi_model_enabled: true
 # the config manually, apply the same permissions:
 #   sudo chown root:pimonitor /etc/pimonitor/config.yaml
 #   sudo chmod 640 /etc/pimonitor/config.yaml
+#
+# Instead of setting it here you can export PIMONITOR_API_KEY, which takes
+# precedence over this value; with systemd, point EnvironmentFile= at a file
+# with the same restricted permissions. Do NOT use the -api-key flag outside
+# local development: a process's command line is world-readable through
+# /proc/<pid>/cmdline, so every local user could read the key.
 api_key: ""
 
 # Color-coding thresholds for the dashboard (and the load-average gauge

packaging/pimonitor.service

@@ -12,6 +12,14 @@ ExecStart=/usr/local/bin/pimonitor -config /etc/pimonitor/config.yaml
 Restart=on-failure
 RestartSec=5
 
+# The REST API key can be set as api_key in the config file above, or kept
+# out of it entirely by uncommenting the line below and putting
+# PIMONITOR_API_KEY=<secret> into a root-only file (chown root:root, chmod
+# 600 - systemd reads it as PID 1, before dropping to the pimonitor user).
+# Never pass the key via the -api-key flag here: the command line of a
+# running process is world-readable through /proc/<pid>/cmdline.
+#EnvironmentFile=/etc/pimonitor/pimonitor.env
+
 # Hardening. PiMonitor only ever reads world-readable files under /proc,
 # /sys/class/thermal, /etc/os-release, and the apt cache under
 # /var/lib/apt/lists/ (via `apt list --upgradable`), plus binds a TCP