Files
Charon/docs/reports/qa_report.md

13 KiB
Raw Blame History

QA Report — PR #800 (feature/beta-release)

Date: 2026-03-06 Auditor: QA Security Agent (QA Security Mode) Branch: feature/beta-release PR: #800 Scope: Security hardening — WebSocket origin validation, CodeQL email-injection suppressions, Semgrep pipeline refactor, security-local Makefile target


Executive Summary

All 10 QA steps pass. No blocking issues. Backend coverage is 87.9% (threshold: 85%). Frontend coverage is 89.73% lines (threshold: 87%). Patch coverage is 90.6% (overall). Zero static-analysis findings. Zero security scan findings. Security changes correctly implemented and individually verified.

Overall Verdict: PASS — Ready to merge


QA Step Results

Step 1: Backend Build, Vet, and Tests

1a — go build ./...

Metric Result
Status PASS
Exit Code 0
Output Clean — no errors or warnings
Command cd /projects/Charon && go build ./...

1b — go vet ./...

Metric Result
Status PASS
Exit Code 0
Output Clean — no issues
Command cd /projects/Charon && go vet ./...

1c — go test ./...

Metric Result
Status PASS
Packages 31 tested, 2 with no test files (skipped)
Failures 0
Slowest Packages crowdsec (93s), services (74s), handlers (66s)
Command cd /projects/Charon && go test ./...

Step 2: Backend Coverage Report

Metric Result
Status PASS
Statement Coverage 87.9% (threshold: 85%)
Line Coverage 88.1% (threshold: 85%)
Command bash /projects/Charon/scripts/go-test-coverage.sh

Packages below 85% (pre-existing, not caused by this PR):

Package Coverage Notes
cmd/api 82.8% Pre-existing; bootstrap/init code difficult to unit-test
internal/util 78.0% Pre-existing; utility helpers with edge-case paths

All other packages meet or exceed the 85% threshold.


Step 3: Frontend Tests and Coverage

Metric Result
Status PASS
Test Files 27
Tests Passed 581
Tests Skipped 1
Failures 0
Line Coverage 89.73% (threshold: 87%)
Statement Coverage 89.0%
Function Coverage 86.26%
Branch Coverage 81.07% (not enforced — only lines is configured)
Command cd /projects/Charon/frontend && npm run test -- --coverage --reporter=verbose

Note: Branch coverage at 81.07% is below a 85% target but is not an enforced threshold in the Vitest configuration. Only line coverage is enforced (configured at 87%). This is pre-existing and not caused by this PR.


Step 4: TypeScript Type Check

Metric Result
Status PASS
Errors 0
Exit Code 0
Command cd /projects/Charon/frontend && npm run type-check

Step 5: Pre-commit Hooks (Non-Semgrep)

Metric Result
Status PASS
Hooks Passed 15/15
Semgrep Correctly absent (now at stages: [pre-push] — see Security Changes)
Command cd /projects/Charon && pre-commit run --all-files

Hooks executed and their status:

Hook Status
fix end of files Passed
trim trailing whitespace Passed
check yaml Passed
check for added large files Passed
shellcheck Passed
actionlint (GitHub Actions) Passed
dockerfile validation Passed
Go Vet Passed
golangci-lint (Fast Linters - BLOCKING) Passed
Check .version matches latest Git tag Passed
Prevent large files not tracked by LFS Passed
Prevent committing CodeQL DB artifacts Passed
Prevent committing data/backups files Passed
Frontend TypeScript Check Passed
Frontend Lint (Fix) Passed

Step 6: Local Patch Coverage Preflight

Metric Result
Status PASS
Overall Patch Coverage 90.6%
Backend Patch Coverage 90.4%
Frontend Patch Coverage 100%
Artifacts test-results/local-patch-report.md , test-results/local-patch-report.json
Command bash /projects/Charon/scripts/local-patch-report.sh

Files with partially covered changed lines:

File Patch Coverage Uncovered Lines Notes
backend/internal/services/mail_service.go 84.2% 334335, 339340, 346347, 351352, 540 SMTP sink lines; CodeQL [go/email-injection] suppressions applied; hard to unit-test directly
backend/internal/services/notification_service.go 97.6% 1 line Minor branch

Frontend patch coverage is 100% — all changed lines in notifications.test.ts and SecurityNotificationSettingsModal.test.tsx are covered.


Step 7: Static Analysis

Metric Result
Status PASS
Issues 0
Linters golangci-lint (--config .golangci-fast.yml)
Command make -C /projects/Charon lint-fast

Step 8: Semgrep Validation (Manual)

Metric Result
Status PASS
Findings 0
Rules Applied 42 (from p/golang ruleset)
Files Scanned 182
Command SEMGREP_CONFIG=p/golang bash /projects/Charon/scripts/pre-commit-hooks/semgrep-scan.sh

This step validates the new p/golang ruleset configuration introduced in this PR.


Step 9: make security-local

Metric Result
Status PASS
govulncheck 0 vulnerabilities
Semgrep (p/golang) 0 findings
Exit Code 0
Command make -C /projects/Charon security-local

This is the new security-local Makefile target introduced by this PR — both constituent checks pass.


Step 10: Git Diff Summary

git diff --name-only reports 9 changed files (unstaged relative to last commit):

File Category Change Summary
.pre-commit-config.yaml Config semgrep-scan moved from stages: [manual]stages: [pre-push]
Makefile Build Added security-local target
backend/internal/api/handlers/cerberus_logs_ws.go Backend Added # nosemgrep annotation on .Upgrade() call
backend/internal/api/handlers/logs_ws.go Backend Replaced insecure CheckOrigin: return true with host-based validation
backend/internal/api/handlers/settings_handler.go Backend Added documentation comment above SendEmail call
backend/internal/api/handlers/user_handler.go Backend Added documentation comments above 2 SendInvite calls
backend/internal/services/mail_service.go Backend Added // codeql[go/email-injection] suppressions on 3 SMTP sink lines
docs/plans/current_spec.md Docs Spec updates
scripts/pre-commit-hooks/semgrep-scan.sh Scripts Default config autop/golang; added --severity flags; scope to frontend/src

HEAD commit (ee224adc) additionally included (committed changes not in the diff above):

  • backend/internal/models/notification_config.go
  • backend/internal/services/mail_service_test.go
  • backend/internal/services/notification_service.go
  • backend/internal/services/notification_service_test.go
  • frontend/src/api/notifications.test.ts
  • frontend/src/components/__tests__/SecurityNotificationSettingsModal.test.tsx

Bonus: GORM Security Scan

Triggered because backend/internal/models/notification_config.go changed in HEAD.

Metric Result
Status PASS
CRITICAL 0
HIGH 0
MEDIUM 0
INFO 2 (pre-existing: missing index suggestions on UserPermittedHost foreign keys in user.go)
Files Scanned 41 Go model files
Command bash /projects/Charon/scripts/scan-gorm-security.sh --check

The 2 INFO findings are pre-existing and unrelated to this PR.


Security Change Verification

1. WebSocket Origin Validation (logs_ws.go)

Change: Replaced CheckOrigin: func(r *http.Request) bool { return true } with proper host-based validation.

Implementation verified:

  • Imports "net/url" (line 5)
  • CheckOrigin function parses Origin header via url.Parse()
  • Compares originURL.Host to r.Host, honoring X-Forwarded-Host for proxy scenarios
  • Returns false on parse error or host mismatch

Assessment: Correct. Addresses the Semgrep websocket-missing-origin-check finding. Guards against cross-site WebSocket hijacking (CWE-346).


2. Nosemgrep Annotation (cerberus_logs_ws.go)

Change: Added # nosemgrep: go.gorilla.security.audit.websocket-missing-origin-check.websocket-missing-origin-check on the .Upgrade() call.

Justification verified: This handler uses the shared upgrader variable defined in logs_ws.go, which now has a valid CheckOrigin function. The annotation is correct — the rule fires on the call site but the underlying upgrader is already secured.

Assessment: Correct. Suppression is justified and scoped to a single line.


3. CodeQL Email-Injection Suppressions (mail_service.go)

Change: Added // codeql[go/email-injection] on lines 370, 534, 588 (SMTP smtp.SendMail() calls).

Assessment: Correct. Each suppressed sink is protected by documented 4-layer defense:

  1. sanitizeForEmail() — strips \r/\n from user inputs
  2. rejectCRLF() — hard-rejects strings containing CRLF sequences
  3. encodeSubject() — RFC 2047 encodes email subject
  4. html.EscapeString() / sanitizeEmailBody() — HTML-escapes body content

Suppressions are placed at the exact CodeQL sink lines per the CodeQL suppression spec.


4. Semgrep Pipeline Refactor

Changes verified:

Change File Assessment
stages: [pre-push] .pre-commit-config.yaml Semgrep now runs on git push, not every commit. Faster commit loop.
Default config autop/golang semgrep-scan.sh Deterministic, focused ruleset. auto was non-deterministic.
--severity ERROR --severity WARNING flags semgrep-scan.sh Explicitly filters noise; only ERROR/WARNING findings are blocking.
Scope to frontend/src semgrep-scan.sh Focuses frontend scanning on source directory.

5. security-local Makefile Target

Target verified (Makefile line 149):

security-local: ## Run govulncheck + semgrep (p/golang) before push — fast local gate
    @echo "[1/2] Running govulncheck..."
    @./scripts/security-scan.sh
    @echo "[2/2] Running Semgrep (p/golang, ERROR+WARNING)..."
    @SEMGREP_CONFIG=p/golang ./scripts/pre-commit-hooks/semgrep-scan.sh

Assessment: Correct. Provides a fast, developer-friendly pre-push gate that mirrors the CI security checks.


Gotify Token Review

  • No Gotify tokens found in diffs, test output, or log artifacts
  • No tokenized URLs (e.g., ?token=...) exposed in any output
  • Clean

Issues and Observations

Blocking Issues

None.

Non-Blocking Observations

Observation Severity Notes
cmd/api backend coverage at 82.8% ⚠️ INFO Pre-existing. Bootstrap/init code. Not caused by this PR.
internal/util backend coverage at 78.0% ⚠️ INFO Pre-existing. Utility helpers. Not caused by this PR.
Frontend branch coverage at 81.07% ⚠️ INFO Pre-existing. Threshold not enforced (only lines is).
mail_service.go patch coverage at 84.2% ⚠️ INFO SMTP sink lines are intentionally difficult to unit-test. CodeQL suppressions are the documented mitigation.
GORM INFO findings (missing FK indexes) ⚠️ INFO Pre-existing in user.go. Unrelated to this PR.

Final Determination

Step Status
1a. go build ./... PASS
1b. go vet ./... PASS
1c. go test ./... PASS
2. Backend coverage PASS — 87.9% / 88.1%
3. Frontend tests + coverage PASS — 581 pass, 89.73% lines
4. TypeScript type check PASS
5. Pre-commit hooks PASS — 15/15
6. Local patch coverage preflight PASS — 90.6% overall
7. make lint-fast PASS — 0 issues
8. Semgrep manual validation PASS — 0 findings
9. make security-local PASS
10. Git diff 9 changed files
GORM security scan PASS — 0 CRITICAL/HIGH

OVERALL: PASS — All gates met. No blocking issues. Ready to merge.