diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java index 4d4cb11..10afd7c 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/bias/BiasImpactJudge.java @@ -30,6 +30,7 @@ import it.cnr.isti.workflow.manager.flows.model.bias.BlockBiasAnnotation; import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCode; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; import tools.jackson.databind.JsonNode; @@ -79,7 +80,7 @@ public class BiasImpactJudge { public BiasJudgeSummary evaluate(BiasImpactReport comparison, LLMDescriptor descriptor, ExecutionObject baseline, ExecutionObject biased) { LLMProvider provider = resolveProvider(descriptor.provider()); - String authorization = resolveAuthorization(provider, baseline); + ProviderCredential authorization = resolveAuthorization(provider, baseline); String interventions = describeInterventions(biased, comparison.annotationIds()); List targets = BiasJudgeTarget.collect(comparison); @@ -117,7 +118,7 @@ public class BiasImpactJudge { errors, verdicts); } - private BiasJudgeVerdict judgeTarget(LLMProvider provider, LLMDescriptor descriptor, String authorization, + private BiasJudgeVerdict judgeTarget(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential authorization, String interventions, BiasJudgeTarget target, List errors) { String prompt = pairPrompt(interventions, target); try { @@ -150,7 +151,7 @@ public class BiasImpactJudge { * this, where the answer is the real one - and the extractor below pulls the object out of * whatever prose it arrives wrapped in. */ - private String askForJson(LLMProvider provider, LLMDescriptor descriptor, String authorization, String prompt) { + private String askForJson(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential authorization, String prompt) { String json = null; try { json = provider.generateJson(descriptor.model(), prompt, authorization, descriptor.parameters()); @@ -183,7 +184,7 @@ public class BiasImpactJudge { * per-pair verdicts in code, so that re-reading a report cannot show a different headline than * the pairs it is made of. */ - private String narrate(LLMProvider provider, LLMDescriptor descriptor, String authorization, String interventions, + private String narrate(LLMProvider provider, LLMDescriptor descriptor, ProviderCredential authorization, String interventions, BiasImpactReport comparison, Map verdicts, BiasJudgeImpactLevel impact, BiasJudgeAttribution attribution, List errors) { String prompt = """ @@ -504,7 +505,7 @@ public class BiasImpactJudge { * run is reached the same way as the models that produced it, and the internal provider - the * one a local install has - needs no credential at all. */ - private String resolveAuthorization(LLMProvider provider, ExecutionObject baseline) { + private ProviderCredential resolveAuthorization(LLMProvider provider, ExecutionObject baseline) { try { return credentialResolver.resolve(provider, baseline.getProvidedAuthorizations(), baseline.getContext().getResolvedExecutionVariables()); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ChatInteractionExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ChatInteractionExecutor.java index ea829f7..565a7da 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ChatInteractionExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ChatInteractionExecutor.java @@ -29,6 +29,7 @@ import it.cnr.isti.workflow.manager.llms.ChatMessage; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.LLMDescriptorInputBinding; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; @Component @@ -65,7 +66,7 @@ public class ChatInteractionExecutor implements BlockExecutor messages = history.stream().map(this::parseHistoryLine).collect(Collectors.toList()); @@ -91,12 +92,12 @@ public class ChatInteractionExecutor implements BlockExecutor updatedHistory = new ArrayList<>(history); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ConditionalExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ConditionalExecutor.java index db6c325..8ffa4de 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ConditionalExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/ConditionalExecutor.java @@ -27,6 +27,7 @@ import it.cnr.isti.workflow.manager.executions.bias.runtime.BiasRuntimeSupport; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.LLMDescriptorInputBinding; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; @Component @@ -84,12 +85,12 @@ public class ConditionalExecutor implements BlockExecutor ExecutionEventLogger eventLogger) { LLMDescriptor llmDescriptor = LLMDescriptorInputBinding.resolve(config.getLlmDescriptor(), inputValues, executionVariables); LLMProvider llmProvider = SimulatedChatSupport.resolveProvider(llmProviders, llmDescriptor.provider()); - String authorization = credentialResolver.resolve(llmProvider, authorizations, executionVariables); + ProviderCredential credential = credentialResolver.resolve(llmProvider, authorizations, executionVariables); String prompt = buildLlmPrompt(config, inputValues, executionVariables); ModelParameterReporting.reportUnsupported(llmProvider, llmDescriptor.parameters(), llmDescriptor.model(), eventLogger); - String response = llmProvider.generate(llmDescriptor.model(), prompt, authorization, + String response = llmProvider.generate(llmDescriptor.model(), prompt, credential, llmDescriptor.parameters()); try { return parseBooleanResponse(response); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java index c9ddd21..83fcc23 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java @@ -26,6 +26,7 @@ import it.cnr.isti.workflow.manager.executions.steps.Input; import it.cnr.isti.workflow.manager.executions.bias.runtime.BiasRuntimeSupport; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; @Component @@ -64,7 +65,7 @@ public class HumanDecisionExecutor implements BlockExecutor "%s= %s".formatted(input.getDescriptor().getName(), input.getValue())) @@ -87,7 +88,7 @@ public class HumanDecisionExecutor implements BlockExecutor { if (llmProvider == null) { throw new IllegalArgumentException("Provider not found: " + llmDescriptor.provider()); } - String authorization = credentialResolver.resolve(llmProvider, authorizations, executionVariables); + ProviderCredential credential = credentialResolver.resolve(llmProvider, authorizations, executionVariables); ModelParameterReporting.reportUnsupported(llmProvider, llmDescriptor.parameters(), llmDescriptor.model(), eventLogger); - String response = llmProvider.generate(llmDescriptor.model(), prompt, authorization, + String response = llmProvider.generate(llmDescriptor.model(), prompt, credential, llmDescriptor.parameters()); if (eventLogger != null) { diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/MCPAgentChatExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/MCPAgentChatExecutor.java index 8037176..75a2a9d 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/MCPAgentChatExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/MCPAgentChatExecutor.java @@ -28,6 +28,7 @@ import it.cnr.isti.workflow.manager.executions.bias.runtime.BiasRuntimeSupport; import it.cnr.isti.workflow.manager.llms.ConfigurableInputBinding; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; import it.cnr.isti.workflow.manager.mcp.MCPAgentService; import it.cnr.isti.workflow.manager.mcp.MCPSharedSessionRegistry; @@ -74,7 +75,7 @@ public class MCPAgentChatExecutor implements BlockExecutor new IllegalArgumentException("Provider not found: " + providerName)); } - static String resolveAuthorization(LLMCredentialResolver credentialResolver, LLMProvider provider, + static ProviderCredential resolveAuthorization(LLMCredentialResolver credentialResolver, LLMProvider provider, Map authorizations, Map executionVariables) { return credentialResolver.resolve(provider, authorizations, executionVariables); } @@ -72,7 +73,7 @@ final class SimulatedChatSupport { } static String generateSimulatorMessage(LLMProvider simulatorProvider, LLMDescriptor simulatorDescriptor, - String simulatorAuthorization, String goalDescription, List inputs, List history, int turn, + ProviderCredential simulatorCredential, String goalDescription, List inputs, List history, int turn, int maxTurns, String blockLabel) { String prompt = """ ###SIMULATED_CHAT_MESSAGE### @@ -89,7 +90,7 @@ final class SimulatedChatSupport { Return only the next user message for the conversation. """.formatted(goalDescription, turn, maxTurns, formatInputs(inputs), formatHistory(history)); - String response = simulatorProvider.generate(simulatorDescriptor.model(), prompt, simulatorAuthorization, + String response = simulatorProvider.generate(simulatorDescriptor.model(), prompt, simulatorCredential, simulatorDescriptor.parameters()); if (!StringUtils.hasText(response)) { throw new IllegalArgumentException("Simulated " + blockLabel + " produced an empty message"); @@ -98,7 +99,7 @@ final class SimulatedChatSupport { } static String generateSimulatorFinalResponse(LLMProvider simulatorProvider, LLMDescriptor simulatorDescriptor, - String simulatorAuthorization, String goalDescription, List inputs, List history, String blockLabel) { + ProviderCredential simulatorCredential, String goalDescription, List inputs, List history, String blockLabel) { String prompt = """ ###SIMULATED_CHAT_FINAL### Goal: @@ -112,7 +113,7 @@ final class SimulatedChatSupport { Return only the final response value that the simulated user would submit. """.formatted(goalDescription, formatInputs(inputs), formatHistory(history)); - String response = simulatorProvider.generate(simulatorDescriptor.model(), prompt, simulatorAuthorization, + String response = simulatorProvider.generate(simulatorDescriptor.model(), prompt, simulatorCredential, simulatorDescriptor.parameters()); if (!StringUtils.hasText(response)) { throw new IllegalArgumentException("Simulated " + blockLabel + " produced an empty final response"); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/SwitchExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/SwitchExecutor.java index 934be2a..4b9723f 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/SwitchExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/SwitchExecutor.java @@ -33,6 +33,7 @@ import it.cnr.isti.workflow.manager.ios.IOType; import it.cnr.isti.workflow.manager.llms.LLMDescriptor; import it.cnr.isti.workflow.manager.llms.LLMDescriptorInputBinding; import it.cnr.isti.workflow.manager.llms.LLMCredentialResolver; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; @Component @@ -112,12 +113,12 @@ public class SwitchExecutor implements BlockExecutor { ExecutionEventLogger eventLogger) { LLMDescriptor llmDescriptor = LLMDescriptorInputBinding.resolve(config.getLlmDescriptor(), inputValues, executionVariables); LLMProvider llmProvider = SimulatedChatSupport.resolveProvider(llmProviders, llmDescriptor.provider()); - String authorization = credentialResolver.resolve(llmProvider, authorizations, executionVariables); + ProviderCredential credential = credentialResolver.resolve(llmProvider, authorizations, executionVariables); String prompt = buildLlmPrompt(config, inputValues, executionVariables, allowedOutputs); ModelParameterReporting.reportUnsupported(llmProvider, llmDescriptor.parameters(), llmDescriptor.model(), eventLogger); - String response = llmProvider.generate(llmDescriptor.model(), prompt, authorization, + String response = llmProvider.generate(llmDescriptor.model(), prompt, credential, llmDescriptor.parameters()); return parseSelectedOutput(response, allowedOutputs); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolver.java b/src/main/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolver.java index be9046a..c3f4a85 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolver.java +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolver.java @@ -23,7 +23,13 @@ public class LLMCredentialResolver { this.userSecretService = userSecretService; } - public String resolve(LLMProvider provider, Map authorizations, + /** + * Returns a {@link ProviderCredential}, carrying the endpoint alongside the secret value for a + * provider whose {@code requiresEndpoint()} is true. Not yet called from every executor: see + * {@code docs/llm-providers-openai-remote-ollama-plan-2026-09-17.md} step 11 for which call + * sites still resolve through the plain secret value only, and why. + */ + public ProviderCredential resolve(LLMProvider provider, Map authorizations, Map executionVariables) { if (!provider.requiresAuthorization()) { return null; @@ -38,7 +44,7 @@ public class LLMCredentialResolver { throw new IllegalArgumentException( "A user-owned execution is required to resolve credential for provider: " + provider.getName()); } - return userSecretService.resolveValue(owner, credentialId.trim(), provider.getName()); + return userSecretService.resolveCredential(owner, credentialId.trim(), provider.getName()); } private String asText(Object value) { diff --git a/src/test/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolverTest.java b/src/test/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolverTest.java new file mode 100644 index 0000000..2cf8ca2 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/llms/LLMCredentialResolverTest.java @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import it.cnr.isti.workflow.manager.executions.ExecutionRuntimeContextSupport; +import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; +import it.cnr.isti.workflow.manager.vault.UserSecretService; + +/** + * Not previously covered on its own - only ever exercised indirectly through a full execution. + * This pins the branching {@code resolve} itself does (no authorization required, no credential + * id, no owner) before it becomes even harder to isolate from what calls it. + */ +class LLMCredentialResolverTest { + + private final UserSecretService userSecretService = mock(UserSecretService.class); + private final LLMCredentialResolver resolver = new LLMCredentialResolver(userSecretService); + + private LLMProvider provider(boolean requiresAuthorization) { + return new LLMProvider() { + @Override + public String getName() { + return "Test"; + } + + @Override + public List getRegisteredModels() { + return List.of(); + } + + @Override + public String generate(String model, String prompt) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean requiresAuthorization() { + return requiresAuthorization; + } + }; + } + + @Test + void returnsNullWithoutTouchingTheVaultWhenTheProviderNeedsNoAuthorization() { + ProviderCredential result = resolver.resolve(provider(false), Map.of(), Map.of()); + + assertNull(result); + verify(userSecretService, never()).resolveCredential(any(), any(), any()); + } + + @Test + void throwsWhenTheAuthorizationsMapHasNoEntryForTheProvidersKey() { + LLMProvider provider = provider(true); + + assertThrows(IllegalArgumentException.class, + () -> resolver.resolve(provider, Map.of(), Map.of(ExecutionRuntimeContextSupport.EXECUTION_OWNER, "alice"))); + } + + @Test + void throwsWhenTheCredentialIdIsBlank() { + LLMProvider provider = provider(true); + Map authorizations = Map.of(provider.authorizationKey(), " "); + + assertThrows(IllegalArgumentException.class, + () -> resolver.resolve(provider, authorizations, + Map.of(ExecutionRuntimeContextSupport.EXECUTION_OWNER, "alice"))); + } + + @Test + void throwsWhenThereIsNoOwnedExecutionContext() { + LLMProvider provider = provider(true); + Map authorizations = Map.of(provider.authorizationKey(), "secret-1"); + + assertThrows(IllegalArgumentException.class, () -> resolver.resolve(provider, authorizations, Map.of())); + } + + @Test + void resolvesThroughUserSecretServiceAndReturnsItsCredentialAsIs() { + LLMProvider provider = provider(true); + Map authorizations = Map.of(provider.authorizationKey(), " secret-1 "); + Map executionVariables = Map.of(ExecutionRuntimeContextSupport.EXECUTION_OWNER, "alice"); + ProviderCredential expected = new ProviderCredential("api-key", "https://gateway.example.com"); + when(userSecretService.resolveCredential("alice", "secret-1", "Test")).thenReturn(expected); + + ProviderCredential result = resolver.resolve(provider, authorizations, executionVariables); + + assertEquals(expected, result); + // The id is trimmed before being used - a pasted value with surrounding whitespace must + // still resolve. + verify(userSecretService).resolveCredential(eq("alice"), eq("secret-1"), eq("Test")); + } + +}