From a54adb825260214254e07ed98f0996c5dc612a78 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 11:06:21 -0800 Subject: [PATCH 01/10] Check Transact Enterprise entitlements before loading an entitled feature Before DBOS loads a Transact Enterprise feature, it now asks DBOS Conductor whether the organization is entitled to it, as Python (dbos/_enterprise.py), TypeScript (src/enterprise.ts) and Go (dbos/entitlements.go) do. No Java feature needs an entitlement yet; suspend mode, auto-upgrade and Kafka consumers will each add their config switch to Entitlements.enabledFeatures as they land. - DBOSConfig gains transactEnterpriseKey, with withTransactEnterpriseKey, masked in toString. The Conductor key stands in for it when unset. The Spring Boot starter maps dbos.transact-enterprise-key. - Entitlements.require sends the key as a bearer token to GET /v2/entitlements, with a 10 s timeout and no redirects, and refuses launch with an IllegalStateException citing the DBOS Conductor Software License when: - an entitlement is missing or not true; - the key is rejected (401); - the org is past its plan limit (403 with X-DBOS-Error: past_limit); - the Conductor has no entitlements route (a bare 404); - any other 4xx comes back. - Only an explicitly configured Conductor is asked without a key. An unreachable or malformed answer fails closed, except from DBOS's hosted Conductor, which warns and starts anyway (as do its 403, 407, 408, 425 and 429 without the past_limit code). DBOS Cloud skips the check. Credentials in the Conductor URL never reach the request or the errors. - DBOSExecutor runs the check at construction, before the Conductor provider is loaded, so a refusal fails launch before anything starts. - The ServiceLoader lookup behind ConductorProviders moves into EnterpriseProviders.load(type, feature), so each entitled feature's provider interface gets the same class-loader handling and the same "requires dev.dbos:transact-enterprise" error. --- .../spring/DBOSAutoConfiguration.java | 3 + .../dbos/transact/spring/DBOSProperties.java | 14 + .../spring/DBOSAutoConfigurationTest.java | 2 + .../transact/spring/DBOSPropertiesTest.java | 7 + .../conductor/ConductorProviders.java | 73 +-- .../dev/dbos/transact/config/DBOSConfig.java | 124 +++-- .../dbos/transact/execution/DBOSExecutor.java | 11 + .../internal/EnterpriseProviders.java | 101 ++++ .../dbos/transact/internal/Entitlements.java | 361 ++++++++++++++ .../config/TransactEnterpriseKeyTest.java | 51 ++ .../transact/internal/EntitlementsTest.java | 462 ++++++++++++++++++ 11 files changed, 1113 insertions(+), 96 deletions(-) create mode 100644 transact/src/main/java/dev/dbos/transact/internal/EnterpriseProviders.java create mode 100644 transact/src/main/java/dev/dbos/transact/internal/Entitlements.java create mode 100644 transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java create mode 100644 transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java diff --git a/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSAutoConfiguration.java b/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSAutoConfiguration.java index 99e1d20f1..03ab74987 100644 --- a/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSAutoConfiguration.java +++ b/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSAutoConfiguration.java @@ -131,6 +131,9 @@ private DBOSConfig buildConfig(DBOSProperties props, String springAppName) { config = config.withConductorDomain(props.getConductor().getDomain()); } config = config.withConductorMetadataOnlyMode(props.getConductor().isMetadataOnlyMode()); + if (props.getTransactEnterpriseKey() != null) { + config = config.withTransactEnterpriseKey(props.getTransactEnterpriseKey()); + } if (props.getApplication().getVersion() != null) { config = config.withAppVersion(props.getApplication().getVersion()); } diff --git a/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSProperties.java b/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSProperties.java index a45898a42..463d66897 100644 --- a/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSProperties.java +++ b/transact-spring-boot-starter/src/main/java/dev/dbos/transact/spring/DBOSProperties.java @@ -28,6 +28,12 @@ public class DBOSProperties { /** Executor ID for this instance. */ private String executorId; + /** + * The DBOS key that entitles this application to Transact Enterprise features. When not set, the + * Conductor key stands in for it. + */ + private String transactEnterpriseKey; + /** Whether to enable workflow patching. Defaults to {@code false}. */ private boolean enablePatching = false; @@ -242,6 +248,14 @@ public void setExecutorId(String executorId) { this.executorId = executorId; } + public String getTransactEnterpriseKey() { + return transactEnterpriseKey; + } + + public void setTransactEnterpriseKey(String transactEnterpriseKey) { + this.transactEnterpriseKey = transactEnterpriseKey; + } + public boolean isEnablePatching() { return enablePatching; } diff --git a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java index 7560282a6..8060f7c0f 100644 --- a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java +++ b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java @@ -73,6 +73,7 @@ void dbosConfigReflectsOptionalProperties() { "dbos.conductor.key=my-key", "dbos.conductor.domain=my-domain", "dbos.conductor.metadata-only-mode=true", + "dbos.transact-enterprise-key=my-enterprise-key", "dbos.application.version=1.2.3", "dbos.executor-id=exec-1", "dbos.datasource.schema=my_schema", @@ -88,6 +89,7 @@ void dbosConfigReflectsOptionalProperties() { assertThat(config.conductorKey()).isEqualTo("my-key"); assertThat(config.conductorDomain()).isEqualTo("my-domain"); assertThat(config.conductorMetadataOnlyMode()).isTrue(); + assertThat(config.transactEnterpriseKey()).isEqualTo("my-enterprise-key"); assertThat(config.appVersion()).isEqualTo("1.2.3"); assertThat(config.executorId()).isEqualTo("exec-1"); assertThat(config.databaseSchema()).isEqualTo("my_schema"); diff --git a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSPropertiesTest.java b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSPropertiesTest.java index fd93963b7..8b895d886 100644 --- a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSPropertiesTest.java +++ b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSPropertiesTest.java @@ -38,6 +38,7 @@ void defaults() { assertNull(props.getConductor().getDomain()); assertFalse(props.getConductor().isMetadataOnlyMode()); assertNull(props.getExecutorId()); + assertNull(props.getTransactEnterpriseKey()); assertNull(props.getDatasource().getSchema()); assertNull(props.getSchedulerPollingInterval()); assertNotNull(props.getDatasource()); @@ -103,6 +104,12 @@ void bindsConductorProperties() { assertTrue(props.getConductor().isMetadataOnlyMode()); } + @Test + void bindsTheTransactEnterpriseKey() { + var props = bind(Map.of("dbos.transact-enterprise-key", "my-enterprise-key")); + assertEquals("my-enterprise-key", props.getTransactEnterpriseKey()); + } + @Test void bindsIdentityProperties() { var props = bind(Map.of("dbos.executor-id", "exec-1")); diff --git a/transact/src/main/java/dev/dbos/transact/conductor/ConductorProviders.java b/transact/src/main/java/dev/dbos/transact/conductor/ConductorProviders.java index c81413764..1eafe299b 100644 --- a/transact/src/main/java/dev/dbos/transact/conductor/ConductorProviders.java +++ b/transact/src/main/java/dev/dbos/transact/conductor/ConductorProviders.java @@ -1,20 +1,13 @@ package dev.dbos.transact.conductor; -import dev.dbos.transact.DBOS; +import dev.dbos.transact.internal.EnterpriseProviders; -import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.List; -import java.util.ServiceConfigurationError; -import java.util.ServiceLoader; -import java.util.regex.Pattern; /** Finds the {@link ConductorProvider} on the class path. */ public final class ConductorProviders { - static final String ENTERPRISE_ARTIFACT = "dev.dbos:transact-enterprise"; - - private static final Pattern RELEASE = Pattern.compile("\\d+\\.\\d+\\.\\d+"); - private static final Pattern MINOR = Pattern.compile("(\\d+\\.\\d+)\\..*"); + // Connecting to Conductor needs the enterprise artifact, but no entitlement. + private static final String FEATURE = "Connecting to DBOS Conductor"; private ConductorProviders() {} @@ -27,66 +20,10 @@ private ConductorProviders() {} * initialize */ public static ConductorProvider load() { - var loaders = new ArrayList(); - var context = Thread.currentThread().getContextClassLoader(); - if (context != null) { - loaders.add(context); - } - var own = ConductorProvider.class.getClassLoader(); - if (own != context) { - loaders.add(own); - } - - // One instance per provider class, constructed once: a loader that delegates to its parent sees - // the parent's registrations too, so duplicates are dropped before they are instantiated. - var providers = new LinkedHashMap, ConductorProvider>(); - try { - for (var loader : loaders) { - ServiceLoader.load(ConductorProvider.class, loader).stream() - .filter(provider -> !providers.containsKey(provider.type())) - .forEach(provider -> providers.put(provider.type(), provider.get())); - } - } catch (ServiceConfigurationError e) { - // A provider's constructor rejected this DBOS, e.g. for a version mismatch. Its own message - // says why; ServiceLoader's names the provider. - var cause = e.getCause(); - var message = - cause != null && cause.getMessage() != null - ? e.getMessage() + ": " + cause.getMessage() - : e.getMessage(); - throw new IllegalStateException(message, e); - } - return select(List.copyOf(providers.values()), DBOS.version()); + return EnterpriseProviders.load(ConductorProvider.class, FEATURE); } static ConductorProvider select(List providers, String version) { - if (providers.isEmpty()) { - throw new IllegalStateException( - "Connecting to DBOS Conductor requires " - + ENTERPRISE_ARTIFACT - + " on the class path. " - + versionAdvice(version)); - } - if (providers.size() > 1) { - var names = providers.stream().map(p -> p.getClass().getName()).toList(); - throw new IllegalStateException( - "Found more than one DBOS Conductor provider on the class path: " + names); - } - return providers.get(0); - } - - // Previews are numbered by commits in each repo, so only a release names the exact version. - private static String versionAdvice(String version) { - if (version != null && RELEASE.matcher(version).matches()) { - return String.format( - "Add %s:%s, the same version as dev.dbos:transact", ENTERPRISE_ARTIFACT, version); - } - var minor = version == null ? null : MINOR.matcher(version); - if (minor != null && minor.matches()) { - return String.format( - "Add a %s %s build, as dev.dbos:transact is %s", - ENTERPRISE_ARTIFACT, minor.group(1), version); - } - return "Add the " + ENTERPRISE_ARTIFACT + " version that matches dev.dbos:transact"; + return EnterpriseProviders.select(ConductorProvider.class, providers, version, FEATURE); } } diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index c43e9c502..6437f15ed 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -76,6 +76,9 @@ * workflow data: no inputs, outputs, errors, step outputs, events, notifications, streams, * schedule context, exports or imports. Requires {@code dev.dbos:transact-enterprise}, which * enforces it + * @param transactEnterpriseKey the DBOS key that entitles this application to Transact Enterprise + * features; must not be empty if non-null. When it is null, {@code conductorKey} stands in for + * it. Masked in {@link #toString()} */ public record DBOSConfig( @NonNull String appName, @@ -100,7 +103,8 @@ public record DBOSConfig( @Nullable Duration notificationCoalesceInterval, @Nullable Integer databasePollingConcurrency, @Nullable Duration systemDatabaseIdleTransactionTimeout, - boolean conductorMetadataOnlyMode) { + boolean conductorMetadataOnlyMode, + @Nullable String transactEnterpriseKey) { /** Default flush interval for batched stream and workflow-event notifications. */ public static final Duration DEFAULT_NOTIFICATION_COALESCE_INTERVAL = Duration.ofMillis(10); @@ -131,6 +135,10 @@ public record DBOSConfig( if (conductorKey != null && conductorKey.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.conductorKey must not be empty if specified"); } + if (transactEnterpriseKey != null && transactEnterpriseKey.isEmpty()) { + throw new IllegalArgumentException( + "DBOSConfig.transactEnterpriseKey must not be empty if specified"); + } if (conductorDomain != null && conductorDomain.isEmpty()) { throw new IllegalArgumentException( "DBOSConfig.conductorDomain must not be empty if specified"); @@ -226,7 +234,8 @@ public DBOSConfig(DBOSConfig other) { other.notificationCoalesceInterval, other.databasePollingConcurrency, other.systemDatabaseIdleTransactionTimeout, - other.conductorMetadataOnlyMode); + other.conductorMetadataOnlyMode, + other.transactEnterpriseKey); } /** @@ -248,7 +257,8 @@ public DBOSConfig(DBOSConfig other) { null, // notificationCoalesceInterval null, // databasePollingConcurrency null, // systemDatabaseIdleTransactionTimeout - false); // conductorMetadataOnlyMode + false, // conductorMetadataOnlyMode + null); // transactEnterpriseKey } /** @@ -301,7 +311,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code databaseUrl} set to {@code v}. */ @@ -329,7 +340,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code dbUser} set to {@code v}. */ @@ -357,7 +369,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code dbPassword} set to {@code v}. */ @@ -385,7 +398,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -418,7 +432,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -451,7 +466,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -484,7 +500,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code migrate} set to {@code v}. */ @@ -512,7 +529,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -543,7 +561,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code conductorDomain} set to {@code v}. */ @@ -571,7 +590,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code conductorExecutorMetadata} set to {@code v}. */ @@ -599,7 +619,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code appVersion} set to {@code v}. */ @@ -627,7 +648,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code executorId} set to {@code v}. */ @@ -655,7 +677,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code databaseSchema} set to {@code v}. */ @@ -683,7 +706,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code enablePatching} set to {@code true}. */ @@ -721,7 +745,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -845,7 +870,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code serializer} set to {@code v}. */ @@ -873,7 +899,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code schedulerPollingInterval} set to {@code v}. */ @@ -901,7 +928,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** Returns a copy of this config with {@code useListenNotify} set to {@code v}. */ @@ -929,7 +957,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -969,7 +998,8 @@ public DBOSConfig(DBOSConfig other) { v, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -1008,7 +1038,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, v, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -1053,7 +1084,8 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, v, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey); } /** @@ -1087,10 +1119,44 @@ public DBOSConfig(DBOSConfig other) { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, + v, + transactEnterpriseKey); + } + + /** + * Returns a copy of this config with {@code transactEnterpriseKey} set to {@code v}: the DBOS key + * that entitles this application to Transact Enterprise features. When it is not set, the {@code + * conductorKey} stands in for it. + */ + public @NonNull DBOSConfig withTransactEnterpriseKey(@Nullable String v) { + return new DBOSConfig( + appName, + databaseUrl, + dbUser, + dbPassword, + dataSource, + adminServer, + adminServerPort, + migrate, + conductorKey, + conductorDomain, + conductorExecutorMetadata, + appVersion, + executorId, + databaseSchema, + enablePatching, + listenQueues, + serializer, + schedulerPollingInterval, + useListenNotify, + notificationCoalesceInterval, + databasePollingConcurrency, + systemDatabaseIdleTransactionTimeout, + conductorMetadataOnlyMode, v); } - // Override toString to mask the DB password + // Override toString to mask the DB password and the Transact Enterprise key @Override public String toString() { return """ @@ -1100,7 +1166,8 @@ public String toString() { conductorExecutorMetadata=%s, appVersion=%s, executorId=%s, enablePatching=%s, \ listenQueues=%s, serializer=%s, schedulerPollingInterval=%s, useListenNotify=%s, \ notificationCoalesceInterval=%s, databasePollingConcurrency=%s, \ - systemDatabaseIdleTransactionTimeout=%s, conductorMetadataOnlyMode=%s] + systemDatabaseIdleTransactionTimeout=%s, conductorMetadataOnlyMode=%s, \ + transactEnterpriseKey=%s] """ .formatted( appName, @@ -1124,6 +1191,7 @@ public String toString() { notificationCoalesceInterval, databasePollingConcurrency, systemDatabaseIdleTransactionTimeout, - conductorMetadataOnlyMode); + conductorMetadataOnlyMode, + transactEnterpriseKey == null ? null : "***"); } } diff --git a/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java b/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java index 73f350021..574dd331f 100644 --- a/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java +++ b/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java @@ -36,6 +36,7 @@ import dev.dbos.transact.exceptions.DBOSWorkflowFunctionNotFoundException; import dev.dbos.transact.exceptions.DBOSWorkflowIdInUseException; import dev.dbos.transact.internal.AppVersionComputer; +import dev.dbos.transact.internal.Entitlements; import dev.dbos.transact.internal.StepCheckpointStore; import dev.dbos.transact.internal.Validation; import dev.dbos.transact.internal.WorkflowRegistry; @@ -246,6 +247,16 @@ public DBOSExecutor(DBOSConfig config) { logger.warn(msg); } + // Every Transact Enterprise feature needs Conductor to report the organization entitled to it. + // Checked before anything is started, so a refusal fails launch cleanly. + var conductorDomain = config.conductorDomain(); + Entitlements.require( + Entitlements.enabledFeatures(config), + config.transactEnterpriseKey(), + config.conductorKey(), + conductorDomain == null || conductorDomain.isBlank() ? null : conductorDomain.trim(), + dbosCloud); + // Found before anything is started, so a missing provider fails launch cleanly. An // application that will not connect, including one on DBOS Cloud without Conductor settings, // needs no provider. diff --git a/transact/src/main/java/dev/dbos/transact/internal/EnterpriseProviders.java b/transact/src/main/java/dev/dbos/transact/internal/EnterpriseProviders.java new file mode 100644 index 000000000..abe569f0c --- /dev/null +++ b/transact/src/main/java/dev/dbos/transact/internal/EnterpriseProviders.java @@ -0,0 +1,101 @@ +package dev.dbos.transact.internal; + +import dev.dbos.transact.DBOS; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; +import java.util.regex.Pattern; + +/** + * Finds the implementation of a Transact Enterprise provider interface on the class path. Core + * declares each interface; the {@code dev.dbos:transact-enterprise} artifact registers its + * implementations with {@link ServiceLoader}. A feature that needs a license is loaded only once + * {@link Entitlements#require} has passed for it. + */ +public final class EnterpriseProviders { + public static final String ENTERPRISE_ARTIFACT = "dev.dbos:transact-enterprise"; + + private static final Pattern RELEASE = Pattern.compile("\\d+\\.\\d+\\.\\d+"); + private static final Pattern MINOR = Pattern.compile("(\\d+\\.\\d+)\\..*"); + + private EnterpriseProviders() {} + + /** + * Returns the one registered implementation of {@code type}. Looks in the thread context class + * loader and in the one that loaded DBOS, so a provider deployed beside the application is found + * when DBOS itself is a shared library, as in a servlet container. + * + * @param type the provider interface + * @param feature what needs the provider, as errors name it (e.g. "Connecting to DBOS Conductor") + * @throws IllegalStateException if none is registered, more than one is, or one fails to + * initialize + */ + public static T load(Class type, String feature) { + var loaders = new ArrayList(); + var context = Thread.currentThread().getContextClassLoader(); + if (context != null) { + loaders.add(context); + } + var own = type.getClassLoader(); + if (own != context) { + loaders.add(own); + } + + // One instance per provider class, constructed once: a loader that delegates to its parent sees + // the parent's registrations too, so duplicates are dropped before they are instantiated. + var providers = new LinkedHashMap, T>(); + try { + for (var loader : loaders) { + ServiceLoader.load(type, loader).stream() + .filter(provider -> !providers.containsKey(provider.type())) + .forEach(provider -> providers.put(provider.type(), provider.get())); + } + } catch (ServiceConfigurationError e) { + // A provider's constructor rejected this DBOS, e.g. for a version mismatch. Its own message + // says why; ServiceLoader's names the provider. + var cause = e.getCause(); + var message = + cause != null && cause.getMessage() != null + ? e.getMessage() + ": " + cause.getMessage() + : e.getMessage(); + throw new IllegalStateException(message, e); + } + return select(type, List.copyOf(providers.values()), DBOS.version(), feature); + } + + /** The one provider in {@code providers}, or an error naming the artifact to add. */ + public static T select(Class type, List providers, String version, String feature) { + if (providers.isEmpty()) { + throw new IllegalStateException( + feature + + " requires " + + ENTERPRISE_ARTIFACT + + " on the class path. " + + versionAdvice(version)); + } + if (providers.size() > 1) { + var names = providers.stream().map(p -> p.getClass().getName()).toList(); + throw new IllegalStateException( + "Found more than one " + type.getSimpleName() + " on the class path: " + names); + } + return providers.get(0); + } + + // Previews are numbered by commits in each repo, so only a release names the exact version. + private static String versionAdvice(String version) { + if (version != null && RELEASE.matcher(version).matches()) { + return String.format( + "Add %s:%s, the same version as dev.dbos:transact", ENTERPRISE_ARTIFACT, version); + } + var minor = version == null ? null : MINOR.matcher(version); + if (minor != null && minor.matches()) { + return String.format( + "Add a %s %s build, as dev.dbos:transact is %s", + ENTERPRISE_ARTIFACT, minor.group(1), version); + } + return "Add the " + ENTERPRISE_ARTIFACT + " version that matches dev.dbos:transact"; + } +} diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java new file mode 100644 index 000000000..c6636c530 --- /dev/null +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -0,0 +1,361 @@ +package dev.dbos.transact.internal; + +import dev.dbos.transact.DBOS; +import dev.dbos.transact.config.DBOSConfig; + +import java.io.IOException; +import java.net.ProxySelector; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; + +/** + * The entitlement check every Transact Enterprise feature passes before DBOS loads it: DBOS + * Conductor must report the key's organization entitled to the feature. + * + *

Transact Enterprise ({@code dev.dbos:transact-enterprise}) is proprietary, not MIT-licensed + * like this project. Its license (https://www.dbos.dev/conductor-license) forbids modifying it, and + * makes disabling or circumventing the check here a material breach. + */ +public final class Entitlements { + private static final Logger logger = LoggerFactory.getLogger(Entitlements.class); + + /** + * A Transact Enterprise feature. + * + * @param entitlement the name Conductor reports the entitlement under, in camelCase + * @param description what the feature does, as messages name it + */ + public record Feature(String entitlement, String description) {} + + static final String ENTITLEMENTS_PATH = "/v2/entitlements"; + static final Duration TIMEOUT = Duration.ofSeconds(10); + static final String HOSTED_CONDUCTOR_URL = "wss://cloud.dbos.dev/conductor/v1alpha1"; + // Conductor's machine-readable error code, in this header, for an org past its plan limit. + private static final String ERROR_CODE_HEADER = "X-DBOS-Error"; + private static final String PAST_LIMIT = "past_limit"; + // From the hosted Conductor, these mean it could not answer, not that it refused the key. + private static final Set HOSTED_UNAVAILABLE = Set.of(403, 407, 408, 425, 429); + // Read no more of an answer than this; an entitlement list is a few hundred bytes. + private static final int MAX_BODY_BYTES = 1 << 20; + private static final String LICENSE_URL = "https://www.dbos.dev/conductor-license"; + private static final String LICENSE_NOTICE = + "Transact Enterprise is licensed under the DBOS Conductor Software License (" + + LICENSE_URL + + "), which makes disabling or circumventing this check a material breach."; + private static final String KEY_HINT = + "Set transactEnterpriseKey or conductorKey in your DBOSConfig."; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private Entitlements() {} + + /** + * The enterprise features a config turns on. Each entitled feature adds its switch here when it + * lands, so the check runs for it at launch. + */ + public static List enabledFeatures(DBOSConfig config) { + return List.of(); + } + + /** + * Refuses to go on unless Conductor reports the organization entitled to every feature in {@code + * features}. A self-hosted Conductor may answer without a key. DBOS's hosted Conductor fails + * open, with a warning, when it cannot answer; any other fails closed. DBOS Cloud passes without + * a check. + * + *

A license control: the Transact Enterprise license makes disabling or circumventing it a + * material breach. + * + * @param features the features to check; nothing is checked when empty + * @param enterpriseKey the configured Transact Enterprise key, or null + * @param conductorKey the configured Conductor key, which stands in for a missing enterprise key + * @param conductorUrl the configured Conductor URL, or null for DBOS's hosted Conductor + * @param dbosCloud whether this application runs in DBOS Cloud + * @throws IllegalStateException if the check refuses, naming why + */ + public static void require( + List features, + @Nullable String enterpriseKey, + @Nullable String conductorKey, + @Nullable String conductorUrl, + boolean dbosCloud) { + if (features.isEmpty() || dbosCloud) { + return; + } + check( + features, + enterpriseKey, + conductorKey, + conductorUrl, + System.getenv("DBOS_DOMAIN"), + Entitlements::fetch); + } + + /** The answer to the entitlement request. */ + record Answer(int status, @Nullable String errorCode, String body) {} + + /** Sends the entitlement request. */ + @FunctionalInterface + interface Http { + Answer get(URI uri, Map headers) throws IOException, InterruptedException; + } + + static void check( + List features, + @Nullable String enterpriseKey, + @Nullable String conductorKey, + @Nullable String conductorUrl, + @Nullable String dbosDomain, + Http http) { + if (features.isEmpty()) { + return; + } + var names = join(features, Feature::description); + var entitlements = join(features, Feature::entitlement); + var key = enterpriseKey != null ? enterpriseKey : conductorKey; + var target = conductorUrl != null ? conductorUrl : defaultConductorUrl(dbosDomain); + // Only a Conductor named explicitly is asked without a key, and never DBOS's hosted one. + if (key == null && (conductorUrl == null || target.equals(HOSTED_CONDUCTOR_URL))) { + throw new IllegalStateException(names + " requires a DBOS key. " + KEY_HINT); + } + String url; + try { + url = entitlementUrl(target); + } catch (URISyntaxException | IllegalArgumentException e) { + throw new IllegalStateException( + names + " requires an entitled DBOS key, but the Conductor URL is invalid: " + e, e); + } + var headers = new HashMap(); + headers.put("User-Agent", "dbos-transact-java/" + DBOS.version()); + headers.put("Accept", "application/json"); + if (key != null) { + headers.put("Authorization", "Bearer " + key); + } + + String failure; + Exception cause = null; + try { + var answer = http.get(URI.create(url), headers); + var status = answer.status(); + if (status >= 200 && status < 300) { + var granted = parseEntitlements(answer.body()); + var missing = + features.stream().filter(f -> !granted.getOrDefault(f.entitlement(), false)).toList(); + if (!missing.isEmpty()) { + throw refusal( + "Your DBOS organization is not entitled to " + + join(missing, Feature::entitlement) + + " (" + + join(missing, Feature::description) + + "): its plan does not include it. Contact DBOS to enable it."); + } + return; + } + var detail = problemDetail(answer.body()); + var reason = detail != null ? ": " + detail : ""; + if (status == 403 && PAST_LIMIT.equals(answer.errorCode())) { + throw refusal( + "Your DBOS organization is past its plan limit, so it is not entitled to " + + entitlements + + " (" + + names + + "). Upgrade its plan to continue."); + } + if (target.equals(HOSTED_CONDUCTOR_URL) && HOSTED_UNAVAILABLE.contains(status)) { + // An outage, handled below. + } else if (status == 404 && detail == null) { + // A bare 404 is a missing route: a Conductor predating per-feature entitlements. + throw refusal( + names + + " requires an entitled DBOS key, but Conductor at " + + url + + " has no entitlements route (HTTP 404). Upgrade Conductor, or check its URL."); + } else if (status == 401 && key == null) { + throw new IllegalStateException( + names + + " requires a DBOS key: Conductor at " + + url + + " refused to check without one (HTTP 401)" + + reason + + ". " + + KEY_HINT); + } else if (status == 401) { + // An unknown key, or a user credential instead of an API key. + throw refusal( + names + + " requires a valid DBOS key. Conductor at " + + url + + " rejected the key (HTTP 401)" + + reason); + } else if (status < 500) { + // Any other refusal, such as the key's organization being gone, or a redirect. + throw refusal( + names + + " requires an entitled DBOS key, but Conductor at " + + url + + " refused the check (HTTP " + + status + + ")" + + reason); + } + failure = "HTTP " + status + reason; + } catch (MalformedAnswer e) { + failure = e.getMessage(); + cause = e; + } catch (IOException e) { + failure = e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); + cause = e; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException( + "Interrupted while checking the DBOS key's entitlements with Conductor at " + url, e); + } + if (target.equals(HOSTED_CONDUCTOR_URL)) { + logger.warn( + "Could not verify the DBOS key's entitlements with Conductor at {} ({}); starting anyway.", + url, + failure); + return; + } + // Any other Conductor fails closed, so an unreachable URL cannot stand in for a key. + var refused = + refusal( + names + + " requires an entitled DBOS key, but checking it with Conductor at " + + url + + " failed (" + + failure + + "). Retry once Conductor is reachable."); + if (cause != null) { + refused.initCause(cause); + } + throw refused; + } + + /** DBOS's hosted Conductor, on the domain {@code DBOS_DOMAIN} names. */ + static String defaultConductorUrl(@Nullable String dbosDomain) { + var domain = dbosDomain == null || dbosDomain.isBlank() ? "cloud.dbos.dev" : dbosDomain.trim(); + return "wss://" + domain + "/conductor/v1alpha1"; + } + + /** The entitlement route of the Conductor at {@code conductorUrl}, over HTTP(S). */ + static String entitlementUrl(String conductorUrl) throws URISyntaxException { + var uri = new URI(conductorUrl); + var scheme = uri.getScheme(); + var authority = uri.getRawAuthority(); + if (scheme == null || authority == null) { + throw new IllegalArgumentException("not an absolute URL: " + conductorUrl); + } + scheme = + switch (scheme.toLowerCase()) { + case "wss" -> "https"; + case "ws" -> "http"; + default -> scheme; + }; + // Without any user:password@, which would otherwise be sent and quoted in errors. + var host = authority.substring(authority.lastIndexOf('@') + 1); + // The hosted websocket URL carries an API version; routes sit beside it. + var path = uri.getRawPath() == null ? "" : uri.getRawPath().replaceAll("/+$", ""); + if (path.endsWith("/v1alpha1")) { + path = path.substring(0, path.length() - "/v1alpha1".length()); + } + return scheme + "://" + host + path + ENTITLEMENTS_PATH; + } + + private static Answer fetch(URI uri, Map headers) + throws IOException, InterruptedException { + var builder = HttpClient.newBuilder().connectTimeout(TIMEOUT); + // Never follow a redirect: it would carry the key to whatever host it names. + builder.followRedirects(HttpClient.Redirect.NEVER); + // Built per call, so proxy settings are read when the check runs. + var proxies = ProxySelector.getDefault(); + if (proxies != null) { + builder.proxy(proxies); + } + var request = HttpRequest.newBuilder(uri).timeout(TIMEOUT).GET(); + headers.forEach(request::header); + var response = builder.build().send(request.build(), HttpResponse.BodyHandlers.ofInputStream()); + try (var body = response.body()) { + return new Answer( + response.statusCode(), + response.headers().firstValue(ERROR_CODE_HEADER).orElse(null), + new String(body.readNBytes(MAX_BODY_BYTES), StandardCharsets.UTF_8)); + } + } + + /** An answer that is not an entitlement list, which counts as Conductor failing to answer. */ + private static final class MalformedAnswer extends Exception { + MalformedAnswer(String message) { + super(message); + } + } + + private static Map parseEntitlements(String body) throws MalformedAnswer { + JsonNode parsed; + try { + parsed = MAPPER.readTree(body); + } catch (JacksonException e) { + throw new MalformedAnswer("the response is not JSON"); + } + var entitlements = parsed == null ? null : parsed.get("entitlements"); + if (entitlements == null || !entitlements.isObject()) { + throw new MalformedAnswer("the response has no entitlements"); + } + var granted = new HashMap(); + // Anything but true grants nothing. + for (var entry : entitlements.properties()) { + granted.put(entry.getKey(), entry.getValue().isBoolean() && entry.getValue().booleanValue()); + } + return granted; + } + + /** The reason Conductor gave in its JSON error body, or null if it sent none. */ + private static @Nullable String problemDetail(String body) { + JsonNode parsed; + try { + parsed = MAPPER.readTree(body); + } catch (JacksonException e) { + // Not JSON: a bare router answer, such as "404 page not found". + return null; + } + if (parsed == null || !parsed.isObject()) { + return null; + } + for (var field : List.of("detail", "message")) { + var value = parsed.get(field); + if (value == null || value.isNull()) { + continue; + } + var text = value.isString() ? value.asString() : value.toString(); + if (!text.isEmpty()) { + return text; + } + } + return null; + } + + private static IllegalStateException refusal(String message) { + return new IllegalStateException(message + " " + LICENSE_NOTICE); + } + + private static String join(List features, Function name) { + return features.stream().map(name).collect(Collectors.joining(" and ")); + } +} diff --git a/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java b/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java new file mode 100644 index 000000000..4f935c6b5 --- /dev/null +++ b/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java @@ -0,0 +1,51 @@ +package dev.dbos.transact.config; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +public class TransactEnterpriseKeyTest { + + @Test + public void theKeyIsUnsetByDefault() { + assertNull(DBOSConfig.defaults("app").transactEnterpriseKey()); + assertNull(DBOSConfig.defaultsFromEnv("app").transactEnterpriseKey()); + } + + @Test + public void theKeyIsKeptThroughCopiesAndOtherSettings() { + var config = DBOSConfig.defaults("app").withTransactEnterpriseKey("enterprise-key"); + assertEquals("enterprise-key", config.transactEnterpriseKey()); + assertEquals("enterprise-key", new DBOSConfig(config).transactEnterpriseKey()); + var changed = + config + .withConductorKey("conductor-key") + .withConductorMetadataOnlyMode(true) + .withAppName("x"); + assertEquals("enterprise-key", changed.transactEnterpriseKey()); + assertNull(changed.withTransactEnterpriseKey(null).transactEnterpriseKey()); + } + + @Test + public void anEmptyKeyIsRefused() { + var e = + assertThrows( + IllegalArgumentException.class, + () -> DBOSConfig.defaults("app").withTransactEnterpriseKey("")); + assertTrue(e.getMessage().contains("transactEnterpriseKey"), e.getMessage()); + } + + @Test + public void theKeyIsMaskedInToString() { + var text = DBOSConfig.defaults("app").withTransactEnterpriseKey("enterprise-key").toString(); + assertFalse(text.contains("enterprise-key"), text); + assertTrue(text.contains("transactEnterpriseKey=***"), text); + assertTrue( + DBOSConfig.defaults("app").toString().contains("transactEnterpriseKey=null"), + DBOSConfig.defaults("app").toString()); + } +} diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java new file mode 100644 index 000000000..712fe9c89 --- /dev/null +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -0,0 +1,462 @@ +package dev.dbos.transact.internal; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +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 dev.dbos.transact.config.DBOSConfig; +import dev.dbos.transact.internal.Entitlements.Answer; +import dev.dbos.transact.internal.Entitlements.Feature; +import dev.dbos.transact.utils.LogCapture; + +import java.io.IOException; +import java.net.ConnectException; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import ch.qos.logback.classic.Level; +import com.sun.net.httpserver.HttpServer; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +public class EntitlementsTest { + + private static final Feature SOME_FEATURE = new Feature("someFeature", "Some feature"); + private static final Feature OTHER_FEATURE = new Feature("otherFeature", "Doing another thing"); + private static final String HOSTED_ROUTE = "https://cloud.dbos.dev/conductor/v2/entitlements"; + + /** A local stand-in for Conductor's entitlements route. */ + static final class FakeConductor implements AutoCloseable { + final HttpServer server; + final String url; + final List> requests = + Collections.synchronizedList(new ArrayList<>()); + volatile int status = 200; + volatile String body = "{\"entitlements\": {\"someFeature\": true}}"; + volatile Map headers = Map.of(); + + FakeConductor() throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext( + "/", + exchange -> { + requests.add( + new java.util.AbstractMap.SimpleEntry<>( + exchange.getRequestURI().getPath(), + exchange.getRequestHeaders().getFirst("Authorization"))); + var payload = body.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "application/json"); + headers.forEach((name, value) -> exchange.getResponseHeaders().add(name, value)); + exchange.sendResponseHeaders(status, payload.length); + try (var out = exchange.getResponseBody()) { + out.write(payload); + } + }); + server.start(); + url = "ws://127.0.0.1:" + server.getAddress().getPort(); + } + + void reject(int status, String detail) { + reject(status, detail, Map.of()); + } + + void reject(int status, String detail, Map headers) { + this.status = status; + this.body = + "{\"title\": \"Rejected\", \"status\": " + status + ", \"detail\": \"" + detail + "\"}"; + this.headers = headers; + } + + void entitle(String json) { + this.status = 200; + this.body = "{\"entitlements\": " + json + "}"; + } + + @Override + public void close() { + server.stop(0); + } + } + + private FakeConductor conductor; + + @BeforeEach + void startConductor() throws IOException { + conductor = new FakeConductor(); + } + + @AfterEach + void stopConductor() { + conductor.close(); + } + + /** Checks against the fake Conductor, over HTTP. */ + private void check(@Nullable String enterpriseKey, @Nullable String conductorKey) { + check(List.of(SOME_FEATURE), enterpriseKey, conductorKey); + } + + private void check( + List features, @Nullable String enterpriseKey, @Nullable String conductorKey) { + Entitlements.require(features, enterpriseKey, conductorKey, conductor.url, false); + } + + private IllegalStateException refused(@Nullable String enterpriseKey) { + return assertThrows(IllegalStateException.class, () -> check(enterpriseKey, null)); + } + + /** Checks against DBOS's hosted Conductor, answered by {@code http}. */ + private static void checkHosted(@Nullable String dbosDomain, Entitlements.Http http) { + Entitlements.check(List.of(SOME_FEATURE), "enterprise-key", null, null, dbosDomain, http); + } + + private static Entitlements.Http answering(int status, @Nullable String errorCode) { + return (uri, headers) -> new Answer(status, errorCode, ""); + } + + @Test + public void entitlementUrlSitsBesideTheConductorUrl() throws Exception { + assertEquals(HOSTED_ROUTE, Entitlements.entitlementUrl(Entitlements.HOSTED_CONDUCTOR_URL)); + assertEquals( + HOSTED_ROUTE, Entitlements.entitlementUrl("wss://cloud.dbos.dev/conductor/v1alpha1/")); + // Credentials in the URL are dropped, so they are never sent or quoted in errors. + assertEquals( + "https://conductor.example.com:8443/conductor/v2/entitlements", + Entitlements.entitlementUrl( + "wss://user:secret@conductor.example.com:8443/conductor/v1alpha1")); + assertEquals( + "http://[::1]:8090/v2/entitlements", + Entitlements.entitlementUrl("ws://user:secret@[::1]:8090")); + // A self-hosted Conductor serves its routes from the root. + assertEquals( + "http://localhost:8090/v2/entitlements", + Entitlements.entitlementUrl("ws://localhost:8090")); + assertEquals( + "https://conductor.example.com/v2/entitlements", + Entitlements.entitlementUrl("https://conductor.example.com/")); + } + + @Test + public void anInvalidConductorUrlIsRefused() { + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.require( + List.of(SOME_FEATURE), "enterprise-key", null, "localhost:8090", false)); + assertTrue(e.getMessage().contains("the Conductor URL is invalid"), e.getMessage()); + } + + @Test + public void theDefaultConductorFollowsTheDbosDomain() { + assertEquals(Entitlements.HOSTED_CONDUCTOR_URL, Entitlements.defaultConductorUrl(null)); + assertEquals(Entitlements.HOSTED_CONDUCTOR_URL, Entitlements.defaultConductorUrl(" ")); + assertEquals( + "wss://staging.dbos.dev/conductor/v1alpha1", + Entitlements.defaultConductorUrl("staging.dbos.dev")); + } + + @Test + public void anEntitledKeyUnlocksEnterpriseFeatures() { + check("enterprise-key", null); + check(null, "conductor-key"); + // The enterprise key is checked when both are set. + check("enterprise-key", "conductor-key"); + assertEquals( + List.of( + Map.entry("/v2/entitlements", "Bearer enterprise-key"), + Map.entry("/v2/entitlements", "Bearer conductor-key"), + Map.entry("/v2/entitlements", "Bearer enterprise-key")), + conductor.requests.stream().map(e -> Map.entry(e.getKey(), e.getValue())).toList()); + } + + @Test + public void eachFeatureNeedsItsOwnEntitlement() { + conductor.entitle("{\"someFeature\": true, \"otherFeature\": false, \"sharding\": true}"); + check("enterprise-key", null); + var e = + assertThrows( + IllegalStateException.class, + () -> check(List.of(SOME_FEATURE, OTHER_FEATURE), "enterprise-key", null)); + var message = e.getMessage(); + // Names only what the org lacks, by entitlement and by what it does. + assertTrue(message.contains("not entitled to otherFeature (Doing another thing)"), message); + assertTrue(message.contains("plan does not include it"), message); + assertTrue(message.contains("DBOS Conductor Software License"), message); + assertFalse(message.contains("someFeature"), message); + assertFalse(message.contains("enterprise-key"), message); + // One request answers for every feature. + assertEquals(2, conductor.requests.size()); + } + + @Test + public void aPastLimitOrgIsToldToUpgrade() { + // Conductor refuses a past-limit org outright, tagging the 403 with a code. + conductor.reject( + 403, + "your DBOS account is past the free trial limit", + Map.of("X-DBOS-Error", "past_limit")); + var message = refused("enterprise-key").getMessage(); + assertTrue( + message.contains("past its plan limit, so it is not entitled to someFeature"), message); + assertTrue(message.contains("Upgrade its plan"), message); + // The code, not the status or the prose, marks it: a bare 403 is another refusal. + conductor.reject(403, "your DBOS account is past the free trial limit"); + message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("refused the check (HTTP 403)"), message); + assertFalse(message.contains("Upgrade its plan"), message); + } + + @Test + public void anEntitlementConductorDoesNotReportIsNotGranted() { + // A Conductor older than the feature doesn't list it, which grants nothing. + conductor.entitle("{\"autoUpgrade\": true}"); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("not entitled to someFeature"), message); + // Anything but true grants nothing either. + conductor.entitle("{\"someFeature\": \"yes\"}"); + message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("not entitled to someFeature"), message); + } + + @Test + public void enterpriseFeaturesRequireAKey() { + // DBOS's hosted Conductor, implicit or named, is never asked without a key. + var asked = new ArrayList(); + Entitlements.Http http = + (uri, headers) -> { + asked.add(uri); + return new Answer(200, null, "{\"entitlements\": {\"someFeature\": true}}"); + }; + for (var url : new String[] {null, Entitlements.HOSTED_CONDUCTOR_URL}) { + var e = + assertThrows( + IllegalStateException.class, + () -> Entitlements.check(List.of(SOME_FEATURE), null, null, url, null, http)); + assertTrue( + e.getMessage().startsWith("Some feature requires a DBOS key. Set transactEnterpriseKey"), + e.getMessage()); + } + assertEquals(List.of(), asked); + } + + @Test + public void aSelfHostedConductorIsAskedWithoutAKey() { + // Without OAuth, a self-hosted Conductor grants every feature to any caller. + check(null, null); + assertEquals(1, conductor.requests.size()); + assertEquals("/v2/entitlements", conductor.requests.get(0).getKey()); + assertEquals(null, conductor.requests.get(0).getValue()); + // With OAuth, it wants a key. + conductor.reject(401, "this endpoint requires a DBOS API key"); + var message = refused(null).getMessage(); + assertTrue(message.contains("requires a DBOS key: Conductor at"), message); + assertTrue(message.contains("(HTTP 401): this endpoint requires a DBOS API key"), message); + assertTrue(message.contains("Set transactEnterpriseKey"), message); + } + + @Test + public void conductorCredentialsNeverReachErrors() { + conductor.reject(401, "invalid API key"); + var withCredentials = conductor.url.replace("ws://", "ws://user:secret@"); + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.require( + List.of(SOME_FEATURE), "enterprise-key", null, withCredentials, false)); + assertTrue(e.getMessage().contains("rejected the key (HTTP 401)"), e.getMessage()); + assertFalse(e.getMessage().contains("secret"), e.getMessage()); + assertFalse(e.getMessage().contains("user"), e.getMessage()); + } + + @ParameterizedTest + @ValueSource( + strings = { + "invalid API key", + // A user credential, which the route refuses in favor of an API key. + "this endpoint requires a DBOS API key" + }) + public void aRejectedKeyIsRefused(String detail) { + conductor.reject(401, detail); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("requires a valid DBOS key."), message); + assertTrue(message.contains("rejected the key (HTTP 401): " + detail), message); + assertFalse(message.contains("enterprise-key"), message); + } + + @ParameterizedTest + @CsvSource({"403, forbidden", "404, organization not found", "409, conflict"}) + public void anyOtherRefusalQuotesConductor(int status, String detail) { + conductor.reject(status, detail); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("refused the check (HTTP " + status + "): " + detail), message); + assertFalse(message.contains("rejected the key"), message); + assertFalse(message.contains("enterprise-key"), message); + } + + @Test + public void aConductorWithoutTheRouteIsRefused() { + // An older Conductor's router answers an unknown route with no problem detail. + conductor.status = 404; + conductor.body = "404 page not found"; + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("has no entitlements route (HTTP 404)"), message); + } + + @Test + public void aRedirectIsRefusedAndNeverCarriesTheKey() throws IOException { + // Following a redirect would re-send the request, headers included, to the host the Location + // names. That would hand the key to whoever answered. + try (var elsewhere = new FakeConductor()) { + var target = elsewhere.url.replace("ws://", "http://"); + conductor.status = 302; + conductor.headers = Map.of("Location", target + "/v2/entitlements"); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("refused the check (HTTP 302)"), message); + // The redirect target never saw the key, or anything at all. + assertEquals(List.of(), elsewhere.requests); + } + } + + @ParameterizedTest + @ValueSource(strings = {"[]", "{\"entitlements\": null}", "{\"entitled\": true}", "not json", ""}) + public void aMalformedAnswerCountsAsAnOutage(String body) { + conductor.body = body; + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("Retry once Conductor is reachable"), message); + } + + @Test + public void anUnreachableConductorBlocksStartup() throws IOException { + conductor.reject(503, "unavailable"); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("checking it with Conductor at"), message); + assertTrue(message.contains("failed (HTTP 503: unavailable)"), message); + // A failure without a problem detail is reported by status alone. + conductor.status = 502; + conductor.body = "Bad Gateway"; + message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("failed (HTTP 502)."), message); + // A port nothing listens on. + int closed; + try (var socket = new ServerSocket(0)) { + closed = socket.getLocalPort(); + } + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.require( + List.of(SOME_FEATURE), + "enterprise-key", + null, + "ws://127.0.0.1:" + closed, + false)); + assertTrue(e.getMessage().contains("checking it with Conductor"), e.getMessage()); + assertTrue(e.getMessage().contains("failed"), e.getMessage()); + assertFalse(e.getMessage().contains("enterprise-key"), e.getMessage()); + } + + @Test + public void aHostedConductorOutageDoesNotBlockStartup() { + var asked = new ArrayList(); + Entitlements.Http unreachable = + (uri, headers) -> { + asked.add(uri); + throw new ConnectException("connection refused"); + }; + // No Conductor URL means DBOS's hosted one, whose outage only warns. + try (var logs = new LogCapture(Entitlements.class)) { + assertDoesNotThrow(() -> checkHosted(null, unreachable)); + var warnings = logs.messages(Level.WARN, "Could not verify the DBOS key"); + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("connection refused"), warnings.get(0)); + assertFalse(warnings.get(0).contains("enterprise-key"), warnings.get(0)); + } + assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); + // Another domain is not DBOS's hosted Conductor, so its outage still refuses. + var e = + assertThrows( + IllegalStateException.class, () -> checkHosted("unreachable.example.com", unreachable)); + assertTrue(e.getMessage().contains("Retry once Conductor is reachable"), e.getMessage()); + } + + @ParameterizedTest + @ValueSource(ints = {403, 407, 408, 425, 429, 500, 503}) + public void hostedThrottlingAndProxiesCountAsOutages(int status) { + // From the hosted Conductor these mean it couldn't answer, not that it refused the key, so + // startup warns and goes on as for any hosted outage. + try (var logs = new LogCapture(Entitlements.class)) { + assertDoesNotThrow(() -> checkHosted(null, answering(status, null))); + var warnings = logs.messages(Level.WARN, "(HTTP " + status + ")"); + assertEquals(1, warnings.size(), warnings.toString()); + } + } + + @Test + public void hostedRefusalsStillRefuse() { + var e = + assertThrows( + IllegalStateException.class, () -> checkHosted(null, answering(403, "past_limit"))); + assertTrue(e.getMessage().contains("past its plan limit"), e.getMessage()); + e = assertThrows(IllegalStateException.class, () -> checkHosted(null, answering(401, null))); + assertTrue(e.getMessage().contains("rejected the key (HTTP 401)"), e.getMessage()); + } + + @Test + public void anotherConductorsThrottlingStillRefuses() { + conductor.reject(429, "slow down"); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("refused the check (HTTP 429)"), message); + } + + @Test + public void dbosCloudPassesWithoutACheck() { + Entitlements.require(List.of(SOME_FEATURE), null, null, conductor.url, true); + assertEquals(List.of(), conductor.requests); + } + + @Test + public void noFeatureNeedsNoCheck() { + Entitlements.require(List.of(), null, null, conductor.url, false); + Entitlements.require(List.of(), null, null, null, false); + assertEquals(List.of(), conductor.requests); + } + + @Test + public void aDefaultConfigTurnsOnNoEnterpriseFeature() { + assertEquals(List.of(), Entitlements.enabledFeatures(DBOSConfig.defaults("app"))); + } + + @Test + public void theRequestNamesTheSdk() { + var sent = new LinkedHashMap(); + Entitlements.check( + List.of(SOME_FEATURE), + "enterprise-key", + null, + conductor.url, + null, + (uri, headers) -> { + sent.putAll(headers); + return new Answer(200, null, "{\"entitlements\": {\"someFeature\": true}}"); + }); + assertTrue(sent.get("User-Agent").startsWith("dbos-transact-java/"), sent.toString()); + assertEquals("application/json", sent.get("Accept")); + assertEquals("Bearer enterprise-key", sent.get("Authorization")); + } +} From 393eb115aeee351c64ac8f4185e29cfc7edbd24d Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 11:39:23 -0800 Subject: [PATCH 02/10] Bound the entitlement check's whole exchange, and keep secrets out of its errors - The 10 s timeout now covers the body as well as the headers. Before, HttpRequest.timeout ended once the headers arrived, so an answer that stalled mid-body held launch forever, with neither the hosted Conductor's warning nor any other Conductor's refusal. The request now goes through sendAsync with a deadline on the whole exchange, as TypeScript's AbortSignal.timeout does. - The key is stripped, and a blank key counts as unset, so a key read from a secret file that ends in a newline works (TypeScript's fetch strips header values too). A key that still contains a control character is refused without being quoted. Before, the HTTP client's "invalid header value" error escaped with the whole key in its message. - An unusable Conductor URL is refused naming the reason, never the URL, and without the URISyntaxException as its cause. That exception's message quotes the URL, including any user:password in it. Only ws, wss, http and https are accepted as schemes. Any other exception the HTTP client throws over the request counts as a failure to reach Conductor, without passing its message on. - DBOSConfig treats a blank transactEnterpriseKey as unset instead of refusing it, so an empty placeholder such as dbos.transact-enterprise-key=${KEY:} falls back to the Conductor key, as in Python and TypeScript. --- .../spring/DBOSAutoConfigurationTest.java | 12 ++ .../dev/dbos/transact/config/DBOSConfig.java | 15 ++- .../dbos/transact/internal/Entitlements.java | 120 +++++++++++++----- .../config/TransactEnterpriseKeyTest.java | 14 +- .../transact/internal/EntitlementsTest.java | 87 +++++++++++++ 5 files changed, 205 insertions(+), 43 deletions(-) diff --git a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java index 8060f7c0f..80fea5a83 100644 --- a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java +++ b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java @@ -49,6 +49,18 @@ void dbosConfigReflectsAppNameProperty() { }); } + @Test + void anEmptyTransactEnterpriseKeyIsUnset() { + // As from a placeholder whose environment variable is unset: the Conductor key stands in. + runner + .withPropertyValues("dbos.transact-enterprise-key=") + .run( + context -> { + assertThat(context).hasNotFailed(); + assertThat(context.getBean(DBOSConfig.class).transactEnterpriseKey()).isNull(); + }); + } + @Test void dbosConfigReflectsDatasourceProperties() { runner diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index 6437f15ed..c24b50534 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -77,8 +77,8 @@ * schedule context, exports or imports. Requires {@code dev.dbos:transact-enterprise}, which * enforces it * @param transactEnterpriseKey the DBOS key that entitles this application to Transact Enterprise - * features; must not be empty if non-null. When it is null, {@code conductorKey} stands in for - * it. Masked in {@link #toString()} + * features. A blank value is the same as null, and when it is null, {@code conductorKey} stands + * in for it. Masked in {@link #toString()} */ public record DBOSConfig( @NonNull String appName, @@ -135,9 +135,10 @@ public record DBOSConfig( if (conductorKey != null && conductorKey.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.conductorKey must not be empty if specified"); } - if (transactEnterpriseKey != null && transactEnterpriseKey.isEmpty()) { - throw new IllegalArgumentException( - "DBOSConfig.transactEnterpriseKey must not be empty if specified"); + // Unset, as in the other SDKs, so an empty placeholder (an unset environment variable) + // falls back to the Conductor key instead of failing. + if (transactEnterpriseKey != null && transactEnterpriseKey.isBlank()) { + transactEnterpriseKey = null; } if (conductorDomain != null && conductorDomain.isEmpty()) { throw new IllegalArgumentException( @@ -1125,8 +1126,8 @@ public DBOSConfig(DBOSConfig other) { /** * Returns a copy of this config with {@code transactEnterpriseKey} set to {@code v}: the DBOS key - * that entitles this application to Transact Enterprise features. When it is not set, the {@code - * conductorKey} stands in for it. + * that entitles this application to Transact Enterprise features. When it is null or blank, the + * {@code conductorKey} stands in for it. */ public @NonNull DBOSConfig withTransactEnterpriseKey(@Nullable String v) { return new DBOSConfig( diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index c6636c530..5a6381518 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -10,12 +10,15 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; -import java.nio.charset.StandardCharsets; +import java.net.http.HttpTimeoutException; import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.function.Function; import java.util.stream.Collectors; @@ -53,8 +56,6 @@ public record Feature(String entitlement, String description) {} private static final String PAST_LIMIT = "past_limit"; // From the hosted Conductor, these mean it could not answer, not that it refused the key. private static final Set HOSTED_UNAVAILABLE = Set.of(403, 407, 408, 425, 429); - // Read no more of an answer than this; an entitlement list is a few hundred bytes. - private static final int MAX_BODY_BYTES = 1 << 20; private static final String LICENSE_URL = "https://www.dbos.dev/conductor-license"; private static final String LICENSE_NOTICE = "Transact Enterprise is licensed under the DBOS Conductor Software License (" @@ -105,7 +106,7 @@ public static void require( conductorKey, conductorUrl, System.getenv("DBOS_DOMAIN"), - Entitlements::fetch); + http(TIMEOUT)); } /** The answer to the entitlement request. */ @@ -129,18 +130,31 @@ static void check( } var names = join(features, Feature::description); var entitlements = join(features, Feature::entitlement); - var key = enterpriseKey != null ? enterpriseKey : conductorKey; + // A blank key is unset, and surrounding whitespace (a trailing newline from a secret file) is + // not part of it. + var key = configuredKey(enterpriseKey); + if (key == null) { + key = configuredKey(conductorKey); + } var target = conductorUrl != null ? conductorUrl : defaultConductorUrl(dbosDomain); // Only a Conductor named explicitly is asked without a key, and never DBOS's hosted one. if (key == null && (conductorUrl == null || target.equals(HOSTED_CONDUCTOR_URL))) { throw new IllegalStateException(names + " requires a DBOS key. " + KEY_HINT); } + if (key != null && key.chars().anyMatch(Character::isISOControl)) { + // Never quoted: an HTTP header cannot carry it, and the message would leak the key. + throw new IllegalStateException( + names + " requires a DBOS key, but the configured key contains a control character."); + } String url; try { url = entitlementUrl(target); - } catch (URISyntaxException | IllegalArgumentException e) { + } catch (InvalidUrl e) { + // The URL is not quoted: it may carry credentials. throw new IllegalStateException( - names + " requires an entitled DBOS key, but the Conductor URL is invalid: " + e, e); + names + + " requires an entitled DBOS key, but the Conductor URL is invalid: " + + e.getMessage()); } var headers = new HashMap(); headers.put("User-Agent", "dbos-transact-java/" + DBOS.version()); @@ -222,6 +236,10 @@ static void check( } catch (IOException e) { failure = e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); cause = e; + } catch (IllegalArgumentException e) { + // The HTTP client refused the request. Its message may quote a header, so it is not passed + // on. + failure = "the request could not be sent"; } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IllegalStateException( @@ -255,19 +273,41 @@ static String defaultConductorUrl(@Nullable String dbosDomain) { return "wss://" + domain + "/conductor/v1alpha1"; } + /** The key as configured, or null if it is unset or blank. */ + private static @Nullable String configuredKey(@Nullable String key) { + if (key == null) { + return null; + } + var stripped = key.strip(); + return stripped.isEmpty() ? null : stripped; + } + + /** A Conductor URL that cannot be used, with a message that never quotes it. */ + static final class InvalidUrl extends Exception { + InvalidUrl(String message) { + super(message); + } + } + /** The entitlement route of the Conductor at {@code conductorUrl}, over HTTP(S). */ - static String entitlementUrl(String conductorUrl) throws URISyntaxException { - var uri = new URI(conductorUrl); + static String entitlementUrl(String conductorUrl) throws InvalidUrl { + URI uri; + try { + uri = new URI(conductorUrl); + } catch (URISyntaxException e) { + // The reason, but not the message, which quotes the whole URL. + throw new InvalidUrl(e.getReason()); + } var scheme = uri.getScheme(); var authority = uri.getRawAuthority(); if (scheme == null || authority == null) { - throw new IllegalArgumentException("not an absolute URL: " + conductorUrl); + throw new InvalidUrl("it is not an absolute URL"); } scheme = switch (scheme.toLowerCase()) { - case "wss" -> "https"; - case "ws" -> "http"; - default -> scheme; + case "wss", "https" -> "https"; + case "ws", "http" -> "http"; + default -> throw new InvalidUrl("its scheme is not ws, wss, http or https"); }; // Without any user:password@, which would otherwise be sent and quoted in errors. var host = authority.substring(authority.lastIndexOf('@') + 1); @@ -279,25 +319,47 @@ static String entitlementUrl(String conductorUrl) throws URISyntaxException { return scheme + "://" + host + path + ENTITLEMENTS_PATH; } - private static Answer fetch(URI uri, Map headers) - throws IOException, InterruptedException { - var builder = HttpClient.newBuilder().connectTimeout(TIMEOUT); - // Never follow a redirect: it would carry the key to whatever host it names. - builder.followRedirects(HttpClient.Redirect.NEVER); - // Built per call, so proxy settings are read when the check runs. - var proxies = ProxySelector.getDefault(); - if (proxies != null) { - builder.proxy(proxies); - } - var request = HttpRequest.newBuilder(uri).timeout(TIMEOUT).GET(); - headers.forEach(request::header); - var response = builder.build().send(request.build(), HttpResponse.BodyHandlers.ofInputStream()); - try (var body = response.body()) { + /** Sends the request over HTTP, giving the whole exchange, body included, {@code timeout}. */ + static Http http(Duration timeout) { + return (uri, headers) -> { + var builder = HttpClient.newBuilder().connectTimeout(timeout); + // Never follow a redirect: it would carry the key to whatever host it names. + builder.followRedirects(HttpClient.Redirect.NEVER); + // Built per call, so proxy settings are read when the check runs. + var proxies = ProxySelector.getDefault(); + if (proxies != null) { + builder.proxy(proxies); + } + var request = HttpRequest.newBuilder(uri).timeout(timeout).GET(); + headers.forEach(request::header); + var pending = + builder.build().sendAsync(request.build(), HttpResponse.BodyHandlers.ofString()); + HttpResponse response; + try { + // The request's own timeout ends once the headers arrive. This deadline covers the body + // too, so an answer that stalls mid-body cannot hold up launch. + response = pending.get(timeout.toMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + pending.cancel(true); + throw new HttpTimeoutException("no answer within " + timeout.toMillis() + " ms"); + } catch (InterruptedException e) { + pending.cancel(true); + throw e; + } catch (ExecutionException e) { + var cause = e.getCause(); + if (cause instanceof IOException io) { + throw io; + } + if (cause instanceof RuntimeException runtime) { + throw runtime; + } + throw new IOException(cause); + } return new Answer( response.statusCode(), response.headers().firstValue(ERROR_CODE_HEADER).orElse(null), - new String(body.readNBytes(MAX_BODY_BYTES), StandardCharsets.UTF_8)); - } + response.body()); + }; } /** An answer that is not an entitlement list, which counts as Conductor failing to answer. */ diff --git a/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java b/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java index 4f935c6b5..20db87417 100644 --- a/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java @@ -3,7 +3,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; @@ -31,12 +30,13 @@ public void theKeyIsKeptThroughCopiesAndOtherSettings() { } @Test - public void anEmptyKeyIsRefused() { - var e = - assertThrows( - IllegalArgumentException.class, - () -> DBOSConfig.defaults("app").withTransactEnterpriseKey("")); - assertTrue(e.getMessage().contains("transactEnterpriseKey"), e.getMessage()); + public void aBlankKeyIsUnset() { + // So an empty placeholder, such as an unset environment variable, falls back to the Conductor + // key instead of failing. + for (var blank : new String[] {"", " ", "\n"}) { + assertNull( + DBOSConfig.defaults("app").withTransactEnterpriseKey(blank).transactEnterpriseKey()); + } } @Test diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index 712fe9c89..6aade3498 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -4,6 +4,7 @@ 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.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.assertTrue; import dev.dbos.transact.config.DBOSConfig; @@ -17,11 +18,13 @@ import java.net.ServerSocket; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; import ch.qos.logback.classic.Level; import com.sun.net.httpserver.HttpServer; @@ -48,6 +51,9 @@ static final class FakeConductor implements AutoCloseable { volatile int status = 200; volatile String body = "{\"entitlements\": {\"someFeature\": true}}"; volatile Map headers = Map.of(); + // When set, the answer's headers go out, then its body stalls until the server closes. + volatile boolean stall = false; + final CountDownLatch closed = new CountDownLatch(1); FakeConductor() throws IOException { server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); @@ -63,6 +69,16 @@ static final class FakeConductor implements AutoCloseable { headers.forEach((name, value) -> exchange.getResponseHeaders().add(name, value)); exchange.sendResponseHeaders(status, payload.length); try (var out = exchange.getResponseBody()) { + if (stall) { + out.write(payload, 0, 1); + out.flush(); + try { + closed.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return; + } out.write(payload); } }); @@ -88,6 +104,7 @@ void entitle(String json) { @Override public void close() { + closed.countDown(); server.stop(0); } } @@ -160,6 +177,29 @@ public void anInvalidConductorUrlIsRefused() { assertTrue(e.getMessage().contains("the Conductor URL is invalid"), e.getMessage()); } + @ParameterizedTest + @ValueSource( + strings = { + // An unencoded % in the password, which java.net.URI rejects quoting the whole URL. + "wss://user:p%zzsecret@host/conductor/v1alpha1", + // A space in the password. + "wss://user:pa secret@host/conductor/v1alpha1", + // No scheme, so the user name reads as one. + "user:secret@host", + "htp://user:secret@host" + }) + public void anInvalidConductorUrlIsNeverQuoted(String url) { + var e = + assertThrows( + IllegalStateException.class, + () -> Entitlements.require(List.of(SOME_FEATURE), "enterprise-key", null, url, false)); + assertTrue(e.getMessage().contains("the Conductor URL is invalid"), e.getMessage()); + assertFalse(e.getMessage().contains("secret"), e.getMessage()); + assertFalse(e.getMessage().contains("user"), e.getMessage()); + // Nor in the stack trace. + assertEquals(null, e.getCause()); + } + @Test public void theDefaultConductorFollowsTheDbosDomain() { assertEquals(Entitlements.HOSTED_CONDUCTOR_URL, Entitlements.defaultConductorUrl(null)); @@ -183,6 +223,53 @@ public void anEntitledKeyUnlocksEnterpriseFeatures() { conductor.requests.stream().map(e -> Map.entry(e.getKey(), e.getValue())).toList()); } + @Test + public void aBlankEnterpriseKeyFallsBackToTheConductorKey() { + check(" ", "conductor-key"); + check("", "conductor-key"); + assertEquals(2, conductor.requests.size()); + conductor.requests.forEach(r -> assertEquals("Bearer conductor-key", r.getValue())); + } + + @Test + public void surroundingWhitespaceIsNotPartOfTheKey() { + // As read from a secret file that ends in a newline. + check("enterprise-key\n", null); + check(null, " conductor-key\r\n"); + assertEquals("Bearer enterprise-key", conductor.requests.get(0).getValue()); + assertEquals("Bearer conductor-key", conductor.requests.get(1).getValue()); + } + + @Test + public void aKeyWithAControlCharacterIsRefusedWithoutQuotingIt() { + var e = refused("enterprise\nkey"); + assertTrue(e.getMessage().contains("contains a control character"), e.getMessage()); + assertFalse(e.getMessage().contains("enterprise"), e.getMessage()); + assertEquals(List.of(), conductor.requests); + } + + @Test + public void anAnswerThatStallsMidBodyTimesOut() { + // Headers arrive, then the body never finishes: the deadline covers the whole exchange. + conductor.stall = true; + var e = + assertTimeoutPreemptively( + Duration.ofSeconds(10), + () -> + assertThrows( + IllegalStateException.class, + () -> + Entitlements.check( + List.of(SOME_FEATURE), + "enterprise-key", + null, + conductor.url, + null, + Entitlements.http(Duration.ofMillis(500))))); + assertTrue(e.getMessage().contains("no answer within 500 ms"), e.getMessage()); + assertTrue(e.getMessage().contains("Retry once Conductor is reachable"), e.getMessage()); + } + @Test public void eachFeatureNeedsItsOwnEntitlement() { conductor.entitle("{\"someFeature\": true, \"otherFeature\": false, \"sharding\": true}"); From f71388d749f6328fd917b116106c502d12dc1451 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 11:45:59 -0800 Subject: [PATCH 03/10] Treat a blank Conductor key as unset, and mask it in DBOSConfig.toString - DBOSConfig.conductorKey now treats a blank value as unset, meaning no Conductor connection, instead of refusing an empty one. That matches transactEnterpriseKey, and Python and TypeScript, where an empty conductor_key / conductorKey is unset. A placeholder such as dbos.conductor.key=${CONDUCTOR_KEY:} with the variable unset no longer fails startup. Before, " " passed the empty check and was used as a key. - toString prints conductorKey as *** (or null), as it already did for dbPassword and transactEnterpriseKey. It printed the key in clear, and the Conductor key also serves as the enterprise key when none is set. --- .../spring/DBOSAutoConfigurationTest.java | 11 ++-- .../dev/dbos/transact/config/DBOSConfig.java | 22 ++++--- .../dev/dbos/transact/config/ConfigTest.java | 4 +- .../transact/config/DBOSConfigKeysTest.java | 65 +++++++++++++++++++ .../config/TransactEnterpriseKeyTest.java | 51 --------------- 5 files changed, 87 insertions(+), 66 deletions(-) create mode 100644 transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java delete mode 100644 transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java diff --git a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java index 80fea5a83..79aedecd8 100644 --- a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java +++ b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java @@ -50,14 +50,17 @@ void dbosConfigReflectsAppNameProperty() { } @Test - void anEmptyTransactEnterpriseKeyIsUnset() { - // As from a placeholder whose environment variable is unset: the Conductor key stands in. + void emptyKeysAreUnset() { + // As from placeholders whose environment variables are unset: no Conductor connection, and no + // enterprise key. runner - .withPropertyValues("dbos.transact-enterprise-key=") + .withPropertyValues("dbos.conductor.key=", "dbos.transact-enterprise-key=") .run( context -> { assertThat(context).hasNotFailed(); - assertThat(context.getBean(DBOSConfig.class).transactEnterpriseKey()).isNull(); + var config = context.getBean(DBOSConfig.class); + assertThat(config.conductorKey()).isNull(); + assertThat(config.transactEnterpriseKey()).isNull(); }); } diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index c24b50534..d1e67780d 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -42,8 +42,9 @@ * @param adminServerPort port for the built-in admin HTTP server; deprecated and will be removed * before 1.0 — use DBOS Conductor instead * @param migrate whether to run database migrations on startup - * @param conductorKey API key for DBOS Conductor; must not be empty if non-null. Connecting to - * Conductor requires {@code dev.dbos:transact-enterprise} on the class path + * @param conductorKey API key for DBOS Conductor; null or blank to not connect. Connecting to + * Conductor requires {@code dev.dbos:transact-enterprise} on the class path. Masked in {@link + * #toString()} * @param conductorDomain custom hostname for DBOS Conductor; must not be empty if non-null * @param conductorExecutorMetadata arbitrary key-value metadata attached to this executor's * registration in Conductor @@ -132,11 +133,11 @@ public record DBOSConfig( if (appName == null || appName.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.appName must not be null or empty"); } - if (conductorKey != null && conductorKey.isEmpty()) { - throw new IllegalArgumentException("DBOSConfig.conductorKey must not be empty if specified"); + // A blank key is unset, as in the other SDKs, so an empty placeholder (an unset environment + // variable) means no key instead of failing. + if (conductorKey != null && conductorKey.isBlank()) { + conductorKey = null; } - // Unset, as in the other SDKs, so an empty placeholder (an unset environment variable) - // falls back to the Conductor key instead of failing. if (transactEnterpriseKey != null && transactEnterpriseKey.isBlank()) { transactEnterpriseKey = null; } @@ -535,8 +536,9 @@ public DBOSConfig(DBOSConfig other) { } /** - * Returns a copy of this config with {@code conductorKey} set to {@code v}. Connecting to - * Conductor requires {@code dev.dbos:transact-enterprise} on the class path. + * Returns a copy of this config with {@code conductorKey} set to {@code v}; null or blank to not + * connect. Connecting to Conductor requires {@code dev.dbos:transact-enterprise} on the class + * path. */ public @NonNull DBOSConfig withConductorKey(@Nullable String v) { return new DBOSConfig( @@ -1157,7 +1159,7 @@ public DBOSConfig(DBOSConfig other) { v); } - // Override toString to mask the DB password and the Transact Enterprise key + // Override toString to mask the DB password and the Conductor and Transact Enterprise keys @Override public String toString() { return """ @@ -1179,7 +1181,7 @@ public String toString() { adminServer, adminServerPort, migrate, - conductorKey, + conductorKey == null ? null : "***", conductorDomain, conductorExecutorMetadata, appVersion, diff --git a/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java b/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java index ac7f9cd3a..4644bffe0 100644 --- a/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -235,7 +236,8 @@ public void cantSetEmptyConfigFields() throws Exception { assertThrows(IllegalArgumentException.class, () -> config.withAppName("")); assertThrows(IllegalArgumentException.class, () -> config.withAppName(null)); - assertThrows(IllegalArgumentException.class, () -> config.withConductorKey("")); + // A blank Conductor key is unset rather than refused (see DBOSConfigKeysTest). + assertNull(config.withConductorKey("").conductorKey()); assertDoesNotThrow(() -> config.withConductorKey(null)); assertThrows(IllegalArgumentException.class, () -> config.withConductorDomain("")); assertDoesNotThrow(() -> config.withConductorDomain(null)); diff --git a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java new file mode 100644 index 000000000..c9a69716c --- /dev/null +++ b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java @@ -0,0 +1,65 @@ +package dev.dbos.transact.config; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +/** The Conductor key and the Transact Enterprise key, which DBOSConfig handles alike. */ +public class DBOSConfigKeysTest { + + @Test + public void theKeysAreUnsetByDefault() { + for (var config : + new DBOSConfig[] {DBOSConfig.defaults("app"), DBOSConfig.defaultsFromEnv("app")}) { + assertNull(config.conductorKey()); + assertNull(config.transactEnterpriseKey()); + } + } + + @Test + public void theKeysAreKeptThroughCopiesAndOtherSettings() { + var config = + DBOSConfig.defaults("app") + .withConductorKey("conductor-key") + .withTransactEnterpriseKey("enterprise-key"); + var copy = new DBOSConfig(config); + assertEquals("conductor-key", copy.conductorKey()); + assertEquals("enterprise-key", copy.transactEnterpriseKey()); + var changed = config.withConductorMetadataOnlyMode(true).withAppName("x"); + assertEquals("conductor-key", changed.conductorKey()); + assertEquals("enterprise-key", changed.transactEnterpriseKey()); + assertNull(changed.withTransactEnterpriseKey(null).transactEnterpriseKey()); + assertNull(changed.withConductorKey(null).conductorKey()); + } + + @Test + public void aBlankKeyIsUnset() { + // So an empty placeholder, such as an unset environment variable, means no key instead of + // failing: no Conductor connection, and the Conductor key standing in for the enterprise key. + for (var blank : new String[] {"", " ", "\n"}) { + var config = + DBOSConfig.defaults("app").withConductorKey(blank).withTransactEnterpriseKey(blank); + assertNull(config.conductorKey(), "conductorKey " + blank); + assertNull(config.transactEnterpriseKey(), "transactEnterpriseKey " + blank); + } + } + + @Test + public void theKeysAreMaskedInToString() { + var text = + DBOSConfig.defaults("app") + .withConductorKey("conductor-key") + .withTransactEnterpriseKey("enterprise-key") + .toString(); + assertFalse(text.contains("conductor-key"), text); + assertFalse(text.contains("enterprise-key"), text); + assertTrue(text.contains("conductorKey=***"), text); + assertTrue(text.contains("transactEnterpriseKey=***"), text); + var unset = DBOSConfig.defaults("app").toString(); + assertTrue(unset.contains("conductorKey=null"), unset); + assertTrue(unset.contains("transactEnterpriseKey=null"), unset); + } +} diff --git a/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java b/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java deleted file mode 100644 index 20db87417..000000000 --- a/transact/src/test/java/dev/dbos/transact/config/TransactEnterpriseKeyTest.java +++ /dev/null @@ -1,51 +0,0 @@ -package dev.dbos.transact.config; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; - -public class TransactEnterpriseKeyTest { - - @Test - public void theKeyIsUnsetByDefault() { - assertNull(DBOSConfig.defaults("app").transactEnterpriseKey()); - assertNull(DBOSConfig.defaultsFromEnv("app").transactEnterpriseKey()); - } - - @Test - public void theKeyIsKeptThroughCopiesAndOtherSettings() { - var config = DBOSConfig.defaults("app").withTransactEnterpriseKey("enterprise-key"); - assertEquals("enterprise-key", config.transactEnterpriseKey()); - assertEquals("enterprise-key", new DBOSConfig(config).transactEnterpriseKey()); - var changed = - config - .withConductorKey("conductor-key") - .withConductorMetadataOnlyMode(true) - .withAppName("x"); - assertEquals("enterprise-key", changed.transactEnterpriseKey()); - assertNull(changed.withTransactEnterpriseKey(null).transactEnterpriseKey()); - } - - @Test - public void aBlankKeyIsUnset() { - // So an empty placeholder, such as an unset environment variable, falls back to the Conductor - // key instead of failing. - for (var blank : new String[] {"", " ", "\n"}) { - assertNull( - DBOSConfig.defaults("app").withTransactEnterpriseKey(blank).transactEnterpriseKey()); - } - } - - @Test - public void theKeyIsMaskedInToString() { - var text = DBOSConfig.defaults("app").withTransactEnterpriseKey("enterprise-key").toString(); - assertFalse(text.contains("enterprise-key"), text); - assertTrue(text.contains("transactEnterpriseKey=***"), text); - assertTrue( - DBOSConfig.defaults("app").toString().contains("transactEnterpriseKey=null"), - DBOSConfig.defaults("app").toString()); - } -} From 254b7dadfde31d866bb26707d8525fd2de2d6463 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 11:50:54 -0800 Subject: [PATCH 04/10] Treat a blank Conductor domain as unset DBOSConfig.conductorDomain now treats a blank value as unset (DBOS's hosted Conductor), like conductorKey and transactEnterpriseKey. Before, "" was refused while " " was accepted, and DBOSExecutor quietly treated " " as unset in two places of its own. Those two checks are gone; the executor now passes the configured domain unchanged to the entitlement check and to the Conductor client, so the two always use the same URL. --- .../spring/DBOSAutoConfigurationTest.java | 4 +++- .../dev/dbos/transact/config/DBOSConfig.java | 17 ++++++++++------- .../dbos/transact/execution/DBOSExecutor.java | 10 +++------- .../dev/dbos/transact/config/ConfigTest.java | 4 ++-- .../transact/config/DBOSConfigKeysTest.java | 8 ++++++-- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java index 79aedecd8..d9c49d8f9 100644 --- a/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java +++ b/transact-spring-boot-starter/src/test/java/dev/dbos/transact/spring/DBOSAutoConfigurationTest.java @@ -54,12 +54,14 @@ void emptyKeysAreUnset() { // As from placeholders whose environment variables are unset: no Conductor connection, and no // enterprise key. runner - .withPropertyValues("dbos.conductor.key=", "dbos.transact-enterprise-key=") + .withPropertyValues( + "dbos.conductor.key=", "dbos.conductor.domain=", "dbos.transact-enterprise-key=") .run( context -> { assertThat(context).hasNotFailed(); var config = context.getBean(DBOSConfig.class); assertThat(config.conductorKey()).isNull(); + assertThat(config.conductorDomain()).isNull(); assertThat(config.transactEnterpriseKey()).isNull(); }); } diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index d1e67780d..7643b5ada 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -45,7 +45,8 @@ * @param conductorKey API key for DBOS Conductor; null or blank to not connect. Connecting to * Conductor requires {@code dev.dbos:transact-enterprise} on the class path. Masked in {@link * #toString()} - * @param conductorDomain custom hostname for DBOS Conductor; must not be empty if non-null + * @param conductorDomain custom hostname for DBOS Conductor; null or blank for DBOS's hosted + * Conductor * @param conductorExecutorMetadata arbitrary key-value metadata attached to this executor's * registration in Conductor * @param appVersion application version string used for versioned workflow routing; must not be @@ -133,17 +134,16 @@ public record DBOSConfig( if (appName == null || appName.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.appName must not be null or empty"); } - // A blank key is unset, as in the other SDKs, so an empty placeholder (an unset environment - // variable) means no key instead of failing. + // A blank Conductor setting is unset, as in the other SDKs, so an empty placeholder (an unset + // environment variable) means the default instead of failing. if (conductorKey != null && conductorKey.isBlank()) { conductorKey = null; } if (transactEnterpriseKey != null && transactEnterpriseKey.isBlank()) { transactEnterpriseKey = null; } - if (conductorDomain != null && conductorDomain.isEmpty()) { - throw new IllegalArgumentException( - "DBOSConfig.conductorDomain must not be empty if specified"); + if (conductorDomain != null && conductorDomain.isBlank()) { + conductorDomain = null; } if (appVersion != null && appVersion.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.appVersion must not be empty if specified"); @@ -568,7 +568,10 @@ public DBOSConfig(DBOSConfig other) { transactEnterpriseKey); } - /** Returns a copy of this config with {@code conductorDomain} set to {@code v}. */ + /** + * Returns a copy of this config with {@code conductorDomain} set to {@code v}; null or blank for + * DBOS's hosted Conductor. + */ public @NonNull DBOSConfig withConductorDomain(@Nullable String v) { return new DBOSConfig( appName, diff --git a/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java b/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java index 574dd331f..9809bb9b3 100644 --- a/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java +++ b/transact/src/main/java/dev/dbos/transact/execution/DBOSExecutor.java @@ -249,12 +249,11 @@ public DBOSExecutor(DBOSConfig config) { // Every Transact Enterprise feature needs Conductor to report the organization entitled to it. // Checked before anything is started, so a refusal fails launch cleanly. - var conductorDomain = config.conductorDomain(); Entitlements.require( Entitlements.enabledFeatures(config), config.transactEnterpriseKey(), config.conductorKey(), - conductorDomain == null || conductorDomain.isBlank() ? null : conductorDomain.trim(), + config.conductorDomain(), dbosCloud); // Found before anything is started, so a missing provider fails launch cleanly. An @@ -450,11 +449,8 @@ public void start( if (conductorKey == null) { return null; } - String domain = config.conductorDomain(); - if (domain != null && domain.trim().isEmpty()) { - domain = null; - } - return new ConductorOptions(conductorKey, null, domain, config.conductorMetadataOnlyMode()); + return new ConductorOptions( + conductorKey, null, config.conductorDomain(), config.conductorMetadataOnlyMode()); } @Nullable ConductorOptions conductorOptions() { diff --git a/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java b/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java index 4644bffe0..ddc9fe711 100644 --- a/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/ConfigTest.java @@ -236,10 +236,10 @@ public void cantSetEmptyConfigFields() throws Exception { assertThrows(IllegalArgumentException.class, () -> config.withAppName("")); assertThrows(IllegalArgumentException.class, () -> config.withAppName(null)); - // A blank Conductor key is unset rather than refused (see DBOSConfigKeysTest). + // Blank Conductor settings are unset rather than refused (see DBOSConfigKeysTest). assertNull(config.withConductorKey("").conductorKey()); assertDoesNotThrow(() -> config.withConductorKey(null)); - assertThrows(IllegalArgumentException.class, () -> config.withConductorDomain("")); + assertNull(config.withConductorDomain("").conductorDomain()); assertDoesNotThrow(() -> config.withConductorDomain(null)); assertThrows(IllegalArgumentException.class, () -> config.withExecutorId("")); assertDoesNotThrow(() -> config.withExecutorId(null)); diff --git a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java index c9a69716c..61a748a49 100644 --- a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java @@ -7,7 +7,7 @@ import org.junit.jupiter.api.Test; -/** The Conductor key and the Transact Enterprise key, which DBOSConfig handles alike. */ +/** The Conductor key and domain and the Transact Enterprise key, which DBOSConfig handles alike. */ public class DBOSConfigKeysTest { @Test @@ -41,8 +41,12 @@ public void aBlankKeyIsUnset() { // failing: no Conductor connection, and the Conductor key standing in for the enterprise key. for (var blank : new String[] {"", " ", "\n"}) { var config = - DBOSConfig.defaults("app").withConductorKey(blank).withTransactEnterpriseKey(blank); + DBOSConfig.defaults("app") + .withConductorKey(blank) + .withConductorDomain(blank) + .withTransactEnterpriseKey(blank); assertNull(config.conductorKey(), "conductorKey " + blank); + assertNull(config.conductorDomain(), "conductorDomain " + blank); assertNull(config.transactEnterpriseKey(), "transactEnterpriseKey " + blank); } } From da949438ffb31995994f36e1b03185334389fd46 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 12:13:32 -0800 Subject: [PATCH 05/10] Never let an unsendable key pass the entitlement check, and give the check and the client one key - A key that is not printable ASCII is refused before any request, without being quoted. Before, the HTTP client refused a key with a character above U+00FF while building the header. The check counted that as an outage, so against DBOS's hosted Conductor it warned and started anyway, with no verified entitlement. - Any request the HTTP client refuses to send now fails closed, even against the hosted Conductor. The client's message is still never passed on. - DBOSConfig strips surrounding whitespace from conductorKey, conductorDomain and transactEnterpriseKey, not only blank values. Before, the check stripped "key\n" but the Conductor client got it as configured, so the check passed and the client then failed to connect. - The answer is read up to 1 MiB again, through a bounded body subscriber under the same deadline. An oversized answer counts as an outage instead of filling the heap. - The HTTP request has no timeout of its own. The one deadline covers the whole exchange, so the two can no longer race to report a timeout. - DBOSConfig.toString masks any user:password@ in conductorDomain. - Entitlements.require treats a blank Conductor URL as unset, as DBOSConfig does, instead of refusing it as invalid. --- .../dev/dbos/transact/config/DBOSConfig.java | 47 +++++--- .../dbos/transact/internal/Entitlements.java | 106 +++++++++++++++--- .../transact/config/DBOSConfigKeysTest.java | 28 +++++ .../transact/internal/EntitlementsTest.java | 75 ++++++++++++- 4 files changed, 217 insertions(+), 39 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index 7643b5ada..a801b3142 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -42,11 +42,12 @@ * @param adminServerPort port for the built-in admin HTTP server; deprecated and will be removed * before 1.0 — use DBOS Conductor instead * @param migrate whether to run database migrations on startup - * @param conductorKey API key for DBOS Conductor; null or blank to not connect. Connecting to - * Conductor requires {@code dev.dbos:transact-enterprise} on the class path. Masked in {@link - * #toString()} + * @param conductorKey API key for DBOS Conductor; null or blank to not connect. Surrounding + * whitespace is removed. Connecting to Conductor requires {@code dev.dbos:transact-enterprise} + * on the class path. Masked in {@link #toString()} * @param conductorDomain custom hostname for DBOS Conductor; null or blank for DBOS's hosted - * Conductor + * Conductor. Surrounding whitespace is removed, and credentials in it are masked in {@link + * #toString()} * @param conductorExecutorMetadata arbitrary key-value metadata attached to this executor's * registration in Conductor * @param appVersion application version string used for versioned workflow routing; must not be @@ -80,7 +81,7 @@ * enforces it * @param transactEnterpriseKey the DBOS key that entitles this application to Transact Enterprise * features. A blank value is the same as null, and when it is null, {@code conductorKey} stands - * in for it. Masked in {@link #toString()} + * in for it. Surrounding whitespace is removed. Masked in {@link #toString()} */ public record DBOSConfig( @NonNull String appName, @@ -135,16 +136,12 @@ public record DBOSConfig( throw new IllegalArgumentException("DBOSConfig.appName must not be null or empty"); } // A blank Conductor setting is unset, as in the other SDKs, so an empty placeholder (an unset - // environment variable) means the default instead of failing. - if (conductorKey != null && conductorKey.isBlank()) { - conductorKey = null; - } - if (transactEnterpriseKey != null && transactEnterpriseKey.isBlank()) { - transactEnterpriseKey = null; - } - if (conductorDomain != null && conductorDomain.isBlank()) { - conductorDomain = null; - } + // environment variable) means the default instead of failing. Surrounding whitespace, such as + // the trailing newline of a secret file, is not part of the value, so the entitlement check + // and the Conductor client always see the same one. + conductorKey = setting(conductorKey); + conductorDomain = setting(conductorDomain); + transactEnterpriseKey = setting(transactEnterpriseKey); if (appVersion != null && appVersion.isEmpty()) { throw new IllegalArgumentException("DBOSConfig.appVersion must not be empty if specified"); } @@ -209,6 +206,21 @@ public int adminServerPort() { return adminServerPort; } + private static @Nullable String setting(@Nullable String value) { + if (value == null) { + return null; + } + var stripped = value.strip(); + return stripped.isEmpty() ? null : stripped; + } + + /** The domain with any {@code user:password@} replaced, for printing. */ + private static @Nullable String withoutCredentials(@Nullable String domain) { + return domain == null + ? null + : domain.replaceFirst("^((?:[A-Za-z][A-Za-z0-9+.-]*://)?)[^/@]*@", "$1***@"); + } + // Copy constructor public DBOSConfig(DBOSConfig other) { this( @@ -1162,7 +1174,8 @@ public DBOSConfig(DBOSConfig other) { v); } - // Override toString to mask the DB password and the Conductor and Transact Enterprise keys + // Override toString to mask the DB password, the Conductor and Transact Enterprise keys, and any + // credentials in the Conductor domain @Override public String toString() { return """ @@ -1185,7 +1198,7 @@ public String toString() { adminServerPort, migrate, conductorKey == null ? null : "***", - conductorDomain, + withoutCredentials(conductorDomain), conductorExecutorMetadata, appVersion, executorId, diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index 5a6381518..e99cf560d 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -3,6 +3,7 @@ import dev.dbos.transact.DBOS; import dev.dbos.transact.config.DBOSConfig; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.ProxySelector; import java.net.URI; @@ -11,12 +12,17 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.net.http.HttpTimeoutException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Flow; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.function.Function; @@ -64,6 +70,8 @@ public record Feature(String entitlement, String description) {} private static final String KEY_HINT = "Set transactEnterpriseKey or conductorKey in your DBOSConfig."; private static final ObjectMapper MAPPER = new ObjectMapper(); + // Read no more of an answer than this; an entitlement list is a few hundred bytes. + static final int MAX_ANSWER_BYTES = 1 << 20; private Entitlements() {} @@ -130,21 +138,25 @@ static void check( } var names = join(features, Feature::description); var entitlements = join(features, Feature::entitlement); - // A blank key is unset, and surrounding whitespace (a trailing newline from a secret file) is - // not part of it. - var key = configuredKey(enterpriseKey); + // As DBOSConfig reads them: blank is unset, and surrounding whitespace (a trailing newline from + // a secret file) is not part of the value. + var key = setting(enterpriseKey); if (key == null) { - key = configuredKey(conductorKey); + key = setting(conductorKey); } + conductorUrl = setting(conductorUrl); var target = conductorUrl != null ? conductorUrl : defaultConductorUrl(dbosDomain); // Only a Conductor named explicitly is asked without a key, and never DBOS's hosted one. if (key == null && (conductorUrl == null || target.equals(HOSTED_CONDUCTOR_URL))) { throw new IllegalStateException(names + " requires a DBOS key. " + KEY_HINT); } - if (key != null && key.chars().anyMatch(Character::isISOControl)) { - // Never quoted: an HTTP header cannot carry it, and the message would leak the key. + // A DBOS key is printable ASCII. Anything else is refused here, unquoted, rather than by the + // HTTP client, whose message would quote the key. + if (key != null && !key.chars().allMatch(c -> c > 0x20 && c < 0x7f)) { throw new IllegalStateException( - names + " requires a DBOS key, but the configured key contains a control character."); + names + + " requires a DBOS key, but the configured key is not one: it contains a character" + + " other than printable ASCII."); } String url; try { @@ -237,9 +249,13 @@ static void check( failure = e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); cause = e; } catch (IllegalArgumentException e) { - // The HTTP client refused the request. Its message may quote a header, so it is not passed - // on. - failure = "the request could not be sent"; + // The HTTP client refused to send the request: not an outage, so it never fails open. Its + // message may quote a header, so it is not passed on. + throw refusal( + names + + " requires an entitled DBOS key, but the entitlement request to Conductor at " + + url + + " could not be sent."); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IllegalStateException( @@ -273,12 +289,12 @@ static String defaultConductorUrl(@Nullable String dbosDomain) { return "wss://" + domain + "/conductor/v1alpha1"; } - /** The key as configured, or null if it is unset or blank. */ - private static @Nullable String configuredKey(@Nullable String key) { - if (key == null) { + /** A setting without surrounding whitespace, or null if it is unset or blank. */ + private static @Nullable String setting(@Nullable String value) { + if (value == null) { return null; } - var stripped = key.strip(); + var stripped = value.strip(); return stripped.isEmpty() ? null : stripped; } @@ -330,14 +346,15 @@ static Http http(Duration timeout) { if (proxies != null) { builder.proxy(proxies); } - var request = HttpRequest.newBuilder(uri).timeout(timeout).GET(); + // No request timeout: it would end at the headers, and race the deadline below. + var request = HttpRequest.newBuilder(uri).GET(); headers.forEach(request::header); var pending = - builder.build().sendAsync(request.build(), HttpResponse.BodyHandlers.ofString()); + builder.build().sendAsync(request.build(), info -> new BoundedBody(MAX_ANSWER_BYTES)); HttpResponse response; try { - // The request's own timeout ends once the headers arrive. This deadline covers the body - // too, so an answer that stalls mid-body cannot hold up launch. + // One deadline for the whole exchange, body included, so an answer that stalls mid-body + // cannot hold up launch. response = pending.get(timeout.toMillis(), TimeUnit.MILLISECONDS); } catch (TimeoutException e) { pending.cancel(true); @@ -362,6 +379,59 @@ static Http http(Duration timeout) { }; } + /** Collects an answer as UTF-8 text, failing once it exceeds {@code max} bytes. */ + static final class BoundedBody implements HttpResponse.BodySubscriber { + private final int max; + private final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + private final CompletableFuture body = new CompletableFuture<>(); + private Flow.@Nullable Subscription subscription; + + BoundedBody(int max) { + this.max = max; + } + + @Override + public void onSubscribe(Flow.Subscription subscription) { + this.subscription = subscription; + subscription.request(Long.MAX_VALUE); + } + + @Override + public void onNext(List buffers) { + if (body.isDone()) { + return; + } + for (var buffer : buffers) { + if (bytes.size() + buffer.remaining() > max) { + if (subscription != null) { + subscription.cancel(); + } + body.completeExceptionally( + new IOException("the answer is larger than " + max + " bytes")); + return; + } + var chunk = new byte[buffer.remaining()]; + buffer.get(chunk); + bytes.write(chunk, 0, chunk.length); + } + } + + @Override + public void onError(Throwable error) { + body.completeExceptionally(error); + } + + @Override + public void onComplete() { + body.complete(bytes.toString(StandardCharsets.UTF_8)); + } + + @Override + public CompletionStage getBody() { + return body; + } + } + /** An answer that is not an entitlement list, which counts as Conductor failing to answer. */ private static final class MalformedAnswer extends Exception { MalformedAnswer(String message) { diff --git a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java index 61a748a49..edaa21a81 100644 --- a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java @@ -51,6 +51,34 @@ public void aBlankKeyIsUnset() { } } + @Test + public void surroundingWhitespaceIsRemoved() { + // As read from a secret file that ends in a newline: the entitlement check and the Conductor + // client must both see the value without it. + var config = + DBOSConfig.defaults("app") + .withConductorKey(" conductor-key\n") + .withConductorDomain(" ws://localhost:8090\r\n") + .withTransactEnterpriseKey("enterprise-key\n"); + assertEquals("conductor-key", config.conductorKey()); + assertEquals("ws://localhost:8090", config.conductorDomain()); + assertEquals("enterprise-key", config.transactEnterpriseKey()); + } + + @Test + public void credentialsInTheConductorDomainAreMaskedInToString() { + var text = + DBOSConfig.defaults("app") + .withConductorDomain("wss://user:secret@conductor.example.com/conductor/v1alpha1") + .toString(); + assertFalse(text.contains("secret"), text); + assertFalse(text.contains("user"), text); + assertTrue( + text.contains("conductorDomain=wss://***@conductor.example.com/conductor/v1alpha1"), text); + var plain = DBOSConfig.defaults("app").withConductorDomain("ws://localhost:8090").toString(); + assertTrue(plain.contains("conductorDomain=ws://localhost:8090,"), plain); + } + @Test public void theKeysAreMaskedInToString() { var text = diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index 6aade3498..cece59045 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -240,12 +240,79 @@ public void surroundingWhitespaceIsNotPartOfTheKey() { assertEquals("Bearer conductor-key", conductor.requests.get(1).getValue()); } - @Test - public void aKeyWithAControlCharacterIsRefusedWithoutQuotingIt() { - var e = refused("enterprise\nkey"); - assertTrue(e.getMessage().contains("contains a control character"), e.getMessage()); + @ParameterizedTest + @ValueSource( + strings = {"enterprise\nkey", "enterprise\u20ackey", "enterprise\u0100key", "enterprise key"}) + public void aKeyThatIsNotPrintableAsciiIsRefusedWithoutQuotingIt(String key) { + var e = refused(key); + assertTrue(e.getMessage().contains("other than printable ASCII"), e.getMessage()); assertFalse(e.getMessage().contains("enterprise"), e.getMessage()); assertEquals(List.of(), conductor.requests); + // Against the hosted Conductor too, where an outage would start anyway. + var asked = new ArrayList(); + var hosted = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.check( + List.of(SOME_FEATURE), + key, + null, + null, + null, + (uri, headers) -> { + asked.add(uri); + return new Answer(200, null, "{\"entitlements\": {\"someFeature\": true}}"); + })); + assertFalse(hosted.getMessage().contains("enterprise"), hosted.getMessage()); + assertEquals(List.of(), asked); + } + + @Test + public void aRequestTheHttpClientRefusesFailsClosedEvenWhenHosted() { + // Not an outage, so the hosted Conductor's fail-open does not apply. The client's message, + // which can quote the Authorization header, is not passed on. + Entitlements.Http refusing = + (uri, headers) -> { + throw new IllegalArgumentException("invalid header value: \"Bearer secret\""); + }; + try (var logs = new LogCapture(Entitlements.class)) { + var e = assertThrows(IllegalStateException.class, () -> checkHosted(null, refusing)); + assertTrue(e.getMessage().contains("could not be sent"), e.getMessage()); + assertFalse(e.getMessage().contains("secret"), e.getMessage()); + assertEquals(null, e.getCause()); + assertEquals(List.of(), logs.messages(Level.WARN, "")); + } + } + + @Test + public void anOversizedAnswerCountsAsAnOutage() { + conductor.body = + "{\"entitlements\": {\"someFeature\": true}, \"padding\": \"" + + "x".repeat(Entitlements.MAX_ANSWER_BYTES) + + "\"}"; + var message = refused("enterprise-key").getMessage(); + assertTrue( + message.contains("the answer is larger than " + Entitlements.MAX_ANSWER_BYTES + " bytes"), + message); + assertTrue(message.contains("Retry once Conductor is reachable"), message); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\n"}) + public void aBlankConductorUrlMeansTheHostedOne(String url) { + var asked = new ArrayList(); + Entitlements.check( + List.of(SOME_FEATURE), + "enterprise-key", + null, + url, + null, + (uri, headers) -> { + asked.add(uri); + return new Answer(200, null, "{\"entitlements\": {\"someFeature\": true}}"); + }); + assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); } @Test From 01673612fc1cc14b9f69765df0ba07c8f1596804 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 12:53:56 -0800 Subject: [PATCH 06/10] Count a malformed answer as an outage, and refuse a Conductor URL without a parseable host - An exchange that fails in flight is an outage, whatever exception the JDK raises. A non-numeric or overflowing Content-Length fails it with a NumberFormatException, an IllegalArgumentException. That was reported as "the request could not be sent", which failed closed even against the hosted Conductor. Only an IllegalArgumentException thrown while building or sending the request is a refusal now. - A Conductor URL whose host java.net.URI cannot identify is refused as invalid. With a raw '/' or '@' in the password, the parser left the credentials inside the authority, and the refusal message quoted them. The same goes for a host with an underscore. The Conductor client cannot connect to any of these either: the JDK WebSocket client refuses a URI without a host. The entitlement route is built from the parsed host and port. - DBOSConfig.toString masks everything up to the last '@' in the Conductor domain, so a raw '/' or '@' in the password no longer leaves part of it showing. A path containing '@' is masked too, which is harmless when printing. - Tests: - an answer of exactly 1 MiB is read; - keys with a tab, DEL and characters 0x80-0xFF are refused. --- .../dev/dbos/transact/config/DBOSConfig.java | 8 +- .../dbos/transact/internal/Entitlements.java | 17 +++- .../transact/config/DBOSConfigKeysTest.java | 9 ++ .../transact/internal/EntitlementsTest.java | 92 ++++++++++++++++++- 4 files changed, 117 insertions(+), 9 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index a801b3142..92b86d9c7 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -214,11 +214,15 @@ public int adminServerPort() { return stripped.isEmpty() ? null : stripped; } - /** The domain with any {@code user:password@} replaced, for printing. */ + /** + * The domain with everything up to its last {@code @} replaced, for printing. That masks any + * {@code user:password@}, even one with a raw '/' or '@' in the password, at the cost of also + * masking a path that contains '@'. + */ private static @Nullable String withoutCredentials(@Nullable String domain) { return domain == null ? null - : domain.replaceFirst("^((?:[A-Za-z][A-Za-z0-9+.-]*://)?)[^/@]*@", "$1***@"); + : domain.replaceFirst("(?s)^((?:[A-Za-z][A-Za-z0-9+.-]*://)?).*@", "$1***@"); } // Copy constructor diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index e99cf560d..1341dfdcc 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -325,8 +325,14 @@ static String entitlementUrl(String conductorUrl) throws InvalidUrl { case "ws", "http" -> "http"; default -> throw new InvalidUrl("its scheme is not ws, wss, http or https"); }; + // A host the parser cannot identify (a raw '/' or '@' in a password, or an underscore in the + // host) leaves the credentials unseparated. The Conductor client cannot connect to such a + // URL either. + if (uri.getHost() == null) { + throw new InvalidUrl("its host is not valid"); + } // Without any user:password@, which would otherwise be sent and quoted in errors. - var host = authority.substring(authority.lastIndexOf('@') + 1); + var host = uri.getHost() + (uri.getPort() == -1 ? "" : ":" + uri.getPort()); // The hosted websocket URL carries an API version; routes sit beside it. var path = uri.getRawPath() == null ? "" : uri.getRawPath().replaceAll("/+$", ""); if (path.endsWith("/v1alpha1")) { @@ -363,14 +369,15 @@ static Http http(Duration timeout) { pending.cancel(true); throw e; } catch (ExecutionException e) { + // The exchange failed in flight, so an outage, whatever the exception. The JDK reports some + // malformed answers (a non-numeric Content-Length) as an IllegalArgumentException, which + // must not read as a request this client refused to send. var cause = e.getCause(); if (cause instanceof IOException io) { throw io; } - if (cause instanceof RuntimeException runtime) { - throw runtime; - } - throw new IOException(cause); + throw new IOException( + "a malformed answer (" + cause.getClass().getSimpleName() + ")", cause); } return new Answer( response.statusCode(), diff --git a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java index edaa21a81..50cc450ce 100644 --- a/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java +++ b/transact/src/test/java/dev/dbos/transact/config/DBOSConfigKeysTest.java @@ -75,6 +75,15 @@ public void credentialsInTheConductorDomainAreMaskedInToString() { assertFalse(text.contains("user"), text); assertTrue( text.contains("conductorDomain=wss://***@conductor.example.com/conductor/v1alpha1"), text); + // Even with a raw '@' or '/' in the password. + for (var domain : + new String[] { + "wss://user:p@secret@host/conductor/v1alpha1", "ws://user:pa/secret@host/x" + }) { + var masked = DBOSConfig.defaults("app").withConductorDomain(domain).toString(); + assertFalse(masked.contains("secret"), masked); + assertFalse(masked.contains("user"), masked); + } var plain = DBOSConfig.defaults("app").withConductorDomain("ws://localhost:8090").toString(); assertTrue(plain.contains("conductorDomain=ws://localhost:8090,"), plain); } diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index cece59045..978f38dc3 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.net.ConnectException; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.URI; @@ -186,7 +187,11 @@ public void anInvalidConductorUrlIsRefused() { "wss://user:pa secret@host/conductor/v1alpha1", // No scheme, so the user name reads as one. "user:secret@host", - "htp://user:secret@host" + "htp://user:secret@host", + // A raw '/' in the password: the parser reads "user:pa" as the whole authority. + "ws://user:pa/secret@127.0.0.1:1/x", + // A raw '@' in the password. + "wss://user:p@secret@host/conductor/v1alpha1" }) public void anInvalidConductorUrlIsNeverQuoted(String url) { var e = @@ -242,7 +247,16 @@ public void surroundingWhitespaceIsNotPartOfTheKey() { @ParameterizedTest @ValueSource( - strings = {"enterprise\nkey", "enterprise\u20ackey", "enterprise\u0100key", "enterprise key"}) + strings = { + "enterprise\nkey", + "enterprise\tkey", + "enterprise\u007fkey", + "enterprise\u0080key", + "enterprise\u00ffkey", + "enterprise\u20ackey", + "enterprise\u0100key", + "enterprise key" + }) public void aKeyThatIsNotPrintableAsciiIsRefusedWithoutQuotingIt(String key) { var e = refused(key); assertTrue(e.getMessage().contains("other than printable ASCII"), e.getMessage()); @@ -285,6 +299,80 @@ public void aRequestTheHttpClientRefusesFailsClosedEvenWhenHosted() { } } + @Test + public void aHostTheParserCannotIdentifyIsInvalid() { + // Such as an underscore in the host. The Conductor client cannot connect to it either. + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.require( + List.of(SOME_FEATURE), + "enterprise-key", + null, + "ws://conductor_svc:8090", + false)); + assertTrue(e.getMessage().contains("its host is not valid"), e.getMessage()); + } + + @Test + public void anAnswerOfExactlyTheLimitIsRead() { + var prefix = "{\"entitlements\": {\"someFeature\": true}, \"padding\": \""; + conductor.body = + prefix + "x".repeat(Entitlements.MAX_ANSWER_BYTES - prefix.length() - 2) + "\"}"; + assertEquals(Entitlements.MAX_ANSWER_BYTES, conductor.body.length()); + check("enterprise-key", null); + } + + @Test + public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { + // The JDK fails the exchange with a NumberFormatException, an IllegalArgumentException, for a + // non-numeric Content-Length. That is the server's fault: an outage, which only the hosted + // Conductor survives, never "could not be sent". + try (var server = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + var answering = + new Thread( + () -> { + try (var socket = server.accept()) { + var in = socket.getInputStream(); + // Read the request up to its blank line. + var seen = 0; + while (seen < 4) { + var c = in.read(); + if (c < 0) { + return; + } + seen = (c == '\r' || c == '\n') ? seen + 1 : 0; + } + socket + .getOutputStream() + .write( + "HTTP/1.1 200 OK\r\nContent-Length: abc\r\n\r\n" + .getBytes(StandardCharsets.US_ASCII)); + socket.getOutputStream().flush(); + Thread.sleep(2000); + } catch (IOException | InterruptedException e) { + // The test is over. + } + }); + answering.setDaemon(true); + answering.start(); + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.require( + List.of(SOME_FEATURE), + "enterprise-key", + null, + "ws://127.0.0.1:" + server.getLocalPort(), + false)); + assertTrue(e.getMessage().contains("Retry once Conductor is reachable"), e.getMessage()); + assertTrue(e.getMessage().contains("a malformed answer"), e.getMessage()); + assertFalse(e.getMessage().contains("could not be sent"), e.getMessage()); + } + } + @Test public void anOversizedAnswerCountsAsAnOutage() { conductor.body = From 80bc742539a3eee0ecde0635033f6f3867b67d3d Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 13:43:18 -0800 Subject: [PATCH 07/10] Read the entitlement answer without a size cap, as Python and TypeScript do Drops the custom bounded body subscriber. Conductor's /v2/entitlements answers with six booleans, or a short problem detail, so the answer is a few hundred bytes. Only a non-Conductor endpoint could send a large body. The JDK has no capped body handler before release 23 (this project builds for 17), and Python and TypeScript read the whole body too. The answer is now read with BodyHandlers.ofString(), still under the one deadline that bounds the whole exchange. --- .../dbos/transact/internal/Entitlements.java | 63 +------------------ .../transact/internal/EntitlementsTest.java | 22 ------- 2 files changed, 1 insertion(+), 84 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index 1341dfdcc..7e94fde48 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -3,7 +3,6 @@ import dev.dbos.transact.DBOS; import dev.dbos.transact.config.DBOSConfig; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.ProxySelector; import java.net.URI; @@ -12,17 +11,12 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.net.http.HttpTimeoutException; -import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutionException; -import java.util.concurrent.Flow; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.function.Function; @@ -70,8 +64,6 @@ public record Feature(String entitlement, String description) {} private static final String KEY_HINT = "Set transactEnterpriseKey or conductorKey in your DBOSConfig."; private static final ObjectMapper MAPPER = new ObjectMapper(); - // Read no more of an answer than this; an entitlement list is a few hundred bytes. - static final int MAX_ANSWER_BYTES = 1 << 20; private Entitlements() {} @@ -356,7 +348,7 @@ static Http http(Duration timeout) { var request = HttpRequest.newBuilder(uri).GET(); headers.forEach(request::header); var pending = - builder.build().sendAsync(request.build(), info -> new BoundedBody(MAX_ANSWER_BYTES)); + builder.build().sendAsync(request.build(), HttpResponse.BodyHandlers.ofString()); HttpResponse response; try { // One deadline for the whole exchange, body included, so an answer that stalls mid-body @@ -386,59 +378,6 @@ static Http http(Duration timeout) { }; } - /** Collects an answer as UTF-8 text, failing once it exceeds {@code max} bytes. */ - static final class BoundedBody implements HttpResponse.BodySubscriber { - private final int max; - private final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); - private final CompletableFuture body = new CompletableFuture<>(); - private Flow.@Nullable Subscription subscription; - - BoundedBody(int max) { - this.max = max; - } - - @Override - public void onSubscribe(Flow.Subscription subscription) { - this.subscription = subscription; - subscription.request(Long.MAX_VALUE); - } - - @Override - public void onNext(List buffers) { - if (body.isDone()) { - return; - } - for (var buffer : buffers) { - if (bytes.size() + buffer.remaining() > max) { - if (subscription != null) { - subscription.cancel(); - } - body.completeExceptionally( - new IOException("the answer is larger than " + max + " bytes")); - return; - } - var chunk = new byte[buffer.remaining()]; - buffer.get(chunk); - bytes.write(chunk, 0, chunk.length); - } - } - - @Override - public void onError(Throwable error) { - body.completeExceptionally(error); - } - - @Override - public void onComplete() { - body.complete(bytes.toString(StandardCharsets.UTF_8)); - } - - @Override - public CompletionStage getBody() { - return body; - } - } - /** An answer that is not an entitlement list, which counts as Conductor failing to answer. */ private static final class MalformedAnswer extends Exception { MalformedAnswer(String message) { diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index 978f38dc3..6ce584a1b 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -315,15 +315,6 @@ public void aHostTheParserCannotIdentifyIsInvalid() { assertTrue(e.getMessage().contains("its host is not valid"), e.getMessage()); } - @Test - public void anAnswerOfExactlyTheLimitIsRead() { - var prefix = "{\"entitlements\": {\"someFeature\": true}, \"padding\": \""; - conductor.body = - prefix + "x".repeat(Entitlements.MAX_ANSWER_BYTES - prefix.length() - 2) + "\"}"; - assertEquals(Entitlements.MAX_ANSWER_BYTES, conductor.body.length()); - check("enterprise-key", null); - } - @Test public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { // The JDK fails the exchange with a NumberFormatException, an IllegalArgumentException, for a @@ -373,19 +364,6 @@ public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { } } - @Test - public void anOversizedAnswerCountsAsAnOutage() { - conductor.body = - "{\"entitlements\": {\"someFeature\": true}, \"padding\": \"" - + "x".repeat(Entitlements.MAX_ANSWER_BYTES) - + "\"}"; - var message = refused("enterprise-key").getMessage(); - assertTrue( - message.contains("the answer is larger than " + Entitlements.MAX_ANSWER_BYTES + " bytes"), - message); - assertTrue(message.contains("Retry once Conductor is reachable"), message); - } - @ParameterizedTest @ValueSource(strings = {"", " ", "\n"}) public void aBlankConductorUrlMeansTheHostedOne(String url) { From 7ff0a7c53f9cabab225b29e2d9df17688b6552e1 Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 14:39:26 -0800 Subject: [PATCH 08/10] Refuse a Conductor URL with '@' after its host, and never treat a JVM Error as an outage - A raw '/' in a password can parse as a port and a path: in "wss://user:2024/pass@host/...", "user" becomes the host, 2024 the port, and the rest of the password the path. The check then quoted the password and would have sent the key to the wrong host. A Conductor URL with '@' in its path, query or fragment is now refused as invalid. The earlier host check only caught the case where java.net.URI found no host. - A port above 65535 and an IPv6 zone ID are refused up front. The HTTP client fails both in flight, which read as an outage reported as "a malformed answer". - A JVM Error raised in flight, such as OutOfMemoryError, now propagates instead of being wrapped as an outage, which would have failed open against the hosted Conductor. Other in-flight failures are reported as "the exchange failed". - DBOSConfig.conductorDomain's javadoc said "custom hostname". It must be a ws:// or wss:// URL, both for the check and for the Conductor client. - Tests: - a malformed answer from the hosted Conductor, through the real HTTP client, starts with a warning; - the numeric-port password cases; - log assertions now match only their own test's warning, since tests log to the same logger in parallel. --- .../dev/dbos/transact/config/DBOSConfig.java | 11 +- .../dbos/transact/internal/Entitlements.java | 32 +++-- .../transact/internal/EntitlementsTest.java | 120 ++++++++++++------ 3 files changed, 114 insertions(+), 49 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java index 92b86d9c7..c9454a994 100644 --- a/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java +++ b/transact/src/main/java/dev/dbos/transact/config/DBOSConfig.java @@ -45,9 +45,9 @@ * @param conductorKey API key for DBOS Conductor; null or blank to not connect. Surrounding * whitespace is removed. Connecting to Conductor requires {@code dev.dbos:transact-enterprise} * on the class path. Masked in {@link #toString()} - * @param conductorDomain custom hostname for DBOS Conductor; null or blank for DBOS's hosted - * Conductor. Surrounding whitespace is removed, and credentials in it are masked in {@link - * #toString()} + * @param conductorDomain URL of a self-hosted DBOS Conductor, such as {@code ws://localhost:8090}; + * null or blank for DBOS's hosted Conductor. Surrounding whitespace is removed, and credentials + * in it are masked in {@link #toString()} * @param conductorExecutorMetadata arbitrary key-value metadata attached to this executor's * registration in Conductor * @param appVersion application version string used for versioned workflow routing; must not be @@ -585,8 +585,9 @@ public DBOSConfig(DBOSConfig other) { } /** - * Returns a copy of this config with {@code conductorDomain} set to {@code v}; null or blank for - * DBOS's hosted Conductor. + * Returns a copy of this config with {@code conductorDomain} set to {@code v}: the URL of a + * self-hosted DBOS Conductor, such as {@code ws://localhost:8090}; null or blank for DBOS's + * hosted Conductor. */ public @NonNull DBOSConfig withConductorDomain(@Nullable String v) { return new DBOSConfig( diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index 7e94fde48..d22c2b204 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -317,12 +317,23 @@ static String entitlementUrl(String conductorUrl) throws InvalidUrl { case "ws", "http" -> "http"; default -> throw new InvalidUrl("its scheme is not ws, wss, http or https"); }; - // A host the parser cannot identify (a raw '/' or '@' in a password, or an underscore in the - // host) leaves the credentials unseparated. The Conductor client cannot connect to such a - // URL either. - if (uri.getHost() == null) { + // A host the parser cannot identify (an underscore, a raw '/' or '@' in a password) leaves the + // credentials unseparated, and so does an IPv6 zone ID. The Conductor client cannot connect to + // such a URL either. + if (uri.getHost() == null || uri.getHost().contains("%")) { throw new InvalidUrl("its host is not valid"); } + // A raw '/' in a password can also parse as a port and a path ("user:2024/pass@host"), so the + // host is wrong and the rest of the password sits in the path. A Conductor URL never has '@' + // after its authority. + for (var part : new String[] {uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment()}) { + if (part != null && part.contains("@")) { + throw new InvalidUrl("it has an '@' after its host"); + } + } + if (uri.getPort() > 65535) { + throw new InvalidUrl("its port is out of range"); + } // Without any user:password@, which would otherwise be sent and quoted in errors. var host = uri.getHost() + (uri.getPort() == -1 ? "" : ":" + uri.getPort()); // The hosted websocket URL carries an API version; routes sit beside it. @@ -361,15 +372,20 @@ static Http http(Duration timeout) { pending.cancel(true); throw e; } catch (ExecutionException e) { - // The exchange failed in flight, so an outage, whatever the exception. The JDK reports some - // malformed answers (a non-numeric Content-Length) as an IllegalArgumentException, which - // must not read as a request this client refused to send. + // The exchange failed in flight: an outage. The JDK reports some malformed answers (a + // non-numeric Content-Length) as an IllegalArgumentException, which must not read as a + // request this client refused to send. URLs the client would refuse in flight (a port out + // of range, a zone ID) are refused by entitlementUrl before any request. var cause = e.getCause(); if (cause instanceof IOException io) { throw io; } + if (cause instanceof Error error) { + // Not an outage: a damaged JVM must never fail open. + throw error; + } throw new IOException( - "a malformed answer (" + cause.getClass().getSimpleName() + ")", cause); + "the exchange failed (" + cause.getClass().getSimpleName() + ")", cause); } return new Answer( response.statusCode(), diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index 6ce584a1b..cfc5ee2ba 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -191,7 +191,10 @@ public void anInvalidConductorUrlIsRefused() { // A raw '/' in the password: the parser reads "user:pa" as the whole authority. "ws://user:pa/secret@127.0.0.1:1/x", // A raw '@' in the password. - "wss://user:p@secret@host/conductor/v1alpha1" + "wss://user:p@secret@host/conductor/v1alpha1", + // A raw '/' after digits: "user:2024" parses as a host and port, and the rest as a path. + "wss://user:2024/secret@conductor.example.com/conductor/v1alpha1", + "ws://user:/secret@127.0.0.1:1" }) public void anInvalidConductorUrlIsNeverQuoted(String url) { var e = @@ -295,10 +298,25 @@ public void aRequestTheHttpClientRefusesFailsClosedEvenWhenHosted() { assertTrue(e.getMessage().contains("could not be sent"), e.getMessage()); assertFalse(e.getMessage().contains("secret"), e.getMessage()); assertEquals(null, e.getCause()); - assertEquals(List.of(), logs.messages(Level.WARN, "")); + // Nothing logged quotes it. (Other tests log to the same logger in parallel, so this cannot + // assert that nothing was logged at all; the throw already shows it did not start anyway.) + assertEquals(List.of(), logs.messages(Level.TRACE, "secret")); } } + @ParameterizedTest + @CsvSource({ + "ws://127.0.0.1:70000, its port is out of range", + "'wss://[fe80::1%25eth0]:8443/x', its host is not valid" + }) + public void aUrlTheHttpClientWouldRefuseInFlightIsInvalid(String url, String reason) { + var e = + assertThrows( + IllegalStateException.class, + () -> Entitlements.require(List.of(SOME_FEATURE), "enterprise-key", null, url, false)); + assertTrue(e.getMessage().contains("the Conductor URL is invalid: " + reason), e.getMessage()); + } + @Test public void aHostTheParserCannotIdentifyIsInvalid() { // Such as an underscore in the host. The Conductor client cannot connect to it either. @@ -315,39 +333,46 @@ public void aHostTheParserCannotIdentifyIsInvalid() { assertTrue(e.getMessage().contains("its host is not valid"), e.getMessage()); } - @Test - public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { - // The JDK fails the exchange with a NumberFormatException, an IllegalArgumentException, for a - // non-numeric Content-Length. That is the server's fault: an outage, which only the hosted - // Conductor survives, never "could not be sent". - try (var server = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { - var answering = - new Thread( - () -> { - try (var socket = server.accept()) { - var in = socket.getInputStream(); - // Read the request up to its blank line. - var seen = 0; - while (seen < 4) { - var c = in.read(); - if (c < 0) { - return; - } - seen = (c == '\r' || c == '\n') ? seen + 1 : 0; + /** + * A local server that answers its first request with a non-numeric Content-Length, which the JDK + * reports as a NumberFormatException: an IllegalArgumentException that is the server's fault. + */ + private static ServerSocket malformedServer() throws IOException { + var server = new ServerSocket(0, 1, InetAddress.getLoopbackAddress()); + var answering = + new Thread( + () -> { + try (var socket = server.accept()) { + var in = socket.getInputStream(); + // Read the request up to its blank line. + var seen = 0; + while (seen < 4) { + var c = in.read(); + if (c < 0) { + return; } - socket - .getOutputStream() - .write( - "HTTP/1.1 200 OK\r\nContent-Length: abc\r\n\r\n" - .getBytes(StandardCharsets.US_ASCII)); - socket.getOutputStream().flush(); - Thread.sleep(2000); - } catch (IOException | InterruptedException e) { - // The test is over. + seen = (c == '\r' || c == '\n') ? seen + 1 : 0; } - }); - answering.setDaemon(true); - answering.start(); + socket + .getOutputStream() + .write( + "HTTP/1.1 200 OK\r\nContent-Length: abc\r\n\r\n" + .getBytes(StandardCharsets.US_ASCII)); + socket.getOutputStream().flush(); + Thread.sleep(2000); + } catch (IOException | InterruptedException e) { + // The test is over. + } + }); + answering.setDaemon(true); + answering.start(); + return server; + } + + @Test + public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { + // An outage, which only the hosted Conductor survives, never "could not be sent". + try (var server = malformedServer()) { var e = assertThrows( IllegalStateException.class, @@ -359,11 +384,33 @@ public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { "ws://127.0.0.1:" + server.getLocalPort(), false)); assertTrue(e.getMessage().contains("Retry once Conductor is reachable"), e.getMessage()); - assertTrue(e.getMessage().contains("a malformed answer"), e.getMessage()); + assertTrue(e.getMessage().contains("the exchange failed"), e.getMessage()); assertFalse(e.getMessage().contains("could not be sent"), e.getMessage()); } } + @Test + public void aMalformedAnswerFromTheHostedConductorStartsAnyway() throws Exception { + // The real HTTP client, with the hosted Conductor's request sent to the local server instead. + try (var server = malformedServer(); + var logs = new LogCapture(Entitlements.class)) { + var local = URI.create("http://127.0.0.1:" + server.getLocalPort() + "/v2/entitlements"); + var http = Entitlements.http(Duration.ofSeconds(5)); + var asked = new ArrayList(); + checkHosted( + null, + (uri, headers) -> { + asked.add(uri); + return http.get(local, headers); + }); + assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); + // Filtered to this test's own warning: other tests log to the same logger in parallel. + var warnings = logs.messages(Level.WARN, "the exchange failed (NumberFormatException)"); + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("starting anyway"), warnings.get(0)); + } + } + @ParameterizedTest @ValueSource(strings = {"", " ", "\n"}) public void aBlankConductorUrlMeansTheHostedOne(String url) { @@ -602,9 +649,10 @@ public void aHostedConductorOutageDoesNotBlockStartup() { // No Conductor URL means DBOS's hosted one, whose outage only warns. try (var logs = new LogCapture(Entitlements.class)) { assertDoesNotThrow(() -> checkHosted(null, unreachable)); - var warnings = logs.messages(Level.WARN, "Could not verify the DBOS key"); + // Filtered to this test's own warning: other tests log to the same logger in parallel. + var warnings = logs.messages(Level.WARN, "(connection refused)"); assertEquals(1, warnings.size(), warnings.toString()); - assertTrue(warnings.get(0).contains("connection refused"), warnings.get(0)); + assertTrue(warnings.get(0).contains("Could not verify the DBOS key"), warnings.get(0)); assertFalse(warnings.get(0).contains("enterprise-key"), warnings.get(0)); } assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); From 5106bdc1dc1980a563d41bc2564576d9d716620d Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 14:51:04 -0800 Subject: [PATCH 09/10] Rethrow a JVM Error anywhere in an outage's cause chain, and accept only ws and wss Conductor URLs - The HTTP client reports an Error in its own threads, such as an OutOfMemoryError in the selector manager, as the cause of an IOException ("selector manager closed"). The previous fix only caught an Error that was the direct cause, so this one still read as an outage and failed open against the hosted Conductor. The whole cause chain is checked now. - A Conductor URL must be ws:// or wss://. The Conductor client is a WebSocket client, which rejects http and https, so such a URL passed the check and then could never connect. - An IPv6 zone ID is accepted again. java.net.URI separates its credentials correctly, and the client can resolve the scoped address, so refusing it only refused a usable URL. - The tests that read Entitlements' shared log take a JUnit resource lock instead of filtering on test-specific text. The malformed-answer tests no longer depend on what the JDK calls a non-numeric Content-Length. --- .../dbos/transact/internal/Entitlements.java | 38 ++++++++++----- .../transact/internal/EntitlementsTest.java | 46 ++++++++++++------- 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index d22c2b204..ef48c3935 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -12,7 +12,9 @@ import java.net.http.HttpResponse; import java.net.http.HttpTimeoutException; import java.time.Duration; +import java.util.Collections; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -238,6 +240,7 @@ static void check( failure = e.getMessage(); cause = e; } catch (IOException e) { + rethrowError(e); failure = e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); cause = e; } catch (IllegalArgumentException e) { @@ -313,14 +316,14 @@ static String entitlementUrl(String conductorUrl) throws InvalidUrl { } scheme = switch (scheme.toLowerCase()) { - case "wss", "https" -> "https"; - case "ws", "http" -> "http"; - default -> throw new InvalidUrl("its scheme is not ws, wss, http or https"); + // The Conductor client is a WebSocket client, which takes only these. + case "wss" -> "https"; + case "ws" -> "http"; + default -> throw new InvalidUrl("its scheme is not ws or wss"); }; // A host the parser cannot identify (an underscore, a raw '/' or '@' in a password) leaves the - // credentials unseparated, and so does an IPv6 zone ID. The Conductor client cannot connect to - // such a URL either. - if (uri.getHost() == null || uri.getHost().contains("%")) { + // credentials unseparated. The Conductor client cannot connect to such a URL either. + if (uri.getHost() == null) { throw new InvalidUrl("its host is not valid"); } // A raw '/' in a password can also parse as a port and a path ("user:2024/pass@host"), so the @@ -374,16 +377,13 @@ static Http http(Duration timeout) { } catch (ExecutionException e) { // The exchange failed in flight: an outage. The JDK reports some malformed answers (a // non-numeric Content-Length) as an IllegalArgumentException, which must not read as a - // request this client refused to send. URLs the client would refuse in flight (a port out - // of range, a zone ID) are refused by entitlementUrl before any request. + // request this client refused to send. A port out of range, which the client would also + // refuse in flight, is refused by entitlementUrl before any request. var cause = e.getCause(); + rethrowError(cause); if (cause instanceof IOException io) { throw io; } - if (cause instanceof Error error) { - // Not an outage: a damaged JVM must never fail open. - throw error; - } throw new IOException( "the exchange failed (" + cause.getClass().getSimpleName() + ")", cause); } @@ -394,6 +394,20 @@ static Http http(Duration timeout) { }; } + /** + * Throws the first {@link Error} in {@code failure}'s cause chain. Not an outage: a damaged JVM + * must never fail open, and the HTTP client reports one (an OutOfMemoryError in its selector + * thread) as the cause of an IOException. + */ + private static void rethrowError(Throwable failure) { + var seen = Collections.newSetFromMap(new IdentityHashMap()); + for (var t = failure; t != null && seen.add(t); t = t.getCause()) { + if (t instanceof Error error) { + throw error; + } + } + } + /** An answer that is not an entitlement list, which counts as Conductor failing to answer. */ private static final class MalformedAnswer extends Exception { MalformedAnswer(String message) { diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index cfc5ee2ba..9405dfabb 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -33,6 +34,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.ResourceLock; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; @@ -41,6 +43,8 @@ public class EntitlementsTest { private static final Feature SOME_FEATURE = new Feature("someFeature", "Some feature"); private static final Feature OTHER_FEATURE = new Feature("otherFeature", "Doing another thing"); + // Tests that read Entitlements' log, which all tests share: they never run alongside each other. + private static final String ENTITLEMENTS_LOG = "Entitlements log"; private static final String HOSTED_ROUTE = "https://cloud.dbos.dev/conductor/v2/entitlements"; /** A local stand-in for Conductor's entitlements route. */ @@ -164,7 +168,7 @@ public void entitlementUrlSitsBesideTheConductorUrl() throws Exception { Entitlements.entitlementUrl("ws://localhost:8090")); assertEquals( "https://conductor.example.com/v2/entitlements", - Entitlements.entitlementUrl("https://conductor.example.com/")); + Entitlements.entitlementUrl("wss://conductor.example.com/")); } @Test @@ -194,7 +198,9 @@ public void anInvalidConductorUrlIsRefused() { "wss://user:p@secret@host/conductor/v1alpha1", // A raw '/' after digits: "user:2024" parses as a host and port, and the rest as a path. "wss://user:2024/secret@conductor.example.com/conductor/v1alpha1", - "ws://user:/secret@127.0.0.1:1" + "ws://user:/secret@127.0.0.1:1", + // Not a WebSocket URL, which the Conductor client cannot use. + "https://user:secret@conductor.example.com" }) public void anInvalidConductorUrlIsNeverQuoted(String url) { var e = @@ -286,6 +292,7 @@ public void aKeyThatIsNotPrintableAsciiIsRefusedWithoutQuotingIt(String key) { } @Test + @ResourceLock(ENTITLEMENTS_LOG) public void aRequestTheHttpClientRefusesFailsClosedEvenWhenHosted() { // Not an outage, so the hosted Conductor's fail-open does not apply. The client's message, // which can quote the Authorization header, is not passed on. @@ -298,17 +305,12 @@ public void aRequestTheHttpClientRefusesFailsClosedEvenWhenHosted() { assertTrue(e.getMessage().contains("could not be sent"), e.getMessage()); assertFalse(e.getMessage().contains("secret"), e.getMessage()); assertEquals(null, e.getCause()); - // Nothing logged quotes it. (Other tests log to the same logger in parallel, so this cannot - // assert that nothing was logged at all; the throw already shows it did not start anyway.) - assertEquals(List.of(), logs.messages(Level.TRACE, "secret")); + assertEquals(List.of(), logs.messages(Level.TRACE, "")); } } @ParameterizedTest - @CsvSource({ - "ws://127.0.0.1:70000, its port is out of range", - "'wss://[fe80::1%25eth0]:8443/x', its host is not valid" - }) + @CsvSource({"ws://127.0.0.1:70000, its port is out of range"}) public void aUrlTheHttpClientWouldRefuseInFlightIsInvalid(String url, String reason) { var e = assertThrows( @@ -384,12 +386,12 @@ public void aMalformedAnswerIsAnOutageNotARefusalToSend() throws Exception { "ws://127.0.0.1:" + server.getLocalPort(), false)); assertTrue(e.getMessage().contains("Retry once Conductor is reachable"), e.getMessage()); - assertTrue(e.getMessage().contains("the exchange failed"), e.getMessage()); assertFalse(e.getMessage().contains("could not be sent"), e.getMessage()); } } @Test + @ResourceLock(ENTITLEMENTS_LOG) public void aMalformedAnswerFromTheHostedConductorStartsAnyway() throws Exception { // The real HTTP client, with the hosted Conductor's request sent to the local server instead. try (var server = malformedServer(); @@ -404,13 +406,24 @@ public void aMalformedAnswerFromTheHostedConductorStartsAnyway() throws Exceptio return http.get(local, headers); }); assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); - // Filtered to this test's own warning: other tests log to the same logger in parallel. - var warnings = logs.messages(Level.WARN, "the exchange failed (NumberFormatException)"); + // Whatever the JDK calls the failure, it is an outage, which the hosted Conductor survives. + var warnings = logs.messages(Level.WARN, "starting anyway"); assertEquals(1, warnings.size(), warnings.toString()); - assertTrue(warnings.get(0).contains("starting anyway"), warnings.get(0)); } } + @Test + public void aJvmErrorBehindAnOutageNeverStartsAnyway() { + // The HTTP client reports an OutOfMemoryError in its selector thread as the cause of an + // IOException. Against the hosted Conductor, reading that as an outage would fail open. + var oom = new OutOfMemoryError("test"); + Entitlements.Http failing = + (uri, headers) -> { + throw new IOException("selector manager closed", oom); + }; + assertSame(oom, assertThrows(OutOfMemoryError.class, () -> checkHosted(null, failing))); + } + @ParameterizedTest @ValueSource(strings = {"", " ", "\n"}) public void aBlankConductorUrlMeansTheHostedOne(String url) { @@ -639,6 +652,7 @@ public void anUnreachableConductorBlocksStartup() throws IOException { } @Test + @ResourceLock(ENTITLEMENTS_LOG) public void aHostedConductorOutageDoesNotBlockStartup() { var asked = new ArrayList(); Entitlements.Http unreachable = @@ -649,10 +663,9 @@ public void aHostedConductorOutageDoesNotBlockStartup() { // No Conductor URL means DBOS's hosted one, whose outage only warns. try (var logs = new LogCapture(Entitlements.class)) { assertDoesNotThrow(() -> checkHosted(null, unreachable)); - // Filtered to this test's own warning: other tests log to the same logger in parallel. - var warnings = logs.messages(Level.WARN, "(connection refused)"); + var warnings = logs.messages(Level.WARN, "Could not verify the DBOS key"); assertEquals(1, warnings.size(), warnings.toString()); - assertTrue(warnings.get(0).contains("Could not verify the DBOS key"), warnings.get(0)); + assertTrue(warnings.get(0).contains("connection refused"), warnings.get(0)); assertFalse(warnings.get(0).contains("enterprise-key"), warnings.get(0)); } assertEquals(List.of(URI.create(HOSTED_ROUTE)), asked); @@ -665,6 +678,7 @@ public void aHostedConductorOutageDoesNotBlockStartup() { @ParameterizedTest @ValueSource(ints = {403, 407, 408, 425, 429, 500, 503}) + @ResourceLock(ENTITLEMENTS_LOG) public void hostedThrottlingAndProxiesCountAsOutages(int status) { // From the hosted Conductor these mean it couldn't answer, not that it refused the key, so // startup warns and goes on as for any hosted outage. From 181340302290bc5c50a48babdaadb334ff8f50da Mon Sep 17 00:00:00 2001 From: Harry Pierson Date: Fri, 13 Nov 2026 16:12:10 -0800 Subject: [PATCH 10/10] Keep the key out of server-supplied diagnostics, and recognize the hosted Conductor by its route - A problem detail from Conductor, and a transport failure's message, are quoted in refusals and in the hosted warning. Any occurrence of the key in them is now masked, so an answer that echoes the key cannot put it in an exception or a log line. - The hosted Conductor is recognized by the entitlement route the check asks, compared case-insensitively, rather than by the exact configured string. So "wss://cloud.dbos.dev/conductor/v1alpha1/" (trailing slash) or an upper-case host is treated like the default: never asked without a key, and failing open on an outage. The URL is validated before the no-key check, so an invalid one is reported as invalid. --- .../dbos/transact/internal/Entitlements.java | 41 +++++---- .../transact/internal/EntitlementsTest.java | 85 +++++++++++++++++++ 2 files changed, 111 insertions(+), 15 deletions(-) diff --git a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java index ef48c3935..241ecf353 100644 --- a/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java +++ b/transact/src/main/java/dev/dbos/transact/internal/Entitlements.java @@ -53,6 +53,8 @@ public record Feature(String entitlement, String description) {} static final String ENTITLEMENTS_PATH = "/v2/entitlements"; static final Duration TIMEOUT = Duration.ofSeconds(10); static final String HOSTED_CONDUCTOR_URL = "wss://cloud.dbos.dev/conductor/v1alpha1"; + // The hosted Conductor's entitlement route, which identifies it however its URL is written. + static final String HOSTED_ROUTE = "https://cloud.dbos.dev/conductor" + ENTITLEMENTS_PATH; // Conductor's machine-readable error code, in this header, for an org past its plan limit. private static final String ERROR_CODE_HEADER = "X-DBOS-Error"; private static final String PAST_LIMIT = "past_limit"; @@ -140,8 +142,21 @@ static void check( } conductorUrl = setting(conductorUrl); var target = conductorUrl != null ? conductorUrl : defaultConductorUrl(dbosDomain); + String url; + try { + url = entitlementUrl(target); + } catch (InvalidUrl e) { + // The URL is not quoted: it may carry credentials. + throw new IllegalStateException( + names + + " requires an entitled DBOS key, but the Conductor URL is invalid: " + + e.getMessage()); + } + // DBOS's hosted Conductor however its URL is written (a trailing slash, upper case), by the + // route the check asks. + var hosted = url.equalsIgnoreCase(HOSTED_ROUTE); // Only a Conductor named explicitly is asked without a key, and never DBOS's hosted one. - if (key == null && (conductorUrl == null || target.equals(HOSTED_CONDUCTOR_URL))) { + if (key == null && (conductorUrl == null || hosted)) { throw new IllegalStateException(names + " requires a DBOS key. " + KEY_HINT); } // A DBOS key is printable ASCII. Anything else is refused here, unquoted, rather than by the @@ -152,16 +167,6 @@ static void check( + " requires a DBOS key, but the configured key is not one: it contains a character" + " other than printable ASCII."); } - String url; - try { - url = entitlementUrl(target); - } catch (InvalidUrl e) { - // The URL is not quoted: it may carry credentials. - throw new IllegalStateException( - names - + " requires an entitled DBOS key, but the Conductor URL is invalid: " - + e.getMessage()); - } var headers = new HashMap(); headers.put("User-Agent", "dbos-transact-java/" + DBOS.version()); headers.put("Accept", "application/json"); @@ -188,7 +193,8 @@ static void check( } return; } - var detail = problemDetail(answer.body()); + // Conductor's own words, but never the key, should an answer echo it. + var detail = redact(problemDetail(answer.body()), key); var reason = detail != null ? ": " + detail : ""; if (status == 403 && PAST_LIMIT.equals(answer.errorCode())) { throw refusal( @@ -198,7 +204,7 @@ static void check( + names + "). Upgrade its plan to continue."); } - if (target.equals(HOSTED_CONDUCTOR_URL) && HOSTED_UNAVAILABLE.contains(status)) { + if (hosted && HOSTED_UNAVAILABLE.contains(status)) { // An outage, handled below. } else if (status == 404 && detail == null) { // A bare 404 is a missing route: a Conductor predating per-feature entitlements. @@ -241,7 +247,7 @@ static void check( cause = e; } catch (IOException e) { rethrowError(e); - failure = e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName(); + failure = e.getMessage() != null ? redact(e.getMessage(), key) : e.getClass().getSimpleName(); cause = e; } catch (IllegalArgumentException e) { // The HTTP client refused to send the request: not an outage, so it never fails open. Its @@ -256,7 +262,7 @@ static void check( throw new IllegalStateException( "Interrupted while checking the DBOS key's entitlements with Conductor at " + url, e); } - if (target.equals(HOSTED_CONDUCTOR_URL)) { + if (hosted) { logger.warn( "Could not verify the DBOS key's entitlements with Conductor at {} ({}); starting anyway.", url, @@ -284,6 +290,11 @@ static String defaultConductorUrl(@Nullable String dbosDomain) { return "wss://" + domain + "/conductor/v1alpha1"; } + /** {@code text} with every occurrence of {@code key} masked, for a diagnostic. */ + private static @Nullable String redact(@Nullable String text, @Nullable String key) { + return text == null || key == null ? text : text.replace(key, "***"); + } + /** A setting without surrounding whitespace, or null if it is unset or blank. */ private static @Nullable String setting(@Nullable String value) { if (value == null) { diff --git a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java index 9405dfabb..00b264b4e 100644 --- a/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java +++ b/transact/src/test/java/dev/dbos/transact/internal/EntitlementsTest.java @@ -412,6 +412,91 @@ public void aMalformedAnswerFromTheHostedConductorStartsAnyway() throws Exceptio } } + @Test + public void anAnswerThatEchoesTheKeyNeverQuotesIt() { + conductor.reject(401, "invalid API key: enterprise-key"); + var message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("rejected the key (HTTP 401): invalid API key: ***"), message); + assertFalse(message.contains("enterprise-key"), message); + conductor.reject(503, "no org for enterprise-key"); + message = refused("enterprise-key").getMessage(); + assertTrue(message.contains("failed (HTTP 503: no org for ***)"), message); + assertFalse(message.contains("enterprise-key"), message); + } + + @Test + @ResourceLock(ENTITLEMENTS_LOG) + public void aTransportFailureThatEchoesTheKeyNeverQuotesIt() { + Entitlements.Http echoing = + (uri, headers) -> { + throw new IOException("bad answer to enterprise-key"); + }; + // Another Conductor fails closed, without the key in its message. + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.check( + List.of(SOME_FEATURE), + "enterprise-key", + null, + "ws://localhost:8090", + null, + echoing)); + assertTrue(e.getMessage().contains("failed (bad answer to ***)"), e.getMessage()); + assertFalse(e.getMessage().contains("enterprise-key"), e.getMessage()); + // The hosted one warns, without the key in its warning. + try (var logs = new LogCapture(Entitlements.class)) { + checkHosted(null, echoing); + var warnings = logs.messages(Level.WARN, "starting anyway"); + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("bad answer to ***"), warnings.get(0)); + assertFalse(warnings.get(0).contains("enterprise-key"), warnings.get(0)); + } + } + + @ParameterizedTest + @ValueSource( + strings = { + "wss://cloud.dbos.dev/conductor/v1alpha1/", + "WSS://Cloud.DBOS.dev/conductor/v1alpha1", + "wss://cloud.dbos.dev/conductor" + }) + @ResourceLock(ENTITLEMENTS_LOG) + public void theHostedConductorIsRecognizedHoweverItsUrlIsWritten(String url) { + // Never asked without a key. + var asked = new ArrayList(); + var e = + assertThrows( + IllegalStateException.class, + () -> + Entitlements.check( + List.of(SOME_FEATURE), + null, + null, + url, + null, + (uri, headers) -> { + asked.add(uri); + return new Answer(200, null, "{\"entitlements\": {\"someFeature\": true}}"); + })); + assertTrue(e.getMessage().startsWith("Some feature requires a DBOS key."), e.getMessage()); + assertEquals(List.of(), asked); + // And its outage starts anyway, as for the default URL. + try (var logs = new LogCapture(Entitlements.class)) { + Entitlements.check( + List.of(SOME_FEATURE), + "enterprise-key", + null, + url, + null, + (uri, headers) -> { + throw new IOException("connection refused"); + }); + assertEquals(1, logs.messages(Level.WARN, "starting anyway").size()); + } + } + @Test public void aJvmErrorBehindAnOutageNeverStartsAnyway() { // The HTTP client reports an OutOfMemoryError in its selector thread as the cause of an