diff --git a/backend/integration/crowdsec_lapi_integration_test.go b/backend/integration/crowdsec_lapi_integration_test.go index 615b66c51..721bad69f 100644 --- a/backend/integration/crowdsec_lapi_integration_test.go +++ b/backend/integration/crowdsec_lapi_integration_test.go @@ -532,8 +532,6 @@ func TestCrowdSecDiagnosticsConfig(t *testing.T) { optionalFields := []string{ "config_valid", "acquis_valid", - "config_path", - "acquis_path", } for _, field := range optionalFields { diff --git a/backend/internal/api/handlers/crowdsec_curated_apply_handler_test.go b/backend/internal/api/handlers/crowdsec_curated_apply_handler_test.go index 80d8059ee..46c26ff5e 100644 --- a/backend/internal/api/handlers/crowdsec_curated_apply_handler_test.go +++ b/backend/internal/api/handlers/crowdsec_curated_apply_handler_test.go @@ -123,7 +123,8 @@ func TestApplyCuratedPresetInstallsViaCSCLI(t *testing.T) { require.Equal(t, "curated-"+slug, resp["cache_key"]) backup, _ := resp["backup"].(string) require.NotEmpty(t, backup) - require.DirExists(t, backup) + require.Equal(t, filepath.Base(backup), backup, "response carries a name, not a path") + require.DirExists(t, filepath.Join(filepath.Dir(c.dataDir), backup)) preset, ok := crowdsec.FindPreset(slug) require.True(t, ok) @@ -133,7 +134,7 @@ func TestApplyCuratedPresetInstallsViaCSCLI(t *testing.T) { require.Len(t, events, 1) require.Equal(t, slug, events[0].Slug) require.Equal(t, "applied", events[0].Status) - require.Equal(t, backup, events[0].BackupPath) + require.Equal(t, backup, filepath.Base(events[0].BackupPath)) } func TestApplyCuratedPresetCSCLIUnavailableReturns503(t *testing.T) { @@ -180,7 +181,7 @@ func TestApplyCuratedPresetInstallFailureReturns500WithBackup(t *testing.T) { events := c.events() require.Len(t, events, 1) require.Equal(t, "failed", events[0].Status) - require.Equal(t, backup, events[0].BackupPath) + require.Equal(t, backup, filepath.Base(events[0].BackupPath)) require.Contains(t, events[0].Error, "install exploded") } diff --git a/backend/internal/api/handlers/crowdsec_data_mutation_test.go b/backend/internal/api/handlers/crowdsec_data_mutation_test.go index ebc3d1422..997cdf697 100644 --- a/backend/internal/api/handlers/crowdsec_data_mutation_test.go +++ b/backend/internal/api/handlers/crowdsec_data_mutation_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -104,8 +105,10 @@ func TestImportConfigReplacesConfigButPreservesEngineStateAndDataDir(t *testing. var resp map[string]any require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - backup, _ := resp["backup"].(string) - require.Equal(t, dir+".backup.", backup[:len(dir)+len(".backup.")]) + backupName, _ := resp["backup"].(string) + require.True(t, strings.HasPrefix(backupName, filepath.Base(dir)+".backup."), backupName) + require.Equal(t, filepath.Base(backupName), backupName, "response carries a name, not a path") + backup := filepath.Join(filepath.Dir(dir), backupName) after, err := os.Stat(dir) require.NoError(t, err) diff --git a/backend/internal/api/handlers/crowdsec_files.go b/backend/internal/api/handlers/crowdsec_files.go index e2bdf3f68..3b0a08a27 100644 --- a/backend/internal/api/handlers/crowdsec_files.go +++ b/backend/internal/api/handlers/crowdsec_files.go @@ -39,8 +39,10 @@ var ( ".yaml": {}, ".yml": {}, ".json": {}, ".txt": {}, ".conf": {}, } - // protectedWriteNames are credential files the editor never overwrites. - protectedWriteNames = map[string]struct{}{ + // hiddenSecretNames are lowercase base names of secret files that are neither listed nor readable nor + // writable through the editor. + hiddenSecretNames = map[string]struct{}{ + "bouncer_key": {}, "local_api_credentials.yaml": {}, "online_api_credentials.yaml": {}, } @@ -75,14 +77,14 @@ func cleanRelPath(raw string) (string, *fileError) { } // isReadable reports whether rel may be listed and read: engine-owned state (databases, the data and -// hub_cache trees) and secrets are hidden, everything else stays visible. +// hub_cache trees) and secrets (see hiddenSecretNames) are hidden, everything else stays visible. func isReadable(rel string) bool { slash := filepath.ToSlash(rel) if slash == "" || slash == "." || crowdsec.IsEngineOwnedPath(slash) { return false } base := strings.ToLower(path.Base(slash)) - if base == "bouncer_key" { + if _, hidden := hiddenSecretNames[base]; hidden { return false } if matched, _ := path.Match("*.db*", base); matched { @@ -102,9 +104,6 @@ func isWritable(rel string) bool { if _, ok := writableExtensions[strings.ToLower(path.Ext(base))]; !ok { return false } - if _, protected := protectedWriteNames[base]; protected { - return false - } if strings.HasPrefix(base, writeTempPrefix) { return false } @@ -412,7 +411,7 @@ func (h *CrowdsecHandler) WriteFile(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to write file"}) return } - c.JSON(http.StatusOK, gin.H{"status": "written", "backup": backupDir}) + c.JSON(http.StatusOK, gin.H{"status": "written", "backup": crowdsec.BackupID(backupDir)}) } // fileErrorLog returns a log entry carrying err with control characters diff --git a/backend/internal/api/handlers/crowdsec_files_test.go b/backend/internal/api/handlers/crowdsec_files_test.go index a1d0504bb..db25481d0 100644 --- a/backend/internal/api/handlers/crowdsec_files_test.go +++ b/backend/internal/api/handlers/crowdsec_files_test.go @@ -92,25 +92,31 @@ func errorOf(t *testing.T, w *httptest.ResponseRecorder) string { func TestCrowdsecFiles_Predicates(t *testing.T) { t.Parallel() readable := map[string]bool{ - "config.yaml": true, - "config/config.yaml": true, - "config/scenarios/x.yaml": true, - "config/acquis.d/a.conf": true, - "notes.md": true, - "config/data/nested.yaml": true, // only the top-level data/ is protected - "bouncer_key": false, - "config/bouncer_key": false, - "crowdsec.db": false, - "config/crowdsec.db-wal": false, - "data/crowdsec.db": false, - "data/GeoLite2-City.mmdb": false, - "hub_cache/a.tgz": false, - "config/other.db": false, - "config/other.db-journal": false, - "config/CASE.DB": false, - "data": false, - "hub_cache": false, - "config/hub_cache/thing.yaml": true, + "config.yaml": true, + "config/config.yaml": true, + "config/scenarios/x.yaml": true, + "config/acquis.d/a.conf": true, + "notes.md": true, + "config/data/nested.yaml": true, // only the top-level data/ is protected + "bouncer_key": false, + "config/bouncer_key": false, + "crowdsec.db": false, + "config/crowdsec.db-wal": false, + "data/crowdsec.db": false, + "data/GeoLite2-City.mmdb": false, + "hub_cache/a.tgz": false, + "config/other.db": false, + "config/other.db-journal": false, + "config/CASE.DB": false, + "data": false, + "hub_cache": false, + "local_api_credentials.yaml": false, + "config/local_api_credentials.yaml": false, + "config/online_api_credentials.yaml": false, + "config/Local_API_Credentials.YAML": false, + "config/ONLINE_API_CREDENTIALS.yaml": false, + "config/local_api_credentials.yaml.bak": true, + "config/hub_cache/thing.yaml": true, } for rel, want := range readable { assert.Equal(t, want, isReadable(rel), "isReadable(%q)", rel) @@ -493,10 +499,12 @@ func TestCrowdsecFiles_Write_BackupHoldsOnlyPreviousVersionAndIsCappedIndependen var resp map[string]string require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) require.NotEmpty(t, resp["backup"]) - prev, err := os.ReadFile(filepath.Join(resp["backup"], "config", "config.yaml")) // #nosec G304 -- test path + require.Equal(t, filepath.Base(resp["backup"]), resp["backup"], "response carries a name, not a path") + backupDir := filepath.Join(f.root, resp["backup"]) + prev, err := os.ReadFile(filepath.Join(backupDir, "config", "config.yaml")) // #nosec G304 -- test path require.NoError(t, err) assert.Equal(t, "v: 0\n", string(prev)) - _, err = os.Stat(filepath.Join(resp["backup"], "config", "other.yaml")) + _, err = os.Stat(filepath.Join(backupDir, "config", "other.yaml")) assert.True(t, os.IsNotExist(err), "backup must hold only the replaced file") for i := 2; i <= 14; i++ { @@ -689,3 +697,51 @@ func TestCrowdsecFiles_Write_FilesystemFailuresAre500(t *testing.T) { assert.NoFileExists(t, filepath.Join(cfg, "new.yaml")) }) } + +var credentialsFileNames = []string{ + "local_api_credentials.yaml", "online_api_credentials.yaml", + "Local_API_Credentials.yaml", "ONLINE_API_CREDENTIALS.YAML", +} + +func TestCrowdsecFiles_CredentialsHiddenFromListReadAndWrite(t *testing.T) { + t.Parallel() + f := newFilesFixture(t) + f.put("config/config.yaml", "a: 1\n") + for _, name := range credentialsFileNames { + f.put("config/"+name, "password: hunter2\n") + } + + assert.Equal(t, []string{filepath.Join("config", "config.yaml")}, f.list()) + + for _, name := range credentialsFileNames { + rel := "config/" + name + w := f.read(rel) + assert.Equal(t, http.StatusBadRequest, w.Code, rel) + assert.Equal(t, "invalid path", errorOf(t, w), rel) + assert.NotContains(t, w.Body.String(), "hunter2", rel) + + w = f.write(rel, "a: 1\n") + assert.Equal(t, http.StatusBadRequest, w.Code, rel) + assert.Equal(t, errFileCannotBeEdited, errorOf(t, w), rel) + } + + for _, name := range credentialsFileNames[:2] { + b, err := os.ReadFile(filepath.Join(f.dir, "config", name)) // #nosec G304 -- test path + require.NoError(t, err) + assert.Equal(t, "password: hunter2\n", string(b), "rejected write must not modify %s", name) + } +} + +func TestCrowdsecFiles_Read_SymlinkToCredentialsRefused(t *testing.T) { + t.Parallel() + f := newFilesFixture(t) + f.put("config/local_api_credentials.yaml", "password: hunter2\n") + require.NoError(t, os.Symlink( + filepath.Join(f.dir, "config", "local_api_credentials.yaml"), + filepath.Join(f.dir, "config", "innocent.yaml"))) + + w := f.read("config/innocent.yaml") + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Equal(t, "invalid path", errorOf(t, w)) + assert.NotContains(t, w.Body.String(), "hunter2") +} diff --git a/backend/internal/api/handlers/crowdsec_handler.go b/backend/internal/api/handlers/crowdsec_handler.go index 8adb5aa57..af97eea72 100644 --- a/backend/internal/api/handlers/crowdsec_handler.go +++ b/backend/internal/api/handlers/crowdsec_handler.go @@ -519,7 +519,8 @@ func (h *CrowdsecHandler) Start(c *gin.Context) { revertSetting := models.Setting{Key: "security.crowdsec.enabled", Value: "false", Category: "security", Type: "bool"} h.DB.Where(models.Setting{Key: "security.crowdsec.enabled"}).Assign(revertSetting).FirstOrCreate(&revertSetting) } - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("failed to start crowdsec") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to start CrowdSec"}) return } @@ -593,7 +594,8 @@ func (h *CrowdsecHandler) Start(c *gin.Context) { func (h *CrowdsecHandler) Stop(c *gin.Context) { ctx := c.Request.Context() if err := h.Executor.Stop(ctx, h.DataDir); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("failed to stop crowdsec") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to stop CrowdSec"}) return } @@ -621,7 +623,8 @@ func (h *CrowdsecHandler) Status(c *gin.Context) { ctx := c.Request.Context() running, pid, err := h.Executor.Status(ctx, h.DataDir) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("failed to read crowdsec status") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read CrowdSec status"}) return } @@ -679,7 +682,7 @@ func (h *CrowdsecHandler) ImportConfig(c *gin.Context) { } if err = validator.Validate(dst); err != nil { - c.JSON(http.StatusUnprocessableEntity, gin.H{"error": fmt.Sprintf("validation failed: %v", err)}) + c.JSON(http.StatusUnprocessableEntity, gin.H{"error": fmt.Sprintf("validation failed: %v", crowdsec.RedactPaths(err.Error()))}) return } @@ -711,8 +714,9 @@ func (h *CrowdsecHandler) ImportConfig(c *gin.Context) { // Extract archive extractErr := h.extractArchive(dst, h.DataDir) if extractErr != nil { + logger.Log().WithField("error", sanitizeForLog(extractErr.Error())).Warn("crowdsec import extraction failed") rollback() - c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("extraction failed: %v", extractErr)}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "extraction failed"}) return } @@ -720,11 +724,11 @@ func (h *CrowdsecHandler) ImportConfig(c *gin.Context) { configPath := filepath.Join(h.DataDir, "config.yaml") if err := validateYAMLFile(configPath); err != nil { rollback() - c.JSON(http.StatusUnprocessableEntity, gin.H{"error": fmt.Sprintf("config validation failed: %v", err)}) + c.JSON(http.StatusUnprocessableEntity, gin.H{"error": fmt.Sprintf("config validation failed: %v", crowdsec.RedactPaths(err.Error()))}) return } - c.JSON(http.StatusOK, gin.H{"status": "imported", "backup": backupDir}) + c.JSON(http.StatusOK, gin.H{"status": "imported", "backup": crowdsec.BackupID(backupDir)}) } // pruneSnapshots bounds the full-tree snapshots kept next to DataDir; failures are logged only. @@ -875,8 +879,9 @@ func (h *CrowdsecHandler) ExportConfig(c *gin.Context) { return nil }) if err != nil { + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("crowdsec export failed") // If any error occurred while creating the archive, return 500 - c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed to export crowdsec config"}) return } } @@ -922,7 +927,7 @@ func (h *CrowdsecHandler) ConsoleEnroll(c *gin.Context) { if h.Security != nil { _ = h.Security.LogAudit(&models.SecurityAudit{Actor: auditActor(c), Action: "crowdsec_console_enroll_failed", Details: fmt.Sprintf("status=%s tenant=%s agent=%s correlation_id=%s", status.Status, payload.Tenant, payload.AgentName, status.CorrelationID)}) } - resp := gin.H{"error": err.Error(), "status": status.Status} + resp := gin.H{"error": crowdsec.RedactPaths(err.Error()), "status": status.Status} if status.CorrelationID != "" { resp["correlation_id"] = status.CorrelationID } @@ -973,7 +978,7 @@ func (h *CrowdsecHandler) DeleteConsoleEnrollment(c *gin.Context) { ctx := c.Request.Context() if err := h.Console.ClearEnrollment(ctx); err != nil { logger.Log().WithError(err).Warn("failed to clear console enrollment state") - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to clear enrollment state"}) return } @@ -1972,7 +1977,7 @@ func (h *CrowdsecHandler) GetAcquisitionConfig(c *gin.Context) { content, err := readAcquisitionConfig(acquisPath) if err != nil { if errors.Is(err, os.ErrNotExist) { - c.JSON(http.StatusNotFound, gin.H{"error": "acquisition config not found", "path": acquisPath}) + c.JSON(http.StatusNotFound, gin.H{"error": "acquisition config not found"}) return } logger.Log().WithError(err).WithField("path", acquisPath).Warn("Failed to read acquisition config") @@ -1982,7 +1987,6 @@ func (h *CrowdsecHandler) GetAcquisitionConfig(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "content": string(content), - "path": acquisPath, }) } @@ -2027,7 +2031,7 @@ func (h *CrowdsecHandler) UpdateAcquisitionConfig(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "status": "updated", - "backup": backupPath, + "backup": crowdsec.BackupID(backupPath), "reload_hint": true, }) } @@ -2162,7 +2166,6 @@ func (h *CrowdsecHandler) DiagnosticsConfig(c *gin.Context) { if _, err := os.Stat(cleanConfigPath); err == nil { validation["config_exists"] = true - validation["config_path"] = cleanConfigPath // Read config and check LAPI port // #nosec G304 -- Path validated against DataDir above @@ -2207,7 +2210,6 @@ func (h *CrowdsecHandler) DiagnosticsConfig(c *gin.Context) { if _, err := os.Stat(cleanAcquisPath); err == nil { validation["acquis_exists"] = true - validation["acquis_path"] = cleanAcquisPath // Check if it has datasources // #nosec G304 -- Path validated against DataDir above @@ -2245,7 +2247,8 @@ func (h *CrowdsecHandler) ConsoleHeartbeat(c *gin.Context) { ctx := c.Request.Context() status, err := h.Console.Status(ctx) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("failed to read console heartbeat status") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read enrollment status"}) return } diff --git a/backend/internal/api/handlers/crowdsec_handler_coverage_test.go b/backend/internal/api/handlers/crowdsec_handler_coverage_test.go index 4bace2daf..209f497d1 100644 --- a/backend/internal/api/handlers/crowdsec_handler_coverage_test.go +++ b/backend/internal/api/handlers/crowdsec_handler_coverage_test.go @@ -43,7 +43,7 @@ func TestCrowdsec_Start_Error(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "failed to start crowdsec") + assert.Contains(t, w.Body.String(), "failed to start CrowdSec") } func TestCrowdsec_Stop_Error(t *testing.T) { @@ -61,7 +61,7 @@ func TestCrowdsec_Stop_Error(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "failed to stop crowdsec") + assert.Contains(t, w.Body.String(), "failed to stop CrowdSec") } func TestCrowdsec_Status_Error(t *testing.T) { @@ -79,7 +79,7 @@ func TestCrowdsec_Status_Error(t *testing.T) { r.ServeHTTP(w, req) assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "failed to get status") + assert.Contains(t, w.Body.String(), "failed to read CrowdSec status") } // ReadFile tests diff --git a/backend/internal/api/handlers/crowdsec_handler_test.go b/backend/internal/api/handlers/crowdsec_handler_test.go index 3fb468f79..18b555aaf 100644 --- a/backend/internal/api/handlers/crowdsec_handler_test.go +++ b/backend/internal/api/handlers/crowdsec_handler_test.go @@ -1082,7 +1082,7 @@ labels: require.Equal(t, http.StatusOK, w.Code) var resp map[string]any require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - require.Equal(t, acquisPath, resp["path"]) + require.NotContains(t, resp, "path") require.Equal(t, acquisContent, resp["content"]) } @@ -1323,7 +1323,7 @@ func TestCrowdsecHandler_Status_Error(t *testing.T) { r.ServeHTTP(w, req) require.Equal(t, http.StatusInternalServerError, w.Code) - require.Contains(t, w.Body.String(), "status check failed") + require.Contains(t, w.Body.String(), "failed to read CrowdSec status") } func TestCrowdsecHandler_Start_ExecutorError(t *testing.T) { @@ -1342,7 +1342,7 @@ func TestCrowdsecHandler_Start_ExecutorError(t *testing.T) { r.ServeHTTP(w, req) require.Equal(t, http.StatusInternalServerError, w.Code) - require.Contains(t, w.Body.String(), "failed to start process") + require.Contains(t, w.Body.String(), "failed to start CrowdSec") } func TestCrowdsecHandler_ExportConfig_DirNotFound(t *testing.T) { @@ -3249,9 +3249,10 @@ func TestCrowdsecHandler_WriteFile_BackupCreation(t *testing.T) { require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) require.Contains(t, resp, "backup") - backupPath := resp["backup"].(string) - require.NotEmpty(t, backupPath) - require.True(t, strings.HasPrefix(backupPath, tmpDir+".filebackup."), backupPath) + backupName := resp["backup"].(string) + require.True(t, strings.HasPrefix(backupName, filepath.Base(tmpDir)+".filebackup."), backupName) + require.Equal(t, filepath.Base(backupName), backupName, "response carries a name, not a path") + backupPath := filepath.Join(filepath.Dir(tmpDir), backupName) // Only the replaced file is backed up, with its previous content; DataDir stays in place. // #nosec G304 -- test reads a path it just created diff --git a/backend/internal/api/handlers/crowdsec_preset_event_test.go b/backend/internal/api/handlers/crowdsec_preset_event_test.go index b2f71891b..9c05cb946 100644 --- a/backend/internal/api/handlers/crowdsec_preset_event_test.go +++ b/backend/internal/api/handlers/crowdsec_preset_event_test.go @@ -62,10 +62,10 @@ func TestApplyResponseHelpers(t *testing.T) { gin.SetMode(gin.TestMode) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) - respondApplySuccess(c, crowdsec.ApplyResult{Status: "applied", BackupPath: "/b", ReloadHint: true, UsedCSCLI: true, CacheKey: "k", AppliedPreset: "s"}) + respondApplySuccess(c, crowdsec.ApplyResult{Status: "applied", BackupPath: "/data/crowdsec.backup.1", ReloadHint: true, UsedCSCLI: true, CacheKey: "k", AppliedPreset: "s"}) require.Equal(t, http.StatusOK, w.Code) - require.JSONEq(t, `{"status":"applied","backup":"/b","reload_hint":true,"used_cscli":true,"cache_key":"k","slug":"s"}`, w.Body.String()) + require.JSONEq(t, `{"status":"applied","backup":"crowdsec.backup.1","reload_hint":true,"used_cscli":true,"cache_key":"k","slug":"s"}`, w.Body.String()) require.Equal(t, gin.H{"error": "m"}, applyFailureBody("m", crowdsec.ApplyResult{})) - require.Equal(t, gin.H{"error": "m", "backup": "/b", "cache_key": "k"}, applyFailureBody("m", crowdsec.ApplyResult{BackupPath: "/b", CacheKey: "k"})) + require.Equal(t, gin.H{"error": "m", "backup": "crowdsec.backup.1", "cache_key": "k"}, applyFailureBody("m", crowdsec.ApplyResult{BackupPath: "/data/crowdsec.backup.1", CacheKey: "k"})) } diff --git a/backend/internal/api/handlers/crowdsec_preset_handler.go b/backend/internal/api/handlers/crowdsec_preset_handler.go index 66f2db62f..e22609ebf 100644 --- a/backend/internal/api/handlers/crowdsec_preset_handler.go +++ b/backend/internal/api/handlers/crowdsec_preset_handler.go @@ -168,7 +168,7 @@ func (h *CrowdsecHandler) PullPreset(c *gin.Context) { // control characters (0x00-0x1F, 0x7F) including CRLF // codeql[go/log-injection] logger.Log().WithField("error", util.SanitizeForLog(err.Error())).WithField("slug", util.SanitizeForLog(slug)).WithField("hub_base_url", util.SanitizeForLog(h.Hub.HubBaseURL)).Warn("crowdsec preset pull failed") - c.JSON(status, gin.H{"error": err.Error(), "hub_endpoints": h.hubEndpoints()}) + c.JSON(status, gin.H{"error": crowdsec.RedactPaths(err.Error()), "hub_endpoints": h.hubEndpoints()}) return } @@ -273,7 +273,7 @@ func (h *CrowdsecHandler) ApplyPreset(c *gin.Context) { logger.Log().WithField("error", util.SanitizeForLog(err.Error())).WithField("slug", util.SanitizeForLog(slug)).WithField("hub_base_url", util.SanitizeForLog(h.Hub.HubBaseURL)).WithField("backup_path", util.SanitizeForLog(res.BackupPath)).WithField("cache_key", util.SanitizeForLog(res.CacheKey)).Warn("crowdsec preset apply failed") h.recordPresetEvent(slug, res, err) // Build detailed error response - errorMsg := err.Error() + errorMsg := crowdsec.RedactPaths(err.Error()) // Add actionable guidance based on error type if errors.Is(err, crowdsec.ErrCacheMiss) || strings.Contains(errorMsg, "cache miss") { errorMsg = "Preset cache missing or expired. Pull the preset again, then retry apply." @@ -299,7 +299,7 @@ func (h *CrowdsecHandler) applyCuratedPreset(c *gin.Context, preset crowdsec.Pre h.recordPresetEvent(preset.Slug, res, err) status := mapCrowdsecStatus(err, http.StatusInternalServerError) - msg := err.Error() + msg := crowdsec.RedactPaths(err.Error()) if errors.Is(err, crowdsec.ErrCSCLIUnavailable) { status = http.StatusServiceUnavailable msg = "CrowdSec CLI is not available; curated presets require cscli" @@ -350,7 +350,7 @@ func (h *CrowdsecHandler) recordPresetEvent(slug string, res crowdsec.ApplyResul func applyFailureBody(msg string, res crowdsec.ApplyResult) gin.H { body := gin.H{"error": msg} if res.BackupPath != "" { - body["backup"] = res.BackupPath + body["backup"] = crowdsec.BackupID(res.BackupPath) } if res.CacheKey != "" { body["cache_key"] = res.CacheKey @@ -362,7 +362,7 @@ func applyFailureBody(msg string, res crowdsec.ApplyResult) gin.H { func respondApplySuccess(c *gin.Context, res crowdsec.ApplyResult) { c.JSON(http.StatusOK, gin.H{ "status": res.Status, - "backup": res.BackupPath, + "backup": crowdsec.BackupID(res.BackupPath), "reload_hint": res.ReloadHint, "used_cscli": res.UsedCSCLI, "cache_key": res.CacheKey, @@ -392,12 +392,14 @@ func (h *CrowdsecHandler) GetCachedPreset(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"error": "cache miss"}) return } - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + logger.Log().WithField("error", util.SanitizeForLog(err.Error())).WithField("slug", util.SanitizeForLog(slug)).Warn("failed to load crowdsec preset preview") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load preset preview"}) return } meta, metaErr := h.Hub.Cache.Load(ctx, slug) if metaErr != nil && !errors.Is(metaErr, crowdsec.ErrCacheMiss) && !errors.Is(metaErr, crowdsec.ErrCacheExpired) { - c.JSON(http.StatusInternalServerError, gin.H{"error": metaErr.Error()}) + logger.Log().WithField("error", util.SanitizeForLog(metaErr.Error())).WithField("slug", util.SanitizeForLog(slug)).Warn("failed to load crowdsec preset cache metadata") + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load preset cache metadata"}) return } cacheTTL := h.Hub.Cache.TTL() diff --git a/backend/internal/api/handlers/crowdsec_preset_handler_test.go b/backend/internal/api/handlers/crowdsec_preset_handler_test.go index 690cc8906..08a6da3fc 100644 --- a/backend/internal/api/handlers/crowdsec_preset_handler_test.go +++ b/backend/internal/api/handlers/crowdsec_preset_handler_test.go @@ -52,9 +52,9 @@ func TestListPresetsIncludesCacheAndIndex(t *testing.T) { require.NoError(t, err) hub := crowdsec.NewHubService(nil, cache, t.TempDir()) - hub.HubBaseURL = "http://example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: presetRoundTripper(func(req *http.Request) (*http.Response, error) { - if req.URL.String() == "http://example.com/api/index.json" { + if req.URL.String() == "https://hub.crowdsec.net/api/index.json" { return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(`{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","type":"collection"}]}`)), Header: make(http.Header)}, nil } return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -97,14 +97,14 @@ func TestPullPresetHandlerSuccess(t *testing.T) { archive := makePresetTar(t, map[string]string{"config.yaml": "key: value"}) hub := crowdsec.NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: presetRoundTripper(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://example.com/api/index.json": - return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(`{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","etag":"e1","download_url":"http://example.com/demo.tgz","preview_url":"http://example.com/demo.yaml"}]}`)), Header: make(http.Header)}, nil - case "http://example.com/demo.yaml": + case "https://hub.crowdsec.net/api/index.json": + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(`{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","etag":"e1","download_url":"https://hub.crowdsec.net/demo.tgz","preview_url":"https://hub.crowdsec.net/demo.yaml"}]}`)), Header: make(http.Header)}, nil + case "https://hub.crowdsec.net/demo.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("preview")), Header: make(http.Header)}, nil - case "http://example.com/demo.tgz": + case "https://hub.crowdsec.net/demo.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -187,7 +187,7 @@ func TestPullPresetHandlerHubError(t *testing.T) { require.NoError(t, err) hub := crowdsec.NewHubService(nil, cache, t.TempDir()) - hub.HubBaseURL = "http://example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: presetRoundTripper(func(req *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusBadGateway, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil })} @@ -213,7 +213,7 @@ func TestPullPresetHandlerTimeout(t *testing.T) { require.NoError(t, err) hub := crowdsec.NewHubService(nil, cache, t.TempDir()) - hub.HubBaseURL = "http://example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: presetRoundTripper(func(req *http.Request) (*http.Response, error) { return nil, context.DeadlineExceeded })} @@ -319,9 +319,9 @@ func TestApplyPresetHandlerBackupFailure(t *testing.T) { func TestListPresetsMergesCuratedAndHub(t *testing.T) { hub := crowdsec.NewHubService(nil, nil, t.TempDir()) - hub.HubBaseURL = "http://hub.example" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: presetRoundTripper(func(req *http.Request) (*http.Response, error) { - if req.URL.String() == "http://hub.example/api/index.json" { + if req.URL.String() == "https://hub.crowdsec.net/api/index.json" { return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(`{"items":[{"name":"crowdsecurity/custom","title":"Custom","description":"d","type":"collection"}]}`)), Header: make(http.Header)}, nil } return nil, errors.New("unexpected request") @@ -435,7 +435,7 @@ func TestGetCachedPresetPreviewError(t *testing.T) { r.ServeHTTP(w, req) require.Equal(t, http.StatusInternalServerError, w.Code) - require.Contains(t, w.Body.String(), "no such file") + require.Contains(t, w.Body.String(), "failed to load preset preview") } func TestPullCuratedPresetSkipsHub(t *testing.T) { diff --git a/backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go b/backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go index f714e42d2..6a277e062 100644 --- a/backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go +++ b/backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go @@ -37,16 +37,16 @@ func TestPullThenApplyIntegration(t *testing.T) { }) hub := crowdsec.NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.hub" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{ Transport: testRoundTripper(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.hub/api/index.json": - body := `{"items":[{"name":"test/preset","title":"Test","description":"Test preset","etag":"abc123","download_url":"http://test.hub/test.tgz","preview_url":"http://test.hub/test.yaml"}]}` + case "https://hub.crowdsec.net/api/index.json": + body := `{"items":[{"name":"test/preset","title":"Test","description":"Test preset","etag":"abc123","download_url":"https://hub.crowdsec.net/test.tgz","preview_url":"https://hub.crowdsec.net/test.yaml"}]}` return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://test.hub/test.yaml": + case "https://hub.crowdsec.net/test.yaml": return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader("preview content")), Header: make(http.Header)}, nil - case "http://test.hub/test.tgz": + case "https://hub.crowdsec.net/test.tgz": return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: 404, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -119,7 +119,7 @@ func TestApplyWithoutPullReturnsProperError(t *testing.T) { // Empty cache, no cscli hub := crowdsec.NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.hub" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: testRoundTripper(func(req *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil })} @@ -165,7 +165,7 @@ func TestApplyRollbackWhenCacheMissingAndRepullFails(t *testing.T) { require.NoError(t, err) hub := crowdsec.NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.hub" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: testRoundTripper(func(req *http.Request) (*http.Response, error) { // Force repull failure return &http.Response{StatusCode: 500, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil diff --git a/backend/internal/api/handlers/crowdsec_response_paths_test.go b/backend/internal/api/handlers/crowdsec_response_paths_test.go new file mode 100644 index 000000000..891c415e0 --- /dev/null +++ b/backend/internal/api/handlers/crowdsec_response_paths_test.go @@ -0,0 +1,259 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/Wikid82/charon/backend/internal/models" +) + +// serverPathPattern matches absolute server path prefixes that must never reach a client. +var serverPathPattern = regexp.MustCompile(`(^|[\s"'(=:])/(app|data|etc|tmp|var|home)(/|\b)`) + +// requireNoServerPaths fails when body leaks an absolute server path or any of the extra +// test-specific directories (typically t.TempDir()). +func requireNoServerPaths(t *testing.T, body string, extra ...string) { + t.Helper() + require.Falsef(t, serverPathPattern.MatchString(body), "response leaks a server path: %s", body) + for _, e := range extra { + require.NotContainsf(t, body, e, "response leaks %s: %s", e, body) + } +} + +func TestResponses_FileWriteCarriesBackupNameOnly(t *testing.T) { + f := newFilesFixture(t) + f.put("config/config.yaml", "v: 0\n") + + w := f.write("config/config.yaml", "v: 1\n") + require.Equal(t, http.StatusOK, w.Code) + requireNoServerPaths(t, w.Body.String(), f.root) + + var resp map[string]string + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Regexp(t, `^crowdsec\.filebackup\.\d{8}-\d{6}\.\d{6}$`, resp["backup"]) +} + +func TestResponses_ImportSuccessAndFailuresCarryNoPaths(t *testing.T) { + t.Parallel() + dir := filepath.Join(t.TempDir(), "crowdsec") + require.NoError(t, os.MkdirAll(dir, 0o750)) + seedEngineState(t, dir) + root := filepath.Dir(dir) + _, r := newImportRouter(t, dir) + + cases := []struct { + name string + files map[string]string + code int + }{ + {"success", map[string]string{"config.yaml": importableConfig}, http.StatusOK}, + {"extraction failure", map[string]string{"config.yaml": importableConfig, "a": "file", "a/b": "nested"}, http.StatusInternalServerError}, + {"validation failure", map[string]string{"other.yaml": "x"}, http.StatusUnprocessableEntity}, + {"config validation failure", map[string]string{"config.yaml": "not: valid"}, http.StatusUnprocessableEntity}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, tc.files)) + require.Equal(t, tc.code, w.Code, w.Body.String()) + requireNoServerPaths(t, w.Body.String(), root) + if tc.code == http.StatusInternalServerError { + require.JSONEq(t, `{"error":"extraction failed"}`, w.Body.String()) + } + }) + } +} + +func TestResponses_CuratedApplyCarriesNoPaths(t *testing.T) { + t.Run("success", func(t *testing.T) { + c := newCuratedHarness(t, nil) + code, resp := c.apply(t, "honeypot-friendly-defaults") + require.Equal(t, http.StatusOK, code) + body, _ := json.Marshal(resp) + requireNoServerPaths(t, string(body), filepath.Dir(c.dataDir)) + require.Regexp(t, `\.backup\.\d{8}-\d{6}\.\d{6}$`, resp["backup"]) + }) + + t.Run("rollback failure", func(t *testing.T) { + var c *curatedHarness + c = newCuratedHarness(t, func(_ context.Context, cmd string) ([]byte, error) { + if strings.Contains(cmd, " install ") { + // Make the restore fail too: DataDir becomes a regular file mid-apply. + require.NoError(t, os.RemoveAll(c.dataDir)) + require.NoError(t, os.WriteFile(c.dataDir, []byte("not a dir"), 0o600)) + return nil, context.DeadlineExceeded + } + return nil, nil + }) + code, resp := c.apply(t, "geoip-enrichment") + require.GreaterOrEqual(t, code, http.StatusInternalServerError) + body, _ := json.Marshal(resp) + require.Contains(t, string(body), "see server logs") + requireNoServerPaths(t, string(body), filepath.Dir(c.dataDir)) + }) +} + +func TestResponses_AcquisitionUpdateCarriesBackupNameOnly(t *testing.T) { + dir := t.TempDir() + acquis := filepath.Join(dir, "acquis.yaml") + require.NoError(t, os.WriteFile(acquis, []byte("old: true\n"), 0o600)) + t.Setenv("CHARON_CROWDSEC_ACQUIS_PATH", acquis) + + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", t.TempDir()) + r := gin.New() + r.PUT("/acquisition", h.UpdateAcquisitionConfig) + + body, _ := json.Marshal(map[string]string{"content": "source: file\n"}) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/acquisition", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + requireNoServerPaths(t, w.Body.String(), dir) + var resp map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Regexp(t, `^acquis\.yaml\.backup\.\d{8}-\d{6}$`, resp["backup"]) + require.FileExists(t, filepath.Join(dir, resp["backup"].(string))) +} + +func TestResponses_ExportErrorCarriesNoPaths(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("permission checks do not apply to root") + } + dir := t.TempDir() + secret := filepath.Join(dir, "unreadable.yaml") + require.NoError(t, os.WriteFile(secret, []byte("x"), 0o000)) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", dir) + r := gin.New() + h.RegisterRoutes(r.Group("/api/v1")) + + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/v1/admin/crowdsec/export", http.NoBody)) + + require.Equal(t, http.StatusInternalServerError, w.Code) + requireNoServerPaths(t, w.Body.String(), dir) + require.Contains(t, w.Body.String(), "failed to export crowdsec config") +} + +// pathErrExec fails every lifecycle call with an error that embeds an absolute server path. +type pathErrExec struct{} + +func (p *pathErrExec) err() error { + return &os.PathError{Op: "open", Path: "/app/data/crowdsec/crowdsec.pid", Err: os.ErrPermission} +} +func (p *pathErrExec) Start(context.Context, string, string) (int, error) { return 0, p.err() } +func (p *pathErrExec) Stop(context.Context, string) error { return p.err() } +func (p *pathErrExec) Status(context.Context, string) (running bool, pid int, err error) { + return false, 0, p.err() +} + +func serveCrowdsec(h *CrowdsecHandler, method, target string) *httptest.ResponseRecorder { + r := gin.New() + h.RegisterRoutes(r.Group("/api/v1")) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(method, target, http.NoBody)) + return w +} + +func TestResponses_AcquisitionGetCarriesNoPath(t *testing.T) { + dir := t.TempDir() + acquis := filepath.Join(dir, "acquis.yaml") + t.Setenv("CHARON_CROWDSEC_ACQUIS_PATH", acquis) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", t.TempDir()) + + w := serveCrowdsec(h, http.MethodGet, "/api/v1/admin/crowdsec/acquisition") + require.Equal(t, http.StatusNotFound, w.Code) + requireNoServerPaths(t, w.Body.String(), dir) + require.JSONEq(t, `{"error":"acquisition config not found"}`, w.Body.String()) + + require.NoError(t, os.WriteFile(acquis, []byte("source: file\n"), 0o600)) + w = serveCrowdsec(h, http.MethodGet, "/api/v1/admin/crowdsec/acquisition") + require.Equal(t, http.StatusOK, w.Code) + requireNoServerPaths(t, w.Body.String(), dir) + require.JSONEq(t, `{"content":"source: file\n"}`, w.Body.String()) +} + +func TestResponses_DiagnosticsConfigCarriesNoPaths(t *testing.T) { + dataDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dataDir, "config"), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(dataDir, "config", "config.yaml"), []byte("listen_uri: 127.0.0.1:8080\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dataDir, "config", "acquis.yaml"), []byte("source: file\nfilenames: [a]\n"), 0o600)) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", dataDir) + + w := serveCrowdsec(h, http.MethodGet, "/api/v1/admin/crowdsec/diagnostics/config") + require.Equal(t, http.StatusOK, w.Code) + requireNoServerPaths(t, w.Body.String(), dataDir) + var resp map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Equal(t, true, resp["config_exists"]) + require.Equal(t, true, resp["acquis_exists"]) + require.NotContains(t, resp, "config_path") + require.NotContains(t, resp, "acquis_path") +} + +func TestResponses_LifecycleFailuresReturnFixedMessages(t *testing.T) { + dataDir := t.TempDir() + h := newTestCrowdsecHandler(t, setupCrowdDB(t), &pathErrExec{}, "/bin/false", dataDir) + cases := []struct { + name, method, target, want string + }{ + {"start", http.MethodPost, "/api/v1/admin/crowdsec/start", "failed to start CrowdSec"}, + {"stop", http.MethodPost, "/api/v1/admin/crowdsec/stop", "failed to stop CrowdSec"}, + {"status", http.MethodGet, "/api/v1/admin/crowdsec/status", "failed to read CrowdSec status"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := serveCrowdsec(h, tc.method, tc.target) + require.Equal(t, http.StatusInternalServerError, w.Code, w.Body.String()) + requireNoServerPaths(t, w.Body.String(), dataDir) + require.JSONEq(t, `{"error":"`+tc.want+`"}`, w.Body.String()) + }) + } +} + +func TestResponses_ConsoleFailuresReturnFixedMessages(t *testing.T) { + t.Setenv("FEATURE_CROWDSEC_CONSOLE_ENROLLMENT", "true") + h := setupTestConsoleEnrollment(t) + require.NoError(t, h.DB.Migrator().DropTable(&models.CrowdsecConsoleEnrollment{})) + + cases := []struct { + name, method, target, want string + }{ + {"status", http.MethodGet, "/api/v1/admin/crowdsec/console/status", "failed to read enrollment status"}, + {"heartbeat", http.MethodGet, "/api/v1/admin/crowdsec/console/heartbeat", "failed to read enrollment status"}, + {"clear", http.MethodDelete, "/api/v1/admin/crowdsec/console/enrollment", "failed to clear enrollment state"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := serveCrowdsec(h, tc.method, tc.target) + require.Equal(t, http.StatusInternalServerError, w.Code, w.Body.String()) + requireNoServerPaths(t, w.Body.String()) + require.JSONEq(t, `{"error":"`+tc.want+`"}`, w.Body.String()) + }) + } +} + +func TestResponses_PresetCacheReadFailureReturnsFixedMessage(t *testing.T) { + dataDir := t.TempDir() + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", dataDir) + meta, err := h.Hub.Cache.Store(context.Background(), "demo", "etag", "hub", "preview", []byte("x")) + require.NoError(t, err) + require.NoError(t, os.Remove(meta.PreviewPath)) + + w := serveCrowdsec(h, http.MethodGet, "/api/v1/admin/crowdsec/presets/cache/demo") + require.Equal(t, http.StatusInternalServerError, w.Code, w.Body.String()) + requireNoServerPaths(t, w.Body.String(), dataDir) + require.JSONEq(t, `{"error":"failed to load preset preview"}`, w.Body.String()) +} diff --git a/backend/internal/api/routes/crowdsec_export_gzip_test.go b/backend/internal/api/routes/crowdsec_export_gzip_test.go new file mode 100644 index 000000000..2a5a65f60 --- /dev/null +++ b/backend/internal/api/routes/crowdsec_export_gzip_test.go @@ -0,0 +1,72 @@ +package routes + +import ( + "archive/tar" + "compress/gzip" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/Wikid82/charon/backend/internal/api/handlers" + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +// TestCrowdsecExport_NotDoubleCompressed guards against the router-wide gzip middleware +// wrapping the export handler's own tar.gz stream a second time. +func TestCrowdsecExport_NotDoubleCompressed(t *testing.T) { + gin.SetMode(gin.TestMode) + db, err := gorm.Open(sqlite.Open(isolatedMemoryDSN(t)), &gorm.Config{}) + require.NoError(t, err) + + dataDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dataDir, "config"), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(dataDir, "config", "config.yaml"), []byte("test: config\n"), 0o600)) + + h := handlers.NewCrowdsecHandler(db, nil, "/bin/false", dataDir) + t.Cleanup(func() { + if h.Security != nil { + h.Security.Close() + } + }) + + r := gin.New() + r.Use(compressionMiddleware()) + h.RegisterRoutes(r.Group("/api/v1")) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crowdsec/export", http.NoBody) + req.Header.Set("Accept-Encoding", "gzip") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, "application/gzip", w.Header().Get("Content-Type")) + require.Contains(t, w.Header().Get("Content-Disposition"), "attachment; filename=crowdsec-config-") + + // Decode transport compression exactly as a client would (honouring Content-Encoding). + var body io.Reader = w.Body + if w.Header().Get("Content-Encoding") == "gzip" { + zr, zerr := gzip.NewReader(body) + require.NoError(t, zerr) + body = zr + } + // What remains must be the tar.gz file itself. + gr, err := gzip.NewReader(body) + require.NoError(t, err, "download is not a single-layer tar.gz") + tr := tar.NewReader(gr) + var names []string + for { + hdr, nerr := tr.Next() + if nerr == io.EOF { + break + } + require.NoError(t, nerr) + names = append(names, hdr.Name) + } + require.Contains(t, names, filepath.Join("config", "config.yaml")) +} diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index de1321d25..fa4ca3184 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -210,7 +210,7 @@ func RegisterWithDeps(ctx context.Context, router *gin.Engine, db *gorm.DB, cfg router.Use(middleware.EmergencyBypass(cfg.Security.ManagementCIDRs, db)) // Enable gzip compression for API responses (reduces payload size ~70%) - router.Use(gzip.Gzip(gzip.DefaultCompression)) + router.Use(compressionMiddleware()) // Apply security headers middleware globally // This sets CSP, HSTS, X-Frame-Options, etc. @@ -1253,3 +1253,13 @@ func logAuthThrottlePolicy(st middleware.AuthRateLimitStatus) { config.EnvAuthRateLimitEnabled) } } + +// precompressedPaths lists routes whose handlers already emit a gzip stream (tar.gz downloads). +// They must bypass the router-wide gzip middleware, otherwise the body is compressed twice +// and the downloaded archive cannot be extracted. +var precompressedPaths = []string{"/api/v1/admin/crowdsec/export"} + +// compressionMiddleware returns the router-wide gzip middleware, skipping precompressed downloads. +func compressionMiddleware() gin.HandlerFunc { + return gzip.Gzip(gzip.DefaultCompression, gzip.WithExcludedPaths(precompressedPaths)) +} diff --git a/backend/internal/crowdsec/archive_error_paths_test.go b/backend/internal/crowdsec/archive_error_paths_test.go new file mode 100644 index 000000000..70264f739 --- /dev/null +++ b/backend/internal/crowdsec/archive_error_paths_test.go @@ -0,0 +1,91 @@ +package crowdsec + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "errors" + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestWithoutPath(t *testing.T) { + t.Parallel() + cause := errors.New("permission denied") + require.Equal(t, cause, withoutPath(&fs.PathError{Op: "open", Path: "/secret/x", Err: cause})) + require.Equal(t, cause, withoutPath(&os.LinkError{Op: "rename", Old: "/a", New: "/b", Err: cause})) + require.Equal(t, cause, withoutPath(cause)) +} + +func TestCreateFileNoSymlinkMkdirParentFailure(t *testing.T) { + t.Parallel() + skipIfRoot(t) + root := t.TempDir() + // parentMode 0 lets MkdirAll create "a" but not its child, so the mkdir fails. + dest := filepath.Join(root, "a", "b", "c.txt") + t.Cleanup(func() { _ = os.Chmod(filepath.Join(root, "a"), 0o700) }) //nolint:gosec // G302: restore dir access so TempDir cleanup works + f, err := CreateFileNoSymlink(root, dest, 0o000, 0o600) + require.Nil(t, f) + require.ErrorContains(t, err, "mkdir parent") + require.NotContains(t, err.Error(), root) +} + +func tarGzRaw(t *testing.T, build func(tw *tar.Writer)) []byte { + t.Helper() + buf := &bytes.Buffer{} + gw := gzip.NewWriter(buf) + tw := tar.NewWriter(gw) + build(tw) + require.NoError(t, gw.Close()) + return buf.Bytes() +} + +func TestExtractTarGzDirMkdirFailure(t *testing.T) { + t.Parallel() + skipIfRoot(t) + target := t.TempDir() + t.Cleanup(func() { _ = os.Chmod(filepath.Join(target, "a"), 0o700) }) //nolint:gosec // G302: restore dir access so TempDir cleanup works + archive := tarGzRaw(t, func(tw *tar.Writer) { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: "a/", Typeflag: tar.TypeDir, Mode: 0o000})) + require.NoError(t, tw.WriteHeader(&tar.Header{Name: "a/b/", Typeflag: tar.TypeDir, Mode: 0o700})) + require.NoError(t, tw.Close()) + }) + err := NewHubService(nil, nil, target).extractTarGz(t.Context(), archive, target) + require.ErrorContains(t, err, "mkdir a/b") + require.NotContains(t, err.Error(), target) +} + +func TestExtractTarGzTruncatedEntry(t *testing.T) { + t.Parallel() + target := t.TempDir() + // Header promises 10 bytes but the stream ends after 3 and the tar is never closed. + archive := tarGzRaw(t, func(tw *tar.Writer) { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: "f.yaml", Mode: 0o600, Size: 10})) + _, err := tw.Write([]byte("abc")) + require.NoError(t, err) + }) + err := NewHubService(nil, nil, target).extractTarGz(t.Context(), archive, target) + require.ErrorContains(t, err, "write f.yaml") + require.NotContains(t, err.Error(), target) +} + +func TestExtractTarGzDecompressionBomb(t *testing.T) { + t.Parallel() + target := t.TempDir() + const size = 100 * 1024 * 1024 + archive := tarGzRaw(t, func(tw *tar.Writer) { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: "bomb.bin", Mode: 0o600, Size: size})) + chunk := make([]byte, 1024*1024) + for i := 0; i < size/len(chunk); i++ { + _, err := tw.Write(chunk) + require.NoError(t, err) + } + require.NoError(t, tw.Close()) + }) + err := NewHubService(nil, nil, target).extractTarGz(t.Context(), archive, target) + require.ErrorContains(t, err, "decompression bomb") +} diff --git a/backend/internal/crowdsec/backup.go b/backend/internal/crowdsec/backup.go index 0e9fa0187..842007b74 100644 --- a/backend/internal/crowdsec/backup.go +++ b/backend/internal/crowdsec/backup.go @@ -6,6 +6,7 @@ import ( "io/fs" "os" "path/filepath" + "regexp" "sort" "strings" "time" @@ -287,10 +288,20 @@ func PruneBackups(dataDir, kind string, keep int) (removed []string, err error) return removed, errors.Join(errs...) } -// rollbackFailure annotates cause with a failed restore and the snapshot kept for manual recovery. +// BackupID returns the client-safe name of a backup: the final path element, never the absolute +// location. It covers the directory kinds (*.backup., *.filebackup.) and file backups +// (acquis.yaml.backup.); an empty path yields an empty name. +func BackupID(path string) string { + if path == "" { + return "" + } + return filepath.Base(path) +} + +// rollbackFailure annotates cause with a failed restore; the snapshot location is logged, never returned. func rollbackFailure(cause, restoreErr error, backupPath string) error { logger.Log().WithError(restoreErr).WithField("backup_path", util.SanitizeForLog(backupPath)).Error("preset rollback failed; backup retained for manual recovery") - return fmt.Errorf("%w (rollback failed: %v; backup retained at %s)", cause, restoreErr, backupPath) + return fmt.Errorf("%w (rollback failed; backup retained, see server logs)", cause) } // BackupFile copies the single file dataDir/rel into a new .filebackup./ directory @@ -335,3 +346,43 @@ func BackupFile(dataDir, rel string) (string, error) { } return dir, nil } + +// absPathPattern matches an absolute filesystem path that starts the string or follows whitespace, +// a quote, or one of ( [ { < = ,. URL path segments are preceded by ":" or a word character +// and are therefore left alone. A path ends at whitespace, a quote, or one of : ; , ) ] } > <. +var absPathPattern = regexp.MustCompile(`(^|[\s(\[{<=,'"])(/[^\s:;,)\]}>'"<]+)`) + +// apiRoutePrefix marks route-looking text (for example " /api/v1/x") that RedactPaths must not +// rewrite, since API routes are not filesystem paths. +const apiRoutePrefix = "/api/" + +// RedactPaths replaces absolute filesystem paths in an error message with "" so server +// layout never reaches an API client. Detail belongs in server logs. +// +// Limits: this is a heuristic over free text. Any absolute path in the recognised positions is +// redacted except those under /api/, so a bare route outside /api/ is also redacted (safe +// direction). Paths containing spaces or ":", Windows-style paths, and relative paths are not +// recognised; callers must not rely on it for those, and should prefer fixed messages. +func RedactPaths(msg string) string { + return absPathPattern.ReplaceAllStringFunc(msg, func(m string) string { + sub := absPathPattern.FindStringSubmatch(m) + if strings.HasPrefix(sub[2], apiRoutePrefix) { + return m + } + return sub[1] + "" + }) +} + +// withoutPath unwraps a filesystem error to its underlying cause (for example "permission +// denied"), dropping the absolute path that *fs.PathError and *os.LinkError embed. +func withoutPath(err error) error { + var pathErr *fs.PathError + if errors.As(err, &pathErr) { + return pathErr.Err + } + var linkErr *os.LinkError + if errors.As(err, &linkErr) { + return linkErr.Err + } + return err +} diff --git a/backend/internal/crowdsec/backup_name_test.go b/backend/internal/crowdsec/backup_name_test.go new file mode 100644 index 000000000..61aec13a5 --- /dev/null +++ b/backend/internal/crowdsec/backup_name_test.go @@ -0,0 +1,44 @@ +package crowdsec + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBackupID(t *testing.T) { + t.Parallel() + require.Equal(t, "crowdsec.backup.20260101-000000.000000", BackupID("/app/data/crowdsec.backup.20260101-000000.000000")) + require.Equal(t, "crowdsec.filebackup.20260101-000000.000000", BackupID("/data/crowdsec.filebackup.20260101-000000.000000/")) + require.Equal(t, "acquis.yaml.backup.20260101-000000", BackupID("/etc/crowdsec/acquis.yaml.backup.20260101-000000")) + require.Empty(t, BackupID("")) +} + +func TestRedactPaths(t *testing.T) { + t.Parallel() + require.Equal(t, "open : no such file", RedactPaths("open /app/data/x/y.yaml: no such file")) + require.Equal(t, "mkdir : denied", RedactPaths("mkdir /tmp/a-b/c: denied")) + require.Equal(t, `fetch ""`, RedactPaths(`fetch "/var/lib/x"`)) + require.Equal(t, "fetch https://hub.crowdsec.net/a/index.json failed", RedactPaths("fetch https://hub.crowdsec.net/a/index.json failed")) + require.Equal(t, "plain message", RedactPaths("plain message")) + require.Equal(t, "", RedactPaths("/app/data/x")) + require.Equal(t, "list []", RedactPaths("list [/data/crowdsec/a.yaml]")) + require.Equal(t, "{}", RedactPaths("{/etc/crowdsec/config.yaml}")) + require.Equal(t, "a=1,,b", RedactPaths("a=1,/srv/x/y,b")) + require.Equal(t, "<>", RedactPaths("")) + require.Equal(t, "stat ; retry ", RedactPaths("stat /custom/dir/f; retry /other/g")) + require.Equal(t, "route /api/v1/x failed", RedactPaths("route /api/v1/x failed")) + require.Equal(t, "GET (/api/v1/admin/crowdsec) failed", RedactPaths("GET (/api/v1/admin/crowdsec) failed")) + require.Equal(t, "see https://hub.crowdsec.net//a and a/b/c", RedactPaths("see https://hub.crowdsec.net//a and a/b/c")) +} + +func TestExtractTarGzErrorsOmitAbsolutePaths(t *testing.T) { + t.Parallel() + // A file entry colliding with a later nested entry forces a mkdir failure. + target := t.TempDir() + svc := NewHubService(nil, nil, target) + require.NoError(t, svc.extractTarGz(t.Context(), makeTarGz(t, map[string]string{"a": "file"}), target)) + err := svc.extractTarGz(t.Context(), makeTarGz(t, map[string]string{"a/b": "nested"}), target) + require.Error(t, err) + require.NotContains(t, err.Error(), target) +} diff --git a/backend/internal/crowdsec/backup_test.go b/backend/internal/crowdsec/backup_test.go index 68f9726c5..3191bbfbd 100644 --- a/backend/internal/crowdsec/backup_test.go +++ b/backend/internal/crowdsec/backup_test.go @@ -639,7 +639,9 @@ func TestApplyRestoreFailureIsReported(t *testing.T) { res, err := svc.Apply(context.Background(), "test/preset") require.ErrorContains(t, err, "rollback failed") - require.ErrorContains(t, err, "backup retained at "+res.BackupPath) + require.ErrorContains(t, err, "backup retained, see server logs") + require.NotContains(t, err.Error(), res.BackupPath, "the backup location stays in server logs") + require.NotContains(t, err.Error(), dir) } func TestApplySnapshotFailureChangesNothing(t *testing.T) { @@ -689,8 +691,9 @@ func TestRollbackFailureWrapsCause(t *testing.T) { cause := errors.New("boom") err := rollbackFailure(cause, errors.New("restore broke"), "/b") require.ErrorIs(t, err, cause) - require.ErrorContains(t, err, "restore broke") - require.ErrorContains(t, err, "/b") + require.ErrorContains(t, err, "see server logs") + require.NotContains(t, err.Error(), "restore broke") + require.NotContains(t, err.Error(), "/b") } // makeTarGzWithSymlink returns an archive that writes one file and then hits a symlink entry. diff --git a/backend/internal/crowdsec/curated_apply_test.go b/backend/internal/crowdsec/curated_apply_test.go index b3eb8dced..22656d690 100644 --- a/backend/internal/crowdsec/curated_apply_test.go +++ b/backend/internal/crowdsec/curated_apply_test.go @@ -441,7 +441,7 @@ func TestApplyCuratedRollbackFailureIsReported(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "install exploded") require.Contains(t, err.Error(), "rollback failed") - require.Contains(t, err.Error(), res.BackupPath) + require.NotContains(t, err.Error(), res.BackupPath, "the backup location stays in server logs") require.Equal(t, "failed", res.Status) require.DirExists(t, dir, "DataDir itself is never removed") } diff --git a/backend/internal/crowdsec/hub_pull_apply_test.go b/backend/internal/crowdsec/hub_pull_apply_test.go index 5d5f8e6b5..c44a00ef2 100644 --- a/backend/internal/crowdsec/hub_pull_apply_test.go +++ b/backend/internal/crowdsec/hub_pull_apply_test.go @@ -51,24 +51,24 @@ func TestPullThenApplyFlow(t *testing.T) { // Create hub service with mock HTTP client hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{ Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.example.com/api/index.json": - body := `{"items":[{"name":"test/preset","title":"Test Preset","description":"Test","etag":"etag123","download_url":"http://test.example.com/test.tgz","preview_url":"http://test.example.com/test.yaml"}]}` + case "https://hub.crowdsec.net/api/index.json": + body := `{"items":[{"name":"test/preset","title":"Test Preset","description":"Test","etag":"etag123","download_url":"https://hub.crowdsec.net/test.tgz","preview_url":"https://hub.crowdsec.net/test.yaml"}]}` return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header), }, nil - case "http://test.example.com/test.yaml": + case "https://hub.crowdsec.net/test.yaml": return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("test: preview\nkey: value")), Header: make(http.Header), }, nil - case "http://test.example.com/test.tgz": + case "https://hub.crowdsec.net/test.tgz": return &http.Response{ StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), @@ -141,15 +141,15 @@ func TestApplyRepullsOnCacheMissAfterCSCLIFailure(t *testing.T) { exec := &stubExec{responses: map[string]error{"install": fmt.Errorf("install failed")}} hub := NewHubService(exec, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.example.com/api/index.json": + case "https://hub.crowdsec.net/api/index.json": body := `{"items":[{"name":"test/preset","title":"Test","etag":"e1"}]}` return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://test.example.com/test/preset.yaml": + case "https://hub.crowdsec.net/test/preset.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("preview")), Header: make(http.Header)}, nil - case "http://test.example.com/test/preset.tgz": + case "https://hub.crowdsec.net/test/preset.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -187,15 +187,15 @@ func TestApplyRepullsOnCacheExpired(t *testing.T) { cache.nowFn = func() time.Time { return time.Now().Add(2 * time.Hour) } hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.example.com/api/index.json": + case "https://hub.crowdsec.net/api/index.json": body := `{"items":[{"name":"expired/preset","title":"Expired","etag":"e2"}]}` return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://test.example.com/expired/preset.yaml": + case "https://hub.crowdsec.net/expired/preset.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("preview new")), Header: make(http.Header)}, nil - case "http://test.example.com/expired/preset.tgz": + case "https://hub.crowdsec.net/expired/preset.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -222,15 +222,15 @@ func TestPullAcceptsNamespacedIndexEntry(t *testing.T) { archive := makeTestArchive(t, map[string]string{"config.yaml": "test: namespaced"}) hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.example.com/api/index.json": + case "https://hub.crowdsec.net/api/index.json": body := `{"items":[{"name":"crowdsecurity/bot-mitigation-essentials","title":"Bot Mitigation Essentials","etag":"etag-bme"}]}` return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://test.example.com/crowdsecurity/bot-mitigation-essentials.yaml": + case "https://hub.crowdsec.net/crowdsecurity/bot-mitigation-essentials.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("namespaced preview")), Header: make(http.Header)}, nil - case "http://test.example.com/crowdsecurity/bot-mitigation-essentials.tgz": + case "https://hub.crowdsec.net/crowdsecurity/bot-mitigation-essentials.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -254,22 +254,22 @@ func TestHubFallbackToMirrorOnForbidden(t *testing.T) { archive := makeTestArchive(t, map[string]string{"config.yaml": "mirror"}) hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://primary.example.com" - hub.MirrorBaseURL = "http://mirror.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" + hub.MirrorBaseURL = "https://hub-data.crowdsec.net" hub.HTTPClient = &http.Client{Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://primary.example.com/api/index.json": + case "https://hub.crowdsec.net/api/index.json": return &http.Response{StatusCode: http.StatusForbidden, Body: io.NopCloser(strings.NewReader("blocked")), Header: make(http.Header)}, nil - case "http://mirror.example.com/api/index.json": + case "https://hub-data.crowdsec.net/api/index.json": body := `{"items":[{"name":"fallback/preset","title":"Fallback","etag":"etag-mirror"}]}` return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://primary.example.com/fallback/preset.yaml": + case "https://hub.crowdsec.net/fallback/preset.yaml": return &http.Response{StatusCode: http.StatusForbidden, Body: io.NopCloser(strings.NewReader("blocked")), Header: make(http.Header)}, nil - case "http://mirror.example.com/fallback/preset.yaml": + case "https://hub-data.crowdsec.net/fallback/preset.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("mirror preview")), Header: make(http.Header)}, nil - case "http://primary.example.com/fallback/preset.tgz": + case "https://hub.crowdsec.net/fallback/preset.tgz": return &http.Response{StatusCode: http.StatusForbidden, Body: io.NopCloser(strings.NewReader("blocked")), Header: make(http.Header)}, nil - case "http://mirror.example.com/fallback/preset.tgz": + case "https://hub-data.crowdsec.net/fallback/preset.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -293,7 +293,7 @@ func TestApplyWithoutPullFails(t *testing.T) { // Create hub service without cscli (nil executor) and empty cache hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{Transport: mockTransport(func(req *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil })} @@ -344,16 +344,16 @@ func TestCacheListAfterPull(t *testing.T) { archive := makeTestArchive(t, map[string]string{"test.yaml": "content"}) hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" hub.HTTPClient = &http.Client{ Transport: mockTransport(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://test.example.com/api/index.json": + case "https://hub.crowdsec.net/api/index.json": body := `{"items":[{"name":"preset1","title":"Preset 1","etag":"e1"}]}` return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body)), Header: make(http.Header)}, nil - case "http://test.example.com/preset1.yaml": + case "https://hub.crowdsec.net/preset1.yaml": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("preview1")), Header: make(http.Header)}, nil - case "http://test.example.com/preset1.tgz": + case "https://hub.crowdsec.net/preset1.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader("")), Header: make(http.Header)}, nil @@ -451,7 +451,7 @@ func TestApplyReadsArchiveBeforeBackup(t *testing.T) { // Create hub service WITHOUT cscli (nil executor) to force cache fallback path hub := NewHubService(nil, cache, dataDir) - hub.HubBaseURL = "http://test.example.com" + hub.HubBaseURL = "https://hub.crowdsec.net" // HTTP client that fails everything - we don't want to hit network hub.HTTPClient = &http.Client{ Transport: mockTransport(func(req *http.Request) (*http.Response, error) { diff --git a/backend/internal/crowdsec/hub_sync.go b/backend/internal/crowdsec/hub_sync.go index 8ef531538..7a320ff68 100644 --- a/backend/internal/crowdsec/hub_sync.go +++ b/backend/internal/crowdsec/hub_sync.go @@ -85,22 +85,18 @@ type HubService struct { PullTimeout time.Duration ApplyTimeout time.Duration + // validateURL overrides validateHubURL when non-nil. Test-only: production + // code never sets it, so every hub request goes through validateHubURL. + validateURL func(string) error + // mu serializes Apply and ApplyCurated so backup/rollback cycles never interleave. mu sync.Mutex } -// hubAllowLoopback is a test-only seam. It is always false in production and is -// only toggled from _test.go files. Tests that toggle it must not call -// t.Parallel(), as it is shared package state. -var hubAllowLoopback bool - // validateHubURL validates a hub URL for security (SSRF protection - HIGH-001). // This function prevents Server-Side Request Forgery by: // 1. Enforcing HTTPS for production hub URLs // 2. Allowlisting known CrowdSec hub domains -// 3. Accepting localhost/test hostnames at this layer; the dial layer -// (network.NewSafeHTTPClient) still blocks loopback and private targets -// unless the test-only hubAllowLoopback seam is set // // Returns: error if URL is invalid or not allowlisted func validateHubURL(rawURL string) error { @@ -109,7 +105,7 @@ func validateHubURL(rawURL string) error { return fmt.Errorf("invalid URL format: %w", err) } - // Only allow http/https schemes + // Only allow http/https schemes at parse level; https is enforced below if parsed.Scheme != "http" && parsed.Scheme != "https" { return fmt.Errorf("unsupported scheme: %s (only http and https are allowed)", parsed.Scheme) } @@ -119,16 +115,7 @@ func validateHubURL(rawURL string) error { return fmt.Errorf("missing hostname in URL") } - // Allow localhost and test domains for development/testing - // This is safe because tests control the mock servers - if host == "localhost" || host == "127.0.0.1" || host == "::1" || - strings.HasSuffix(host, ".example.com") || strings.HasSuffix(host, ".example") || - host == "example.com" || strings.HasSuffix(host, ".local") || - host == "test.hub" { // Allow test.hub for integration tests - return nil - } - - // For production URLs, must be HTTPS + // Hub URLs must be HTTPS if parsed.Scheme != "https" { return fmt.Errorf("hub URLs must use HTTPS (got: %s)", parsed.Scheme) } @@ -155,6 +142,15 @@ func validateHubURL(rawURL string) error { return nil } +// checkURL validates target with the test override when set, otherwise with +// the production validateHubURL. +func (s *HubService) checkURL(target string) error { + if s.validateURL != nil { + return s.validateURL(target) + } + return validateHubURL(target) +} + // NewHubService constructs a HubService with sane defaults. func NewHubService(exec CommandExecutor, cache *HubCache, dataDir string) *HubService { pullTimeout := defaultPullTimeout @@ -187,19 +183,11 @@ func NewHubService(exec CommandExecutor, cache *HubCache, dataDir string) *HubSe // Hub URLs are validated by validateHubURL() which: // - Enforces HTTPS for production // - Allowlists known CrowdSec domains (hub-data.crowdsec.net, hub.crowdsec.net, raw.githubusercontent.com) -// - Blocks loopback unless the test-only hubAllowLoopback seam is set +// - Blocks loopback and private targets at the dial layer // Using network.NewSafeHTTPClient provides defense-in-depth at the connection level. func newHubHTTPClient(timeout time.Duration) *http.Client { opts := []network.Option{ network.WithTimeout(timeout), - network.WithAllowedDomains( - "hub-data.crowdsec.net", - "hub.crowdsec.net", - "raw.githubusercontent.com", - ), - } - if hubAllowLoopback { - opts = append(opts, network.WithAllowLocalhost()) } return network.NewSafeHTTPClient(opts...) } @@ -450,7 +438,7 @@ func (h hubHTTPError) CanFallback() bool { func (s *HubService) fetchIndexHTTPFromURL(ctx context.Context, target string) (HubIndex, error) { // CRITICAL FIX: Validate hub URL before making HTTP request (HIGH-001) - if err := validateHubURL(target); err != nil { + if err := s.checkURL(target); err != nil { return HubIndex{}, fmt.Errorf("invalid hub URL: %w", err) } @@ -751,7 +739,7 @@ func (s *HubService) fetchWithFallback(ctx context.Context, urls []string) (data func (s *HubService) fetchWithLimitFromURL(ctx context.Context, url string) ([]byte, error) { // CRITICAL FIX: Validate hub URL before making HTTP request (HIGH-001) - if err := validateHubURL(url); err != nil { + if err := s.checkURL(url); err != nil { return nil, fmt.Errorf("invalid hub URL: %w", err) } @@ -943,7 +931,7 @@ func (s *HubService) extractTarGz(ctx context.Context, archive []byte, targetDir continue } cleanName := filepath.Clean(hdr.Name) - if strings.HasPrefix(cleanName, "..") || strings.Contains(cleanName, ".."+string(os.PathSeparator)) || filepath.IsAbs(cleanName) { + if hasParentComponent(cleanName) || filepath.IsAbs(cleanName) { return fmt.Errorf("unsafe path %s", hdr.Name) } if IsEngineOwnedPath(cleanName) { @@ -960,7 +948,7 @@ func (s *HubService) extractTarGz(ctx context.Context, archive []byte, targetDir if hdr.FileInfo().IsDir() { if mkdirErr := os.MkdirAll(destPath, mode); mkdirErr != nil { //nolint:gosec // G703: destPath contained under targetDir by the filepath.Rel check above - return fmt.Errorf("mkdir %s: %w", destPath, mkdirErr) + return fmt.Errorf("mkdir %s: %w", cleanName, withoutPath(mkdirErr)) } continue } @@ -975,15 +963,15 @@ func (s *HubService) extractTarGz(ctx context.Context, archive []byte, targetDir written, err := io.Copy(f, limitedReader) if err != nil { _ = f.Close() - return fmt.Errorf("write %s: %w", destPath, err) + return fmt.Errorf("write %s: %w", cleanName, withoutPath(err)) } // Verify we didn't hit the limit (potential attack) if written >= maxDecompressedSize { _ = f.Close() - return fmt.Errorf("file %s exceeded decompression limit (%d bytes), potential decompression bomb", destPath, maxDecompressedSize) + return fmt.Errorf("file %s exceeded decompression limit (%d bytes), potential decompression bomb", cleanName, maxDecompressedSize) } if err := f.Close(); err != nil { - return fmt.Errorf("close %s: %w", destPath, err) + return fmt.Errorf("close %s: %w", cleanName, withoutPath(err)) } } return nil @@ -1037,3 +1025,14 @@ func isGzip(data []byte) bool { } return data[0] == 0x1f && data[1] == 0x8b } + +// hasParentComponent reports whether any path component equals "..". +// Names that merely start with two dots (e.g. "..foo") are legitimate. +func hasParentComponent(name string) bool { + for _, part := range strings.Split(filepath.ToSlash(name), "/") { + if part == ".." { + return true + } + } + return false +} diff --git a/backend/internal/crowdsec/hub_sync_raw_index_test.go b/backend/internal/crowdsec/hub_sync_raw_index_test.go index 619b7f29d..b33a5f45f 100644 --- a/backend/internal/crowdsec/hub_sync_raw_index_test.go +++ b/backend/internal/crowdsec/hub_sync_raw_index_test.go @@ -10,7 +10,7 @@ import ( func TestFetchIndexParsesRawIndexFormat(t *testing.T) { svc := NewHubService(nil, nil, t.TempDir()) - svc.HubBaseURL = "http://example.com" + svc.HubBaseURL = "https://hub.crowdsec.net" // This JSON represents the "raw" index format (map of maps) which has no "items" field. // json.Unmarshal into HubIndex will succeed but result in empty Items. @@ -33,7 +33,7 @@ func TestFetchIndexParsesRawIndexFormat(t *testing.T) { }` svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { - if req.URL.String() == "http://example.com"+defaultHubIndexPath { + if req.URL.String() == "https://hub.crowdsec.net"+defaultHubIndexPath { resp := newResponse(http.StatusOK, rawIndexBody) resp.Header.Set("Content-Type", "application/json") return resp, nil diff --git a/backend/internal/crowdsec/hub_sync_test.go b/backend/internal/crowdsec/hub_sync_test.go index c6c79e9d3..15858765c 100644 --- a/backend/internal/crowdsec/hub_sync_test.go +++ b/backend/internal/crowdsec/hub_sync_test.go @@ -114,6 +114,7 @@ func TestFetchIndexFallbackHTTP(t *testing.T) { defer hubServer.Close() svc.HubBaseURL = hubServer.URL + svc.validateURL = func(string) error { return nil } // local httptest server is not an allowlisted hub host svc.HTTPClient = hubServer.Client() idx, err := svc.FetchIndex(context.Background()) @@ -128,7 +129,7 @@ func TestFetchIndexHTTPRejectsRedirect(t *testing.T) { } t.Parallel() svc := NewHubService(nil, nil, t.TempDir()) - svc.HubBaseURL = "http://hub.example" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { resp := newResponse(http.StatusMovedPermanently, "") resp.Header.Set("Location", "https://hub.crowdsec.net/") @@ -229,14 +230,14 @@ func TestPullCachesPreview(t *testing.T) { archiveBytes := makeTarGz(t, map[string]string{"config.yaml": "value: 1"}) svc := NewHubService(nil, cache, dataDir) - svc.HubBaseURL = "http://example.com" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://example.com" + defaultHubIndexPath: - return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","type":"collection","etag":"etag1","download_url":"http://example.com/demo.tgz","preview_url":"http://example.com/demo.yaml"}]}`), nil - case "http://example.com/demo.yaml": + case "https://hub.crowdsec.net" + defaultHubIndexPath: + return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","type":"collection","etag":"etag1","download_url":"https://hub.crowdsec.net/demo.tgz","preview_url":"https://hub.crowdsec.net/demo.yaml"}]}`), nil + case "https://hub.crowdsec.net/demo.yaml": return newResponse(http.StatusOK, "preview-body"), nil - case "http://example.com/demo.tgz": + case "https://hub.crowdsec.net/demo.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archiveBytes)), Header: make(http.Header)}, nil default: return newResponse(http.StatusNotFound, ""), nil @@ -341,14 +342,14 @@ func TestPullEvictsExpiredCacheAndRefreshes(t *testing.T) { cache.nowFn = func() time.Time { return fixed.Add(3 * time.Second) } svc := NewHubService(nil, cache, t.TempDir()) - svc.HubBaseURL = "http://example.com" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { switch req.URL.String() { - case "http://example.com" + defaultHubIndexPath: - return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","type":"collection","etag":"etag2","download_url":"http://example.com/demo.tgz","preview_url":"http://example.com/demo.yaml"}]}`), nil - case "http://example.com/demo.yaml": + case "https://hub.crowdsec.net" + defaultHubIndexPath: + return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","description":"desc","type":"collection","etag":"etag2","download_url":"https://hub.crowdsec.net/demo.tgz","preview_url":"https://hub.crowdsec.net/demo.yaml"}]}`), nil + case "https://hub.crowdsec.net/demo.yaml": return newResponse(http.StatusOK, "fresh-preview"), nil - case "http://example.com/demo.tgz": + case "https://hub.crowdsec.net/demo.tgz": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil default: return newResponse(http.StatusNotFound, ""), nil @@ -368,15 +369,15 @@ func TestPullFallsBackToArchivePreview(t *testing.T) { archive := makeTarGz(t, map[string]string{"scenarios/demo.yaml": "title: demo"}) svc := NewHubService(nil, cache, t.TempDir()) - svc.HubBaseURL = "http://example.com" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { - if req.URL.String() == "http://example.com"+defaultHubIndexPath { - return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","etag":"etag1","download_url":"http://example.com/demo.tgz","preview_url":"http://example.com/demo.yaml"}]}`), nil + if req.URL.String() == "https://hub.crowdsec.net"+defaultHubIndexPath { + return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/demo","title":"Demo","etag":"etag1","download_url":"https://hub.crowdsec.net/demo.tgz","preview_url":"https://hub.crowdsec.net/demo.yaml"}]}`), nil } - if req.URL.String() == "http://example.com/demo.yaml" { + if req.URL.String() == "https://hub.crowdsec.net/demo.yaml" { return newResponse(http.StatusInternalServerError, ""), nil } - if req.URL.String() == "http://example.com/demo.tgz" { + if req.URL.String() == "https://hub.crowdsec.net/demo.tgz" { return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(archive)), Header: make(http.Header)}, nil } return newResponse(http.StatusNotFound, ""), nil @@ -395,7 +396,7 @@ func TestPullFallsBackToMirrorArchiveOnForbidden(t *testing.T) { archiveBytes := makeTarGz(t, map[string]string{"config.yml": "foo: bar"}) svc := NewHubService(nil, cache, dataDir) - svc.HubBaseURL = "https://primary.example" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.MirrorBaseURL = defaultHubMirrorBaseURL calls := make([]string, 0) @@ -403,13 +404,13 @@ func TestPullFallsBackToMirrorArchiveOnForbidden(t *testing.T) { svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { calls = append(calls, req.URL.String()) switch req.URL.String() { - case "https://primary.example/api/index.json": + case "https://hub.crowdsec.net/api/index.json": resp := newResponse(http.StatusOK, indexBody) resp.Header.Set("Content-Type", "application/json") return resp, nil - case "https://primary.example/crowdsecurity/demo.tgz": + case "https://hub.crowdsec.net/crowdsecurity/demo.tgz": return newResponse(http.StatusForbidden, ""), nil - case "https://primary.example/crowdsecurity/demo.yaml": + case "https://hub.crowdsec.net/crowdsecurity/demo.yaml": return newResponse(http.StatusForbidden, ""), nil case defaultHubMirrorBaseURL + "/.index.json": resp := newResponse(http.StatusOK, indexBody) @@ -441,7 +442,7 @@ func TestFetchWithLimitRejectsLargePayload(t *testing.T) { return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(big)), Header: make(http.Header)}, nil })} - _, err := svc.fetchWithLimitFromURL(context.Background(), "http://example.com/large.tgz") + _, err := svc.fetchWithLimitFromURL(context.Background(), "https://hub.crowdsec.net/large.tgz") require.Error(t, err) require.Contains(t, err.Error(), "payload too large") } @@ -511,7 +512,7 @@ func TestPullValidatesSlugAndMissingPreset(t *testing.T) { cache, cacheErr := NewHubCache(t.TempDir(), time.Hour) require.NoError(t, cacheErr) svc.Cache = cache - svc.HubBaseURL = "http://hub.example" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { return newResponse(http.StatusOK, `{"items":[{"name":"crowdsecurity/other","title":"Other","description":"d","type":"collection"}]}`), nil })} @@ -531,7 +532,7 @@ func TestFetchWithLimitRequiresClient(t *testing.T) { t.Parallel() svc := NewHubService(nil, nil, t.TempDir()) svc.HTTPClient = nil - _, err := svc.fetchWithLimitFromURL(context.Background(), "http://example.com/demo.tgz") + _, err := svc.fetchWithLimitFromURL(context.Background(), "https://hub.crowdsec.net/demo.tgz") require.Error(t, err) } @@ -568,7 +569,7 @@ func TestFetchIndexCSCLIParseError(t *testing.T) { t.Parallel() exec := &recordingExec{outputs: map[string][]byte{"cscli hub list -o json": []byte("not-json")}} svc := NewHubService(exec, nil, t.TempDir()) - svc.HubBaseURL = "http://hub.example" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { return newResponse(http.StatusInternalServerError, ""), nil })} @@ -580,12 +581,12 @@ func TestFetchIndexCSCLIParseError(t *testing.T) { func TestFetchWithLimitStatusError(t *testing.T) { t.Parallel() svc := NewHubService(nil, nil, t.TempDir()) - svc.HubBaseURL = "http://hub.example" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { return newResponse(http.StatusNotFound, ""), nil })} - _, err := svc.fetchWithLimitFromURL(context.Background(), "http://hub.example/demo.tgz") + _, err := svc.fetchWithLimitFromURL(context.Background(), "https://hub.crowdsec.net/demo.tgz") require.Error(t, err) } @@ -839,9 +840,9 @@ func TestIndexURLCandidates_GitHubMirror(t *testing.T) { func TestBuildResourceURLs_DeduplicatesExplicitAndBases(t *testing.T) { t.Parallel() - urls := buildResourceURLs("https://hub.example/preset.tgz", "crowdsecurity/demo", "/%s.tgz", []string{"https://hub.example", "https://hub.example"}) + urls := buildResourceURLs("https://hub.crowdsec.net/preset.tgz", "crowdsecurity/demo", "/%s.tgz", []string{"https://hub.crowdsec.net", "https://hub.crowdsec.net"}) require.NotEmpty(t, urls) - require.Equal(t, "https://hub.example/preset.tgz", urls[0]) + require.Equal(t, "https://hub.crowdsec.net/preset.tgz", urls[0]) require.Len(t, urls, 2) } @@ -849,9 +850,9 @@ func TestHubHTTPErrorMethods(t *testing.T) { t.Parallel() inner := errors.New("inner") - err := hubHTTPError{url: "https://hub.example", statusCode: 404, inner: inner, fallback: true} + err := hubHTTPError{url: "https://hub.crowdsec.net", statusCode: 404, inner: inner, fallback: true} - require.Contains(t, err.Error(), "https://hub.example") + require.Contains(t, err.Error(), "https://hub.crowdsec.net") require.ErrorIs(t, err, inner) require.True(t, err.CanFallback()) } @@ -991,27 +992,43 @@ func TestValidateHubURL_InvalidSchemes(t *testing.T) { } } -func TestValidateHubURL_LocalhostExceptions(t *testing.T) { +func TestValidateHubURL_RejectsNonProductionHosts(t *testing.T) { t.Parallel() - localhostURLs := []string{ + nonProductionURLs := []string{ "http://localhost:8080/index.json", "http://127.0.0.1:8080/index.json", "http://[::1]:8080/index.json", - "http://test.hub/api/index.json", + "https://localhost/index.json", + "https://127.0.0.1/index.json", + "https://[::1]/index.json", + "https://test.hub/api/index.json", + "https://example.com/api/index.json", + "https://test.example.com/api/index.json", + "https://server.local/api/index.json", "http://example.com/api/index.json", - "http://test.example.com/api/index.json", - "http://server.local/api/index.json", } - for _, url := range localhostURLs { + for _, url := range nonProductionURLs { t.Run(url, func(t *testing.T) { t.Parallel() - err := validateHubURL(url) - require.NoError(t, err, "Expected localhost/test domain to be allowed") + require.Error(t, validateHubURL(url), "Expected non-production host to be rejected") }) } } +func TestNewHubHTTPClient_RejectsLoopback(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + resp, err := newHubHTTPClient(2 * time.Second).Get(srv.URL) + if err == nil { + _ = resp.Body.Close() + } + require.Error(t, err, "hub client must reject loopback") +} + func TestValidateHubURL_UnknownDomainRejection(t *testing.T) { t.Parallel() unknownURLs := []string{ @@ -1101,7 +1118,7 @@ func TestParseRawIndex(t *testing.T) { } }` - idx, err := parseRawIndex([]byte(rawJSON), "https://hub.example.com/api/index.json") + idx, err := parseRawIndex([]byte(rawJSON), "https://hub.crowdsec.net/api/index.json") require.NoError(t, err) require.Len(t, idx.Items, 2) @@ -1122,14 +1139,14 @@ func TestParseRawIndex(t *testing.T) { t.Run("returns error on invalid JSON", func(t *testing.T) { t.Parallel() - _, err := parseRawIndex([]byte("not json"), "https://hub.example.com") + _, err := parseRawIndex([]byte("not json"), "https://hub.crowdsec.net") require.Error(t, err) require.Contains(t, err.Error(), "parse raw index") }) t.Run("returns error on empty index", func(t *testing.T) { t.Parallel() - _, err := parseRawIndex([]byte("{}"), "https://hub.example.com") + _, err := parseRawIndex([]byte("{}"), "https://hub.crowdsec.net") require.Error(t, err) require.Contains(t, err.Error(), "empty raw index") }) @@ -1154,7 +1171,7 @@ func TestFetchIndexHTTPFromURL_HTMLDetection(t *testing.T) { return resp, nil })} - _, err := svc.fetchIndexHTTPFromURL(context.Background(), "http://test.hub/index.json") + _, err := svc.fetchIndexHTTPFromURL(context.Background(), "https://hub.crowdsec.net/index.json") require.Error(t, err) require.Contains(t, err.Error(), "HTML") } @@ -1195,11 +1212,11 @@ func TestHubService_Apply_CacheRefresh(t *testing.T) { // Reset time to trigger expiration cache.nowFn = time.Now - indexBody := `{"items":[{"name":"test/preset","title":"Test","etag":"etag2","download_url":"http://test.hub/preset.tgz"}]}` + indexBody := `{"items":[{"name":"test/preset","title":"Test","etag":"etag2","download_url":"https://hub.crowdsec.net/preset.tgz"}]}` newArchive := makeTarGz(t, map[string]string{"config.yml": "new"}) svc := NewHubService(nil, cache, dataDir) - svc.HubBaseURL = "http://test.hub" + svc.HubBaseURL = "https://hub.crowdsec.net" svc.HTTPClient = &http.Client{Transport: roundTripperFunc(func(req *http.Request) (*http.Response, error) { if strings.Contains(req.URL.String(), "index.json") { return newResponse(http.StatusOK, indexBody), nil @@ -1443,14 +1460,14 @@ func TestHubHTTPErrorError(t *testing.T) { t.Parallel() inner := errors.New("connection refused") err := hubHTTPError{ - url: "https://hub.example.com/index.json", + url: "https://hub.crowdsec.net/index.json", statusCode: 503, inner: inner, fallback: true, } msg := err.Error() - require.Contains(t, msg, "https://hub.example.com/index.json") + require.Contains(t, msg, "https://hub.crowdsec.net/index.json") require.Contains(t, msg, "503") require.Contains(t, msg, "connection refused") }) @@ -1458,14 +1475,14 @@ func TestHubHTTPErrorError(t *testing.T) { t.Run("error without inner error", func(t *testing.T) { t.Parallel() err := hubHTTPError{ - url: "https://hub.example.com/index.json", + url: "https://hub.crowdsec.net/index.json", statusCode: 404, inner: nil, fallback: false, } msg := err.Error() - require.Contains(t, msg, "https://hub.example.com/index.json") + require.Contains(t, msg, "https://hub.crowdsec.net/index.json") require.Contains(t, msg, "404") require.NotContains(t, msg, "nil") }) @@ -1477,7 +1494,7 @@ func TestHubHTTPErrorUnwrap(t *testing.T) { t.Parallel() inner := errors.New("underlying error") err := hubHTTPError{ - url: "https://hub.example.com", + url: "https://hub.crowdsec.net", statusCode: 500, inner: inner, } @@ -1489,7 +1506,7 @@ func TestHubHTTPErrorUnwrap(t *testing.T) { t.Run("unwrap returns nil when no inner", func(t *testing.T) { t.Parallel() err := hubHTTPError{ - url: "https://hub.example.com", + url: "https://hub.crowdsec.net", statusCode: 500, inner: nil, } @@ -1502,7 +1519,7 @@ func TestHubHTTPErrorUnwrap(t *testing.T) { t.Parallel() inner := context.Canceled err := hubHTTPError{ - url: "https://hub.example.com", + url: "https://hub.crowdsec.net", statusCode: 0, inner: inner, } @@ -1517,7 +1534,7 @@ func TestHubHTTPErrorCanFallback(t *testing.T) { t.Run("returns true when fallback is true", func(t *testing.T) { t.Parallel() err := hubHTTPError{ - url: "https://hub.example.com", + url: "https://hub.crowdsec.net", statusCode: 503, fallback: true, } @@ -1528,7 +1545,7 @@ func TestHubHTTPErrorCanFallback(t *testing.T) { t.Run("returns false when fallback is false", func(t *testing.T) { t.Parallel() err := hubHTTPError{ - url: "https://hub.example.com", + url: "https://hub.crowdsec.net", statusCode: 404, fallback: false, } @@ -1607,29 +1624,39 @@ func TestValidateHubURL_EdgeCases(t *testing.T) { errorMsg: "unsupported scheme", }, { - name: "Test domain allowed", - url: "http://test.hub/api/index.json", + name: "Allowlisted hosts over HTTPS accepted", + url: "https://raw.githubusercontent.com/crowdsecurity/hub/master/.index.json", wantError: false, }, { - name: "Example.com allowed for testing", - url: "http://example.com/index.json", - wantError: false, + name: "Test domain rejected", + url: "https://test.hub/api/index.json", + wantError: true, + errorMsg: "unknown hub domain", }, { - name: ".local domain allowed", - url: "http://myserver.local/index.json", - wantError: false, + name: "Example.com rejected", + url: "https://example.com/index.json", + wantError: true, + errorMsg: "unknown hub domain", }, { - name: "Subdomain of example.com allowed", - url: "http://test.example.com/index.json", - wantError: false, + name: ".local domain rejected", + url: "https://myserver.local/index.json", + wantError: true, + errorMsg: "unknown hub domain", + }, + { + name: "Plain HTTP loopback rejected", + url: "http://localhost:8080/index.json", + wantError: true, + errorMsg: "must use HTTPS", }, { - name: "IPv6 loopback allowed", + name: "IPv6 loopback rejected", url: "http://[::1]:8080/index.json", - wantError: false, + wantError: true, + errorMsg: "must use HTTPS", }, { name: "Unknown production domain rejected", @@ -1723,20 +1750,20 @@ func TestNewHubService_EnvVarTimeouts_Whitespace(t *testing.T) { func TestNewHubService_CustomHubBaseURL(t *testing.T) { // Note: Cannot use t.Parallel() with t.Setenv() - t.Setenv("HUB_BASE_URL", "https://custom.hub.example.com") + t.Setenv("HUB_BASE_URL", "https://hub.crowdsec.net") svc := NewHubService(nil, nil, t.TempDir()) - require.Equal(t, "https://custom.hub.example.com", svc.HubBaseURL) + require.Equal(t, "https://hub.crowdsec.net", svc.HubBaseURL) } func TestNewHubService_CustomMirrorBaseURL(t *testing.T) { // Note: Cannot use t.Parallel() with t.Setenv() - t.Setenv("HUB_MIRROR_BASE_URL", "https://mirror.example.com") + t.Setenv("HUB_MIRROR_BASE_URL", "https://hub-data.crowdsec.net") svc := NewHubService(nil, nil, t.TempDir()) - require.Equal(t, "https://mirror.example.com", svc.MirrorBaseURL) + require.Equal(t, "https://hub-data.crowdsec.net", svc.MirrorBaseURL) } // ============================================ @@ -1834,19 +1861,6 @@ func TestExtractTarGz_RejectsBareParentEntry(t *testing.T) { require.Contains(t, err.Error(), "unsafe path") } -func TestExtractTarGz_RejectsDotDotPrefixedName(t *testing.T) { - t.Parallel() - svc := NewHubService(nil, nil, t.TempDir()) - targetDir := t.TempDir() - - // Documents the existing name check: any cleaned name starting with ".." is rejected. - archive := buildSingleEntryTarGz(t, &tar.Header{Name: "..foo", Mode: 0o644, Size: 3}, "abc") - err := svc.extractTarGz(context.Background(), archive, targetDir) - require.Error(t, err) - require.Contains(t, err.Error(), "unsafe path") - require.NoFileExists(t, filepath.Join(targetDir, "..foo")) -} - func TestExtractTarGz_MasksSpecialModeBits(t *testing.T) { t.Parallel() svc := NewHubService(nil, nil, t.TempDir()) @@ -2065,7 +2079,7 @@ func TestFetchIndexHTTPFromURL_ParseRawIndexFallback(t *testing.T) { return resp, nil })} - idx, err := svc.fetchIndexHTTPFromURL(context.Background(), "http://test.hub/.index.json") + idx, err := svc.fetchIndexHTTPFromURL(context.Background(), "https://hub.crowdsec.net/.index.json") require.NoError(t, err) require.Len(t, idx.Items, 1) require.Equal(t, "crowdsecurity/nginx", idx.Items[0].Name) @@ -2090,7 +2104,7 @@ func TestFetchIndexHTTPFromURL_EmptyJSONArray(t *testing.T) { // Empty items array triggers raw index parsing (map[string]map[string]...), which succeeds // but returns empty index. This is actually valid JSON but semantically empty. // The code returns idx even if empty in this case (no error), so we should not expect an error. - idx, err := svc.fetchIndexHTTPFromURL(context.Background(), "http://test.hub/index.json") + idx, err := svc.fetchIndexHTTPFromURL(context.Background(), "https://hub.crowdsec.net/index.json") require.NoError(t, err) require.Empty(t, idx.Items, "should parse successfully but return empty items") } @@ -2108,7 +2122,7 @@ func TestFetchIndexHTTPFromURL_InvalidJSON(t *testing.T) { return resp, nil })} - _, err := svc.fetchIndexHTTPFromURL(context.Background(), "http://test.hub/index.json") + _, err := svc.fetchIndexHTTPFromURL(context.Background(), "https://hub.crowdsec.net/index.json") require.Error(t, err) } @@ -2245,32 +2259,43 @@ func TestFindIndexEntry_EmptySlug(t *testing.T) { require.False(t, found) } -// setHubAllowLoopbackForTest toggles the package-level seam. Callers must not -// use t.Parallel(). -func setHubAllowLoopbackForTest(t *testing.T, v bool) { - t.Helper() - prev := hubAllowLoopback - hubAllowLoopback = v - t.Cleanup(func() { hubAllowLoopback = prev }) -} +func TestExtractTarGzDotDotNames(t *testing.T) { + t.Parallel() + svc := NewHubService(nil, nil, t.TempDir()) -// Must not run in parallel: relies on shared seam state. -func TestNewHubHTTPClient_LoopbackSeam(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - })) - defer srv.Close() + for _, name := range []string{"..foo", "a/..b", "...", "a/..."} { + t.Run("accepts "+name, func(t *testing.T) { + t.Parallel() + targetDir := t.TempDir() + archive := makeTarGz(t, map[string]string{name: "ok"}) + require.NoError(t, svc.extractTarGz(context.Background(), archive, targetDir)) + require.FileExists(t, filepath.Join(targetDir, filepath.FromSlash(name))) + }) + } - get := func() error { - resp, err := newHubHTTPClient(2 * time.Second).Get(srv.URL) - if err == nil { - _ = resp.Body.Close() - } - return err + for _, name := range []string{"../x", "a/../../x", "/abs", ".."} { + t.Run("rejects "+name, func(t *testing.T) { + t.Parallel() + parent := t.TempDir() + targetDir := filepath.Join(parent, "data") + archive := makeTarGz(t, map[string]string{name: "bad"}) + err := svc.extractTarGz(context.Background(), archive, targetDir) + require.Error(t, err) + require.Contains(t, err.Error(), "unsafe path") + require.NoFileExists(t, filepath.Join(parent, "x")) + }) } +} - require.Error(t, get(), "default client must reject loopback") +func TestHubBaseCandidates_DistinctRolesKeepOrder(t *testing.T) { + t.Parallel() + svc := NewHubService(nil, nil, t.TempDir()) + svc.HubBaseURL = "https://hub.crowdsec.net" + svc.MirrorBaseURL = defaultHubMirrorBaseURL - setHubAllowLoopbackForTest(t, true) - require.NoError(t, get(), "seam permits loopback in tests") + require.Equal(t, []string{ + "https://hub.crowdsec.net", + defaultHubMirrorBaseURL, + defaultHubBaseURL, + }, svc.hubBaseCandidates()) } diff --git a/backend/internal/crowdsec/safe_create.go b/backend/internal/crowdsec/safe_create.go index 01d68d19d..545f6d99d 100644 --- a/backend/internal/crowdsec/safe_create.go +++ b/backend/internal/crowdsec/safe_create.go @@ -19,7 +19,7 @@ func CreateFileNoSymlink(root, dest string, parentMode, fileMode os.FileMode) (* root = filepath.Clean(root) rel, err := filepath.Rel(root, filepath.Clean(dest)) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { - return nil, fmt.Errorf("path escapes target: %s", dest) + return nil, fmt.Errorf("path escapes target: %s", rel) } cur := root for _, part := range strings.Split(rel, string(os.PathSeparator)) { @@ -29,18 +29,18 @@ func CreateFileNoSymlink(root, dest string, parentMode, fileMode os.FileMode) (* break } if lerr != nil { - return nil, fmt.Errorf("inspect %s: %w", cur, lerr) + return nil, fmt.Errorf("inspect %s: %w", rel, withoutPath(lerr)) } if info.Mode()&os.ModeSymlink != 0 { - return nil, fmt.Errorf("%w: %s", ErrSymlinkInPath, cur) + return nil, fmt.Errorf("%w: %s", ErrSymlinkInPath, rel) } } if err = os.MkdirAll(filepath.Dir(dest), parentMode); err != nil { //nolint:gosec // G703: dest verified contained under root and symlink-free above - return nil, fmt.Errorf("mkdir parent: %w", err) + return nil, fmt.Errorf("mkdir parent: %w", withoutPath(err)) } f, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, fileMode) //nolint:gosec // G304,G703: dest verified contained under root and symlink-free above if err != nil { - return nil, fmt.Errorf("open %s: %w", dest, err) + return nil, fmt.Errorf("open %s: %w", rel, withoutPath(err)) } return f, nil } diff --git a/backend/internal/network/safeclient.go b/backend/internal/network/safeclient.go index 8c0059bd9..4c3f8d1d5 100644 --- a/backend/internal/network/safeclient.go +++ b/backend/internal/network/safeclient.go @@ -262,10 +262,6 @@ type ClientOptions struct { // Use only for testing or when connecting to known-safe local services. AllowLocalhost bool - // AllowedDomains restricts requests to specific domains (optional). - // If set, only these domains will be allowed (in addition to localhost if AllowLocalhost is true). - AllowedDomains []string - // MaxRedirects sets the maximum number of redirects to follow (default: 0) // Set to 0 to disable redirects entirely. MaxRedirects int @@ -303,7 +299,6 @@ func defaultOptions() ClientOptions { return ClientOptions{ Timeout: 10 * time.Second, AllowLocalhost: false, - AllowedDomains: nil, MaxRedirects: 0, DialTimeout: 5 * time.Second, } @@ -324,14 +319,6 @@ func WithAllowLocalhost() Option { } } -// WithAllowedDomains restricts requests to specific domains. -// When set, only requests to these domains will be permitted. -func WithAllowedDomains(domains ...string) Option { - return func(opts *ClientOptions) { - opts.AllowedDomains = append(opts.AllowedDomains, domains...) - } -} - // WithMaxRedirects sets the maximum number of redirects to follow. // Default is 0 (no redirects). Each redirect target is validated against SSRF rules. func WithMaxRedirects(maxRedirects int) Option { diff --git a/backend/internal/network/safeclient_test.go b/backend/internal/network/safeclient_test.go index f78e7f895..d43d8afc0 100644 --- a/backend/internal/network/safeclient_test.go +++ b/backend/internal/network/safeclient_test.go @@ -150,33 +150,6 @@ func TestSafeDialer_AllowsLocalhost(t *testing.T) { _ = conn.Close() } -func TestSafeDialer_AllowedDomains(t *testing.T) { - t.Parallel() - opts := &ClientOptions{ - AllowLocalhost: false, - AllowedDomains: []string{"app.crowdsec.net", "hub.crowdsec.net"}, - DialTimeout: time.Second, - } - dialer := safeDialer(opts) - - // Test that allowed domain passes validation (we can't actually connect) - // This is a structural test - we're verifying the domain check passes - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - // This will fail to connect (no server) but should NOT fail validation - _, err := dialer(ctx, "tcp", "app.crowdsec.net:443") - if err != nil { - // Check it's a connection error, not a validation error - if _, ok := err.(*net.OpError); !ok { - // Context deadline exceeded is also acceptable (DNS/connection timeout) - if err != context.DeadlineExceeded { - t.Logf("Got expected error type for allowed domain: %T: %v", err, err) - } - } - } -} - func TestNewSafeHTTPClient_DefaultOptions(t *testing.T) { t.Parallel() client := NewSafeHTTPClient() @@ -288,21 +261,6 @@ func TestNewSafeHTTPClient_WithMaxRedirects(t *testing.T) { } } -func TestNewSafeHTTPClient_WithAllowedDomains(t *testing.T) { - t.Parallel() - client := NewSafeHTTPClient( - WithTimeout(2*time.Second), - WithAllowedDomains("example.com"), - ) - - if client == nil { - t.Fatal("NewSafeHTTPClient() returned nil") - } - - // We can't actually connect, but we verify the client is created - // with the correct configuration -} - func TestClientOptions_Defaults(t *testing.T) { t.Parallel() opts := defaultOptions() @@ -846,7 +804,6 @@ func TestClientOptions_AllFunctionalOptions(t *testing.T) { client := NewSafeHTTPClient( WithTimeout(15*time.Second), WithAllowLocalhost(), - WithAllowedDomains("example.com", "api.example.com"), WithMaxRedirects(5), WithDialTimeout(3*time.Second), ) diff --git a/docs/features.md b/docs/features.md index 75d4777f3..04f830b25 100644 --- a/docs/features.md +++ b/docs/features.md @@ -105,6 +105,8 @@ Protect your applications using behavior-based threat detection powered by a glo Preset changes are backed up first and rolled back automatically if anything goes wrong, and CrowdSec's downloaded data now survives container recreation. +The CrowdSec file editor no longer lists CrowdSec's account and connection credential files, and backups are shown by name. + → [Learn More](features/crowdsec.md) • [Setup Guide](guides/crowdsec-setup.md) --- diff --git a/docs/features/crowdsec.md b/docs/features/crowdsec.md index 2b1c66e79..c83193053 100644 --- a/docs/features/crowdsec.md +++ b/docs/features/crowdsec.md @@ -65,6 +65,8 @@ Hub presets are applied on the server too, with the same safety net: before anyt - Charon keeps the 5 most recent full backups and the 10 most recent single-file backups, and quietly deletes older ones. - Only one change runs at a time, so two edits can't collide. - The config file editor only accepts common text and settings files (`.yaml`, `.yml`, `.json`, `.txt`, `.conf`) up to 1 MiB, and validates file type, size and YAML. +- The config file editor does not list CrowdSec's account and connection credential files. They stay safely on disk; an operator can still reach them from the container shell if needed. +- Backups are shown by name and are stored next to the CrowdSec data folder (for example `/app/data/`). Full snapshots are named `crowdsec.backup.`, single-file saves `crowdsec.filebackup.`. Saved acquisition settings are backed up as `acquis.yaml.backup.` right beside the acquisition file instead. To recover by hand, stop CrowdSec, copy the backup you want back into place, and start it again. ### Console Enrollment diff --git a/docs/plans/archive/crowdsec_preset_apply_hardening_1525_spec.md b/docs/plans/archive/crowdsec_preset_apply_hardening_1525_spec.md new file mode 100644 index 000000000..5be618358 --- /dev/null +++ b/docs/plans/archive/crowdsec_preset_apply_hardening_1525_spec.md @@ -0,0 +1,286 @@ +# Plan: CrowdSec preset-apply and data/hub persistence hardening (follow-ups of PR #1513) + +Type: ONE branch, ONE PR into `development`: `fix/crowdsec-preset-apply-hardening`. Medium/large fix (touches Go backend, React frontend, Dockerfile, entrypoint, E2E, docs), so per CLAUDE.md it gets a dedicated branch and PR, never a direct commit to `development`. +Closes: #1514, #1515, #1516, #1517, #1518, #1523, #1524. +Status: rev 2 (addresses Supervisor CHANGES REQUESTED: 7 blocking items), open questions Q1-Q5 resolved by the user. +Predecessor: the #1513 plan (curated preset apply) that previously lived in this file; it merged as `3a76804b`. + +## 1. Introduction + +PR #1513 made curated CrowdSec presets server-authoritative. While verifying it in a rebuilt container, seven follow-ups were filed. They share one theme: **the CrowdSec preset/hub apply path and the persistent CrowdSec directory (`/app/data/crowdsec`) are not safe or durable**. Backups are taken by renaming the live directory, a generic file-write endpoint also renames the live directory, hub data files live on an ephemeral path, the bundled binary does not know its own version, and the tests that should guard this are tolerant skips. + +Goal: after this PR, applying/editing CrowdSec config can never destroy the live config, hub data files survive container recreation, `cscli version` is truthful, backups are bounded, and E2E/unit tests fail when any of this regresses. + +## 2. Research Findings (root cause per issue, CLAUDE.md "Context First") + +Key facts established from code (all paths verified in this repo): + +- `crowdsecDataDir = cfg.Security.CrowdSecConfigDir` (`backend/internal/api/routes/routes.go:1030`), set to `/app/data/crowdsec` by `Dockerfile:1142` (`CHARON_CROWDSEC_CONFIG_DIR`). The same directory is `CS_PERSIST_DIR` in `.docker/docker-entrypoint.sh`. It therefore contains the **whole persistent CrowdSec tree**: `config/` (the target of the runtime symlink `/etc/crowdsec -> /app/data/crowdsec/config`), `data/` (LAPI db), `hub_cache/`, `bouncer_key`. +- That same path is `HubService.DataDir` and `CrowdsecHandler.DataDir` (`crowdsec_handler.go:374-397`). Every "backup the DataDir" in the code is a backup directory next to it, `/app/data/crowdsec.backup.`. + +### 2.1 #1514 applyPresetLocally writes unvalidated client content (root cause is wider than the issue says) + +- Entry: `frontend/src/pages/CrowdSecConfig.tsx:429` `applyPresetLocally`; reached only when apply returns HTTP 501 for a non-curated (hub) preset (line ~508). It writes `presetPreview` (text from the pull preview, held in client state) to `selectedPath ?? files[0]` through `writeCrowdsecFile` (`frontend/src/api/crowdsec.ts`). +- Confirmed: the backend **never returns 501** for apply any more (grep of `crowdsec_handler.go` shows no `StatusNotImplemented`). The fallback is dead code in practice and can only be triggered by a proxy/misconfig, which makes removing it risk-free. +- Transformation/persistence: `POST /admin/crowdsec/file` -> `CrowdsecHandler.WriteFile` (`crowdsec_handler.go:950`, registered under `managementAdmin`, so admin-only). Verified defects in the handler, independent of the frontend: + 1. **Destructive backup.** It `os.Rename(h.DataDir, backupDir)` (moves the *entire* persistent CrowdSec tree away, config symlink target included), then `MkdirAll(filepath.Dir(p))` and writes just the one file. Everything else (config.yaml, hub, bouncer key, data) is left only in the backup dir. The live tree becomes a single file. There is no restore. This is the same defect class as #1515 and is a medium-severity data-loss bug, not just "unvalidated content". + 2. **The file endpoints get a validation pass.** The fix is described at policy level in 3.2. +- Exit: `{status:"written", backup}`; the UI toasts "File saved" / "Preset applied locally". +- Severity/impact: low-medium, authenticated admin only (not an unauthenticated vulnerability). See section 4. + +### 2.2 #1515 HubService.Apply renames the whole data dir (CONFIRMED in code; end-to-end not executed) + +- Entry: `ApplyPreset` -> `Hub.Apply` for non-curated presets (`hub_sync.go:602`). +- Transformation: `backupExisting` first tries `os.Rename(DataDir, .backup.)`; only if rename fails (EBUSY on a mount point, cross-device) does it fall back to `copyDir`. Then `runCSCLI` (`cscli hub update` + `cscli hub install `). +- Why it is wrong, confirmed by reading the code and by the behavior seen in a throwaway container (below): after the rename `/etc/crowdsec -> /app/data/crowdsec/config` dangles, so `cscli` cannot read `config.yaml` (observed error: `open /etc/crowdsec/config.yaml: no such file or directory`). The cscli path therefore fails, the code falls to `extractTarGz` into a freshly created `DataDir`, and on success the live dir holds only the extracted archive while config, hub, `bouncer_key`, LAPI db and `hub_cache` sit in the backup. CrowdSec is running while its db directory is moved. Rollback (`rollback`) does `RemoveAll(DataDir)` then renames the backup back, so it is also only valid for that directory and cannot undo anything `cscli` wrote outside it. +- Not confirmed: that a *real* hub-preset apply on a running container hits exactly this sequence (needs a hub preset pull + a running agent). The verification step is part of the gate in Commit 3. Whether the extract fallback ever "succeeds" with a usable tree is irrelevant; the rename itself is the defect. +- Fix direction (matches the issue): reuse the copy-based, symlink-preserving, db-excluding `backupCopy`/`restoreCopy` added for `ApplyCurated`. + +### 2.3 #1516 backup directories accumulate (CONFIRMED) + +Creators of `.backup.*`: `HubService.Apply` (rename, second-resolution timestamp), `ApplyCurated` (copy, microsecond timestamp), `CrowdsecHandler.ImportConfig` (`crowdsec_handler.go:696`, rename), `WriteFile` (rename). Nothing ever deletes any of them. grep for prune/retention in `backend/internal/crowdsec` returns nothing. Because the curated backup is a full copy of the persistent tree (minus the db) and the legacy ones are the whole tree, growth is unbounded on a volume the user sizes for a home server. + +### 2.4 #1517 handler file size (CONFIRMED) + +`crowdsec_handler.go` is 2,906 lines. Preset handlers live at lines ~990-1325 (`ListPresets`, `PullPreset`, `ApplyPreset`, `applyCuratedPreset`, `curatedPresetPreview`, `recordPresetEvent`, `applyFailureBody`, `respondApplySuccess`, `GetCachedPreset` at ~1426). Pure move, same package. + +### 2.5 #1518 tolerant Playwright tests (CONFIRMED) + +`tests/security/crowdsec-config.spec.ts` "Preset Management" block (lines ~73-145) and "Configuration Files" block are `isVisible().catch(() => false)` guarded and annotate "feature may not be implemented"; they cannot fail on a regression. `tests/security/crowdsec-preset-apply.spec.ts` (from #1513) is the model: route-stubs `**/api/v1/admin/crowdsec/presets/apply` and the file-write route and asserts toasts and write counts. Missing deterministic coverage: hub (non-curated) preset apply success/failure surfaced, no client-side write on any apply failure, file editor save/failure, file-list error cases. + +### 2.6 #1523 hub data files missing after container recreation (ROOT CAUSE CONFIRMED by experiment) + +- Config: CrowdSec's default `config_paths.data_dir` is `/var/lib/crowdsec/data/` and `hub_dir` is `/etc/crowdsec/hub/` (upstream `config/config.yaml`). The entrypoint (`.docker/docker-entrypoint.sh:310-319`) redirects **only** `db_path` to the volume. `Dockerfile:1089` creates `/var/lib/crowdsec/data` inside the image, so on a recreated container it is an empty directory (confirmed: the issue shows only `trace/`). `/etc/crowdsec` is a symlink into the volume so the **hub items persist but their data files do not**. +- `configs/crowdsec/install_hub_items.sh` skips items for which `cscli inspect ` exits 0 ("already installed, skipping"). +- Experiment (throwaway `charon:local` container run with `--entrypoint sh`, no volumes, no live host touched): + - Install `crowdsecurity/http-backdoors-attempts` -> `backdoors.txt` appears in `data_dir`. + - Wipe `data_dir` (simulating recreate), keep the hub: `cscli scenarios install ` -> "Nothing to install or remove", data dir stays empty. `cscli scenarios install --force ` -> same, still empty. **`cscli hub upgrade` repopulates `backdoors.txt`, and so does `cscli hub upgrade --force`.** The issue's suggestion "`install --force` or `hub upgrade`" is therefore half right: only `hub upgrade` works. +- **Additional defect found (CONFIRMED, not in the issue):** on a hub with an index but nothing installed, `cscli scenarios inspect crowdsecurity/http-backdoors-attempts` exits **0** and prints `installed:false` metadata. `install_if_missing` uses the exit code, so on a fresh volume it may print "already installed, skipping" and install nothing. (In the experiment inspect exited 1 when the item was installed but its data file was missing, so the exit code is not a reliable installed/absent signal in either direction.) #1513's `ApplyCurated` already uses the correct signal: `inspect -o json` -> top-level `installed`. +- Not confirmed: whether the missing-install path is masked in real startups by something else (e.g. the collections installing their children). Needs the recreate test in Commit 6 on a clean volume. + +### 2.7 #1524 empty version (ROOT CAUSE CONFIRMED; the fix has a gotcha) + +- Confirmed: the injected symbol is wrong. In CrowdSec v1.8.1, `pkg/cwversion/version.go` declares only `Codename` and `Libre2` (and no `Version`); the version comes from `github.com/crowdsecurity/go-cs-lib/version` (`Version`, `BuildDate`, `Tag`, `System`), pinned at `go-cs-lib v0.0.25` by CrowdSec v1.8.1's `go.mod`. Upstream's own Makefile uses `-X 'github.com/crowdsecurity/go-cs-lib/version.Version=$(BUILD_VERSION)'` (plus `BuildDate`, `Tag`, and `cwversion.Codename`). `-X` on a nonexistent symbol is silently ignored, so both `Dockerfile:915` and `:923` are no-ops. Observed in the built image: `cscli version` prints empty `version:`, `Codename:`, `BuildDate:`, and `cscli` logs `Crowdsec version is not set, using hub branch 'master'`. +- The existing app-stage sanity check (`Dockerfile` ~L1054) says "CrowdSec 1.8.x prints an empty `version:` field here regardless of the -X ldflag". That comment is wrong and hides the bug; it must be corrected. +- **Gotcha, verified in `cmd/crowdsec-cli/core/require/branch.go` (v1.8.1):** with a real version cscli calls `https://version.crowdsec.net/latest` on every hub command and returns a hard error if it cannot reach it; if the running version equals latest it uses `master` (identical to today), if it is older than latest it uses the branch named after the version. `https://hub-cdn.crowdsec.net/v1.8.1/.index.json` returns **404** today (v1.8.0, v1.7.8 and master return 200). So a correct version could (a) make `cscli hub *` fail offline/air-gapped and (b) break hub updates once 1.8.1 is no longer the latest and the CDN has no `v1.8.1` branch. The issue's "hub items may be newer than the engine supports" claim is also overstated: while 1.8.1 is the latest release the behavior is already `master`. The fix therefore needs an explicit hub-branch decision (see D1) rather than a bare `-X` correction. +- The toolchain delivery path: the default Dockerfile build consumes the prebuilt `ghcr.io/wikid82/charon-toolchain` image (content-hash tag `CHARON_TOOLCHAIN_TAG`, `Dockerfile:22`). Editing the `crowdsec-inline` stage changes the key, `.github/workflows/toolchain-image.yml` builds the new image and its bot commits the new TAG/DIGEST onto the PR head branch (workflow lines ~365-420) so `verify-toolchain-pin` goes green. Until that happens the app image still contains the old binary, so the app-side assertion cannot be added in the same commit as the ldflags change. + +### 2.8 Reproduction/evidence commands run for this plan (all foreground, repo-local or throwaway container) + +`gh issue view` for each issue, `gh pr view 1513`; code reads listed above; `curl` of upstream v1.8.1 source files and `hub-cdn.crowdsec.net//.index.json`; shallow clone of upstream v1.8.1 into the session scratchpad; throwaway `docker run --rm --entrypoint sh charon:local` experiments (data-dir wipe, inspect exit codes, `hub upgrade`). Nothing on live hosts or volumes was touched. + +## 3. Technical Specifications + +### 3.1 Shared backup helper, Apply flow, ImportConfig (#1515, #1516) + +New file `backend/internal/crowdsec/backup.go` (move `backupCopy`, `restoreCopy`, `isLiveDBFile`, `emptyDirExcept` out of `curated_apply.go`; the move itself changes no behavior): + +- **Two namespaces, two caps, so they cannot evict each other (blocking item 3).** + - Full-tree snapshots: backup dirs next to `DataDir`: `.backup.`, `const DefaultBackupRetention = 5`. + - Single-file edit backups (from `WriteFile`, section 3.2): backup dirs next to `DataDir`: `.filebackup./`, `const DefaultFileBackupRetention = 10`. The `.backup.*` glob never matches `.filebackup.*`. +- **Snapshot exclusions (blocking item 4).** `snapshot` copies `DataDir` (symlinks preserved) but **excludes**: the live `crowdsec.db`, `-wal`, `-shm` (anywhere), and the top-level `data/` directory (LAPI db plus hub data files and the GeoLite2 mmdb, which after #1523 live there; hub data files are recoverable with the gated `cscli hub upgrade`, the db is owned by the running engine) and the top-level `hub_cache/` (regenerable download cache). `restore` empties `DataDir` except those same excluded entries and copies the snapshot back, so it never touches `data/` or `hub_cache/`. Only the *top-level* `data/` and `hub_cache/` are excluded; a nested directory with the same name inside `config/` is still backed up. Result: a snapshot is essentially `config/` plus `bouncer_key` (small), x5 retained. +- `func (s *HubService) snapshot() (path string, err error)`: microsecond timestamp name, created `0o700` with exclusive `os.Mkdir`. +- `func (s *HubService) restore(path string) error`. +- `func PruneBackups(dataDir, kind string, keep int) (removed []string, err error)` where `kind` is `backup` or `filebackup`. Lists siblings matching `..*`, `Lstat`-checks each is a real directory (never follows or deletes symlinks or plain files), **parses the timestamp from the directory name** (both the legacy `20060102-150405` and the microsecond `20060102-150405.000000` layouts; names that do not parse are ignored, never deleted), sorts newest first, deletes everything past `keep`. `keep < 1` is clamped to 1 so the newest is always kept. Never touches `dataDir`. Single-removal errors are logged and aggregated, never failing the caller. +- **Prune runs after every attempt, successful or failed**, always preserving the newest `keep` snapshots. A failed apply's own snapshot is the newest, so it is always retained for manual recovery. + +**`HubService.Apply` new control flow (blocking item 2).** Hold `HubService.mu` and the handler-level lock (section 6). +1. Validate slug; load cache meta (non-fatal). +2. `snapshot()`; any error -> return failure, nothing changed. `BackupPath` is set only after success. The old "read the archive into memory before the backup because the cache lives in `DataDir`" workaround is **removed**: nothing is moved any more, and `hub_cache/` is excluded from snapshots and from restore, so the archive stays valid for the whole call. +3. If `cscli` is available: `cscli hub update` (non-fatal), `cscli hub install `. Success -> status `applied`, `UsedCSCLI=true`, reload, prune, return. +4. **A cscli failure still falls back to the cached-archive extract** (this keeps the offline/hub-unreachable behavior that exists today). Before extracting, `restore(snapshot)` so the extract starts from a clean pre-apply tree and any partial files `cscli` wrote are removed (this closes the partial-overlay risk of overlaying an archive on a half-installed state). +5. Extract `extractTarGz` onto `DataDir` (cache refresh from the hub if the cache is missing, as today). **Residual risk, documented:** extraction overlays archive files on top of the live tree rather than replacing it, so files in the archive replace same-named live files; mitigation is the snapshot. Any extract/refresh failure -> `restore(snapshot)` and return the error with `BackupPath`. +6. Success -> status `applied`, `UsedCSCLI=false`, reload, prune. +`backupExisting`, `rollback`, `emptyDir`, and `copyDir` (if now unused) are deleted. + +**`CrowdsecHandler.ImportConfig` (blocking item 6): converted to snapshot/restore semantics, never renaming `DataDir`.** Flow: handler lock -> `snapshot()` -> empty `DataDir` except the snapshot-excluded entries (so the live db, `data/`, `hub_cache/` survive the import) -> extract the uploaded archive, **skipping any archive entry that is a live db file (`isLiveDBFile`) or lies under the top-level `data/` or `hub_cache/`** so an upload can never overwrite engine-owned state -> on any failure `restore(snapshot)` and return the error -> prune. Net behavior difference: the live db and `data/` are no longer moved or lost by an import; everything else is replaced exactly as before. Existing `ImportConfig` tests are updated, plus the new tests listed in Commit 3. + +### 3.2 File endpoints hardening (#1514) + +Details of the validation defects and their fixes are intentionally not written here (see section 4). Public-safe design: + +- New file `backend/internal/api/handlers/crowdsec_files.go` with a single shared path-resolution helper used by `ReadFile`, `WriteFile` and `ListFiles`, so all three apply one policy ("harden file-endpoint path validation"). +- Write policy: bounded body size (1 MiB); allowed file types (Q3); YAML files must parse; protected files and trees (secrets, databases, caches, engine data) are never writable; `.yaml/.yml` parsing uses `gopkg.in/yaml.v3` if already a dependency (verify). +- **Symlink policy (blocking item 7):** the editor never writes through or replaces a symlink (HTTP 400), so symlinked hub items are read-only from the editor. Reads stay confined to `DataDir`, and symlinked hub items that resolve inside it stay readable. Test in Commit 4. +- **Two predicates, not one.** The viewer today lists and reads any file under `DataDir` (hub items, `config.yaml`, etc.), and filtering the list by the write predicate would hide files users can currently read. So: `isReadable(rel)` governs `ListFiles` and `ReadFile` (secrets and databases such as `bouncer_key`, `*.db*`, and the `hub_cache/` and `data/` trees are neither listed nor readable; everything else stays visible, including symlinked hub items), and the stricter `isWritable(rel)` governs `WriteFile`. A file that is readable but not writable (symlinked hub items, non-allowlisted types) stays viewable and its save returns the server's 400 message, which the existing `writeMutation` error path already shows; no frontend contract change. `ExportConfig` is unaffected. Commit 4 tests: `ReadFile` refuses `bouncer_key` and db files, reads a symlinked hub item that resolves inside `DataDir`, and refuses a symlink resolving outside it; `ListFiles` omits them but still lists a symlinked hub item and `config.yaml`; every file `WriteFile` accepts is also listed. +- `WriteFile` algorithm: lock -> validate -> if the target exists, copy just that file to `.filebackup./` -> atomic write (temp file in the same directory, `fsync`, `rename`, preserve mode, default `0o600`) -> `PruneBackups(kind=filebackup, 10)` -> 200 `{status:"written", backup}`. `DataDir` is never renamed. +- Error contract (JSON `{"error": ...}`): 400 invalid request / path / type / YAML / symlink, 413 too large, 500 write failure. +- No new endpoints, no schema changes, no frontend contract changes. + +### 3.3 Frontend (#1514) + +`frontend/src/pages/CrowdSecConfig.tsx`: delete `applyPresetLocally`, the `501` branch in `handleApplyPreset`, the `applied-locally` apply-info status, and anything left unused (`backupMutation` if only used there, imports, `presetPreview` write path). Non-2xx apply for hub presets is surfaced like curated: server error text, backup path if present, no write. The generic file editor (`writeMutation`) is unchanged but now shows server 400/413 messages. Update Vitest files: `CrowdSecConfig.test.tsx`, `.coverage.test.tsx`, `.spec.tsx`, `.curatedApply.test.tsx` (remove tests that assert the local fallback; add "501 for a hub preset shows an error and performs no file write"). + +### 3.4 Persistent CrowdSec data dir and hub data (#1523) + +`.docker/docker-entrypoint.sh` (next to the existing `db_path` redirect, ~L310-320): + +- `sed` `data_dir: /var/lib/crowdsec/data/` -> `data_dir: ${CS_DATA_DIR}/` in `$CS_CONFIG_DIR/config.yaml` (idempotent; works for both fresh and already-initialized volumes), with the same verify/echo pattern as the db redirect. `CS_DATA_DIR` is `/app/data/crowdsec/data` and is already `mkdir`ed and chowned. +- **Gated, never unconditional (blocking item 5).** After `install_hub_items.sh`, the entrypoint runs `cscli hub upgrade` **only if** at least one installed item's data files are missing. Detection: for each installed item (`cscli list -o json`), check that every file the item declares is present under `data_dir`; the exact JSON field for declared data files is confirmed by the implementer against `cscli inspect -o json` before coding (open implementation detail, verified in the Commit 6 gate). Fallback if the field is not exposed: treat an empty `data_dir` (ignoring `trace/` and the db) with at least one installed item as "missing". The call is bounded (`timeout 120s`), failure is a logged warning and never fatal, and it never uses `--force`. A normal restart with intact data files performs no upgrade, so items are never silently upgraded on a healthy start. Note `hub upgrade` upgrades every installed item; this is acceptable only in the recovery case it is gated on, and the log line must say so. +- It is skipped in the same condition as the rest of hub init (`CHARON_SECURITY_TESTS_ENABLED=false`). + +`configs/crowdsec/install_hub_items.sh`: change `install_if_missing` to decide on `cscli inspect -o json` reporting `"installed": true`, matching `ApplyCurated.verifyItem`. **Verified:** `jq` is not in the `charon:local` image, so use `grep -q '"installed": *true'` (not jq). **Verified:** `inspect -o json` still prints full JSON and exits 0 when an item is installed but its data file is missing, so the JSON signal is reliable in exactly the recreate scenario (the plain-text `inspect` exit code was not). Keep the "failed to install" warnings non-fatal. + +`Dockerfile`: keep creating `/var/lib/crowdsec/data` (a stale `data_dir` on an un-migrated config must still exist); no other change. +`.docker/compose/docker-compose.playwright-ci.yml:137` currently mounts a volume at `/var/lib/crowdsec/data`; repoint or remove it so E2E exercises the same persistent path as production. `docs/security.md` hardened-deployment text that tmpfs-mounts `/var/lib/crowdsec` stays valid (the db and data now live under `/app/data/crowdsec`) but its description must be updated. +`scripts/diagnose-crowdsec.sh` default `DATA_DIR` updated to the new location. + +Optional passive health signal (Q4 resolved: log line only, no UI): log a startup summary line listing installed-but-missing data files. No banner or badge (Passive UI, no nudges). + +### 3.5 Version injection (#1524) + +`Dockerfile` `crowdsec-inline` stage (both `xx-go build` commands, ~L915 and ~L923): replace the ldflags with `-s -w -X github.com/crowdsecurity/go-cs-lib/version.Version=v${CROWDSEC_VERSION} -X github.com/crowdsecurity/go-cs-lib/version.BuildDate=`. Do not set `Tag` (empty `Tag` leaves `version.String()` equal to `Version`). Do not add `BuildDate` from the wall clock: that would break the content-addressed, reproducible toolchain key; prefer the release date derived from the pinned tag or omit it. + +App-side assertion (second commit, after the toolchain bot has pushed the new TAG/DIGEST to the PR branch): in the existing N5 `RUN` block (~L1054) replace the wrong comment and add `grep -q "^version: v${CROWDSEC_VERSION}\$"` against `cscli version`. The final stage must redeclare `ARG CROWDSEC_VERSION` (currently declared only globally at L41 and in the caddy/crowdsec-inline stages; a global ARG is not visible inside a later stage without redeclaring). The build fails if the symbol ever silently no-ops again. + +Hub branch (decision D1, recommended): set `cscli.hub_branch` explicitly in the entrypoint-managed `config.yaml` so behavior is deterministic and does not depend on `version.crowdsec.net` reachability or on `hub-cdn` having a `v1.8.1` branch. Verified in upstream v1.8.1 `chooseBranch`: a non-empty `cscli.hub_branch` is returned immediately, before the `version.crowdsec.net/latest` lookup, so setting it removes both the network dependency and the CDN-404 risk. The online/offline test result is recorded in the PR description (Commit 8 gate). + +### 3.6 Handler split (#1517) + +Move to `backend/internal/api/handlers/crowdsec_preset_handler.go` (same package, no behavior change, no export changes): `ListPresets`, `PullPreset`, `ApplyPreset`, `applyCuratedPreset`, `curatedPresetPreview`, `recordPresetEvent`, `applyFailureBody`, `respondApplySuccess`, `GetCachedPreset`, and preset-only helpers. Move the matching unit tests into `crowdsec_preset_handler_test.go` files where they are clearly preset-only (leave shared fixtures where they are). `RegisterRoutes` stays in `crowdsec_handler.go`. The new file must be < 700 lines. + +## 4. Priorities & Ordering (CLAUDE.md "Findings Triage") + +None of the seven issues is user-submitted (all filed by the maintainer from PR #1513). Rules applied: bug fixes before new implementation (there is no `feat` here); critical/high ships as its own PR first; medium/low is scheduled after. + +Severity after root-causing (some differ from the issue labels): + +| Rank | Issue | Severity | Why | +|------|-------|----------|-----| +| 1 | #1523 data files missing after recreate | **medium** (issue says medium) | Silent degraded detection and no GeoIP on every recreated container; install guard defect makes fresh volumes suspect. Highest user impact. | +| 2 | #1514 + `WriteFile` destructive rename | **medium** (raised from low-medium) | Admin action can wipe the live CrowdSec tree; weak input validation. Security-sensitive (below). | +| 3 | #1515 `HubService.Apply` rename | **low-medium, confirmed** | Same defect class and same helper as #1514/#1516; fixed together. | +| 4 | #1524 empty version / hub branch | **low-medium** | Fix is easy but changes runtime hub behavior (offline failure, CDN 404 risk); needs the D1 decision and the toolchain bot. Scheduled after the fixes it must not destabilize. | +| 5 | #1516 backup accumulation | low | Piggybacks on the shared helper. | +| 6 | #1518 tolerant E2E | low | Test hardening; first in commit order (as `fixme` specs) and enabled last. | +| 7 | #1517 handler split | low | `refactor`, no behavior change; sequenced first in the *commit* order (mechanical, makes later diffs reviewable) but lowest in *priority*. | + +No finding is critical or high, so none must ship as its own PR ahead of this work. Priority above governs attention and what to cut first under contingency; commit order in section 5 is dependency-driven. + +**Security handling (#1514).** It is admin-only and already public as an issue. Assessment: low severity (authenticated admin only; the endpoint grants nothing beyond what an admin already has). **Q2 resolved by the user: #1514 is already public, so no private advisory is opened unless root-causing shows real impact beyond what an admin already has; it does not, so the default is NO advisory.** Handling: +- The existing public issue #1514 stays as is. Keep PR text, commit bodies and code comments free of exploit specifics anyway (no value in handing detail to un-upgraded installs). +- Exactly one commit in the PR carries the `(security)` scope, and its subject must stay in the vague category-only form: `fix(security): harden CrowdSec configuration file handling`. Keep the subject category-only, and Commit 4's body and the PR description stay equally vague (for example "tighten validation in the CrowdSec configuration file endpoints"). Every commit subject in a feature PR merge-commit appears verbatim in the public changelog, so the other commits use plain scopes (`fix:`, `refactor:`, `test:`, `docs:`, `chore:`) and describe user-visible behavior only. The destructive-rename data-loss fix is a plain `fix:` (it is a reliability bug, not an exploit). + +**Out-of-scope findings discovered while planning:** none left open. Items found beyond the issue text are folded in (install guard, wrong N5 comment, `ImportConfig` conversion, playwright-ci compose mount). If implementation finds anything else, file it per the triage rule at that time. + +## 5. Branch/PR Decision and Commit Slicing Strategy + +### 5.1 Branch/PR decision: ONE branch, ONE PR + +Single branch `fix/crowdsec-preset-apply-hardening` -> PR into `development`, titled e.g. `fix: harden CrowdSec preset apply and keep hub data across container recreation`. + +Why one PR (cohesion): all seven issues are one defect family, "the CrowdSec persistent tree is mutated unsafely and its hub/data state is not durable", and they share code: #1514, #1515 and #1516 all consume the same new `backup.go` helper (a single fix for rename-based backups, the file endpoint, and pruning); #1517 is the file those changes land in; #1518 is the test net for #1514/#1515; #1523 and #1524 are the same persistent-dir/hub-state story at the container layer and are both verified by the same rebuilt-container gate. Splitting would leave the helper without its callers, the E2E net without the behavior, or a half-fixed persistence story. + +Considered splits and why rejected: +- Docker/entrypoint (#1523, #1524) vs Go/frontend: they can build independently, but they do not conflict, share a single verification environment (rebuilt `charon:local`), and splitting would double the merge-soak the maintainer applies before `main`. The #1524 toolchain bot is explicitly designed to push the pin bump onto the PR head branch, so it works inside this PR. +- `(security)` fix alone: not required; severity is low (authenticated admin), and the rule "own PR" applies to critical/high only. + +**Contingency without splitting:** if Commits 7-8 (#1524) cannot be validated (hub-branch decision unresolved, toolchain build blocked), drop those commits from the PR and leave #1524 open rather than opening a second PR for it; the remaining six issues still form a complete, coherent PR. Rollback of the PR as a whole is a revert of the merge commit; a revert leaves a valid `data_dir` in `config.yaml` (the old image still creates the default directory), so nothing breaks. + +### 5.2 Ordered commits (all on the one branch) + +Every command run by implementers is foreground/blocking per CLAUDE.md. Each commit must build and pass its gate; the PR as a whole passes the full Definition of Done (section 8). + +**Commit 1 - `test: add deterministic E2E specs for CrowdSec preset and file flows (fixme)`** (#1518, step 1 of the suggested sequence) +- Files: new `tests/security/crowdsec-hub-preset-apply.spec.ts`, new `tests/security/crowdsec-file-editor.spec.ts`. +- Content, all stubbed with `page.route` like `crowdsec-preset-apply.spec.ts`, marked `test.fixme` until their feature commits: hub preset apply success shows success only when body `status==='applied'`; hub preset apply 500/503/504 shows an error and **zero** `POST /admin/crowdsec/file` calls; hub preset apply 501 shows an error and zero writes; file editor save success posts exactly one write for exactly the selected path; file editor shows the server message for 400 (disallowed type / invalid YAML) and 413. +- Depends on: none. Gate: `npx playwright test tests/security/crowdsec-hub-preset-apply.spec.ts tests/security/crowdsec-file-editor.spec.ts --project=security-tests` runs and reports the tests as fixme/skipped with no failures; lint/type-check clean. + +**Commit 2 - `refactor: move CrowdSec preset handlers into their own file`** (#1517) +- Files: new `backend/internal/api/handlers/crowdsec_preset_handler.go`; edit `crowdsec_handler.go`; move/split preset-only tests into `crowdsec_preset_handler_test.go`. +- Pure move: no signature, route, or behavior changes. Depends on: none. +- Gate: `cd backend && go build ./... && go test ./internal/api/handlers/... -race` all green with the same test count; `git diff --stat -M` shows moves, not rewrites; `make lint-fast`. + +**Commit 3 - `fix: keep CrowdSec backups bounded and copy-based for preset apply`** (#1515, #1516) +- Files: new `backend/internal/crowdsec/backup.go` + `backup_test.go`; edit `hub_sync.go` (`Apply` per the flow in 3.1; remove `backupExisting`/`rollback`/`emptyDir`/dead `copyDir`), `curated_apply.go` (move helpers, add prune call), `crowdsec_handler.go` `ImportConfig` (snapshot/restore conversion, handler lock), `hub_pull_apply_test.go`, `device_busy_test.go` (the rename/EBUSY tests are obsolete and are replaced by copy-path tests). +- Depends on: Commit 2 (preset code already in its own file; `ImportConfig` stays in the main handler). +- Unit tests (new, all with `t.TempDir()`): snapshot excludes live db files, top-level `data/` and `hub_cache/`, preserves symlinks, and still copies a nested directory named `data` under `config/`; **size test:** a large file under `data/` and under `hub_cache/` is not copied while a small `config/` file is (assert snapshot size); restore leaves live db, `data/` and `hub_cache/` untouched; `PruneBackups` orders by the timestamp parsed from the name (not mtime; touching an old dir does not save it), handles both name layouts, ignores unparsable names, non-matching names, symlinks and plain files, never deletes `dataDir`, clamps `keep<1` to 1, always keeps the newest, handles a missing parent, and `backup` and `filebackup` namespaces never evict each other; `HubService.Apply` flows: (a) cscli success leaves `DataDir` in place (inode unchanged) and prunes; (b) **cscli failure -> snapshot restored -> cached-archive extract succeeds -> `applied`, `UsedCSCLI=false`, no cscli leftovers**; (c) cscli failure and extract failure -> snapshot restored, error returned with `BackupPath`; (d) cscli failure with cache miss and hub refresh failure -> restored; (e) prune also runs after a failed apply and keeps the failed apply's snapshot; `ImportConfig`: success replaces config but preserves live db/`data/`/`hub_cache/` and never renames `DataDir`; extract failure restores the previous config; prune runs; concurrent `ImportConfig` + apply serialize on the handler lock; an uploaded archive containing `crowdsec.db`, `data/x` and `hub_cache/y` entries extracts none of them. +- Gate: `go test ./internal/crowdsec/... ./internal/api/handlers/... -race`; package coverage not below the current level; **rebuilt-container verification of #1515**: pull + apply one hub preset against a running agent, confirm `/app/data/crowdsec` is never renamed (inode unchanged), the apply result is correct, and at most 5 full-snapshot dirs remain after 7 applies (this also settles the "not confirmed" item in 2.2). + +**Commit 4 - `fix(security): harden CrowdSec configuration file handling`** (#1514 backend; subject intentionally vague, see section 4) +- Files: new `backend/internal/api/handlers/crowdsec_files.go` + `crowdsec_files_test.go`; edit `crowdsec_handler.go` (`ListFiles`, `ReadFile`, `WriteFile` delegate to the shared helper); update existing `TestCrowdsec_WriteFile_*` tests in `crowdsec_handler_coverage_test.go`. +- Depends on: Commit 3 (per-file backups use the `filebackup` namespace and handler lock). Commit body and PR text stay vague (section 4). +- Unit tests (categories only here): path validation rejection cases, symlink policy (write through or replacement of a symlink is refused, a regular file under `DataDir` is written), type/content/size validation (disallowed type, invalid YAML, oversize -> 413, protected files), atomic write leaves no temp file on failure, `DataDir` and unrelated files untouched after a write (regression test for the destructive rename), `filebackup` contains only the previous version of the target and is capped independently of full snapshots, the read/list predicate and the stricter write predicate behave as in 3.2, `ReadFile` shares the validation. +- Gate: `go test ./internal/api/handlers/... -race`, `make lint-fast`; GORM scan not required (no models); the Playwright file-editor spec enabled in Commit 9 depends on this behavior. + +**Commit 5 - `fix: remove the dead local-write fallback from CrowdSec preset apply`** (#1514 frontend) +- Files: `frontend/src/pages/CrowdSecConfig.tsx`, its four test files under `frontend/src/pages/__tests__/`, `frontend/src/api/crowdsec.ts` only if a now-unused export remains. +- Depends on: none technically (the backend never returns 501); ordered after Commit 4 so the whole #1514 story is reviewable in sequence. +- Gate: `cd frontend && npm run type-check && npx vitest run src/pages/__tests__/CrowdSecConfig*` green; no unused-export/lint findings; `npm run build`. + +**Commit 6 - `fix: persist CrowdSec hub data files across container recreation`** (#1523) +- Files: `.docker/docker-entrypoint.sh`, `configs/crowdsec/install_hub_items.sh`, `.docker/compose/docker-compose.playwright-ci.yml`, `scripts/diagnose-crowdsec.sh`, `docs/security.md` (wording), new integration test `scripts/crowdsec_data_persistence_test.sh` (follows the pattern of `scripts/crowdsec_startup_test.sh`). +- Depends on: none (independent of the Go commits; ordered after so a failing container gate does not block the Go work from being reviewed). +- Tests: shell-level test (start container with a named volume, wait for hub init, assert data files and `GeoLite2-City.mmdb` exist under `/app/data/crowdsec/data`, `docker rm` and recreate with the same volume, assert files are present and `crowdsec.log` has no `unable to init data for file` / `unable to initialize GeoIP` lines after enabling CrowdSec; also run it on a volume initialized by the old image to prove the sed migration). Add `shellcheck` cleanliness for changed scripts. +- Gate: rebuild `charon:local` (skill `docker-rebuild-e2e` / `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e`), run `scripts/crowdsec_data_persistence_test.sh` and `scripts/crowdsec_startup_test.sh`; the fresh-volume case must show the install guard now really installing. +- Additional tests: the gate must prove `hub upgrade` is **not** run on a restart with intact data files (log assertion) and **is** run (once, bounded) when data files are missing; `shellcheck` is a required gate for every changed or new shell script (`.docker/docker-entrypoint.sh`, `install_hub_items.sh`, the new test script). +- CI wiring and ignore files: wire `scripts/crowdsec_data_persistence_test.sh` into the existing CrowdSec integration job via a skill wrapper like `.github/skills/integration-test-crowdsec-startup-scripts` and the `integration-tests.yml` workflow (rather than a new workflow). Check `.dockerignore` (it already excludes `scripts/tests/` and the toolchain scripts; the new script must be excluded from the image the same way if it lives under `scripts/`), `.gitignore`, and `.codecov.yml` (shell scripts are not coverage-measured; confirm no new exclusion needed). +- Snapshot interaction (item 4 sequencing): Commit 3's snapshot already excludes top-level `data/` and `hub_cache/`, so Commit 6's relocation of `data_dir` into `/app/data/crowdsec/data` does not enlarge any backup; the Commit 3 size test is the regression guard. + +**Commit 7 - `fix: report the real CrowdSec version in the bundled binaries`** (#1524, step A: ldflags) +- Files: `Dockerfile` (`crowdsec-inline`, both `xx-go build` lines). Bot-authored follow-up commit on the branch updates `CHARON_TOOLCHAIN_TAG` / `CHARON_TOOLCHAIN_DIGEST` (do not hand-edit those lines; the workflow owns them). +- Depends on: none. **Wait here until the toolchain-image workflow has built and pushed the pin bump to the PR branch** (this is the only asynchronous step in the plan). +- Gate: `toolchain-image.yml` green on the PR (build + Trivy gate), `verify-toolchain-pin` green, and a local `make build-offline`-style build (`--build-arg CROWDSEC_BUILDER_SRC=crowdsec-inline`) shows `cscli version` -> `version: v1.8.1`. +- Expected CI state: after this commit `verify-toolchain-pin` (and any check that compares TAG/DIGEST to the recipe key) is **red until the bot pushes the new pin**. Implementers must `git pull` the bot's commit before starting Commit 8 and must not hand-edit the two ARG lines. + +**Commit 8 - `fix: fail the image build if CrowdSec reports no version and make the hub branch deterministic`** (#1524, step B) +- Files: `Dockerfile` (N5 block: correct the comment, add the version assertion, add `ARG CROWDSEC_VERSION` to the final stage), `.docker/docker-entrypoint.sh` (hub branch setting per D1). +- Depends on: Commit 7 and the bot pin bump. +- Tests / build-time assertion: the Dockerfile `RUN` fails with a clear message if `cscli version` does not print `version: v${CROWDSEC_VERSION}`. Prove it fails by temporarily building with a mismatched `--build-arg CROWDSEC_VERSION` against the pinned toolchain (expected hard failure), then with the correct value (pass). Runtime check in the rebuilt container: `cscli version` truthful; `cscli hub update` succeeds online; with the network blocked (`--network none`) hub commands behave as the D1 decision specifies (no hard failure when `hub_branch` is set). +- Gate: image builds; `scripts/crowdsec_startup_test.sh` green; both stderr lines `Crowdsec version is not set` and `unable to retrieve latest crowdsec version` absent from `docker logs` in the online and offline runs. +- Record in the PR description: `cscli.hub_branch` is honoured by v1.8.1 and skips the `version.crowdsec.net` lookup (source-verified in `chooseBranch`), plus the measured online and offline (`--network none`) results. + +**Commit 9 - `test: enable CrowdSec preset and file-editor E2E specs and drop tolerant ones`** (#1518, final step) +- Files: the two specs from Commit 1 (remove `fixme`), `tests/security/crowdsec-config.spec.ts` (delete the tolerant "Preset Management" assertions that are now covered by deterministic specs, or convert each remaining case to a deterministic stub; no `isVisible().catch(() => false)` guards, no "feature may not be implemented" annotations remain in `tests/security/crowdsec-*.spec.ts`). +- Depends on: Commits 4 and 5. +- Gate: `npx playwright test tests/security/crowdsec-hub-preset-apply.spec.ts tests/security/crowdsec-file-editor.spec.ts tests/security/crowdsec-preset-apply.spec.ts tests/security/crowdsec-config.spec.ts --project=security-tests` green. Mutation check: temporarily re-add the local-write fallback (or revert the success-status check) and confirm at least one of the new specs fails, then undo. Full-suite and cross-browser runs are CI-only. + +**Commit 10 - `docs: document CrowdSec preset apply safety and data persistence`** +- Files: `ARCHITECTURE.md`, `docs/features/crowdsec.md`, `docs/troubleshooting/` entry if appropriate (see 7). +- Depends on: all. Gate: markdown lint/pre-commit; `docs-site` manifest unchanged (new content goes into already-manifested `docs/features/` / `docs/troubleshooting/`). + +## 6. Decisions and Edge Cases + +- **D1 hub branch (Q1 resolved: option A).** Options: (A, recommended) set `cscli.hub_branch: master` explicitly: identical to today's effective behavior while 1.8.1 is the latest, no dependency on `version.crowdsec.net`, no 404 risk, truthful `cscli version`. (B) Leave auto-selection with the fixed version: matches upstream's intent but can hard-fail hub commands offline and break hub updates when 1.8.1 stops being latest and the CDN has no `v1.8.1` branch (404 today). (C) Pin to the version branch: broken today (404). Commit 8 gate must run the empirical online/offline test either way. +- **Locking (one mechanism).** A single handler-level `sync.Mutex` on `CrowdsecHandler` (e.g. `dataMu`) is held for the whole of `ApplyPreset` (both curated and hub paths), `ImportConfig` and `WriteFile`. `HubService.mu` remains as an inner guard for direct service callers; lock order is always handler mutex first, then `HubService.mu`, so no deadlock. No other lock is introduced. +- Edge cases to cover in tests: snapshot dir name collisions (microsecond timestamp plus exclusive `os.Mkdir`), parent dir not writable, `DataDir` being a mount point (the copy path must work where rename used to fall back), symlinks inside `DataDir` never followed by restore/prune/write, concurrent apply/import/file write serialized by the handler mutex (held for the full `ApplyTimeout`, so file saves block during a long apply; `ReadFile` and `ListFiles` stay lock-free), CrowdSec stopped, existing volumes whose `config.yaml` still points at `/var/lib/crowdsec/data` (sed migration, then the gated `hub upgrade` repopulates), tmpfs `/var/lib/crowdsec` deployments (no longer relevant for data after migration). +- Reload behavior from #1513 (SIGHUP via `reloadCrowdSec`) is unchanged and should also be triggered, best-effort, after the entrypoint-independent hub changes that happen at runtime only (not applicable to startup). + +## 7. Docs Impact + +- `ARCHITECTURE.md`: update the Layer 2 CrowdSec section (~L868-890): CrowdSec `data_dir` and LAPI db live under `/app/data/crowdsec/data` (persistent), hub data files are re-synced when missing via a gated `cscli hub upgrade`, preset applies and config edits are copy-based/atomic with bounded backup retention, `cscli version` reports the pinned version and the hub branch is explicit. The file layout table around L288-294 gets the data subdirectory note. +- `docs/features/crowdsec.md`: extend "Curated Presets" (L57-59) / Hub Presets with the backup retention (last N kept), that hub presets are also applied on the server with a rollback, and that the config editor only accepts specific file types with validation. Add a short troubleshooting entry for the `unable to init data for file` log line (what it meant, that it self-heals on restart after this release). Keep it novice-friendly. +- `docs/features.md`: no new capability; no change expected (confirm during Commit 10). +- `docs/security.md`: tmpfs `/var/lib/crowdsec` guidance wording (Commit 6). +- Changelog is generated from commit subjects; subjects in section 5.2 are written for that. + +## 8. Test Plan Summary and Definition of Done + +- **Backend unit:** new tests in Commits 3 and 4 (list above). Coverage: package and patch coverage must not drop; overall >= 85% (`scripts/go-test-coverage.sh`); `bash scripts/local-patch-report.sh` produces `test-results/local-patch-report.md/.json`. +- **Frontend unit:** updated Vitest in Commit 5; `scripts/frontend-test-coverage.sh` >= 85%; `npm run type-check`; `npm run build`. +- **Playwright (#1518):** deterministic stubbed specs per Commit 1/9, role-based locators, no `waitForTimeout`, no visibility-guard skips. Locally only the targeted spec files with `--project=security-tests` (security specs are excluded from the browser projects; other specs use `--project=firefox`); CI runs the full matrix. +- **Build-time assertion (#1524):** Dockerfile `RUN` check in Commit 8 with the negative (mismatched version) and positive proof recorded in the PR description. +- **Container integration:** `scripts/crowdsec_data_persistence_test.sh` and `scripts/crowdsec_startup_test.sh` (Commit 6/8). +- **Security scans:** this is a `fix:` with no new endpoints; CodeQL/Trivy are deferred to CI per CLAUDE.md, except that Commit 7's toolchain Trivy gate runs in CI by design. GORM scan not required (no model changes); run it only if a model is touched. +- **Full DoD checklist before merge (in order):** targeted Playwright (firefox) -> patch coverage preflight -> `lefthook run pre-commit` -> staticcheck (`make lint-fast`) -> backend/frontend coverage -> type-check -> `go build ./...` and `npm run build` -> clean-up (no debug prints, dead code, unused imports). Any failing test, even pre-existing, is fixed in this PR. +- **Pipeline after the plan:** Supervisor plan review -> user approval -> implement commit by commit (`backend-dev` for 2-4, `frontend-dev` for 5, `devops` for 6-8, `playwright-dev` for 1 and 9, `docs-writer` for 10) -> Supervisor implementation review -> `qa-security` last -> `docs-writer` pass. +- **Dispatch instructions (every subagent prompt must include these, verbatim in spirit):** run all tests, builds, coverage scripts, Playwright, linters, Docker builds and `lefthook` as **foreground, blocking** calls with the maximum timeout; never use `run_in_background`, `&`, `nohup` or end the turn waiting for a notification; if a call auto-backgrounds, immediately re-attach and poll until a real result; no worktrees; stay inside the repo (no live hosts); do not commit unless the orchestrator says so; Commit 4's text (body, comments, PR text) must stay vague per section 4; and the Commit 7 to Commit 8 hand-off requires pulling the toolchain bot's commit first. + +## 9. Acceptance Criteria + +1. Applying a hub preset or editing a CrowdSec file never renames, empties or removes `/app/data/crowdsec`; the live tree is intact after success and after any failure (tests prove it). +2. Full-tree snapshot dirs (`.backup.*`) are bounded to 5 and single-file edit backups (`.filebackup.*`) to 10, independently, after any apply/import/write attempt; snapshots exclude `data/` and `hub_cache/` so they stay small. +3. `POST /admin/crowdsec/file` validates paths, file types, YAML and size, refuses symlink writes, never renames `DataDir`, and `ReadFile`/`ListFiles` share the same validation; the frontend has no local-write fallback. +4. After `docker rm` + recreate with the same volume, the 12 listed hub data files and `GeoLite2-City.mmdb` exist and `crowdsec.log` has no `unable to init data` / GeoIP lines; fresh volumes actually install the items in `install_hub_items.sh`. +5. `cscli version` and `crowdsec -version` print `v1.8.1`; the image build fails if they do not; hub commands work online and offline per D1. +6. `crowdsec_handler.go` no longer contains the preset handlers; no behavior change from the move. +7. No tolerant "may not be implemented" Playwright tests remain for CrowdSec presets, and the mutation check in Commit 9 shows the new specs fail on regression. +8. Only one commit uses the `(security)` scope and its subject is the vague form above. + +## 10. Resolved Questions (all decided by the user) + +- Q1 (D1): RESOLVED: explicit `cscli.hub_branch: master`. +- Q2: RESOLVED by the user: no private advisory (issue is public; severity low, admin-only). Revisit only if implementation finds impact beyond admin privileges. +- Q3: RESOLVED: write allowlist, protected-file list, 1 MiB cap and the refuse-symlinks rule as specced in 3.2, with a separate looser read/list predicate. +- Q4: RESOLVED: startup log line only (no diagnostics field, no UI). +- Q5: RESOLVED: constants (5 full snapshots, 10 file backups), not configurable. diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 5be618358..ef4d3446b 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,286 +1,189 @@ -# Plan: CrowdSec preset-apply and data/hub persistence hardening (follow-ups of PR #1513) +# CrowdSec Follow-ups Batch: #1479, #1481, #1526, #1528 -Type: ONE branch, ONE PR into `development`: `fix/crowdsec-preset-apply-hardening`. Medium/large fix (touches Go backend, React frontend, Dockerfile, entrypoint, E2E, docs), so per CLAUDE.md it gets a dedicated branch and PR, never a direct commit to `development`. -Closes: #1514, #1515, #1516, #1517, #1518, #1523, #1524. -Status: rev 2 (addresses Supervisor CHANGES REQUESTED: 7 blocking items), open questions Q1-Q5 resolved by the user. -Predecessor: the #1513 plan (curated preset apply) that previously lived in this file; it merged as `3a76804b`. +Status: DRAFT rev 2 (supervisor changes applied) for re-review. Uncommitted. Separate from `docs/plans/current_spec.md`. +Base: `origin/development`, which already includes #1525 (113f646c). All line numbers below were re-verified against it; re-verify again immediately before implementation. ## 1. Introduction -PR #1513 made curated CrowdSec presets server-authoritative. While verifying it in a rebuilt container, seven follow-ups were filed. They share one theme: **the CrowdSec preset/hub apply path and the persistent CrowdSec directory (`/app/data/crowdsec`) are not safe or durable**. Backups are taken by renaming the live directory, a generic file-write endpoint also renames the live directory, hub data files live on an ephemeral path, the bundled binary does not know its own version, and the tests that should guard this are tolerant skips. +Four open issues, all authored by the maintainer (none is user-submitted), all low severity: + +| Issue | Type | Area | Summary | +|---|---|---|---| +| #1528 | fix / product decision | backend (+ small frontend) | F1 archive entry check too broad, F3 absolute server paths in CrowdSec responses, F5 credentials YAML files still listed/readable in the editor | +| #1479 | hardening | backend `network` pkg | `WithAllowedDomains` option stores a list nothing reads | +| #1481 | cleanup | backend `crowdsec` pkg | production hub URL validator carries test-only host allowances | +| #1526 | test | Playwright | tolerant CrowdSec specs can pass without asserting the feature works | -Goal: after this PR, applying/editing CrowdSec config can never destroy the live config, hub data files survive container recreation, `cscli version` is truthful, backups are bounded, and E2E/unit tests fail when any of this regresses. +### Is #1479 CrowdSec-specific? +No. The option lives in the generic `backend/internal/network/safeclient.go` (`ClientOptions.AllowedDomains`, `WithAllowedDomains`). The `crowdsec` label is there only because its single production caller is `newHubHTTPClient` in `backend/internal/crowdsec/hub_sync.go`. It is included because it was found in the same investigation as #1481 (#1476) and the right fix for #1481 (one validator, no test allowances in production) determines the right fix for #1479 (remove the unused option so `validateHubURL` is the only allowlist). -## 2. Research Findings (root cause per issue, CLAUDE.md "Context First") +## 2. Research Findings -Key facts established from code (all paths verified in this repo): +### 2.1 #1479 `WithAllowedDomains` +- Stored in `ClientOptions.AllowedDomains`; `safeDialer` and `validateRedirectTarget` never read it. Its doc comment ("only these domains will be allowed") is false: a latent footgun. +- Only production caller: `newHubHTTPClient` (hub_sync.go). That client uses default `MaxRedirects` 0, so redirect enforcement is moot. `validateHubURL` already holds its own copy of the same three hosts, so two lists exist and neither is authoritative. +- **Three test references** in `backend/internal/network/safeclient_test.go` must change, otherwise compilation breaks when the field is removed: + 1. `TestSafeDialer_AllowedDomains` (~L153-175): sets `AllowedDomains` on options and asserts nothing about them. Dead test. **Delete.** + 2. `TestNewSafeHTTPClient_WithAllowedDomains` (~L291): **Delete.** + 3. ~L849 (`WithAllowedDomains("example.com", "api.example.com")` inside a larger test): **Edit** to drop the option only; keep the rest of the test. +- Redirect/option parity (the issue's second ask): `safeDialer` and `validateRedirectTarget` both go through `opts.policy()` / `AddressPolicy.Blocked`, so `AllowLocalhost`/`AllowRFC1918`/`AllowCGNAT` already behave identically. Pin that with one table test running the same host/option matrix through both rather than changing behavior. + +### 2.2 #1481 test-only allowances +- `validateHubURL` (hub_sync.go ~L106-160) accepts at any scheme (http included): `localhost`, `127.0.0.1`, `::1`, `*.example.com`, `*.example`, `example.com`, `*.local`, `test.hub`, before the HTTPS + three-host allowlist check (`hub-data.crowdsec.net`, `hub.crowdsec.net`, `raw.githubusercontent.com`). +- Production impact (for a docs/changelog note): an operator-set `HUB_BASE_URL`/`HUB_MIRROR_BASE_URL` using http, localhost or `.local` already fails at the dial layer (`network.NewSafeHTTPClient` blocks loopback/private), so tightening the validator changes the error text and where it is raised, not what works. No supported configuration breaks. Mention in release notes in one sentence. +- Existing seam: package var `hubAllowLoopback` (~L95), toggled by `setHubAllowLoopbackForTest` (hub_sync_test.go ~L2252) and exercised by `TestNewHubHTTPClient_LoopbackSeam` (~L2258). +- **Only 2 `httptest.NewServer` uses** exist in the crowdsec package tests: `hub_sync_test.go` ~L106 and ~L2259 (the latter is inside `TestNewHubHTTPClient_LoopbackSeam`). Everything else uses a stub `RoundTripper` with fake hostnames. +- Test reference sizing: roughly 100-110 hits of `test.hub`/`example.com`/`.local`/`hub.example` across `hub_pull_apply_test.go`, `hub_sync_test.go`, `hub_sync_raw_index_test.go`, and the handler package (`crowdsec_preset_handler_test.go`, `crowdsec_pull_apply_integration_test.go`); re-grep before starting and do not rely on exact figures. Handler tests are in `package handlers`, cannot reach an unexported field, and embed hosts in index JSON fixtures such as `download_url`/`preview_url`. +- `hubBaseCandidates()` (~L215) builds `[HubBaseURL, MirrorBaseURL, defaultHubMirrorBaseURL, defaultHubBaseURL]` and de-duplicates with `uniqueStrings`. The defaults are `https://hub-data.crowdsec.net` and `https://raw.githubusercontent.com/crowdsecurity/hub/master`. If migrated tests reuse those same hosts for the primary/mirror roles, candidates collapse via `uniqueStrings` and fallback-order tests silently change meaning. + +### 2.3 #1528 +- **F1**: `extractTarGz` (hub_sync.go ~L946) uses `strings.HasPrefix(cleanName, "..")`, which also rejects legitimate names like `..hidden.yaml`. The containment check below it (`filepath.Rel`, ~L955) already uses `".."+separator`, so the first check is redundant-but-stricter. +- **F5**: `isReadable` (crowdsec_files.go ~L79) hides `bouncer_key`, `*.db*` and engine-owned trees, but `local_api_credentials.yaml` and `online_api_credentials.yaml` are only in `protectedWriteNames` (write-protected; still listed and readable). Real error messages today: reads of a hidden path return `invalid path` (`resolveReadable` first check ~L128, and the resolved-path re-check ~L149); writes of a hidden path return `file cannot be edited` (`resolveWritable` ~L167-169, `errFileCannotBeEdited`). Mixed-case names: `isReadable` lowercases the base only for the `bouncer_key` and `*.db*` checks, so once the credentials names are added to its deny rules (using the same lowercase comparison) mixed-case variants become hidden too, a deliberate tightening. Nothing in `frontend/src` or the Playwright specs references the credentials files; `docs/troubleshooting/crowdsec.md` (~L644) stays valid. +- **F5 scope limit: `ExportConfig` is not covered.** `ExportConfig` (crowdsec_handler.go ~L810-880) walks the whole `DataDir` and tars it with no `isReadable` filter (it only skips symlinks), so credentials files and `bouncer_key` remain downloadable by an authenticated admin through the export endpoint. The "consistent with `bouncer_key`" argument therefore applies to the editor list/read surface only, not to export. +- **F3 leak sites** (all admin-authenticated; absolute server paths or archive entry names reach the client): + 1. File write `backup` field: `crowdsec_files.go` ~L415 (from `crowdsec.BackupFile`, kind `filebackup`, a directory). + 2. Import `backup` field: `crowdsec_handler.go` ~L727 (from `Snapshot`, kind `backup`, a directory). + 3. Preset apply: `crowdsec_preset_handler.go` ~L353 and ~L365 (`res.BackupPath`). + 4. Acquisition update: `crowdsec_handler.go` ~L2030 (`backupPath`, a **file** named `acquis.yaml.backup.`, not a directory). + 5. `rollbackFailure` (crowdsec/backup.go ~L291-293): message `"... (rollback failed: %v; backup retained at %s)"` flows via `err.Error()` into the preset-apply and curated-apply JSON error responses. + 6. `extractTarGz` errors (hub_sync.go ~L947, ~L956 and the mkdir/`destPath` errors in the loop): reach the client through import's `extraction failed: %v` (crowdsec_handler.go ~L715). + 7. `ExportConfig` error path: `gin.H{"error": err.Error()}` (~L877) can carry filesystem paths from the walk. + 8. Frontend print sites in `frontend/src/pages/CrowdSecConfig.tsx`: ~L474 (`Backup stored at ...`), ~L493 (`Backup created at ...`), ~L1116 (`t('crowdsecConfig.presets.backup'): {applyInfo.backup}`). Existing vitest `frontend/src/pages/__tests__/CrowdSecConfig.curatedApply.test.tsx` (~L130 stubs `/tmp/backup.tar.gz`, ~L170 asserts the absolute path) must change. + 9. Playwright stub fixtures carrying absolute paths: `tests/security/crowdsec-hub-preset-apply.spec.ts` ~L110, `tests/security/crowdsec-file-editor.spec.ts` ~L89, and `tests/security/crowdsec-preset-apply.spec.ts` (`/app/data/backups/...`). + Backup directory kinds are `.backup.` (Snapshot) and `.filebackup.` (single-file edits); the acquisition backup is a plain file next to `acquis.yaml`. No client uses the value programmatically. +- Docs: `docs/api` has no mention of the `backup` field (checked); `ARCHITECTURE.md` mentions CrowdSec only as directory layout, so it needs no change (confirm no-op at implementation). Locale files carry `crowdsecConfig.presets.backup` ("Backup") in each language under `frontend/src/locales/*/translation.json`; any new UI string needs all locale files updated (en plus the existing languages). + +### 2.4 #1526 tolerant specs +Files under `tests/security/`: `crowdsec-dashboard.spec.ts`, `crowdsec-decisions.spec.ts`, `crowdsec-diagnostics.spec.ts`, `crowdsec-console-enrollment.spec.ts`, `crowdsec-import.spec.ts`, and the non-preset cases of `crowdsec-config.spec.ts` (back navigation, config file list/content, export, console enrollment, status indicators). +Counts depend on the pattern; for exactness use two named patterns on `origin/development`: (a) `catch(() => false)` only: dashboard 10, decisions 15, diagnostics 3, console-enrollment 9, import 0, config 11; (b) the broader "tolerance" pattern (adds "may not be", "not implemented", annotation pushes) gave higher numbers in my first pass and 13/15/18/17/1/12 in the supervisor's pass. Do not rely on either number; the gate in section 6 (Bz) defines the final, exact pattern set. +Two styles: +1. UI specs (`dashboard`, `decisions`, `config`, parts of `console-enrollment`) that `goto('/security/crowdsec')` against the real backend and guard on visibility. Convert to stubbed-API specs following `crowdsec-preset-apply.spec.ts` (`page.route` + `route.fulfill({ json })`, `loginUser`, `waitForLoadingComplete`). +2. API-contract specs (`diagnostics`, most of `import`, parts of `console-enrollment`) that call `request.get/post('/api/v1/admin/crowdsec/...')` against the live backend and accept multiple statuses because cscli/LAPI are absent in the E2E container (`crowdsec-import` hides this behind a multi-status `expect([...]).toContain(status)` assertion). Convert to (a) strict assertions on a status the E2E container deterministically produces, or (b) stubbed UI tests plus Go handler tests for the real status mapping. +Strict assertions may expose real bugs (see section 8). + +## 3. Decisions + +### 3.1 F5: recommendation (needs user confirmation) +**Hide `local_api_credentials.yaml` and `online_api_credentials.yaml` from editor list and read** by adding them to the `isReadable` deny rules next to `bouncer_key` (same lowercase comparison), then delete `protectedWriteNames` and its branch in `isWritable` (redundant: `isWritable` is a subset of `isReadable`). + +Behavior change to state in docs: previously readable but write-protected, now hidden entirely (list omits them; read returns `invalid path`; write returns `file cannot be edited`, because `resolveWritable` checks `isReadable` first; new tests must assert these two real messages, not one generic message). Mixed-case names are now hidden too. + +Rationale (reworded; this is information-level hardening, not a closed exposure): the editor is a browser surface where plaintext secrets leak through screen shares, screenshots/support captures and the client-side query cache; novices have no need to read or edit these generated files; advanced operators still have disk access. Scope limit, stated plainly: this protects the editor surface only. Admin config export still includes these files and `bouncer_key` (section 2.3), so F5 does not remove the secrets from anything an admin can download. See Open Question on export. + +Alternative rejected: list but mask values (extra UI/parser complexity and a new leak path if masking misses a field). + +### 3.2 F3: recommendation +Return a **backup name**, not a path: a single helper `backupID(path string) string` (in `backend/internal/crowdsec/backup.go`) returning `filepath.Base`, used at every site in 2.3 items 1-4. It is a name, not an opaque token: the three kinds are `*.backup.*` (directory), `*.filebackup.*` (directory) and `acquis.yaml.backup.*` (file). The absolute path stays in server logs only (already `SanitizeForLog`ged). +- Errors: `rollbackFailure` keeps the cause but replaces the path with the backup name plus the hint "see server logs for the backup location"; `extractTarGz`/import errors become fixed messages (for example `archive contains an unsafe path`, `archive extraction failed`) with the entry name/dest path logged sanitized; `ExportConfig` returns a fixed `failed to export configuration` and logs the error. +- UI keeps manual recovery possible: text such as "Backup created: (next to the CrowdSec data directory)" in the three print sites, via a new i18n key (all locales) rather than hard-coded English; docs pointer in `docs/troubleshooting/crowdsec.md` explains where backups live. +- Field name `backup` is kept. Acceptance criterion: **no absolute path appears in any CrowdSec API response, including error text** (asserted by handler tests with a regex over response bodies for a leading `/` path or the data-dir prefix). + +### 3.3 F1 +Reject an archive entry only if any path component equals `..` or the path is absolute; keep the `Rel`-based containment check as backstop. Tests: allowed `..hidden.yaml`, `a/..b/c.yaml`, `foo..bar`; rejected `../x`, `a/../../x`, `/etc/x`, NUL. No exploit detail in public text. + +### 3.4 #1479: remove vs enforce +Remove `WithAllowedDomains`, `ClientOptions.AllowedDomains` (and its default), the caller in `newHubHTTPClient`, and the three test references in 2.1. Rationale: the only caller has a better-fitting validator, redirects are disabled for that client, and a second list would drift. + +### 3.5 #1481: seam design and migration +- Production `validateHubURL`: https only, host in the three-host allowlist. +- Test seam: unexported `validateURL func(string) error` on `HubService` (nil means `validateHubURL`), both call sites (`fetchIndexHTTPFromURL` ~L453, `fetchWithLimitFromURL` ~L754) go through one `s.checkURL(target)`. +- Only the `hub_sync_test.go` ~L106 `httptest.NewServer` test needs the `validateURL` seam (permissive validator) plus a `network.WithAllowLocalhost()` `HTTPClient`. The ~L2259 server lives inside `TestNewHubHTTPClient_LoopbackSeam`, which is deleted together with the var. +- **`hubAllowLoopback` and `setHubAllowLoopbackForTest` are deleted. `TestNewHubHTTPClient_LoopbackSeam` is deleted** (it only tests the removed var); replace with a small test that `newHubHTTPClient` rejects loopback (the first half of the old test, `require.Error`). +- `validateHubURL` tests flip to assert rejection: `localhostURLs` loop (~L996-1010), the other table tests in the range ~L969-1044, and the table at ~L1645; add acceptance cases for the three https hosts and rejection for http on an allowlisted host. +- Stub-transport migration: the roughly 100-110 hits above (including `download_url`/`preview_url` inside index JSON fixtures in `crowdsec_pull_apply_integration_test.go`, ~L40-50, and `crowdsec_preset_handler_test.go`) become allowlisted https hosts. Use one distinct allowlisted host per role so `uniqueStrings` in `hubBaseCandidates()` does not collapse candidates: primary `https://hub-data.crowdsec.net` (default), mirror `https://raw.githubusercontent.com/crowdsecurity/hub/master` (default), and `https://hub.crowdsec.net` for tests that need an extra distinct base. Tests that previously set `HubBaseURL = "http://test.hub"` use `https://hub.crowdsec.net`; tests whose assertions depend on candidate ordering get an explicit check. +- Estimated effort: mechanical host mapping across roughly 100-110 hits (re-grep first) plus one seam-using test; a single commit. Re-grep all line numbers in this spec right before implementation starts. + +## 4. Priorities & Ordering + +Classification: all four are the maintainer's own findings (not user-submitted), none critical or high, no `feat` in the batch. Order: +1. **#1528**: decide F5 first (as the issue says), then F5, F3, F1 (user-visible and information-exposure items first). +2. **#1479**, then **#1481** (latent hardening of the hub URL policy; no active bypass). +3. **#1526** last: largest and open-ended; it should assert the final contracts from 1 and 2. + +Findings rules: a critical/high finding discovered in #1526 ships first as its own PR. **Medium/low findings from #1526 are filed as GitHub issues by default**; they are fixed in-PR only when needed to make a converted test pass or to unblock CI (CLAUDE.md DoD). Security-sensitive findings go to a private advisory per SECURITY.md. + +Advisory screening: no private advisory needed for any of the four. #1479/#1481 are public, low, latent, no demonstrated bypass; F3/F5 are admin-authenticated information-level hardening with no known exploit chain; the export limitation is recorded as a known limitation, not a vulnerability (admin already has full control). If any item proves reachable by a lower-privileged actor, stop and convert to a private advisory before further public wording. All public wording (issue comments, commit subjects, PR bodies, docs) stays vague. + +## 5. Branch / PR Slicing Decision + +**Default: one PR with ordered commits** (PR A commits first, then PR B commits). This respects "one feature = one PR" and the user's dislike of PR slicing. + +**Exception, only with the user's approval before work starts: two PRs.** PR A (`fix/crowdsec-followups-hardening`: #1528, #1479, #1481) and PR B (`test/crowdsec-deterministic-e2e`: #1526, branched from `development` after A merges, no stacking). Justification if approved: (1) B is about 2,000 lines of test rewrite with an open-ended findings tail and would hold the small hardening work hostage; (2) a secrets-visibility change cannot be meaningfully reviewed beside that churn; (3) B must assert A's final contracts. Cost of the default: larger review and A blocked on B. This is flagged as an open question; no slicing beyond commits happens unless approved. + +## 6. Commit Slicing Strategy + +Conventional commits; no session ID/link anywhere. All subagent dispatch prompts must state explicitly: run every test, build, E2E and lint command in the foreground and block until it finishes (no background/detached runs, per CLAUDE.md). Playwright security specs run only with `--project=security-tests` (single project, targeted files only, never the full suite). Backend lint must use the full config (`cd backend && golangci-lint run --config .golangci.yml ./...` or `make lint-backend`), not only the fast hook. GORM scan: N/A (no model/query changes; state this at the gate). + +### Part A (hardening) + +| # | Commit | Scope | Files | Depends on | Validation gate | +|---|---|---|---|---|---| +| A0 | `test(e2e): add fixme specs for editor secrets and response paths` | `test.fixme` specs for F5 (credentials not listed; read rejected) and F3 (`backup` and error bodies contain no absolute path); update fixture stubs to name-only values | `tests/security/crowdsec-file-editor.spec.ts` (~L89 stub + new cases), `tests/security/crowdsec-hub-preset-apply.spec.ts` (~L110), `tests/security/crowdsec-preset-apply.spec.ts` | none | `npx playwright test tests/security/crowdsec-file-editor.spec.ts tests/security/crowdsec-preset-apply.spec.ts tests/security/crowdsec-hub-preset-apply.spec.ts --project=security-tests` | +| A1 | `fix(security): harden CrowdSec editor access` | F5 per 3.1; tests assert real messages: list omits both names, read -> `invalid path`, write -> `file cannot be edited`, mixed-case and symlink-to-credentials cases (symlink caught by the resolved-path re-check ~L149) | `backend/internal/api/handlers/crowdsec_files.go`, `crowdsec_files_test.go` | A0 | `cd backend && go test ./internal/api/handlers/... ./internal/crowdsec/...` | +| A2 | `fix: stop returning server paths in CrowdSec responses` (see Q5 on whether this deserves `(security)`; if the user says yes use `fix(security): harden CrowdSec API responses`) | F3 per 3.2 across all nine items in 2.3 (`backupID` helper, `rollbackFailure`, extract/export/import error text, three UI print sites + i18n key in all locales, vitest and Playwright fixtures) | `backend/internal/crowdsec/backup.go` (+test), `hub_sync.go`, `crowdsec_files.go`, `crowdsec_handler.go`, `crowdsec_preset_handler.go`, `frontend/src/pages/CrowdSecConfig.tsx`, `frontend/src/locales/*/translation.json`, `frontend/src/pages/__tests__/CrowdSecConfig.curatedApply.test.tsx` and any other test asserting the old value, `docs/troubleshooting/crowdsec.md` | A1 | backend `go test` for touched packages; `cd frontend && npm run type-check && npx vitest run src/pages/__tests__/CrowdSecConfig*`; response-body regex test shows no absolute path | +| A3 | `fix: accept archive entries that merely start with two dots` | F1 | `hub_sync.go` (~L946), `hub_sync_test.go` | none | `go test ./internal/crowdsec/...` | +| A4 | `refactor: remove the unenforced allowed-domains client option` | #1479 incl. the three test references and option-parity table test | `backend/internal/network/safeclient.go`, `safeclient_test.go`, `hub_sync.go` | none | `go test ./internal/network/... ./internal/crowdsec/...` | +| A5 | `refactor: restrict the hub URL check to production hosts` | #1481 per 3.5 | `hub_sync.go`, `hub_sync_test.go`, `hub_pull_apply_test.go`, `hub_sync_raw_index_test.go`, `backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go`, `crowdsec_preset_handler_test.go` | A4 | `go test ./internal/crowdsec/... ./internal/api/handlers/...`; grep shows no `test.hub`/`example.com`/`hubAllowLoopback`/`AllowedDomains` outside intentional places | +| A6 | `docs/test: enable e2e specs and document the changes` | un-fixme A0; docs: credentials not shown in editor, backup names, one-line production note for #1481; confirm ARCHITECTURE.md and docs/api need no change | `tests/security/*.spec.ts`, `docs/features.md`, `docs/troubleshooting/crowdsec.md` (authored under `docs/`, never `docs-site/docs/`) | A1-A5 | targeted `npx playwright test tests/security/crowdsec-file-editor.spec.ts tests/security/crowdsec-preset-apply.spec.ts tests/security/crowdsec-hub-preset-apply.spec.ts --project=security-tests` | + +### Part B (#1526), after A + +| # | Commit | Scope | Files | Validation gate | +|---|---|---|---|---| +| B1 | `test(e2e): shared CrowdSec stub helpers` | extract repeated route stubs and fixtures (status, decisions, enrollment, file list/content, diagnostics) | new helper under `tests/utils/` or `tests/fixtures/` | one spec run | +| B2 | `test(e2e): deterministic crowdsec-config non-preset cases` | back nav, file list/content, export, enrollment, status indicators | `crowdsec-config.spec.ts` | `npx playwright test tests/security/crowdsec-config.spec.ts --project=security-tests` | +| B3 | `test(e2e): deterministic crowdsec dashboard and decisions` | | `crowdsec-dashboard.spec.ts`, `crowdsec-decisions.spec.ts` | same per file | +| B4 | `test(e2e): deterministic crowdsec console enrollment` | stubbed states: not enrolled / pending / enrolled / failed / heartbeat | `crowdsec-console-enrollment.spec.ts` | same | +| B5 | `test(e2e): deterministic crowdsec diagnostics and import` | strict deterministic statuses or stubbed UI + Go handler tests for status mapping | `crowdsec-diagnostics.spec.ts`, `crowdsec-import.spec.ts` (+ Go tests if gaps) | same | +| B6 | `fix: ...` only when required | in-PR fix only if needed to make a converted test pass or to unblock CI, with a regression test; every other medium/low finding is filed as an issue instead (section 4); high/critical goes to its own PR | as needed | the relevant spec + unit tests | +| Bz | `test(e2e): enforce no tolerant patterns in CrowdSec specs` | gate commit: no tolerance residue in the six files | none (verification only; optionally a lint script) | the command in the code block below (must return nothing, except reviewed legitimate `test.skip` hits); also review by hand for `console.log`-only branches and annotation pushes | + +Bz gate command (plain `|` alternation; ripgrep treats `\|` as a literal pipe). `test\.skip` hits need manual review because conditional skips can be legitimate: + +```bash +rg -n "catch\(\(\) => false\)|catch\(\(\) => \{\}\)|may not be|not implemented|test\.skip|expect\(\[[0-9, ]+\]\)\.toContain" \ + tests/security/crowdsec-config.spec.ts tests/security/crowdsec-dashboard.spec.ts \ + tests/security/crowdsec-decisions.spec.ts tests/security/crowdsec-diagnostics.spec.ts \ + tests/security/crowdsec-console-enrollment.spec.ts tests/security/crowdsec-import.spec.ts +``` -- `crowdsecDataDir = cfg.Security.CrowdSecConfigDir` (`backend/internal/api/routes/routes.go:1030`), set to `/app/data/crowdsec` by `Dockerfile:1142` (`CHARON_CROWDSEC_CONFIG_DIR`). The same directory is `CS_PERSIST_DIR` in `.docker/docker-entrypoint.sh`. It therefore contains the **whole persistent CrowdSec tree**: `config/` (the target of the runtime symlink `/etc/crowdsec -> /app/data/crowdsec/config`), `data/` (LAPI db), `hub_cache/`, `bouncer_key`. -- That same path is `HubService.DataDir` and `CrowdsecHandler.DataDir` (`crowdsec_handler.go:374-397`). Every "backup the DataDir" in the code is a backup directory next to it, `/app/data/crowdsec.backup.`. +Full Definition of Done for the PR(s): targeted Playwright as above; full-config backend lint; `scripts/go-test-coverage.sh` >= 85%; `scripts/frontend-test-coverage.sh`; `bash scripts/local-patch-report.sh`; `npm run type-check` and `npm run build` in `frontend/`; `go build ./...`; `lefthook run pre-commit`. CodeQL/Trivy are CI-deferred (fix/refactor, no new surface); a local CodeQL Go run is optional for A1/A2. -### 2.1 #1514 applyPresetLocally writes unvalidated client content (root cause is wider than the issue says) +## 7. Test Plan -- Entry: `frontend/src/pages/CrowdSecConfig.tsx:429` `applyPresetLocally`; reached only when apply returns HTTP 501 for a non-curated (hub) preset (line ~508). It writes `presetPreview` (text from the pull preview, held in client state) to `selectedPath ?? files[0]` through `writeCrowdsecFile` (`frontend/src/api/crowdsec.ts`). -- Confirmed: the backend **never returns 501** for apply any more (grep of `crowdsec_handler.go` shows no `StatusNotImplemented`). The fallback is dead code in practice and can only be triggered by a proxy/misconfig, which makes removing it risk-free. -- Transformation/persistence: `POST /admin/crowdsec/file` -> `CrowdsecHandler.WriteFile` (`crowdsec_handler.go:950`, registered under `managementAdmin`, so admin-only). Verified defects in the handler, independent of the frontend: - 1. **Destructive backup.** It `os.Rename(h.DataDir, backupDir)` (moves the *entire* persistent CrowdSec tree away, config symlink target included), then `MkdirAll(filepath.Dir(p))` and writes just the one file. Everything else (config.yaml, hub, bouncer key, data) is left only in the backup dir. The live tree becomes a single file. There is no restore. This is the same defect class as #1515 and is a medium-severity data-loss bug, not just "unvalidated content". - 2. **The file endpoints get a validation pass.** The fix is described at policy level in 3.2. -- Exit: `{status:"written", backup}`; the UI toasts "File saved" / "Preset applied locally". -- Severity/impact: low-medium, authenticated admin only (not an unauthenticated vulnerability). See section 4. +- Backend: `isReadable`/`isWritable`/`resolveReadable`/`resolveWritable` table tests asserting real messages (`invalid path` for reads, `file cannot be edited` for writes) for both credentials names, mixed case, nested, symlink target; `backupID` table; handler tests with a response-body regex for absolute paths on success and error paths (file write, import including extraction failure, preset apply, curated apply with a forced rollback failure, acquisition update, export error); `validateHubURL` rejection/acceptance tables; safeclient dialer-vs-redirect parity table; tar entry-name table. +- Frontend: update `CrowdSecConfig.curatedApply.test.tsx` and add tests for the three print sites and the new i18n key. +- E2E: per commit tables. +- Grep gates: no `WithAllowedDomains`/`AllowedDomains`/`hubAllowLoopback` remnants; no test hosts in non-test code. -### 2.2 #1515 HubService.Apply renames the whole data dir (CONFIRMED in code; end-to-end not executed) +## 8. Risks and Mitigations -- Entry: `ApplyPreset` -> `Hub.Apply` for non-curated presets (`hub_sync.go:602`). -- Transformation: `backupExisting` first tries `os.Rename(DataDir, .backup.)`; only if rename fails (EBUSY on a mount point, cross-device) does it fall back to `copyDir`. Then `runCSCLI` (`cscli hub update` + `cscli hub install `). -- Why it is wrong, confirmed by reading the code and by the behavior seen in a throwaway container (below): after the rename `/etc/crowdsec -> /app/data/crowdsec/config` dangles, so `cscli` cannot read `config.yaml` (observed error: `open /etc/crowdsec/config.yaml: no such file or directory`). The cscli path therefore fails, the code falls to `extractTarGz` into a freshly created `DataDir`, and on success the live dir holds only the extracted archive while config, hub, `bouncer_key`, LAPI db and `hub_cache` sit in the backup. CrowdSec is running while its db directory is moved. Rollback (`rollback`) does `RemoveAll(DataDir)` then renames the backup back, so it is also only valid for that directory and cannot undo anything `cscli` wrote outside it. -- Not confirmed: that a *real* hub-preset apply on a running container hits exactly this sequence (needs a hub preset pull + a running agent). The verification step is part of the gate in Commit 3. Whether the extract fallback ever "succeeds" with a usable tree is irrelevant; the rename itself is the defect. -- Fix direction (matches the issue): reuse the copy-based, symlink-preserving, db-excluding `backupCopy`/`restoreCopy` added for `ApplyCurated`. +| Risk | Mitigation | +|---|---| +| ~100-110-hit test migration (A5) breaks many tests | Single commit, package-by-package runs, mechanical mapping with distinct hosts per role (3.5) | +| Migration weakens tests via `uniqueStrings` collapse | Distinct hosts per role; ordering assertions where relevant | +| F5 hides a file someone relies on viewing | Documented as a behavior change; disk access unchanged; masked read-only view is the fallback | +| F5 gives false assurance because export still includes secrets | Stated as known limitation; export question below | +| F3 breaks manual-recovery UX | Name plus location hint in UI, docs pointer, server log keeps full path | +| #1526 exposes real bugs | Stop rule: if more than N=6 real bugs surface, stop and re-plan with the user; medium/low are filed as issues by default (section 4) | +| Live-backend E2E flake | Prefer stubs; where live, assert only deterministic states | +| Specs silently not run without `--project=security-tests` | Every gate names the project; confirm test count in output | -### 2.3 #1516 backup directories accumulate (CONFIRMED) - -Creators of `.backup.*`: `HubService.Apply` (rename, second-resolution timestamp), `ApplyCurated` (copy, microsecond timestamp), `CrowdsecHandler.ImportConfig` (`crowdsec_handler.go:696`, rename), `WriteFile` (rename). Nothing ever deletes any of them. grep for prune/retention in `backend/internal/crowdsec` returns nothing. Because the curated backup is a full copy of the persistent tree (minus the db) and the legacy ones are the whole tree, growth is unbounded on a volume the user sizes for a home server. - -### 2.4 #1517 handler file size (CONFIRMED) - -`crowdsec_handler.go` is 2,906 lines. Preset handlers live at lines ~990-1325 (`ListPresets`, `PullPreset`, `ApplyPreset`, `applyCuratedPreset`, `curatedPresetPreview`, `recordPresetEvent`, `applyFailureBody`, `respondApplySuccess`, `GetCachedPreset` at ~1426). Pure move, same package. - -### 2.5 #1518 tolerant Playwright tests (CONFIRMED) - -`tests/security/crowdsec-config.spec.ts` "Preset Management" block (lines ~73-145) and "Configuration Files" block are `isVisible().catch(() => false)` guarded and annotate "feature may not be implemented"; they cannot fail on a regression. `tests/security/crowdsec-preset-apply.spec.ts` (from #1513) is the model: route-stubs `**/api/v1/admin/crowdsec/presets/apply` and the file-write route and asserts toasts and write counts. Missing deterministic coverage: hub (non-curated) preset apply success/failure surfaced, no client-side write on any apply failure, file editor save/failure, file-list error cases. - -### 2.6 #1523 hub data files missing after container recreation (ROOT CAUSE CONFIRMED by experiment) - -- Config: CrowdSec's default `config_paths.data_dir` is `/var/lib/crowdsec/data/` and `hub_dir` is `/etc/crowdsec/hub/` (upstream `config/config.yaml`). The entrypoint (`.docker/docker-entrypoint.sh:310-319`) redirects **only** `db_path` to the volume. `Dockerfile:1089` creates `/var/lib/crowdsec/data` inside the image, so on a recreated container it is an empty directory (confirmed: the issue shows only `trace/`). `/etc/crowdsec` is a symlink into the volume so the **hub items persist but their data files do not**. -- `configs/crowdsec/install_hub_items.sh` skips items for which `cscli inspect ` exits 0 ("already installed, skipping"). -- Experiment (throwaway `charon:local` container run with `--entrypoint sh`, no volumes, no live host touched): - - Install `crowdsecurity/http-backdoors-attempts` -> `backdoors.txt` appears in `data_dir`. - - Wipe `data_dir` (simulating recreate), keep the hub: `cscli scenarios install ` -> "Nothing to install or remove", data dir stays empty. `cscli scenarios install --force ` -> same, still empty. **`cscli hub upgrade` repopulates `backdoors.txt`, and so does `cscli hub upgrade --force`.** The issue's suggestion "`install --force` or `hub upgrade`" is therefore half right: only `hub upgrade` works. -- **Additional defect found (CONFIRMED, not in the issue):** on a hub with an index but nothing installed, `cscli scenarios inspect crowdsecurity/http-backdoors-attempts` exits **0** and prints `installed:false` metadata. `install_if_missing` uses the exit code, so on a fresh volume it may print "already installed, skipping" and install nothing. (In the experiment inspect exited 1 when the item was installed but its data file was missing, so the exit code is not a reliable installed/absent signal in either direction.) #1513's `ApplyCurated` already uses the correct signal: `inspect -o json` -> top-level `installed`. -- Not confirmed: whether the missing-install path is masked in real startups by something else (e.g. the collections installing their children). Needs the recreate test in Commit 6 on a clean volume. - -### 2.7 #1524 empty version (ROOT CAUSE CONFIRMED; the fix has a gotcha) - -- Confirmed: the injected symbol is wrong. In CrowdSec v1.8.1, `pkg/cwversion/version.go` declares only `Codename` and `Libre2` (and no `Version`); the version comes from `github.com/crowdsecurity/go-cs-lib/version` (`Version`, `BuildDate`, `Tag`, `System`), pinned at `go-cs-lib v0.0.25` by CrowdSec v1.8.1's `go.mod`. Upstream's own Makefile uses `-X 'github.com/crowdsecurity/go-cs-lib/version.Version=$(BUILD_VERSION)'` (plus `BuildDate`, `Tag`, and `cwversion.Codename`). `-X` on a nonexistent symbol is silently ignored, so both `Dockerfile:915` and `:923` are no-ops. Observed in the built image: `cscli version` prints empty `version:`, `Codename:`, `BuildDate:`, and `cscli` logs `Crowdsec version is not set, using hub branch 'master'`. -- The existing app-stage sanity check (`Dockerfile` ~L1054) says "CrowdSec 1.8.x prints an empty `version:` field here regardless of the -X ldflag". That comment is wrong and hides the bug; it must be corrected. -- **Gotcha, verified in `cmd/crowdsec-cli/core/require/branch.go` (v1.8.1):** with a real version cscli calls `https://version.crowdsec.net/latest` on every hub command and returns a hard error if it cannot reach it; if the running version equals latest it uses `master` (identical to today), if it is older than latest it uses the branch named after the version. `https://hub-cdn.crowdsec.net/v1.8.1/.index.json` returns **404** today (v1.8.0, v1.7.8 and master return 200). So a correct version could (a) make `cscli hub *` fail offline/air-gapped and (b) break hub updates once 1.8.1 is no longer the latest and the CDN has no `v1.8.1` branch. The issue's "hub items may be newer than the engine supports" claim is also overstated: while 1.8.1 is the latest release the behavior is already `master`. The fix therefore needs an explicit hub-branch decision (see D1) rather than a bare `-X` correction. -- The toolchain delivery path: the default Dockerfile build consumes the prebuilt `ghcr.io/wikid82/charon-toolchain` image (content-hash tag `CHARON_TOOLCHAIN_TAG`, `Dockerfile:22`). Editing the `crowdsec-inline` stage changes the key, `.github/workflows/toolchain-image.yml` builds the new image and its bot commits the new TAG/DIGEST onto the PR head branch (workflow lines ~365-420) so `verify-toolchain-pin` goes green. Until that happens the app image still contains the old binary, so the app-side assertion cannot be added in the same commit as the ldflags change. - -### 2.8 Reproduction/evidence commands run for this plan (all foreground, repo-local or throwaway container) - -`gh issue view` for each issue, `gh pr view 1513`; code reads listed above; `curl` of upstream v1.8.1 source files and `hub-cdn.crowdsec.net//.index.json`; shallow clone of upstream v1.8.1 into the session scratchpad; throwaway `docker run --rm --entrypoint sh charon:local` experiments (data-dir wipe, inspect exit codes, `hub upgrade`). Nothing on live hosts or volumes was touched. - -## 3. Technical Specifications - -### 3.1 Shared backup helper, Apply flow, ImportConfig (#1515, #1516) - -New file `backend/internal/crowdsec/backup.go` (move `backupCopy`, `restoreCopy`, `isLiveDBFile`, `emptyDirExcept` out of `curated_apply.go`; the move itself changes no behavior): - -- **Two namespaces, two caps, so they cannot evict each other (blocking item 3).** - - Full-tree snapshots: backup dirs next to `DataDir`: `.backup.`, `const DefaultBackupRetention = 5`. - - Single-file edit backups (from `WriteFile`, section 3.2): backup dirs next to `DataDir`: `.filebackup./`, `const DefaultFileBackupRetention = 10`. The `.backup.*` glob never matches `.filebackup.*`. -- **Snapshot exclusions (blocking item 4).** `snapshot` copies `DataDir` (symlinks preserved) but **excludes**: the live `crowdsec.db`, `-wal`, `-shm` (anywhere), and the top-level `data/` directory (LAPI db plus hub data files and the GeoLite2 mmdb, which after #1523 live there; hub data files are recoverable with the gated `cscli hub upgrade`, the db is owned by the running engine) and the top-level `hub_cache/` (regenerable download cache). `restore` empties `DataDir` except those same excluded entries and copies the snapshot back, so it never touches `data/` or `hub_cache/`. Only the *top-level* `data/` and `hub_cache/` are excluded; a nested directory with the same name inside `config/` is still backed up. Result: a snapshot is essentially `config/` plus `bouncer_key` (small), x5 retained. -- `func (s *HubService) snapshot() (path string, err error)`: microsecond timestamp name, created `0o700` with exclusive `os.Mkdir`. -- `func (s *HubService) restore(path string) error`. -- `func PruneBackups(dataDir, kind string, keep int) (removed []string, err error)` where `kind` is `backup` or `filebackup`. Lists siblings matching `..*`, `Lstat`-checks each is a real directory (never follows or deletes symlinks or plain files), **parses the timestamp from the directory name** (both the legacy `20060102-150405` and the microsecond `20060102-150405.000000` layouts; names that do not parse are ignored, never deleted), sorts newest first, deletes everything past `keep`. `keep < 1` is clamped to 1 so the newest is always kept. Never touches `dataDir`. Single-removal errors are logged and aggregated, never failing the caller. -- **Prune runs after every attempt, successful or failed**, always preserving the newest `keep` snapshots. A failed apply's own snapshot is the newest, so it is always retained for manual recovery. - -**`HubService.Apply` new control flow (blocking item 2).** Hold `HubService.mu` and the handler-level lock (section 6). -1. Validate slug; load cache meta (non-fatal). -2. `snapshot()`; any error -> return failure, nothing changed. `BackupPath` is set only after success. The old "read the archive into memory before the backup because the cache lives in `DataDir`" workaround is **removed**: nothing is moved any more, and `hub_cache/` is excluded from snapshots and from restore, so the archive stays valid for the whole call. -3. If `cscli` is available: `cscli hub update` (non-fatal), `cscli hub install `. Success -> status `applied`, `UsedCSCLI=true`, reload, prune, return. -4. **A cscli failure still falls back to the cached-archive extract** (this keeps the offline/hub-unreachable behavior that exists today). Before extracting, `restore(snapshot)` so the extract starts from a clean pre-apply tree and any partial files `cscli` wrote are removed (this closes the partial-overlay risk of overlaying an archive on a half-installed state). -5. Extract `extractTarGz` onto `DataDir` (cache refresh from the hub if the cache is missing, as today). **Residual risk, documented:** extraction overlays archive files on top of the live tree rather than replacing it, so files in the archive replace same-named live files; mitigation is the snapshot. Any extract/refresh failure -> `restore(snapshot)` and return the error with `BackupPath`. -6. Success -> status `applied`, `UsedCSCLI=false`, reload, prune. -`backupExisting`, `rollback`, `emptyDir`, and `copyDir` (if now unused) are deleted. - -**`CrowdsecHandler.ImportConfig` (blocking item 6): converted to snapshot/restore semantics, never renaming `DataDir`.** Flow: handler lock -> `snapshot()` -> empty `DataDir` except the snapshot-excluded entries (so the live db, `data/`, `hub_cache/` survive the import) -> extract the uploaded archive, **skipping any archive entry that is a live db file (`isLiveDBFile`) or lies under the top-level `data/` or `hub_cache/`** so an upload can never overwrite engine-owned state -> on any failure `restore(snapshot)` and return the error -> prune. Net behavior difference: the live db and `data/` are no longer moved or lost by an import; everything else is replaced exactly as before. Existing `ImportConfig` tests are updated, plus the new tests listed in Commit 3. - -### 3.2 File endpoints hardening (#1514) - -Details of the validation defects and their fixes are intentionally not written here (see section 4). Public-safe design: - -- New file `backend/internal/api/handlers/crowdsec_files.go` with a single shared path-resolution helper used by `ReadFile`, `WriteFile` and `ListFiles`, so all three apply one policy ("harden file-endpoint path validation"). -- Write policy: bounded body size (1 MiB); allowed file types (Q3); YAML files must parse; protected files and trees (secrets, databases, caches, engine data) are never writable; `.yaml/.yml` parsing uses `gopkg.in/yaml.v3` if already a dependency (verify). -- **Symlink policy (blocking item 7):** the editor never writes through or replaces a symlink (HTTP 400), so symlinked hub items are read-only from the editor. Reads stay confined to `DataDir`, and symlinked hub items that resolve inside it stay readable. Test in Commit 4. -- **Two predicates, not one.** The viewer today lists and reads any file under `DataDir` (hub items, `config.yaml`, etc.), and filtering the list by the write predicate would hide files users can currently read. So: `isReadable(rel)` governs `ListFiles` and `ReadFile` (secrets and databases such as `bouncer_key`, `*.db*`, and the `hub_cache/` and `data/` trees are neither listed nor readable; everything else stays visible, including symlinked hub items), and the stricter `isWritable(rel)` governs `WriteFile`. A file that is readable but not writable (symlinked hub items, non-allowlisted types) stays viewable and its save returns the server's 400 message, which the existing `writeMutation` error path already shows; no frontend contract change. `ExportConfig` is unaffected. Commit 4 tests: `ReadFile` refuses `bouncer_key` and db files, reads a symlinked hub item that resolves inside `DataDir`, and refuses a symlink resolving outside it; `ListFiles` omits them but still lists a symlinked hub item and `config.yaml`; every file `WriteFile` accepts is also listed. -- `WriteFile` algorithm: lock -> validate -> if the target exists, copy just that file to `.filebackup./` -> atomic write (temp file in the same directory, `fsync`, `rename`, preserve mode, default `0o600`) -> `PruneBackups(kind=filebackup, 10)` -> 200 `{status:"written", backup}`. `DataDir` is never renamed. -- Error contract (JSON `{"error": ...}`): 400 invalid request / path / type / YAML / symlink, 413 too large, 500 write failure. -- No new endpoints, no schema changes, no frontend contract changes. - -### 3.3 Frontend (#1514) - -`frontend/src/pages/CrowdSecConfig.tsx`: delete `applyPresetLocally`, the `501` branch in `handleApplyPreset`, the `applied-locally` apply-info status, and anything left unused (`backupMutation` if only used there, imports, `presetPreview` write path). Non-2xx apply for hub presets is surfaced like curated: server error text, backup path if present, no write. The generic file editor (`writeMutation`) is unchanged but now shows server 400/413 messages. Update Vitest files: `CrowdSecConfig.test.tsx`, `.coverage.test.tsx`, `.spec.tsx`, `.curatedApply.test.tsx` (remove tests that assert the local fallback; add "501 for a hub preset shows an error and performs no file write"). - -### 3.4 Persistent CrowdSec data dir and hub data (#1523) - -`.docker/docker-entrypoint.sh` (next to the existing `db_path` redirect, ~L310-320): - -- `sed` `data_dir: /var/lib/crowdsec/data/` -> `data_dir: ${CS_DATA_DIR}/` in `$CS_CONFIG_DIR/config.yaml` (idempotent; works for both fresh and already-initialized volumes), with the same verify/echo pattern as the db redirect. `CS_DATA_DIR` is `/app/data/crowdsec/data` and is already `mkdir`ed and chowned. -- **Gated, never unconditional (blocking item 5).** After `install_hub_items.sh`, the entrypoint runs `cscli hub upgrade` **only if** at least one installed item's data files are missing. Detection: for each installed item (`cscli list -o json`), check that every file the item declares is present under `data_dir`; the exact JSON field for declared data files is confirmed by the implementer against `cscli inspect -o json` before coding (open implementation detail, verified in the Commit 6 gate). Fallback if the field is not exposed: treat an empty `data_dir` (ignoring `trace/` and the db) with at least one installed item as "missing". The call is bounded (`timeout 120s`), failure is a logged warning and never fatal, and it never uses `--force`. A normal restart with intact data files performs no upgrade, so items are never silently upgraded on a healthy start. Note `hub upgrade` upgrades every installed item; this is acceptable only in the recovery case it is gated on, and the log line must say so. -- It is skipped in the same condition as the rest of hub init (`CHARON_SECURITY_TESTS_ENABLED=false`). - -`configs/crowdsec/install_hub_items.sh`: change `install_if_missing` to decide on `cscli inspect -o json` reporting `"installed": true`, matching `ApplyCurated.verifyItem`. **Verified:** `jq` is not in the `charon:local` image, so use `grep -q '"installed": *true'` (not jq). **Verified:** `inspect -o json` still prints full JSON and exits 0 when an item is installed but its data file is missing, so the JSON signal is reliable in exactly the recreate scenario (the plain-text `inspect` exit code was not). Keep the "failed to install" warnings non-fatal. - -`Dockerfile`: keep creating `/var/lib/crowdsec/data` (a stale `data_dir` on an un-migrated config must still exist); no other change. -`.docker/compose/docker-compose.playwright-ci.yml:137` currently mounts a volume at `/var/lib/crowdsec/data`; repoint or remove it so E2E exercises the same persistent path as production. `docs/security.md` hardened-deployment text that tmpfs-mounts `/var/lib/crowdsec` stays valid (the db and data now live under `/app/data/crowdsec`) but its description must be updated. -`scripts/diagnose-crowdsec.sh` default `DATA_DIR` updated to the new location. - -Optional passive health signal (Q4 resolved: log line only, no UI): log a startup summary line listing installed-but-missing data files. No banner or badge (Passive UI, no nudges). - -### 3.5 Version injection (#1524) - -`Dockerfile` `crowdsec-inline` stage (both `xx-go build` commands, ~L915 and ~L923): replace the ldflags with `-s -w -X github.com/crowdsecurity/go-cs-lib/version.Version=v${CROWDSEC_VERSION} -X github.com/crowdsecurity/go-cs-lib/version.BuildDate=`. Do not set `Tag` (empty `Tag` leaves `version.String()` equal to `Version`). Do not add `BuildDate` from the wall clock: that would break the content-addressed, reproducible toolchain key; prefer the release date derived from the pinned tag or omit it. - -App-side assertion (second commit, after the toolchain bot has pushed the new TAG/DIGEST to the PR branch): in the existing N5 `RUN` block (~L1054) replace the wrong comment and add `grep -q "^version: v${CROWDSEC_VERSION}\$"` against `cscli version`. The final stage must redeclare `ARG CROWDSEC_VERSION` (currently declared only globally at L41 and in the caddy/crowdsec-inline stages; a global ARG is not visible inside a later stage without redeclaring). The build fails if the symbol ever silently no-ops again. - -Hub branch (decision D1, recommended): set `cscli.hub_branch` explicitly in the entrypoint-managed `config.yaml` so behavior is deterministic and does not depend on `version.crowdsec.net` reachability or on `hub-cdn` having a `v1.8.1` branch. Verified in upstream v1.8.1 `chooseBranch`: a non-empty `cscli.hub_branch` is returned immediately, before the `version.crowdsec.net/latest` lookup, so setting it removes both the network dependency and the CDN-404 risk. The online/offline test result is recorded in the PR description (Commit 8 gate). - -### 3.6 Handler split (#1517) - -Move to `backend/internal/api/handlers/crowdsec_preset_handler.go` (same package, no behavior change, no export changes): `ListPresets`, `PullPreset`, `ApplyPreset`, `applyCuratedPreset`, `curatedPresetPreview`, `recordPresetEvent`, `applyFailureBody`, `respondApplySuccess`, `GetCachedPreset`, and preset-only helpers. Move the matching unit tests into `crowdsec_preset_handler_test.go` files where they are clearly preset-only (leave shared fixtures where they are). `RegisterRoutes` stays in `crowdsec_handler.go`. The new file must be < 700 lines. - -## 4. Priorities & Ordering (CLAUDE.md "Findings Triage") - -None of the seven issues is user-submitted (all filed by the maintainer from PR #1513). Rules applied: bug fixes before new implementation (there is no `feat` here); critical/high ships as its own PR first; medium/low is scheduled after. - -Severity after root-causing (some differ from the issue labels): - -| Rank | Issue | Severity | Why | -|------|-------|----------|-----| -| 1 | #1523 data files missing after recreate | **medium** (issue says medium) | Silent degraded detection and no GeoIP on every recreated container; install guard defect makes fresh volumes suspect. Highest user impact. | -| 2 | #1514 + `WriteFile` destructive rename | **medium** (raised from low-medium) | Admin action can wipe the live CrowdSec tree; weak input validation. Security-sensitive (below). | -| 3 | #1515 `HubService.Apply` rename | **low-medium, confirmed** | Same defect class and same helper as #1514/#1516; fixed together. | -| 4 | #1524 empty version / hub branch | **low-medium** | Fix is easy but changes runtime hub behavior (offline failure, CDN 404 risk); needs the D1 decision and the toolchain bot. Scheduled after the fixes it must not destabilize. | -| 5 | #1516 backup accumulation | low | Piggybacks on the shared helper. | -| 6 | #1518 tolerant E2E | low | Test hardening; first in commit order (as `fixme` specs) and enabled last. | -| 7 | #1517 handler split | low | `refactor`, no behavior change; sequenced first in the *commit* order (mechanical, makes later diffs reviewable) but lowest in *priority*. | - -No finding is critical or high, so none must ship as its own PR ahead of this work. Priority above governs attention and what to cut first under contingency; commit order in section 5 is dependency-driven. - -**Security handling (#1514).** It is admin-only and already public as an issue. Assessment: low severity (authenticated admin only; the endpoint grants nothing beyond what an admin already has). **Q2 resolved by the user: #1514 is already public, so no private advisory is opened unless root-causing shows real impact beyond what an admin already has; it does not, so the default is NO advisory.** Handling: -- The existing public issue #1514 stays as is. Keep PR text, commit bodies and code comments free of exploit specifics anyway (no value in handing detail to un-upgraded installs). -- Exactly one commit in the PR carries the `(security)` scope, and its subject must stay in the vague category-only form: `fix(security): harden CrowdSec configuration file handling`. Keep the subject category-only, and Commit 4's body and the PR description stay equally vague (for example "tighten validation in the CrowdSec configuration file endpoints"). Every commit subject in a feature PR merge-commit appears verbatim in the public changelog, so the other commits use plain scopes (`fix:`, `refactor:`, `test:`, `docs:`, `chore:`) and describe user-visible behavior only. The destructive-rename data-loss fix is a plain `fix:` (it is a reliability bug, not an exploit). - -**Out-of-scope findings discovered while planning:** none left open. Items found beyond the issue text are folded in (install guard, wrong N5 comment, `ImportConfig` conversion, playwright-ci compose mount). If implementation finds anything else, file it per the triage rule at that time. - -## 5. Branch/PR Decision and Commit Slicing Strategy - -### 5.1 Branch/PR decision: ONE branch, ONE PR - -Single branch `fix/crowdsec-preset-apply-hardening` -> PR into `development`, titled e.g. `fix: harden CrowdSec preset apply and keep hub data across container recreation`. - -Why one PR (cohesion): all seven issues are one defect family, "the CrowdSec persistent tree is mutated unsafely and its hub/data state is not durable", and they share code: #1514, #1515 and #1516 all consume the same new `backup.go` helper (a single fix for rename-based backups, the file endpoint, and pruning); #1517 is the file those changes land in; #1518 is the test net for #1514/#1515; #1523 and #1524 are the same persistent-dir/hub-state story at the container layer and are both verified by the same rebuilt-container gate. Splitting would leave the helper without its callers, the E2E net without the behavior, or a half-fixed persistence story. - -Considered splits and why rejected: -- Docker/entrypoint (#1523, #1524) vs Go/frontend: they can build independently, but they do not conflict, share a single verification environment (rebuilt `charon:local`), and splitting would double the merge-soak the maintainer applies before `main`. The #1524 toolchain bot is explicitly designed to push the pin bump onto the PR head branch, so it works inside this PR. -- `(security)` fix alone: not required; severity is low (authenticated admin), and the rule "own PR" applies to critical/high only. - -**Contingency without splitting:** if Commits 7-8 (#1524) cannot be validated (hub-branch decision unresolved, toolchain build blocked), drop those commits from the PR and leave #1524 open rather than opening a second PR for it; the remaining six issues still form a complete, coherent PR. Rollback of the PR as a whole is a revert of the merge commit; a revert leaves a valid `data_dir` in `config.yaml` (the old image still creates the default directory), so nothing breaks. - -### 5.2 Ordered commits (all on the one branch) - -Every command run by implementers is foreground/blocking per CLAUDE.md. Each commit must build and pass its gate; the PR as a whole passes the full Definition of Done (section 8). - -**Commit 1 - `test: add deterministic E2E specs for CrowdSec preset and file flows (fixme)`** (#1518, step 1 of the suggested sequence) -- Files: new `tests/security/crowdsec-hub-preset-apply.spec.ts`, new `tests/security/crowdsec-file-editor.spec.ts`. -- Content, all stubbed with `page.route` like `crowdsec-preset-apply.spec.ts`, marked `test.fixme` until their feature commits: hub preset apply success shows success only when body `status==='applied'`; hub preset apply 500/503/504 shows an error and **zero** `POST /admin/crowdsec/file` calls; hub preset apply 501 shows an error and zero writes; file editor save success posts exactly one write for exactly the selected path; file editor shows the server message for 400 (disallowed type / invalid YAML) and 413. -- Depends on: none. Gate: `npx playwright test tests/security/crowdsec-hub-preset-apply.spec.ts tests/security/crowdsec-file-editor.spec.ts --project=security-tests` runs and reports the tests as fixme/skipped with no failures; lint/type-check clean. - -**Commit 2 - `refactor: move CrowdSec preset handlers into their own file`** (#1517) -- Files: new `backend/internal/api/handlers/crowdsec_preset_handler.go`; edit `crowdsec_handler.go`; move/split preset-only tests into `crowdsec_preset_handler_test.go`. -- Pure move: no signature, route, or behavior changes. Depends on: none. -- Gate: `cd backend && go build ./... && go test ./internal/api/handlers/... -race` all green with the same test count; `git diff --stat -M` shows moves, not rewrites; `make lint-fast`. - -**Commit 3 - `fix: keep CrowdSec backups bounded and copy-based for preset apply`** (#1515, #1516) -- Files: new `backend/internal/crowdsec/backup.go` + `backup_test.go`; edit `hub_sync.go` (`Apply` per the flow in 3.1; remove `backupExisting`/`rollback`/`emptyDir`/dead `copyDir`), `curated_apply.go` (move helpers, add prune call), `crowdsec_handler.go` `ImportConfig` (snapshot/restore conversion, handler lock), `hub_pull_apply_test.go`, `device_busy_test.go` (the rename/EBUSY tests are obsolete and are replaced by copy-path tests). -- Depends on: Commit 2 (preset code already in its own file; `ImportConfig` stays in the main handler). -- Unit tests (new, all with `t.TempDir()`): snapshot excludes live db files, top-level `data/` and `hub_cache/`, preserves symlinks, and still copies a nested directory named `data` under `config/`; **size test:** a large file under `data/` and under `hub_cache/` is not copied while a small `config/` file is (assert snapshot size); restore leaves live db, `data/` and `hub_cache/` untouched; `PruneBackups` orders by the timestamp parsed from the name (not mtime; touching an old dir does not save it), handles both name layouts, ignores unparsable names, non-matching names, symlinks and plain files, never deletes `dataDir`, clamps `keep<1` to 1, always keeps the newest, handles a missing parent, and `backup` and `filebackup` namespaces never evict each other; `HubService.Apply` flows: (a) cscli success leaves `DataDir` in place (inode unchanged) and prunes; (b) **cscli failure -> snapshot restored -> cached-archive extract succeeds -> `applied`, `UsedCSCLI=false`, no cscli leftovers**; (c) cscli failure and extract failure -> snapshot restored, error returned with `BackupPath`; (d) cscli failure with cache miss and hub refresh failure -> restored; (e) prune also runs after a failed apply and keeps the failed apply's snapshot; `ImportConfig`: success replaces config but preserves live db/`data/`/`hub_cache/` and never renames `DataDir`; extract failure restores the previous config; prune runs; concurrent `ImportConfig` + apply serialize on the handler lock; an uploaded archive containing `crowdsec.db`, `data/x` and `hub_cache/y` entries extracts none of them. -- Gate: `go test ./internal/crowdsec/... ./internal/api/handlers/... -race`; package coverage not below the current level; **rebuilt-container verification of #1515**: pull + apply one hub preset against a running agent, confirm `/app/data/crowdsec` is never renamed (inode unchanged), the apply result is correct, and at most 5 full-snapshot dirs remain after 7 applies (this also settles the "not confirmed" item in 2.2). - -**Commit 4 - `fix(security): harden CrowdSec configuration file handling`** (#1514 backend; subject intentionally vague, see section 4) -- Files: new `backend/internal/api/handlers/crowdsec_files.go` + `crowdsec_files_test.go`; edit `crowdsec_handler.go` (`ListFiles`, `ReadFile`, `WriteFile` delegate to the shared helper); update existing `TestCrowdsec_WriteFile_*` tests in `crowdsec_handler_coverage_test.go`. -- Depends on: Commit 3 (per-file backups use the `filebackup` namespace and handler lock). Commit body and PR text stay vague (section 4). -- Unit tests (categories only here): path validation rejection cases, symlink policy (write through or replacement of a symlink is refused, a regular file under `DataDir` is written), type/content/size validation (disallowed type, invalid YAML, oversize -> 413, protected files), atomic write leaves no temp file on failure, `DataDir` and unrelated files untouched after a write (regression test for the destructive rename), `filebackup` contains only the previous version of the target and is capped independently of full snapshots, the read/list predicate and the stricter write predicate behave as in 3.2, `ReadFile` shares the validation. -- Gate: `go test ./internal/api/handlers/... -race`, `make lint-fast`; GORM scan not required (no models); the Playwright file-editor spec enabled in Commit 9 depends on this behavior. - -**Commit 5 - `fix: remove the dead local-write fallback from CrowdSec preset apply`** (#1514 frontend) -- Files: `frontend/src/pages/CrowdSecConfig.tsx`, its four test files under `frontend/src/pages/__tests__/`, `frontend/src/api/crowdsec.ts` only if a now-unused export remains. -- Depends on: none technically (the backend never returns 501); ordered after Commit 4 so the whole #1514 story is reviewable in sequence. -- Gate: `cd frontend && npm run type-check && npx vitest run src/pages/__tests__/CrowdSecConfig*` green; no unused-export/lint findings; `npm run build`. - -**Commit 6 - `fix: persist CrowdSec hub data files across container recreation`** (#1523) -- Files: `.docker/docker-entrypoint.sh`, `configs/crowdsec/install_hub_items.sh`, `.docker/compose/docker-compose.playwright-ci.yml`, `scripts/diagnose-crowdsec.sh`, `docs/security.md` (wording), new integration test `scripts/crowdsec_data_persistence_test.sh` (follows the pattern of `scripts/crowdsec_startup_test.sh`). -- Depends on: none (independent of the Go commits; ordered after so a failing container gate does not block the Go work from being reviewed). -- Tests: shell-level test (start container with a named volume, wait for hub init, assert data files and `GeoLite2-City.mmdb` exist under `/app/data/crowdsec/data`, `docker rm` and recreate with the same volume, assert files are present and `crowdsec.log` has no `unable to init data for file` / `unable to initialize GeoIP` lines after enabling CrowdSec; also run it on a volume initialized by the old image to prove the sed migration). Add `shellcheck` cleanliness for changed scripts. -- Gate: rebuild `charon:local` (skill `docker-rebuild-e2e` / `.github/skills/scripts/skill-runner.sh docker-rebuild-e2e`), run `scripts/crowdsec_data_persistence_test.sh` and `scripts/crowdsec_startup_test.sh`; the fresh-volume case must show the install guard now really installing. -- Additional tests: the gate must prove `hub upgrade` is **not** run on a restart with intact data files (log assertion) and **is** run (once, bounded) when data files are missing; `shellcheck` is a required gate for every changed or new shell script (`.docker/docker-entrypoint.sh`, `install_hub_items.sh`, the new test script). -- CI wiring and ignore files: wire `scripts/crowdsec_data_persistence_test.sh` into the existing CrowdSec integration job via a skill wrapper like `.github/skills/integration-test-crowdsec-startup-scripts` and the `integration-tests.yml` workflow (rather than a new workflow). Check `.dockerignore` (it already excludes `scripts/tests/` and the toolchain scripts; the new script must be excluded from the image the same way if it lives under `scripts/`), `.gitignore`, and `.codecov.yml` (shell scripts are not coverage-measured; confirm no new exclusion needed). -- Snapshot interaction (item 4 sequencing): Commit 3's snapshot already excludes top-level `data/` and `hub_cache/`, so Commit 6's relocation of `data_dir` into `/app/data/crowdsec/data` does not enlarge any backup; the Commit 3 size test is the regression guard. - -**Commit 7 - `fix: report the real CrowdSec version in the bundled binaries`** (#1524, step A: ldflags) -- Files: `Dockerfile` (`crowdsec-inline`, both `xx-go build` lines). Bot-authored follow-up commit on the branch updates `CHARON_TOOLCHAIN_TAG` / `CHARON_TOOLCHAIN_DIGEST` (do not hand-edit those lines; the workflow owns them). -- Depends on: none. **Wait here until the toolchain-image workflow has built and pushed the pin bump to the PR branch** (this is the only asynchronous step in the plan). -- Gate: `toolchain-image.yml` green on the PR (build + Trivy gate), `verify-toolchain-pin` green, and a local `make build-offline`-style build (`--build-arg CROWDSEC_BUILDER_SRC=crowdsec-inline`) shows `cscli version` -> `version: v1.8.1`. -- Expected CI state: after this commit `verify-toolchain-pin` (and any check that compares TAG/DIGEST to the recipe key) is **red until the bot pushes the new pin**. Implementers must `git pull` the bot's commit before starting Commit 8 and must not hand-edit the two ARG lines. - -**Commit 8 - `fix: fail the image build if CrowdSec reports no version and make the hub branch deterministic`** (#1524, step B) -- Files: `Dockerfile` (N5 block: correct the comment, add the version assertion, add `ARG CROWDSEC_VERSION` to the final stage), `.docker/docker-entrypoint.sh` (hub branch setting per D1). -- Depends on: Commit 7 and the bot pin bump. -- Tests / build-time assertion: the Dockerfile `RUN` fails with a clear message if `cscli version` does not print `version: v${CROWDSEC_VERSION}`. Prove it fails by temporarily building with a mismatched `--build-arg CROWDSEC_VERSION` against the pinned toolchain (expected hard failure), then with the correct value (pass). Runtime check in the rebuilt container: `cscli version` truthful; `cscli hub update` succeeds online; with the network blocked (`--network none`) hub commands behave as the D1 decision specifies (no hard failure when `hub_branch` is set). -- Gate: image builds; `scripts/crowdsec_startup_test.sh` green; both stderr lines `Crowdsec version is not set` and `unable to retrieve latest crowdsec version` absent from `docker logs` in the online and offline runs. -- Record in the PR description: `cscli.hub_branch` is honoured by v1.8.1 and skips the `version.crowdsec.net` lookup (source-verified in `chooseBranch`), plus the measured online and offline (`--network none`) results. - -**Commit 9 - `test: enable CrowdSec preset and file-editor E2E specs and drop tolerant ones`** (#1518, final step) -- Files: the two specs from Commit 1 (remove `fixme`), `tests/security/crowdsec-config.spec.ts` (delete the tolerant "Preset Management" assertions that are now covered by deterministic specs, or convert each remaining case to a deterministic stub; no `isVisible().catch(() => false)` guards, no "feature may not be implemented" annotations remain in `tests/security/crowdsec-*.spec.ts`). -- Depends on: Commits 4 and 5. -- Gate: `npx playwright test tests/security/crowdsec-hub-preset-apply.spec.ts tests/security/crowdsec-file-editor.spec.ts tests/security/crowdsec-preset-apply.spec.ts tests/security/crowdsec-config.spec.ts --project=security-tests` green. Mutation check: temporarily re-add the local-write fallback (or revert the success-status check) and confirm at least one of the new specs fails, then undo. Full-suite and cross-browser runs are CI-only. - -**Commit 10 - `docs: document CrowdSec preset apply safety and data persistence`** -- Files: `ARCHITECTURE.md`, `docs/features/crowdsec.md`, `docs/troubleshooting/` entry if appropriate (see 7). -- Depends on: all. Gate: markdown lint/pre-commit; `docs-site` manifest unchanged (new content goes into already-manifested `docs/features/` / `docs/troubleshooting/`). - -## 6. Decisions and Edge Cases +## 9. Acceptance Criteria -- **D1 hub branch (Q1 resolved: option A).** Options: (A, recommended) set `cscli.hub_branch: master` explicitly: identical to today's effective behavior while 1.8.1 is the latest, no dependency on `version.crowdsec.net`, no 404 risk, truthful `cscli version`. (B) Leave auto-selection with the fixed version: matches upstream's intent but can hard-fail hub commands offline and break hub updates when 1.8.1 stops being latest and the CDN has no `v1.8.1` branch (404 today). (C) Pin to the version branch: broken today (404). Commit 8 gate must run the empirical online/offline test either way. -- **Locking (one mechanism).** A single handler-level `sync.Mutex` on `CrowdsecHandler` (e.g. `dataMu`) is held for the whole of `ApplyPreset` (both curated and hub paths), `ImportConfig` and `WriteFile`. `HubService.mu` remains as an inner guard for direct service callers; lock order is always handler mutex first, then `HubService.mu`, so no deadlock. No other lock is introduced. -- Edge cases to cover in tests: snapshot dir name collisions (microsecond timestamp plus exclusive `os.Mkdir`), parent dir not writable, `DataDir` being a mount point (the copy path must work where rename used to fall back), symlinks inside `DataDir` never followed by restore/prune/write, concurrent apply/import/file write serialized by the handler mutex (held for the full `ApplyTimeout`, so file saves block during a long apply; `ReadFile` and `ListFiles` stay lock-free), CrowdSec stopped, existing volumes whose `config.yaml` still points at `/var/lib/crowdsec/data` (sed migration, then the gated `hub upgrade` repopulates), tmpfs `/var/lib/crowdsec` deployments (no longer relevant for data after migration). -- Reload behavior from #1513 (SIGHUP via `reloadCrowdSec`) is unchanged and should also be triggered, best-effort, after the entrypoint-independent hub changes that happen at runtime only (not applicable to startup). - -## 7. Docs Impact - -- `ARCHITECTURE.md`: update the Layer 2 CrowdSec section (~L868-890): CrowdSec `data_dir` and LAPI db live under `/app/data/crowdsec/data` (persistent), hub data files are re-synced when missing via a gated `cscli hub upgrade`, preset applies and config edits are copy-based/atomic with bounded backup retention, `cscli version` reports the pinned version and the hub branch is explicit. The file layout table around L288-294 gets the data subdirectory note. -- `docs/features/crowdsec.md`: extend "Curated Presets" (L57-59) / Hub Presets with the backup retention (last N kept), that hub presets are also applied on the server with a rollback, and that the config editor only accepts specific file types with validation. Add a short troubleshooting entry for the `unable to init data for file` log line (what it meant, that it self-heals on restart after this release). Keep it novice-friendly. -- `docs/features.md`: no new capability; no change expected (confirm during Commit 10). -- `docs/security.md`: tmpfs `/var/lib/crowdsec` guidance wording (Commit 6). -- Changelog is generated from commit subjects; subjects in section 5.2 are written for that. +Part A: credentials files absent from editor list and unreadable/unwritable with the correct messages; no absolute path in any CrowdSec API response, including error text, and UI still names the backup and where it is; `..`-prefixed legitimate names accepted, parent components rejected; `WithAllowedDomains` and `hubAllowLoopback` gone and all three safeclient test references handled; `validateHubURL` accepts only https allowlisted hosts; DoD passes. Part B: the six files pass the Bz grep gate; every converted test fails when the feature it names is removed (mutation spot-check per file); findings handled per section 4; targeted Playwright green. -## 8. Test Plan Summary and Definition of Done +## 10. Rollback and Contingency -- **Backend unit:** new tests in Commits 3 and 4 (list above). Coverage: package and patch coverage must not drop; overall >= 85% (`scripts/go-test-coverage.sh`); `bash scripts/local-patch-report.sh` produces `test-results/local-patch-report.md/.json`. -- **Frontend unit:** updated Vitest in Commit 5; `scripts/frontend-test-coverage.sh` >= 85%; `npm run type-check`; `npm run build`. -- **Playwright (#1518):** deterministic stubbed specs per Commit 1/9, role-based locators, no `waitForTimeout`, no visibility-guard skips. Locally only the targeted spec files with `--project=security-tests` (security specs are excluded from the browser projects; other specs use `--project=firefox`); CI runs the full matrix. -- **Build-time assertion (#1524):** Dockerfile `RUN` check in Commit 8 with the negative (mismatched version) and positive proof recorded in the PR description. -- **Container integration:** `scripts/crowdsec_data_persistence_test.sh` and `scripts/crowdsec_startup_test.sh` (Commit 6/8). -- **Security scans:** this is a `fix:` with no new endpoints; CodeQL/Trivy are deferred to CI per CLAUDE.md, except that Commit 7's toolchain Trivy gate runs in CI by design. GORM scan not required (no model changes); run it only if a model is touched. -- **Full DoD checklist before merge (in order):** targeted Playwright (firefox) -> patch coverage preflight -> `lefthook run pre-commit` -> staticcheck (`make lint-fast`) -> backend/frontend coverage -> type-check -> `go build ./...` and `npm run build` -> clean-up (no debug prints, dead code, unused imports). Any failing test, even pre-existing, is fixed in this PR. -- **Pipeline after the plan:** Supervisor plan review -> user approval -> implement commit by commit (`backend-dev` for 2-4, `frontend-dev` for 5, `devops` for 6-8, `playwright-dev` for 1 and 9, `docs-writer` for 10) -> Supervisor implementation review -> `qa-security` last -> `docs-writer` pass. -- **Dispatch instructions (every subagent prompt must include these, verbatim in spirit):** run all tests, builds, coverage scripts, Playwright, linters, Docker builds and `lefthook` as **foreground, blocking** calls with the maximum timeout; never use `run_in_background`, `&`, `nohup` or end the turn waiting for a notification; if a call auto-backgrounds, immediately re-attach and poll until a real result; no worktrees; stay inside the repo (no live hosts); do not commit unless the orchestrator says so; Commit 4's text (body, comments, PR text) must stay vague per section 4; and the Commit 7 to Commit 8 hand-off requires pulling the toolchain bot's commit first. +Commits are independent except A2 after A1 (shared fixtures) and A5 after A4. If A5 proves too large, ship A1-A4 and keep A5 as a follow-up rather than a half-migrated seam. If B surfaces a high/critical bug, split it out per section 4. -## 9. Acceptance Criteria +## 11. Open Questions (need the user's decision) -1. Applying a hub preset or editing a CrowdSec file never renames, empties or removes `/app/data/crowdsec`; the live tree is intact after success and after any failure (tests prove it). -2. Full-tree snapshot dirs (`.backup.*`) are bounded to 5 and single-file edit backups (`.filebackup.*`) to 10, independently, after any apply/import/write attempt; snapshots exclude `data/` and `hub_cache/` so they stay small. -3. `POST /admin/crowdsec/file` validates paths, file types, YAML and size, refuses symlink writes, never renames `DataDir`, and `ReadFile`/`ListFiles` share the same validation; the frontend has no local-write fallback. -4. After `docker rm` + recreate with the same volume, the 12 listed hub data files and `GeoLite2-City.mmdb` exist and `crowdsec.log` has no `unable to init data` / GeoIP lines; fresh volumes actually install the items in `install_hub_items.sh`. -5. `cscli version` and `crowdsec -version` print `v1.8.1`; the image build fails if they do not; hub commands work online and offline per D1. -6. `crowdsec_handler.go` no longer contains the preset handlers; no behavior change from the move. -7. No tolerant "may not be implemented" Playwright tests remain for CrowdSec presets, and the mutation check in Commit 9 shows the new specs fail on regression. -8. Only one commit uses the `(security)` scope and its subject is the vague form above. - -## 10. Resolved Questions (all decided by the user) - -- Q1 (D1): RESOLVED: explicit `cscli.hub_branch: master`. -- Q2: RESOLVED by the user: no private advisory (issue is public; severity low, admin-only). Revisit only if implementation finds impact beyond admin privileges. -- Q3: RESOLVED: write allowlist, protected-file list, 1 MiB cap and the refuse-symlinks rule as specced in 3.2, with a separate looser read/list predicate. -- Q4: RESOLVED: startup log line only (no diagnostics field, no UI). -- Q5: RESOLVED: constants (5 full snapshots, 10 file backups), not configurable. +1. **F5**: approve hiding the two credentials files from editor list and read (recommended), or masked values, or leave as is? +2. **Admin export**: `ExportConfig` currently includes credentials files and `bouncer_key`. Exclude secrets from the export (behavior change: an export-then-import round trip would no longer carry credentials; machine re-registers) or leave as is and document the limitation? Recommendation: leave for this batch, document it, and file a separate issue for the export decision. +3. **Slicing**: default is one PR with commit slicing; approve the two-PR exception (A then B)? +4. **F3 shape**: base name of the backup (recommended) plus UI location hint, versus a random id with a server-side map (not recommended). +5. **`(security)` scope**: A1 as `fix(security): harden CrowdSec editor access` (recommended). A2 as plain `fix:` (recommended; admin-only low information exposure, and every `(security)` subject goes to the public changelog) or `fix(security): harden CrowdSec API responses`? +6. **#1479**: remove (recommended) versus enforce in the dialer. +7. **Include the acquisition-update `backup` field and the non-listed leak sites** (rollback text, extract/export errors) in F3 (recommended; the acceptance criterion requires it). +8. **PR B stop rule**: N=6 real bugs then re-plan with you (recommended)? diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 53cca0df6..cf657b559 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,11 +1,11 @@ -# QA and Security Report: fix/crowdsec-preset-apply-hardening (PR #1525) +# QA and Security Report: fix/crowdsec-hub-url-and-editor-hardening -Date: 2026-10-09 -Scope: full diff `origin/development...HEAD` (HEAD 513440f5), spec `docs/plans/current_spec.md` (issues #1514-#1518, #1523, #1524). +Date: 2026-10-10 +Scope: full diff `origin/development...HEAD` (HEAD 5bb1889d, 24 commits, 57 files), spec `docs/plans/current_spec.md` and `docs/plans/crowdsec_preset_apply_hardening_1525_spec.md`. Parts: (A) CrowdSec editor access hardening, backup names instead of paths, archive-name fix, removal of the unenforced `WithAllowedDomains` option, strict hub URL allowlist; (B) #1526 Playwright rewrite of six CrowdSec specs; (C) fixes found by it (i18n keys, `sanitizeSecret`, enrollment form, export gzip). ## Verdict: PASS (no blocking findings) -No critical or high findings. Two low-severity security hardening candidates are flagged for the maintainer, plus a few low/medium non-security items. None block the PR. +No critical or high findings. One medium-low gap against the spec's own acceptance criterion ("no absolute path in any CrowdSec API response") is flagged as F1; it is admin-authenticated information exposure, so it is a maintainer decision whether to close it in this PR or file it. All Definition of Done gates pass. ## Definition of Done evidence @@ -17,51 +17,55 @@ No critical or high findings. Two low-severity security hardening candidates are | golangci-lint, agent | `cd agent && golangci-lint run --config ../backend/.golangci.yml ./...` | 0 issues | | Frontend type-check | `npm run type-check` | pass | | Frontend build | `npm run build` | pass | -| GORM scan | not run | no `backend/internal/models/**`, migration or GORM query files changed | -| Lefthook | `lefthook run pre-commit --all-files` (plain run skips every hook with nothing staged) | all 17 hooks pass, including semgrep (0 findings, 367 rules), shellcheck, actionlint, dockerfile-check, golangci-lint-fast, frontend-lint | -| govulncheck | skill `security-scan-go-vuln` | 0 affected. 1 module-level advisory, GO-2026-5932 (x/crypto/openpgp, no fix), not called by Charon code | -| Trivy | `trivy fs --scanners vuln,misconfig --severity HIGH,CRITICAL` on go.mod/go.sum (backend, agent), package-lock.json and Dockerfile | no HIGH/CRITICAL output. The snap-confined trivy cannot read `/projects`, so the manifests were copied under `$HOME` for the scan. No container image scan was done. CI runs the full Trivy/CodeQL. | -| CodeQL | not run locally | change is `fix:`-scoped with no new feature surface, so deferred to CI per CLAUDE.md | -| E2E | `docker-rebuild-e2e` skill, then `npx playwright test tests/security/crowdsec-hub-preset-apply.spec.ts tests/security/crowdsec-file-editor.spec.ts tests/security/crowdsec-config.spec.ts --project=security-tests` | 27 passed, 0 failed (image rebuilt first because the Dockerfile and merge commits were newer than the running container) | -| Coverage (given, not rerun) | prior runs | backend 90.9% stmt, frontend 91.4% lines, patch 93.9% | - -The `go-test-coverage` and `local-patch-report` scripts were not rerun, as instructed. The working tree stayed clean until this report was written. - -## Security review - -Reviewed against SECURITY.md. No exploit detail is given here. - -- `backend/internal/crowdsec/backup.go`: snapshots and restores copy rather than rename. Symlinks are preserved literally and never followed when copying. Clearing the config removes symlinks themselves, because `DirEntry.IsDir` is false for links. Directories are created 0700. Engine-owned state (live db and WAL/SHM at any depth, top-level `data/` and `hub_cache/`) is excluded from snapshot, restore, clear and import. Prune sorts by the timestamp in the directory name, uses `Lstat` and skips non-directories, so a symlinked "backup" is ignored. No issue. -- `backend/internal/api/handlers/crowdsec_files.go`: relative-path checks use `filepath.IsLocal`, so NUL, absolute and `..` paths are rejected. Read resolves symlinks, checks containment, re-applies the deny-list to the resolved path and requires a regular file. Write refuses any symlink along the path and any non-regular target. It also enforces the extension allowlist, the credential-file deny-list, the temp-prefix deny-list, `hub/` and `config/hub/` exclusion, a 1 MiB content cap and an 8 MiB body cap (`MaxBytesReader`). Writes go through a temp file, fsync and rename, and the original mode is preserved. A single-file backup is taken before the write. Operations serialize on `dataMu`. Logged errors go through `sanitizeForLog`. Adequate. Minor items are F2 to F4 below. -- `crowdsec_handler.go` `ImportConfig`: takes `dataMu`, then snapshots, clears, extracts and validates. It rolls back in place on failure and then prunes. The extractor blocks traversal and skips engine-owned entries, and symlink and hardlink entries are not materialized. Lock order is documented (`dataMu` before `HubService.mu`). See F3. -- `hub_sync.go` and `curated_apply.go`: apply is now snapshot, then cscli or extract, then restore on failure, with prune afterwards. The extractor rejects symlink entries, traversal and absolute paths, strips setuid and setgid bits, caps decompressed size at 100 MB and skips engine-owned paths. See F1. -- `.docker/docker-entrypoint.sh`: the new `data_dir` redirect, `hub_branch` pin and missing-data recovery are idempotent and non-fatal. `hub upgrade` is bounded by `timeout 120s`. Values read from hub YAML (`dest_file`) are only used in `[ -e ]` tests and echoes, never executed or written. All variables are quoted. shellcheck passes. -- `configs/crowdsec/install_hub_items.sh`: the `is_installed` helper greps the JSON "installed" flag. No injection surface (arguments are static). -- `Dockerfile`: the XMOD pin is parameterized and registered in `scripts/toolchain-key.sh` and the toolchain key. The x/net and x/crypto re-pin and the post-build `go version -m` guards are correct (`sort -V` floor check, fails the build). The ldflags target `go-cs-lib/version.Version`, and a new cscli `version: v${CROWDSEC_VERSION}` assertion guards against a silent no-op `-X`. The toolchain tag and digest were synced. No weakening. -- Frontend `CrowdSecConfig.tsx`: the client-side preset fallback write is removed, so the server is authoritative. Server error text is rendered via toast text (React-escaped), so there is no XSS. Mutation errors now surface. -- CI: the new `notify-codecov` job uses a SHA-pinned action, `persist-credentials: false` and a minimal `if` guard. The persistence integration step is wired with cleanup. +| GORM scan | not run | no `backend/internal/models/**`, migration or GORM query files changed (0 matches in the diff) | +| Lefthook | `lefthook run pre-commit --all-files` | all 17 hooks pass, including semgrep (0 findings, 367 rules; it logged timeout warnings on three large test files, which are scan-coverage notes, not findings), shellcheck, actionlint, golangci-lint-fast, frontend-lint. Working tree stayed clean. | +| govulncheck | skill `security-scan-go-vuln` | 0 affected; 1 module-level advisory in a required module that Charon code does not call | +| Trivy | `trivy fs --scanners vuln,misconfig --severity HIGH,CRITICAL` on go.mod/go.sum (backend, agent), package-lock.json and Dockerfile | no HIGH/CRITICAL. The snap trivy cannot read `/projects`, so the manifests were copied under `$HOME` and removed afterwards. No container-image scan locally. | +| CodeQL | not run locally | `fix:`/`test:` scope, no new feature surface, deferred to CI per CLAUDE.md | +| E2E | `npx playwright test --project=security-tests` | 179 passed, 0 failed (8.4 min). The E2E container (built at 841955af) is valid for HEAD: `git diff 841955af..HEAD` touches only `_test.go`, vitest and Playwright files. | +| Coverage (given, not rerun) | prior runs | backend 91.0% stmt / 90.1% line, frontend 91.46% lines, patch 98.9% | + +## Check-by-check results + +1. **No absolute path in responses.** All nine leak sites from spec section 2.3 are closed: `backup` fields now go through `crowdsec.BackupID` (file write, import, preset apply success and failure, curated apply, acquisition update); `rollbackFailure`, extractor, `CreateFileNoSymlink` and import/export errors no longer embed paths; preset pull/apply, import validation errors pass through `RedactPaths`. Backed by `crowdsec_response_paths_test.go` and the live E2E checks. Residual gaps are listed as F1 and F2. +2. **`sanitizeSecret`.** Still masks anything alphanumeric 10-64 chars that mixes letters and digits, and letters-only runs of 32+. A letters-only token of 10-31 chars, or one split by `-`/`_`, is not masked client-side (previously every 10-64 char run was masked). Real enrollment keys are random alphanumeric, so the chance of a digit-free 25-char key is negligible, and the backend already redacts the exact token (`redactSecret`, `console_enroll.go:421`) before any message reaches the client. Acceptable as defense in depth; see F3. +3. **Hub URL check.** `validateHubURL` now accepts only https plus the three allowlisted hosts. Previously `localhost`, `127.0.0.1`, `::1`, `*.example.com`, `*.example`, `*.local` and `test.hub` passed this layer, but the dial layer (`NewSafeHTTPClient`) already blocked loopback and private targets in production, and unknown public hosts were already rejected by the allowlist. So no production behavior changes for `HUB_BASE_URL` or `HUB_MIRROR_BASE_URL`; the docs note (`docs/troubleshooting/crowdsec.md`) is accurate. Redirects: the hub client uses default `MaxRedirects` 0 (`CheckRedirect` returns `ErrUseLastResponse`), so a redirect cannot reach an unvalidated host. `WithAllowedDomains` was never read anywhere (`AllowedDomains` had no consumers), so its removal is behavior-neutral and the SSRF protection actually relied on the dial layer plus `validateHubURL`. The test seam `HubService.validateURL` is an unexported field set in exactly one test (`hub_sync_test.go:117`, a local httptest server); production never sets it and nothing outside the package can. The other tests use allowlisted hostnames with mock transports, or unreachable `127.0.0.1:1` URLs that only assert failure (`backup_test.go:598-663`), so none depends on a real non-production host. +4. **gzip exclusion.** gin-contrib/gzip v1.2.8 matches `strings.HasPrefix(requestURI, path)`. The only registered route with the prefix `/api/v1/admin/crowdsec/export` is `GET /admin/crowdsec/export`; `/admin/crowdsec/decisions/export` does not start with it. No other route is excluded. Caveat in F5. +5. **E2E tolerance.** The Bz command from the spec over the six files returns nothing (exit 1). An extra scan of the six specs and both helpers for `console.log`, `annotations.push`, `test.fixme` and `.skip(` also returns nothing. `tests/` is in `.dockerignore`, not copied by the Dockerfile, and `crowdsec-stubs.ts` / `archive-helpers.ts` are not imported from `frontend/` or `backend/`, so no test helper ships in a production bundle. +6. **Commit subjects.** Reviewed with `git log origin/development..HEAD --format=%s`. The only `(security)` subject, `fix(security): harden CrowdSec editor access`, is vague. See F4 for two non-scoped subjects that are slightly specific. +7. **Security-sensitive items.** See F1 and F6; no exploit detail is given. ## Findings No critical or high findings. -### Security-sensitive (do not file publicly; maintainer to decide on advisory or hardening commit) +### Security-sensitive (do not file publicly; maintainer to decide) -**S1: low.** `backend/internal/crowdsec/hub_sync.go:951-971` (`extractTarGz` file open). The old flow emptied the target before extracting; the new documented overlay design does not. File creation uses `O_CREATE|O_TRUNC` without a symlink check on the destination, so it follows any pre-existing link in the live tree. The archive is hub-sourced and cached, and symlink entries in the archive are already rejected, so exploitation needs a hostile hub archive plus an existing link. The spec's "residual risk" note covers overlaying but not this write-through. Suggested hardening: `Lstat` the destination (and ideally its parents) and refuse symlinks, or open with `O_NOFOLLOW`. +**F1: low-medium (information exposure, admin-authenticated).** Absolute server paths can still reach CrowdSec API responses at sites that the spec did not enumerate, contradicting its acceptance criterion "no absolute path in any CrowdSec API response, including error text": +- `backend/internal/api/handlers/crowdsec_handler.go:1977` and `:1986`: `GET /admin/crowdsec/acquisition` returns `path` (the acquisition file location) on both 404 and 200. The frontend does not read it. +- `crowdsec_handler.go` diagnostics config (`~2136-2230`): `validation["config_path"]` and `validation["acquis_path"]` return absolute paths. The frontend does not read them. +- `crowdsec_handler.go:522` (Start), `:596` (Stop), `:624` (Status), `:978` (ClearEnrollment), `:2250` (console status), and `crowdsec_preset_handler.go:395` (preview) return raw `err.Error()`. The executor errors wrap filesystem errors (`crowdsec_exec.go:71` "failed to write pid file: %w", `:92` "pid file read: %w"), which embed the pid-file path. +Suggested remediation: drop the `path` fields (or return a fixed label), and either pass those errors through `crowdsec.RedactPaths` or return fixed messages and log the detail. Add the sites to `crowdsec_response_paths_test.go`. -**S2: low.** `backend/internal/api/handlers/crowdsec_handler.go:793` (`extractArchive`). Same pattern: `OpenFile` without `O_EXCL` or `O_TRUNC`, which can leave stale trailing bytes when an archive repeats a name. The tree is cleared first, so impact is limited to duplicate entries. Fix: `O_CREATE|O_WRONLY|O_TRUNC` plus a symlink check. +**F6: low (documented limitation).** `ExportConfig` still archives the credentials files and `bouncer_key` that the editor now hides (the spec records this as a known limitation and defers the decision). `docs/features/crowdsec.md` and `docs/features.md` say the editor no longer lists the credential files but do not mention that the config export still includes them, which could give false assurance. Suggested: one sentence in the docs and a tracked issue for the export decision. -### Medium and low non-security (for the maintainer to file) +### Non-security (for the maintainer to file) -- **F1: low.** `backend/internal/crowdsec/hub_sync.go:954-957`. The containment check compares against `rel == ".."` and uses `HasPrefix(cleanName, "..")`, which also rejects legitimate names starting with `..` (for example `..foo`). Cosmetic and safe, since it errs on the strict side. -- **F2: low (information exposure to authenticated admin).** `crowdsec_files.go:300` and `:325` return `err.Error()` to the client on 500, which can embed absolute server paths. Other branches use fixed messages. Suggested: generic message and log the detail with `fileErrorLog`. -- **F3: low.** The `backup` field in the `WriteFile` (`crowdsec_files.go:391`) and `ImportConfig` (`crowdsec_handler.go:727`) responses returns an absolute server path. This is consistent with the existing preset-apply behavior and with UI expectations, so it is likely intentional. The extraction error response in `crowdsec_handler.go:715` also interpolates the archive entry name and `%v` error text. -- **F4: low.** `ReadFile` (`crowdsec_files.go:319`) has no read-size cap (the write cap is 1 MiB). The risk is small because the endpoint is admin-only and the tree is config-sized. -- **F5: info.** `local_api_credentials.yaml` and `online_api_credentials.yaml` are write-protected but readable and listed by the editor. This matches the design where `isWritable` is a subset of `isReadable`; confirm that exposing them to admins in the UI is intended. -- **F6: low (test quality).** Known and accepted: #1526 (tolerant Playwright specs). +- **F2: low.** `RedactPaths` (`crowdsec/backup.go:351-356`) only redacts absolute paths preceded by whitespace, a quote, `(` or `=` and ends them at whitespace, `:`, `;`, `)` or a quote. Paths preceded by `[`, `{`, `,`, `<`, `@` or `:`, and the tail of a path containing a space or colon, would pass through. Today every redacted source (cscli output, validators, extractor) uses the "open /path: reason" form, so nothing leaks in tested cases; the function is a safety net, not a guarantee. It also rewrites non-path text such as " /api/v1/x" to `` (cosmetic). +- **F3: low.** `frontend/src/utils/sanitizeSecret.ts:10-18` no longer masks letters-only runs of 10-31 characters (see check 2). Backend redaction is the primary control. Optional: keep masking any 20+ char run regardless of content. +- **F4: low (changelog wording).** `fix: stop returning server paths in CrowdSec responses` (66a4aced) and `refactor: remove the unenforced allowed-domains client option` (1725feb8) are not `(security)`-scoped but name the issue category fairly directly and will appear verbatim in the public changelog. Consider rewording before merge, for example `fix: show CrowdSec backups by name` and `refactor: remove unused HTTP client option`. No hash is pushed yet, so rewording is cheap. +- **F5: info.** `routes.go:1260` excludes by URI prefix, so any future route that begins with `/api/v1/admin/crowdsec/export` (for example `export-foo`, or a query string) would also skip compression. Harmless today; an exact-path or regex exclusion would be stricter. +- **F7: info.** The prior report's S1/S2 (extraction opens destination files without a symlink check in `extractTarGz` and `ImportConfig.extractArchive`) were not re-audited in this pass beyond the changed lines; `CreateFileNoSymlink` is unchanged in behavior. ## Out of scope and not verified -- Codecov manual-trigger behavior cannot be tested locally. - Full-suite and cross-browser E2E are deferred to CI. - CodeQL and a container-image Trivy scan are deferred to CI. +- Coverage and local patch scripts were not rerun, as instructed. + +## Resolution (F1, F2 and the remaining findings) + +- **F1: fixed.** `GET /admin/crowdsec/acquisition` no longer returns `path`; `GET /admin/crowdsec/diagnostics/config` no longer returns `config_path` or `acquis_path` (nothing in `frontend/src` or `tests/` read them). Start, Stop, Status, ClearEnrollment, console heartbeat and the preset cache preview/metadata handlers now return fixed messages and log the detail server-side; console enrollment errors, which are user-facing validation text, pass through `RedactPaths`. Covered by `crowdsec_response_paths_test.go`. +- **F2: fixed.** `RedactPaths` now matches absolute paths at the start of the string or after whitespace, a quote, `(`, `[`, `{`, `<`, `=` or `,`, ends them at whitespace, a quote or `: ; , ) ] } > <`, and leaves text under `/api/` untouched. Remaining limits (paths containing spaces or `:`, Windows or relative paths) are documented on the function; handlers prefer fixed messages. Covered by `TestRedactPaths`. +- **F3, F5: reviewed and left** as low or informational, as recommended (backend redaction is the primary control; the gzip exclusion is exact for every registered route). +- **F4: handled by commit rewording** before merge. diff --git a/docs/troubleshooting/crowdsec.md b/docs/troubleshooting/crowdsec.md index 7279858a4..5eb44dd40 100644 --- a/docs/troubleshooting/crowdsec.md +++ b/docs/troubleshooting/crowdsec.md @@ -15,8 +15,11 @@ Keep Cerberus terminology and the Configuration Packages flow in mind while debu - Bare-metal deployments: install cscli for Hub preset sync or use HTTP fallback with HUB_BASE_URL. - HUB_BASE_URL points to a JSON hub endpoint (default: ). Redirects to HTML will be rejected. - A loopback hub mirror is no longer reachable from the hub client; RFC 1918 hosts were already blocked. + - The hub address must be an `https` address on the official CrowdSec hub hosts. Custom `http`, local, or other mirror addresses are not supported. - Proxy env is set when required: HTTP(S)_PROXY and NO_PROXY are respected by the hub client. - For slow or proxied networks, increase HUB_PULL_TIMEOUT_SECONDS (default 25) and HUB_APPLY_TIMEOUT_SECONDS (default 45) to avoid premature timeouts. +- The config file editor hides CrowdSec's account and connection credential files; they remain on disk (see `docker exec charon ls -la /app/data/crowdsec/config`). +- Backups sit next to the CrowdSec data folder, named `crowdsec.backup.` and `crowdsec.filebackup.`. Acquisition settings are backed up as `acquis.yaml.backup.` right beside the acquisition file. For manual recovery, run `docker exec charon ls -la /app/data` to find them, stop CrowdSec, copy the one you want back, then start it again. - Preset workflow: pull from Hub using cache keys/ETags → preview changes → apply with automatic backup and reload flag. - Preset pull/apply requires either cscli or cached presets. - Offline/curated presets remain available at all times. diff --git a/frontend/src/components/crowdsec/DashboardTimeRangeSelector.tsx b/frontend/src/components/crowdsec/DashboardTimeRangeSelector.tsx index 80897a3bf..0c0511fd3 100644 --- a/frontend/src/components/crowdsec/DashboardTimeRangeSelector.tsx +++ b/frontend/src/components/crowdsec/DashboardTimeRangeSelector.tsx @@ -57,7 +57,7 @@ export function DashboardTimeRangeSelector({ value, onChange }: DashboardTimeRan return (
{RANGES.map((range, i) => ( diff --git a/frontend/src/hooks/__tests__/useConsoleEnrollment.test.tsx b/frontend/src/hooks/__tests__/useConsoleEnrollment.test.tsx index 24e750b3c..3dadd2209 100644 --- a/frontend/src/hooks/__tests__/useConsoleEnrollment.test.tsx +++ b/frontend/src/hooks/__tests__/useConsoleEnrollment.test.tsx @@ -3,11 +3,13 @@ import { renderHook, waitFor } from '@testing-library/react' import { describe, it, expect, vi, beforeEach } from 'vitest' import * as consoleEnrollmentApi from '../../api/consoleEnrollment' -import { useConsoleStatus, useEnrollConsole } from '../useConsoleEnrollment' +import { toast } from '../../utils/toast' +import { useClearConsoleEnrollment, useConsoleStatus, useEnrollConsole } from '../useConsoleEnrollment' import type { ConsoleEnrollmentStatus, ConsoleEnrollPayload } from '../../api/consoleEnrollment' vi.mock('../../api/consoleEnrollment') +vi.mock('../../utils/toast', () => ({ toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() } })) describe('useConsoleEnrollment hooks', () => { let queryClient: QueryClient @@ -528,4 +530,28 @@ describe('useConsoleEnrollment hooks', () => { expect(result.current.data?.agent_name).toBeUndefined() }) }) + + describe('useClearConsoleEnrollment', () => { + it('invalidates console status on success', async () => { + vi.mocked(consoleEnrollmentApi.clearConsoleEnrollment).mockResolvedValue(undefined) + const invalidate = vi.spyOn(queryClient, 'invalidateQueries') + + const { result } = renderHook(() => useClearConsoleEnrollment(), { wrapper }) + result.current.mutate() + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(invalidate).toHaveBeenCalledWith({ queryKey: ['crowdsec-console-status'] }) + expect(toast.error).not.toHaveBeenCalled() + }) + + it('shows an error toast when clearing fails', async () => { + vi.mocked(consoleEnrollmentApi.clearConsoleEnrollment).mockRejectedValue(new Error('backend unavailable')) + + const { result } = renderHook(() => useClearConsoleEnrollment(), { wrapper }) + result.current.mutate() + + await waitFor(() => expect(result.current.isError).toBe(true)) + expect(toast.error).toHaveBeenCalledWith('Failed to clear enrollment state: backend unavailable') + }) + }) }) diff --git a/frontend/src/hooks/useConsoleEnrollment.ts b/frontend/src/hooks/useConsoleEnrollment.ts index 9dc199a80..6ef970470 100644 --- a/frontend/src/hooks/useConsoleEnrollment.ts +++ b/frontend/src/hooks/useConsoleEnrollment.ts @@ -1,6 +1,10 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { isAxiosError } from 'axios' +import { useTranslation } from 'react-i18next' import { enrollConsole, getConsoleStatus, clearConsoleEnrollment, type ConsoleEnrollPayload, type ConsoleEnrollmentStatus } from '../api/consoleEnrollment' +import { sanitizeSecret } from '../utils/sanitizeSecret' +import { toast } from '../utils/toast' export function useConsoleStatus(enabled = true) { return useQuery({ queryKey: ['crowdsec-console-status'], queryFn: getConsoleStatus, enabled }) @@ -18,11 +22,16 @@ export function useEnrollConsole() { export function useClearConsoleEnrollment() { const queryClient = useQueryClient() + const { t } = useTranslation() return useMutation({ mutationFn: clearConsoleEnrollment, onSuccess: () => { queryClient.invalidateQueries({ queryKey: ['crowdsec-console-status'] }) }, + onError: (err: unknown) => { + const detail = isAxiosError(err) ? err.response?.data?.error || err.message : err instanceof Error ? err.message : '' + toast.error(t('crowdsecConfig.reenroll.clearFailed', { message: sanitizeSecret(String(detail)) })) + }, }) } diff --git a/frontend/src/locales/de/translation.json b/frontend/src/locales/de/translation.json index 1f9cf3085..cb0c69668 100644 --- a/frontend/src/locales/de/translation.json +++ b/frontend/src/locales/de/translation.json @@ -384,8 +384,17 @@ "24h": "24h", "7d": "7d", "30d": "30d" - } - } + }, + "timeRangeLabel": "Zeitraum" + }, + "bouncerApiKey": "Bouncer-API-Schlüssel", + "keyCopied": "API-Schlüssel in die Zwischenablage kopiert", + "keyStoredAt": "Schlüssel gespeichert unter", + "noKeyConfigured": "Kein Bouncer-Schlüssel konfiguriert. Aktivieren Sie CrowdSec für die automatische Registrierung.", + "notRegistered": "Nicht registriert", + "registered": "Registriert", + "sourceEnvVar": "Aus Umgebungsvariable", + "sourceFile": "Aus Datei" }, "acl": { "title": "Zugriffskontrolle", @@ -1167,7 +1176,7 @@ "goToSecurity": "Zum Sicherheits-Dashboard", "enrollmentToken": "Registrierungstoken", "tokenPlaceholder": "Fügen Sie Ihr Registrierungstoken von der CrowdSec-Konsole ein", - "agentName": "Agent-Name (optional)", + "agentName": "Agent-Name", "agentPlaceholder": "meine-charon-instanz", "tenant": "Mandant/Organisation (optional)", "tenantPlaceholder": "org-id-aus-konsole", @@ -1186,7 +1195,15 @@ "tenantLabel": "Mandant", "lastAttempt": "Letzter Versuch", "enrolledAt": "Registriert", - "correlationId": "Korrelations-ID" + "correlationId": "Korrelations-ID", + "lastError": "Letzter Fehler", + "ackText": "Mir ist bekannt, dass meine LAPI-Zugangsdaten rotiert werden, falls bereits registriert", + "description": "Verbinden Sie Ihre CrowdSec-LAPI mit der CrowdSec Console für zentrale Verwaltung und Warnungen.", + "enrollToken": "Registrierungstoken", + "lastHeartbeat": "Letzter Heartbeat", + "privacyNote": "Hinweis: Die Registrierung sendet Heartbeat-Daten und Metriken an CrowdSec. Details finden Sie in der Datenschutzrichtlinie von CrowdSec.", + "tenantHelper": "Optional: Mandanten-/Organisations-ID angeben, falls Sie ein Multi-Tenant-Setup verwenden", + "tokenHelper": "Holen Sie sich Ihr Registrierungstoken aus der CrowdSec Console" }, "reenroll": { "title": "Neu-Registrierungsoptionen", @@ -1202,14 +1219,16 @@ "reenrolling": "Neu registrieren...", "clearState": "Registrierungsstatus löschen", "clearing": "Lösche...", - "clearConfirm": "Dies löscht den lokalen Registrierungsstatus. Möglicherweise müssen Sie sich erneut registrieren. Fortfahren?" + "clearConfirm": "Dies löscht den lokalen Registrierungsstatus. Möglicherweise müssen Sie sich erneut registrieren. Fortfahren?", + "clearFailed": "Der Registrierungsstatus konnte nicht gelöscht werden: {{message}}" }, "packages": { "title": "Konfigurationspakete", "description": "Exportieren oder importieren Sie CrowdSec-Konfigurationspakete (tar.gz/zip). Vor dem Import wird ein Backup erstellt.", "export": "Exportieren", "import": "Importieren", - "importing": "Importiere..." + "importing": "Importiere...", + "selectFile": "CrowdSec-Paketdatei auswählen" }, "presets": { "title": "Vorlagen", @@ -1235,18 +1254,22 @@ "previewUnavailable": "Vorschau nicht verfügbar", "applied": "Angewendet", "backup": "Backup", + "backupCreated": "Sicherung erstellt: {{name}} ({{location}})", + "backupLocation": "neben dem CrowdSec-Datenverzeichnis gespeichert", "reload": "Neu laden", "required": "Erforderlich", "method": "Methode", "filesystem": "Dateisystem", "loadCached": "Zwischengespeicherte Vorschau laden", "applyDisabled": "Anwenden deaktiviert, solange Hub offline ist.", - "noPresets": "Keine Vorlagen verfügbar. Stellen Sie sicher, dass Cerberus aktiviert ist." + "noPresets": "Keine Vorlagen verfügbar. Stellen Sie sicher, dass Cerberus aktiviert ist.", + "sortBy": "Presets sortieren" }, "files": { "title": "Konfigurationsdateien bearbeiten", "selectFile": "Datei auswählen...", - "refresh": "Aktualisieren" + "refresh": "Aktualisieren", + "content": "Dateiinhalt" }, "bannedIps": { "title": "Gesperrte IPs", @@ -1281,7 +1304,38 @@ "title": "Entsperrung bestätigen", "confirm": "Sind Sie sicher, dass Sie entsperren möchten", "submit": "Entsperren" - } + }, + "lapiInitializing": "CrowdSec läuft, aber die lokale API wird noch initialisiert.", + "lapiInitializingDesc": "Die Console-Registrierung ist verfügbar, sobald die lokale API bereit ist. Das kann nach dem Start von CrowdSec bis zu einer Minute dauern.", + "checkingAgain": "(wird erneut geprüft...)", + "notRunning": "CrowdSec läuft nicht", + "notRunningDesc": "Starten Sie CrowdSec, bevor Sie sich in der Console registrieren.", + "starting": "CrowdSec wird gestartet...", + "startFailed": "CrowdSec konnte nicht gestartet werden", + "startCrowdsec": "CrowdSec starten", + "goToSecurity": "Zu Sicherheit wechseln", + "whitelist": { + "tabLabel": "Whitelist", + "title": "IP-Whitelist", + "description": "IPs und CIDRs auf der Whitelist werden von CrowdSec nie blockiert, auch wenn sie Warnungen auslösen.", + "ipLabel": "IP oder CIDR", + "reasonLabel": "Grund", + "reasonPlaceholder": "Optionaler Grund", + "addMyIp": "Meine IP hinzufügen", + "none": "Keine Whitelist-Einträge", + "columnIp": "IP / CIDR", + "columnReason": "Grund", + "columnAdded": "Hinzugefügt", + "deleteAriaLabel": "Whitelist-Eintrag entfernen für", + "delete": "Löschen", + "deleteModal": { + "title": "Whitelist-Eintrag entfernen", + "body": "{{ip}} von der Whitelist entfernen? CrowdSec kann diese IP dann blockieren, wenn sie Warnungen auslöst.", + "submit": "Entfernen" + } + }, + "noConfig": "CrowdSec-Konfiguration im Sicherheitsstatus nicht gefunden", + "noteText": "CrowdSec wird über den Schalter auf der Seite gesteuert:" }, "smtp": { "title": "E-Mail (SMTP) Einstellungen", diff --git a/frontend/src/locales/en/translation.json b/frontend/src/locales/en/translation.json index a72858cbd..46c2de23c 100644 --- a/frontend/src/locales/en/translation.json +++ b/frontend/src/locales/en/translation.json @@ -488,7 +488,8 @@ "24h": "24h", "7d": "7d", "30d": "30d" - } + }, + "timeRangeLabel": "Time range" } }, "acl": { @@ -1505,7 +1506,7 @@ "goToSecurity": "Go to Security Dashboard", "enrollmentToken": "Enrollment Token", "tokenPlaceholder": "Paste your enrollment token from CrowdSec Console", - "agentName": "Agent Name (optional)", + "agentName": "Agent Name", "agentPlaceholder": "my-charon-instance", "tenant": "Tenant/Organization (optional)", "tenantPlaceholder": "org-id-from-console", @@ -1524,7 +1525,8 @@ "tenantLabel": "Tenant", "lastAttempt": "Last attempt", "enrolledAt": "Enrolled", - "correlationId": "Correlation ID" + "correlationId": "Correlation ID", + "lastError": "Last error" }, "reenroll": { "title": "Re-enrollment Options", @@ -1540,14 +1542,16 @@ "reenrolling": "Re-enrolling...", "clearState": "Clear enrollment state", "clearing": "Clearing...", - "clearConfirm": "This will clear local enrollment state. You may need to re-enroll. Continue?" + "clearConfirm": "This will clear local enrollment state. You may need to re-enroll. Continue?", + "clearFailed": "Failed to clear enrollment state: {{message}}" }, "packages": { "title": "Configuration Packages", "description": "Export or import CrowdSec configuration packages (tar.gz/zip). A backup is created before importing.", "export": "Export", "import": "Import", - "importing": "Importing..." + "importing": "Importing...", + "selectFile": "Select CrowdSec package file" }, "presets": { "title": "Presets", @@ -1573,18 +1577,22 @@ "previewUnavailable": "Preview unavailable", "applied": "Applied", "backup": "Backup", + "backupCreated": "Backup created: {{name}} ({{location}})", + "backupLocation": "stored next to the CrowdSec data directory", "reload": "Reload", "required": "Required", "method": "Method", "filesystem": "Filesystem", "loadCached": "Load cached preview", "applyDisabled": "Apply disabled while hub is offline.", - "noPresets": "No presets available. Ensure Cerberus is enabled." + "noPresets": "No presets available. Ensure Cerberus is enabled.", + "sortBy": "Sort presets" }, "files": { "title": "Edit Configuration Files", "selectFile": "Select a file...", - "refresh": "Refresh" + "refresh": "Refresh", + "content": "File content" }, "bannedIps": { "title": "Banned IPs", @@ -1619,6 +1627,35 @@ "title": "Confirm Unban", "confirm": "Are you sure you want to unban", "submit": "Unban" + }, + "lapiInitializing": "CrowdSec is running, but the Local API is still initializing.", + "lapiInitializingDesc": "Console enrollment will be available once the Local API is ready. This can take up to a minute after CrowdSec starts.", + "checkingAgain": "(checking again...)", + "notRunning": "CrowdSec is not running", + "notRunningDesc": "Start CrowdSec before enrolling in the Console.", + "starting": "Starting CrowdSec...", + "startFailed": "Failed to start CrowdSec", + "startCrowdsec": "Start CrowdSec", + "goToSecurity": "Go to Security", + "whitelist": { + "tabLabel": "Whitelist", + "title": "IP Whitelist", + "description": "Whitelisted IPs and CIDRs are never blocked by CrowdSec, even if they trigger alerts.", + "ipLabel": "IP or CIDR", + "reasonLabel": "Reason", + "reasonPlaceholder": "Optional reason", + "addMyIp": "Add My IP", + "none": "No whitelist entries", + "columnIp": "IP / CIDR", + "columnReason": "Reason", + "columnAdded": "Added", + "deleteAriaLabel": "Remove whitelist entry for", + "delete": "Delete", + "deleteModal": { + "title": "Remove Whitelist Entry", + "body": "Remove {{ip}} from the whitelist? CrowdSec may then block this IP if it triggers alerts.", + "submit": "Remove" + } } }, "smtp": { diff --git a/frontend/src/locales/es/translation.json b/frontend/src/locales/es/translation.json index 7768f0d6f..8720ce0f4 100644 --- a/frontend/src/locales/es/translation.json +++ b/frontend/src/locales/es/translation.json @@ -384,8 +384,17 @@ "24h": "24h", "7d": "7d", "30d": "30d" - } - } + }, + "timeRangeLabel": "Rango de tiempo" + }, + "bouncerApiKey": "Clave API del bouncer", + "keyCopied": "Clave API copiada al portapapeles", + "keyStoredAt": "Clave almacenada en", + "noKeyConfigured": "No hay clave de bouncer configurada. Active CrowdSec para el registro automático.", + "notRegistered": "No registrado", + "registered": "Registrado", + "sourceEnvVar": "Desde variable de entorno", + "sourceFile": "Desde archivo" }, "acl": { "title": "Control de Acceso", @@ -1169,7 +1178,7 @@ "goToSecurity": "Ir al Panel de Seguridad", "enrollmentToken": "Token de inscripción", "tokenPlaceholder": "Pegue su token de inscripción de la Consola CrowdSec", - "agentName": "Nombre del agente (opcional)", + "agentName": "Nombre del agente", "agentPlaceholder": "mi-instancia-charon", "tenant": "Inquilino/Organización (opcional)", "tenantPlaceholder": "id-org-de-consola", @@ -1188,7 +1197,15 @@ "tenantLabel": "Inquilino", "lastAttempt": "Último intento", "enrolledAt": "Inscrito", - "correlationId": "ID de correlación" + "correlationId": "ID de correlación", + "lastError": "Último error", + "ackText": "Entiendo que esto rotará mis credenciales de LAPI si ya estoy registrado", + "description": "Conecte su LAPI de CrowdSec a la consola de CrowdSec para gestión centralizada y alertas.", + "enrollToken": "Token de registro", + "lastHeartbeat": "Último latido", + "privacyNote": "Nota: El registro envía datos de latido y métricas a CrowdSec. Consulte la política de privacidad de CrowdSec para más detalles.", + "tenantHelper": "Opcional: indique el ID de inquilino/organización si usa una configuración multiinquilino", + "tokenHelper": "Obtenga su token de registro en la consola de CrowdSec" }, "reenroll": { "title": "Opciones de reinscripción", @@ -1204,14 +1221,16 @@ "reenrolling": "Reinscribiendo...", "clearState": "Limpiar estado de inscripción", "clearing": "Limpiando...", - "clearConfirm": "Esto limpiará el estado de inscripción local. Puede que necesite reinscribirse. ¿Continuar?" + "clearConfirm": "Esto limpiará el estado de inscripción local. Puede que necesite reinscribirse. ¿Continuar?", + "clearFailed": "No se pudo borrar el estado de registro: {{message}}" }, "packages": { "title": "Paquetes de configuración", "description": "Exporte o importe paquetes de configuración de CrowdSec (tar.gz/zip). Se crea una copia de seguridad antes de importar.", "export": "Exportar", "import": "Importar", - "importing": "Importando..." + "importing": "Importando...", + "selectFile": "Seleccionar archivo de paquete de CrowdSec" }, "presets": { "title": "Plantillas", @@ -1237,18 +1256,22 @@ "previewUnavailable": "Vista previa no disponible", "applied": "Aplicado", "backup": "Respaldo", + "backupCreated": "Copia de seguridad creada: {{name}} ({{location}})", + "backupLocation": "almacenada junto al directorio de datos de CrowdSec", "reload": "Recargar", "required": "Requerido", "method": "Método", "filesystem": "Sistema de archivos", "loadCached": "Cargar vista previa en caché", "applyDisabled": "Aplicar deshabilitado mientras el hub está fuera de línea.", - "noPresets": "No hay plantillas disponibles. Asegúrese de que Cerberus esté habilitado." + "noPresets": "No hay plantillas disponibles. Asegúrese de que Cerberus esté habilitado.", + "sortBy": "Ordenar presets" }, "files": { "title": "Editar archivos de configuración", "selectFile": "Seleccionar un archivo...", - "refresh": "Actualizar" + "refresh": "Actualizar", + "content": "Contenido del archivo" }, "bannedIps": { "title": "IPs bloqueadas", @@ -1283,7 +1306,38 @@ "title": "Confirmar desbloqueo", "confirm": "¿Está seguro de que desea desbloquear", "submit": "Desbloquear" - } + }, + "lapiInitializing": "CrowdSec se está ejecutando, pero la API local aún se está inicializando.", + "lapiInitializingDesc": "El registro en la consola estará disponible cuando la API local esté lista. Puede tardar hasta un minuto después de iniciar CrowdSec.", + "checkingAgain": "(comprobando de nuevo...)", + "notRunning": "CrowdSec no se está ejecutando", + "notRunningDesc": "Inicie CrowdSec antes de registrarse en la consola.", + "starting": "Iniciando CrowdSec...", + "startFailed": "No se pudo iniciar CrowdSec", + "startCrowdsec": "Iniciar CrowdSec", + "goToSecurity": "Ir a Seguridad", + "whitelist": { + "tabLabel": "Lista blanca", + "title": "Lista blanca de IP", + "description": "Las IP y CIDR de la lista blanca nunca son bloqueadas por CrowdSec, aunque generen alertas.", + "ipLabel": "IP o CIDR", + "reasonLabel": "Motivo", + "reasonPlaceholder": "Motivo opcional", + "addMyIp": "Añadir mi IP", + "none": "No hay entradas en la lista blanca", + "columnIp": "IP / CIDR", + "columnReason": "Motivo", + "columnAdded": "Añadida", + "deleteAriaLabel": "Eliminar entrada de la lista blanca de", + "delete": "Eliminar", + "deleteModal": { + "title": "Eliminar entrada de la lista blanca", + "body": "¿Eliminar {{ip}} de la lista blanca? CrowdSec podrá bloquear esta IP si genera alertas.", + "submit": "Eliminar" + } + }, + "noConfig": "No se encontró la configuración de CrowdSec en el estado de seguridad", + "noteText": "CrowdSec se controla mediante el interruptor de la página" }, "smtp": { "title": "Configuración de Correo (SMTP)", diff --git a/frontend/src/locales/fr/translation.json b/frontend/src/locales/fr/translation.json index cf003bd2a..7d0790a6f 100644 --- a/frontend/src/locales/fr/translation.json +++ b/frontend/src/locales/fr/translation.json @@ -384,8 +384,17 @@ "24h": "24h", "7d": "7d", "30d": "30d" - } - } + }, + "timeRangeLabel": "Plage de temps" + }, + "bouncerApiKey": "Clé API du bouncer", + "keyCopied": "Clé API copiée dans le presse-papiers", + "keyStoredAt": "Clé stockée dans", + "noKeyConfigured": "Aucune clé de bouncer configurée. Activez CrowdSec pour l'enregistrement automatique.", + "notRegistered": "Non enregistré", + "registered": "Enregistré", + "sourceEnvVar": "Depuis une variable d'environnement", + "sourceFile": "Depuis un fichier" }, "acl": { "title": "Contrôle d'Accès", @@ -1169,7 +1178,7 @@ "goToSecurity": "Aller au tableau de bord de sécurité", "enrollmentToken": "Jeton d'inscription", "tokenPlaceholder": "Collez votre jeton d'inscription de la Console CrowdSec", - "agentName": "Nom de l'agent (optionnel)", + "agentName": "Nom de l'agent", "agentPlaceholder": "mon-instance-charon", "tenant": "Locataire/Organisation (optionnel)", "tenantPlaceholder": "id-org-de-console", @@ -1188,7 +1197,15 @@ "tenantLabel": "Locataire", "lastAttempt": "Dernière tentative", "enrolledAt": "Inscrit", - "correlationId": "ID de corrélation" + "correlationId": "ID de corrélation", + "lastError": "Dernière erreur", + "ackText": "Je comprends que cela renouvellera mes identifiants LAPI si je suis déjà inscrit", + "description": "Connectez votre LAPI CrowdSec à la console CrowdSec pour une gestion centralisée et des alertes.", + "enrollToken": "Jeton d'inscription", + "lastHeartbeat": "Dernier signal de vie", + "privacyNote": "Remarque : l'inscription envoie des signaux de vie et des métriques à CrowdSec. Consultez la politique de confidentialité de CrowdSec pour plus de détails.", + "tenantHelper": "Facultatif : indiquez l'ID du locataire/de l'organisation si vous utilisez une configuration multi-locataire", + "tokenHelper": "Obtenez votre jeton d'inscription depuis la console CrowdSec" }, "reenroll": { "title": "Options de réinscription", @@ -1204,14 +1221,16 @@ "reenrolling": "Réinscription...", "clearState": "Effacer l'état d'inscription", "clearing": "Effacement...", - "clearConfirm": "Cela effacera l'état d'inscription local. Vous devrez peut-être vous réinscrire. Continuer ?" + "clearConfirm": "Cela effacera l'état d'inscription local. Vous devrez peut-être vous réinscrire. Continuer ?", + "clearFailed": "Impossible d'effacer l'état d'inscription : {{message}}" }, "packages": { "title": "Paquets de configuration", "description": "Exportez ou importez des paquets de configuration CrowdSec (tar.gz/zip). Une sauvegarde est créée avant l'importation.", "export": "Exporter", "import": "Importer", - "importing": "Importation..." + "importing": "Importation...", + "selectFile": "Sélectionner le fichier de paquet CrowdSec" }, "presets": { "title": "Préréglages", @@ -1237,18 +1256,22 @@ "previewUnavailable": "Aperçu non disponible", "applied": "Appliqué", "backup": "Sauvegarde", + "backupCreated": "Sauvegarde créée : {{name}} ({{location}})", + "backupLocation": "stockée à côté du répertoire de données CrowdSec", "reload": "Recharger", "required": "Requis", "method": "Méthode", "filesystem": "Système de fichiers", "loadCached": "Charger l'aperçu en cache", "applyDisabled": "Application désactivée tant que le hub est hors ligne.", - "noPresets": "Aucun préréglage disponible. Assurez-vous que Cerberus est activé." + "noPresets": "Aucun préréglage disponible. Assurez-vous que Cerberus est activé.", + "sortBy": "Trier les préréglages" }, "files": { "title": "Modifier les fichiers de configuration", "selectFile": "Sélectionner un fichier...", - "refresh": "Actualiser" + "refresh": "Actualiser", + "content": "Contenu du fichier" }, "bannedIps": { "title": "IPs bannies", @@ -1283,7 +1306,38 @@ "title": "Confirmer le débannissement", "confirm": "Êtes-vous sûr de vouloir débannir", "submit": "Débannir" - } + }, + "lapiInitializing": "CrowdSec est en cours d'exécution, mais l'API locale est encore en cours d'initialisation.", + "lapiInitializingDesc": "L'inscription à la console sera disponible dès que l'API locale sera prête. Cela peut prendre jusqu'à une minute après le démarrage de CrowdSec.", + "checkingAgain": "(nouvelle vérification...)", + "notRunning": "CrowdSec n'est pas en cours d'exécution", + "notRunningDesc": "Démarrez CrowdSec avant de vous inscrire à la console.", + "starting": "Démarrage de CrowdSec...", + "startFailed": "Échec du démarrage de CrowdSec", + "startCrowdsec": "Démarrer CrowdSec", + "goToSecurity": "Aller à Sécurité", + "whitelist": { + "tabLabel": "Liste blanche", + "title": "Liste blanche d'IP", + "description": "Les IP et CIDR de la liste blanche ne sont jamais bloqués par CrowdSec, même s'ils déclenchent des alertes.", + "ipLabel": "IP ou CIDR", + "reasonLabel": "Raison", + "reasonPlaceholder": "Raison facultative", + "addMyIp": "Ajouter mon IP", + "none": "Aucune entrée dans la liste blanche", + "columnIp": "IP / CIDR", + "columnReason": "Raison", + "columnAdded": "Ajoutée", + "deleteAriaLabel": "Supprimer l'entrée de la liste blanche pour", + "delete": "Supprimer", + "deleteModal": { + "title": "Supprimer l'entrée de la liste blanche", + "body": "Supprimer {{ip}} de la liste blanche ? CrowdSec pourra alors bloquer cette IP si elle déclenche des alertes.", + "submit": "Supprimer" + } + }, + "noConfig": "Configuration de CrowdSec introuvable dans l'état de sécurité", + "noteText": "CrowdSec est contrôlé via l'interrupteur de la page" }, "smtp": { "title": "Paramètres Email (SMTP)", diff --git a/frontend/src/locales/zh/translation.json b/frontend/src/locales/zh/translation.json index 874691e4a..ee38da191 100644 --- a/frontend/src/locales/zh/translation.json +++ b/frontend/src/locales/zh/translation.json @@ -384,8 +384,17 @@ "24h": "24小时", "7d": "7天", "30d": "30天" - } - } + }, + "timeRangeLabel": "时间范围" + }, + "bouncerApiKey": "Bouncer API 密钥", + "keyCopied": "API 密钥已复制到剪贴板", + "keyStoredAt": "密钥存储位置", + "noKeyConfigured": "未配置 Bouncer 密钥。启用 CrowdSec 以自动注册。", + "notRegistered": "未注册", + "registered": "已注册", + "sourceEnvVar": "来自环境变量", + "sourceFile": "来自文件" }, "acl": { "title": "访问控制", @@ -1168,7 +1177,7 @@ "goToSecurity": "前往安全仪表板", "enrollmentToken": "注册令牌", "tokenPlaceholder": "粘贴您从 CrowdSec 控制台获取的注册令牌", - "agentName": "代理名称(可选)", + "agentName": "代理名称", "agentPlaceholder": "我的-charon-实例", "tenant": "租户/组织(可选)", "tenantPlaceholder": "控制台中的组织ID", @@ -1187,7 +1196,15 @@ "tenantLabel": "租户", "lastAttempt": "最后尝试", "enrolledAt": "注册于", - "correlationId": "关联ID" + "correlationId": "关联ID", + "lastError": "上次错误", + "ackText": "我了解如果已注册,这将轮换我的 LAPI 凭据", + "description": "将您的 CrowdSec LAPI 连接到 CrowdSec 控制台,以进行集中管理和接收警报。", + "enrollToken": "注册令牌", + "lastHeartbeat": "上次心跳", + "privacyNote": "注意:注册会向 CrowdSec 发送心跳数据和指标。详情请参阅 CrowdSec 的隐私政策。", + "tenantHelper": "可选:如果使用多租户设置,请指定租户/组织 ID", + "tokenHelper": "从 CrowdSec 控制台获取您的注册令牌" }, "reenroll": { "title": "重新注册选项", @@ -1203,14 +1220,16 @@ "reenrolling": "正在重新注册...", "clearState": "清除注册状态", "clearing": "正在清除...", - "clearConfirm": "这将清除本地注册状态。您可能需要重新注册。是否继续?" + "clearConfirm": "这将清除本地注册状态。您可能需要重新注册。是否继续?", + "clearFailed": "清除注册状态失败:{{message}}" }, "packages": { "title": "配置包", "description": "导出或导入 CrowdSec 配置包(tar.gz/zip)。导入前会创建备份。", "export": "导出", "import": "导入", - "importing": "正在导入..." + "importing": "正在导入...", + "selectFile": "选择 CrowdSec 软件包文件" }, "presets": { "title": "预设", @@ -1236,18 +1255,22 @@ "previewUnavailable": "预览不可用", "applied": "已应用", "backup": "备份", + "backupCreated": "备份已创建:{{name}}({{location}})", + "backupLocation": "存放在 CrowdSec 数据目录旁", "reload": "重新加载", "required": "必需", "method": "方法", "filesystem": "文件系统", "loadCached": "加载缓存的预览", "applyDisabled": "Hub 离线时禁用应用。", - "noPresets": "没有可用的预设。请确保 Cerberus 已启用。" + "noPresets": "没有可用的预设。请确保 Cerberus 已启用。", + "sortBy": "排序预设" }, "files": { "title": "编辑配置文件", "selectFile": "选择文件...", - "refresh": "刷新" + "refresh": "刷新", + "content": "文件内容" }, "bannedIps": { "title": "已封禁 IP", @@ -1282,7 +1305,38 @@ "title": "确认解封", "confirm": "您确定要解封", "submit": "解封" - } + }, + "lapiInitializing": "CrowdSec 正在运行,但本地 API 仍在初始化。", + "lapiInitializingDesc": "本地 API 就绪后即可进行控制台注册。CrowdSec 启动后可能需要最多一分钟。", + "checkingAgain": "(正在重新检查...)", + "notRunning": "CrowdSec 未运行", + "notRunningDesc": "请先启动 CrowdSec,再注册到控制台。", + "starting": "正在启动 CrowdSec...", + "startFailed": "启动 CrowdSec 失败", + "startCrowdsec": "启动 CrowdSec", + "goToSecurity": "前往安全页面", + "whitelist": { + "tabLabel": "白名单", + "title": "IP 白名单", + "description": "白名单中的 IP 和 CIDR 即使触发警报也不会被 CrowdSec 拦截。", + "ipLabel": "IP 或 CIDR", + "reasonLabel": "原因", + "reasonPlaceholder": "可选原因", + "addMyIp": "添加我的 IP", + "none": "没有白名单条目", + "columnIp": "IP / CIDR", + "columnReason": "原因", + "columnAdded": "添加时间", + "deleteAriaLabel": "移除白名单条目:", + "delete": "删除", + "deleteModal": { + "title": "移除白名单条目", + "body": "要将 {{ip}} 从白名单中移除吗?如果该 IP 触发警报,CrowdSec 之后可能会拦截它。", + "submit": "移除" + } + }, + "noConfig": "在安全状态中未找到 CrowdSec 配置", + "noteText": "CrowdSec 由以下页面上的开关控制:" }, "smtp": { "title": "电子邮件 (SMTP) 设置", diff --git a/frontend/src/pages/CrowdSecConfig.tsx b/frontend/src/pages/CrowdSecConfig.tsx index cc6b017a1..95958737f 100644 --- a/frontend/src/pages/CrowdSecConfig.tsx +++ b/frontend/src/pages/CrowdSecConfig.tsx @@ -1,7 +1,7 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import { isAxiosError } from 'axios' import { Shield, ShieldOff, Trash2, Search, AlertTriangle, ExternalLink } from 'lucide-react' -import { lazy, Suspense, useEffect, useMemo, useState } from 'react' +import { lazy, Suspense, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { useNavigate, Link } from 'react-router' @@ -22,6 +22,7 @@ import { CROWDSEC_PRESETS, type CrowdsecPreset } from '../data/crowdsecPresets' import { useConsoleStatus, useEnrollConsole, useClearConsoleEnrollment } from '../hooks/useConsoleEnrollment' import { useWhitelistEntries, useAddWhitelist, useDeleteWhitelist } from '../hooks/useCrowdSecWhitelist' import { buildCrowdsecExportFilename, downloadCrowdsecExport, promptCrowdsecFilename } from '../utils/crowdsecExport' +import { sanitizeSecret } from '../utils/sanitizeSecret' import { toast } from '../utils/toast' /** Prefer the server's `error` message for Axios failures, falling back to the given text. */ @@ -57,6 +58,8 @@ export default function CrowdSecConfig() { const [validationError, setValidationError] = useState(null) const [applyInfo, setApplyInfo] = useState<{ status?: string; backup?: string; reloadHint?: boolean; usedCscli?: boolean; cacheKey?: string } | null>(null) const queryClient = useQueryClient() + const backupCreatedMessage = (name: string) => + t('crowdsecConfig.presets.backupCreated', { name, location: t('crowdsecConfig.presets.backupLocation') }) // Read the "CrowdSec is starting" signal broadcast by Security.tsx via the // QueryClient cache. No HTTP call is made; this is pure in-memory coordination. const { data: crowdsecStartingCache } = useQuery<{ isStarting: boolean; startedAt?: number }>({ @@ -80,8 +83,11 @@ export default function CrowdSecConfig() { const [enrollmentToken, setEnrollmentToken] = useState('') const [consoleTenant, setConsoleTenant] = useState('') const [consoleAgentName, setConsoleAgentName] = useState((typeof window !== 'undefined' && window.location?.hostname) || 'charon-agent') + // Tracks manual edits so late-arriving enrollment status never overwrites what the user typed + const consoleAgentNameEdited = useRef(false) + const consoleTenantEdited = useRef(false) const [consoleAck, setConsoleAck] = useState(false) - const [consoleErrors, setConsoleErrors] = useState<{ token?: string; agent?: string; tenant?: string; ack?: string; submit?: string }>({}) + const [consoleErrors, setConsoleErrors] = useState<{ token?: string; agent?: string; ack?: string; submit?: string }>({}) const consoleStatusQuery = useConsoleStatus(consoleEnrollmentEnabled) const enrollConsoleMutation = useEnrollConsole() const clearEnrollmentMutation = useClearConsoleEnrollment() @@ -216,15 +222,28 @@ export default function CrowdSecConfig() { } }, [presetCatalog, selectedPresetSlug]) + // Prefer the enrolled identity over the hostname default, unless the user already edited the field useEffect(() => { - if (consoleStatusQuery.data?.agent_name) { - setConsoleAgentName((prev) => prev || consoleStatusQuery.data?.agent_name || prev) + const enrolledAgent = consoleStatusQuery.data?.agent_name + if (enrolledAgent && !consoleAgentNameEdited.current) { + setConsoleAgentName(enrolledAgent) } - if (consoleStatusQuery.data?.tenant) { - setConsoleTenant((prev) => prev || consoleStatusQuery.data?.tenant || prev) + const enrolledTenant = consoleStatusQuery.data?.tenant + if (enrolledTenant && !consoleTenantEdited.current) { + setConsoleTenant(enrolledTenant) } }, [consoleStatusQuery.data?.agent_name, consoleStatusQuery.data?.tenant]) + const handleConsoleAgentNameChange = (value: string) => { + consoleAgentNameEdited.current = true + setConsoleAgentName(value) + } + + const handleConsoleTenantChange = (value: string) => { + consoleTenantEdited.current = true + setConsoleTenant(value) + } + const selectedPreset = presetCatalog.find((preset) => preset.slug === selectedPresetSlug) const selectedPresetRequiresHub = selectedPreset?.requiresHub ?? false @@ -295,8 +314,6 @@ export default function CrowdSecConfig() { const canRotateKey = normalizedConsoleStatus === 'enrolled' || normalizedConsoleStatus === 'degraded' || isConsolePendingAcceptance const consoleDocsHref = 'https://wikid82.github.io/charon/security/' - const sanitizeSecret = (msg: string) => msg.replace(/\b[A-Za-z0-9]{10,64}\b/g, '***') - const sanitizeErrorMessage = (err: unknown) => { if (isAxiosError(err)) { return sanitizeSecret(err.response?.data?.error || err.message) @@ -305,17 +322,14 @@ export default function CrowdSecConfig() { return 'Console enrollment failed' } - const validateConsoleEnrollment = (options?: { allowMissingTenant?: boolean; requireAck?: boolean }) => { - const nextErrors: { token?: string; agent?: string; tenant?: string; ack?: string } = {} + const validateConsoleEnrollment = (options?: { requireAck?: boolean }) => { + const nextErrors: { token?: string; agent?: string; ack?: string } = {} if (!enrollmentToken.trim()) { nextErrors.token = 'Enrollment token is required' } if (!consoleAgentName.trim()) { nextErrors.agent = 'Agent name is required' } - if (!consoleTenant.trim() && !options?.allowMissingTenant) { - nextErrors.tenant = 'Tenant / organization is required' - } if (options?.requireAck && !consoleAck) { nextErrors.ack = 'You must acknowledge the console data-sharing notice' } @@ -324,9 +338,8 @@ export default function CrowdSecConfig() { } const submitConsoleEnrollment = async (force = false) => { - const allowMissingTenant = force && !consoleTenant.trim() const requireAck = normalizedConsoleStatus === 'not_enrolled' - if (!validateConsoleEnrollment({ allowMissingTenant, requireAck })) return + if (!validateConsoleEnrollment({ requireAck })) return const tenantValue = consoleTenant.trim() || consoleStatusQuery.data?.tenant || consoleAgentName || 'charon-agent' try { await enrollConsoleMutation.mutateAsync({ @@ -471,7 +484,7 @@ export default function CrowdSecConfig() { const reloadNote = res.reload_hint ? ' (reload required)' : '' toast.success(`Preset applied via backend${reloadNote}`) if (res.backup) { - setPresetStatusMessage(`Backup stored at ${res.backup}`) + setPresetStatusMessage(backupCreatedMessage(res.backup)) } } catch (err) { if (!isAxiosError(err)) { @@ -481,7 +494,7 @@ export default function CrowdSecConfig() { // The server is authoritative for every preset source: surface its message for any // non-2xx response and never write preset content client-side. const status = err.response?.status - const backupPath = (err.response?.data as { backup?: string } | undefined)?.backup + const backupName = (err.response?.data as { backup?: string } | undefined)?.backup const message = getServerErrorMessage(err, 'Failed to apply preset') if (status === 400) { setValidationError(message) @@ -489,8 +502,8 @@ export default function CrowdSecConfig() { if (status === 503) { setHubUnavailable(true) } - setApplyInfo({ status: 'failed', backup: backupPath || undefined, cacheKey: presetMeta?.cacheKey }) - toast.error(`Apply failed: ${message}${backupPath ? `. Backup created at ${backupPath}` : ''}`) + setApplyInfo({ status: 'failed', backup: backupName || undefined, cacheKey: presetMeta?.cacheKey }) + toast.error(`Apply failed: ${message}${backupName ? `. ${backupCreatedMessage(backupName)}` : ''}`) } finally { setIsApplyingPreset(false) } @@ -708,7 +721,7 @@ export default function CrowdSecConfig() { id="console-agent-name" label={t('crowdsecConfig.consoleEnrollment.agentName')} value={consoleAgentName} - onChange={(e) => setConsoleAgentName(e.target.value)} + onChange={(e) => handleConsoleAgentNameChange(e.target.value)} error={consoleErrors.agent} errorTestId="console-enroll-error" data-testid="console-agent-name" @@ -717,10 +730,8 @@ export default function CrowdSecConfig() { id="console-tenant" label={t('crowdsecConfig.consoleEnrollment.tenant')} value={consoleTenant} - onChange={(e) => setConsoleTenant(e.target.value)} + onChange={(e) => handleConsoleTenantChange(e.target.value)} helperText={t('crowdsecConfig.consoleEnrollment.tenantHelper')} - error={consoleErrors.tenant} - errorTestId="console-enroll-error" data-testid="console-tenant" />
@@ -863,7 +874,7 @@ export default function CrowdSecConfig() { id="reenroll-agent-name" type="text" value={consoleAgentName} - onChange={(e) => setConsoleAgentName(e.target.value)} + onChange={(e) => handleConsoleAgentNameChange(e.target.value)} placeholder={t('crowdsecConfig.reenroll.agentPlaceholder')} /> @@ -875,7 +886,7 @@ export default function CrowdSecConfig() { id="reenroll-tenant" type="text" value={consoleTenant} - onChange={(e) => setConsoleTenant(e.target.value)} + onChange={(e) => handleConsoleTenantChange(e.target.value)} placeholder={t('crowdsecConfig.reenroll.orgPlaceholder')} /> @@ -966,7 +977,7 @@ export default function CrowdSecConfig() { onChange={(e) => setFile(e.target.files?.[0] ?? null)} data-testid="import-file" accept=".tar.gz,.zip" - aria-label={t('crowdsecConfig.packages.selectFile') || "Select CrowdSec package"} + aria-label={t('crowdsecConfig.packages.selectFile')} /> @@ -992,7 +1003,7 @@ export default function CrowdSecConfig() {