From 0724149ad3414572cbb335c7c60c54d0bc755e03 Mon Sep 17 00:00:00 2001 From: Robin de Silva Jayasinghe Date: Wed, 1 Jul 2026 10:11:36 +0200 Subject: [PATCH 1/5] security: fail fast when multiple AEM validation-service bindings are present Previously, the first aem-validation-service binding returned by the runtime was selected via findFirst() and reused for every AEM messaging binding. In deployments with more than one such binding this made the validation context non-deterministic and could validate an AEM endpoint against a mismatched validation tenant. Reject configurations with more than one aem-validation-service binding at startup with a clear ServiceException listing the offending binding names. The empty and single-binding cases are unchanged. --- .../AemMessagingServiceConfiguration.java | 32 ++++++++++--- .../AemMessagingServiceConfigurationTest.java | 46 +++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java index 1bcbc2e..c05d87b 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java @@ -17,6 +17,7 @@ import com.sap.cloud.environment.servicebinding.api.ServiceBinding; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,12 +48,13 @@ public void services(CdsRuntimeConfigurer configurer) { || binding.getTags().contains(BINDING_AEM_LABEL)) .toList(); Optional validationBinding = - configurer - .getCdsRuntime() - .getEnvironment() - .getServiceBindings() - .filter(binding -> ServiceBindingUtils.matches(binding, BINDING_AEM_VALIDATION_LABEL)) - .findFirst(); + selectValidationBinding( + configurer + .getCdsRuntime() + .getEnvironment() + .getServiceBindings() + .filter(binding -> ServiceBindingUtils.matches(binding, BINDING_AEM_VALIDATION_LABEL)) + .toList()); if (bindings.isEmpty()) { logger.info("No service bindings with name '{}' found", BINDING_AEM_LABEL); @@ -176,4 +178,22 @@ private MessagingService createMessagingService( return outboxed(service, serviceConfig, runtime); } + + static Optional selectValidationBinding(List validationBindings) { + if (validationBindings.size() > 1) { + String names = + validationBindings.stream() + .map(b -> b.getName().orElse("")) + .collect(Collectors.joining(", ")); + throw new ServiceException( + "Found " + + validationBindings.size() + + " '" + + BINDING_AEM_VALIDATION_LABEL + + "' service bindings (" + + names + + "). Exactly one validation binding is supported per application."); + } + return validationBindings.isEmpty() ? Optional.empty() : Optional.of(validationBindings.get(0)); + } } diff --git a/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfigurationTest.java b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfigurationTest.java index 54c39ae..f084224 100644 --- a/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfigurationTest.java +++ b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfigurationTest.java @@ -3,15 +3,21 @@ import static com.sap.cds.services.outbox.OutboxService.unboxed; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.sap.cds.services.Service; +import com.sap.cds.services.ServiceException; import com.sap.cds.services.environment.CdsProperties; import com.sap.cds.services.environment.CdsProperties.Messaging.MessagingServiceConfig; import com.sap.cds.services.impl.environment.SimplePropertiesProvider; import com.sap.cds.services.outbox.OutboxService; import com.sap.cds.services.runtime.CdsRuntimeConfigurer; +import com.sap.cloud.environment.servicebinding.api.DefaultServiceBinding; +import com.sap.cloud.environment.servicebinding.api.ServiceBinding; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; @@ -225,4 +231,44 @@ void testServiceConfigurationWithEnabledSkipManagement() { assertTrue(services.stream().findFirst().get().getSkipManagement()); } + + @Test + void selectValidationBinding_returnsEmpty_whenNoneProvided() { + Optional result = + AemMessagingServiceConfiguration.selectValidationBinding(List.of()); + assertTrue(result.isEmpty()); + } + + @Test + void selectValidationBinding_returnsSingle_whenExactlyOneProvided() { + ServiceBinding only = validationBinding("primary"); + Optional result = + AemMessagingServiceConfiguration.selectValidationBinding(List.of(only)); + assertTrue(result.isPresent()); + assertEquals("primary", result.get().getName().orElseThrow()); + } + + @Test + void selectValidationBinding_throws_whenMultipleProvided() { + ServiceBinding first = validationBinding("primary"); + ServiceBinding second = validationBinding("secondary"); + + ServiceException ex = + assertThrows( + ServiceException.class, + () -> AemMessagingServiceConfiguration.selectValidationBinding(List.of(first, second))); + + assertTrue(ex.getMessage().contains("primary"), ex.getMessage()); + assertTrue(ex.getMessage().contains("secondary"), ex.getMessage()); + assertTrue(ex.getMessage().contains("aem-validation-service"), ex.getMessage()); + } + + private static ServiceBinding validationBinding(String name) { + return DefaultServiceBinding.builder() + .copy(Map.of()) + .withName(name) + .withServiceName(AemMessagingServiceConfiguration.BINDING_AEM_VALIDATION_LABEL) + .withCredentials(Map.of()) + .build(); + } } From 38745e7379420ab61d1be572b8f177b16439be24 Mon Sep 17 00:00:00 2001 From: Robin de Silva Jayasinghe Date: Wed, 1 Jul 2026 10:14:04 +0200 Subject: [PATCH 2/5] security: reject non-AMQPS URIs and cross-host mismatches in AEM binding The AMQP URI is taken from the service binding without local checks and then used as the destination URI for a JmsConnectionFactory that receives an OAuth bearer token as its password override. A binding advertising 'amqp://' would therefore send the bearer token over an unencrypted transport, and a binding whose AMQP host does not match the management endpoint host would send the bearer to a broker outside the validated tenant. Validate the AMQP URI before building the destination: require scheme 'amqps' (case insensitive), and, when the management URI is present in the same binding, require the AMQP host to equal the management host. Malformed AMQP URIs are rejected; a malformed management URI is tolerated because getServiceUri() will fail explicitly later. --- .../jms/AemMessagingConnectionProvider.java | 40 +++++++++ .../AemMessagingConnectionProviderTest.java | 89 +++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java index 9df13ee..5eb4397 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java @@ -13,6 +13,7 @@ import com.sap.cloud.sdk.cloudplatform.connectivity.ServiceBindingDestinationOptions; import jakarta.jms.Connection; import java.net.URI; +import java.net.URISyntaxException; import java.util.Map; import java.util.Optional; import java.util.function.BiFunction; @@ -44,6 +45,7 @@ public AemMessagingConnectionProvider(ServiceBinding binding) { () -> new ServiceException( "AMQP URI key is missing in the service binding. Please check the service binding configuration.")); + validateAmqpUri(amqpUri, endpointView.getUri().orElse(null), binding.getName().orElse("")); amqpUri = amqpUri + SASL_MECHANISM_URI_PARAMETER; ServiceBindingDestinationOptions options = @@ -109,4 +111,42 @@ private String getToken(String value) { return token; } + + static void validateAmqpUri(String amqpUri, String managementUri, String bindingName) { + URI parsed; + try { + parsed = new URI(amqpUri); + } catch (URISyntaxException e) { + throw new ServiceException( + "Invalid AMQP URI in binding '" + bindingName + "': " + e.getMessage(), e); + } + if (parsed.getScheme() == null || !parsed.getScheme().equalsIgnoreCase("amqps")) { + throw new ServiceException( + "AMQP URI in binding '" + + bindingName + + "' must use scheme 'amqps' (got '" + + parsed.getScheme() + + "')."); + } + if (managementUri != null) { + URI mgmt; + try { + mgmt = new URI(managementUri); + } catch (URISyntaxException e) { + return; + } + String amqpHost = parsed.getHost(); + String mgmtHost = mgmt.getHost(); + if (amqpHost != null && mgmtHost != null && !amqpHost.equalsIgnoreCase(mgmtHost)) { + throw new ServiceException( + "AMQP URI host '" + + amqpHost + + "' does not match the management URI host '" + + mgmtHost + + "' in binding '" + + bindingName + + "'."); + } + } + } } diff --git a/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java new file mode 100644 index 0000000..9a0ebd7 --- /dev/null +++ b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java @@ -0,0 +1,89 @@ +package com.sap.cds.feature.messaging.aem.jms; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.sap.cds.services.ServiceException; +import org.junit.jupiter.api.Test; + +public class AemMessagingConnectionProviderTest { + + @Test + void validateAmqpUri_acceptsAmqpsWithMatchingHost() { + assertDoesNotThrow( + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps://broker.example.com:5671", + "https://broker.example.com:943", + "my-binding")); + } + + @Test + void validateAmqpUri_acceptsAmqpsWhenManagementUriIsNull() { + assertDoesNotThrow( + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps://broker.example.com:5671", null, "my-binding")); + } + + @Test + void validateAmqpUri_isCaseInsensitiveOnScheme() { + assertDoesNotThrow( + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "AMQPS://broker.example.com:5671", null, "my-binding")); + } + + @Test + void validateAmqpUri_rejectsPlainAmqp() { + ServiceException ex = + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqp://broker.example.com:5672", null, "my-binding")); + assertTrue(ex.getMessage().contains("amqps"), ex.getMessage()); + assertTrue(ex.getMessage().contains("my-binding"), ex.getMessage()); + } + + @Test + void validateAmqpUri_rejectsUnexpectedScheme() { + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "ws://broker.example.com", null, "my-binding")); + } + + @Test + void validateAmqpUri_rejectsMalformedUri() { + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "::not a uri::", null, "my-binding")); + } + + @Test + void validateAmqpUri_rejectsHostMismatchAgainstManagementUri() { + ServiceException ex = + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps://attacker.example.com:5671", + "https://broker.example.com:943", + "my-binding")); + assertTrue(ex.getMessage().contains("attacker.example.com"), ex.getMessage()); + assertTrue(ex.getMessage().contains("broker.example.com"), ex.getMessage()); + } + + @Test + void validateAmqpUri_toleratesMalformedManagementUri() { + assertDoesNotThrow( + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps://broker.example.com:5671", "::not a uri::", "my-binding")); + } +} From 7d77dd92e986a3feee0d511851ddba326084ecfd Mon Sep 17 00:00:00 2001 From: Robin de Silva Jayasinghe Date: Wed, 1 Jul 2026 13:42:01 +0200 Subject: [PATCH 3/5] security: reject null-host URIs and log skipped host check Address PR review feedback on validateAmqpUri: - Fail fast when the AMQP URI parses but has no host component (e.g., opaque URIs like 'amqps:foo'), rather than silently skipping the cross-host check. - Fail fast when the management URI parses but has no host component. - When the management URI is malformed and cannot be parsed, log a warning naming the binding instead of silently swallowing. Also drop the java.util.stream.Collectors import from AemMessagingServiceConfiguration in favour of String.join with Stream.toList(), which is already the idiom used elsewhere in the file. --- .../jms/AemMessagingConnectionProvider.java | 16 ++++++++++-- .../AemMessagingServiceConfiguration.java | 7 +++-- .../AemMessagingConnectionProviderTest.java | 26 +++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java index 38bc832..fe748ea 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java @@ -138,16 +138,28 @@ static void validateAmqpUri(String amqpUri, String managementUri, String binding + parsed.getScheme() + "')."); } + String amqpHost = parsed.getHost(); + if (amqpHost == null) { + throw new ServiceException( + "AMQP URI in binding '" + bindingName + "' has no host component."); + } if (managementUri != null) { URI mgmt; try { mgmt = new URI(managementUri); } catch (URISyntaxException e) { + logger.warn( + "Skipping host-consistency check for binding '{}': management URI is malformed ({}).", + bindingName, + e.getMessage()); return; } - String amqpHost = parsed.getHost(); String mgmtHost = mgmt.getHost(); - if (amqpHost != null && mgmtHost != null && !amqpHost.equalsIgnoreCase(mgmtHost)) { + if (mgmtHost == null) { + throw new ServiceException( + "Management URI in binding '" + bindingName + "' has no host component."); + } + if (!amqpHost.equalsIgnoreCase(mgmtHost)) { throw new ServiceException( "AMQP URI host '" + amqpHost diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java index c05d87b..80be64b 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/service/AemMessagingServiceConfiguration.java @@ -17,7 +17,6 @@ import com.sap.cloud.environment.servicebinding.api.ServiceBinding; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -182,9 +181,9 @@ private MessagingService createMessagingService( static Optional selectValidationBinding(List validationBindings) { if (validationBindings.size() > 1) { String names = - validationBindings.stream() - .map(b -> b.getName().orElse("")) - .collect(Collectors.joining(", ")); + String.join( + ", ", + validationBindings.stream().map(b -> b.getName().orElse("")).toList()); throw new ServiceException( "Found " + validationBindings.size() diff --git a/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java index b97c1d3..2fe9fcc 100644 --- a/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java +++ b/cds-feature-advanced-event-mesh/src/test/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProviderTest.java @@ -170,4 +170,30 @@ void validateAmqpUri_toleratesMalformedManagementUri() { AemMessagingConnectionProvider.validateAmqpUri( "amqps://broker.example.com:5671", "::not a uri::", "my-binding")); } + + @Test + void validateAmqpUri_rejectsAmqpUriWithoutHost() { + // Opaque URI parses successfully but has no host component. + ServiceException ex = + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps:opaque-body", null, "my-binding")); + assertTrue(ex.getMessage().contains("my-binding"), ex.getMessage()); + assertTrue(ex.getMessage().contains("host"), ex.getMessage()); + } + + @Test + void validateAmqpUri_rejectsManagementUriWithoutHost() { + // Management URI parses but yields a null host — must not silently pass the check. + ServiceException ex = + assertThrows( + ServiceException.class, + () -> + AemMessagingConnectionProvider.validateAmqpUri( + "amqps://broker.example.com:5671", "https:opaque-body", "my-binding")); + assertTrue(ex.getMessage().contains("my-binding"), ex.getMessage()); + assertTrue(ex.getMessage().contains("host"), ex.getMessage()); + } } From 061a643f0c5026adca94762a33abf82b3d08b8a9 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 1 Jul 2026 13:48:38 +0200 Subject: [PATCH 4/5] Update cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com> --- .../jms/AemMessagingConnectionProvider.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java index fe748ea..1ad60c6 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java @@ -152,7 +152,28 @@ static void validateAmqpUri(String amqpUri, String managementUri, String binding "Skipping host-consistency check for binding '{}': management URI is malformed ({}).", bindingName, e.getMessage()); - return; + mgmt = null; + } + if (mgmt != null) { + String mgmtHost = mgmt.getHost(); + if (mgmtHost == null) { + throw new ServiceException( + "Management URI in binding '" + bindingName + "' has no host component."); + } + if (!amqpHost.equalsIgnoreCase(mgmtHost)) { + throw new ServiceException( + "AMQP URI host '" + + amqpHost + + "' does not match the management URI host '" + + mgmtHost + + "' in binding '" + + bindingName + + "'."); + } + } + } + } + } String mgmtHost = mgmt.getHost(); if (mgmtHost == null) { From 2aa6c2f3b4e318a75b3e13cd36ec9b97c806ef17 Mon Sep 17 00:00:00 2001 From: Robin de Silva Jayasinghe Date: Wed, 1 Jul 2026 14:08:24 +0200 Subject: [PATCH 5/5] fix: remove orphaned code left by web-UI suggestion apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 061a643 accepted a review suggestion in the GitHub web UI. The suggestion replaced the malformed-mgmt-URI 'return' with 'mgmt = null' plus a null-guarded check, but the web UI insertion left the old block dangling outside the class body — producing 'class, interface, enum, or record expected' compilation errors at lines 178, 179, 182, and 192 and turning PR #223's build red. Drop the orphaned trailing block. The new logic (mgmt = null on parse failure, then null-guarded host check) is preserved unchanged; 72/72 tests pass locally. --- .../jms/AemMessagingConnectionProvider.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java index 1ad60c6..91aa333 100644 --- a/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java +++ b/cds-feature-advanced-event-mesh/src/main/java/com/sap/cds/feature/messaging/aem/jms/AemMessagingConnectionProvider.java @@ -172,24 +172,5 @@ static void validateAmqpUri(String amqpUri, String managementUri, String binding } } } - } - - } - String mgmtHost = mgmt.getHost(); - if (mgmtHost == null) { - throw new ServiceException( - "Management URI in binding '" + bindingName + "' has no host component."); - } - if (!amqpHost.equalsIgnoreCase(mgmtHost)) { - throw new ServiceException( - "AMQP URI host '" - + amqpHost - + "' does not match the management URI host '" - + mgmtHost - + "' in binding '" - + bindingName - + "'."); - } - } } }