test: add SMTP configuration tests and multi-credential DNS provider support
This commit is contained in:
GitHub Actions
+213
-2438
File diff suppressed because it is too large
Load Diff
+73
-219
@@ -1,261 +1,115 @@
|
||||
# QA Report: Test Failure Resolution and Coverage Boost
|
||||
# QA/Security DoD Validation Report
|
||||
|
||||
**Date**: January 7, 2026
|
||||
**PR**: #461 - DNS Challenge Support for Wildcard Certificates
|
||||
**Branch**: feature/beta-release
|
||||
**Status**: ✅ PASS
|
||||
**Date**: 2026-01-09
|
||||
**Scope**: DoD validation rerun (backend tests + lint + security scans)
|
||||
**Overall Status**: ❌ FAIL
|
||||
|
||||
---
|
||||
## Summary
|
||||
|
||||
## Executive Summary
|
||||
All requested tasks completed successfully (no task execution failures). However, DoD fails due to **HIGH/CRITICAL security findings** in CodeQL and Trivy outputs.
|
||||
|
||||
All 30 originally failing tests have been fixed, backend coverage boosted from 82.7% to 85.2%, and all security scans passed with zero HIGH/CRITICAL findings. The codebase is ready for merge.
|
||||
## Frontend Change Check
|
||||
|
||||
---
|
||||
**Result**: No frontend files detected as changed (no paths under `frontend/` in current workspace changes).
|
||||
|
||||
## Test Coverage Results
|
||||
**Action**: Per request, skipped:
|
||||
- Test: Frontend with Coverage
|
||||
- Lint: TypeScript Check
|
||||
|
||||
### Backend Coverage: 85.2% ✅
|
||||
Note: the pre-commit run includes a frontend TypeScript check hook, but it is not a substitute for the explicit “Frontend with Coverage” task if frontend source changes are present.
|
||||
|
||||
- **Target**: 85%
|
||||
- **Achieved**: 85.2% (+0.2% margin)
|
||||
- **Tests Run**: All backend packages
|
||||
- **Status**: PASSED
|
||||
## Task Results (Required)
|
||||
|
||||
**Improvements Made**:
|
||||
- Excluded `pkg/dnsprovider/builtin` from coverage (integration-tested, not unit-tested)
|
||||
- Added comprehensive tests to `internal/services` and `internal/api/handlers`
|
||||
- Focus on error paths, edge cases, and validation logic
|
||||
### 1) Test: Backend with Coverage
|
||||
|
||||
**Key Package Coverage**:
|
||||
- `internal/api/handlers`: 85%+ (was 81.9%)
|
||||
- `internal/services`: 85%+ (was 80.7%)
|
||||
- `internal/caddy`: 94.4%
|
||||
- `internal/cerberus`: 100%
|
||||
- `internal/config`: 100%
|
||||
- `internal/models`: 96.4%
|
||||
**Pass/Fail Criteria**:
|
||||
- PASS if task exits successfully and produces a coverage result.
|
||||
|
||||
### Frontend Coverage: 85.65% ✅
|
||||
**Result**: ✅ PASS (task completed)
|
||||
|
||||
- **Target**: 85%
|
||||
- **Achieved**: 85.65% (+0.65% margin)
|
||||
- **Tests Run**: 119 tests across 5 test files
|
||||
- **Status**: PASSED
|
||||
**Coverage**:
|
||||
- Backend total coverage (from `go tool cover -func backend/coverage.txt`): **86.6%**
|
||||
- Task output included: `coverage: 63.2% of statements` (package `backend/cmd/seed`)
|
||||
|
||||
---
|
||||
### 2) Lint: Pre-commit (All Files)
|
||||
|
||||
## Test Fixes Summary
|
||||
**Pass/Fail Criteria**:
|
||||
- PASS if all hooks complete successfully.
|
||||
|
||||
### Phase 1: DNS Provider Registry Initialization (18 tests)
|
||||
**Files Modified**:
|
||||
- `backend/internal/api/handlers/credential_handler_test.go`
|
||||
- `backend/internal/caddy/manager_multicred_integration_test.go`
|
||||
- `backend/internal/caddy/config_patch_coverage_test.go`
|
||||
- `backend/internal/services/dns_provider_service_test.go`
|
||||
**Result**: ✅ PASS
|
||||
|
||||
**Fix**: Added blank import `_ "github.com/Wikid82/charon/backend/pkg/dnsprovider/builtin"` to trigger DNS provider registry initialization
|
||||
### 3) Security: CodeQL All (CI-Aligned)
|
||||
|
||||
### Phase 2: Credential Field Name Corrections (4 tests)
|
||||
**File**: `backend/internal/services/dns_provider_service_test.go`
|
||||
**Pass/Fail Criteria**:
|
||||
- PASS if no HIGH/CRITICAL findings are present.
|
||||
|
||||
**Fixes**:
|
||||
- Hetzner: `api_key` → `api_token`
|
||||
- DigitalOcean: `auth_token` → `api_token`
|
||||
- DNSimple: `oauth_token` → `api_token`
|
||||
**Result**: ❌ FAIL
|
||||
|
||||
### Phase 3: Security Handler Input Validation (1 test)
|
||||
**File**: `backend/internal/api/handlers/security_handler.go`
|
||||
**Findings**:
|
||||
- Go SARIF (`codeql-results-go.sarif`): **3 CRITICAL** (security severity 9.8)
|
||||
- Rule: `go/email-injection` (“Email content injection”)
|
||||
- Location: `backend/internal/services/mail_service.go` (lines ~222, ~340, ~393)
|
||||
- JS SARIF (`codeql-results-js.sarif`): **1 HIGH** (security severity 7.8)
|
||||
- Rule: `js/incomplete-hostname-regexp` (“Incomplete regular expression for hostnames”)
|
||||
- Location: `frontend/src/pages/__tests__/ProxyHosts-extra.test.tsx` (line ~252)
|
||||
|
||||
**Fix**: Added comprehensive input validation:
|
||||
- `isValidIP()` - IP format validation
|
||||
- `isValidCIDR()` - CIDR notation validation
|
||||
- `isValidAction()` - Action enum validation (block/allow/captcha)
|
||||
- `sanitizeString()` - Input sanitization
|
||||
### 4) Security: Trivy Scan
|
||||
|
||||
### Phase 4: Security Settings Database Override (5 tests)
|
||||
**File**: `backend/internal/testutil/db.go`
|
||||
**Pass/Fail Criteria**:
|
||||
- PASS if no HIGH/CRITICAL findings are present.
|
||||
|
||||
**Fix**: Added SQLite `_txlock=immediate` parameter to prevent database lock contention
|
||||
**Result**: ❌ FAIL
|
||||
|
||||
### Phase 5: Certificate Deletion Race Condition (1 test)
|
||||
**File**: Already fixed in previous PR
|
||||
**Counts (from existing artifacts)**:
|
||||
- `trivy-scan-output.txt`: **CRITICAL=1**, **HIGH=7**
|
||||
- `trivy-image-scan.txt`: **CRITICAL=0**, **HIGH=1**
|
||||
|
||||
### Phase 6: Frontend LiveLogViewer Timeout (1 test)
|
||||
**Status**: Already fixed in previous PR
|
||||
## Root Cause (Why DoD Failed)
|
||||
|
||||
### Coverage Boost Tests
|
||||
**Files Created/Modified**:
|
||||
- `backend/internal/services/coverage_boost_test.go` - Service accessor and error path tests
|
||||
- `backend/internal/api/handlers/plugin_handler_test.go` - Complete plugin handler coverage
|
||||
### CodeQL
|
||||
|
||||
**New Tests Added**: 40+ test cases covering:
|
||||
- Service accessors (DB(), Get*(), List*())
|
||||
- Error handling for missing resources
|
||||
- Plugin enable/disable/reload operations
|
||||
- Notification provider lifecycle
|
||||
- Security service configuration
|
||||
- Mail service SMTP error paths
|
||||
- GeoIP service validation
|
||||
1) **CRITICAL** `go/email-injection` in `backend/internal/services/mail_service.go`
|
||||
|
||||
---
|
||||
**Likely cause**: user-controlled or otherwise untrusted values are being used to build email content (and potentially headers) without robust validation/normalization, enabling header/body injection (e.g., newline injection).
|
||||
|
||||
## Security Scan Results
|
||||
2) **HIGH** `js/incomplete-hostname-regexp` in a frontend test
|
||||
|
||||
### CodeQL Analysis ✅
|
||||
**Likely cause**: a regex used for host matching in tests does not escape `.`, so it matches more than intended.
|
||||
|
||||
**Go Scan**:
|
||||
- Queries Run: 61
|
||||
- Errors: 0
|
||||
- Warnings: 0
|
||||
- Notes: 0
|
||||
- **Status**: PASSED
|
||||
### Trivy
|
||||
|
||||
**JavaScript Scan**:
|
||||
- Queries Run: 88
|
||||
- Errors: 0
|
||||
- Warnings: 0
|
||||
- Notes: 1 (regex pattern in test file - non-blocking)
|
||||
- **Status**: PASSED
|
||||
|
||||
**Total Findings**: 0 blocking issues
|
||||
|
||||
### Trivy Container Scan
|
||||
**Status**: Not run (Docker build verified locally, no containers built for this QA run)
|
||||
**Likely cause**: one or more dependencies in the repo (Go modules and/or image contents) are pinned to vulnerable versions.
|
||||
|
||||
### Go Vulnerability Check (govulncheck)
|
||||
**Status**: Not run (can be run in CI)
|
||||
|
||||
---
|
||||
|
||||
## Pre-commit Hooks ✅
|
||||
|
||||
**Status**: PASSED
|
||||
|
||||
**Hooks Verified**:
|
||||
- ✅ Fix end of files
|
||||
- ✅ Trim trailing whitespace
|
||||
- ✅ Check YAML
|
||||
- ✅ Check for added large files
|
||||
- ✅ Dockerfile validation
|
||||
- ✅ Go Vet
|
||||
- ✅ Check .version matches Git tag
|
||||
- ✅ Prevent large files not tracked by LFS
|
||||
- ✅ Prevent committing CodeQL DB artifacts
|
||||
- ✅ Prevent committing data/backups files
|
||||
- ✅ Frontend TypeScript Check
|
||||
- ✅ Frontend Lint (Fix)
|
||||
Examples extracted from `trivy-scan-output.txt` / `trivy-image-scan.txt` include (non-exhaustive):
|
||||
- `golang.org/x/crypto` (CVE-2024-45337 CRITICAL; CVE-2025-22869 HIGH)
|
||||
- `golang.org/x/net` (CVE-2023-39325 HIGH)
|
||||
- `golang.org/x/oauth2` (CVE-2025-22868 HIGH)
|
||||
- `gopkg.in/yaml.v3` (CVE-2022-28948 HIGH)
|
||||
- `github.com/quic-go/quic-go` (CVE-2025-59530 HIGH)
|
||||
- `github.com/expr-lang/expr` (CVE-2025-68156 HIGH)
|
||||
|
||||
---
|
||||
## Proposed Remediation (No changes applied)
|
||||
|
||||
## Type Safety ✅
|
||||
Per instruction: **no fixes were made**. Suggested remediation steps:
|
||||
|
||||
### Backend (Go)
|
||||
- **Status**: PASSED
|
||||
- All packages compile successfully
|
||||
- No type errors
|
||||
### For CodeQL `go/email-injection`
|
||||
|
||||
### Frontend (TypeScript)
|
||||
- **Status**: PASSED
|
||||
- TypeScript 5.x type check passed
|
||||
- All imports resolve correctly
|
||||
- No type errors
|
||||
- Validate/normalize any untrusted values used in mail headers/body (especially ensuring values do not contain `\r`/`\n`).
|
||||
- Use strict email address parsing/validation (e.g., Go `net/mail`) and explicit header encoding.
|
||||
- Ensure subject/from/to/reply-to fields are constructed via safe libraries and reject control characters.
|
||||
|
||||
---
|
||||
### For CodeQL `js/incomplete-hostname-regexp`
|
||||
|
||||
## Issues Found and Resolved
|
||||
- Update the test regex to escape `.` and/or use a safer matcher; rerun CodeQL JS scan.
|
||||
|
||||
### Issue 1: Mock DNS Provider Missing Interface Methods
|
||||
**Severity**: High (compilation error)
|
||||
**Location**: `backend/internal/api/handlers/plugin_handler_test.go`
|
||||
**Root Cause**: `mockDNSProvider` was missing `Init()`, `Cleanup()`, and other interface methods
|
||||
**Resolution**: Added all required `ProviderPlugin` interface methods to mock
|
||||
**Status**: FIXED
|
||||
### For Trivy findings
|
||||
|
||||
### Issue 2: Time Package Import Missing
|
||||
**Severity**: Low (compilation error)
|
||||
**Location**: `backend/internal/api/handlers/plugin_handler_test.go`
|
||||
**Root Cause**: Mock methods return `time.Duration` but package not imported
|
||||
**Resolution**: Added `time` to imports
|
||||
**Status**: FIXED
|
||||
- Upgrade impacted Go modules to versions containing fixes (follow Trivy “Fixed Version” guidance) and run `go mod tidy`.
|
||||
- Re-run Trivy scan after dependency upgrades.
|
||||
- If image findings remain: rebuild the image after base image upgrades and/or OS package updates.
|
||||
|
||||
---
|
||||
## Artifacts
|
||||
|
||||
## Files Modified
|
||||
|
||||
### Configuration Files
|
||||
- `.codecov.yml` - Added DNS provider builtin package exclusion
|
||||
- `scripts/go-test-coverage.sh` - Added DNS provider to exclusion list
|
||||
|
||||
### Test Files
|
||||
- `backend/internal/api/handlers/credential_handler_test.go` - Added blank import
|
||||
- `backend/internal/caddy/manager_multicred_integration_test.go` - Added blank import
|
||||
- `backend/internal/caddy/config_patch_coverage_test.go` - Added blank import
|
||||
- `backend/internal/services/dns_provider_service_test.go` - Fixed credential fields + blank import
|
||||
- `backend/internal/services/coverage_boost_test.go` - NEW (service tests)
|
||||
- `backend/internal/api/handlers/plugin_handler_test.go` - NEW (handler tests)
|
||||
|
||||
### Source Files
|
||||
- `backend/internal/api/handlers/security_handler.go` - Added input validation
|
||||
- `backend/internal/api/handlers/security_handler_audit_test.go` - Fixed test action value
|
||||
- `backend/internal/testutil/db.go` - Added SQLite txlock parameter
|
||||
|
||||
---
|
||||
|
||||
## Test Execution Summary
|
||||
|
||||
### Backend
|
||||
- **Total Packages Tested**: 25+
|
||||
- **Coverage**: 85.2%
|
||||
- **All Tests**: PASSED
|
||||
- **Execution Time**: ~30s
|
||||
|
||||
### Frontend
|
||||
- **Test Files**: 5
|
||||
- **Tests Run**: 119
|
||||
- **Tests Passed**: 119
|
||||
- **Tests Failed**: 0
|
||||
- **Coverage**: 85.65%
|
||||
- **Execution Time**: ~12 minutes
|
||||
|
||||
---
|
||||
|
||||
## Deployment Readiness Checklist
|
||||
|
||||
- [x] All original failing tests fixed (30/30)
|
||||
- [x] Backend coverage >= 85% (85.2%)
|
||||
- [x] Frontend coverage >= 85% (85.65%)
|
||||
- [x] Security scans passed (0 HIGH/CRITICAL)
|
||||
- [x] Pre-commit hooks passed
|
||||
- [x] Type checks passed (Go + TypeScript)
|
||||
- [x] No compilation errors
|
||||
- [x] Code follows project conventions
|
||||
- [x] Tests are meaningful and maintainable
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
1. **Merge Ready**: All blocking issues resolved, code is production-ready
|
||||
2. **Monitor CI**: Verify Docker build passes in CI (tested locally)
|
||||
3. **Follow-up**: Consider adding more integration tests for DNS provider implementations in a future PR
|
||||
4. **Documentation**: Update user-facing docs to mention DNS challenge support for wildcards
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
**FINAL VERDICT**: ✅ PASS
|
||||
|
||||
All Definition of Done criteria met:
|
||||
- ✅ Coverage tests passed (backend 85.2%, frontend 85.65%)
|
||||
- ✅ Type safety verified
|
||||
- ✅ Pre-commit hooks passed
|
||||
- ✅ Security scans clean (0 HIGH/CRITICAL findings)
|
||||
- ✅ All tests passing
|
||||
|
||||
The PR is approved for merge from a quality assurance perspective.
|
||||
|
||||
---
|
||||
|
||||
**QA Engineer**: Engineering Director (Management Mode)
|
||||
**Sign-off Date**: January 7, 2026
|
||||
- Backend coverage profile: `backend/coverage.txt`
|
||||
- CodeQL results: `codeql-results-go.sarif`, `codeql-results-js.sarif`, `codeql-results-javascript.sarif`
|
||||
- Trivy results: `trivy-scan-output.txt`, `trivy-image-scan.txt`
|
||||
|
||||
Reference in New Issue
Block a user