Charon/docs/reports/qa_report_pr928.md

QA Audit Report — PR #928: CI Test Fix

Date: 2026-04-13 Scope: Targeted fix for two CI test failures across three files Auditor: QA Security Agent

Modified Files

File	Change
backend/internal/api/handlers/certificate_handler.go	Added key_file validation guard for non-PFX uploads
backend/internal/api/handlers/certificate_handler_test.go	Added TestCertificateHandler_Upload_MissingKeyFile_MultipartWithCert test
backend/internal/services/certificate_service_coverage_test.go	Removed duplicate ALTER TABLE in TestCertificateService_MigratePrivateKeys

Check Results

#	Check	Result	Details
1	Backend Build (go build ./...)	PASS	Clean build, no errors
2a	Targeted Test 1 (TestCertificateHandler_Upload_MissingKeyFile_MultipartWithCert)	PASS	Previously failing, now passes
2b	Targeted Test 2 (TestCertificateService_MigratePrivateKeys)	PASS	Previously failing (duplicate ALTER TABLE), now passes — all 3 subtests pass
3a	Regression: Handler Suite (TestCertificateHandler*, 37 tests)	PASS	All 37 tests pass in 1.27s
3b	Regression: Service Suite (TestCertificateService*)	PASS	All tests pass in 2.96s
4	Go Vet (go vet ./...)	PASS	No issues
5	Golangci-lint (handlers + services)	PASS	All warnings are pre-existing in unmodified files (crowdsec, audit_log). Zero new lint issues in modified files
6	Security Review	PASS	See analysis below

Security Analysis

The handler change (lines 169–175 of certificate_handler.go) was reviewed for:

Vector	Assessment
Injection	services.DetectFormat() operates on already-read certBytes (bounded by maxFileSize = 1MB via io.LimitReader). No additional I/O or shell invocation.
Information Disclosure	Returns a static error string "key_file is required for PEM/DER certificate uploads". No user-controlled data reflected in the response.
Auth Bypass	Route POST /certificates is registered inside the management group (confirmed at routes.go:696), which requires authentication. The guard is additive — it rejects earlier, not later.
DoS	No new allocations or expensive operations. DetectFormat is a simple byte-header check on data already in memory.

Verdict: No new attack surface introduced. The change is a pure input validation tightening.

Warnings

• Pre-existing lint warnings exist in unmodified files (crowdsec_handler.go, crowdsec_*_test.go, audit_log_handler_test.go). These are tracked separately and are not related to this PR.

Final Verdict

PASS — All six checks pass. Both previously failing CI tests now succeed. No regressions detected in the broader handler and service suites. No security concerns with the changes.