diff --git a/.github/agents/QA_Security.agent.md b/.github/agents/QA_Security.agent.md index 499cf198..b724ab78 100644 --- a/.github/agents/QA_Security.agent.md +++ b/.github/agents/QA_Security.agent.md @@ -30,8 +30,8 @@ Your job is to act as an ADVERSARY. The Developer says "it works"; your job is t - **Path Verification**: Run `list_dir internal/api` to verify where tests should go. - **Creation**: Write a new test file (e.g., `internal/api/tests/audit_test.go`) to test the *flow*. - **Run**: Execute `.github/skills`, `go test ./internal/api/tests/...` (or specific path). Run local CodeQL and Trivy scans (they are built as VS Code Tasks so they just need to be triggered to run), pre-commit all files, and triage any findings. - - When running golangci-lint, always run it in docker to ensure consistent linting. - - When creating tests, if there are folders that don't require testing make sure to update `codecove.yml` to exclude them from coverage reports or this throws off the difference betwoeen local and CI coverage. + - **GolangCI-Lint (CRITICAL)**: Always run VS Code task "Lint: GolangCI-Lint (Docker)" - NOT "Lint: Go Vet". The Go Vet task only runs `go vet` which misses gocritic, bodyclose, and other linters that CI runs. GolangCI-Lint in Docker ensures parity with CI. + - When creating tests, if there are folders that don't require testing make sure to update `codecov.yml` to exclude them from coverage reports or this throws off the difference between local and CI coverage. - **Cleanup**: If the test was temporary, delete it. If it's valuable, keep it. @@ -69,9 +69,10 @@ When Trivy or CodeQLreports CVEs in container dependencies (especially Caddy tra The task is not complete until ALL of the following pass with zero issues: 1. **Security Scans**: - - CodeQL: Run as VS Code task or via GitHub Actions - - Trivy: Run as VS Code task or via Docker - - Zero issues allowed + - CodeQL: Run VS Code task "Security: CodeQL All (CI-Aligned)" or individual Go/JS tasks + - Trivy: Run VS Code task "Security: Trivy Scan" + - Go Vulnerabilities: Run VS Code task "Security: Go Vulnerability Check" + - Zero Critical/High issues allowed 2. **Coverage Tests (MANDATORY - Run Explicitly)**: - **Backend**: Run VS Code task "Test: Backend with Coverage" or execute `scripts/go-test-coverage.sh` @@ -87,8 +88,13 @@ The task is not complete until ALL of the following pass with zero issues: 4. **Pre-commit Hooks**: Run `pre-commit run --all-files` (this runs fast hooks only; coverage was verified in step 1) - -5. **Linting**: All language-specific linters must pass (Go vet, ESLint, markdownlint) +5. **Linting (MANDATORY - Run All Explicitly)**: + - **Backend GolangCI-Lint**: Run VS Code task "Lint: GolangCI-Lint (Docker)" - This is the FULL linter suite including gocritic, bodyclose, etc. + - **Why**: "Lint: Go Vet" only runs `go vet`, NOT the full golangci-lint suite. CI runs golangci-lint, so you MUST run this task to match CI behavior. + - **Command**: `cd backend && docker run --rm -v $(pwd):/app:ro -w /app golangci/golangci-lint:latest golangci-lint run -v` + - **Frontend ESLint**: Run VS Code task "Lint: Frontend" + - **Markdownlint**: Run VS Code task "Lint: Markdownlint" + - **Hadolint**: Run VS Code task "Lint: Hadolint Dockerfile" (if Dockerfile was modified) **Critical Note**: Leaving this unfinished prevents commit, push, and leaves users open to security concerns. All issues must be fixed regardless of whether they are unrelated to the original task. This rule must never be skipped. It is non-negotiable anytime any bit of code is added or changed. diff --git a/backend/internal/api/handlers/additional_handlers_test.go b/backend/internal/api/handlers/additional_handlers_test.go index 70b00a71..081c2d29 100644 --- a/backend/internal/api/handlers/additional_handlers_test.go +++ b/backend/internal/api/handlers/additional_handlers_test.go @@ -205,7 +205,7 @@ func TestDomainHandler_Delete_Additional(t *testing.T) { require.NoError(t, db.Create(&domain).Error) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodDelete, "/domains/"+testUUID, nil) + req := httptest.NewRequest(http.MethodDelete, "/domains/"+testUUID, http.NoBody) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -228,7 +228,7 @@ func TestDomainHandler_Delete_NotFound_Additional(t *testing.T) { router.DELETE("/domains/:id", handler.Delete) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodDelete, "/domains/nonexistent", nil) + req := httptest.NewRequest(http.MethodDelete, "/domains/nonexistent", http.NoBody) router.ServeHTTP(w, req) // Should still return OK (delete is idempotent) @@ -259,7 +259,7 @@ func TestNotificationHandler_List_Additional(t *testing.T) { require.NoError(t, db.Create(¬if2).Error) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/notifications", nil) + req := httptest.NewRequest(http.MethodGet, "/notifications", http.NoBody) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -287,7 +287,7 @@ func TestNotificationHandler_MarkAsRead_Additional(t *testing.T) { require.NoError(t, db.Create(¬if).Error) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodPut, "/notifications/"+notif.ID+"/read", nil) + req := httptest.NewRequest(http.MethodPut, "/notifications/"+notif.ID+"/read", http.NoBody) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -318,7 +318,7 @@ func TestNotificationHandler_MarkAllAsRead_Additional(t *testing.T) { require.NoError(t, db.Create(¬if2).Error) w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodPut, "/notifications/read-all", nil) + req := httptest.NewRequest(http.MethodPut, "/notifications/read-all", http.NoBody) router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) diff --git a/backend/internal/api/handlers/logs_ws.go b/backend/internal/api/handlers/logs_ws.go index 573a8d45..9e7b3fcf 100644 --- a/backend/internal/api/handlers/logs_ws.go +++ b/backend/internal/api/handlers/logs_ws.go @@ -43,6 +43,7 @@ func NewLogsWSHandler(tracker *services.WebSocketTracker) *LogsWSHandler { } // LogsWebSocketHandler handles WebSocket connections for live log streaming. +// // Deprecated: Use NewLogsWSHandler().HandleWebSocket instead. Kept for backward compatibility. func LogsWebSocketHandler(c *gin.Context) { // For backward compatibility, create a nil tracker if called directly diff --git a/backend/internal/crowdsec/console_enroll_test.go b/backend/internal/crowdsec/console_enroll_test.go index 71c4d970..8c9603e2 100644 --- a/backend/internal/crowdsec/console_enroll_test.go +++ b/backend/internal/crowdsec/console_enroll_test.go @@ -1086,7 +1086,7 @@ func TestFindConfigPath_RootLayout(t *testing.T) { // Create config.yaml in root (not in config/ subdirectory) configPath := filepath.Join(tmpDir, "config.yaml") - require.NoError(t, os.WriteFile(configPath, []byte("common:\n daemonize: false"), 0644)) + require.NoError(t, os.WriteFile(configPath, []byte("common:\n daemonize: false"), 0o644)) exec := &stubEnvExecutor{} svc := NewConsoleEnrollmentService(db, exec, tmpDir, "secret") diff --git a/backend/internal/crowdsec/hub_sync_test.go b/backend/internal/crowdsec/hub_sync_test.go index 7e228657..c319ca79 100644 --- a/backend/internal/crowdsec/hub_sync_test.go +++ b/backend/internal/crowdsec/hub_sync_test.go @@ -987,13 +987,14 @@ func TestParseRawIndex(t *testing.T) { // Verify collection entry var demoFound bool for _, item := range idx.Items { - if item.Name == "crowdsecurity/demo" { - demoFound = true - require.Equal(t, "collections", item.Type) - require.Equal(t, "1.0", item.Version) - require.Equal(t, "Demo collection", item.Description) - require.Contains(t, item.DownloadURL, "collections/crowdsecurity/demo.tgz") + if item.Name != "crowdsecurity/demo" { + continue } + demoFound = true + require.Equal(t, "collections", item.Type) + require.Equal(t, "1.0", item.Version) + require.Equal(t, "Demo collection", item.Description) + require.Contains(t, item.DownloadURL, "collections/crowdsecurity/demo.tgz") } require.True(t, demoFound) }) @@ -1063,7 +1064,7 @@ func TestHubService_Apply_CacheRefresh(t *testing.T) { require.NoError(t, err) // Reset time to trigger expiration - cache.nowFn = func() time.Time { return time.Now() } + cache.nowFn = time.Now indexBody := `{"items":[{"name":"test/preset","title":"Test","etag":"etag2","download_url":"http://test.hub/preset.tgz"}]}` newArchive := makeTarGz(t, map[string]string{"config.yml": "new"}) diff --git a/backend/internal/network/safeclient_test.go b/backend/internal/network/safeclient_test.go index 9016ddda..86fc7c90 100644 --- a/backend/internal/network/safeclient_test.go +++ b/backend/internal/network/safeclient_test.go @@ -698,7 +698,10 @@ func TestNewSafeHTTPClient_TooManyRedirects(t *testing.T) { WithMaxRedirects(3), ) - _, err := client.Get(server.URL) + resp, err := client.Get(server.URL) + if resp != nil { + resp.Body.Close() + } if err == nil { t.Error("expected error for too many redirects") } diff --git a/backend/internal/security/audit_logger.go b/backend/internal/security/audit_logger.go index c3419610..bc1e3890 100644 --- a/backend/internal/security/audit_logger.go +++ b/backend/internal/security/audit_logger.go @@ -53,7 +53,7 @@ func (al *AuditLogger) LogURLValidation(event AuditEvent) { } // LogURLTest is a convenience method for logging URL connectivity tests. -func (al *AuditLogger) LogURLTest(host, requestID, userID, sourceIP string, result string) { +func (al *AuditLogger) LogURLTest(host, requestID, userID, sourceIP, result string) { event := AuditEvent{ Timestamp: time.Now().UTC().Format(time.RFC3339), Action: "url_connectivity_test", diff --git a/backend/internal/services/crowdsec_startup_test.go b/backend/internal/services/crowdsec_startup_test.go index 301754fc..e2054f77 100644 --- a/backend/internal/services/crowdsec_startup_test.go +++ b/backend/internal/services/crowdsec_startup_test.go @@ -34,7 +34,7 @@ func (m *mockCrowdsecExecutor) Stop(ctx context.Context, configDir string) error return nil } -func (m *mockCrowdsecExecutor) Status(ctx context.Context, configDir string) (bool, int, error) { +func (m *mockCrowdsecExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) { m.statusCalled = true return m.running, m.pid, m.statusErr } @@ -57,7 +57,7 @@ func (m *smartMockCrowdsecExecutor) Stop(ctx context.Context, configDir string) return nil } -func (m *smartMockCrowdsecExecutor) Status(ctx context.Context, configDir string) (bool, int, error) { +func (m *smartMockCrowdsecExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) { m.statusCalled = true // Return running=true if Start was called (simulates successful start) if m.startCalled { @@ -611,7 +611,7 @@ func (m *verificationFailExecutor) Stop(ctx context.Context, configDir string) e return nil } -func (m *verificationFailExecutor) Status(ctx context.Context, configDir string) (bool, int, error) { +func (m *verificationFailExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) { m.statusCalls++ // First call (pre-start check): not running // Second call (post-start verify): still not running (FAIL) @@ -639,7 +639,7 @@ func (m *verificationErrorExecutor) Stop(ctx context.Context, configDir string) return nil } -func (m *verificationErrorExecutor) Status(ctx context.Context, configDir string) (bool, int, error) { +func (m *verificationErrorExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) { m.statusCalls++ // First call: not running // Second call: return error during verification