diff --git a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java index 9411e8d..df3b0a7 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java +++ b/src/main/java/it/cnr/isti/workflow/manager/controllers/ExecutionsController.java @@ -336,7 +336,7 @@ public class ExecutionsController { public ExecutionView prepareFileInputs(@PathVariable String executionId, @PathVariable String nodeId, @PathVariable String inputName, @RequestParam MultipartFile file, @AuthenticationPrincipal LoginEntity userDetails) { try { - File myFile = File.createTempFile(inputName, file.getOriginalFilename()); + File myFile = createUploadTempFile(inputName, file.getOriginalFilename()); file.transferTo(myFile); return ExecutionView.fromExecution( executionService.prepareInput(visibleExecution(executionId, userDetails).getId(), nodeId, inputName, myFile)); @@ -346,6 +346,28 @@ public class ExecutionsController { } + /** + * Names the temp file an upload lands in. + * + *

Both parts are sanitised because neither is the uploader's mistake to pay for: + * {@link File#createTempFile} rejects a prefix shorter than three characters, so an input named + * "dc" used to fail the whole upload with a 500 that reads as "Failed to upload file" in the UI, + * and a filename carrying a path separator fails the same way. The names only make the temp file + * readable - nothing downstream reads meaning out of them beyond the extension. + */ + private static File createUploadTempFile(String inputName, String originalFilename) throws IOException { + StringBuilder prefix = new StringBuilder(sanitizeTempNamePart(inputName)); + while (prefix.length() < 3) { + prefix.append('_'); + } + String suffix = sanitizeTempNamePart(originalFilename); + return File.createTempFile(prefix.toString(), suffix.isEmpty() ? ".tmp" : suffix); + } + + private static String sanitizeTempNamePart(String value) { + return value == null ? "" : value.replaceAll("[^A-Za-z0-9._-]", ""); + } + @PutMapping(path = "{executionId}/node/{nodeId}/input/{inputName}/files", consumes = "multipart/form-data") @Operation(summary = "Prepares file array inputs", description = "Prepares multiple file inputs for a specific execution node input.") public ExecutionView prepareFileArrayInputs(@PathVariable String executionId, @PathVariable String nodeId, @@ -354,7 +376,7 @@ public class ExecutionsController { List preparedFiles = new ArrayList<>(); try { for (MultipartFile file : files) { - File myFile = File.createTempFile(inputName, file.getOriginalFilename()); + File myFile = createUploadTempFile(inputName, file.getOriginalFilename()); file.transferTo(myFile); preparedFiles.add(myFile); } diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java index 56dc73e..d47fa49 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/ExecutionControllerTest.java @@ -744,6 +744,41 @@ public class ExecutionControllerTest { .get(new it.cnr.isti.workflow.manager.executions.FieldKey(block.getId(), "response"))); } + @Test + public void fileInputUploadAcceptsAnInputNameShorterThanATempFilePrefix() { + // File.createTempFile rejects a prefix shorter than three characters, and the input's name + // was handed to it verbatim: uploading to an input called "dc" threw IllegalArgumentException + // past the IOException catch, so the client only saw a 500 reading "Failed to upload file". + Block agentBlock = + blocksController.create(it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentBlockConfiguration.builder() + .name("Read uploaded doc") + .model("llama3.1:8b") + .prompt("Summarize the attached document") + .uploadInputs(List.of(new it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentUploadInput( + "dc", + it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentUploadInput.MCPAgentUploadKind.DOCUMENT, + false))) + .build()); + + FlowCreateRequest request = new FlowCreateRequest( + "Short Upload Input Flow", + "Flow whose upload input name is shorter than a temp file prefix", + FlowData.builder().block(agentBlock).build()); + ResponseEntity createdFlow = flowController.createFlow(request, testUser()); + ExecutionView execution = executionsController.create(createdFlow.getBody().id(), testUser()); + + org.springframework.mock.web.MockMultipartFile upload = new org.springframework.mock.web.MockMultipartFile( + "file", "plan.pdf", "application/pdf", "%PDF-1.7\nbody".getBytes(java.nio.charset.StandardCharsets.US_ASCII)); + + ExecutionView withFile = executionsController.prepareFileInputs(execution.getId(), agentBlock.getId(), "dc", + upload, testUser()); + + Object storedInput = withFile.getContext().getInputs().values().stream().findFirst().orElse(null); + org.junit.jupiter.api.Assertions.assertNotNull(storedInput); + org.junit.jupiter.api.Assertions.assertTrue(String.valueOf(storedInput).endsWith("plan.pdf"), + "the uploaded file keeps its extension, so whatever reads it back still sees a PDF: " + storedInput); + } + @Test public void cancelExecutionClearsRuntimeStateAndMarksExecutionCancelled() { LLMDescriptor llmDescriptor = LLMDescriptor.builder()