fix: P0+P1 critical bug fixes across controller (24 files)
Concurrency fixes: - Deep-copy stacks in GetStack/GetStacks to prevent shared state mutation (C04) - Add per-state mutex to watchdog pathProbeState (C05) - Guard MetricsCollector.Start() with sync.Once against double-start (C06) - Hold diskJobMu across entire raw mount operation (C07) - Add mutex to SetEncryptionKey (C08), MigrateEncryption write lock (H03) - Use sync.Once for sync.Stop() channel close (H08) - Set syncing=true before releasing lock in TriggerSync (H09) - Deep-copy lastDBDump/lastBackup in GetFullStatus (H11) - Add WaitGroup for stderr goroutine in MigrateDrive (H19) - Add mutex to SetBackupRunningCheck (M18) Security fixes: - Validate Bearer token against Hub API key in CSRF middleware (H16) - Validate backup paths start with expected prefix in RemoveStack (M12) - Guard uuid[:8] slice with length check (H20) - Parse fstab fields exactly for mount target matching (H21) Bug fixes: - Use decrypted env vars for compose deploy (C01) - Log decrypt failures in DecryptMap instead of swallowing (C02) - Move Deployed=false inside lock in runComposeDeploy (C03) - Fix activeDrives() to skip disconnected drives (H02) - Fix Snapshot() stderr extraction from exec.ExitError (H01) - Check unlockCmd.Run() error in restic (H01) - Buffer template rendering via bytes.Buffer (H07) - Thread context.Context through cloudflare client (H10) - Fix leaf-name collision detection in cross-drive backup (H15) - Add nil check for crossDriveRunner (H17) - Use strings.TrimSpace instead of slice on command output (H18) - Make SaveAppConfig atomic with write-to-tmp+rename (H04) - Pass encKey on deploy failure SaveAppConfig (H05) - Fix IPv6 address format in TCP health probe Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -301,8 +301,14 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo
|
||||
}
|
||||
|
||||
// Step 5: Handle backup data cleanup
|
||||
backupsBase := filepath.Join(hddPath, felhomDataDir, "backups")
|
||||
for _, bkPath := range backupPathsToRemove {
|
||||
cleanPath := filepath.Clean(bkPath)
|
||||
// Validate path is under the expected backups directory
|
||||
if hddPath == "" || !strings.HasPrefix(cleanPath, backupsBase+string(filepath.Separator)) {
|
||||
m.logger.Printf("[WARN] Refusing to remove backup path outside expected directory: %s", cleanPath)
|
||||
continue
|
||||
}
|
||||
if _, err := os.Stat(cleanPath); os.IsNotExist(err) {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -298,7 +298,7 @@ func (m *Manager) runComposeDeploy(name, stackDir string, env map[string]string,
|
||||
|
||||
if composeErr != nil {
|
||||
m.logger.Printf("[ERROR] Stack %s deploy failed after %.1fs: %v", name, time.Since(start).Seconds(), composeErr)
|
||||
// Revert in-memory state
|
||||
// Revert in-memory and disk state
|
||||
m.mu.Lock()
|
||||
if s, ok := m.stacks[name]; ok {
|
||||
s.Deployed = false
|
||||
@@ -306,10 +306,12 @@ func (m *Manager) runComposeDeploy(name, stackDir string, env map[string]string,
|
||||
s.DeployError = composeErr.Error()
|
||||
s.AppConfig = nil
|
||||
}
|
||||
m.mu.Unlock()
|
||||
// Revert disk state — keep app.yaml for debugging but mark as not deployed
|
||||
// Also revert the shared appCfg under lock (C03 fix)
|
||||
appCfg.Deployed = false
|
||||
_ = SaveAppConfig(stackDir, appCfg, nil, nil)
|
||||
m.mu.Unlock()
|
||||
// Save reverted state to disk with encryption (H05 fix)
|
||||
meta := LoadMetadata(stackDir)
|
||||
_ = SaveAppConfig(stackDir, appCfg, m.encKey, SensitiveEnvVars(&meta))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -363,8 +365,10 @@ func (m *Manager) UpdateStackConfig(name string, values map[string]string) error
|
||||
return fmt.Errorf("saving updated config: %w", err)
|
||||
}
|
||||
|
||||
_, err := m.composeExecWithEnv(stackDir, appCfg.Env, "up", "-d")
|
||||
if err != nil {
|
||||
// Use stackEnv which loads decrypted values for docker compose (C01 fix).
|
||||
// appCfg.Env may contain encrypted values from LoadAppConfig.
|
||||
env := m.stackEnv(stackDir)
|
||||
if _, err := m.composeExecCustomEnv(stackDir, env, "up", "-d"); err != nil {
|
||||
return fmt.Errorf("restarting with new config: %w", err)
|
||||
}
|
||||
|
||||
@@ -552,8 +556,15 @@ func SaveAppConfig(stackDir string, cfg *AppConfig, encKey []byte, sensitiveVars
|
||||
path := filepath.Join(stackDir, "app.yaml")
|
||||
header := "# Auto-generated by felhom-controller — do not edit locked fields manually\n"
|
||||
content := header + string(data)
|
||||
if err := os.WriteFile(path, []byte(content), 0600); err != nil {
|
||||
return fmt.Errorf("writing %s: %w", path, err)
|
||||
|
||||
// Atomic write: write to .tmp then rename (H04 fix)
|
||||
tmpPath := path + ".tmp"
|
||||
if err := os.WriteFile(tmpPath, []byte(content), 0600); err != nil {
|
||||
return fmt.Errorf("writing %s: %w", tmpPath, err)
|
||||
}
|
||||
if err := os.Rename(tmpPath, path); err != nil {
|
||||
_ = os.Remove(tmpPath)
|
||||
return fmt.Errorf("renaming %s to %s: %w", tmpPath, path, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -165,7 +165,7 @@ func (m *Manager) runSingleCheck(containerName string, check HealthCheckItem) He
|
||||
// probeTCP tests if a TCP port is reachable on the container.
|
||||
func (m *Manager) probeTCP(containerName string, port int, target string) HealthCheckDetail {
|
||||
start := time.Now()
|
||||
addr := fmt.Sprintf("%s:%d", containerName, port)
|
||||
addr := net.JoinHostPort(containerName, fmt.Sprintf("%d", port))
|
||||
conn, err := net.DialTimeout("tcp", addr, 5*time.Second)
|
||||
latency := time.Since(start)
|
||||
|
||||
|
||||
@@ -112,6 +112,8 @@ func NewManager(cfg *config.Config, logger *log.Logger) (*Manager, error) {
|
||||
|
||||
// SetEncryptionKey sets the AES-256 key used to encrypt/decrypt sensitive values in app.yaml.
|
||||
func (m *Manager) SetEncryptionKey(key []byte) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
m.encKey = key
|
||||
}
|
||||
|
||||
@@ -121,8 +123,8 @@ func (m *Manager) MigrateEncryption() {
|
||||
if m.encKey == nil {
|
||||
return
|
||||
}
|
||||
m.mu.RLock()
|
||||
defer m.mu.RUnlock()
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
migrated := 0
|
||||
for _, s := range m.stacks {
|
||||
@@ -446,7 +448,7 @@ func (m *Manager) GetStacks() []Stack {
|
||||
|
||||
result := make([]Stack, 0, len(m.stacks))
|
||||
for _, s := range m.stacks {
|
||||
result = append(result, *s)
|
||||
result = append(result, deepCopyStack(s))
|
||||
}
|
||||
|
||||
// Sort alphabetically by display name for consistent UI ordering
|
||||
@@ -465,8 +467,53 @@ func (m *Manager) GetStack(name string) (*Stack, bool) {
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
copy := *s
|
||||
return ©, true
|
||||
cp := deepCopyStack(s)
|
||||
return &cp, true
|
||||
}
|
||||
|
||||
// deepCopyStack creates a deep copy of a Stack, including pointer fields.
|
||||
func deepCopyStack(s *Stack) Stack {
|
||||
cp := *s
|
||||
|
||||
// Deep-copy Containers slice
|
||||
if s.Containers != nil {
|
||||
cp.Containers = make([]ContainerInfo, len(s.Containers))
|
||||
copy(cp.Containers, s.Containers)
|
||||
}
|
||||
|
||||
// Deep-copy AppConfig pointer
|
||||
if s.AppConfig != nil {
|
||||
acCopy := *s.AppConfig
|
||||
if s.AppConfig.Env != nil {
|
||||
acCopy.Env = make(map[string]string, len(s.AppConfig.Env))
|
||||
for k, v := range s.AppConfig.Env {
|
||||
acCopy.Env[k] = v
|
||||
}
|
||||
}
|
||||
if s.AppConfig.LockedFields != nil {
|
||||
acCopy.LockedFields = make([]string, len(s.AppConfig.LockedFields))
|
||||
copy(acCopy.LockedFields, s.AppConfig.LockedFields)
|
||||
}
|
||||
cp.AppConfig = &acCopy
|
||||
}
|
||||
|
||||
// Deep-copy HealthProbe pointer
|
||||
if s.HealthProbe != nil {
|
||||
hpCopy := *s.HealthProbe
|
||||
if s.HealthProbe.Details != nil {
|
||||
hpCopy.Details = make([]HealthCheckDetail, len(s.HealthProbe.Details))
|
||||
copy(hpCopy.Details, s.HealthProbe.Details)
|
||||
}
|
||||
cp.HealthProbe = &hpCopy
|
||||
}
|
||||
|
||||
// Deep-copy Meta.DeployFields slice
|
||||
if s.Meta.DeployFields != nil {
|
||||
cp.Meta.DeployFields = make([]DeployField, len(s.Meta.DeployFields))
|
||||
copy(cp.Meta.DeployFields, s.Meta.DeployFields)
|
||||
}
|
||||
|
||||
return cp
|
||||
}
|
||||
|
||||
// --- Stack operations ---
|
||||
|
||||
Reference in New Issue
Block a user