fix: update HTTP request handling and improve test coverage in various handlers
This commit is contained in:
20
.github/agents/QA_Security.agent.md
vendored
20
.github/agents/QA_Security.agent.md
vendored
@@ -30,8 +30,8 @@ Your job is to act as an ADVERSARY. The Developer says "it works"; your job is t
|
||||
- **Path Verification**: Run `list_dir internal/api` to verify where tests should go.
|
||||
- **Creation**: Write a new test file (e.g., `internal/api/tests/audit_test.go`) to test the *flow*.
|
||||
- **Run**: Execute `.github/skills`, `go test ./internal/api/tests/...` (or specific path). Run local CodeQL and Trivy scans (they are built as VS Code Tasks so they just need to be triggered to run), pre-commit all files, and triage any findings.
|
||||
- When running golangci-lint, always run it in docker to ensure consistent linting.
|
||||
- When creating tests, if there are folders that don't require testing make sure to update `codecove.yml` to exclude them from coverage reports or this throws off the difference betwoeen local and CI coverage.
|
||||
- **GolangCI-Lint (CRITICAL)**: Always run VS Code task "Lint: GolangCI-Lint (Docker)" - NOT "Lint: Go Vet". The Go Vet task only runs `go vet` which misses gocritic, bodyclose, and other linters that CI runs. GolangCI-Lint in Docker ensures parity with CI.
|
||||
- When creating tests, if there are folders that don't require testing make sure to update `codecov.yml` to exclude them from coverage reports or this throws off the difference between local and CI coverage.
|
||||
- **Cleanup**: If the test was temporary, delete it. If it's valuable, keep it.
|
||||
</workflow>
|
||||
|
||||
@@ -69,9 +69,10 @@ When Trivy or CodeQLreports CVEs in container dependencies (especially Caddy tra
|
||||
The task is not complete until ALL of the following pass with zero issues:
|
||||
|
||||
1. **Security Scans**:
|
||||
- CodeQL: Run as VS Code task or via GitHub Actions
|
||||
- Trivy: Run as VS Code task or via Docker
|
||||
- Zero issues allowed
|
||||
- CodeQL: Run VS Code task "Security: CodeQL All (CI-Aligned)" or individual Go/JS tasks
|
||||
- Trivy: Run VS Code task "Security: Trivy Scan"
|
||||
- Go Vulnerabilities: Run VS Code task "Security: Go Vulnerability Check"
|
||||
- Zero Critical/High issues allowed
|
||||
|
||||
2. **Coverage Tests (MANDATORY - Run Explicitly)**:
|
||||
- **Backend**: Run VS Code task "Test: Backend with Coverage" or execute `scripts/go-test-coverage.sh`
|
||||
@@ -87,8 +88,13 @@ The task is not complete until ALL of the following pass with zero issues:
|
||||
|
||||
4. **Pre-commit Hooks**: Run `pre-commit run --all-files` (this runs fast hooks only; coverage was verified in step 1)
|
||||
|
||||
|
||||
5. **Linting**: All language-specific linters must pass (Go vet, ESLint, markdownlint)
|
||||
5. **Linting (MANDATORY - Run All Explicitly)**:
|
||||
- **Backend GolangCI-Lint**: Run VS Code task "Lint: GolangCI-Lint (Docker)" - This is the FULL linter suite including gocritic, bodyclose, etc.
|
||||
- **Why**: "Lint: Go Vet" only runs `go vet`, NOT the full golangci-lint suite. CI runs golangci-lint, so you MUST run this task to match CI behavior.
|
||||
- **Command**: `cd backend && docker run --rm -v $(pwd):/app:ro -w /app golangci/golangci-lint:latest golangci-lint run -v`
|
||||
- **Frontend ESLint**: Run VS Code task "Lint: Frontend"
|
||||
- **Markdownlint**: Run VS Code task "Lint: Markdownlint"
|
||||
- **Hadolint**: Run VS Code task "Lint: Hadolint Dockerfile" (if Dockerfile was modified)
|
||||
|
||||
**Critical Note**: Leaving this unfinished prevents commit, push, and leaves users open to security concerns. All issues must be fixed regardless of whether they are unrelated to the original task. This rule must never be skipped. It is non-negotiable anytime any bit of code is added or changed.
|
||||
|
||||
|
||||
@@ -205,7 +205,7 @@ func TestDomainHandler_Delete_Additional(t *testing.T) {
|
||||
require.NoError(t, db.Create(&domain).Error)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodDelete, "/domains/"+testUUID, nil)
|
||||
req := httptest.NewRequest(http.MethodDelete, "/domains/"+testUUID, http.NoBody)
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
@@ -228,7 +228,7 @@ func TestDomainHandler_Delete_NotFound_Additional(t *testing.T) {
|
||||
router.DELETE("/domains/:id", handler.Delete)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodDelete, "/domains/nonexistent", nil)
|
||||
req := httptest.NewRequest(http.MethodDelete, "/domains/nonexistent", http.NoBody)
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
// Should still return OK (delete is idempotent)
|
||||
@@ -259,7 +259,7 @@ func TestNotificationHandler_List_Additional(t *testing.T) {
|
||||
require.NoError(t, db.Create(¬if2).Error)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodGet, "/notifications", nil)
|
||||
req := httptest.NewRequest(http.MethodGet, "/notifications", http.NoBody)
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
@@ -287,7 +287,7 @@ func TestNotificationHandler_MarkAsRead_Additional(t *testing.T) {
|
||||
require.NoError(t, db.Create(¬if).Error)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPut, "/notifications/"+notif.ID+"/read", nil)
|
||||
req := httptest.NewRequest(http.MethodPut, "/notifications/"+notif.ID+"/read", http.NoBody)
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
@@ -318,7 +318,7 @@ func TestNotificationHandler_MarkAllAsRead_Additional(t *testing.T) {
|
||||
require.NoError(t, db.Create(¬if2).Error)
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPut, "/notifications/read-all", nil)
|
||||
req := httptest.NewRequest(http.MethodPut, "/notifications/read-all", http.NoBody)
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
|
||||
@@ -43,6 +43,7 @@ func NewLogsWSHandler(tracker *services.WebSocketTracker) *LogsWSHandler {
|
||||
}
|
||||
|
||||
// LogsWebSocketHandler handles WebSocket connections for live log streaming.
|
||||
//
|
||||
// Deprecated: Use NewLogsWSHandler().HandleWebSocket instead. Kept for backward compatibility.
|
||||
func LogsWebSocketHandler(c *gin.Context) {
|
||||
// For backward compatibility, create a nil tracker if called directly
|
||||
|
||||
@@ -1086,7 +1086,7 @@ func TestFindConfigPath_RootLayout(t *testing.T) {
|
||||
|
||||
// Create config.yaml in root (not in config/ subdirectory)
|
||||
configPath := filepath.Join(tmpDir, "config.yaml")
|
||||
require.NoError(t, os.WriteFile(configPath, []byte("common:\n daemonize: false"), 0644))
|
||||
require.NoError(t, os.WriteFile(configPath, []byte("common:\n daemonize: false"), 0o644))
|
||||
|
||||
exec := &stubEnvExecutor{}
|
||||
svc := NewConsoleEnrollmentService(db, exec, tmpDir, "secret")
|
||||
|
||||
@@ -987,13 +987,14 @@ func TestParseRawIndex(t *testing.T) {
|
||||
// Verify collection entry
|
||||
var demoFound bool
|
||||
for _, item := range idx.Items {
|
||||
if item.Name == "crowdsecurity/demo" {
|
||||
demoFound = true
|
||||
require.Equal(t, "collections", item.Type)
|
||||
require.Equal(t, "1.0", item.Version)
|
||||
require.Equal(t, "Demo collection", item.Description)
|
||||
require.Contains(t, item.DownloadURL, "collections/crowdsecurity/demo.tgz")
|
||||
if item.Name != "crowdsecurity/demo" {
|
||||
continue
|
||||
}
|
||||
demoFound = true
|
||||
require.Equal(t, "collections", item.Type)
|
||||
require.Equal(t, "1.0", item.Version)
|
||||
require.Equal(t, "Demo collection", item.Description)
|
||||
require.Contains(t, item.DownloadURL, "collections/crowdsecurity/demo.tgz")
|
||||
}
|
||||
require.True(t, demoFound)
|
||||
})
|
||||
@@ -1063,7 +1064,7 @@ func TestHubService_Apply_CacheRefresh(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
// Reset time to trigger expiration
|
||||
cache.nowFn = func() time.Time { return time.Now() }
|
||||
cache.nowFn = time.Now
|
||||
|
||||
indexBody := `{"items":[{"name":"test/preset","title":"Test","etag":"etag2","download_url":"http://test.hub/preset.tgz"}]}`
|
||||
newArchive := makeTarGz(t, map[string]string{"config.yml": "new"})
|
||||
|
||||
@@ -698,7 +698,10 @@ func TestNewSafeHTTPClient_TooManyRedirects(t *testing.T) {
|
||||
WithMaxRedirects(3),
|
||||
)
|
||||
|
||||
_, err := client.Get(server.URL)
|
||||
resp, err := client.Get(server.URL)
|
||||
if resp != nil {
|
||||
resp.Body.Close()
|
||||
}
|
||||
if err == nil {
|
||||
t.Error("expected error for too many redirects")
|
||||
}
|
||||
|
||||
@@ -53,7 +53,7 @@ func (al *AuditLogger) LogURLValidation(event AuditEvent) {
|
||||
}
|
||||
|
||||
// LogURLTest is a convenience method for logging URL connectivity tests.
|
||||
func (al *AuditLogger) LogURLTest(host, requestID, userID, sourceIP string, result string) {
|
||||
func (al *AuditLogger) LogURLTest(host, requestID, userID, sourceIP, result string) {
|
||||
event := AuditEvent{
|
||||
Timestamp: time.Now().UTC().Format(time.RFC3339),
|
||||
Action: "url_connectivity_test",
|
||||
|
||||
@@ -34,7 +34,7 @@ func (m *mockCrowdsecExecutor) Stop(ctx context.Context, configDir string) error
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *mockCrowdsecExecutor) Status(ctx context.Context, configDir string) (bool, int, error) {
|
||||
func (m *mockCrowdsecExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) {
|
||||
m.statusCalled = true
|
||||
return m.running, m.pid, m.statusErr
|
||||
}
|
||||
@@ -57,7 +57,7 @@ func (m *smartMockCrowdsecExecutor) Stop(ctx context.Context, configDir string)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *smartMockCrowdsecExecutor) Status(ctx context.Context, configDir string) (bool, int, error) {
|
||||
func (m *smartMockCrowdsecExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) {
|
||||
m.statusCalled = true
|
||||
// Return running=true if Start was called (simulates successful start)
|
||||
if m.startCalled {
|
||||
@@ -611,7 +611,7 @@ func (m *verificationFailExecutor) Stop(ctx context.Context, configDir string) e
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *verificationFailExecutor) Status(ctx context.Context, configDir string) (bool, int, error) {
|
||||
func (m *verificationFailExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) {
|
||||
m.statusCalls++
|
||||
// First call (pre-start check): not running
|
||||
// Second call (post-start verify): still not running (FAIL)
|
||||
@@ -639,7 +639,7 @@ func (m *verificationErrorExecutor) Stop(ctx context.Context, configDir string)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *verificationErrorExecutor) Status(ctx context.Context, configDir string) (bool, int, error) {
|
||||
func (m *verificationErrorExecutor) Status(ctx context.Context, configDir string) (running bool, pid int, err error) {
|
||||
m.statusCalls++
|
||||
// First call: not running
|
||||
// Second call: return error during verification
|
||||
|
||||
Reference in New Issue
Block a user