From 68684892d8fbd66f69b6fd19f7864395d6b6a074 Mon Sep 17 00:00:00 2001
From: kisfenyo <nagyfenyvesi.viktor@gmail.com>
Date: Sun, 14 Jun 2026 09:50:50 +0200
Subject: [PATCH] F8: persist controller.yaml as 0600 (holds infra secrets)

The Hub config-apply handler wrote controller.yaml 0644; it holds cf_api_token,
cf_tunnel_token and hub api_key in plaintext. New writeConfig0600 helper writes
0600 atomically (tmp+rename, bind-mount fallback) and chmods to enforce 0600 even
when the file pre-existed 0644 (os.WriteFile doesn't chmod existing files).
Test asserts mode 0600 (Linux; skipped on Windows). Setup path already used 0600.
---
 controller/internal/api/config_perms_test.go | 39 +++++++++++++++++
 controller/internal/api/router.go            | 44 ++++++++++++--------
 2 files changed, 66 insertions(+), 17 deletions(-)
 create mode 100644 controller/internal/api/config_perms_test.go

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 {