Merge pull request #185 from rostilos/1.5.7-rc

rostilos · web-flow · commit 57938b767c17 · 2026-05-22T11:47:09.000+03:00
feat: Update validation constraints for analysis type and enhance err…
diff --git a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequest.java b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequest.java
@@ -14,7 +14,7 @@ public class BranchProcessRequest implements AnalysisProcessRequest {
     @NotBlank(message = "Commit hash is required")
     public String commitHash;
 
-    @NotBlank(message = "Specify analysis type")
+    @NotNull(message = "Specify analysis type")
     public AnalysisType analysisType;
 
     /**
diff --git a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequest.java b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequest.java
@@ -23,7 +23,7 @@ public class PrProcessRequest implements AnalysisProcessRequest {
     @NotBlank(message = "Commit hash is required")
     public String commitHash;
 
-    @NotBlank(message = "Specify analysis type")
+    @NotNull(message = "Specify analysis type")
     public AnalysisType analysisType;
     
     /**
diff --git a/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequestTest.java b/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/BranchProcessRequestTest.java
@@ -1,5 +1,7 @@
 package org.rostilos.codecrow.analysisengine.dto.request.processor;
 
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -114,4 +116,18 @@ void shouldImplementAnalysisProcessRequest() {
             assertThat(request).isInstanceOf(AnalysisProcessRequest.class);
         }
     }
+
+    @Nested
+    @DisplayName("Validation")
+    class ValidationTests {
+
+        @Test
+        @DisplayName("should use enum-compatible constraint for analysis type")
+        void shouldUseEnumCompatibleConstraintForAnalysisType() throws NoSuchFieldException {
+            var analysisTypeField = BranchProcessRequest.class.getField("analysisType");
+
+            assertThat(analysisTypeField.getAnnotation(NotNull.class)).isNotNull();
+            assertThat(analysisTypeField.getAnnotation(NotBlank.class)).isNull();
+        }
+    }
 }
diff --git a/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequestTest.java b/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/dto/request/processor/PrProcessRequestTest.java
@@ -1,5 +1,7 @@
 package org.rostilos.codecrow.analysisengine.dto.request.processor;
 
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
@@ -109,4 +111,18 @@ void shouldImplementAnalysisProcessRequest() {
             assertThat(request).isInstanceOf(AnalysisProcessRequest.class);
         }
     }
+
+    @Nested
+    @DisplayName("Validation")
+    class ValidationTests {
+
+        @Test
+        @DisplayName("should use enum-compatible constraint for analysis type")
+        void shouldUseEnumCompatibleConstraintForAnalysisType() throws NoSuchFieldException {
+            var analysisTypeField = PrProcessRequest.class.getField("analysisType");
+
+            assertThat(analysisTypeField.getAnnotation(NotNull.class)).isNotNull();
+            assertThat(analysisTypeField.getAnnotation(NotBlank.class)).isNull();
+        }
+    }
 }
diff --git a/java-ecosystem/services/pipeline-agent/src/it/java/org/rostilos/codecrow/pipelineagent/PipelineAgentSecurityIT.java b/java-ecosystem/services/pipeline-agent/src/it/java/org/rostilos/codecrow/pipelineagent/PipelineAgentSecurityIT.java
@@ -111,17 +111,14 @@ void processingEndpoint_validAuth_notUnauthorized() {
             projectAuthRequest(projectId)
                 .body("""
                     {
-                        "projectId": %d,
-                        "pullRequestId": 1,
-                        "sourceBranch": "feature",
-                        "targetBranch": "main"
+                        "projectId": %d
                     }
                     """.formatted(projectId))
             .when()
                 .post("/api/processing/webhook/pr")
             .then()
-                // Should pass authentication; may still fail for other reasons
-                .statusCode(not(401));
+                // Authentication passed; the intentionally incomplete body fails validation.
+                .statusCode(400);
         }
     }
 }
diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/controller/ProviderPipelineActionController.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/controller/ProviderPipelineActionController.java
@@ -3,6 +3,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import jakarta.validation.Valid;
 import org.rostilos.codecrow.core.dto.project.ProjectDTO;
+import org.rostilos.codecrow.core.model.codeanalysis.AnalysisType;
 import org.rostilos.codecrow.core.model.job.Job;
 import org.rostilos.codecrow.analysisengine.dto.request.processor.AnalysisProcessRequest;
 import org.rostilos.codecrow.analysisengine.dto.request.processor.BranchProcessRequest;
@@ -149,6 +150,9 @@ public ResponseEntity<StreamingResponseBody> handleBranchWebhook(
         } catch (IOException e) {
             log.error("Failed to parse request or read archive", e);
             return createErrorResponse(HttpServletResponse.SC_BAD_REQUEST, "invalid_request", e.getMessage());
+        } catch (IllegalArgumentException e) {
+            log.error("Invalid webhook request: {}", e.getMessage());
+            return createErrorResponse(HttpServletResponse.SC_BAD_REQUEST, "invalid_request", e.getMessage());
         }
     }
 
@@ -164,7 +168,9 @@ private ResponseEntity<StreamingResponseBody> processWebhookWithJob(
             JobAwareWebhookProcessor webhookProcessorFunc
     ) {
         try {
+            validateProjectIdPresent(payload);
             validateAuthentication(authenticationPrincipal, payload);
+            validateProcessPayload(payload);
 
             if (job != null) {
                 pipelineJobService.getJobService().startJob(job);
@@ -269,6 +275,12 @@ private ResponseEntity<StreamingResponseBody> processWebhookWithJob(
         }
     }
 
+    private void validateProjectIdPresent(AnalysisProcessRequest payload) {
+        if (payload == null || payload.getProjectId() == null) {
+            throw new IllegalArgumentException("Project ID is required");
+        }
+    }
+
     private void validateAuthentication(ProjectDTO authenticationPrincipal, AnalysisProcessRequest payload) {
         if (!payload.getProjectId().equals(authenticationPrincipal.id())) {
             log.warn("Request body projectId {} does not match JWT projectId {}",
@@ -277,6 +289,27 @@ private void validateAuthentication(ProjectDTO authenticationPrincipal, Analysis
         }
     }
 
+    private void validateProcessPayload(AnalysisProcessRequest payload) {
+        requireNotBlank(payload.getCommitHash(), "Commit hash is required");
+        requireNotBlank(payload.getTargetBranchName(), "Target branch name is required");
+        if (payload.getAnalysisType() == null) {
+            throw new IllegalArgumentException("Analysis type is required");
+        }
+
+        if (payload instanceof PrProcessRequest prPayload) {
+            requireNotBlank(prPayload.getSourceBranchName(), "Source branch name is required");
+            if (prPayload.getAnalysisType() == AnalysisType.PR_REVIEW && prPayload.getPullRequestId() == null) {
+                throw new IllegalArgumentException("Pull Request ID is required for PR_REVIEW analysis type");
+            }
+        }
+    }
+
+    private void requireNotBlank(String value, String message) {
+        if (value == null || value.isBlank()) {
+            throw new IllegalArgumentException(message);
+        }
+    }
+
     private PipelineActionProcessor.EventConsumer createEventConsumer(LinkedBlockingQueue<String> queue) {
         return event -> {
             try {
@@ -371,6 +404,7 @@ private void enqueueEOF(LinkedBlockingQueue<String> queue) {
 
     private ResponseEntity<StreamingResponseBody> createErrorResponse(int status, String error, String message) {
         return ResponseEntity.status(status)
+                .contentType(MediaType.APPLICATION_JSON)
                 .body(outputStream -> {
                     PrintWriter w = new PrintWriter(outputStream, true, StandardCharsets.UTF_8);
                     try {

Original file line number	Diff line number	Diff line change
`@@ -111,17 +111,14 @@ void processingEndpoint_validAuth_notUnauthorized() {`
`111`	`111`	`projectAuthRequest(projectId)`
`112`	`112`	`.body("""`
`113`	`113`	`{`
`114`		`- "projectId": %d,`
`115`		`- "pullRequestId": 1,`
`116`		`- "sourceBranch": "feature",`
`117`		`- "targetBranch": "main"`
	`114`	`+ "projectId": %d`
`118`	`115`	`}`
`119`	`116`	`""".formatted(projectId))`
`120`	`117`	`.when()`
`121`	`118`	`.post("/api/processing/webhook/pr")`
`122`	`119`	`.then()`
`123`		`- // Should pass authentication; may still fail for other reasons`
`124`		`- .statusCode(not(401));`
	`120`	`+ // Authentication passed; the intentionally incomplete body fails validation.`
	`121`	`+ .statusCode(400);`
`125`	`122`	`}`
`126`	`123`	`}`
`127`	`124`	`}`