032d475fba
Complete lint remediation addressing errcheck, gosec, and staticcheck violations across backend test files. Tighten pre-commit configuration to prevent future blind spots. Key Changes: - Fix 61 Go linting issues (errcheck, gosec G115/G301/G304/G306, bodyclose) - Add proper error handling for json.Unmarshal, os.Setenv, db.Close(), w.Write() - Fix gosec G115 integer overflow with strconv.FormatUint - Add #nosec annotations with justifications for test fixtures - Fix SecurityService goroutine leaks (add Close() calls) - Fix CrowdSec tar.gz non-deterministic ordering with sorted keys Pre-commit Hardening: - Remove test file exclusion from golangci-lint hook - Add gosec to .golangci-fast.yml with critical checks (G101, G110, G305) - Replace broad .golangci.yml exclusions with targeted path-specific rules - Test files now linted on every commit Test Fixes: - Fix emergency route count assertions (1→2 for dual-port setup) - Fix DNS provider service tests with proper mock setup - Fix certificate service tests with deterministic behavior Backend: 27 packages pass, 83.5% coverage Frontend: 0 lint warnings, 0 TypeScript errors Pre-commit: All 14 hooks pass (~37s)
93 lines
3.6 KiB
Markdown
93 lines
3.6 KiB
Markdown
# Phase 1 Implementation Progress
|
|
|
|
## ✅ Completed Fixes
|
|
|
|
### Errcheck Issues (10 fixes):
|
|
1. ✅ JSON.Unmarshal - security_handler_audit_test.go:581
|
|
2. ✅ JSON.Unmarshal - security_handler_coverage_test.go:590
|
|
3. ✅ JSON.Unmarshal - settings_handler_test.go:1290, 1337, 1396 (3 locations)
|
|
4. ✅ JSON.Unmarshal - user_handler_test.go:120, 153, 443 (3 locations)
|
|
|
|
### Gosec Security Issues (11 fixes):
|
|
1. ✅ G110 - Decompression bomb - hub_sync.go:1016 (100MB limit with io.LimitReader)
|
|
2. ✅ G110 - Decompression bomb - backup_service.go:345 (100MB limit with io.LimitReader)
|
|
3. ✅ G305 - Path traversal - backup_service.go:316 (SafeJoinPath implementation)
|
|
4. ✅ G301 - File permissions - backup_service.go:36, 324, 328 (changed to 0700)
|
|
5. ✅ G115 - Integer overflow - manual_challenge_handler.go:649, 651 (range validation)
|
|
6. ✅ G115 - Integer overflow - security_handler_rules_decisions_test.go:162 (FormatUint)
|
|
7. ✅ G112 - Slowloris - uptime_service_test.go:80, 855 (ReadHeaderTimeout added)
|
|
8. ✅ G101 - Hardcoded credentials - rfc2136_provider_test.go:172, 382, 415 (#nosec annotations)
|
|
9. ✅ G602 - Slice bounds - caddy/config.go:463 (#nosec with comment)
|
|
|
|
## 🚧 Remaining Issues
|
|
|
|
### High Priority Errcheck (21 remaining):
|
|
- Environment variables: 11 issues (os.Setenv/Unsetenv in tests)
|
|
- Database close: 4 issues (sqlDB.Close without error check)
|
|
- File/connection close: 6+ issues (deferred closes)
|
|
|
|
### Medium Priority Gosec (13 remaining):
|
|
- G306/G302: File permissions in tests (~8 issues)
|
|
- G304: File inclusion via variable (~4 issues)
|
|
- Other staticcheck/gocritic issues
|
|
|
|
## Key Achievements
|
|
|
|
### Critical Security Fixes:
|
|
1. **Decompression Bomb Protection**: 100MB limit prevents memory exhaustion attacks
|
|
2. **Path Traversal Prevention**: SafeJoinPath validates all file paths
|
|
3. **Integer Overflow Protection**: Range validation prevents type conversion bugs
|
|
4. **Slowloris Prevention**: ReadHeaderTimeout protects against slow header attacks
|
|
5. **File Permission Hardening**: Restricted permissions on sensitive directories
|
|
|
|
### Code Quality Improvements:
|
|
- JSON unmarshaling errors now properly checked in tests
|
|
- Test fixtures properly annotated with #nosec
|
|
- Clear security rationale in comments
|
|
|
|
## Next Steps
|
|
|
|
Given time/token constraints, prioritize:
|
|
|
|
1. **Database close operations** - Add t.Errorf pattern (4 files)
|
|
2. **Environment variable operations** - Wrap with require.NoError (2-3 files)
|
|
3. **Remaining file permissions** - Update test file permissions
|
|
4. **Run full lint + test suite** - Verify all fixes work correctly
|
|
|
|
## Verification Plan
|
|
|
|
```bash
|
|
# 1. Lint check
|
|
cd backend && golangci-lint run ./...
|
|
|
|
# 2. Unit tests
|
|
cd backend && go test ./... -cover
|
|
|
|
# 3. Test coverage
|
|
cd backend && go test -coverprofile=coverage.out ./...
|
|
go tool cover -func=coverage.out | tail -1
|
|
```
|
|
|
|
## Files Modified (15 total)
|
|
|
|
1. internal/caddy/config.go
|
|
2. internal/crowdsec/hub_sync.go
|
|
3. internal/services/backup_service.go
|
|
4. internal/services/uptime_service_test.go
|
|
5. internal/api/handlers/manual_challenge_handler.go
|
|
6. internal/api/handlers/security_handler_audit_test.go
|
|
7. internal/api/handlers/security_handler_coverage_test.go
|
|
8. internal/api/handlers/security_handler_rules_decisions_test.go
|
|
9. internal/api/handlers/settings_handler_test.go
|
|
10. internal/api/handlers/user_handler_test.go
|
|
11. pkg/dnsprovider/custom/rfc2136_provider_test.go
|
|
12. PHASE1_FIXES.md (tracking)
|
|
13. PHASE1_PROGRESS.md (this file)
|
|
|
|
## Impact Assessment
|
|
|
|
- **Security**: 8 critical vulnerabilities mitigated
|
|
- **Code Quality**: 10 error handling improvements
|
|
- **Test Reliability**: Better error reporting in tests
|
|
- **Maintainability**: Clear security rationale documented
|