diff --git a/.github/renovate.json b/.github/renovate.json index 154547e5..d77b639d 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -129,7 +129,7 @@ "packageRules": [ { - "description": "THE MEGAZORD: Group ALL non-major updates (NPM, Docker, Go, Actions) into one weekly PR", + "description": "THE MEGAZORD: Group ALL non-major updates (NPM, Docker, Go, Actions) into one PR", "matchPackagePatterns": ["*"], "matchUpdateTypes": [ "minor", @@ -153,7 +153,7 @@ "minimumReleaseAge": "14 days" }, { - "description": "Preserve your custom Caddy patch labels but allow them to group into the weekly PR", + "description": "Preserve your custom Caddy patch labels but allow them to group into a single PR", "matchManagers": ["custom.regex"], "matchFileNames": ["Dockerfile"], "labels": ["caddy-patch", "security"], diff --git a/.github/workflows/cerberus-integration.yml b/.github/workflows/cerberus-integration.yml index 23c299b6..071d5927 100644 --- a/.github/workflows/cerberus-integration.yml +++ b/.github/workflows/cerberus-integration.yml @@ -11,6 +11,8 @@ on: type: string pull_request: push: + branches: + - main # Prevent race conditions when PR is updated mid-test # Cancels old test runs when new build completes with different SHA diff --git a/.github/workflows/crowdsec-integration.yml b/.github/workflows/crowdsec-integration.yml index 9afb550a..5a2fc20c 100644 --- a/.github/workflows/crowdsec-integration.yml +++ b/.github/workflows/crowdsec-integration.yml @@ -11,6 +11,8 @@ on: type: string pull_request: push: + branches: + - main # Prevent race conditions when PR is updated mid-test # Cancels old test runs when new build completes with different SHA diff --git a/.github/workflows/rate-limit-integration.yml b/.github/workflows/rate-limit-integration.yml index 5b11e512..8c74f3a7 100644 --- a/.github/workflows/rate-limit-integration.yml +++ b/.github/workflows/rate-limit-integration.yml @@ -11,6 +11,8 @@ on: type: string pull_request: push: + branches: + - main # Prevent race conditions when PR is updated mid-test # Cancels old test runs when new build completes with different SHA diff --git a/.github/workflows/waf-integration.yml b/.github/workflows/waf-integration.yml index 529b1636..65b6fe79 100644 --- a/.github/workflows/waf-integration.yml +++ b/.github/workflows/waf-integration.yml @@ -11,6 +11,8 @@ on: type: string pull_request: push: + branches: + - main # Prevent race conditions when PR is updated mid-test # Cancels old test runs when new build completes with different SHA diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 6362c94c..d3924291 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -132,6 +132,34 @@ "group": "test", "problemMatcher": [] }, + { + "label": "Test: Backend Flaky - Certificate List Stability Loop", + "type": "shell", + "command": "cd /projects/Charon && mkdir -p test-results/flaky && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_List_WithCertificates$' -count=100 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-list-stability.jsonl", + "group": "test", + "problemMatcher": [] + }, + { + "label": "Test: Backend Flaky - Certificate List Race Loop", + "type": "shell", + "command": "cd /projects/Charon && mkdir -p test-results/flaky && go test -race ./backend/internal/api/handlers -run '^TestCertificateHandler_List_WithCertificates$' -count=30 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-list-race.jsonl", + "group": "test", + "problemMatcher": [] + }, + { + "label": "Test: Backend Flaky - Certificate DB Setup Ordering Loop", + "type": "shell", + "command": "cd /projects/Charon && mkdir -p test-results/flaky && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_DBSetupOrdering$' -count=50 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-db-setup-ordering.jsonl", + "group": "test", + "problemMatcher": [] + }, + { + "label": "Test: Backend Flaky - Certificate Handler Focused Regression", + "type": "shell", + "command": "cd /projects/Charon && mkdir -p test-results/flaky && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_' -count=1 -json 2>&1 | tee test-results/flaky/cert-handler-regression.jsonl", + "group": "test", + "problemMatcher": [] + }, { "label": "Test: Coverage Inputs for Local Patch Report", "type": "shell", diff --git a/backend/internal/api/handlers/certificate_handler_coverage_test.go b/backend/internal/api/handlers/certificate_handler_coverage_test.go index e382e1da..e936bc00 100644 --- a/backend/internal/api/handlers/certificate_handler_coverage_test.go +++ b/backend/internal/api/handlers/certificate_handler_coverage_test.go @@ -4,19 +4,16 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" - "gorm.io/driver/sqlite" - "gorm.io/gorm" "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" ) func TestCertificateHandler_List_DBError(t *testing.T) { - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + db := OpenTestDB(t) // Don't migrate to cause error gin.SetMode(gin.TestMode) @@ -34,8 +31,7 @@ func TestCertificateHandler_List_DBError(t *testing.T) { } func TestCertificateHandler_Delete_InvalidID(t *testing.T) { - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) - _ = db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}) + db := OpenTestDBWithMigrations(t) gin.SetMode(gin.TestMode) r := gin.New() @@ -52,9 +48,7 @@ func TestCertificateHandler_Delete_InvalidID(t *testing.T) { } func TestCertificateHandler_Delete_NotFound(t *testing.T) { - // Use unique in-memory DB per test to avoid SQLite locking issues in parallel test runs - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) - _ = db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}) + db := OpenTestDBWithMigrations(t) gin.SetMode(gin.TestMode) r := gin.New() @@ -71,9 +65,7 @@ func TestCertificateHandler_Delete_NotFound(t *testing.T) { } func TestCertificateHandler_Delete_NoBackupService(t *testing.T) { - // Use unique in-memory DB per test to avoid SQLite locking issues in parallel test runs - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) - _ = db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}) + db := OpenTestDBWithMigrations(t) // Create certificate cert := models.SSLCertificate{UUID: "test-cert-no-backup", Name: "no-backup-cert", Provider: "custom", Domains: "nobackup.example.com"} @@ -83,17 +75,6 @@ func TestCertificateHandler_Delete_NoBackupService(t *testing.T) { r := gin.New() r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db) - // Wait for background sync goroutine to complete to avoid race with -race flag - // NewCertificateService spawns a goroutine that immediately queries the DB - // which can race with our test HTTP request. Give it time to complete. - // In real usage, this isn't an issue because the server starts before receiving requests. - // Alternative would be to add a WaitGroup to CertificateService, but that's overkill for tests. - // A simple sleep is acceptable here as it's test-only code. - // 100ms is more than enough for the goroutine to finish its initial sync. - // This is the minimum reliable wait time based on empirical testing with -race flag. - // The goroutine needs to: acquire mutex, stat directory, query DB, release mutex. - // On CI runners, this can take longer than on local dev machines. - time.Sleep(200 * time.Millisecond) // No backup service h := NewCertificateHandler(svc, nil, nil) @@ -108,8 +89,7 @@ func TestCertificateHandler_Delete_NoBackupService(t *testing.T) { } func TestCertificateHandler_Delete_CheckUsageDBError(t *testing.T) { - // Use unique in-memory DB per test to avoid SQLite locking issues in parallel test runs - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + db := OpenTestDB(t) // Only migrate SSLCertificate, not ProxyHost to cause error when checking usage _ = db.AutoMigrate(&models.SSLCertificate{}) @@ -132,9 +112,7 @@ func TestCertificateHandler_Delete_CheckUsageDBError(t *testing.T) { } func TestCertificateHandler_List_WithCertificates(t *testing.T) { - // Use unique in-memory DB per test to avoid SQLite locking issues in parallel test runs - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) - _ = db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}) + db := OpenTestDBWithMigrations(t) // Create certificates db.Create(&models.SSLCertificate{UUID: "cert-1", Name: "Cert 1", Provider: "custom", Domains: "one.example.com"}) @@ -159,8 +137,7 @@ func TestCertificateHandler_List_WithCertificates(t *testing.T) { func TestCertificateHandler_Delete_ZeroID(t *testing.T) { // Tests the ID=0 validation check (line 149-152 in certificate_handler.go) // DELETE /api/certificates/0 should return 400 Bad Request - db, _ := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) - _ = db.AutoMigrate(&models.SSLCertificate{}, &models.ProxyHost{}) + db := OpenTestDBWithMigrations(t) gin.SetMode(gin.TestMode) r := gin.New() @@ -176,3 +153,37 @@ func TestCertificateHandler_Delete_ZeroID(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "invalid id") } + +func TestCertificateHandler_DBSetupOrdering(t *testing.T) { + db := OpenTestDBWithMigrations(t) + + var certTableCount int64 + if err := db.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name = ?", "ssl_certificates").Scan(&certTableCount).Error; err != nil { + t.Fatalf("failed to verify ssl_certificates table: %v", err) + } + if certTableCount != 1 { + t.Fatalf("expected ssl_certificates table to exist before service initialization") + } + + var proxyHostsTableCount int64 + if err := db.Raw("SELECT count(*) FROM sqlite_master WHERE type='table' AND name = ?", "proxy_hosts").Scan(&proxyHostsTableCount).Error; err != nil { + t.Fatalf("failed to verify proxy_hosts table: %v", err) + } + if proxyHostsTableCount != 1 { + t.Fatalf("expected proxy_hosts table to exist before service initialization") + } + + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(mockAuthMiddleware()) + + svc := services.NewCertificateService("/tmp", db) + h := NewCertificateHandler(svc, nil, nil) + r.GET("/api/certificates", h.List) + + req := httptest.NewRequest(http.MethodGet, "/api/certificates", http.NoBody) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) +} diff --git a/backend/internal/services/certificate_service.go b/backend/internal/services/certificate_service.go index 9110a375..f6806d8a 100644 --- a/backend/internal/services/certificate_service.go +++ b/backend/internal/services/certificate_service.go @@ -52,12 +52,6 @@ func NewCertificateService(dataDir string, db *gorm.DB) *CertificateService { db: db, scanTTL: 5 * time.Minute, // Only rescan disk every 5 minutes } - // Perform initial scan in background - go func() { - if err := svc.SyncFromDisk(); err != nil { - logger.Log().WithError(err).Error("CertificateService: initial sync failed") - } - }() return svc } diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index defb78d4..fdb29d54 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,737 +1,369 @@ -## PR #718 CodeQL Remediation Master Plan (Detailed) +## Fix Flaky Go Test: `TestCertificateHandler_List_WithCertificates` -### Introduction +### 1) Introduction -This plan defines a full remediation program for CodeQL findings associated with PR #718, using repository evidence from: +This specification defines a focused, low-risk plan to eliminate intermittent CI failures for: -- `docs/reports/codeql_pr718_origin_map.md` -- `codeql-results-go.sarif` -- `codeql-results-js.sarif` -- `codeql-results-javascript.sarif` -- GitHub Code Scanning API snapshot for PR #718 (`state=open`) +- `backend/internal/api/handlers.TestCertificateHandler_List_WithCertificates` -Objectives: +while preserving production behavior in: -1. Close all PR #718 findings with deterministic verification. -2. Prioritize security-impacting findings first, then correctness/quality findings. -3. Minimize review overhead by slicing work into the fewest safe PRs. -4. Harden repository hygiene in `.gitignore`, `.dockerignore`, `codecov.yml`, and `.codecov.yml`. +- `backend/internal/services/certificate_service.go` +- `backend/internal/api/handlers/certificate_handler.go` +- `backend/internal/api/routes/routes.go` -### Research Findings +Primary objective: -#### Evidence summary +- Remove nondeterministic startup concurrency in certificate service initialization that intermittently races with handler list calls under CI parallelism/race detection. -- Origin-map report identifies **67 high alerts** mapped to PR #718 integration context: - - `go/log-injection`: 58 - - `js/regex/missing-regexp-anchor`: 6 - - `js/insecure-temporary-file`: 3 -- Current PR #718 open alert snapshot contains **100 open alerts**: - - `js/unused-local-variable`: 95 - - `js/automatic-semicolon-insertion`: 4 - - `js/comparison-between-incompatible-types`: 1 -- Current local SARIF snapshots show: - - `codeql-results-go.sarif`: 84 results (83 `go/log-injection`, 1 `go/cookie-secure-not-set`) - - `codeql-results-js.sarif`: 142 results (includes 6 `js/regex/missing-regexp-anchor`, 3 `js/insecure-temporary-file`) - - `codeql-results-javascript.sarif`: 0 results (stale/alternate artifact format) +### Decision Record - 2026-02-18 -#### Architecture and hotspot mapping (files/functions/components) +**Decision:** For this targeted flaky-fix, `docs/plans/current_spec.md` is the temporary single source of truth instead of creating separate `requirements.md`, `design.md`, and `tasks.md` artifacts. -Primary backend hotspots (security-sensitive log sinks): +**Context:** The scope is intentionally narrow (single flaky test root-cause + deterministic validation loop), and splitting artifacts now would add process overhead without reducing delivery risk. -- `backend/internal/api/handlers/crowdsec_handler.go` - - `(*CrowdsecHandler) PullPreset` - - `(*CrowdsecHandler) ApplyPreset` -- `backend/internal/api/handlers/proxy_host_handler.go` - - `(*ProxyHostHandler) Update` -- `backend/internal/api/handlers/emergency_handler.go` - - `(*EmergencyHandler) SecurityReset` - - `(*EmergencyHandler) performSecurityReset` -- `backend/internal/services/uptime_service.go` - - `(*UptimeService) CreateMonitor` -- `backend/internal/crowdsec/hub_sync.go` - - `(*HubService) Pull` - - `(*HubService) Apply` - - `(*HubService) fetchWithFallback` - - `(*HubService) loadCacheMeta` - - `(*HubService) refreshCache` +**Traceability mapping:** -Primary frontend/test hotspots: +- Requirements content is captured in Section 3 (EARS requirements). +- Design content is captured in Section 4 (technical specifications). +- Task breakdown and validation gates are captured in Section 5 (implementation plan). -- `tests/fixtures/auth-fixtures.ts` - - `acquireLock` - - `saveTokenCache` -- `tests/tasks/import-caddyfile.spec.ts` - - `test('should accept valid Caddyfile via file upload', ...)` - - `test('should accept valid Caddyfile via paste', ...)` -- `frontend/src/components/__tests__/SecurityHeaderProfileForm.test.tsx` - - CSP report-only URI test case -- `frontend/src/components/CredentialManager.tsx` - - incompatible type comparison at line 274 +**Review trigger:** If scope expands beyond the certificate flaky-fix boundary, restore full artifact split into `requirements.md`, `design.md`, and `tasks.md`. -#### Risk interpretation +--- -- The 67 high-security findings are blocking from a security posture perspective. -- The 100 open findings are mostly non-blocking quality/test hygiene, but they increase review noise and hide true security deltas. -- The most important engineering risk is inconsistent scanning/reporting context between CI, local tasks, and artifact naming. +### 2) Research Findings -### Requirements (EARS) +#### 2.1 CI failure evidence -1. **WHEN** PR #718 findings are remediated, **THE SYSTEM SHALL** produce zero high/critical CodeQL findings in Go and JavaScript scans. -2. **WHEN** log lines include user-influenced data, **THE SYSTEM SHALL** sanitize or quote those values before logging. -3. **WHEN** URL host regexes are used in assertions or validation, **THE SYSTEM SHALL** anchor expressions with explicit start/end boundaries. -4. **WHEN** temporary files are created in tests/fixtures, **THE SYSTEM SHALL** use secure creation semantics with restricted permissions and deterministic cleanup. -5. **WHEN** lint/quality-only findings are present, **THE SYSTEM SHALL** resolve them in a dedicated cleanup slice that does not change runtime behavior. -6. **IF** scan artifacts conflict (`codeql-results-javascript.sarif` vs `codeql-results-js.sarif`), **THEN THE SYSTEM SHALL** standardize to one canonical artifact path per language. -7. **WHILE** remediation is in progress, **THE SYSTEM SHALL** preserve deployability and pass DoD gates for each PR slice. +Observed in `.github/logs/ci_failure.log`: -### Technical Specifications +- `WARNING: DATA RACE` +- failing test: `TestCertificateHandler_List_WithCertificates` +- conflicting access path: + - **Write** from `(*CertificateService).SyncFromDisk` (`certificate_service.go`) + - **Read** from `(*CertificateService).ListCertificates` (`certificate_service.go`), triggered via handler list path -#### API / Backend design targets +#### 2.2 Existing architecture and hotspot map -- Introduce a consistent log-sanitization pattern: - - Use `utils.SanitizeForLog(...)` on user-controlled values. - - Prefer structured logging with placeholders instead of string concatenation. - - For ambiguous fields, use `%q`/quoted output where readability permits. -- Apply changes in targeted handlers/services only (no broad refactor in same PR): - - `backup_handler.go`, `crowdsec_handler.go`, `docker_handler.go`, `emergency_handler.go`, `proxy_host_handler.go`, `security_handler.go`, `settings_handler.go`, `uptime_handler.go`, `user_handler.go` - - `middleware/emergency.go` - - `cerberus/cerberus.go`, `cerberus/rate_limit.go` - - `crowdsec/console_enroll.go`, `crowdsec/hub_cache.go`, `crowdsec/hub_sync.go` - - `server/emergency_server.go` - - `services/backup_service.go`, `services/emergency_token_service.go`, `services/mail_service.go`, `services/manual_challenge_service.go`, `services/uptime_service.go` +**Service layer** -#### Frontend/test design targets +- File: `backend/internal/services/certificate_service.go` +- Key symbols: + - `NewCertificateService(caddyDataDir string, db *gorm.DB) *CertificateService` + - `SyncFromDisk() error` + - `ListCertificates() ([]models.SSLCertificate, error)` + - `InvalidateCache()` + - `refreshCacheFromDB() error` -- Regex remediation: - - Replace unanchored host patterns with anchored variants: `^https?:\/\/(allowed-host)(:\d+)?$` style. -- Insecure temp-file remediation: - - Replace ad hoc temp writes with `fs.mkdtemp`-scoped directories, `0o600` file permissions, and cleanup in `finally`. -- Quality warning remediation: - - Remove unused locals/imports in test utilities/specs. - - Resolve ASI warnings with explicit semicolons / expression wrapping. - - Resolve one incompatible comparison with explicit type normalization and guard. +**Handler layer** -#### CI/reporting hardening targets +- File: `backend/internal/api/handlers/certificate_handler.go` +- Key symbols: + - `func (h *CertificateHandler) List(c *gin.Context)` + - `func (h *CertificateHandler) Upload(c *gin.Context)` + - `func (h *CertificateHandler) Delete(c *gin.Context)` -- Standardize scan outputs: - - Go: `codeql-results-go.sarif` - - JS/TS: `codeql-results-js.sarif` -- Enforce single source of truth for local scans: - - `.vscode/tasks.json` → existing `scripts/pre-commit-hooks/codeql-*.sh` wrappers. -- Keep `security-and-quality` suite explicit and consistent. +**Route wiring** -### Finding-by-Finding Remediation Matrix +- File: `backend/internal/api/routes/routes.go` +- Key symbol usage: + - `services.NewCertificateService(caddyDataDir, db)` -#### Matrix A — High-risk units correlated to PR #718 origin commits +**Tests and setup patterns** -Scope: 75 location-level units from repository evidence (weighted counts), covering `go/log-injection`, `js/regex/missing-regexp-anchor`, and `js/insecure-temporary-file`. +- Files: + - `backend/internal/api/handlers/certificate_handler_coverage_test.go` + - `backend/internal/api/handlers/certificate_handler_test.go` + - `backend/internal/api/handlers/certificate_handler_security_test.go` + - `backend/internal/services/certificate_service_test.go` + - `backend/internal/api/handlers/testdb.go` +- Findings: + - many handler tests instantiate the real constructor directly. + - one test already includes a timing workaround (`time.Sleep`) due to startup race behavior. + - service tests already use a helper pattern that avoids constructor-side async startup, which validates the architectural direction for deterministic initialization in tests. -| Finding Unit | Count | Rule | Severity | File | Line | Function/Test Context | Root cause hypothesis | Fix pattern | Verification | Rollback | -|---|---:|---|---|---|---:|---|---|---|---|---| -| HR-001 | 4 | go/log-injection | high | internal/crowdsec/hub_sync.go | 579 | (s *HubService) Pull(ctx context.Context, slug string) (PullResult, error) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan + grep for raw log interpolations | Revert per-file sanitization patch | -| HR-002 | 4 | go/log-injection | high | internal/api/handlers/crowdsec_handler.go | 1110 | (h *CrowdsecHandler) PullPreset(c *gin.Context) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan + grep for raw log interpolations | Revert per-file sanitization patch | -| HR-003 | 3 | go/log-injection | high | internal/crowdsec/console_enroll.go | 213 | (s *ConsoleEnrollmentService) Enroll(ctx context.Context, req ConsoleEnrollRequest) (ConsoleEnrollmentStatus, error) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan + grep for raw log interpolations | Revert per-file sanitization patch | -| HR-004 | 2 | go/log-injection | high | internal/crowdsec/hub_sync.go | 793 | (s *HubService) refreshCache(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-005 | 2 | go/log-injection | high | internal/crowdsec/hub_sync.go | 720 | (s *HubService) fetchWithFallback(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-006 | 2 | go/log-injection | high | internal/crowdsec/hub_sync.go | 641 | (s *HubService) Apply(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-007 | 2 | go/log-injection | high | internal/crowdsec/hub_sync.go | 571 | (s *HubService) Pull(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-008 | 2 | go/log-injection | high | internal/crowdsec/hub_sync.go | 567 | (s *HubService) Pull(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-009 | 2 | go/log-injection | high | internal/crowdsec/console_enroll.go | 246 | (s *ConsoleEnrollmentService) Enroll(...) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-010 | 2 | go/log-injection | high | internal/cerberus/cerberus.go | 244 | (c *Cerberus) Middleware() gin.HandlerFunc | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-011 | 2 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 496 | (h *ProxyHostHandler) Update(c *gin.Context) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-012 | 2 | go/log-injection | high | internal/api/handlers/crowdsec_handler.go | 1216 | (h *CrowdsecHandler) ApplyPreset(c *gin.Context) | Unsanitized user-controlled data interpolated into logs | Wrap tainted fields with `utils.SanitizeForLog` or `%q`; avoid raw concatenation | Go unit tests + CodeQL Go scan | Revert per-file sanitization patch | -| HR-013 | 1 | js/regex/missing-regexp-anchor | high | tests/tasks/import-caddyfile.spec.ts | 324 | import-caddyfile paste test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Playwright/Vitest + CodeQL JS scan | Revert regex patch | -| HR-014 | 1 | js/regex/missing-regexp-anchor | high | tests/tasks/import-caddyfile.spec.ts | 307 | import-caddyfile upload test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Playwright/Vitest + CodeQL JS scan | Revert regex patch | -| HR-015 | 1 | js/regex/missing-regexp-anchor | high | tests/security-enforcement/zzz-caddy-imports/caddy-import-cross-browser.spec.ts | 204 | caddy import cross-browser test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Playwright/Vitest + CodeQL JS scan | Revert regex patch | -| HR-016 | 1 | js/regex/missing-regexp-anchor | high | frontend/src/pages/__tests__/ProxyHosts-progress.test.tsx | 141 | proxy hosts progress test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Vitest + CodeQL JS scan | Revert regex patch | -| HR-017 | 1 | js/regex/missing-regexp-anchor | high | frontend/src/components/__tests__/SecurityHeaderProfileForm.test.tsx | 310 | CSP report-only test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Vitest + CodeQL JS scan | Revert regex patch | -| HR-018 | 1 | js/regex/missing-regexp-anchor | high | frontend/src/components/__tests__/SecurityHeaderProfileForm.test.tsx | 298 | CSP report-only test | Regex host match not anchored | Add `^...$` anchors and explicit host escape | Targeted Vitest + CodeQL JS scan | Revert regex patch | -| HR-019 | 1 | js/insecure-temporary-file | high | tests/fixtures/auth-fixtures.ts | 181 | saveTokenCache helper | Temp file created in shared OS temp dir | Use `fs.mkdtemp` + `0o600` + deterministic cleanup | Fixture tests + CodeQL JS scan | Revert temp-file patch | -| HR-020 | 1 | js/insecure-temporary-file | high | tests/fixtures/auth-fixtures.ts | 129 | acquireLock helper | Temp file created in shared OS temp dir | Use `fs.mkdtemp` + `0o600` + deterministic cleanup | Fixture tests + CodeQL JS scan | Revert temp-file patch | -| HR-021 | 1 | js/insecure-temporary-file | high | tests/fixtures/auth-fixtures.ts | 107 | acquireLock helper | Temp file created in shared OS temp dir | Use `fs.mkdtemp` + `0o600` + deterministic cleanup | Fixture tests + CodeQL JS scan | Revert temp-file patch | -| HR-022 | 1 | go/log-injection | high | internal/api/handlers/backup_handler.go | 104 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-023 | 1 | go/log-injection | high | internal/api/handlers/crowdsec_handler.go | 1102 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-024 | 1 | go/log-injection | high | internal/api/handlers/crowdsec_handler.go | 1115 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-025 | 1 | go/log-injection | high | internal/api/handlers/crowdsec_handler.go | 1119 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-026 | 1 | go/log-injection | high | internal/api/handlers/docker_handler.go | 59 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-027 | 1 | go/log-injection | high | internal/api/handlers/docker_handler.go | 74 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-028 | 1 | go/log-injection | high | internal/api/handlers/docker_handler.go | 82 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-029 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 104 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-030 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 113 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-031 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 128 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-032 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 144 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-033 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 160 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-034 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 182 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-035 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 199 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-036 | 1 | go/log-injection | high | internal/api/handlers/emergency_handler.go | 92 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-037 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 459 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-038 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 468 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-039 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 472 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-040 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 474 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-041 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 477 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-042 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 481 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-043 | 1 | go/log-injection | high | internal/api/handlers/proxy_host_handler.go | 483 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-044 | 1 | go/log-injection | high | internal/api/handlers/security_handler.go | 1219 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-045 | 1 | go/log-injection | high | internal/api/handlers/settings_handler.go | 191 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-046 | 1 | go/log-injection | high | internal/api/handlers/uptime_handler.go | 103 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-047 | 1 | go/log-injection | high | internal/api/handlers/uptime_handler.go | 115 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-048 | 1 | go/log-injection | high | internal/api/handlers/uptime_handler.go | 64 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-049 | 1 | go/log-injection | high | internal/api/handlers/uptime_handler.go | 75 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-050 | 1 | go/log-injection | high | internal/api/handlers/uptime_handler.go | 82 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-051 | 1 | go/log-injection | high | internal/api/handlers/user_handler.go | 545 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-052 | 1 | go/log-injection | high | internal/api/middleware/emergency.go | 106 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-053 | 1 | go/log-injection | high | internal/api/middleware/emergency.go | 79 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-054 | 1 | go/log-injection | high | internal/cerberus/cerberus.go | 154 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-055 | 1 | go/log-injection | high | internal/cerberus/rate_limit.go | 128 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-056 | 1 | go/log-injection | high | internal/cerberus/rate_limit.go | 205 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-057 | 1 | go/log-injection | high | internal/crowdsec/console_enroll.go | 229 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-058 | 1 | go/log-injection | high | internal/crowdsec/hub_cache.go | 110 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-059 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 575 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-060 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 629 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-061 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 715 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-062 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 771 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-063 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 774 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-064 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 777 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-065 | 1 | go/log-injection | high | internal/crowdsec/hub_sync.go | 790 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-066 | 1 | go/log-injection | high | internal/server/emergency_server.go | 111 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-067 | 1 | go/log-injection | high | internal/services/backup_service.go | 685 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-068 | 1 | go/log-injection | high | internal/services/emergency_token_service.go | 128 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-069 | 1 | go/log-injection | high | internal/services/emergency_token_service.go | 303 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-070 | 1 | go/log-injection | high | internal/services/mail_service.go | 616 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-071 | 1 | go/log-injection | high | internal/services/manual_challenge_service.go | 184 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-072 | 1 | go/log-injection | high | internal/services/manual_challenge_service.go | 211 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-073 | 1 | go/log-injection | high | internal/services/manual_challenge_service.go | 286 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-074 | 1 | go/log-injection | high | internal/services/manual_challenge_service.go | 355 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | -| HR-075 | 1 | go/log-injection | high | internal/services/uptime_service.go | 1090 | Sanitized logging at sink (context in baseline export) | Unsanitized user-influenced value reaches log sink | Apply `utils.SanitizeForLog(...)` and structured logging placeholders; avoid raw concatenation | CodeQL Go scan (CI-aligned) + targeted go test for touched package + grep check for raw interpolations | Revert file-local sanitization commit owned by backend phase lead | +#### 2.3 Root cause summary -#### Matrix B — Current PR #718 open findings (per-file ownership) +Two explicit root-cause branches are tracked for this flaky area: -| Rule | Severity | Count | File | Alert IDs | Owner role | Root cause hypothesis | Fix pattern | Verification | Rollback | -|---|---|---:|---|---|---|---|---|---|---| -| js/automatic-semicolon-insertion | note | 1 | frontend/src/pages/__tests__/ProxyHosts-bulk-acl.test.tsx | 1248 | Frontend test owner | ASI-sensitive multiline statements in tests | Add explicit semicolons / wrap expressions | Targeted test files + CodeQL JS scan | Revert syntax-only commit | -| js/automatic-semicolon-insertion | note | 3 | tests/core/navigation.spec.ts | 1251,1250,1249 | E2E owner | ASI-sensitive multiline statements in tests | Add explicit semicolons / wrap expressions | Targeted test files + CodeQL JS scan | Revert syntax-only commit | -| js/comparison-between-incompatible-types | warning | 1 | frontend/src/components/CredentialManager.tsx | 1247 | Frontend owner | Incompatible operand types in `CredentialManager` | Normalize types before compare; add type guard | Unit test + `npm run type-check` + CodeQL JS scan | Revert isolated type fix | -| js/unused-local-variable | note | 1 | tests/global-setup.ts | 1156 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 4 | tests/integration/import-to-production.spec.ts | 1155,1154,1153,1152 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 5 | tests/integration/multi-feature-workflows.spec.ts | 1162,1160,1159,1158,1157 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 4 | tests/integration/proxy-certificate.spec.ts | 1170,1164,1163,1161 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 5 | tests/integration/proxy-dns-integration.spec.ts | 1169,1168,1167,1166,1165 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/modal-dropdown-triage.spec.ts | 1171 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/monitoring/uptime-monitoring.spec.ts | 1173 | E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/reporters/debug-reporter.ts | 1172 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security-enforcement/combined-enforcement.spec.ts | 1194 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/security-enforcement/emergency-server/emergency-server.spec.ts | 1196,1195 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security-enforcement/emergency-token.spec.ts | 1197 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security-enforcement/zzz-caddy-imports/caddy-import-firefox.spec.ts | 1198 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security-enforcement/zzz-caddy-imports/caddy-import-webkit.spec.ts | 1199 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 6 | tests/security-enforcement/zzz-security-ui/access-lists-crud.spec.ts | 1217,1213,1205,1204,1203,1202 | Security UI owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/security-enforcement/zzz-security-ui/crowdsec-import.spec.ts | 1201,1200 | Security UI owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 3 | tests/security-enforcement/zzz-security-ui/encryption-management.spec.ts | 1215,1214,1209 | Security UI owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 7 | tests/security-enforcement/zzz-security-ui/real-time-logs.spec.ts | 1216,1212,1211,1210,1208,1207,1206 | Security UI owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/security-enforcement/zzz-security-ui/system-security-settings.spec.ts | 1219,1218 | Security UI owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security-enforcement/zzzz-break-glass-recovery.spec.ts | 1220 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 8 | tests/security/acl-integration.spec.ts | 1184,1183,1182,1181,1180,1179,1178,1177 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security/audit-logs.spec.ts | 1175 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security/crowdsec-config.spec.ts | 1174 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security/crowdsec-decisions.spec.ts | 1179 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security/rate-limiting.spec.ts | 1185 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/security/security-headers.spec.ts | 1186 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 4 | tests/security/suite-integration.spec.ts | 1190,1189,1188,1187 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 3 | tests/security/waf-config.spec.ts | 1193,1192,1191 | Security E2E owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 5 | tests/settings/account-settings.spec.ts | 1227,1226,1224,1222,1221 | Settings test owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/settings/notifications.spec.ts | 1233,1225 | Settings test owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/settings/smtp-settings.spec.ts | 1223 | Settings test owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/settings/user-management.spec.ts | 1235,1234 | Settings test owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 3 | tests/tasks/backups-create.spec.ts | 1230,1229,1228 | Task flow owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/tasks/backups-restore.spec.ts | 1232,1231 | Task flow owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 2 | tests/tasks/import-caddyfile.spec.ts | 1237,1236 | Task flow owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/tasks/logs-viewing.spec.ts | 1238 | Task flow owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 3 | tests/utils/archive-helpers.ts | 1241,1240,1239 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/utils/debug-logger.ts | 1243 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/utils/diagnostic-helpers.ts | 1242 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/utils/phase5-helpers.ts | 1244 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/utils/test-steps.ts | 1245 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | -| js/unused-local-variable | note | 1 | tests/utils/wait-helpers.spec.ts | 1246 | QA tooling owner | Test helper variables/imports retained after refactors | Remove dead locals/imports; enforce lint gate | `npm run lint`, `npm run type-check`, CodeQL JS scan | Revert individual cleanup commits | +- **Branch A — constructor startup race:** `NewCertificateService` starts async state mutation (`SyncFromDisk`) immediately, while handler-driven reads (`ListCertificates`) may execute concurrently. This matches CI race output in `.github/logs/ci_failure.log` for `TestCertificateHandler_List_WithCertificates`. +- **Branch B — DB schema/setup ordering drift in tests:** some test paths reach service queries before deterministic schema setup is complete, producing intermittent setup-order failures such as `no such table: ssl_certificates` and `no such table: proxy_hosts`. -### Baseline Freshness Gate (Mandatory before each PR slice) +Plan intent: address both branches together in one tight PR by making constructor behavior deterministic and enforcing migration/setup ordering in certificate handler test setup. -1. Re-pull PR #718 open alerts immediately before opening/updating PR-1, PR-2, and PR-3. -2. Compare fresh snapshot against frozen baseline (`docs/reports/pr718_open_alerts_baseline.json`) by `alert_number`, `rule.id`, `location.path`, and `location.start_line`. -3. If drift is detected (new alert, missing alert, rule/line migration), planning fails closed and matrices must be regenerated before implementation proceeds. -4. Persist each freshness run to `docs/reports/pr718_open_alerts_freshness_.json` and add a delta summary in `docs/reports/`. +--- -Drift policy: +### 3) EARS Requirements (Source of Truth) -- `No drift`: proceed with current phase. -- `Additive drift`: block and expand Matrix A/B ownership before coding. -- `Subtractive drift`: verify closure source (already fixed vs query change) and update baseline evidence. +#### 3.1 Ubiquitous requirements -### Disposition Workflow (false-positive / won't-fix / out-of-scope) +- THE SYSTEM SHALL construct `CertificateService` in a deterministic initialization state before first handler read in tests. +- THE SYSTEM SHALL provide race-free access to certificate cache state during list operations. -All non-fixed findings require an explicit disposition record, no exceptions. +#### 3.2 Event-driven requirements -Required record fields: +- WHEN `NewCertificateService` returns, THE SYSTEM SHALL guarantee that any required first-read synchronization state is consistent for immediate `ListCertificates` calls. +- WHEN `CertificateHandler.List` is called, THE SYSTEM SHALL return data or a deterministic error without concurrency side effects caused by service startup. +- WHEN certificate handler tests initialize database state, THE SYSTEM SHALL complete required schema setup for certificate-related tables before service/handler execution. -- Alert ID, rule ID, file, line, severity. -- Disposition (`false-positive`, `won't-fix`, `out-of-scope`). -- Technical justification (query semantics, unreachable path, accepted risk, or external ownership). -- Evidence link (code reference, scan artifact, upstream issue, or policy decision). -- Owner role, reviewer/approver, decision date, next review date. -- Audit trail entry in `docs/reports/codeql_pr718_dispositions.md`. +#### 3.3 Unwanted-behavior requirements -Disposition gating rules: +- IF CI executes tests with race detector or parallel scheduling, THEN THE SYSTEM SHALL NOT produce data races between service initialization and list reads. +- IF startup synchronization fails, THEN THE SYSTEM SHALL surface explicit errors and preserve handler-level HTTP error behavior. +- IF test setup ordering is incorrect, THEN THE SYSTEM SHALL fail fast in setup with explicit diagnostics instead of deferred `no such table` runtime query failures. -1. `false-positive`: requires reviewer approval and reproducible evidence. -2. `won't-fix`: requires explicit risk acceptance and rollback/mitigation note. -3. `out-of-scope`: requires linked issue/PR and target milestone. -4. Any undispositioned unresolved finding blocks phase closure. +#### 3.4 Optional requirements -### Implementation Plan (Phase ↔ PR mapped execution) +- WHERE test-only construction paths are needed, THE SYSTEM SHALL use explicit test helpers/options instead of sleeps or timing assumptions. -#### Phase metadata (ownership, ETA, rollback) +--- -| Phase | PR slice | Primary owner role | ETA | Rollback owner | Merge dependency | -|---|---|---|---|---|---| -| Phase 1: Baseline freeze and freshness gate | PR-0 (no code changes) | Security lead | 0.5 day | Security lead | none | -| Phase 2: Security remediations | PR-1 | Backend security owner | 2-3 days | Backend owner | Phase 1 complete | -| Phase 3: Open alert cleanup | PR-2 | Frontend/E2E owner | 1-2 days | Frontend owner | PR-1 merged | -| Phase 4: Hygiene and scanner hardening | PR-3 | DevEx/CI owner | 1 day | DevEx owner | PR-1 and PR-2 merged | -| Phase 5: Final verification and closure | Post PR-3 | Release/security lead | 0.5 day | Release lead | PR-3 merged | +### 4) Technical Specifications -#### Phase 1 — Baseline freeze and freshness gate (PR-0) +#### 4.1 Service design changes + +Primary design target (preferred): + +1. Update `NewCertificateService` to remove nondeterministic startup behavior at construction boundary. +2. Ensure `SyncFromDisk` / cache state transition is serialized relative to early `ListCertificates` usage. +3. Keep existing mutex discipline explicit and auditable (single writer, safe readers). +4. Enforce deterministic DB setup ordering in certificate handler tests through shared setup helper behavior (migrate/setup before constructor and request execution). + +Design constraints: + +- No public API break for callers in `routes.go`. +- No broad refactor of certificate business logic. +- Keep behavior compatible with current handler response contracts. + +#### 4.2 Handler and route impact + +- `certificate_handler.go` should require no contract changes. +- `routes.go` keeps existing construction call shape unless a minimally invasive constructor option path is required. + +#### 4.3 Data flow (target state) + +1. Route wiring constructs certificate service. +2. Service enters stable initialized state before first list read path. +3. Handler `List` requests certificate data. +4. Service reads synchronized cache/DB state. +5. Handler responds `200` with deterministic payload or explicit `500` error. + +#### 4.4 Error handling matrix + +| Scenario | Expected behavior | Validation | +|---|---|---| +| Startup sync succeeds | List path returns stable data | handler list tests pass repeatedly | +| Startup sync fails | Error bubbles predictably to handler | HTTP 500 tests remain deterministic | +| Empty store | 200 + empty list (or existing contract shape) | existing empty-list tests pass | +| Concurrent test execution | No race detector findings in certificate tests | `go test -race` targeted suite | +| Test DB setup ordering incomplete | setup fails immediately with explicit setup error; no deferred query-time table errors | dedicated setup-ordering test + repeated run threshold | + +#### 4.5 API and schema impact + +- API endpoints: **no external contract changes**. +- Request/response schema: **unchanged** for certificate list/upload/delete. +- Database schema: **no migrations required**. + +--- + +### 5) Implementation Plan (Phased) + +## Phase 1: Playwright / UI-UX Baseline (Gate) + +Although this fix is backend-test focused, follow test protocol gate: + +- Reuse healthy E2E environment when possible; rebuild only if runtime image inputs changed. +- Use exact task/suite gate: + - task ID `shell: Test: E2E Playwright (FireFox) - Core: Certificates` + - suite path executed by the task: `tests/core/certificates.spec.ts` +- If rebuild is required by test protocol, run task ID `shell: Docker: Rebuild E2E Environment` before Playwright. Deliverables: -- Freeze baseline artifacts: - - `codeql-results-go.sarif` - - `codeql-results-js.sarif` - - `docs/reports/pr718_open_alerts_baseline.json` -- Confirm scanner parity and canonical artifact naming. +- passing targeted Playwright suite for certificate list interactions +- archived output in standard test artifacts -Tasks: - -1. Confirm all scan entrypoints produce canonical SARIF names. -2. Re-run CodeQL Go/JS scans locally with CI-aligned tasks. -3. Store pre-remediation summary in `docs/reports/`. -4. Run freshness gate and block if baseline drift is detected. - -#### Phase 2 — Security-first remediation (PR-1) +## Phase 2: Backend Service Stabilization Scope: -- `go/log-injection` units `HR-001`..`HR-075` -- `js/regex/missing-regexp-anchor` units `HR-013`..`HR-018` -- `js/insecure-temporary-file` units `HR-019`..`HR-021` +- `backend/internal/services/certificate_service.go` Tasks: -1. Patch backend log sinks file-by-file using consistent sanitization helper policy. -2. Patch regex patterns in affected test/component files with anchors. -3. Patch temp-file helpers in `tests/fixtures/auth-fixtures.ts`. -4. Run targeted tests after each module group to isolate regressions. -5. Re-run freshness gate before merge to ensure matrix parity. +1. Make constructor initialization deterministic at first-use boundary. +2. Remove startup timing dependence that allows early `ListCertificates` to overlap with constructor-initiated sync mutation. +3. Preserve current cache invalidation and DB refresh semantics. +4. Keep lock lifecycle simple and reviewable. -#### Phase 3 — Quality cleanup (PR-2) +Complexity: **Medium** (single component, concurrency-sensitive) + +## Phase 3: Backend Test Hardening Scope: -- 100 current open findings (`js/unused-local-variable`, `js/automatic-semicolon-insertion`, `js/comparison-between-incompatible-types`) using Matrix B ownership rows. +- `backend/internal/api/handlers/certificate_handler_coverage_test.go` +- `backend/internal/api/handlers/certificate_handler_test.go` +- `backend/internal/api/handlers/certificate_handler_security_test.go` +- optional shared test helpers: + - `backend/internal/api/handlers/testdb.go` Tasks: -1. Remove unused vars/imports by directory cluster (`tests/utils`, `tests/security*`, `tests/integration*`, `tests/settings*`, etc.). -2. Resolve ASI findings in: - - `tests/core/navigation.spec.ts` - - `frontend/src/pages/__tests__/ProxyHosts-bulk-acl.test.tsx` -3. Resolve type comparison warning in: - - `frontend/src/components/CredentialManager.tsx` -4. Record dispositions for any non-fixed findings. +1. Remove sleep-based or timing-dependent assumptions. +2. Standardize deterministic service setup helper for handler tests. +3. Ensure flaky case (`TestCertificateHandler_List_WithCertificates`) is fully deterministic. +4. Preserve assertion intent and API-level behavior checks. +5. Add explicit setup-ordering validation in tests to guarantee required schema migration/setup completes before handler/service invocation. -#### Phase 4 — Hygiene and scanner hardening (PR-3) +Complexity: **Medium** (many callsites, low logic risk) + +## Phase 4: Integration & Validation Tasks: -1. Normalize `.gitignore`/`.dockerignore` scan artifact handling and remove duplication. -2. Select one canonical Codecov config path and deprecate the other. -3. Normalize scan task outputs in `.vscode/tasks.json` and `scripts/pre-commit-hooks/` if required. -4. Re-run freshness gate before merge to confirm no PR #718 drift. +1. Run reproducible stability stress loop for the known flaky case via task ID `shell: Test: Backend Flaky - Certificate List Stability Loop`. + - **Task command payload:** + - `mkdir -p test-results/flaky && cd /projects/Charon && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_List_WithCertificates$' -count=100 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-list-stability.jsonl` + - **Artifactized logging requirement:** persist `test-results/flaky/cert-list-stability.jsonl` for reproducibility and CI comparison. + - **Pass threshold:** `100/100` successful runs, zero failures. +2. Run race-mode stress for the same path via task ID `shell: Test: Backend Flaky - Certificate List Race Loop`. + - **Task command payload:** + - `mkdir -p test-results/flaky && cd /projects/Charon && go test -race ./backend/internal/api/handlers -run '^TestCertificateHandler_List_WithCertificates$' -count=30 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-list-race.jsonl` + - **Artifactized logging requirement:** persist `test-results/flaky/cert-list-race.jsonl`. + - **Pass threshold:** exit code `0` and zero `WARNING: DATA RACE` occurrences. +3. Run setup-ordering validation loop via task ID `shell: Test: Backend Flaky - Certificate DB Setup Ordering Loop`. + - **Task command payload:** + - `mkdir -p test-results/flaky && cd /projects/Charon && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_DBSetupOrdering' -count=50 -shuffle=on -parallel=8 -json 2>&1 | tee test-results/flaky/cert-db-setup-ordering.jsonl` + - **Pass threshold:** `50/50` successful runs and zero `no such table: ssl_certificates|proxy_hosts` messages from positive-path setup runs. +4. Run focused certificate handler regression suite via task ID `shell: Test: Backend Flaky - Certificate Handler Focused Regression`. + - **Task command payload:** + - `mkdir -p test-results/flaky && cd /projects/Charon && go test ./backend/internal/api/handlers -run '^TestCertificateHandler_' -count=1 -json 2>&1 | tee test-results/flaky/cert-handler-regression.jsonl` + - **Pass threshold:** all selected tests pass in one clean run. +5. Execute patch-coverage preflight via existing project task ID `shell: Test: Local Patch Report`. + - **Task command payload:** `bash scripts/local-patch-report.sh` + - **Pass threshold:** both artifacts exist: `test-results/local-patch-report.md` and `test-results/local-patch-report.json`. -#### Phase 5 — Final verification and closure (post PR-3) +Task definition status for Phase 4 gates: + +- Existing task ID in `.vscode/tasks.json`: + - `shell: Test: Local Patch Report` +- New task IDs to add in `.vscode/tasks.json` with the exact command payloads above: + - `shell: Test: Backend Flaky - Certificate List Stability Loop` + - `shell: Test: Backend Flaky - Certificate List Race Loop` + - `shell: Test: Backend Flaky - Certificate DB Setup Ordering Loop` + - `shell: Test: Backend Flaky - Certificate Handler Focused Regression` + +Complexity: **Low-Medium** + +## Phase 5: Documentation & Operational Hygiene Tasks: -1. Run E2E-first verification path. -2. If runtime inputs changed (`backend/**`, `frontend/**`, `go.mod`, `go.sum`, `package.json`, `package-lock.json`, `Dockerfile`, `.docker/**`, compose files), rebuild E2E environment before running Playwright. -3. Run CodeQL Go/JS scans and validate zero high/critical findings. -4. Run coverage gates and type checks. -5. Confirm no SARIF/db artifacts are accidentally committed. -6. Update remediation report with before/after counts and close PR #718 checklist. +1. Update `docs/plans/current_spec.md` (this file) if implementation details evolve. +2. Record final decision outcomes and any deferred cleanup tasks. +3. Defer unrelated repository config hygiene (`.gitignore`, `codecov.yml`, `.dockerignore`, `Dockerfile`) unless a direct causal link to this flaky test is proven during implementation. -### Phase-to-PR Merge Dependency Contract +Complexity: **Low** -1. PR-1 cannot open until Phase 1 baseline and freshness gate pass. -2. PR-2 cannot merge until PR-1 merges and a fresh alert snapshot confirms no drift. -3. PR-3 cannot merge until PR-1 and PR-2 both merge and freshness gate passes again. -4. Phase 5 closure is blocked until all three PRs are merged and disposition log is complete. +--- -### PR Slicing Strategy +### 6) PR Slicing Strategy -#### Decision +### Decision -Use **three PRs** (minimum safe split). Single-PR delivery is rejected due to: +**Primary decision: single PR** for this fix. -- cross-domain blast radius (backend + frontend + test infra + CI hygiene), -- security-critical codepaths, -- reviewer load and rollback risk. +Why: -#### PR-1 — Security remediations only (high risk) +- Scope is tightly coupled (constructor semantics + related tests). +- Minimizes context switching and user review requests. +- Reduces risk of partially landed concurrency behavior. + +### Trigger reasons to split into multiple PRs + +Split only if one or more occur: + +- Constructor change causes wider runtime behavior concerns requiring staged rollout. +- Test hardening touches many unrelated test modules beyond certificate handlers. +- Validation reveals additional root-cause branches outside certificate service/test setup ordering. + +### Ordered fallback slices (if split is required) + +#### PR-1: Service Determinism Core Scope: -- Backend `go/log-injection` hotspots (`HR-001`..`HR-075`) -- Frontend/test security hotspots (`HR-013`..`HR-021`) - -Primary files: - -- `backend/internal/api/handlers/*` -- `backend/internal/api/middleware/emergency.go` -- `backend/internal/cerberus/*` -- `backend/internal/crowdsec/*` -- `backend/internal/server/emergency_server.go` -- `backend/internal/services/*` -- `tests/fixtures/auth-fixtures.ts` -- `tests/tasks/import-caddyfile.spec.ts` -- `tests/security-enforcement/zzz-caddy-imports/caddy-import-cross-browser.spec.ts` -- `frontend/src/components/__tests__/SecurityHeaderProfileForm.test.tsx` -- `frontend/src/pages/__tests__/ProxyHosts-progress.test.tsx` +- `backend/internal/services/certificate_service.go` Dependencies: -- Phase 1 baseline freeze and freshness gate must be complete. +- none Acceptance criteria: -1. No remaining `go/log-injection`, `js/regex/missing-regexp-anchor`, `js/insecure-temporary-file` findings in fresh scan. -2. Targeted tests pass for modified suites. -3. No behavior regressions in emergency/security control flows. +- service constructor/list path no longer exhibits startup race in targeted race tests +- no public API break at route wiring callsite -Rollback: +Rollback/contingency: -- Revert by module batch (handlers, services, crowdsec, tests) to isolate regressions. +- revert constructor change only; preserve tests unchanged for quick recovery -#### PR-2 — Open alert cleanup (quality/non-blocking) +#### PR-2: Handler Test Determinism Scope: -- `js/unused-local-variable` (95) -- `js/automatic-semicolon-insertion` (4) -- `js/comparison-between-incompatible-types` (1) +- `backend/internal/api/handlers/certificate_handler_coverage_test.go` +- `backend/internal/api/handlers/certificate_handler_test.go` +- `backend/internal/api/handlers/certificate_handler_security_test.go` +- optional helper consolidation in `backend/internal/api/handlers/testdb.go` Dependencies: -- PR-1 merged (required). +- PR-1 merged (preferred) Acceptance criteria: -1. `codeql-results-js.sarif` shows zero of the three rules above. -2. `npm run lint` and `npm run type-check` pass. -3. Playwright/Vitest suites touched by cleanup pass. +- `TestCertificateHandler_List_WithCertificates` stable across repeated CI-style runs +- no `time.Sleep` timing guard required for constructor race avoidance -Rollback: +Rollback/contingency: -- Revert by directory cluster commits (`tests/utils`, `tests/security*`, etc.). +- revert only test refactor while keeping stable service change -#### PR-3 — Hygiene and scanner hardening +--- -Scope: +### 7) Acceptance Criteria (Definition of Done) -- `.gitignore` -- `.dockerignore` -- `codecov.yml` -- `.codecov.yml` -- Optional: normalize scan task outputs in `.vscode/tasks.json` and `scripts/pre-commit-hooks/` +1. Stability gate: task ID `shell: Test: Backend Flaky - Certificate List Stability Loop` passes `100/100`. +2. Race gate: task ID `shell: Test: Backend Flaky - Certificate List Race Loop` passes with zero race reports. +3. Setup-ordering gate: task ID `shell: Test: Backend Flaky - Certificate DB Setup Ordering Loop` passes `50/50`, with no `no such table: ssl_certificates|proxy_hosts` in positive-path setup runs. +4. Regression gate: task ID `shell: Test: Backend Flaky - Certificate Handler Focused Regression` passes. +5. Playwright baseline gate is completed via task ID `shell: Test: E2E Playwright (FireFox) - Core: Certificates` (suite: `tests/core/certificates.spec.ts`), with task ID `shell: Docker: Rebuild E2E Environment` run first only when rebuild criteria are met. +6. Patch coverage preflight task ID `shell: Test: Local Patch Report` generates `test-results/local-patch-report.md` and `test-results/local-patch-report.json`. +7. Scope gate: no changes to `.gitignore`, `codecov.yml`, `.dockerignore`, or `Dockerfile` unless directly required to fix this flaky test. +8. Reproducibility gate: stress-loop outputs are artifactized under `test-results/flaky/` and retained in PR evidence (`cert-list-stability.jsonl`, `cert-list-race.jsonl`, `cert-db-setup-ordering.jsonl`, `cert-handler-regression.jsonl`). +9. If any validation fails, failure evidence and explicit follow-up tasks are recorded before completion. -Dependencies: +--- -- PR-1 and PR-2 complete. +### 8) Risks and Mitigations -Acceptance criteria: +| Risk | Impact | Mitigation | +|---|---|---| +| Constructor behavior change impacts startup timing | medium | keep API stable; run targeted route/handler regression tests | +| Over-fixing spreads beyond certificate scope | medium | constrain edits to service + certificate tests only | +| DB setup-ordering fixes accidentally mask true migration problems | medium | fail fast during setup with explicit diagnostics and dedicated setup-ordering tests | +| Hidden race persists in adjacent tests | medium | run repeated/race-targeted suite; expand only if evidence requires | +| Out-of-scope config churn creates review noise | low | explicitly defer unrelated config hygiene from this PR | -1. No duplicate/contradictory ignore patterns that mask source or commit scan artifacts unexpectedly. -2. Single canonical Codecov config path selected (either keep `codecov.yml` and deprecate `.codecov.yml`, or vice-versa). -3. Docker context excludes scan/report artifacts but preserves required runtime/build inputs. +--- -Rollback: +### 9) Handoff -- Revert config-only commit; no application runtime risk. +After this plan is approved: -### PR-3 Addendum — `js/insecure-temporary-file` in auth token cache - -#### Scope and intent - -This addendum defines the concrete remediation plan for the CodeQL `js/insecure-temporary-file` pattern in `tests/fixtures/auth-fixtures.ts`, focused on token cache logic that currently persists refreshed auth tokens to temporary files (`token.lock`, `token.json`) under OS temp storage. - -#### 1) Root cause analysis - -- The fixture stores bearer tokens on disk in a temp location, which is unnecessary for test execution and increases secret exposure risk. -- Even with restrictive permissions and lock semantics, the pattern still relies on filesystem primitives in a shared temp namespace and is flagged as insecure temporary-file usage. -- The lock/cache design uses predictable filenames (`token.lock`, `token.json`) and file lifecycle management; this creates avoidable risk and complexity for what is effectively process-local test state. -- The vulnerability is in the storage approach, not only in file flags/permissions; therefore suppression is not an acceptable fix. - -#### 2) Recommended proper fix (no suppression) - -- Replace file-based token cache + lock with an in-memory cache guarded by an async mutex/serialization helper. -- Keep existing behavior contract intact: - - cached token reuse while valid, - - refresh when inside threshold, - - safe concurrent calls to `refreshTokenIfNeeded`. -- Remove all temp-directory/file operations from the token-cache path. -- Preserve JWT expiry extraction and fallback behavior when refresh fails. - -Design target: - -- `TokenCache` remains as a module-level in-memory object. -- Introduce a module-level promise-queue lock helper (single-writer section) to serialize read/update operations. -- `readTokenCache` / `saveTokenCache` become in-memory helpers only. - -#### 3) Exact files/functions to edit - -- `tests/fixtures/auth-fixtures.ts` - - Remove/replace file-based helpers: - - `getTokenCacheFilePath` - - `getTokenLockFilePath` - - `cleanupTokenCacheDir` - - `ensureCacheDir` - - `acquireLock` - - Refactor: - - `readTokenCache` (memory-backed) - - `saveTokenCache` (memory-backed) - - `refreshTokenIfNeeded` (use in-memory lock path; no filesystem writes) - - Remove unused imports/constants tied to temp files (`fs`, `path`, `os`, lock/cache file constants). - -- `tests/fixtures/token-refresh-validation.spec.ts` - - Update concurrency test intent text from file-lock semantics to in-memory serialized access semantics. - - Keep behavioral assertions (valid token, no corruption/no throw under concurrent refresh requests). - -- `docs/reports/pr718_open_alerts_freshness_.md` (or latest freshness report in `docs/reports/`) - - Add a PR-3 note that the insecure temp-file finding for auth-fixtures moved to memory-backed token caching and is expected to close in next scan. - -#### 4) Acceptance criteria - -- CodeQL JavaScript scan reports zero `js/insecure-temporary-file` findings for `tests/fixtures/auth-fixtures.ts`. -- No auth token artifacts (`token.json`, `token.lock`, or `charon-test-token-cache-*`) are created by token refresh tests. -- `refreshTokenIfNeeded` still supports concurrent calls without token corruption or unhandled errors. -- `tests/fixtures/token-refresh-validation.spec.ts` passes in targeted execution. -- No regression to authentication fixture consumers using `refreshTokenIfNeeded`. - -#### 5) Targeted verification commands (no full E2E suite) - -- Targeted fixture tests: - - `cd /projects/Charon && npx playwright test tests/fixtures/token-refresh-validation.spec.ts --project=firefox` - -- Targeted static check for removed temp-file pattern: - - `cd /projects/Charon && rg "tmpdir\(|token\.lock|token\.json|mkdtemp" tests/fixtures/auth-fixtures.ts` - -- Targeted JS security scan (CI-aligned task): - - VS Code task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` - - or CLI equivalent: `cd /projects/Charon && pre-commit run --hook-stage manual codeql-js-scan --all-files` - -- Targeted freshness evidence generation: - - `cd /projects/Charon && ls -1t docs/reports/pr718_open_alerts_freshness_*.md | head -n 1` - -#### 6) PR-3 documentation/report updates required - -- Keep this addendum in `docs/plans/current_spec.md` as the planning source of truth for the token-cache remediation. -- Update the latest PR-3 freshness report in `docs/reports/` to include: - - finding scope (`js/insecure-temporary-file`, auth fixture token cache), - - remediation approach (memory-backed cache, no disk token persistence), - - verification evidence references (targeted Playwright + CodeQL JS scan). -- If PR-3 has a dedicated summary report, include a short “Security Remediation Delta” subsection with before/after status for this rule. - -### Configuration Review and Suggested Updates - -#### `.gitignore` - -Observed issues: - -- Duplicated patterns (`backend/main`, `codeql-linux64.zip`, `.docker/compose/docker-compose.test.yml` repeated). -- Broad ignores (`*.sarif`) acceptable, but duplicate SARIF patterns increase maintenance noise. -- Multiple planning/docs ignore entries may hide useful artifacts accidentally. - -Suggested updates: - -1. Deduplicate repeated entries. -2. Keep one CodeQL artifact block with canonical patterns. -3. Keep explicit allow-list comments for intentionally tracked plan/report docs. - -#### `.dockerignore` - -Observed issues: - -- Broad `*.md` exclusion with exceptions is valid, but easy to break when docs are needed during build metadata steps. -- Both `codecov.yml` and `.codecov.yml` ignored (good), but duplicate conceptual config handling elsewhere remains. - -Suggested updates: - -1. Keep current exclusions for scan artifacts (`*.sarif`, `codeql-db*`). -2. Add explicit comment that only runtime-required docs are whitelisted (`README.md`, `CONTRIBUTING.md`, `LICENSE`). -3. Validate no required frontend/backend build file is accidentally excluded when adding new tooling. - -#### `codecov.yml` and `.codecov.yml` - -Observed issues: - -- Two active Codecov configs create ambiguity. -- `codecov.yml` is richer and appears primary; `.codecov.yml` may be legacy overlap. - -Suggested updates: - -1. Choose one canonical config (recommended: `codecov.yml`). -2. Remove or archive `.codecov.yml` to avoid precedence confusion. -3. Ensure ignore patterns align with real source ownership and avoid suppressing legitimate production code coverage. - -#### `Dockerfile` - -Observed issues relative to CodeQL remediation scope: - -- Large and security-focused already; no direct blocker for PR #718 findings. -- Potentially excessive complexity for fallback build paths can hinder deterministic scanning/debugging. - -Suggested updates (non-blocking, PR-3 backlog): - -1. Add a short “security patch policy” comment block for dependency pin rationale consistency. -2. Add CI check to verify `CADDY_VERSION`, `CROWDSEC_VERSION`, and pinned Go/node versions are in expected policy ranges. -3. Keep build deterministic and avoid hidden side-effects in fallback branches. - -### Validation Strategy - -Execution order (required): - -1. E2E Playwright targeted suites for touched areas. -2. Local patch coverage report generation. -3. CodeQL Go + JS scans (CI-aligned). -4. Pre-commit fast hooks. -5. Backend/frontend coverage checks. -6. TypeScript type-check. - -Success gates: - -- Zero high/critical security findings. -- No regression in emergency/security workflow behavior. -- Codecov thresholds remain green. - -### Acceptance Criteria - -1. DoD checks complete without errors. -2. PR #718 high-risk findings remediated and verified. -3. Current open PR #718 findings remediated and verified. -4. Config hardening updates approved and merged. -5. Post-remediation evidence published in `docs/reports/` with before/after counts. - -### Risks and Mitigations - -- Risk: over-sanitizing logs reduces operational diagnostics. - - Mitigation: preserve key context with safe quoting/sanitization and structured fields. -- Risk: regex anchor changes break tests with dynamic URLs. - - Mitigation: update patterns with explicit optional groups and escape strategies. -- Risk: temp-file hardening affects test parallelism. - - Mitigation: per-test unique temp dirs and teardown guards. -- Risk: cleanup PR introduces noisy churn. - - Mitigation: file-cluster commits + narrow CI checks per cluster. - -### Handoff - -After user approval of this plan: - -1. Execute PR-1 (security) first. -2. Execute PR-2 (quality/open findings) second. -3. Execute PR-3 (hygiene/config hardening) third. -4. Submit final supervisor review with linked evidence and closure checklist. - -## Patch-Coverage Uplift Addendum (CodeQL Remediation Branch) - -### Scope - -Input baseline (`docs/plans/codecove_patch_report.md`): 18 uncovered patch lines across 9 backend files. - -Goal: close uncovered branches with minimal, branch-specific tests only (no broad refactors). - -### 1) Exact test files to add/update - -- Update `backend/internal/api/handlers/emergency_handler_test.go` -- Update `backend/internal/api/handlers/proxy_host_handler_update_test.go` -- Update `backend/internal/crowdsec/hub_sync_test.go` -- Update `backend/internal/api/handlers/crowdsec_pull_apply_integration_test.go` -- Update `backend/internal/services/backup_service_wave3_test.go` -- Update `backend/internal/services/uptime_service_unit_test.go` -- Update `backend/internal/api/middleware/emergency_test.go` -- Update `backend/internal/cerberus/cerberus_middleware_test.go` -- Update `backend/internal/crowdsec/hub_cache_test.go` - -### 2) Minimal branch-execution scenarios - -#### `backend/internal/api/handlers/emergency_handler.go` (3 lines) -- Add middleware-prevalidated reset test: set `emergency_bypass=true` in context and assert `SecurityReset` takes middleware path and returns success. -- Add reset failure-path test: force module-disable failure (closed DB/failed upsert) and assert HTTP 500 path executes. - -#### `backend/internal/api/handlers/proxy_host_handler.go` (3 lines) -- Add update payload case with `security_header_profile_id` as valid string to execute string-conversion success path. -- Add update payload case with invalid string to execute string parse failure branch. -- Add update payload case with unsupported type (boolean/object) to execute unsupported-type branch. - -#### `backend/internal/crowdsec/hub_sync.go` (3 lines) -- Add apply scenario where cache metadata exists but archive read fails, forcing refresh path and post-refresh archive read. -- Add fallback fetch scenario with first endpoint returning fallback-eligible error, second endpoint success. -- Add fallback-stop scenario with non-fallback error to execute early break path. - -#### `backend/internal/api/handlers/crowdsec_handler.go` (2 lines) -- Add apply test where cached meta exists but archive/preview file stat fails to execute missing-file log branches before apply. -- Add pull/apply branch case that exercises cache-miss diagnostics and response payload path. - -#### `backend/internal/services/backup_service.go` (2 lines) -- Add unzip-with-skip test with oversized decompressed entry to execute decompression-limit rejection branch. -- Add unzip-with-skip error-path test that validates extraction abort handling for invalid archive entry flow. - -#### `backend/internal/services/uptime_service.go` (2 lines) -- Add `CreateMonitor` test with `interval<=0` and `max_retries<=0` to execute defaulting branches. -- Add TCP monitor validation case with invalid `host:port` input to execute TCP validation error path. - -#### `backend/internal/api/middleware/emergency.go` (1 line) -- Add malformed client IP test (`RemoteAddr` unparsable) with token present to execute invalid-IP branch and confirm bypass is not set. - -#### `backend/internal/cerberus/cerberus.go` (1 line) -- Add middleware test with `emergency_bypass=true` in gin context and ACL enabled to execute bypass short-circuit branch. - -#### `backend/internal/crowdsec/hub_cache.go` (1 line) -- Add cache-load test that causes non-ENOENT metadata read failure (e.g., invalid metadata path state) to execute hard read-error branch (not `ErrCacheMiss`). - -### 3) Verification commands (targeted + patch report) - -Run targeted backend tests only: - -```bash -cd /projects/Charon -go test ./backend/internal/api/handlers -run 'TestEmergency|TestProxyHostUpdate|TestPullThenApply|TestApplyWithoutPull|TestApplyRollbackWhenCacheMissingAndRepullFails' -go test ./backend/internal/crowdsec -run 'TestPull|TestApply|TestFetchWith|TestHubCache' -go test ./backend/internal/services -run 'TestBackupService_UnzipWithSkip|TestCreateMonitor|TestUpdateMonitor|TestDeleteMonitor' -go test ./backend/internal/api/middleware -run 'TestEmergencyBypass' -go test ./backend/internal/cerberus -run 'TestMiddleware_' -``` - -Generate local patch coverage report artifacts: - -```bash -cd /projects/Charon -bash scripts/local-patch-report.sh -``` - -Expected artifacts: -- `test-results/local-patch-report.md` -- `test-results/local-patch-report.json` - -### 4) Acceptance criteria - -- Patch coverage increases from `79.31034%` to `>= 90%` for this remediation branch. -- Missing patch lines decrease from `18` to `<= 6` (target `0` if all branches are feasibly testable). -- All nine listed backend files show reduced missing-line counts in local patch report output. -- Targeted test commands pass with zero failures. +1. Delegate execution to Supervisor/implementation agent with this spec as the source of truth. +2. Execute phases in order with validation gates between phases. +3. Keep PR scope narrow and deterministic; prefer single PR unless split triggers are hit. diff --git a/docs/reports/qa_report.md b/docs/reports/qa_report.md index 6928b9ac..57fa6883 100644 --- a/docs/reports/qa_report.md +++ b/docs/reports/qa_report.md @@ -1,656 +1,170 @@ ---- -post_title: "Definition of Done QA Report" -post_slug: "definition-of-done-qa-report-2026-02-10" -featured_image: "https://wikid82.github.io/charon/assets/images/featured/charon.png" -categories: - - testing - - security - - ci -tags: - - coverage - - lint - - codeql - - trivy - - grype -summary: "Definition of Done validation results, including coverage, security scans, linting, and pre-commit checks." -post_date: "2026-02-10" ---- +# QA/Security Validation Report - Flaky Certificate Test Fix -## PR-3 Closure Audit (Config/Docs Hygiene Slice) - 2026-02-18 +**Date:** 2026-02-19 +**Scope:** Validation/audit gates for flaky-test fix in certificate handler/service paths. -### Scope and Constraints +## Gate Summary -- Scope: config/docs hygiene only (ignore/canonicalization/freshness artifacts). -- User directive honored: full local Playwright E2E was not run; complete E2E deferred to CI. +| Gate | Status | Evidence | +|---|---|---| +| 1) Playwright E2E certificates gate | **PASS** | Task: `Test: E2E Playwright (FireFox) - Core: Certificates`; status file: `test-results/.last-run.json` (`passed`) | +| 1b) Durable flaky artifacts in `test-results/flaky/` | **PASS** | `cert-list-stability.jsonl`, `cert-list-race.jsonl`, `cert-db-setup-ordering.jsonl`, `cert-handler-regression.jsonl` | +| 2) Local patch preflight artifacts present | **PASS (warn-mode)** | `test-results/local-patch-report.md`, `test-results/local-patch-report.json` | +| 3) Backend coverage gate >=85% | **PASS** | `test-backend-coverage` rerun with valid `CHARON_ENCRYPTION_KEY`; line coverage `87.3%`, statements `87.0%` | +| 4) Pre-commit all files | **PASS** | Task: `Lint: Pre-commit (All Files)` -> all hooks passed | +| 5a) Trivy filesystem scan | **PASS** | Task: `Security: Trivy Scan` -> 0 vulnerabilities, 0 secrets | +| 5b) Docker image scan | **FAIL** | Task: `Security: Scan Docker Image (Local)` -> 1 High, 9 Medium, 1 Low | +| 5c) CodeQL Go CI-aligned | **PASS** | Task: `Security: CodeQL Go Scan (CI-Aligned) [~60s]` completed | +| 5d) CodeQL JS CI-aligned | **PASS** | Task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` completed | +| 5e) CodeQL high/critical findings gate | **PASS** | `pre-commit run --hook-stage manual codeql-check-findings --all-files` | +| 6) Lint/type checks relevant to scope | **PASS** | `Lint: Staticcheck (Fast)` passed; `Lint: TypeScript Check` passed | +| 7) Flaky loop thresholds from plan | **PASS** | stability=100, race=30, dbordering=50, raceWarnings=0, noSuchTable=0 | -### Commands Run and Outcomes +## Detailed Evidence -1. `git status --short` - - Result: shows docs/report artifacts plus config changes (`.codecov.yml` deleted in working tree, `codecov.yml` modified). -2. `git diff --name-only | grep -E '^(backend/|frontend/|Dockerfile$|\.docker/|scripts/.*\.sh$|go\.mod$|go\.sum$|package\.json$|package-lock\.json$)' || true` - - Result: no output (no runtime-impacting paths in current unstaged diff). -3. `bash scripts/ci/check-codeql-parity.sh` - - Result: **PASS** (`CodeQL parity check passed ...`). -4. `bash scripts/pr718-freshness-gate.sh` - - Result: **PASS**; generated: - - `docs/reports/pr718_open_alerts_freshness_20260218T163918Z.json` - - `docs/reports/pr718_open_alerts_freshness_20260218T163918Z.md` -5. `pre-commit run check-yaml --files codecov.yml` - - Result: **PASS**. -6. `pre-commit run --files .dockerignore codecov.yml docs/reports/pr3_hygiene_scanner_hardening_2026-02-18.md docs/reports/pr718_open_alerts_baseline.json docs/reports/pr718_open_alerts_freshness_20260218T163918Z.json docs/reports/pr718_open_alerts_freshness_20260218T163918Z.md` - - Result: **PASS** (applicable hooks passed; non-applicable hooks skipped). -7. `grep -n '^codecov\.yml$' .dockerignore` - - Result: canonical entry present. -8. `python3` SARIF summary (`codeql-results-go.sarif`, `codeql-results-js.sarif`) - - Result: `total=0 error=0 warning=0 note=0` for both artifacts. -9. `python3` freshness summary (`docs/reports/pr718_open_alerts_freshness_20260218T163918Z.json`) - - Result: `baseline_status=present`, `drift_status=no_drift`, `fresh_total=0`, `added=0`, `removed=0`. +### 1) Playwright Certificates Gate -### PR-3 Slice Verdict +- Executed task: `Test: E2E Playwright (FireFox) - Core: Certificates` +- Base URL: `http://127.0.0.1:8080` +- Result marker: `test-results/.last-run.json`: -- Config/docs formatting/lint hooks (relevant to touched files): **PASS**. -- CodeQL parity/freshness consistency and blocker regression check: **PASS**. -- Runtime-impacting changes introduced by this slice: **NONE DETECTED**. +```json +{ + "status": "passed", + "failedTests": [] +} +``` -**Final PR-3 slice status: PASS** +### 2) Local Patch Preflight -## Final Re-check After Blocker Fix - 2026-02-18 +- Executed task: `Test: Local Patch Report` +- Artifacts exist: + - `test-results/local-patch-report.md` + - `test-results/local-patch-report.json` +- Current mode: `warn` +- Warning recorded: missing frontend coverage input (`frontend/coverage/lcov.info`) -### Scope of This Re-check +### 3) Backend Coverage -- Objective: confirm blocker-fix status and publish final PASS/FAIL summary. -- Required minimum reruns executed: - - `shellcheck scripts/pre-commit-hooks/codeql-go-scan.sh scripts/ci/check-codeql-parity.sh` - - `pre-commit run --hook-stage manual codeql-check-findings --all-files` -- Additional confirmations executed for this final verdict: - - backend handler tests - - actionlint - - CodeQL parity guard script - - CodeQL Go/JS CI-aligned scan status +- Task invocation failed initially due missing `CHARON_ENCRYPTION_KEY`. +- Rerun with valid env key: + - `CHARON_ENCRYPTION_KEY="$(openssl rand -base64 32)" .github/skills/scripts/skill-runner.sh test-backend-coverage` +- Final result: + - `Coverage gate: PASS` + - `Line coverage: 87.3%` + - `Statement coverage: 87.0%` -### Final PASS/FAIL Summary +### 4) Pre-commit -- `shellcheck scripts/pre-commit-hooks/codeql-go-scan.sh scripts/ci/check-codeql-parity.sh` → **PASS** (`SHELLCHECK_OK`) -- `pre-commit run --hook-stage manual codeql-check-findings --all-files` → **PASS** (no HIGH/CRITICAL findings in Go or JS) -- `go test ./internal/api/handlers/...` → **PASS** (`ok .../internal/api/handlers`) -- `actionlint` → **PASS** (`ACTIONLINT_OK`) -- `bash scripts/ci/check-codeql-parity.sh` (from repo root) → **PASS** (`CodeQL parity check passed ...`) -- `Security: CodeQL Go Scan (CI-Aligned) [~60s]` task → **PASS** (task completed) -- `Security: CodeQL JS Scan (CI-Aligned) [~90s]` task → **PASS** (task completed) -- `npx playwright test tests/security-enforcement/zzz-caddy-imports/caddy-import-cross-browser.spec.ts --project=chromium --project=firefox --project=webkit` → **PASS** (`19 passed`, no `No tests found`) +- Executed task: `Lint: Pre-commit (All Files)` +- Result: all configured hooks passed, including: + - yaml checks + - shellcheck + - actionlint + - go vet + - golangci-lint fast + - frontend type check and lint fix hooks -### PR-1 Blocker Update (Playwright Test Discovery) +### 5) Security Scans -- Previous blocker: `No tests found` for `tests/security-enforcement/zzz-caddy-imports/caddy-import-cross-browser.spec.ts` when run with browser projects. -- Root cause: browser projects in `playwright.config.js` ignored `**/security-enforcement/**`, excluding this spec from chromium/firefox/webkit discovery. -- Resolution: browser project `testIgnore` was narrowed to continue excluding security-enforcement tests except this cross-browser import spec. -- Verification: reran the exact blocker command and it passed (`19 passed`, cross-browser execution succeeded). +#### Trivy Filesystem -### Accepted Risk Clarification - -- Accepted-risk identifier/path: `docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md` (`GHSA-69x3-g4r3-p962`, `github.com/slackhq/nebula@v1.9.7`). -- Why non-blocking: this High finding is a documented upstream dependency-chain exception (Caddy/CrowdSec bouncer → ipstore → nebula) with no currently compatible upstream fix path in Charon control. -- Next review trigger: re-open immediately when upstream Caddy dependency chain publishes compatible `nebula >= v1.10.3` support (or if advisory severity/exploitability materially changes). - -### Notes - -- A transient parity-script failure (`Missing workflow file: .github/workflows/codeql.yml`) occurred only when executed outside repo root context; root-context rerun passed and is the authoritative result. - -### Final Verdict - -**PASS** - -### Remaining Blockers - -- **None** for the requested blocker-fix re-check scope. - -## Current Branch QA/Security Audit - 2026-02-17 - -### Patch Coverage Push Handoff (Latest Local Report) - -- Source: `test-results/local-patch-report.json` -- Generated: `2026-02-17T18:40:46Z` -- Mode: **warn** +- Executed task: `Security: Trivy Scan` - Summary: - - Overall patch coverage: **85.4%** (threshold 90%) → **warn** - - Backend patch coverage: **85.1%** (threshold 85%) → **pass** - - Frontend patch coverage: **91.0%** (threshold 85%) → **pass** -- Current warn-mode trigger: - - Overall is below threshold by **4.6 points**; rollout remains non-blocking while artifacts are still required. -- Key files still needing patch coverage (highest handoff priority): - - `backend/internal/services/mail_service.go` — 20.8% patch coverage, 19 uncovered changed lines - - `frontend/src/pages/UsersPage.tsx` — 30.8% patch coverage, 9 uncovered changed lines - - `backend/internal/crowdsec/hub_sync.go` — 37.5% patch coverage, 10 uncovered changed lines - - `backend/internal/services/security_service.go` — 46.4% patch coverage, 15 uncovered changed lines - - `backend/internal/api/handlers/backup_handler.go` — 53.6% patch coverage, 26 uncovered changed lines - - `backend/internal/api/handlers/import_handler.go` — 67.5% patch coverage, 26 uncovered changed lines - - `backend/internal/api/handlers/settings_handler.go` — 73.6% patch coverage, 24 uncovered changed lines - - `backend/internal/util/permissions.go` — 74.4% patch coverage, 34 uncovered changed lines + - `backend/go.mod`: 0 vulnerabilities + - `frontend/package-lock.json`: 0 vulnerabilities + - `package-lock.json`: 0 vulnerabilities + - secrets: 0 -### 1) E2E Ordering Requirement and Evidence +#### Docker Image Scan (Grype via skill) -- Status: **FAIL (missing current-cycle evidence)** -- Requirement: E2E must run before unit coverage and local patch preflight. +- Executed task: `Security: Scan Docker Image (Local)` +- Artifacts generated: + - `sbom.cyclonedx.json` + - `grype-results.json` + - `grype-results.sarif` +- Summary from `grype-results.json`: + - High: 1 + - Medium: 9 + - Low: 1 + - Critical: 0 -### 2) Local Patch Preflight Artifacts (Presence + Validity) +#### CodeQL -- Artifacts present: - - `test-results/local-patch-report.json` -- Generated: `2026-02-17T18:40:46Z` -- Validity summary: - - Overall patch coverage: `85.4%` (**warn**, threshold `90%`) - - Backend patch coverage: `85.1%` (**pass**, threshold `85%`) - - Frontend patch coverage: `91.0%` (**pass**, threshold `85%`) +- Go CI-aligned task completed and generated `codeql-results-go.sarif`. +- JS CI-aligned task completed and generated `codeql-results-js.sarif`. +- Manual findings gate: + - `pre-commit run --hook-stage manual codeql-check-findings --all-files` + - result: no HIGH/CRITICAL findings in Go or JS. -### 3) Backend/Frontend Coverage Status and Thresholds +### 6) Linting/Type Checks -- Threshold baseline: **85% minimum** (project QA/testing instructions) -- Backend coverage (current artifact `backend/coverage.txt`): **87.0%** → **PASS** -### 4) Fast Lint / Pre-commit Status +- `Lint: Staticcheck (Fast)` -> `0 issues` +- `Lint: TypeScript Check` -> `tsc --noEmit` passed -- Status: **FAIL** -- Failing gate: `golangci-lint-fast` -- Current blocker categories from output: - - `unused`: unused helper functions in tests +### 7) Flaky-Specific Loop Artifacts and Thresholds -- **Go vulnerability scan (`security-scan-go-vuln`)**: **PASS** (`No vulnerabilities found`) -- **GORM security scan (`security-scan-gorm --check`)**: **PASS** (0 critical/high/medium; info-only suggestions) -- **CodeQL (CI-aligned via skill)**: **PASS (non-blocking)** - - Go SARIF: `5` results (non-error/non-warning categories in this run) - - JavaScript SARIF: `0` results -- **Trivy filesystem scan (`security-scan-trivy`)**: **FAIL** +- Artifacts in `test-results/flaky/`: + - `cert-list-stability.jsonl` + - `cert-list-race.jsonl` + - `cert-db-setup-ordering.jsonl` + - `cert-handler-regression.jsonl` -### 6) Merge-Readiness Summary (Blockers + Exact Next Commands) +- Measured thresholds: + - `stability=100` (expected 100) + - `race=30` (expected 30) + - `dbordering=50` (expected 50) + - `raceWarnings=0` + - `noSuchTable=0` -1. Missing E2E-first ordering evidence for this cycle. -2. Frontend coverage below threshold (`74.70% < 85%`). -3. Fast pre-commit/lint failing (`golangci-lint-fast`). -4. Security scans failing: - - Trivy filesystem scan - - Docker image scan (1 High vulnerability) -```bash -cd /projects/Charon && .github/skills/scripts/skill-runner.sh docker-rebuild-e2e -cd /projects/Charon && bash scripts/local-patch-report.sh -cd /projects/Charon && .github/skills/scripts/skill-runner.sh test-frontend-coverage -cd /projects/Charon && pre-commit run --all-files +## Filesystem vs Image Findings Comparison -cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-trivy vuln,secret,misconfig json -cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-docker-image -``` +- Filesystem scan (Trivy): **0 vulnerabilities**. +- Image scan (Grype): **11 vulnerabilities**. +- **Additional image-only vulnerabilities:** 11 -#### Re-check command set after fixes +Image-only findings: -```bash -cd /projects/Charon && npx playwright test --project=firefox -cd /projects/Charon && bash scripts/local-patch-report.sh -cd /projects/Charon && .github/skills/scripts/skill-runner.sh test-frontend-coverage -cd /projects/Charon && pre-commit run --all-files -cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-go-vuln -cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-gorm --check -cd /projects/Charon && .github/skills/scripts/skill-runner.sh security-scan-codeql all summary -``` +| Severity | ID | Package | Version | Fix | +|---|---|---|---|---| +| High | GHSA-69x3-g4r3-p962 | github.com/slackhq/nebula | v1.9.7 | 1.10.3 | +| Medium | CVE-2025-60876 | busybox | 1.37.0-r30 | N/A | +| Medium | CVE-2025-60876 | busybox-binsh | 1.37.0-r30 | N/A | +| Medium | CVE-2025-60876 | busybox-extras | 1.37.0-r30 | N/A | +| Medium | CVE-2025-60876 | ssl_client | 1.37.0-r30 | N/A | +| Medium | CVE-2025-14819 | curl | 8.17.0-r1 | N/A | +| Medium | CVE-2025-13034 | curl | 8.17.0-r1 | N/A | +| Medium | CVE-2025-14524 | curl | 8.17.0-r1 | N/A | +| Medium | CVE-2025-15079 | curl | 8.17.0-r1 | N/A | +| Medium | CVE-2025-14017 | curl | 8.17.0-r1 | N/A | +| Low | CVE-2025-15224 | curl | 8.17.0-r1 | N/A | -## Validation Checklist +## Failed Gates and Remediation -- Phase 1 - E2E Tests: PASS (provided: notification tests now pass) -- Phase 2 - Backend Coverage: PASS (92.0% statements) -- Phase 2 - Frontend Coverage: FAIL (lines 86.91%, statements 86.4%, functions 82.71%, branches 78.78%; min 88%) -- Phase 3 - Type Safety (Frontend): INCONCLUSIVE (task output did not confirm completion) -- Phase 4 - Pre-commit Hooks: INCONCLUSIVE (output truncated after shellcheck) -- Phase 5 - Trivy Filesystem Scan: INCONCLUSIVE (no vulnerabilities listed in artifacts) -- Phase 5 - Docker Image Scan: ACCEPTED RISK (1 High severity vulnerability; see [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md)) -- Phase 5 - CodeQL Go Scan: PASS (results array empty) -- Phase 5 - CodeQL JS Scan: PASS (results array empty) -- Phase 6 - Linters: FAIL (markdownlint and hadolint failures) +### Failed Gate: Security Docker Image Scan -## Coverage Results +- Failing evidence: image scan task ended with non-zero exit due vulnerability policy (`1 High`). +- Primary blocker: `GHSA-69x3-g4r3-p962` in `github.com/slackhq/nebula@v1.9.7` (fix `1.10.3`). -- Backend coverage: 92.0% statements (meets >=85%) -- Frontend coverage: lines 86.91%, statements 86.4%, functions 82.71%, branches 78.78% (below 88% gate) -- Evidence: [frontend/coverage.log](../../frontend/coverage.log) +Recommended remediation: -## Type Safety (Frontend) +1. Update dependency chain to a version resolving `nebula >= 1.10.3` (or update parent component that pins it). +2. Rebuild image and rerun: + - `Security: Scan Docker Image (Local)` + - `Security: Trivy Scan` +3. If immediate upgrade is not feasible, document/renew security exception with review date and compensating controls. -- Task: Lint: TypeScript Check -- Status: INCONCLUSIVE (output did not show completion or errors) +### Warning (Non-blocking for requested artifact-presence check): Local Patch Preflight -## Pre-commit Hooks (Fast) - - Exception: [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md) -- CodeQL Go scan: PASS (results array empty in [codeql-results-go.sarif](../../codeql-results-go.sarif)) -- CodeQL JS scan: PASS (results array empty in [codeql-results-js.sarif](../../codeql-results-js.sarif)) -- Trivy filesystem artifacts do not list vulnerabilities. -- Docker image scan found 1 High severity vulnerability (accepted risk; see [docs/security/SECURITY-EXCEPTION-nebula-v1.9.7.md](../security/SECURITY-EXCEPTION-nebula-v1.9.7.md)). -- Result: MISMATCH - Docker image scan reveals issues not surfaced by Trivy filesystem artifacts. +- Current warning: missing frontend coverage input `frontend/coverage/lcov.info`. +- Artifacts are present and valid for preflight evidence. -- Staticcheck (Fast): PASS -- Frontend ESLint: PASS (no errors reported in task output) +Recommended remediation: -## Blocking Issues and Remediation +1. Generate frontend coverage (`test-frontend-coverage`) to populate `frontend/coverage/lcov.info`. +2. Re-run `Test: Local Patch Report` to remove warn-mode status. -- Markdownlint failures in [tests/README.md](../../tests/README.md). Fix table spacing and re-run markdownlint. -- Hadolint failures (DL3059, SC2012). Consolidate consecutive RUN instructions and replace ls usage; re-run hadolint. -- TypeScript check and pre-commit status not confirmed. Re-run and capture final pass output. -## Verdict +## Final Verdict -CONDITIONAL -- This report is generated with accessibility in mind, but accessibility issues may still exist. Please review and test with tools such as Accessibility Insights. - -## Frontend Unit Coverage Push - 2026-02-16 - 2. `frontend/src/api/__tests__/import.test.ts` - 3. `frontend/src/api/__tests__/client.test.ts` - - - Before (securityHeaders + import): 100.00% - - After (securityHeaders + import): 100.00% - - Client focused after expansion: lines 100.00% (branches 90.9%) - -### Threshold Status - -- Frontend coverage minimum gate (85%): **FAIL for this execution run** (gate could not be conclusively evaluated from the required full approved run due unrelated suite failures/oom before final coverage gate output). -### Commands/Tasks Run - -- `/.github/skills/scripts/skill-runner.sh test-frontend-coverage` (baseline attempt) -- `cd frontend && npm run test:coverage -- src/api/__tests__/securityHeaders.test.ts src/api/__tests__/import.test.ts --run` (before) -- `cd frontend && npm run test:coverage -- src/api/__tests__/securityHeaders.test.ts src/api/__tests__/import.test.ts --run` (after) - -- `frontend/src/api/__tests__/securityHeaders.test.ts` - - Added UUID-path coverage for `getProfile` and explicit error-forwarding assertion for `listProfiles`. -- `frontend/src/api/__tests__/client.test.ts` - - Added interceptor branch coverage for non-object payload handling, `error` vs `message` precedence, non-401 auth-handler bypass, and fulfilled response passthrough. - - - Lines 42-49: `getProfile accepts UUID string identifiers` - - Lines 78-83: `forwards API errors from listProfiles` -- `frontend/src/api/__tests__/import.test.ts` - - Lines 40-46: `uploadCaddyfilesMulti accepts empty file arrays` - - Lines 81-86: `forwards commitImport errors` -- `frontend/src/api/__tests__/client.test.ts` - - Lines 173-195: `does not invoke auth error handler when status is not 401` - -### Blockers / Residual Risks - -- Full approved frontend coverage run currently fails for unrelated pre-existing tests and memory pressure: - - `src/pages/__tests__/ProxyHosts-extra.test.tsx` role-name mismatch - - Worker OOM during full-suite coverage execution -- As requested, no out-of-scope fixes were applied to those unrelated suites in this run. -- Threshold used for this run: `CHARON_MIN_COVERAGE=85`. - -### Exact Commands Run -- `cd /projects/Charon/frontend && npm run type-check` -- `cd /projects/Charon && /projects/Charon/.github/skills/scripts/skill-runner.sh qa-precommit-all` - -### Coverage Metrics - -- Baseline frontend lines %: `86.91%` (pre-existing baseline from prior full-suite run in this report) -- Final frontend lines %: `87.35%` (latest full gate execution) -- Net delta: `+0.44%` -- Threshold: `85%` - -### Full Unit Coverage Gate Status - -- Final full gate: **PASS** (`Coverage gate: PASS (lines 87.35% vs minimum 85%)`) - -### Quarantine/Fix Summary and Justification - - `src/components/__tests__/ProxyHostForm-dns.test.tsx` - - `src/pages/__tests__/Notifications.test.tsx` - - `src/pages/__tests__/ProxyHosts-coverage.test.tsx` - - `src/pages/__tests__/ProxyHosts-extra.test.tsx` - - `src/pages/__tests__/Security.functional.test.tsx` -- Justification: these suites reproduced pre-existing selector mismatches, timer timeouts, and worker instability/OOM under full coverage gate; quarantine was used only after reproducibility proof and scoped to unrelated suites. - -### Patch Coverage and Validation - -- Modified-line patch scope in this run is limited to test configuration/reporting updates; no production frontend logic changed. -- Full frontend unit coverage gate passed at policy threshold and existing API coverage additions remain intact. - -### Residual Risk and Follow-up - -- Residual risk: quarantined suites are temporarily excluded from full coverage runs and may mask regressions in those specific areas. -- Follow-up action: restore quarantined suites after stabilizing selectors/timer handling and addressing worker instability; remove temporary excludes in `frontend/vitest.config.ts` in the same remediation PR. - -## CI Encryption-Key Remediation Audit - 2026-02-17 - -### Scope Reviewed - -- `.github/workflows/quality-checks.yml` -- `.github/workflows/codecov-upload.yml` -- `scripts/go-test-coverage.sh` -- `scripts/ci/check-codecov-trigger-parity.sh` - -### Commands Executed and Outcomes - -1. **Required pre-commit fast hooks** - - Command: `cd /projects/Charon && pre-commit run --all-files` - - Result: **PASS** - - Notes: `check yaml`, `shellcheck`, `actionlint`, fast Go linters, and frontend checks all passed in this run. - -2. **Targeted workflow/script validation** - - Command: `cd /projects/Charon && python3 - <<'PY' ... yaml.safe_load(...) ... PY` - - Result: **PASS** (`quality-checks.yml`, `codecov-upload.yml` parsed successfully) - - Command: `cd /projects/Charon && actionlint .github/workflows/quality-checks.yml .github/workflows/codecov-upload.yml` - - Result: **PASS** - - Command: `cd /projects/Charon && bash -n scripts/go-test-coverage.sh scripts/ci/check-codecov-trigger-parity.sh` - - Result: **PASS** - - Command: `cd /projects/Charon && shellcheck scripts/go-test-coverage.sh scripts/ci/check-codecov-trigger-parity.sh` - - Result: **INFO finding** (SC2016 in expected-comment string), non-blocking under warning-level policy - - Command: `cd /projects/Charon && shellcheck -S warning scripts/go-test-coverage.sh scripts/ci/check-codecov-trigger-parity.sh` - - Result: **PASS** - - Command: `cd /projects/Charon && bash scripts/ci/check-codecov-trigger-parity.sh` - - Result: **PASS** (`Codecov trigger/comment parity check passed`) - -3. **Security scans feasible in this environment** - - Command (task): `Security: Go Vulnerability Check` - - Result: **PASS** (`No vulnerabilities found`) - - Command (task): `Security: CodeQL Go Scan (CI-Aligned) [~60s]` - - Result: **COMPLETED** (SARIF generated: `codeql-results-go.sarif`) - - Command (task): `Security: CodeQL JS Scan (CI-Aligned) [~90s]` - - Result: **COMPLETED** (SARIF generated: `codeql-results-js.sarif`) - - Command: `cd /projects/Charon && pre-commit run --hook-stage manual codeql-check-findings --all-files` - - Result: **PASS** (hook reported no HIGH/CRITICAL) - - Command (task): `Security: Scan Docker Image (Local)` - - Result: **FAIL** (1 High vulnerability, 0 Critical; GHSA-69x3-g4r3-p962 in `github.com/slackhq/nebula@v1.9.7`, fixed in 1.10.3) - - Command (MCP tool): Trivy filesystem scan via `mcp_trivy_mcp_scan_filesystem` - - Result: **NOT FEASIBLE LOCALLY** (tool returned `failed to scan project`) - - Nearest equivalent validation: CI-aligned CodeQL scans + Go vuln check + local Docker image SBOM/Grype scan task. - -4. **Coverage script encryption-key preflight validation** - - Command: `env -u CHARON_ENCRYPTION_KEY bash scripts/go-test-coverage.sh` - - Result: **PASS (expected failure path)** exit 1 with missing-key message - - Command: `CHARON_ENCRYPTION_KEY='@@not-base64@@' bash scripts/go-test-coverage.sh` - - Result: **PASS (expected failure path)** exit 1 with base64 validation message - - Command: `CHARON_ENCRYPTION_KEY='c2hvcnQ=' bash scripts/go-test-coverage.sh` - - Result: **PASS (expected failure path)** exit 1 with decoded-length validation message - - Command: `CHARON_ENCRYPTION_KEY="$(openssl rand -base64 32)" timeout 8 bash scripts/go-test-coverage.sh` - - Result: **PASS (preflight success path)** no preflight key error before timeout (exit 124 due test timeout guard) - -### Security Findings Snapshot - -- `codeql-results-js.sarif`: 0 results -- `codeql-results-go.sarif`: 5 results (`go/path-injection` x4, `go/cookie-secure-not-set` x1) -- `grype-results.json`: 1 High, 0 Critical - -### Residual Risks - -- Docker image scan currently reports one High severity vulnerability (GHSA-69x3-g4r3-p962). -- Trivy MCP filesystem scanner could not run in this environment; equivalent checks were used, but Trivy parity is not fully proven locally. -- CodeQL manual findings gate reported PASS while raw Go SARIF contains security-query results; this discrepancy should be reconciled in follow-up tooling validation. - -### QA Verdict (This Audit) - -- **NOT APPROVED** for security sign-off due unresolved High-severity vulnerability in local Docker image scan and unresolved scanner-parity discrepancy. -- **APPROVED** for functional remediation behavior of encryption-key preflight and anti-drift checks. - -## Focused Backend CI Failure Investigation (PR #666) - 2026-02-17 - -### Scope - -- Objective: reproduce failing backend CI tests locally with CI-parity commands and classify root cause. -- Workflow correlation targets: - - `.github/workflows/quality-checks.yml` → `backend-quality` job - - `.github/workflows/codecov-upload.yml` → `backend-codecov` job - -### CI Parity Observed - -- Both workflows resolve `CHARON_ENCRYPTION_KEY` before backend tests. -- Both workflows run backend coverage via: - - `CGO_ENABLED=1 bash scripts/go-test-coverage.sh 2>&1 | tee backend/test-output.txt` -- Local investigation mirrored these commands and environment expectations. - -### Encryption Key Trusted-Context Simulation - -- Command: `export CHARON_ENCRYPTION_KEY="$(openssl rand -base64 32)"` -- Validation: `charon_key_decoded_bytes=32` -- Classification: **not an encryption-key preflight failure** in this run. - -### Commands Executed and Outcomes - -1. **Coverage script (CI parity)** - - Command: `cd /projects/Charon && CGO_ENABLED=1 bash scripts/go-test-coverage.sh` - - Log: `docs/reports/artifacts/pr666-go-test-coverage.log` - - Result: **FAIL** - -2. **Verbose backend package sweep (requested)** - - Command: `cd /projects/Charon/backend && CGO_ENABLED=1 go test ./... -count=1 -v` - - Log: `docs/reports/artifacts/pr666-go-test-all-v.log` - - Result: **PASS** - -3. **Targeted reruns for failing areas (`-race -count=1 -v`)** - - `./internal/api/handlers` (package rerun): `docs/reports/artifacts/pr666-target-handlers-race.log` → **PASS** - - `./internal/crowdsec` (package rerun): `docs/reports/artifacts/pr666-target-crowdsec-race.log` → **PASS** - - `./internal/services` (package rerun): `docs/reports/artifacts/pr666-target-services-race.log` → **FAIL** - - Isolated test reruns: - - `./internal/api/handlers -run 'TestSecurityHandler_UpsertRuleSet_XSSInContent|TestSecurityHandler_UpsertDeleteTriggersApplyConfig'` → **FAIL** (`XSSInContent`), `ApplyConfig` pass - - `./internal/crowdsec -run 'TestHeartbeatPoller_ConcurrentSafety'` → **FAIL** (data race) - - `./internal/services -run 'TestSecurityService_LogAudit_ChannelFullFallsBackToSyncWrite|TestCredentialService_Delete'` → **FAIL** (`LogAudit...`), `CredentialService_Delete` pass in isolation - -### Exact Failing Tests (from coverage CI-parity run) - -- `TestSecurityHandler_UpsertRuleSet_XSSInContent` -- `TestSecurityHandler_UpsertDeleteTriggersApplyConfig` -- `TestHeartbeatPoller_ConcurrentSafety` -- `TestSecurityService_LogAudit_ChannelFullFallsBackToSyncWrite` -- `TestCredentialService_Delete` - -### Key Error Snippets - -- `TestSecurityHandler_UpsertRuleSet_XSSInContent` - - `expected: 200 actual: 500` - - `"{\"error\":\"failed to list rule sets\"}" does not contain "\\u003cscript\\u003e"` - -- `TestSecurityHandler_UpsertDeleteTriggersApplyConfig` - - `database table is locked` - - `timed out waiting for manager ApplyConfig /load post on delete` - -- `TestHeartbeatPoller_ConcurrentSafety` - - `WARNING: DATA RACE` - - `testing.go:1712: race detected during execution of test` - -- `TestSecurityService_LogAudit_ChannelFullFallsBackToSyncWrite` - - `no such table: security_audits` - - expected audit fallback marker `"sync-fallback"`, got empty value - -- `TestCredentialService_Delete` (coverage run) - - `database table is locked` - - Note: passes in isolated rerun, indicating contention/order sensitivity. - -### Failure Classification - -- **Encryption key preflight**: Not the cause (valid 32-byte base64 key verified). -- **Environment mismatch**: Not primary; same core commands as CI reproduced failures. -- **Flaky/contention-sensitive tests**: Present (`database table is locked`, timeout waiting for apply-config side-effect). -- **Real logic/concurrency regressions**: Present: - - Confirmed race in `TestHeartbeatPoller_ConcurrentSafety`. - - Deterministic missing-table failure in `TestSecurityService_LogAudit_ChannelFullFallsBackToSyncWrite`. - - Deterministic handler regression in `TestSecurityHandler_UpsertRuleSet_XSSInContent` under isolated rerun. - -### Most Probable Root Cause - -- Mixed failure mode dominated by **concurrency and test-isolation defects** in backend tests: - - race condition in heartbeat poller lifecycle, - - incomplete DB/migration setup assumptions in some tests, - - SQLite table-lock contention under broader coverage/race execution. - -### Minimal Proper Next Fix Recommendation - -1. **Fix race first (highest confidence, highest impact):** - - Guard `HeartbeatPoller` start/stop shared state with synchronization (mutex/atomic + single lifecycle transition). - -2. **Fix deterministic schema dependency in services test:** - - Ensure `security_audits` table migration/setup is guaranteed in `TestSecurityService_LogAudit_ChannelFullFallsBackToSyncWrite` before assertions. - -3. **Stabilize handler/service DB write contention:** - - Isolate SQLite DB per test (or serialized critical sections) for tests that perform concurrent writes and apply-config side effects. - -4. **Re-run CI-parity sequence after fixes:** - - `CGO_ENABLED=1 bash scripts/go-test-coverage.sh` - - `cd backend && CGO_ENABLED=1 go test ./... -count=1 -v` - -### Local Backend Status for PR #666 - -- **Overall investigation status: FAIL (reproduced backend CI-like failures locally).** - -## PR #666 CI-Only Backend Failure Deep Dive Addendum - 2026-02-17 - -### Exact CI Failure Evidence - -- Source: GitHub Actions run `22087372370`, job `63824895671` (`backend-quality`). -- Exact failing assertion extracted from job logs: - - `--- FAIL: TestFetchIndexFallbackHTTP` - - `open testdata/hub_index.json: no such file or directory` - -### CI-Parity Local Matrix Executed - -All commands were run from `/projects/Charon` or `/projects/Charon/backend` with a valid 32-byte base64 `CHARON_ENCRYPTION_KEY`. - -1. `bash scripts/go-test-coverage.sh` -2. `go test ./... -race -count=1 -shuffle=on -v` -3. `go test ./... -race -count=1 -shuffle=on -v -p 1` -4. `go test ./... -race -count=1 -shuffle=on -v -p 4` - -### Reproduction Outcomes - -- CI-specific missing fixture (`testdata/hub_index.json`) was confirmed in CI logs. -- Local targeted stress for the CI-failing test (`internal/crowdsec` `TestFetchIndexFallbackHTTP`) passed repeatedly (10/10). -- Full matrix runs repeatedly surfaced lock/closure instability outside the single CI assertion: - - `database table is locked` - - `sql: database is closed` -- Representative failing packages in parity reruns: - - `internal/api/handlers` - - `internal/config` - - `internal/services` - - `internal/caddy` (deterministic fallback-env-key test failure in local matrix) - -### Root Cause (Evidence-Based) - -Primary root cause is **test isolation breakdown under race+shuffle execution**, not encryption-key preflight: - -1. **SQLite cross-test contamination/contention** - - Shared DB state patterns caused row leakage and lock events under shuffled execution. - -2. **Process-level environment variable contamination** - - CrowdSec env-key tests depended on mutable global env without full reset, causing order-sensitive behavior. - -3. **Separate CI-only fixture-path issue** - - CI log shows missing `testdata/hub_index.json` for `TestFetchIndexFallbackHTTP`, which did not reproduce locally. - -### Low-Risk Fixes Applied During Investigation - -1. `backend/internal/api/handlers/notification_handler_test.go` - - Reworked test DB setup from shared in-memory sqlite to per-test sqlite file in `t.TempDir()` with WAL + busy timeout. - - Updated tests to call `setupNotificationTestDB(t)`. - -2. `backend/internal/api/handlers/crowdsec_bouncer_test.go` - - Hardened `TestGetBouncerAPIKeyFromEnv` to reset all supported env keys per subtest before setting case-specific values. - -3. `backend/internal/api/handlers/crowdsec_coverage_target_test.go` - - Added explicit reset of all relevant CrowdSec env keys in `TestGetLAPIKeyLookup`, `TestGetLAPIKeyEmpty`, and `TestGetLAPIKeyAlternative`. - -### Post-Fix Verification - -- Targeted suites stabilized after fixes: - - Notification handler list flake (row leakage) no longer reproduced in repeated stress loops. - - CrowdSec env-key tests remained stable in repeated shuffled runs. -- Broad matrix remained unstable with additional pre-existing failures (`sql: database is closed`/`database table is locked`) across multiple packages. - -### Final Parity Status - -- **Scoped fix validation**: PASS (targeted flaky tests stabilized). -- **Full CI-parity matrix**: FAIL (broader baseline instability remains; not fully resolved in this pass). - -## CodeQL Hardening Validation - 2026-02-18 - -### Scope - -- `.github/workflows/codeql.yml` -- `.vscode/tasks.json` -- `scripts/ci/check-codeql-parity.sh` -- `scripts/pre-commit-hooks/codeql-js-scan.sh` - -### Validation Results - -- `actionlint .github/workflows/codeql.yml` -> **PASS** (`ACTIONLINT_OK`) -- `shellcheck scripts/ci/check-codeql-parity.sh scripts/pre-commit-hooks/codeql-js-scan.sh` -> **PASS** (`SHELLCHECK_OK`) -- `bash scripts/ci/check-codeql-parity.sh` -> **PASS** (`CodeQL parity check passed ...`, `PARITY_OK`) -- `pre-commit run --hook-stage manual codeql-check-findings --all-files` -> **PASS** (`Block HIGH/CRITICAL CodeQL Findings...Passed`, `FINDINGS_GATE_OK`) - -### JS CI-Aligned Task Scope/Output Check - -- Task `Security: CodeQL JS Scan (CI-Aligned) [~90s]` in `.vscode/tasks.json` invokes `bash scripts/pre-commit-hooks/codeql-js-scan.sh` -> **PASS** -- Script uses `--source-root=.` so repository-wide JavaScript/TypeScript analysis scope includes `tests/` and other TS/JS paths, not only `frontend/` -> **PASS** -- Script SARIF output remains `--output=codeql-results-js.sarif` -> **PASS** - -### Overall Verdict - -- **PASS** - -### Blockers - -- **None** for this validation scope. - -## PR-3 Insecure Temporary File Remediation Gate (Targeted) - 2026-02-18 - -### Scope - -- `tests/fixtures/auth-fixtures.ts` -- `tests/fixtures/token-refresh-validation.spec.ts` -- `docs/reports/pr3_hygiene_scanner_hardening_2026-02-18.md` -- User constraint honored: no full local Playwright E2E run. - -### Required Checks and Evidence - -1. **Targeted Playwright spec execution** - - Command: - `PLAYWRIGHT_HTML_OPEN=never PLAYWRIGHT_COVERAGE=0 PLAYWRIGHT_BASE_URL=http://127.0.0.1:8080 PLAYWRIGHT_SKIP_SECURITY_DEPS=1 npx playwright test --project=firefox tests/fixtures/token-refresh-validation.spec.ts` - - Environment readiness evidence: - - `docker ps` shows `charon-e2e` healthy. - - `curl -sf http://127.0.0.1:8080/api/v1/health` returned `{"status":"ok",...}`. - - Result: **PASS** (`10 passed`, `9.5s`). - -2. **CI-aligned JS CodeQL targeted verification (`js/insecure-temporary-file`)** - - Task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` - - Artifact: `codeql-results-js.sarif` - - Targeted SARIF verification command (touched paths only): - - Rule: `js/insecure-temporary-file` - - Files: `tests/fixtures/auth-fixtures.ts`, `tests/fixtures/token-refresh-validation.spec.ts` - - Result: **PASS** - - `TOUCHED_MATCHES=0` - - `TOTAL_RESULTS=0` - -3. **Basic lint/type sanity for touched files** - - Lint command: - `npx eslint --no-error-on-unmatched-pattern --no-warn-ignored tests/fixtures/auth-fixtures.ts tests/fixtures/token-refresh-validation.spec.ts && echo ESLINT_TOUCHED_OK` - - Lint result: **PASS** (`ESLINT_TOUCHED_OK`) - - Type command: - `npx tsc --pretty false --noEmit --skipLibCheck --target ES2022 --module ESNext --moduleResolution Bundler --types node,@playwright/test tests/fixtures/auth-fixtures.ts tests/fixtures/token-refresh-validation.spec.ts && echo TYPECHECK_OK` - - Type result: **PASS** (`TYPECHECK_OK`) - -### Gate Verdict - -- **PASS** (targeted QA/Security gate for requested scope) - -### Remaining Blockers - -- **None** for the requested targeted gate scope. - -## PR-3 Closure Addendum - Auth Fixture Token Refresh/Cache Remediation - 2026-02-18 - -### Objective - -- Confirm closure evidence remains present for the targeted `js/insecure-temporary-file` remediation in auth fixture token refresh/cache handling. - -### Evidence - -- Targeted Playwright verification: `tests/fixtures/token-refresh-validation.spec.ts` -> **PASS** (`10 passed`). -- CI-aligned JavaScript CodeQL scan task: `Security: CodeQL JS Scan (CI-Aligned) [~90s]` -> **PASS** (exit code `0`). -- Touched-path CodeQL verification for `js/insecure-temporary-file` -> **PASS** (`TOUCHED_MATCHES=0`). -- Freshness artifact for PR-3 closure context: - - `docs/reports/pr718_open_alerts_freshness_20260218T163918Z.md` - -### Closure Status - -- PR-3 slice targeted insecure-temp remediation QA evidence: **COMPLETE**. - -### Recommended Next Fix Plan (No Sleep/Retry Band-Aids) - -1. Enforce per-test DB isolation in remaining backend test helpers still using shared sqlite state. -2. Eliminate global mutable env leakage by standardizing full-key reset in all env-sensitive tests. -3. Fix CI fixture path robustness for `TestFetchIndexFallbackHTTP` (`testdata` resolution independent of working directory). -4. Re-run parity matrix (`coverage`, `race+shuffle`, `-p 1`, `-p 4`) after each isolation patch batch. +- **Overall QA/Security Result: FAIL** (blocked by Docker image security gate). +- All non-image gates requested for flaky-fix validation passed or produced required artifacts. diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 5ccb5b0f..9efee181 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -52,7 +52,7 @@ "eslint-plugin-react-hooks": "^7.0.1", "eslint-plugin-react-refresh": "^0.5.0", "jsdom": "28.1.0", - "knip": "^5.84.0", + "knip": "^5.84.1", "postcss": "^8.5.6", "tailwindcss": "^4.2.0", "typescript": "^5.9.3", @@ -5282,9 +5282,9 @@ } }, "node_modules/knip": { - "version": "5.84.0", - "resolved": "https://registry.npmjs.org/knip/-/knip-5.84.0.tgz", - "integrity": "sha512-gWXgr9HxRvghijn9t+7AueEwp3vy7uPIV+Ckl72xqBRw+tK2nNI9H0oknVE9J/NSk1jE5WuShzTp4A+40PjYhg==", + "version": "5.84.1", + "resolved": "https://registry.npmjs.org/knip/-/knip-5.84.1.tgz", + "integrity": "sha512-F1+yACEsSapAwmQLzfD4i9uPsnI82P4p5ABpNQ9pcc4fpQtjHEX34XDtNl5863I4O6SCECpymylcWDHI3ouhQQ==", "dev": true, "funding": [ { diff --git a/frontend/package.json b/frontend/package.json index caf118ff..6a5ad9df 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -71,7 +71,7 @@ "eslint-plugin-react-hooks": "^7.0.1", "eslint-plugin-react-refresh": "^0.5.0", "jsdom": "28.1.0", - "knip": "^5.84.0", + "knip": "^5.84.1", "postcss": "^8.5.6", "tailwindcss": "^4.2.0", "typescript": "^5.9.3",