diff --git a/controller/internal/api/config_perms_test.go b/controller/internal/api/config_perms_test.go new file mode 100644 index 0000000..7b39a6c --- /dev/null +++ b/controller/internal/api/config_perms_test.go @@ -0,0 +1,39 @@ +package api + +import ( + "os" + "path/filepath" + "runtime" + "testing" +) + +// TestWriteConfig0600 asserts F8: controller.yaml is persisted 0600 (it holds infra secrets), even when +// the target file already existed with looser (0644) permissions. POSIX modes only — skipped on Windows. +func TestWriteConfig0600(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX file modes not represented on Windows") + } + dir := t.TempDir() + path := filepath.Join(dir, "controller.yaml") + + // Pre-create with world-readable 0644 to prove the helper tightens an existing file. + if err := os.WriteFile(path, []byte("old: true\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := writeConfig0600(path, []byte("hub:\n api_key: redacted\n")); err != nil { + t.Fatalf("writeConfig0600: %v", err) + } + + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if mode := fi.Mode().Perm(); mode != 0o600 { + t.Fatalf("config mode = %o, want 0600", mode) + } + // No leftover temp file. + if _, err := os.Stat(path + ".tmp"); !os.IsNotExist(err) { + t.Fatalf("temp file not cleaned up") + } +} diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index 878fe12..1920a1b 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -1011,26 +1011,15 @@ func (r *Router) configApply(w http.ResponseWriter, req *http.Request) { return } - // Write config: try atomic rename first, fall back to direct write - // (os.Rename fails on Docker bind mounts with "device or resource busy") - tmpPath := r.configPath + ".tmp" - if err := os.WriteFile(tmpPath, body, 0644); err != nil { - r.logger.Printf("[ERROR] [api] Config apply: failed to write temp file: %v", err) - writeJSON(w, http.StatusInternalServerError, apiResponse{OK: false, Error: "failed to write config"}) + // Write config 0600: it holds infra credentials (cf_api_token, cf_tunnel_token, hub api_key) in + // plaintext (F8), so it must not be world-readable. writeConfig0600 enforces the mode even when the + // target file already existed with looser perms (os.WriteFile does not chmod an existing file). + if err := writeConfig0600(r.configPath, body); err != nil { + r.logger.Printf("[ERROR] [api] Config apply: failed to write config: %v", err) + writeJSON(w, http.StatusInternalServerError, apiResponse{OK: false, Error: "failed to apply config"}) return } - if err := os.Rename(tmpPath, r.configPath); err != nil { - os.Remove(tmpPath) - // Rename failed (likely Docker bind mount) — write directly - if err := os.WriteFile(r.configPath, body, 0644); err != nil { - r.logger.Printf("[ERROR] [api] Config apply: failed to write config: %v", err) - writeJSON(w, http.StatusInternalServerError, apiResponse{OK: false, Error: "failed to apply config"}) - return - } - r.logger.Printf("[INFO] [api] Config apply: rename failed, wrote directly (bind mount)") - } - r.logger.Printf("[INFO] [api] Config applied from Hub (%d bytes), restart needed to take effect", len(body)) writeJSON(w, http.StatusOK, apiResponse{OK: true, Message: "Config applied. Restart controller to apply changes."}) @@ -1040,6 +1029,27 @@ func (r *Router) configApply(w http.ResponseWriter, req *http.Request) { } } +// writeConfig0600 writes config bytes to path with mode 0600, atomically when possible (tmp+rename), +// falling back to a direct write for Docker bind mounts (where os.Rename returns EBUSY). It ALWAYS +// enforces 0600 on the final file — even if it already existed with looser perms — because controller.yaml +// holds infra credentials in plaintext (F8); os.WriteFile only applies the mode when creating a new file. +func writeConfig0600(path string, body []byte) error { + tmpPath := path + ".tmp" + if err := os.WriteFile(tmpPath, body, 0600); err != nil { + return fmt.Errorf("writing temp config: %w", err) + } + if err := os.Rename(tmpPath, path); err != nil { + os.Remove(tmpPath) + if err := os.WriteFile(path, body, 0600); err != nil { // bind-mount fallback + return fmt.Errorf("writing config: %w", err) + } + } + if err := os.Chmod(path, 0600); err != nil { + return fmt.Errorf("chmod config 0600: %w", err) + } + return nil +} + func (r *Router) configHash(w http.ResponseWriter, _ *http.Request) { hash, err := config.FileHash(r.configPath) if err != nil {