From 9622f7e83e6dca425b95894df76be7b616169d6b Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 04:44:46 +0000 Subject: [PATCH 01/10] test(e2e): add fixme specs for the CrowdSec export and import changes --- .../zzz-security-ui/crowdsec-import.spec.ts | 8 ++ tests/security/crowdsec-import.spec.ts | 77 +++++++++++++++++++ tests/utils/archive-helpers.ts | 19 +++++ 3 files changed, 104 insertions(+) diff --git a/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts b/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts index 314482f27..ef5f8f104 100644 --- a/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts +++ b/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts @@ -154,6 +154,14 @@ test.describe('Import CrowdSec Configuration', () => { await expect(page.locator(SELECTORS.importButton)).toBeEnabled(); }); + test.fixme('commit 4: should offer only .tar.gz in the file picker', async ({ page, adminUser }) => { + await loginUser(page, adminUser); + await page.goto('/tasks/import/crowdsec'); + await waitForLoadingComplete(page); + + await expect(page.locator(SELECTORS.fileInput)).toHaveAttribute('accept', '.tar.gz'); + }); + test('should disable import button when no file selected', async ({ page, adminUser }) => { await loginUser(page, adminUser); await page.goto('/tasks/import/crowdsec'); diff --git a/tests/security/crowdsec-import.spec.ts b/tests/security/crowdsec-import.spec.ts index 4baedddcf..d6363e2ef 100644 --- a/tests/security/crowdsec-import.spec.ts +++ b/tests/security/crowdsec-import.spec.ts @@ -21,6 +21,7 @@ import { createTarGz, createZip, createZipBomb, + listTarGzEntries, } from '../utils/archive-helpers'; const ADMIN = '/api/v1/admin/crowdsec'; @@ -250,4 +251,80 @@ no proper structure`, expect(files).not.toContain('extra-from-bad-import.yaml'); }); }); + + // New behaviour, enabled by the commit named in each title. + test.describe('Export contents and import validation (pending)', () => { + test.fixme('commit 2: export omits engine data, hub cache, database files and account files', async ({ request }) => { + const response = await request.get(`${ADMIN}/export`, { timeout: 120_000 }); + expect(response.status()).toBe(200); + + const entries = await listTarGzEntries(await response.body()); + const files = entries.filter((name) => !name.endsWith('/')); + const baseNames = files.map((name) => name.split('/').pop()?.toLowerCase()); + + expect(files).toEqual(expect.arrayContaining([expect.stringMatching(/^(config\/)?config\.yaml$/)])); + expect(files.filter((name) => /^(\.\/)?(data|hub_cache)\//.test(name))).toEqual([]); + expect(files.filter((name) => /\.db(-wal|-shm)?$/.test(name))).toEqual([]); + expect(baseNames).not.toContain('bouncer_key'); + expect(baseNames).not.toContain('local_api_credentials.yaml'); + expect(baseNames).not.toContain('online_api_credentials.yaml'); + }); + + test.fixme('commit 2: import accepts an export laid out as config/config.yaml', async ({ request }, testInfo) => { + const archive = await createTarGz( + { 'config/config.yaml': VALID_CONFIG, 'config/acquis.yaml': ACQUIS }, + testInfo.outputPath('real-layout.tar.gz'), + ); + + const response = await upload(request, archive, 'real-layout.tar.gz'); + + expect(response.status()).toBe(200); + expect((await response.json()).status).toBe('imported'); + }); + + test.fixme('commit 3: import rejects a config.yaml that is not valid YAML', async ({ request }, testInfo) => { + const archive = await createTarGz( + { 'config.yaml': 'api:\n server: [unclosed\n bad indentation\n' }, + testInfo.outputPath('invalid-yaml-api.tar.gz'), + ); + + const response = await upload(request, archive, 'invalid-yaml-api.tar.gz'); + + expect(response.status()).toBe(422); + expect((await response.json()).error).toBe('config validation failed: config.yaml is not valid YAML'); + }); + + test.fixme('commit 3: import rejects a config.yaml with no CrowdSec sections', async ({ request }, testInfo) => { + const archive = await createTarGz( + { 'config.yaml': 'other_config:\n field: value\n' }, + testInfo.outputPath('no-sections.tar.gz'), + ); + + const response = await upload(request, archive, 'no-sections.tar.gz'); + + expect(response.status()).toBe(422); + expect((await response.json()).error).toBe( + 'config validation failed: config.yaml has no CrowdSec configuration sections', + ); + }); + + test.fixme('commit 3: import rejects a ZIP with the single archive-format message', async ({ request }, testInfo) => { + const archive = await createZip({}, testInfo.outputPath('config.zip')); + + const response = await upload(request, archive, 'config.zip', 'application/zip'); + + expect(response.status()).toBe(422); + expect((await response.json()).error).toBe('validation failed: only .tar.gz archives are supported'); + }); + + test.fixme('commit 3: import rejects a .txt upload with the single archive-format message', async ({ request }, testInfo) => { + const notAnArchive = testInfo.outputPath('notes.txt'); + await fs.writeFile(notAnArchive, 'plain text'); + + const response = await upload(request, notAnArchive, 'notes.txt', 'text/plain'); + + expect(response.status()).toBe(422); + expect((await response.json()).error).toBe('validation failed: only .tar.gz archives are supported'); + }); + }); }); diff --git a/tests/utils/archive-helpers.ts b/tests/utils/archive-helpers.ts index 5f224ac42..3d94a719c 100644 --- a/tests/utils/archive-helpers.ts +++ b/tests/utils/archive-helpers.ts @@ -246,3 +246,22 @@ export async function createRawTarGz(entries: RawTarEntry[], outputPath: string) await fs.writeFile(outputPath, gzipSync(Buffer.concat(blocks))); return outputPath; } + +/** + * List the entry names of a tar.gz archive held in memory, in archive order. + * Directory entries keep their trailing slash so callers can tell them apart. + */ +export async function listTarGzEntries(archive: Buffer): Promise { + const names: string[] = []; + await new Promise((resolve, reject) => { + const parser = new tar.Parser(); + parser.on('entry', (entry) => { + names.push(entry.type === 'Directory' && !entry.path.endsWith('/') ? `${entry.path}/` : entry.path); + entry.resume(); + }); + parser.on('end', resolve); + parser.on('error', reject); + parser.end(archive); + }); + return names; +} From 5ef90f4aedeb98231f9c70325b8bd0461e9a8dfe Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 05:05:04 +0000 Subject: [PATCH 02/10] fix: slim down the CrowdSec configuration export The export now contains configuration only, and importing an export from a real install works again. Imports keep the running server's own connection details and roll back without touching them. --- .../handlers/crowdsec_export_import_test.go | 374 ++++++++++++++++++ .../internal/api/handlers/crowdsec_files.go | 15 +- .../internal/api/handlers/crowdsec_handler.go | 82 ++-- backend/internal/crowdsec/backup.go | 18 +- backend/internal/crowdsec/console_enroll.go | 10 +- backend/internal/crowdsec/heartbeat_poller.go | 12 +- backend/internal/crowdsec/hub_sync.go | 2 +- backend/internal/crowdsec/secrets.go | 49 +++ backend/internal/crowdsec/secrets_test.go | 168 ++++++++ tests/security/crowdsec-diagnostics.spec.ts | 15 +- tests/security/crowdsec-import.spec.ts | 27 +- 11 files changed, 692 insertions(+), 80 deletions(-) create mode 100644 backend/internal/api/handlers/crowdsec_export_import_test.go create mode 100644 backend/internal/crowdsec/secrets.go create mode 100644 backend/internal/crowdsec/secrets_test.go diff --git a/backend/internal/api/handlers/crowdsec_export_import_test.go b/backend/internal/api/handlers/crowdsec_export_import_test.go new file mode 100644 index 000000000..95fcf92c2 --- /dev/null +++ b/backend/internal/api/handlers/crowdsec_export_import_test.go @@ -0,0 +1,374 @@ +package handlers + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "encoding/json" + "errors" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/Wikid82/charon/backend/internal/crowdsec" + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" +) + +// seedRealLayout builds the on-disk layout of a real install (config/ holds the configuration). +func seedRealLayout(t *testing.T, dir string) { + t.Helper() + writeTree(t, dir, map[string]string{ + "config/config.yaml": importableConfig, + "config/acquis.yaml": "filenames: [x]\n", + "config/local_api_credentials.yaml": "live-lapi", + "config/online_api_credentials.yaml": "live-capi", + "config/hub/index.json": "{}", + "config/notes.yaml.bak": "user copy", + "bouncer_key": "live-key", + "bouncer_key.bak": "near-miss", + "caddy.yaml": "misc", + "crowdsec.db": "live-db", + "data/crowdsec.db": "lapi-db", + "data/GeoLite2-City.mmdb": "mmdb", + "hub_cache/x/bundle.tgz": "archive", + "nested/data/keep.yaml": "nested", + "nested/crowdsec.db-wal": "nested-wal", + "config/collections/placeholder.yaml": "real file", + "config/sub/Local_API_Credentials.YAML": "mixed-case", + }) + require.NoError(t, os.Symlink("../hub/index.json", filepath.Join(dir, "config", "collections", "linked.yaml"))) +} + +func exportBytes(t *testing.T, r *gin.Engine) []byte { + t.Helper() + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/v1/admin/crowdsec/export", http.NoBody)) + require.Equal(t, http.StatusOK, w.Code) + return w.Body.Bytes() +} + +func archiveNames(t *testing.T, data []byte) []string { + t.Helper() + gr, err := gzip.NewReader(bytes.NewReader(data)) + require.NoError(t, err) + tr := tar.NewReader(gr) + var names []string + for { + hdr, nextErr := tr.Next() + if errors.Is(nextErr, io.EOF) { + break + } + require.NoError(t, nextErr) + names = append(names, filepath.ToSlash(hdr.Name)) + } + sort.Strings(names) + return names +} + +func importBytesRequest(t *testing.T, data []byte) *http.Request { + t.Helper() + buf := &bytes.Buffer{} + mw := multipart.NewWriter(buf) + fw, err := mw.CreateFormFile("file", "cfg.tar.gz") + require.NoError(t, err) + _, err = fw.Write(data) + require.NoError(t, err) + require.NoError(t, mw.Close()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/crowdsec/import", buf) + req.Header.Set("Content-Type", mw.FormDataContentType()) + return req +} + +func assertLiveSecretsIntact(t *testing.T, dir string) { + t.Helper() + require.Equal(t, "live-lapi", readTreeFile(t, filepath.Join(dir, "config", "local_api_credentials.yaml"))) + require.Equal(t, "live-capi", readTreeFile(t, filepath.Join(dir, "config", "online_api_credentials.yaml"))) + require.Equal(t, "live-key", readTreeFile(t, filepath.Join(dir, "bouncer_key"))) +} + +func TestExportConfigRealLayoutEntryList(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + + require.Equal(t, []string{ + "bouncer_key.bak", + "caddy.yaml", + "config/acquis.yaml", + "config/collections/placeholder.yaml", + "config/config.yaml", + "config/hub/index.json", + "config/notes.yaml.bak", + "nested/data/keep.yaml", + }, archiveNames(t, exportBytes(t, r))) +} + +func TestExportConfigSkippedSecretKeepsSiblings(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "a.yaml": "a", + "bouncer_key": "k", + "z.yaml": "z", + "config/aaa.yaml": "a", + "config/local_api_credentials.yaml": "s", + "config/zzz.yaml": "z", + "crowdsec.db": "db", + "zz.yaml": "zz", + }) + _, r := newImportRouter(t, dir) + + require.Equal(t, []string{"a.yaml", "config/aaa.yaml", "config/zzz.yaml", "z.yaml", "zz.yaml"}, + archiveNames(t, exportBytes(t, r))) +} + +func TestExportThenImportRealLayoutRoundTrip(t *testing.T) { + t.Parallel() + src := t.TempDir() + seedRealLayout(t, src) + _, srcRouter := newImportRouter(t, src) + exported := exportBytes(t, srcRouter) + + // A different server with its own account details, database and a hub link. + dst := t.TempDir() + seedRealLayout(t, dst) + writeTree(t, dst, map[string]string{ + "config/config.yaml": "api:\n server:\n listen_uri: 127.0.0.1:1\n", + "config/extra.yaml": "gone after import", + "config/local_api_credentials.yaml": "live-lapi", + "config/online_api_credentials.yaml": "live-capi", + }) + _, dstRouter := newImportRouter(t, dst) + + w := httptest.NewRecorder() + dstRouter.ServeHTTP(w, importBytesRequest(t, exported)) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + require.Equal(t, importableConfig, readTreeFile(t, filepath.Join(dst, "config", "config.yaml"))) + require.Equal(t, "filenames: [x]\n", readTreeFile(t, filepath.Join(dst, "config", "acquis.yaml"))) + require.NoFileExists(t, filepath.Join(dst, "config", "extra.yaml")) + assertLiveSecretsIntact(t, dst) + require.Equal(t, "live-db", readTreeFile(t, filepath.Join(dst, "crowdsec.db"))) + require.Equal(t, "lapi-db", readTreeFile(t, filepath.Join(dst, "data", "crowdsec.db"))) + require.Equal(t, "archive", readTreeFile(t, filepath.Join(dst, "hub_cache", "x", "bundle.tgz"))) + + // Known limitation: installed hub item links are not part of an export, and import removes them. + _, err := os.Lstat(filepath.Join(dst, "config", "collections", "linked.yaml")) + require.True(t, errors.Is(err, os.ErrNotExist), "hub item symlinks are not restored") +} + +func TestImportConfigLayouts(t *testing.T) { + t.Parallel() + cases := map[string]map[string]string{ + "config only": {"config/config.yaml": importableConfig}, + "root layout": {"config.yaml": importableConfig, "other.yaml": "o"}, + "real and extra": {"config/config.yaml": importableConfig, "config/acquis.yaml": "a"}, + } + for name, files := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, files)) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + assertLiveSecretsIntact(t, dir) + for rel, content := range files { + require.Equal(t, content, readTreeFile(t, filepath.Join(dir, rel))) + } + }) + } +} + +func TestImportConfigNeverTakesProtectedEntriesFromUpload(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + + // A legacy export carried these entries; they must not replace this server's values. + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{ + "config/config.yaml": importableConfig, + "config/local_api_credentials.yaml": "uploaded", + "config/online_api_credentials.yaml": "uploaded", + "bouncer_key": "uploaded", + "config/Online_API_Credentials.yaml": "uploaded-mixed", + "config/bouncer_key/inner.yaml": "uploaded-dir", + "config/sibling.yaml": "sibling", + })) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + assertLiveSecretsIntact(t, dir) + require.Equal(t, "sibling", readTreeFile(t, filepath.Join(dir, "config", "sibling.yaml"))) + require.NoDirExists(t, filepath.Join(dir, "config", "bouncer_key")) +} + +func TestImportConfigKeepsSymlinkedSecret(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + store := filepath.Join(t.TempDir(), "docker-secret") + require.NoError(t, os.WriteFile(store, []byte("from-store"), 0o600)) + link := filepath.Join(dir, "config", "local_api_credentials.yaml") + require.NoError(t, os.Remove(link)) + require.NoError(t, os.Symlink(store, link)) + _, r := newImportRouter(t, dir) + + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{ + "config/config.yaml": importableConfig, + "config/local_api_credentials.yaml": "uploaded", + })) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + target, err := os.Readlink(link) + require.NoError(t, err) + require.Equal(t, store, target) + require.Equal(t, "from-store", readTreeFile(t, store)) +} + +func TestImportConfigFailureKeepsSecretsAndRestoresConfig(t *testing.T) { + t.Parallel() + t.Run("invalid config", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{ + "config/config.yaml": "{{ not: [valid yaml", + "config/local_api_credentials.yaml": "uploaded", + })) + require.Equal(t, http.StatusUnprocessableEntity, w.Code, w.Body.String()) + require.Equal(t, importableConfig, readTreeFile(t, filepath.Join(dir, "config", "config.yaml"))) + require.Equal(t, "filenames: [x]\n", readTreeFile(t, filepath.Join(dir, "config", "acquis.yaml"))) + assertLiveSecretsIntact(t, dir) + }) + t.Run("config only in a nested directory", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{"foo/config.yaml": importableConfig})) + require.Equal(t, http.StatusUnprocessableEntity, w.Code, w.Body.String()) + require.Contains(t, w.Body.String(), "config.yaml was not found at the top level or in config/") + require.Equal(t, importableConfig, readTreeFile(t, filepath.Join(dir, "config", "config.yaml"))) + assertLiveSecretsIntact(t, dir) + }) + t.Run("extraction failure", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + seedRealLayout(t, dir) + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{"config/config.yaml": importableConfig, "a": "file", "a/b": "nested"})) + require.Equal(t, http.StatusInternalServerError, w.Code, w.Body.String()) + assertLiveSecretsIntact(t, dir) + require.Equal(t, "filenames: [x]\n", readTreeFile(t, filepath.Join(dir, "config", "acquis.yaml"))) + }) +} + +// recordingCmdExec captures the arguments passed to the diagnostics commands. +type recordingCmdExec struct{ calls [][]string } + +func (r *recordingCmdExec) Execute(_ context.Context, _ string, args ...string) ([]byte, error) { + r.calls = append(r.calls, args) + return nil, nil +} + +func TestDiagnosticsConfigLocatesConfigFile(t *testing.T) { + t.Parallel() + run := func(t *testing.T, files map[string]string) (map[string]any, *recordingCmdExec) { + t.Helper() + dir := t.TempDir() + writeTree(t, dir, files) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", dir) + rec := &recordingCmdExec{} + h.CmdExec = rec + r := gin.New() + r.GET("/d", h.DiagnosticsConfig) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/d", http.NoBody)) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var out map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + return out, rec + } + + t.Run("config dir layout", func(t *testing.T) { + t.Parallel() + out, rec := run(t, map[string]string{"config/config.yaml": importableConfig}) + require.Equal(t, true, out["config_exists"]) + require.NotEmpty(t, rec.calls) + }) + t.Run("root layout", func(t *testing.T) { + t.Parallel() + out, _ := run(t, map[string]string{"config.yaml": importableConfig}) + require.Equal(t, true, out["config_exists"]) + }) + t.Run("no config reports not found without running checks", func(t *testing.T) { + t.Parallel() + out, rec := run(t, map[string]string{"config/acquis.yaml": "x"}) + require.Equal(t, false, out["config_exists"]) + require.Contains(t, out["errors"], "config.yaml not found") + require.Empty(t, rec.calls) + }) +} + +func TestDiagnosticsConnectivityPassesLocatedConfigToCscli(t *testing.T) { + t.Parallel() + for name, files := range map[string]map[string]string{ + "config dir": {"config/config.yaml": importableConfig, "config/online_api_credentials.yaml": "x"}, + "root": {"config.yaml": importableConfig, "config/online_api_credentials.yaml": "x"}, + "none": {"config/online_api_credentials.yaml": "x"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, files) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{started: true}, "/bin/false", dir) + rec := &recordingCmdExec{} + h.CmdExec = rec + r := gin.New() + r.GET("/c", h.DiagnosticsConnectivity) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/c", http.NoBody)) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + want := crowdsec.FindConfigFile(dir) + require.Len(t, rec.calls, 2, "lapi status and capi status") + for _, args := range rec.calls { + if want == "" { + require.NotContains(t, args, "-c") + } else { + require.Equal(t, []string{"-c", want}, args[:2]) + } + } + }) + } +} + +func TestEditorHidesSecretsUsingSharedRule(t *testing.T) { + t.Parallel() + for rel, readable := range map[string]bool{ + "bouncer_key": false, + "config/Local_API_Credentials.yaml": false, + "config/online_api_credentials.yaml": false, + "bouncer_key/inner.yaml": false, + "bouncer_key.bak": true, + "config/my_local_api_credentials.yaml": true, + "config/config.yaml": true, + } { + require.Equal(t, readable, isReadable(rel), rel) + } + require.False(t, isWritable("config/online_api_credentials.yaml")) +} diff --git a/backend/internal/api/handlers/crowdsec_files.go b/backend/internal/api/handlers/crowdsec_files.go index 3b0a08a27..10b9c8b7d 100644 --- a/backend/internal/api/handlers/crowdsec_files.go +++ b/backend/internal/api/handlers/crowdsec_files.go @@ -38,14 +38,6 @@ var ( writableExtensions = map[string]struct{}{ ".yaml": {}, ".yml": {}, ".json": {}, ".txt": {}, ".conf": {}, } - - // 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": {}, - } ) // fileError carries the HTTP status and client-facing message for a rejected file request. @@ -77,16 +69,13 @@ 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 (see hiddenSecretNames) are hidden, everything else stays visible. +// hub_cache trees) and secrets (see crowdsec.IsSecretPath) are hidden, everything else stays visible. func isReadable(rel string) bool { slash := filepath.ToSlash(rel) - if slash == "" || slash == "." || crowdsec.IsEngineOwnedPath(slash) { + if slash == "" || slash == "." || crowdsec.IsPreservedPath(slash) { return false } base := strings.ToLower(path.Base(slash)) - if _, hidden := hiddenSecretNames[base]; hidden { - return false - } if matched, _ := path.Match("*.db*", base); matched { return false } diff --git a/backend/internal/api/handlers/crowdsec_handler.go b/backend/internal/api/handlers/crowdsec_handler.go index af97eea72..0265348c3 100644 --- a/backend/internal/api/handlers/crowdsec_handler.go +++ b/backend/internal/api/handlers/crowdsec_handler.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "net/url" "os" @@ -334,6 +335,10 @@ func listArchiveContents(path, format string) ([]string, error) { } } +// errConfigFileNotFound is the fixed message for an import that contains no config.yaml in a +// location the engine reads (top level or config/). +var errConfigFileNotFound = errors.New("config.yaml was not found at the top level or in config/") + // validateYAMLFile validates CrowdSec YAML configuration structure. func validateYAMLFile(path string) error { // #nosec G304 -- path is validated upstream @@ -699,13 +704,13 @@ func (h *CrowdsecHandler) ImportConfig(c *gin.Context) { // rollback restores the snapshot in place; DataDir itself is never removed or renamed. rollback := func() { - if rbErr := crowdsec.Restore(backupDir, h.DataDir); rbErr != nil { + if rbErr := crowdsec.RestoreKeeping(backupDir, h.DataDir, crowdsec.IsPreservedPath); rbErr != nil { logger.Log().WithError(rbErr).WithField("backup_path", util.SanitizeForLog(backupDir)).Error("crowdsec import rollback failed; backup retained for manual recovery") } } - // Replace the configuration but keep engine-owned state (live db, data/, hub_cache/). - if err := crowdsec.ClearConfig(h.DataDir); err != nil { + // Replace the configuration but keep server-local state (engine-owned data and stored account details). + if err := crowdsec.ClearConfigKeeping(h.DataDir, crowdsec.IsPreservedPath); err != nil { rollback() c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create config dir"}) return @@ -721,8 +726,11 @@ func (h *CrowdsecHandler) ImportConfig(c *gin.Context) { } // Validate extracted config - configPath := filepath.Join(h.DataDir, "config.yaml") - if err := validateYAMLFile(configPath); err != nil { + validateErr := errConfigFileNotFound + if configPath := crowdsec.FindConfigFile(h.DataDir); configPath != "" { + validateErr = validateYAMLFile(configPath) + } + if err := validateErr; err != nil { rollback() c.JSON(http.StatusUnprocessableEntity, gin.H{"error": fmt.Sprintf("config validation failed: %v", crowdsec.RedactPaths(err.Error()))}) return @@ -770,8 +778,8 @@ func (h *CrowdsecHandler) extractArchive(archivePath, destDir string) error { if !strings.HasPrefix(target, filepath.Clean(destDir)+string(os.PathSeparator)) { return fmt.Errorf("invalid file path: %s", header.Name) } - // Engine-owned state (live db files, top-level data/ and hub_cache/) is never taken from an upload. - if crowdsec.IsEngineOwnedPath(strings.TrimPrefix(target, filepath.Clean(destDir)+string(os.PathSeparator))) { + // Server-local state (engine-owned data, stored account details) is never taken from an upload. + if crowdsec.IsPreservedPath(strings.TrimPrefix(target, filepath.Clean(destDir)+string(os.PathSeparator))) { continue } @@ -835,21 +843,33 @@ func (h *CrowdsecHandler) ExportConfig(c *gin.Context) { } }() - // Walk the DataDir and add files to the archive - err := filepath.Walk(h.DataDir, func(path string, info os.FileInfo, err error) error { + // Walk the DataDir and add files to the archive. Engine-owned state and stored account details are + // never exported; SkipDir is returned only for excluded directories so that skipping a file never + // drops its siblings. + err := filepath.WalkDir(h.DataDir, func(path string, d fs.DirEntry, err error) error { if err != nil { - logger.Log().WithError(err).Warnf("failed to access path %s during export walk", path) + logger.Log().WithError(err).Warnf("failed to access path %s during export walk", util.SanitizeForLog(path)) return nil // Skip files we cannot access } - if info.IsDir() { + rel, err := filepath.Rel(h.DataDir, path) + if err != nil { + return err + } + if rel != "." && crowdsec.IsPreservedPath(rel) { + if d.IsDir() { + return fs.SkipDir + } return nil } - // Never follow symlinks: filepath.Walk reports them via Lstat, so skip them here. - if info.Mode()&os.ModeSymlink != 0 { + if d.IsDir() { + return nil + } + // Never follow symlinks: WalkDir reports them via Lstat, so skip them here. + if d.Type()&fs.ModeSymlink != 0 { logger.Log().Warnf("skipping symlink %s during export", util.SanitizeForLog(path)) return nil } - rel, err := filepath.Rel(h.DataDir, path) + info, err := d.Info() if err != nil { return err } @@ -2060,15 +2080,8 @@ func (h *CrowdsecHandler) DiagnosticsConnectivity(c *gin.Context) { // Check 2: LAPI ready (responds to cscli lapi status) if running { args := []string{"lapi", "status"} - configPath := filepath.Join(h.DataDir, "config", "config.yaml") - if _, err := os.Stat(configPath); err == nil { + if configPath := crowdsec.FindConfigFile(h.DataDir); configPath != "" { args = append([]string{"-c", configPath}, args...) - } else { - // Fallback to root config - configPath = filepath.Join(h.DataDir, "config.yaml") - if _, err := os.Stat(configPath); err == nil { - args = append([]string{"-c", configPath}, args...) - } } checkCtx, cancel := context.WithTimeout(ctx, 5*time.Second) _, err := h.CmdExec.Execute(checkCtx, "cscli", args...) @@ -2087,8 +2100,7 @@ func (h *CrowdsecHandler) DiagnosticsConnectivity(c *gin.Context) { // Check 4: CAPI reachable (cscli capi status) if checks["capi_registered"].(bool) { args := []string{"capi", "status"} - configPath := filepath.Join(h.DataDir, "config", "config.yaml") - if _, err := os.Stat(configPath); err == nil { + if configPath := crowdsec.FindConfigFile(h.DataDir); configPath != "" { args = append([]string{"-c", configPath}, args...) } checkCtx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -2151,20 +2163,17 @@ func (h *CrowdsecHandler) DiagnosticsConfig(c *gin.Context) { validationErrs := []string{} // Check config.yaml - try config subdirectory first, then root - configPath := filepath.Join(h.DataDir, "config", "config.yaml") - if _, err := os.Stat(configPath); os.IsNotExist(err) { - configPath = filepath.Join(h.DataDir, "config.yaml") - } - - // Path traversal protection: ensure path is within DataDir - cleanConfigPath := filepath.Clean(configPath) - cleanDataDir := filepath.Clean(h.DataDir) - if !strings.HasPrefix(cleanConfigPath, cleanDataDir) { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid config path"}) - return + cleanConfigPath := "" + if configPath := crowdsec.FindConfigFile(h.DataDir); configPath != "" { + // Path traversal protection: ensure path is within DataDir + cleanConfigPath = filepath.Clean(configPath) + if !strings.HasPrefix(cleanConfigPath, filepath.Clean(h.DataDir)) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid config path"}) + return + } } - if _, err := os.Stat(cleanConfigPath); err == nil { + if cleanConfigPath != "" { validation["config_exists"] = true // Read config and check LAPI port @@ -2201,6 +2210,7 @@ func (h *CrowdsecHandler) DiagnosticsConfig(c *gin.Context) { acquisPath = filepath.Join(h.DataDir, "acquis.yaml") } + cleanDataDir := filepath.Clean(h.DataDir) // Path traversal protection cleanAcquisPath := filepath.Clean(acquisPath) if !strings.HasPrefix(cleanAcquisPath, cleanDataDir) { diff --git a/backend/internal/crowdsec/backup.go b/backend/internal/crowdsec/backup.go index 842007b74..ef40b5d76 100644 --- a/backend/internal/crowdsec/backup.go +++ b/backend/internal/crowdsec/backup.go @@ -97,10 +97,16 @@ func (s *HubService) restore(path string) error { return Restore(path, s.DataDir // Restore replaces the contents of dataDir with the snapshot while keeping dataDir itself (and the // engine-owned entries inside it) in place. func Restore(snapshotPath, dataDir string) error { - if err := ClearConfig(dataDir); err != nil { + return RestoreKeeping(snapshotPath, dataDir, IsEngineOwnedPath) +} + +// RestoreKeeping is Restore for callers that preserve more than engine-owned state: entries for which +// keep(rel) returns true are neither removed from dataDir nor overwritten from the snapshot. +func RestoreKeeping(snapshotPath, dataDir string, keep func(rel string) bool) error { + if err := ClearConfigKeeping(dataDir, keep); err != nil { return err } - if err := copyTree(snapshotPath, dataDir, IsEngineOwnedPath); err != nil { + if err := copyTree(snapshotPath, dataDir, keep); err != nil { return fmt.Errorf("restore backup: %w", err) } return nil @@ -109,10 +115,16 @@ func Restore(snapshotPath, dataDir string) error { // ClearConfig creates dataDir when missing and removes everything in it except engine-owned state // (live db files, top-level data/ and hub_cache/). dataDir itself is never removed or renamed. func ClearConfig(dataDir string) error { + return ClearConfigKeeping(dataDir, IsEngineOwnedPath) +} + +// ClearConfigKeeping is ClearConfig with a caller-supplied keep predicate. keep is evaluated before +// the entry type is inspected, so a kept name survives whether it is a file, directory or symlink. +func ClearConfigKeeping(dataDir string, keep func(rel string) bool) error { if err := os.MkdirAll(dataDir, 0o700); err != nil { return fmt.Errorf("mkdir data dir: %w", err) } - if err := emptyDirExcept(dataDir, IsEngineOwnedPath); err != nil { + if err := emptyDirExcept(dataDir, keep); err != nil { return fmt.Errorf("empty data dir: %w", err) } return nil diff --git a/backend/internal/crowdsec/console_enroll.go b/backend/internal/crowdsec/console_enroll.go index 36da0a24b..cd1a11405 100644 --- a/backend/internal/crowdsec/console_enroll.go +++ b/backend/internal/crowdsec/console_enroll.go @@ -315,15 +315,7 @@ func (s *ConsoleEnrollmentService) ensureCAPIRegistered(ctx context.Context) err // config subdirectory first (standard layout), then dataDir root. // Returns empty string if no config file is found. func (s *ConsoleEnrollmentService) findConfigPath() string { - configPath := filepath.Join(s.dataDir, "config", "config.yaml") - if _, err := os.Stat(configPath); err == nil { - return configPath - } - configPath = filepath.Join(s.dataDir, "config.yaml") - if _, err := os.Stat(configPath); err == nil { - return configPath - } - return "" + return FindConfigFile(s.dataDir) } func (s *ConsoleEnrollmentService) load(ctx context.Context) (*models.CrowdsecConsoleEnrollment, error) { diff --git a/backend/internal/crowdsec/heartbeat_poller.go b/backend/internal/crowdsec/heartbeat_poller.go index 02372ab99..575eae9b0 100644 --- a/backend/internal/crowdsec/heartbeat_poller.go +++ b/backend/internal/crowdsec/heartbeat_poller.go @@ -2,8 +2,6 @@ package crowdsec import ( "context" - "os" - "path/filepath" "strings" "sync" "sync/atomic" @@ -218,13 +216,5 @@ func (p *HeartbeatPoller) isEnrolledOutput(output string) bool { // findConfigPath returns the path to the CrowdSec config file. func (p *HeartbeatPoller) findConfigPath() string { - configPath := filepath.Join(p.dataDir, "config", "config.yaml") - if _, err := os.Stat(configPath); err == nil { - return configPath - } - configPath = filepath.Join(p.dataDir, "config.yaml") - if _, err := os.Stat(configPath); err == nil { - return configPath - } - return "" + return FindConfigFile(p.dataDir) } diff --git a/backend/internal/crowdsec/hub_sync.go b/backend/internal/crowdsec/hub_sync.go index 32bc58c2f..94162a302 100644 --- a/backend/internal/crowdsec/hub_sync.go +++ b/backend/internal/crowdsec/hub_sync.go @@ -935,7 +935,7 @@ func (s *HubService) extractTarGz(ctx context.Context, archive []byte, targetDir return fmt.Errorf("unsafe path %s", hdr.Name) } cleanName := filepath.Clean(hdr.Name) - if IsEngineOwnedPath(cleanName) { + if IsEngineOwnedPath(cleanName) || IsSecretPath(cleanName) { continue } destPath := filepath.Join(targetDir, cleanName) diff --git a/backend/internal/crowdsec/secrets.go b/backend/internal/crowdsec/secrets.go new file mode 100644 index 000000000..1c4ef5102 --- /dev/null +++ b/backend/internal/crowdsec/secrets.go @@ -0,0 +1,49 @@ +package crowdsec + +import ( + "os" + "path/filepath" + "strings" +) + +// secretNames are the lowercase file names of stored account and connection details. They belong to +// the live server: the editor hides them, exports omit them and imports never replace them. +var secretNames = map[string]struct{}{ + "bouncer_key": {}, + "local_api_credentials.yaml": {}, + "online_api_credentials.yaml": {}, +} + +// IsSecretPath reports whether rel (a slash- or OS-separated path relative to DataDir) names a stored +// secret. Matching is case-insensitive and exact (near-miss names such as "bouncer_key.bak" are not +// secrets). Any path component may match, so entries beneath a directory named like a secret are +// covered as well. +func IsSecretPath(rel string) bool { + slash := strings.ToLower(filepath.ToSlash(filepath.Clean(rel))) + for _, part := range strings.Split(slash, "/") { + if _, ok := secretNames[part]; ok { + return true + } + } + return false +} + +// IsPreservedPath reports whether rel is server-local state that imports must neither remove, replace +// nor take from an upload: engine-owned state and stored secrets. +func IsPreservedPath(rel string) bool { + return IsEngineOwnedPath(rel) || IsSecretPath(rel) +} + +// FindConfigFile returns the CrowdSec config file below dataDir: config/config.yaml when it exists, +// else config.yaml at the root, else "". +func FindConfigFile(dataDir string) string { + for _, p := range []string{ + filepath.Join(dataDir, "config", "config.yaml"), + filepath.Join(dataDir, "config.yaml"), + } { + if _, err := os.Stat(p); err == nil { + return p + } + } + return "" +} diff --git a/backend/internal/crowdsec/secrets_test.go b/backend/internal/crowdsec/secrets_test.go new file mode 100644 index 000000000..929e370b9 --- /dev/null +++ b/backend/internal/crowdsec/secrets_test.go @@ -0,0 +1,168 @@ +package crowdsec + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIsSecretPath(t *testing.T) { + t.Parallel() + cases := map[string]bool{ + "bouncer_key": true, + "config/local_api_credentials.yaml": true, + "config/online_api_credentials.yaml": true, + "a/b/c/bouncer_key": true, + "BOUNCER_KEY": true, + "config/Local_API_Credentials.YAML": true, + "bouncer_key/x": true, + "./config/online_api_credentials.yaml": true, + "config//local_api_credentials.yaml": true, + `config\local_api_credentials.yaml`: filepath.Separator == '\\', + "bouncer_key.bak": false, + "my_local_api_credentials.yaml": false, + "local_api_credentials.yaml.bak": false, + "config/online_api_credentials.yml": false, + "config/config.yaml": false, + "notifications/bouncer_key_notes.yaml": false, + "": false, + ".": false, + } + for rel, want := range cases { + require.Equal(t, want, IsSecretPath(rel), rel) + } +} + +func TestIsPreservedPath(t *testing.T) { + t.Parallel() + require.True(t, IsPreservedPath("data/x")) + require.True(t, IsPreservedPath("crowdsec.db")) + require.True(t, IsPreservedPath("bouncer_key")) + require.False(t, IsPreservedPath("config/config.yaml")) +} + +func TestFindConfigFile(t *testing.T) { + t.Parallel() + t.Run("config dir preferred", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config", "config.yaml"), "a") + writeFile(t, filepath.Join(dir, "config.yaml"), "b") + require.Equal(t, filepath.Join(dir, "config", "config.yaml"), FindConfigFile(dir)) + }) + t.Run("root fallback", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.yaml"), "b") + require.Equal(t, filepath.Join(dir, "config.yaml"), FindConfigFile(dir)) + }) + t.Run("none", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config", "acquis.yaml"), "x") + require.Empty(t, FindConfigFile(dir)) + require.Empty(t, FindConfigFile(filepath.Join(dir, "missing"))) + }) +} + +func seedSecretTree(t *testing.T) string { + t.Helper() + dir := filepath.Join(t.TempDir(), "crowdsec") + writeFile(t, filepath.Join(dir, "config", "config.yaml"), "api: {}\n") + writeFile(t, filepath.Join(dir, "config", "local_api_credentials.yaml"), "live-lapi") + writeFile(t, filepath.Join(dir, "config", "online_api_credentials.yaml"), "live-capi") + writeFile(t, filepath.Join(dir, "bouncer_key"), "live-key") + writeFile(t, filepath.Join(dir, "other.yaml"), "o") + return dir +} + +func TestClearConfigKeepingPreservesSecrets(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + require.NoError(t, ClearConfigKeeping(dir, IsPreservedPath)) + require.Equal(t, "live-lapi", readFile(t, filepath.Join(dir, "config", "local_api_credentials.yaml"))) + require.Equal(t, "live-capi", readFile(t, filepath.Join(dir, "config", "online_api_credentials.yaml"))) + require.Equal(t, "live-key", readFile(t, filepath.Join(dir, "bouncer_key"))) + require.NoFileExists(t, filepath.Join(dir, "config", "config.yaml")) + require.NoFileExists(t, filepath.Join(dir, "other.yaml")) +} + +func TestClearConfigKeepingKeepsSymlinkedSecret(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + target := filepath.Join(t.TempDir(), "docker-secret") + writeFile(t, target, "from-secret-store") + link := filepath.Join(dir, "config", "local_api_credentials.yaml") + require.NoError(t, os.Remove(link)) + require.NoError(t, os.Symlink(target, link)) + + require.NoError(t, ClearConfigKeeping(dir, IsPreservedPath)) + got, err := os.Readlink(link) + require.NoError(t, err) + require.Equal(t, target, got) + require.Equal(t, "from-secret-store", readFile(t, target)) +} + +func TestClearConfigKeepingPlainClearConfigStillRemovesSecrets(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + require.NoError(t, ClearConfig(dir)) + require.NoFileExists(t, filepath.Join(dir, "bouncer_key")) +} + +func TestRestoreKeepingLeavesLiveSecretsAlone(t *testing.T) { + t.Parallel() + t.Run("secret changed since snapshot keeps the newer value", func(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + snap, err := Snapshot(dir) + require.NoError(t, err) + writeFile(t, filepath.Join(dir, "bouncer_key"), "newer-key") + writeFile(t, filepath.Join(dir, "config", "config.yaml"), "uploaded: true\n") + writeFile(t, filepath.Join(dir, "extra.yaml"), "x") + + require.NoError(t, RestoreKeeping(snap, dir, IsPreservedPath)) + require.Equal(t, "newer-key", readFile(t, filepath.Join(dir, "bouncer_key"))) + require.Equal(t, "live-lapi", readFile(t, filepath.Join(dir, "config", "local_api_credentials.yaml"))) + require.Equal(t, "api: {}\n", readFile(t, filepath.Join(dir, "config", "config.yaml"))) + require.NoFileExists(t, filepath.Join(dir, "extra.yaml")) + }) + t.Run("restore copy failure does not delete live secrets", func(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + err := RestoreKeeping(filepath.Join(t.TempDir(), "missing-snapshot"), dir, IsPreservedPath) + require.ErrorContains(t, err, "restore backup") + require.Equal(t, "live-key", readFile(t, filepath.Join(dir, "bouncer_key"))) + require.Equal(t, "live-capi", readFile(t, filepath.Join(dir, "config", "online_api_credentials.yaml"))) + }) + t.Run("plain Restore still restores the snapshot copy", func(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + snap, err := Snapshot(dir) + require.NoError(t, err) + writeFile(t, filepath.Join(dir, "bouncer_key"), "changed") + require.NoError(t, Restore(snap, dir)) + require.Equal(t, "live-key", readFile(t, filepath.Join(dir, "bouncer_key"))) + }) +} + +func TestExtractTarGzSkipsProtectedNamesAndKeepsSiblings(t *testing.T) { + t.Parallel() + dir := seedSecretTree(t) + svc := NewHubService(nil, nil, dir) + archive := makeTarGz(t, map[string]string{ + "config/new.yaml": "new", + "config/local_api_credentials.yaml": "overlay", + "config/online_api_credentials.yaml": "overlay", + "bouncer_key": "overlay", + "bouncer_key/x": "overlay", + }) + + require.NoError(t, svc.extractTarGz(t.Context(), archive, dir)) + require.Equal(t, "new", readFile(t, filepath.Join(dir, "config", "new.yaml"))) + require.Equal(t, "live-lapi", readFile(t, filepath.Join(dir, "config", "local_api_credentials.yaml"))) + require.Equal(t, "live-capi", readFile(t, filepath.Join(dir, "config", "online_api_credentials.yaml"))) + require.Equal(t, "live-key", readFile(t, filepath.Join(dir, "bouncer_key"))) +} diff --git a/tests/security/crowdsec-diagnostics.spec.ts b/tests/security/crowdsec-diagnostics.spec.ts index 82e02d9dd..d51029fb6 100644 --- a/tests/security/crowdsec-diagnostics.spec.ts +++ b/tests/security/crowdsec-diagnostics.spec.ts @@ -148,14 +148,19 @@ test.describe('CrowdSec Diagnostics', () => { expect(connectivity.lapi_ready).toBe(status.lapi_ready); }); - test('should report CAPI as not registered without the online credentials file', async ({ request }) => { + // Import treats the account files as server-local state: it never deletes the live + // online_api_credentials.yaml and never takes one from an archive, so the CAPI registration the + // diagnostics report is the same before and after any import. + test('should still report CAPI as registered after importing an archive without the online credentials file', async ({ request }) => { + // The E2E image ships online_api_credentials.yaml, and the beforeEach import carries none. const connectivity = await (await request.get(`${ADMIN}/diagnostics/connectivity`)).json(); - expect(connectivity.capi_registered).toBe(false); - expect(connectivity.capi_reachable).toBe(false); + expect(connectivity.capi_registered).toBe(true); }); - test('should report CAPI as registered once the online credentials file exists', async ({ request }, testInfo) => { + test('should keep the CAPI registration unchanged when the imported archive carries an online credentials file', async ({ request }, testInfo) => { + const registeredBefore = (await (await request.get(`${ADMIN}/diagnostics/connectivity`)).json()).capi_registered; + await importKnownConfig(request, testInfo.outputPath('with-capi.tar.gz'), { 'config.yaml': CONFIG_YAML, 'acquis.yaml': ACQUIS_YAML, @@ -164,7 +169,7 @@ test.describe('CrowdSec Diagnostics', () => { const connectivity = await (await request.get(`${ADMIN}/diagnostics/connectivity`)).json(); - expect(connectivity.capi_registered).toBe(true); + expect(connectivity.capi_registered).toBe(registeredBefore); }); }); diff --git a/tests/security/crowdsec-import.spec.ts b/tests/security/crowdsec-import.spec.ts index d6363e2ef..2218d6a1a 100644 --- a/tests/security/crowdsec-import.spec.ts +++ b/tests/security/crowdsec-import.spec.ts @@ -254,7 +254,7 @@ no proper structure`, // New behaviour, enabled by the commit named in each title. test.describe('Export contents and import validation (pending)', () => { - test.fixme('commit 2: export omits engine data, hub cache, database files and account files', async ({ request }) => { + test('commit 2: export omits engine data, hub cache, database files and account files', async ({ request }) => { const response = await request.get(`${ADMIN}/export`, { timeout: 120_000 }); expect(response.status()).toBe(200); @@ -270,7 +270,7 @@ no proper structure`, expect(baseNames).not.toContain('online_api_credentials.yaml'); }); - test.fixme('commit 2: import accepts an export laid out as config/config.yaml', async ({ request }, testInfo) => { + test('commit 2: import accepts an export laid out as config/config.yaml', async ({ request }, testInfo) => { const archive = await createTarGz( { 'config/config.yaml': VALID_CONFIG, 'config/acquis.yaml': ACQUIS }, testInfo.outputPath('real-layout.tar.gz'), @@ -282,6 +282,29 @@ no proper structure`, expect((await response.json()).status).toBe('imported'); }); + test('commit 2: an export from this instance re-imports and keeps the CAPI registration', async ({ request }) => { + // The connectivity diagnostics report registration from the presence of online_api_credentials.yaml, + // an account file that must survive an import (it is never part of an export). + const readRegistration = async () => { + const response = await request.get(`${ADMIN}/diagnostics/connectivity`, { timeout: 60_000 }); + expect(response.status()).toBe(200); + return (await response.json()).capi_registered as boolean; + }; + const registeredBefore = await readRegistration(); + + const exported = await request.get(`${ADMIN}/export`, { timeout: 120_000 }); + expect(exported.status()).toBe(200); + + const response = await request.post(`${ADMIN}/import`, { + multipart: { file: { name: 'roundtrip.tar.gz', mimeType: 'application/gzip', buffer: await exported.body() } }, + timeout: 120_000, + }); + + expect(response.status()).toBe(200); + expect((await response.json()).status).toBe('imported'); + expect(await readRegistration()).toBe(registeredBefore); + }); + test.fixme('commit 3: import rejects a config.yaml that is not valid YAML', async ({ request }, testInfo) => { const archive = await createTarGz( { 'config.yaml': 'api:\n server: [unclosed\n bad indentation\n' }, From 0858c32114a65de511f1f80cb8d8a97eb812ec7f Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 05:25:51 +0000 Subject: [PATCH 03/10] fix: validate CrowdSec configuration files by parsing them Imported configuration is now parsed as YAML instead of being matched by text, and any upload that is not a .tar.gz archive gets one clear message. --- .../api/handlers/crowdsec_archive_test.go | 110 +++------ .../crowdsec_archive_validation_test.go | 4 +- .../crowdsec_config_validation_test.go | 137 ++++++++++++ .../internal/api/handlers/crowdsec_files.go | 44 +++- .../internal/api/handlers/crowdsec_handler.go | 210 ++++++++++-------- .../testdata/crowdsec_default_config.yaml | 62 ++++++ tests/security/crowdsec-import.spec.ts | 95 ++++---- 7 files changed, 415 insertions(+), 247 deletions(-) create mode 100644 backend/internal/api/handlers/crowdsec_config_validation_test.go create mode 100644 backend/internal/api/handlers/testdata/crowdsec_default_config.yaml diff --git a/backend/internal/api/handlers/crowdsec_archive_test.go b/backend/internal/api/handlers/crowdsec_archive_test.go index 1b1090703..b19bbb56c 100644 --- a/backend/internal/api/handlers/crowdsec_archive_test.go +++ b/backend/internal/api/handlers/crowdsec_archive_test.go @@ -5,85 +5,37 @@ import ( "compress/gzip" "os" "path/filepath" - "strings" "testing" ) -// TestDetectArchiveFormat tests the detectArchiveFormat helper function. -func TestDetectArchiveFormat(t *testing.T) { +// TestRequireTarGz tests the requireTarGz helper function. +func TestRequireTarGz(t *testing.T) { tests := []struct { - name string - path string - wantFormat string - wantErr bool - errContains string + name string + path string + wantErr bool }{ - { - name: "tar.gz extension", - path: "/path/to/archive.tar.gz", - wantFormat: "tar.gz", - wantErr: false, - }, - { - name: "TAR.GZ uppercase", - path: "/path/to/ARCHIVE.TAR.GZ", - wantFormat: "tar.gz", - wantErr: false, - }, - { - name: "zip extension", - path: "/path/to/archive.zip", - wantFormat: "zip", - wantErr: false, - }, - { - name: "ZIP uppercase", - path: "/path/to/ARCHIVE.ZIP", - wantFormat: "zip", - wantErr: false, - }, - { - name: "unsupported extension", - path: "/path/to/archive.rar", - wantFormat: "", - wantErr: true, - errContains: "unsupported format", - }, - { - name: "no extension", - path: "/path/to/archive", - wantFormat: "", - wantErr: true, - errContains: "unsupported format", - }, - { - name: "txt extension", - path: "/path/to/archive.txt", - wantFormat: "", - wantErr: true, - errContains: "unsupported format", - }, + {"tar.gz extension", "/path/to/archive.tar.gz", false}, + {"TAR.GZ uppercase", "/path/to/ARCHIVE.TAR.GZ", false}, + {"zip extension", "/path/to/archive.zip", true}, + {"ZIP uppercase", "/path/to/ARCHIVE.ZIP", true}, + {"tgz extension", "/path/to/archive.tgz", true}, + {"rar extension", "/path/to/archive.rar", true}, + {"no extension", "/path/to/archive", true}, + {"txt extension", "/path/to/archive.txt", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - format, err := detectArchiveFormat(tt.path) - if tt.wantErr { - if err == nil { - t.Errorf("detectArchiveFormat() expected error, got nil") - return - } - if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) { - t.Errorf("detectArchiveFormat() error = %v, want error containing %q", err, tt.errContains) + err := requireTarGz(tt.path) + if !tt.wantErr { + if err != nil { + t.Errorf("requireTarGz() unexpected error = %v", err) } return } - if err != nil { - t.Errorf("detectArchiveFormat() unexpected error = %v", err) - return - } - if format != tt.wantFormat { - t.Errorf("detectArchiveFormat() = %q, want %q", format, tt.wantFormat) + if err == nil || err.Error() != "only .tar.gz archives are supported" { + t.Errorf("requireTarGz() error = %v, want the fixed archive format message", err) } }) } @@ -149,7 +101,7 @@ func TestCalculateUncompressedSize(t *testing.T) { // Test calculateUncompressedSize expectedSize := int64(len(testContent) + len(content2)) - size, err := calculateUncompressedSize(archivePath, "tar.gz") + size, err := calculateUncompressedSize(archivePath) if err != nil { t.Errorf("calculateUncompressedSize() unexpected error = %v", err) return @@ -158,14 +110,8 @@ func TestCalculateUncompressedSize(t *testing.T) { t.Errorf("calculateUncompressedSize() = %d, want %d", size, expectedSize) } - // Test with unsupported format - _, err = calculateUncompressedSize(archivePath, "unsupported") - if err == nil { - t.Error("calculateUncompressedSize() expected error for unsupported format") - } - // Test with non-existent file - _, err = calculateUncompressedSize("/nonexistent/path.tar.gz", "tar.gz") + _, err = calculateUncompressedSize("/nonexistent/path.tar.gz") if err == nil { t.Error("calculateUncompressedSize() expected error for non-existent file") } @@ -225,7 +171,7 @@ func TestListArchiveContents(t *testing.T) { } // Test listArchiveContents - contents, err := listArchiveContents(archivePath, "tar.gz") + contents, err := listArchiveContents(archivePath) if err != nil { t.Errorf("listArchiveContents() unexpected error = %v", err) return @@ -253,14 +199,8 @@ func TestListArchiveContents(t *testing.T) { t.Errorf("listArchiveContents() returned %d files, want %d", len(contents), len(expectedFiles)) } - // Test with unsupported format - _, err = listArchiveContents(archivePath, "unsupported") - if err == nil { - t.Error("listArchiveContents() expected error for unsupported format") - } - // Test with non-existent file - _, err = listArchiveContents("/nonexistent/path.tar.gz", "tar.gz") + _, err = listArchiveContents("/nonexistent/path.tar.gz") if err == nil { t.Error("listArchiveContents() expected error for non-existent file") } @@ -320,8 +260,8 @@ func TestConfigArchiveValidator_Validate(t *testing.T) { t.Fatalf("Failed to create dummy file: %v", writeErr) } err = validator.Validate(unsupportedPath) - if err == nil { - t.Error("Validate() expected error for unsupported format") + if err == nil || err.Error() != "only .tar.gz archives are supported" { + t.Errorf("Validate() error = %v, want the fixed archive format message", err) } } diff --git a/backend/internal/api/handlers/crowdsec_archive_validation_test.go b/backend/internal/api/handlers/crowdsec_archive_validation_test.go index 74902bdba..b19edeff3 100644 --- a/backend/internal/api/handlers/crowdsec_archive_validation_test.go +++ b/backend/internal/api/handlers/crowdsec_archive_validation_test.go @@ -117,13 +117,13 @@ func TestConfigArchiveValidator_InvalidFormats(t *testing.T) { name: "txt file", filename: "test.txt", content: "not an archive", - wantErr: "unsupported format", + wantErr: "only .tar.gz archives are supported", }, { name: "rar file", filename: "test.rar", content: "Rar!\x1a\x07\x00", - wantErr: "unsupported format", + wantErr: "only .tar.gz archives are supported", }, } diff --git a/backend/internal/api/handlers/crowdsec_config_validation_test.go b/backend/internal/api/handlers/crowdsec_config_validation_test.go new file mode 100644 index 000000000..b687229c0 --- /dev/null +++ b/backend/internal/api/handlers/crowdsec_config_validation_test.go @@ -0,0 +1,137 @@ +package handlers + +import ( + "bytes" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func writeConfigFile(t *testing.T, content string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(p, []byte(content), 0o600)) + return p +} + +func TestValidateYAMLFile(t *testing.T) { + tests := []struct { + name string + content string + wantErr string // empty means valid + }{ + {"empty file", "", "config.yaml is empty"}, + {"comment only", "# nothing here\n", "config.yaml is empty"}, + {"list root", "- api\n- common\n", "config.yaml must be a YAML mapping"}, + {"scalar root", "api\n", "config.yaml must be a YAML mapping"}, + {"duplicate key", "api:\n a: 1\napi:\n b: 2\n", "config.yaml is not valid YAML"}, + {"non-string key", "1: x\napi: {}\n", "config.yaml is not valid YAML"}, + {"broken yaml containing api text", "api:\n server: [unclosed\n", "config.yaml is not valid YAML"}, + {"unknown keys only", "foo: bar\nserver: {}\n", "config.yaml has no CrowdSec configuration sections"}, + {"disable flag only", "disable_agent: true\n", "config.yaml has no CrowdSec configuration sections"}, + {"minimal api", "api:\n server:\n listen_uri: 0.0.0.0:8080\n", ""}, + {"common only", "common:\n log_level: info\n", ""}, + {"first document only", "api: {}\n---\n- list\n", ""}, + {"first document invalid", "foo: bar\n---\napi: {}\n", "config.yaml has no CrowdSec configuration sections"}, + {"later document unparsed", "api: {}\n---\nbad: [unclosed\n", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateYAMLFile(writeConfigFile(t, tt.content)) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.EqualError(t, err, tt.wantErr) + }) + } +} + +func TestValidateYAMLFileRejectsOversize(t *testing.T) { + content := "api: {}\n# " + strings.Repeat("x", maxCrowdsecFileBytes) + "\n" + require.EqualError(t, validateYAMLFile(writeConfigFile(t, content)), "config.yaml is too large") +} + +func TestValidateYAMLFileMissingFile(t *testing.T) { + err := validateYAMLFile(filepath.Join(t.TempDir(), "missing.yaml")) + require.ErrorContains(t, err, "failed to read file") +} + +func TestValidateYAMLFileDefaultConfigFixture(t *testing.T) { + // testdata/crowdsec_default_config.yaml is the stock config.yaml shipped in the Charon image + // (/etc/crowdsec.dist/config.yaml, captured from the E2E container). + require.NoError(t, validateYAMLFile(filepath.Join("testdata", "crowdsec_default_config.yaml"))) +} + +func TestValidateYAMLFileImportableFixture(t *testing.T) { + require.NoError(t, validateYAMLFile(writeConfigFile(t, importableConfig))) +} + +func TestValidateYAMLFileErrorsHideParserDetail(t *testing.T) { + err := validateYAMLFile(writeConfigFile(t, "api:\n server: [unclosed\n")) + require.Error(t, err) + require.NotContains(t, err.Error(), "line") + require.NotContains(t, err.Error(), "yaml:") +} + +func TestParseYAMLDocumentsEmptyStream(t *testing.T) { + _, err := parseYAMLDocuments(strings.NewReader(""), false) + require.ErrorIs(t, err, io.EOF) +} + +func importRequestNamed(t *testing.T, filename string, data []byte) *http.Request { + t.Helper() + buf := &bytes.Buffer{} + mw := multipart.NewWriter(buf) + fw, err := mw.CreateFormFile("file", filename) + require.NoError(t, err) + _, err = fw.Write(data) + require.NoError(t, err) + require.NoError(t, mw.Close()) + req := httptest.NewRequest(http.MethodPost, "/api/v1/admin/crowdsec/import", buf) + req.Header.Set("Content-Type", mw.FormDataContentType()) + return req +} + +func TestImportConfigRejectsNonTarGzUploads(t *testing.T) { + t.Parallel() + for _, name := range []string{"cfg.zip", "cfg.ZIP", "cfg.tgz", "cfg.txt", "cfg"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequestNamed(t, name, []byte("not an archive"))) + require.Equal(t, http.StatusUnprocessableEntity, w.Code, w.Body.String()) + require.JSONEq(t, `{"error":"validation failed: only .tar.gz archives are supported"}`, w.Body.String()) + }) + } +} + +func TestImportConfigConfigYAMLValidationMessages(t *testing.T) { + t.Parallel() + cases := map[string]struct{ content, want string }{ + "empty": {"# only a comment\n", "config validation failed: config.yaml is empty"}, + "list root": {"- api\n", "config validation failed: config.yaml must be a YAML mapping"}, + "no keys": {"foo: bar\n", "config validation failed: config.yaml has no CrowdSec configuration sections"}, + "invalid": {"api: [unclosed\n", "config validation failed: config.yaml is not valid YAML"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, r := newImportRouter(t, dir) + w := httptest.NewRecorder() + r.ServeHTTP(w, importRequest(t, map[string]string{"config.yaml": tc.content})) + require.Equal(t, http.StatusUnprocessableEntity, w.Code, w.Body.String()) + require.JSONEq(t, `{"error":"`+tc.want+`"}`, w.Body.String()) + }) + } +} diff --git a/backend/internal/api/handlers/crowdsec_files.go b/backend/internal/api/handlers/crowdsec_files.go index 10b9c8b7d..0c03e9cee 100644 --- a/backend/internal/api/handlers/crowdsec_files.go +++ b/backend/internal/api/handlers/crowdsec_files.go @@ -195,20 +195,44 @@ func checkSize(content string) *fileError { return nil } +// parseYAMLDocuments decodes the documents of a YAML stream, each first into a yaml.Node and then +// into a generic value so duplicate mapping keys and alias abuse are rejected. It returns the first +// document's node, or io.EOF when the stream has no document. With firstOnly set, later documents are +// never read; otherwise every document must parse. +func parseYAMLDocuments(r io.Reader, firstOnly bool) (*yaml.Node, error) { + dec := yaml.NewDecoder(r) + var first *yaml.Node + for { + var node yaml.Node + err := dec.Decode(&node) + if errors.Is(err, io.EOF) { + if first == nil { + return nil, io.EOF + } + return first, nil + } + if err != nil { + return nil, fmt.Errorf("decode yaml document: %w", err) + } + var doc any + if err := node.Decode(&doc); err != nil { + return nil, fmt.Errorf("decode yaml value: %w", err) + } + if first == nil { + first = &node + } + if firstOnly { + return first, nil + } + } +} + // checkYAML verifies that YAML files parse; other allowed types are not inspected. func checkYAML(rel, content string) *fileError { switch strings.ToLower(filepath.Ext(rel)) { case ".yaml", ".yml": - dec := yaml.NewDecoder(strings.NewReader(content)) - for { - var doc any - err := dec.Decode(&doc) - if errors.Is(err, io.EOF) { - return nil - } - if err != nil { - return badRequest("invalid YAML") - } + if _, err := parseYAMLDocuments(strings.NewReader(content), false); err != nil && !errors.Is(err, io.EOF) { + return badRequest("invalid YAML") } } return nil diff --git a/backend/internal/api/handlers/crowdsec_handler.go b/backend/internal/api/handlers/crowdsec_handler.go index 0265348c3..4b47d7cc4 100644 --- a/backend/internal/api/handlers/crowdsec_handler.go +++ b/backend/internal/api/handlers/crowdsec_handler.go @@ -2,6 +2,7 @@ package handlers import ( "archive/tar" + "bytes" "compress/gzip" "context" "encoding/json" @@ -15,6 +16,7 @@ import ( "os/exec" "path/filepath" "regexp" + "slices" "strconv" "strings" "sync" @@ -31,6 +33,7 @@ import ( "github.com/Wikid82/charon/backend/internal/util" "github.com/gin-gonic/gin" + "gopkg.in/yaml.v3" "gorm.io/gorm" ) @@ -193,14 +196,13 @@ func (v *ConfigArchiveValidator) Validate(path string) error { return fmt.Errorf("archive exceeds maximum size: %d > %d", info.Size(), v.MaxSize) } - // Detect format - format, err := detectArchiveFormat(path) - if err != nil { - return err + // Only tar.gz is supported + if formatErr := requireTarGz(path); formatErr != nil { + return formatErr } // Calculate uncompressed size and check for zip bombs - uncompressedSize, err := calculateUncompressedSize(path, format) + uncompressedSize, err := calculateUncompressedSize(path) if err != nil { return err } @@ -216,7 +218,7 @@ func (v *ConfigArchiveValidator) Validate(path string) error { } // List contents and verify required files - contents, err := listArchiveContents(path, format) + contents, err := listArchiveContents(path) if err != nil { return err } @@ -237,127 +239,149 @@ func (v *ConfigArchiveValidator) Validate(path string) error { return nil } -// detectArchiveFormat detects the archive format (tar.gz or zip). -func detectArchiveFormat(path string) (string, error) { - ext := strings.ToLower(filepath.Ext(path)) +// errArchiveFormat is the fixed message for any upload that is not a .tar.gz archive. +var errArchiveFormat = errors.New("only .tar.gz archives are supported") - if strings.HasSuffix(strings.ToLower(path), ".tar.gz") { - return "tar.gz", nil +// requireTarGz rejects every path that does not end in .tar.gz (case-insensitive). +func requireTarGz(path string) error { + if !strings.HasSuffix(strings.ToLower(path), ".tar.gz") { + return errArchiveFormat } + return nil +} - if ext == ".zip" { - return "zip", nil +// calculateUncompressedSize calculates the total uncompressed size of the tar.gz archive. +func calculateUncompressedSize(path string) (int64, error) { + // #nosec G304 -- path is validated upstream + f, err := os.Open(path) + if err != nil { + return 0, fmt.Errorf("failed to open archive: %w", err) } + defer func() { _ = f.Close() }() - return "", fmt.Errorf("unsupported format: %s", ext) -} + gr, err := gzip.NewReader(f) + if err != nil { + return 0, fmt.Errorf("failed to create gzip reader: %w", err) + } + defer func() { _ = gr.Close() }() -// calculateUncompressedSize calculates the total uncompressed size of the archive. -func calculateUncompressedSize(path, format string) (int64, error) { - switch format { - case "tar.gz": - // #nosec G304 -- path is validated upstream - f, err := os.Open(path) - if err != nil { - return 0, fmt.Errorf("failed to open archive: %w", err) - } - defer func() { _ = f.Close() }() + tr := tar.NewReader(gr) + var total int64 - gr, err := gzip.NewReader(f) + for { + header, err := tr.Next() + if err == io.EOF { + break + } if err != nil { - return 0, fmt.Errorf("failed to create gzip reader: %w", err) + return 0, fmt.Errorf("failed to read tar header: %w", err) } - defer func() { _ = gr.Close() }() - - tr := tar.NewReader(gr) - var total int64 - - for { - header, err := tr.Next() - if err == io.EOF { - break - } - if err != nil { - return 0, fmt.Errorf("failed to read tar header: %w", err) - } - // Only count regular files - if header.Typeflag == tar.TypeReg { - total += header.Size - } + // Only count regular files + if header.Typeflag == tar.TypeReg { + total += header.Size } + } - return total, nil + return total, nil +} - default: - return 0, fmt.Errorf("unsupported format for size calculation: %s", format) +// listArchiveContents lists all regular files in the tar.gz archive. +func listArchiveContents(path string) ([]string, error) { + // #nosec G304 -- path is validated upstream + f, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("failed to open archive: %w", err) } -} + defer func() { _ = f.Close() }() -// listArchiveContents lists all files in the archive. -func listArchiveContents(path, format string) ([]string, error) { - switch format { - case "tar.gz": - // #nosec G304 -- path is validated upstream - f, err := os.Open(path) - if err != nil { - return nil, fmt.Errorf("failed to open archive: %w", err) - } - defer func() { _ = f.Close() }() + gr, err := gzip.NewReader(f) + if err != nil { + return nil, fmt.Errorf("failed to create gzip reader: %w", err) + } + defer func() { _ = gr.Close() }() - gr, err := gzip.NewReader(f) + tr := tar.NewReader(gr) + var files []string + + for { + header, err := tr.Next() + if err == io.EOF { + break + } if err != nil { - return nil, fmt.Errorf("failed to create gzip reader: %w", err) + return nil, fmt.Errorf("failed to read tar header: %w", err) } - defer func() { _ = gr.Close() }() - - tr := tar.NewReader(gr) - var files []string - for { - header, err := tr.Next() - if err == io.EOF { - break - } - if err != nil { - return nil, fmt.Errorf("failed to read tar header: %w", err) - } - - if header.Typeflag == tar.TypeReg { - files = append(files, header.Name) - } + if header.Typeflag == tar.TypeReg { + files = append(files, header.Name) } - - return files, nil - - default: - return nil, fmt.Errorf("unsupported format for listing: %s", format) } + + return files, nil } // errConfigFileNotFound is the fixed message for an import that contains no config.yaml in a // location the engine reads (top level or config/). var errConfigFileNotFound = errors.New("config.yaml was not found at the top level or in config/") -// validateYAMLFile validates CrowdSec YAML configuration structure. +// crowdsecConfigSections are the top-level keys of CrowdSec's csconfig.Config +// (pkg/csconfig/config.go). DisableAPI/DisableAgent are command-line flags, not file keys. +var crowdsecConfigSections = []string{ + "common", "prometheus", "crowdsec_service", "cscli", "db_config", "api", "config_paths", "plugin_config", +} + +// Fixed config.yaml validation messages. Parser detail is deliberately never surfaced. +var ( + errConfigTooLarge = errors.New("config.yaml is too large") + errConfigEmpty = errors.New("config.yaml is empty") + errConfigInvalid = errors.New("config.yaml is not valid YAML") + errConfigNotMap = errors.New("config.yaml must be a YAML mapping") + errConfigNoKeys = errors.New("config.yaml has no CrowdSec configuration sections") +) + +// validateYAMLFile checks that path is a size-bounded YAML mapping holding at least one known +// CrowdSec top-level section. Nested values are left to CrowdSec. Only the first document of a +// multi-document file is read, matching the engine. Duplicate mapping keys are rejected on purpose: +// this is stricter than the engine's own loader, which tolerates some duplicates. func validateYAMLFile(path string) error { - // #nosec G304 -- path is validated upstream - data, err := os.ReadFile(path) + data, err := readCapped(path) + if errors.Is(err, errFileTooLarge) { + return errConfigTooLarge + } if err != nil { return fmt.Errorf("failed to read file: %w", err) } - // Basic YAML syntax check - var config map[string]interface{} - if err := json.Unmarshal(data, &config); err != nil { - // Try basic structure validation - check for key CrowdSec fields - content := string(data) - if !strings.Contains(content, "api:") && !strings.Contains(content, "server:") { - return fmt.Errorf("invalid CrowdSec config structure") - } + root, err := parseYAMLDocuments(bytes.NewReader(data), true) + if errors.Is(err, io.EOF) { + return errConfigEmpty + } + if err != nil { + logger.Log().WithField("error", sanitizeForLog(err.Error())).Warn("crowdsec config.yaml failed YAML parsing") + return errConfigInvalid + } + if root.Kind != yaml.DocumentNode || len(root.Content) == 0 || root.Content[0].Kind != yaml.MappingNode { + return errConfigNotMap } - return nil + // yaml.v3 coerces scalar keys such as 1 or true into strings when decoding into a map, so key + // types are checked on the nodes. Only the top level is inspected; nested content is CrowdSec's. + found := false + mapping := root.Content[0] + for i := 0; i+1 < len(mapping.Content); i += 2 { + key := mapping.Content[i] + if key.Kind != yaml.ScalarNode || key.Tag != "!!str" { + return errConfigInvalid + } + if slices.Contains(crowdsecConfigSections, key.Value) { + found = true + } + } + if found { + return nil + } + return errConfigNoKeys } func mapCrowdsecStatus(err error, defaultCode int) int { diff --git a/backend/internal/api/handlers/testdata/crowdsec_default_config.yaml b/backend/internal/api/handlers/testdata/crowdsec_default_config.yaml new file mode 100644 index 000000000..a1baea5aa --- /dev/null +++ b/backend/internal/api/handlers/testdata/crowdsec_default_config.yaml @@ -0,0 +1,62 @@ +common: + daemonize: true + log_media: file + log_level: info + log_dir: /var/log/ + log_max_size: 20 + compress_logs: true + log_max_files: 10 +config_paths: + config_dir: /etc/crowdsec/ + data_dir: /var/lib/crowdsec/data/ + simulation_path: /etc/crowdsec/simulation.yaml + hub_dir: /etc/crowdsec/hub/ + index_path: /etc/crowdsec/hub/.index.json + notification_dir: /etc/crowdsec/notifications/ + plugin_dir: /usr/local/lib/crowdsec/plugins/ +crowdsec_service: + #console_context_path: /etc/crowdsec/console/context.yaml + acquisition_path: /etc/crowdsec/acquis.yaml + acquisition_dir: /etc/crowdsec/acquis.d + parser_routines: 1 +cscli: + output: human + color: auto +db_config: + log_level: info + type: sqlite + db_path: /var/lib/crowdsec/data/crowdsec.db + #max_open_conns: 100 + #user: + #password: + #db_name: + #host: + #port: + flush: + max_items: 5000 + max_age: 7d +plugin_config: + user: nobody # plugin process would be ran on behalf of this user + group: nogroup # plugin process would be ran on behalf of this group +api: + client: + insecure_skip_verify: false + credentials_path: /etc/crowdsec/local_api_credentials.yaml + server: + log_level: info + listen_uri: 127.0.0.1:8080 + profiles_path: /etc/crowdsec/profiles.yaml + console_path: /etc/crowdsec/console.yaml + online_client: # Central API credentials (to push signals and receive bad IPs) + credentials_path: /etc/crowdsec/online_api_credentials.yaml + trusted_ips: # IP ranges, or IPs which can have admin API access + - 127.0.0.1 + - ::1 +# tls: +# cert_file: /etc/crowdsec/ssl/cert.pem +# key_file: /etc/crowdsec/ssl/key.pem +prometheus: + enabled: true + level: full + listen_addr: 127.0.0.1 + listen_port: 6060 diff --git a/tests/security/crowdsec-import.spec.ts b/tests/security/crowdsec-import.spec.ts index 2218d6a1a..64d2295ba 100644 --- a/tests/security/crowdsec-import.spec.ts +++ b/tests/security/crowdsec-import.spec.ts @@ -124,35 +124,6 @@ test.describe('CrowdSec Config Import Validation', () => { expect((await response.json()).error).toBe('validation failed: required file missing: config.yaml'); }); - test('should reject a config.yaml that is not YAML and has no CrowdSec sections', async ({ request }, testInfo) => { - const archive = await createTarGz( - { - 'config.yaml': `invalid: yaml: syntax: here: - unclosed: [bracket - bad indentation -no proper structure`, - }, - testInfo.outputPath('invalid-yaml.tar.gz'), - ); - - const response = await upload(request, archive, 'invalid-yaml.tar.gz'); - - expect(response.status()).toBe(422); - expect((await response.json()).error).toBe('config validation failed: invalid CrowdSec config structure'); - }); - - test('should reject a config.yaml without the required CrowdSec fields', async ({ request }, testInfo) => { - const archive = await createTarGz( - { 'config.yaml': 'other_config:\n field: value\n nested:\n key: data\n' }, - testInfo.outputPath('missing-fields.tar.gz'), - ); - - const response = await upload(request, archive, 'missing-fields.tar.gz'); - - expect(response.status()).toBe(422); - expect((await response.json()).error).toBe('config validation failed: invalid CrowdSec config structure'); - }); - test('should reject an archive larger than 50 MB', async ({ request }, testInfo) => { const archive = await createOversizedArchive(testInfo.outputPath('oversized.tar.gz'), 51); expect((await fs.stat(archive)).size).toBeGreaterThan(MAX_ARCHIVE_BYTES); @@ -176,25 +147,6 @@ no proper structure`, ); }); - test('should reject a ZIP file because only tar.gz is supported', async ({ request }, testInfo) => { - const archive = await createZip({}, testInfo.outputPath('config.zip')); - - const response = await upload(request, archive, 'config.zip', 'application/zip'); - - expect(response.status()).toBe(422); - expect((await response.json()).error).toBe('validation failed: unsupported format for size calculation: zip'); - }); - - test('should reject a file that is not an archive at all', async ({ request }, testInfo) => { - const notAnArchive = testInfo.outputPath('notes.txt'); - await fs.writeFile(notAnArchive, 'plain text'); - - const response = await upload(request, notAnArchive, 'notes.txt', 'text/plain'); - - expect(response.status()).toBe(422); - expect((await response.json()).error).toBe('validation failed: unsupported format: .txt'); - }); - test('should reject a corrupted archive', async ({ request }, testInfo) => { const archive = await createCorruptedArchive(testInfo.outputPath('corrupted.tar.gz')); @@ -252,9 +204,8 @@ no proper structure`, }); }); - // New behaviour, enabled by the commit named in each title. - test.describe('Export contents and import validation (pending)', () => { - test('commit 2: export omits engine data, hub cache, database files and account files', async ({ request }) => { + test.describe('Export contents and import validation', () => { + test('export omits engine data, hub cache, database files and account files', async ({ request }) => { const response = await request.get(`${ADMIN}/export`, { timeout: 120_000 }); expect(response.status()).toBe(200); @@ -270,7 +221,7 @@ no proper structure`, expect(baseNames).not.toContain('online_api_credentials.yaml'); }); - test('commit 2: import accepts an export laid out as config/config.yaml', async ({ request }, testInfo) => { + test('import accepts an export laid out as config/config.yaml', async ({ request }, testInfo) => { const archive = await createTarGz( { 'config/config.yaml': VALID_CONFIG, 'config/acquis.yaml': ACQUIS }, testInfo.outputPath('real-layout.tar.gz'), @@ -282,7 +233,7 @@ no proper structure`, expect((await response.json()).status).toBe('imported'); }); - test('commit 2: an export from this instance re-imports and keeps the CAPI registration', async ({ request }) => { + test('an export from this instance re-imports and keeps the CAPI registration', async ({ request }) => { // The connectivity diagnostics report registration from the presence of online_api_credentials.yaml, // an account file that must survive an import (it is never part of an export). const readRegistration = async () => { @@ -305,7 +256,7 @@ no proper structure`, expect(await readRegistration()).toBe(registeredBefore); }); - test.fixme('commit 3: import rejects a config.yaml that is not valid YAML', async ({ request }, testInfo) => { + test('import rejects a config.yaml that is not valid YAML even when it contains api: text', async ({ request }, testInfo) => { const archive = await createTarGz( { 'config.yaml': 'api:\n server: [unclosed\n bad indentation\n' }, testInfo.outputPath('invalid-yaml-api.tar.gz'), @@ -317,7 +268,7 @@ no proper structure`, expect((await response.json()).error).toBe('config validation failed: config.yaml is not valid YAML'); }); - test.fixme('commit 3: import rejects a config.yaml with no CrowdSec sections', async ({ request }, testInfo) => { + test('import rejects a config.yaml with no CrowdSec sections', async ({ request }, testInfo) => { const archive = await createTarGz( { 'config.yaml': 'other_config:\n field: value\n' }, testInfo.outputPath('no-sections.tar.gz'), @@ -331,7 +282,37 @@ no proper structure`, ); }); - test.fixme('commit 3: import rejects a ZIP with the single archive-format message', async ({ request }, testInfo) => { + const configRejections: Array<{ title: string; content: string; message: string }> = [ + { title: 'is empty', content: '', message: 'config validation failed: config.yaml is empty' }, + { + title: 'has a list at the top level', + content: '- api\n- common\n', + message: 'config validation failed: config.yaml must be a YAML mapping', + }, + { + title: 'has a duplicate key', + content: 'api:\n server:\n log_level: info\napi:\n server:\n log_level: debug\n', + message: 'config validation failed: config.yaml is not valid YAML', + }, + { + title: 'has a non-string key', + content: '1: value\napi:\n server:\n log_level: info\n', + message: 'config validation failed: config.yaml is not valid YAML', + }, + ]; + for (const { title, content, message } of configRejections) { + test(`import rejects a config.yaml that ${title}`, async ({ request }, testInfo) => { + const archive = await createTarGz({ 'config.yaml': content }, testInfo.outputPath('rejected-config.tar.gz')); + + const response = await upload(request, archive, 'rejected-config.tar.gz'); + + expect(response.status()).toBe(422); + expect((await response.json()).error).toBe(message); + expect(await readConfigFile(request, 'config.yaml')).toBe(BASELINE_CONFIG); + }); + } + + test('import rejects a ZIP with the single archive-format message', async ({ request }, testInfo) => { const archive = await createZip({}, testInfo.outputPath('config.zip')); const response = await upload(request, archive, 'config.zip', 'application/zip'); @@ -340,7 +321,7 @@ no proper structure`, expect((await response.json()).error).toBe('validation failed: only .tar.gz archives are supported'); }); - test.fixme('commit 3: import rejects a .txt upload with the single archive-format message', async ({ request }, testInfo) => { + test('import rejects a .txt upload with the single archive-format message', async ({ request }, testInfo) => { const notAnArchive = testInfo.outputPath('notes.txt'); await fs.writeFile(notAnArchive, 'plain text'); From 270f85a8c9b126b0f2bb45d3d3ce56cbe79d1e5c Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 05:26:26 +0000 Subject: [PATCH 04/10] fix: offer only .tar.gz in the CrowdSec import and translate Check Now The import picker no longer suggests ZIP files, and the Check Now buttons use the translation files in every supported language. --- frontend/src/locales/de/translation.json | 5 +- frontend/src/locales/en/translation.json | 5 +- frontend/src/locales/es/translation.json | 5 +- frontend/src/locales/fr/translation.json | 5 +- frontend/src/locales/zh/translation.json | 5 +- frontend/src/pages/CrowdSecConfig.tsx | 6 +-- frontend/src/pages/ImportCrowdSec.tsx | 2 +- .../pages/__tests__/CrowdSecConfig.test.tsx | 7 +++ .../pages/__tests__/ImportCrowdSec.spec.tsx | 3 +- .../zzz-security-ui/crowdsec-import.spec.ts | 51 ++----------------- 10 files changed, 32 insertions(+), 62 deletions(-) diff --git a/frontend/src/locales/de/translation.json b/frontend/src/locales/de/translation.json index cb0c69668..f7af1c5e6 100644 --- a/frontend/src/locales/de/translation.json +++ b/frontend/src/locales/de/translation.json @@ -1069,7 +1069,7 @@ }, "importCrowdSec": { "title": "CrowdSec-Konfigurationspakete", - "description": "Laden Sie ein tar.gz- oder zip-Paket hoch. Vor dem Import wird eine Sicherung erstellt, damit Sie bei Bedarf zurückrollen können. Exportieren Sie das aktuelle Paket vom Cerberus-Dashboard oder der CrowdSec-Konfigurationsseite.", + "description": "Laden Sie ein tar.gz-Paket hoch. Vor dem Import wird eine Sicherung erstellt, damit Sie bei Bedarf zurückrollen können. Exportieren Sie das aktuelle Paket vom Cerberus-Dashboard oder der CrowdSec-Konfigurationsseite.", "import": "Importieren", "configImported": "CrowdSec-Konfiguration importiert", "importFailed": "Import fehlgeschlagen: {{error}}", @@ -1157,6 +1157,7 @@ }, "crowdsecConfig": { "title": "CrowdSec-Konfiguration", + "checkNow": "Jetzt prüfen", "loading": "Lade CrowdSec-Konfiguration...", "loadError": "Fehler beim Laden des CrowdSec-Status", "noStatus": "CrowdSec-Status nicht verfügbar", @@ -1224,7 +1225,7 @@ }, "packages": { "title": "Konfigurationspakete", - "description": "Exportieren oder importieren Sie CrowdSec-Konfigurationspakete (tar.gz/zip). Vor dem Import wird ein Backup erstellt.", + "description": "Exportieren oder importieren Sie CrowdSec-Konfigurationspakete (tar.gz). Vor dem Import wird ein Backup erstellt.", "export": "Exportieren", "import": "Importieren", "importing": "Importiere...", diff --git a/frontend/src/locales/en/translation.json b/frontend/src/locales/en/translation.json index 46c2de23c..930974029 100644 --- a/frontend/src/locales/en/translation.json +++ b/frontend/src/locales/en/translation.json @@ -1254,7 +1254,7 @@ }, "importCrowdSec": { "title": "CrowdSec Configuration Packages", - "description": "Upload a tar.gz or zip package. A backup is created before importing so you can roll back if needed. Export the current package from the Cerberus dashboard or CrowdSec config page.", + "description": "Upload a tar.gz package. A backup is created before importing so you can roll back if needed. Export the current package from the Cerberus dashboard or CrowdSec config page.", "import": "Import", "configImported": "CrowdSec config imported", "importFailed": "Import failed: {{error}}", @@ -1478,6 +1478,7 @@ }, "crowdsecConfig": { "title": "CrowdSec Configuration", + "checkNow": "Check Now", "loading": "Loading CrowdSec configuration...", "loadError": "Error loading CrowdSec status", "noStatus": "CrowdSec status unavailable", @@ -1547,7 +1548,7 @@ }, "packages": { "title": "Configuration Packages", - "description": "Export or import CrowdSec configuration packages (tar.gz/zip). A backup is created before importing.", + "description": "Export or import CrowdSec configuration packages (tar.gz). A backup is created before importing.", "export": "Export", "import": "Import", "importing": "Importing...", diff --git a/frontend/src/locales/es/translation.json b/frontend/src/locales/es/translation.json index 8720ce0f4..3c1d875f8 100644 --- a/frontend/src/locales/es/translation.json +++ b/frontend/src/locales/es/translation.json @@ -1069,7 +1069,7 @@ }, "importCrowdSec": { "title": "Paquetes de Configuración de CrowdSec", - "description": "Suba un paquete tar.gz o zip. Se crea una copia de seguridad antes de importar para que pueda revertir si es necesario. Exporte el paquete actual desde el panel de Cerberus o la página de configuración de CrowdSec.", + "description": "Suba un paquete tar.gz. Se crea una copia de seguridad antes de importar para que pueda revertir si es necesario. Exporte el paquete actual desde el panel de Cerberus o la página de configuración de CrowdSec.", "import": "Importar", "configImported": "Configuración de CrowdSec importada", "importFailed": "Importación fallida: {{error}}", @@ -1159,6 +1159,7 @@ }, "crowdsecConfig": { "title": "Configuración de CrowdSec", + "checkNow": "Comprobar ahora", "loading": "Cargando configuración de CrowdSec...", "loadError": "Error al cargar el estado de CrowdSec", "noStatus": "Estado de CrowdSec no disponible", @@ -1226,7 +1227,7 @@ }, "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.", + "description": "Exporte o importe paquetes de configuración de CrowdSec (tar.gz). Se crea una copia de seguridad antes de importar.", "export": "Exportar", "import": "Importar", "importing": "Importando...", diff --git a/frontend/src/locales/fr/translation.json b/frontend/src/locales/fr/translation.json index 7d0790a6f..228b7734c 100644 --- a/frontend/src/locales/fr/translation.json +++ b/frontend/src/locales/fr/translation.json @@ -1069,7 +1069,7 @@ }, "importCrowdSec": { "title": "Paquets de Configuration CrowdSec", - "description": "Téléversez un paquet tar.gz ou zip. Une sauvegarde est créée avant l'importation pour vous permettre de revenir en arrière si nécessaire. Exportez le paquet actuel depuis le tableau de bord Cerberus ou la page de configuration CrowdSec.", + "description": "Téléversez un paquet tar.gz. Une sauvegarde est créée avant l'importation pour vous permettre de revenir en arrière si nécessaire. Exportez le paquet actuel depuis le tableau de bord Cerberus ou la page de configuration CrowdSec.", "import": "Importer", "configImported": "Configuration CrowdSec importée", "importFailed": "Échec de l'importation : {{error}}", @@ -1159,6 +1159,7 @@ }, "crowdsecConfig": { "title": "Configuration CrowdSec", + "checkNow": "Vérifier maintenant", "loading": "Chargement de la configuration CrowdSec...", "loadError": "Erreur lors du chargement du statut CrowdSec", "noStatus": "Statut CrowdSec non disponible", @@ -1226,7 +1227,7 @@ }, "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.", + "description": "Exportez ou importez des paquets de configuration CrowdSec (tar.gz). Une sauvegarde est créée avant l'importation.", "export": "Exporter", "import": "Importer", "importing": "Importation...", diff --git a/frontend/src/locales/zh/translation.json b/frontend/src/locales/zh/translation.json index ee38da191..7811968ab 100644 --- a/frontend/src/locales/zh/translation.json +++ b/frontend/src/locales/zh/translation.json @@ -1068,7 +1068,7 @@ }, "importCrowdSec": { "title": "CrowdSec 配置包", - "description": "上传 tar.gz 或 zip 包。导入前会创建备份，以便需要时回滚。从 Cerberus 仪表板或 CrowdSec 配置页面导出当前包。", + "description": "上传 tar.gz 包。导入前会创建备份，以便需要时回滚。从 Cerberus 仪表板或 CrowdSec 配置页面导出当前包。", "import": "导入", "configImported": "CrowdSec 配置已导入", "importFailed": "导入失败：{{error}}", @@ -1158,6 +1158,7 @@ }, "crowdsecConfig": { "title": "CrowdSec 配置", + "checkNow": "立即检查", "loading": "正在加载 CrowdSec 配置...", "loadError": "加载 CrowdSec 状态时出错", "noStatus": "CrowdSec 状态不可用", @@ -1225,7 +1226,7 @@ }, "packages": { "title": "配置包", - "description": "导出或导入 CrowdSec 配置包（tar.gz/zip）。导入前会创建备份。", + "description": "导出或导入 CrowdSec 配置包（tar.gz）。导入前会创建备份。", "export": "导出", "import": "导入", "importing": "正在导入...", diff --git a/frontend/src/pages/CrowdSecConfig.tsx b/frontend/src/pages/CrowdSecConfig.tsx index 95958737f..ea359871b 100644 --- a/frontend/src/pages/CrowdSecConfig.tsx +++ b/frontend/src/pages/CrowdSecConfig.tsx @@ -647,7 +647,7 @@ export default function CrowdSecConfig() { onClick={() => lapiStatusQuery.refetch()} disabled={lapiStatusQuery.isRefetching} > - Check Now + {t('crowdsecConfig.checkNow')} @@ -690,7 +690,7 @@ export default function CrowdSecConfig() { onClick={() => lapiStatusQuery.refetch()} disabled={lapiStatusQuery.isRefetching} > - Check Now + {t('crowdsecConfig.checkNow')} diff --git a/frontend/src/pages/__tests__/CrowdSecConfig.test.tsx b/frontend/src/pages/__tests__/CrowdSecConfig.test.tsx index b628621dd..5bd3947e5 100644 --- a/frontend/src/pages/__tests__/CrowdSecConfig.test.tsx +++ b/frontend/src/pages/__tests__/CrowdSecConfig.test.tsx @@ -138,6 +138,13 @@ describe('CrowdSecConfig', () => { }) }) + it('restricts the import file picker to .tar.gz archives', async () => { + renderComponent() + + const input = await screen.findByTestId('import-file') + expect(input).toHaveAttribute('accept', '.tar.gz') + }) + // 2. File Editor it('allows reading and saving config files', async () => { const user = userEvent.setup() diff --git a/frontend/src/pages/__tests__/ImportCrowdSec.spec.tsx b/frontend/src/pages/__tests__/ImportCrowdSec.spec.tsx index a9f4f4e33..1611ac015 100644 --- a/frontend/src/pages/__tests__/ImportCrowdSec.spec.tsx +++ b/frontend/src/pages/__tests__/ImportCrowdSec.spec.tsx @@ -29,13 +29,14 @@ describe('ImportCrowdSec page', () => { }) it('creates a backup then imports crowdsec', async () => { - const file = new File(['fake'], 'crowdsec.zip', { type: 'application/zip' }) + const file = new File(['fake'], 'crowdsec.tar.gz', { type: 'application/gzip' }) vi.mocked(backups.createBackup).mockResolvedValue({ job_id: 'job-1', type: 'create', status: 'pending' }) vi.mocked(api.importCrowdsecConfig).mockResolvedValue({ success: true }) renderWithProviders() const fileInput = document.querySelector('input[type="file"]') expect(fileInput).toBeTruthy() + expect(fileInput).toHaveAttribute('accept', '.tar.gz') fireEvent.change(fileInput!, { target: { files: [file] } }) const importBtn = screen.getByText('Import') const user = userEvent.setup() diff --git a/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts b/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts index ef5f8f104..0c6ba39de 100644 --- a/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts +++ b/tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts @@ -6,7 +6,7 @@ * * Test Categories: * - Page Layout (2 tests): heading, form display - * - File Validation (3 tests): valid file, invalid format, missing fields + * - File Validation (3 tests): valid file, only .tar.gz offered, import disabled without a file * - Import Execution (3 tests): import success, error handling, already exists */ @@ -37,13 +37,6 @@ function createMockTarGzBuffer(): Buffer { return Buffer.from('mock tar.gz content for crowdsec config'); } -/** - * Helper to create a mock zip file buffer - */ -function createMockZipBuffer(): Buffer { - return Buffer.from('mock zip content for crowdsec config'); -} - test.describe('Import CrowdSec Configuration', () => { // ========================================================================= // Page Layout Tests (2 tests) @@ -66,8 +59,8 @@ test.describe('Import CrowdSec Configuration', () => { const fileInput = page.locator(SELECTORS.fileInput); await expect(fileInput).toBeVisible(); - // Verify it accepts proper file types (.tar.gz, .zip) - await expect(fileInput).toHaveAttribute('accept', /\.tar\.gz|\.zip/); + // Verify it accepts only .tar.gz + await expect(fileInput).toHaveAttribute('accept', '.tar.gz'); // Verify import button exists const importButton = page.locator(SELECTORS.importButton); @@ -118,43 +111,7 @@ test.describe('Import CrowdSec Configuration', () => { await expect(page.locator(SELECTORS.importButton)).toBeEnabled(); }); - test('should accept valid .zip configuration files', async ({ page, adminUser }) => { - await loginUser(page, adminUser); - - // Mock backup and import APIs - await page.route('**/api/v1/backups', async (route) => { - if (route.request().method() === 'POST') { - await route.fulfill({ - status: 201, - json: { filename: 'pre-import-backup.tar.gz', size: 1000, time: new Date().toISOString() }, - }); - } else { - await route.continue(); - } - }); - await page.route('**/api/v1/admin/crowdsec/import', async (route) => { - await route.fulfill({ - status: 200, - json: { message: 'Import successful' }, - }); - }); - - await page.goto('/tasks/import/crowdsec'); - await waitForLoadingComplete(page); - - // Upload .zip file - const fileInput = page.locator(SELECTORS.fileInput); - await fileInput.setInputFiles({ - name: 'crowdsec-config.zip', - mimeType: 'application/zip', - buffer: createMockZipBuffer(), - }); - - // Verify file was accepted (import button should be enabled) - await expect(page.locator(SELECTORS.importButton)).toBeEnabled(); - }); - - test.fixme('commit 4: should offer only .tar.gz in the file picker', async ({ page, adminUser }) => { + test('should offer only .tar.gz in the file picker', async ({ page, adminUser }) => { await loginUser(page, adminUser); await page.goto('/tasks/import/crowdsec'); await waitForLoadingComplete(page); From 6dcc71de405e70b73d9aca5bfead6c2dff3dda3d Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 05:37:45 +0000 Subject: [PATCH 05/10] docs: describe the CrowdSec export and import behavior --- docs/features/crowdsec.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/features/crowdsec.md b/docs/features/crowdsec.md index c83193053..1a52e4d32 100644 --- a/docs/features/crowdsec.md +++ b/docs/features/crowdsec.md @@ -66,6 +66,8 @@ Hub presets are applied on the server too, with the same safety net: before anyt - 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. +- An exported CrowdSec configuration contains configuration only. Account and connection details are not included, and when you import one, your running system keeps its own. CrowdSec re-creates its local connection details on its own. If you move to a new host and use community blocklists or the CrowdSec Console, re-enable console enrollment there. +- The CrowdSec import accepts only `.tar.gz` archives and checks that the main configuration file is valid YAML before applying anything. If it isn't, Charon tells you what's wrong, for example `config.yaml is not valid YAML` or `only .tar.gz archives are supported`. - 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 From a6416f8d1505c0ed27a188c25feae8f201fc21ff Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 06:44:46 +0000 Subject: [PATCH 06/10] docs: update QA report for the CrowdSec export and import changes --- docs/reports/qa_report.md | 70 ++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index cf657b559..44c7f684b 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,11 +1,11 @@ -# QA and Security Report: fix/crowdsec-hub-url-and-editor-hardening +# QA and Security Report: fix/crowdsec-export-import-followups 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). +Scope: full diff `origin/development...HEAD` (HEAD 6dcc71de, 5 commits, 27 files) against `SECURITY.md`, `CLAUDE.md` (Definition of Done) and the local plan for the CrowdSec export and import follow-ups (#1532, #1533, #1534). Parts: (A) the export omits engine-owned data and stored account details, and import keeps them (including on rollback); (B) the import locates `config.yaml` at the top level or in `config/` (a real-layout export now re-imports) and validates it by parsing; (C) a single `.tar.gz`-only message and UI hint; (D) the translated "Check Now" buttons. ## Verdict: PASS (no blocking findings) -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. +No critical, high or medium findings. Three low observations (F1-F3) and two informational items (F4-F5) are listed for the maintainer; none blocks merge. All Definition of Done gates pass. ## Definition of Done evidence @@ -17,55 +17,49 @@ No critical or high findings. One medium-low gap against the spec's own acceptan | 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 (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. | +| Frontend unit tests (touched) | `npx vitest run src/pages/__tests__/CrowdSecConfig* src/pages/__tests__/ImportCrowdSec* src/__tests__/i18n.test.ts` | 10 files, 113 tests passed | +| Race tests, crowdsec package | `go test -race ./internal/crowdsec/... -count=1` | ok (106 s) | +| Race tests, handlers | `go test -race -run 'Import\|Export\|Rollback' ./internal/api/handlers/... -count=1` | ok | +| GORM scan | not run | no `backend/internal/models/**`, migration or GORM query files in the diff (0 matches) | +| Lefthook | `lefthook run pre-commit --all-files` | all 17 hooks pass, including semgrep (0 findings, 367 rules; it logged one scan-timeout note on a large test file, which is a coverage note, not a finding). 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% | +| CodeQL | not run locally | `fix:`/`test:`/`docs:` scope, no new endpoint or feature surface, deferred to CI per CLAUDE.md | +| E2E | `npx playwright test --project=security-tests` against a container rebuilt from HEAD (`docker-rebuild-e2e`) | 194 passed, 0 failed (9.2 min) | +| Coverage (given, not rerun) | prior runs | backend 91.2% stmt / 90.2% line, frontend 91.46% lines, patch coverage 93.8% | ## 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. +1. **Export contents.** `ExportConfig` now uses `filepath.WalkDir` and skips an entry when `crowdsec.IsPreservedPath(rel)` is true (engine-owned state or a stored secret). `fs.SkipDir` is returned only for excluded directories; excluded files return `nil`, so siblings are never dropped (`TestExportConfigSkippedSecretKeepsSiblings`, golden list in `TestExportConfigRealLayoutEntryList`). Top-level `data/` and `hub_cache/` are not descended into; live db files and the three account/connection files are excluded at any depth and in any letter case (`TestIsSecretPath` covers case, depth, `./` and `//` forms, a directory named like a secret, and near-miss names that stay non-secret). Symlinks are still skipped. The gzip exclusion is unchanged (`routes.go:1260`, not in the diff). +2. **Import.** Uploaded archives cannot write protected entries: `extractArchive` skips any entry whose cleaned relative path is preserved, which covers case variants, nested directories named like a secret, and `..` forms that clean onto a secret. Tar symlink and hardlink entries are ignored by the extractor (regular files and directories only), and file creation still goes through `CreateFileNoSymlink`. Live files survive success: `ClearConfigKeeping(DataDir, IsPreservedPath)` evaluates `keep` before the entry type, so a kept name survives whether it is a file, directory or symlink (`TestClearConfigKeepingKeepsSymlinkedSecret`). Rollback uses `RestoreKeeping`, which clears with the same predicate and `copyTree` skips kept entries, so a live protected file is never deleted or re-copied; verified by tests for "secret changed since snapshot keeps the newer value", "restore copy failure does not delete live secrets" and `TestImportConfigFailureKeepsSecretsAndRestoresConfig`. A legacy archive that contains the protected files imports cleanly without replacing the live ones (`TestImportConfigNeverTakesProtectedEntriesFromUpload`). The real layout round trip (`config/config.yaml`) succeeds (`TestExportThenImportRealLayoutRoundTrip`, and live in E2E: "an export from this instance re-imports"). +3. **`FindConfigFile` and callers.** The helper prefers `config/config.yaml`, then the root file, else an empty string. `console_enroll.go` and `heartbeat_poller.go` are one-line delegations to it and are behavior-equivalent. In `DiagnosticsConnectivity` the `lapi status` site is equivalent; the `capi status` site gains the root fallback (benign, decided in the plan). In `DiagnosticsConfig` the traversal check still runs whenever a config is found, and the empty result keeps the `config.yaml not found` path unchanged (`TestDiagnosticsConfigLocatesConfigFile`, `TestDiagnosticsConnectivityPassesLocatedConfigToCscli`). `ImportConfig` returns the fixed `config.yaml was not found at the top level or in config/` when none is found (rollback runs first). +4. **YAML validator.** Verified: 1 MiB cap (`readCapped`, fixed message), mapping root required, at least one of the 8 `csconfig.Config` top-level keys, duplicate keys and non-string keys rejected (key types are checked on the nodes), first document only, empty file reported as empty, and all client messages are fixed strings (parser detail goes to the log only, through `sanitizeForLog`). The real default `config.yaml` fixture passes. **Alias and nesting abuse (probed with a temporary test, since removed):** a 420-byte document with 10 levels of 9-way aliases (about 9^10 expanded nodes) is rejected in under 1 ms with `config.yaml is not valid YAML` (yaml.v3: "document contains excessive aliasing", about 0 MB allocated); a 400 KB flow-sequence nesting bomb is rejected in 15 ms (6 MB allocated, "exceeded max depth of 10000"); 5000-deep nesting is accepted in 10 ms (3 MB). Together with the 1 MiB read cap, the import path cannot be exhausted this way. Note the probe went through `validateYAMLFile`; there is no existing unit test for alias input (see F3). +5. **ZIP/.tgz.** `requireTarGz` is the single format gate; every other extension (`.zip`, `.tgz`, `.txt`, none) returns `only .tar.gz archives are supported`. No zip code remains in the Go handler (the remaining "zip bomb" strings are comments and the compression-ratio message). Both file inputs use `accept=".tar.gz"`, and no ZIP wording remains in any locale. +6. **Preset apply.** `hub_sync.go` `extractTarGz` now skips `IsEngineOwnedPath || IsSecretPath` entries, with `TestExtractTarGzSkipsProtectedNamesAndKeepsSiblings` (siblings still extracted). +7. **Frontend.** `crowdsecConfig.checkNow` exists in all five locales (en exactly `Check Now`, de `Jetzt prüfen`, es `Comprobar ahora`, fr `Vérifier maintenant`, zh `立即检查`), both buttons use `t('crowdsecConfig.checkNow')` (lines 650 and 693), no hardcoded literal remains, and the i18n key test passes. +8. **E2E tolerance.** A grep over `tests/security/crowdsec-import.spec.ts`, `tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts` and `tests/security/crowdsec-diagnostics.spec.ts` for conditional assertions, `.catch(`, `expect([...])` status lists, alternation patterns, `toBeOneOf`, `expect.soft`, `test.fixme`, `.skip(`, `console.log` and `annotations` returned nothing. The diff adds one helper (`listTarGzEntries`) in `tests/utils/archive-helpers.ts`, which is in a directory excluded from the Docker build context. +9. **Commit subjects.** `git log origin/development..HEAD --format=%s`: `test(e2e): add fixme specs for the CrowdSec export and import changes`, `fix: slim down the CrowdSec configuration export`, `fix: validate CrowdSec configuration files by parsing them`, `fix: offer only .tar.gz in the CrowdSec import and translate Check Now`, `docs: describe the CrowdSec export and import behavior`. All generic, none uses the `(security)` scope, and a search of the full commit messages for secret, credential, key, overwrite, vulnerability and session terms found nothing. The docs sentence says "account and connection details are not included" without file names. +10. **Security-sensitive items.** None found that needs a private advisory. The change is a protective default plus an import correctness fix and is admin-authenticated throughout. See F1 for a related low observation. ## Findings -No critical or high findings. +No critical, high or medium findings. -### Security-sensitive (do not file publicly; maintainer to decide) +### Low -**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`. +- **F1: low (docs wording, maintainer decision).** `docs/features/crowdsec.md:69` says an export "contains configuration only". The export still includes other configuration that can hold connection details of its own (notification configs and a database section of `config.yaml`), so the sentence could give slightly more assurance than is true. The plan lists this as a known follow-up. Suggested: soften to "does not include CrowdSec's own account and connection details" or similar, and file the tracked low issue for redacting those parts. Keep the public wording generic. +- **F2: low (pre-existing, out of scope).** `DiagnosticsConfig` and similar sites check containment with `strings.HasPrefix(cleanPath, cleanDataDir)` without a path separator (`backend/internal/api/handlers/crowdsec_handler.go`, the "Path traversal protection" blocks around lines 2195 and 2240). A sibling directory sharing the prefix would pass. It is unreachable here because the path is always built from `DataDir` plus fixed names. Suggested: use `filepath.Rel` or compare with a trailing separator when next touched. +- **F3: low (test gap).** No unit test feeds an alias-expansion or deeply nested document to `validateYAMLFile` (probed manually, behavior is safe, see check 4). Suggested: add a small regression test for an alias bomb in `crowdsec_config_validation_test.go` so a yaml.v3 upgrade that changes the limits is caught. -**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. +### Informational (for the maintainer to file or ignore) -### Non-security (for the maintainer to file) +- **F4: info.** Preset apply and file-edit flows still use plain `Restore`, which copies snapshot copies of the protected files back on rollback (the plan keeps this unchanged on purpose). A rollback in those flows could therefore restore an older copy of a file that changed since the snapshot. Same files as before this change; no regression. +- **F5: info.** The root-only `-c` argument builders (`crowdsec_handler.go`, the Start/Stop/status helpers around lines 542, 635, 1774, 1858, 1899 on the base branch) were not switched to `FindConfigFile`; `cscli` falls back to its default location, so nothing breaks. Optional DRY follow-up. -- **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. +Planned follow-up issues from the plan (hub-item symlinks not restored by import, medium; export not serialized with import and truncated 200 on a mid-walk error, low) are unchanged by this PR and still need filing. ## Out of scope and not verified -- Full-suite and cross-browser E2E are deferred to CI. +- Full-suite and cross-browser E2E are deferred to CI; the targeted run used the single `security-tests` project. - 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. +- Coverage and local patch scripts were not rerun, as instructed (figures above are the earlier results). From fac0df36a48c5bfa9d10d75a550c636ebc9d7c98 Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 06:49:50 +0000 Subject: [PATCH 07/10] test: cover YAML alias expansion limits in CrowdSec config validation --- .../crowdsec_config_validation_test.go | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/backend/internal/api/handlers/crowdsec_config_validation_test.go b/backend/internal/api/handlers/crowdsec_config_validation_test.go index b687229c0..456a2a6d3 100644 --- a/backend/internal/api/handlers/crowdsec_config_validation_test.go +++ b/backend/internal/api/handlers/crowdsec_config_validation_test.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "fmt" "io" "mime/multipart" "net/http" @@ -10,6 +11,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -135,3 +137,66 @@ func TestImportConfigConfigYAMLValidationMessages(t *testing.T) { }) } } + +// validateWithin runs validateYAMLFile and fails the test if it does not return within the bound. +func validateWithin(t *testing.T, content string) error { + const bound = 10 * time.Second + t.Helper() + path := writeConfigFile(t, content) + done := make(chan error, 1) + go func() { done <- validateYAMLFile(path) }() + select { + case err := <-done: + return err + case <-time.After(bound): + t.Fatalf("validateYAMLFile did not return within %s", bound) + return nil + } +} + +// aliasBombDocument builds a small document whose fully expanded form is fanout^levels nodes. +func aliasBombDocument(levels, fanout int) string { + var b strings.Builder + b.WriteString("api:\n server: {}\n") + b.WriteString("l0: &l0 [" + strings.TrimSuffix(strings.Repeat("x,", fanout), ",") + "]\n") + for i := 1; i <= levels; i++ { + refs := make([]string, fanout) + for j := range refs { + refs[j] = fmt.Sprintf("*l%d", i-1) + } + fmt.Fprintf(&b, "l%d: &l%d [%s]\n", i, i, strings.Join(refs, ", ")) + } + return b.String() +} + +func TestValidateYAMLFileRejectsAliasExpansionBomb(t *testing.T) { + doc := aliasBombDocument(10, 9) + require.Less(t, len(doc), 1024, "the bomb must stay tiny on disk") + + start := time.Now() + err := validateWithin(t, doc) + require.EqualError(t, err, "config.yaml is not valid YAML") + require.Less(t, time.Since(start), 5*time.Second) +} + +func TestValidateYAMLFileAcceptsModerateAliasing(t *testing.T) { + // Two levels of 9-way reuse (81 expanded nodes) is ordinary anchor reuse and must still be accepted. + require.NoError(t, validateWithin(t, aliasBombDocument(2, 9))) +} + +func TestValidateYAMLFileNesting(t *testing.T) { + nested := func(depth int) string { + return "api:\n server: " + strings.Repeat("[", depth) + strings.Repeat("]", depth) + "\n" + } + + t.Run("moderate nesting is accepted", func(t *testing.T) { + require.NoError(t, validateWithin(t, nested(20))) + }) + + t.Run("very deep nesting returns promptly without panicking", func(t *testing.T) { + err := validateWithin(t, nested(5000)) + if err != nil { + require.EqualError(t, err, "config.yaml is not valid YAML") + } + }) +} From ec9dbb7aa1fb078dd5f7ec67e50a46ecb0eaf419 Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 06:50:37 +0000 Subject: [PATCH 08/10] docs: clarify what a CrowdSec configuration export contains --- docs/features/crowdsec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/crowdsec.md b/docs/features/crowdsec.md index 1a52e4d32..7756ae380 100644 --- a/docs/features/crowdsec.md +++ b/docs/features/crowdsec.md @@ -66,7 +66,7 @@ Hub presets are applied on the server too, with the same safety net: before anyt - 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. -- An exported CrowdSec configuration contains configuration only. Account and connection details are not included, and when you import one, your running system keeps its own. CrowdSec re-creates its local connection details on its own. If you move to a new host and use community blocklists or the CrowdSec Console, re-enable console enrollment there. +- An exported CrowdSec configuration is meant for moving settings, not for sharing. CrowdSec's account and connection details are not included, but other saved settings (for example notification and database settings) still can be, so treat the file as private. When you import one, your running system keeps its own. CrowdSec re-creates its local connection details on its own. If you move to a new host and use community blocklists or the CrowdSec Console, re-enable console enrollment there. - The CrowdSec import accepts only `.tar.gz` archives and checks that the main configuration file is valid YAML before applying anything. If it isn't, Charon tells you what's wrong, for example `config.yaml is not valid YAML` or `only .tar.gz archives are supported`. - 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. From 9b09b9f28787fa4e71d7ea99fd1c515e933996c3 Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 13:32:21 +0000 Subject: [PATCH 09/10] ci: fix the Codecov configuration so it is actually applied require_ci_to_pass was at the top level, which Codecov rejects, so the whole file was ignored and default coverage targets were used. --- codecov.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codecov.yml b/codecov.yml index 7f9588f19..2bd1ec3d8 100644 --- a/codecov.yml +++ b/codecov.yml @@ -19,8 +19,6 @@ coverage: target: 87% threshold: 1% -# Fail CI if Codecov upload/report indicates a problem -require_ci_to_pass: yes # ----------------------------------------------------------------------------- # Notification gating @@ -41,6 +39,8 @@ require_ci_to_pass: yes # after_n_builds: 3 may keep the status pending. If a test job fails, nothing # is uploaded; re-run the failed job. Normal pull_request runs execute all 3. codecov: + # Fail CI if Codecov upload/report indicates a problem + require_ci_to_pass: yes notify: after_n_builds: 3 wait_for_ci: yes From 159dc7d82ca8649772df567437b11da89f886d54 Mon Sep 17 00:00:00 2001 From: Jeremy Hatfield Date: Sat, 10 Oct 2026 13:34:16 +0000 Subject: [PATCH 10/10] ci: validate codecov.yml in the Codecov parity guard Post codecov.yml to Codecov's validator so an invalid file (which Codecov silently ignores wholesale) fails CI. Validation errors fail the check; an unreachable validator only warns. Adds bats coverage with a stubbed curl. --- .github/workflows/quality-checks.yml | 13 +++ scripts/ci/check-codecov-trigger-parity.sh | 40 +++++++++ .../tests/check-codecov-trigger-parity.bats | 85 +++++++++++++++++++ 3 files changed, 138 insertions(+) create mode 100644 scripts/tests/check-codecov-trigger-parity.bats diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 317adf4f8..01f8c084e 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -91,6 +91,19 @@ jobs: run: | bash scripts/ci/check-codecov-trigger-parity.sh + - name: Install bats + shellcheck + run: | + set -euo pipefail + sudo apt-get update + sudo apt-get install -y bats shellcheck + + - name: shellcheck (shell severity=error) + run: | + shellcheck --severity=error scripts/ci/check-codecov-trigger-parity.sh + + - name: bats + run: bats scripts/tests/check-codecov-trigger-parity.bats + toolchain-key-tests: name: Toolchain key / freshness-guard scripts (bats) runs-on: ubuntu-latest diff --git a/scripts/ci/check-codecov-trigger-parity.sh b/scripts/ci/check-codecov-trigger-parity.sh index ef59280f7..5647b0fea 100644 --- a/scripts/ci/check-codecov-trigger-parity.sh +++ b/scripts/ci/check-codecov-trigger-parity.sh @@ -11,6 +11,44 @@ fail() { exit 1 } +# Validate codecov.yml against Codecov's own validator. An invalid file is +# ignored wholesale by Codecov (default targets apply, settings never take +# effect), so drift here is otherwise silent. Validation errors fail the check; +# an unreachable validator only warns so a third-party outage cannot break CI. +validate_codecov_config() { + local config="${CODECOV_CONFIG:-codecov.yml}" + local url="${CODECOV_VALIDATE_URL:-https://codecov.io/validate}" + local body_file http_code response + + [[ -f "$config" ]] || fail "Missing Codecov config: $config" + + body_file="$(mktemp)" + if ! http_code="$(curl --silent --show-error --max-time 30 \ + --data-binary "@$config" --output "$body_file" \ + --write-out '%{http_code}' "$url")" || [[ "$http_code" != "200" && "$http_code" != "400" ]]; then + rm -f "$body_file" + echo "::warning title=Codecov validation skipped::Could not reach the Codecov validator (HTTP ${http_code:-none}); $config was not validated" + return 0 + fi + + # The validator answers 200 "Valid!" or 400 with the validation errors; any + # other status (5xx, 429, proxy errors) is treated as an outage above. + response="$(cat "$body_file")" + rm -f "$body_file" + if [[ -z "$response" ]]; then + echo "::warning title=Codecov validation skipped::Empty response from the Codecov validator; $config was not validated" + return 0 + fi + + if [[ "$(head -n 1 <<<"$response")" == "Valid!" ]]; then + echo "$config is valid according to Codecov" + return 0 + fi + + # Collapse newlines so the message survives in a single ::error annotation. + fail "$config failed Codecov validation: $(tr '\n' ' ' <<<"$response" | cut -c1-500)" +} + [[ -f "$QUALITY_WORKFLOW" ]] || fail "Missing workflow file: $QUALITY_WORKFLOW" [[ -f "$CODECOV_WORKFLOW" ]] || fail "Missing workflow file: $CODECOV_WORKFLOW" @@ -33,3 +71,5 @@ if ! grep -Fq "$EXPECTED_COMMENT" "$QUALITY_WORKFLOW"; then fi echo "Codecov trigger/comment parity check passed" + +validate_codecov_config diff --git a/scripts/tests/check-codecov-trigger-parity.bats b/scripts/tests/check-codecov-trigger-parity.bats new file mode 100644 index 000000000..1bb3828e8 --- /dev/null +++ b/scripts/tests/check-codecov-trigger-parity.bats @@ -0,0 +1,85 @@ +#!/usr/bin/env bats +# +# Covers the codecov.yml validation step of +# scripts/ci/check-codecov-trigger-parity.sh. curl is replaced by a stub on +# PATH so no test touches the network. + +setup() { + REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../.." && pwd)" + SCRIPT_UNDER_TEST="$REPO_ROOT/scripts/ci/check-codecov-trigger-parity.sh" + + TMPROOT="$(mktemp -d)" + mkdir "$TMPROOT/bin" + export CODECOV_CONFIG="$TMPROOT/codecov.yml" + echo "codecov: {}" >"$CODECOV_CONFIG" + export PATH="$TMPROOT/bin:$PATH" +} + +teardown() { + rm -rf "$TMPROOT" +} + +# stub_curl [exit_code]: fake curl writing to the +# --output file and printing (the --write-out value). +stub_curl() { + local code="$1" body="$2" rc="${3:-0}" + cat >"$TMPROOT/bin/curl" <"\$out" +printf '%s' '$code' +exit $rc +STUB + chmod +x "$TMPROOT/bin/curl" +} + +run_check() { + (cd "$REPO_ROOT" && bash "$SCRIPT_UNDER_TEST") +} + +@test "Valid! response passes" { + stub_curl 200 $'Valid!\n{"codecov":{}}' + run run_check + [ "$status" -eq 0 ] + [[ "$output" == *"is valid according to Codecov"* ]] +} + +@test "validation errors fail with the validator message" { + stub_curl 400 "Error at ['require_ci_to_pass']: unknown field" + run run_check + [ "$status" -eq 1 ] + [[ "$output" == *"::error"* ]] + [[ "$output" == *"unknown field"* ]] +} + +@test "curl failure only warns" { + stub_curl 000 "" 7 + run run_check + [ "$status" -eq 0 ] + [[ "$output" == *"::warning"* ]] +} + +@test "non-200 response only warns" { + stub_curl 503 "Service Unavailable" + run run_check + [ "$status" -eq 0 ] + [[ "$output" == *"::warning"* ]] +} + +@test "empty 200 response only warns" { + stub_curl 200 "" + run run_check + [ "$status" -eq 0 ] + [[ "$output" == *"::warning"* ]] +} + +@test "missing codecov config fails" { + rm -f "$CODECOV_CONFIG" + stub_curl 200 "Valid!" + run run_check + [ "$status" -eq 1 ] + [[ "$output" == *"Missing Codecov config"* ]] +}