From b6f300eac8d2c3c521f5d3356be065645a0a36ee Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 19:43:24 -0700 Subject: [PATCH 1/9] refactor(server): unify credential storage on Conductor's SecretsDAO Follow-up to #312 (already on main). Collapses AgentSpan's bespoke credential SPI into Conductor's SecretsDAO so the Credentials UI, credential resolution, env seeding, and Conductor's own ${workflow.secrets.NAME} substitution all read/write one interface. - Add CredentialsDAO extends com.netflix.conductor.dao.SecretsDAO (adds only listWithMeta() for the Credentials UI); remove the CredentialStoreProvider SPI. - AgentspanSecretsDAO becomes the encrypted store itself: AES-256-GCM over credentials_store via JDBC, with an atomic ON CONFLICT DO UPDATE upsert (fixes an earlier first-write race). Delete EncryptedDbCredentialStoreProvider. - Remove the OSS no-op masking layer (SecretOutputMasker SPI, NoOpSecretOutputMasker, CredentialMaskingResponseAdvice). - Rewire SecretController / CredentialResolutionService / CredentialEnvSeeder onto SecretsDAO / CredentialsDAO. - Build: bump conductor 3.32.0-rc.5 -> rc.6; order mavenLocal before Central; add micrometer-registry-prometheus; HTTP/LLM worker threads. Known issue (to be addressed separately): the agentspan.embedded default (false->true) and four @ConditionalOnProperty gates were flipped together, inverting the flag vs. its documentation. NativeSecretGatingTest and ProviderStatusEndpointTest fail as a result (both pass on main). --- server/build.gradle | 13 +- .../conductor-agentspan-server/build.gradle | 1 + .../CredentialMaskingResponseAdvice.java | 142 ------------ .../credentials/AgentspanSecretsDAO.java | 209 +++++++++++++++--- .../credentials/CredentialEnvSeeder.java | 22 +- .../EncryptedDbCredentialStoreProvider.java | 188 ---------------- .../runtime/credentials/MasterKeyConfig.java | 2 +- .../credentials/NoOpSecretOutputMasker.java | 31 --- .../src/main/resources/application.properties | 12 +- .../CredentialMaskingIntegrationTest.java | 107 --------- .../CredentialMaskingWorkflowOptInTest.java | 104 --------- .../ProviderStatusEndpointTest.java | 12 +- .../credentials/AgentspanSecretsDAOTest.java | 145 +++++++----- .../credentials/ConcurrentPutRaceTest.java | 19 +- .../CredentialEnvSeederIntegrationTest.java | 11 +- .../credentials/CredentialEnvSeederTest.java | 64 +++--- .../CredentialResolutionServiceTest.java | 25 ++- ...ncryptedDbCredentialStoreProviderTest.java | 99 --------- .../SchemaMigratorUpgradePathTest.java | 17 +- .../runtime/controller/SecretController.java | 25 +-- .../CredentialResolutionService.java | 15 +- .../runtime/service/AgentHumanTaskConfig.java | 2 +- .../runtime/spi/CredentialStoreProvider.java | 41 ---- .../agentspan/runtime/spi/CredentialsDAO.java | 33 +++ .../runtime/spi/SecretOutputMasker.java | 25 --- .../dev/agentspan/runtime/tasks/Join.java | 2 +- .../runtime/util/ProviderValidator.java | 2 +- .../credentials/NativeSecretGatingTest.java | 4 +- 28 files changed, 425 insertions(+), 947 deletions(-) delete mode 100644 server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/controller/CredentialMaskingResponseAdvice.java delete mode 100644 server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProvider.java delete mode 100644 server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/NoOpSecretOutputMasker.java delete mode 100644 server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingIntegrationTest.java delete mode 100644 server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingWorkflowOptInTest.java delete mode 100644 server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProviderTest.java delete mode 100644 server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialStoreProvider.java create mode 100644 server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialsDAO.java delete mode 100644 server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SecretOutputMasker.java diff --git a/server/build.gradle b/server/build.gradle index abfabb53..4a91f317 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -9,17 +9,15 @@ plugins { // Root project needs repositories too — spotless (applied here) resolves // the palantir-java-format artifact at the root level. repositories { - mavenCentral() + // mavenLocal first: a locally-published conductor-ai (same version) must win + // over Central so an in-place patched build is picked up. mavenLocal() + mavenCentral() } // ── Version catalog ────────────────────────────────────────────── ext { - // TARGET branch: worker secrets use TaskDef.runtimeMetadata (conductor-oss PR #1255), so the - // server references TaskDef.setRuntimeMetadata and must build against a conductor that has it. - // Pinned to the local runtimemeta build (superset of 3.32.0-rc.3); revert to a published version - // once PR #1255 ships. (The interim on feature/embedded-secret-toggle builds against 3.32.0-rc.3.) - conductorVersion = '3.32.0-rc.5' + conductorVersion = '3.32.0-rc.6' lombokVersion = '1.18.42' log4jVersion = '2.24.3' sqliteJdbcVersion = '3.47.0.0' @@ -46,8 +44,9 @@ subprojects { } repositories { - mavenCentral() + // mavenLocal first: locally-published conductor-ai patch wins over Central. mavenLocal() + mavenCentral() } configurations.all { diff --git a/server/conductor-agentspan-server/build.gradle b/server/conductor-agentspan-server/build.gradle index 165684b9..5e6ce9aa 100644 --- a/server/conductor-agentspan-server/build.gradle +++ b/server/conductor-agentspan-server/build.gradle @@ -59,6 +59,7 @@ dependencies { implementation 'org.springframework.boot:spring-boot-starter-web' implementation 'org.springframework.boot:spring-boot-starter-validation' implementation 'org.springframework.boot:spring-boot-starter-actuator' + implementation 'io.micrometer:micrometer-registry-prometheus' implementation 'org.springframework.retry:spring-retry' annotationProcessor 'org.springframework.boot:spring-boot-configuration-processor' implementation 'org.springframework.boot:spring-boot-starter-log4j2' diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/controller/CredentialMaskingResponseAdvice.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/controller/CredentialMaskingResponseAdvice.java deleted file mode 100644 index 4be523e4..00000000 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/controller/CredentialMaskingResponseAdvice.java +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.controller; - -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.core.MethodParameter; -import org.springframework.http.MediaType; -import org.springframework.http.converter.HttpMessageConverter; -import org.springframework.http.server.ServerHttpRequest; -import org.springframework.http.server.ServerHttpResponse; -import org.springframework.web.bind.annotation.ControllerAdvice; -import org.springframework.web.servlet.mvc.method.annotation.ResponseBodyAdvice; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; - -import dev.agentspan.runtime.context.RequestContextHolder; -import dev.agentspan.runtime.spi.SecretOutputMasker; - -/** - * Redacts disclosed credential values from execution-read response bodies. - * - *

Activates for endpoints that include an {@code executionId} in their URL — - * specifically, {@code /api/agent/executions/{id}}, its {@code /full}, - * {@code /tasks}, {@code /status} sub-paths, and {@code /api/agent/execution/{id}}.

- * - *

Host-owned endpoints. {@code /api/workflow/{id}} is the raw Conductor - * workflow read, owned by the host — not AgentSpan. Masking it is opt-in via - * {@code agentspan.credentials.mask-workflow-reads=true} (default {@code false}), so - * merely embedding this library never mutates a host's workflow responses. AgentSpan's - * own {@code /api/agent/*} reads are always masked.

- * - *

How it works:

- *
    - *
  1. Extract {@code executionId} from the request URI.
    2. - *
  2. Get {@code userId} from the request-scoped {@link RequestContextHolder}.
    3. - *
  3. Serialize the response body to JSON.
    4. - *
  4. Run {@link SecretOutputMasker#mask} over the JSON string — it looks up - * the secrets disclosed during this execution and replaces literal - * occurrences of their plaintext with {@code ***NAME***}.
    5. - *
  5. Parse the masked JSON back to a {@link JsonNode} so Spring serializes it - * in place of the original body.
    6. - *
- * - *

If anything goes wrong (no execution id, no user, no disclosures, parse - * error) the body is returned untouched. Masking is best-effort defense in - * depth — it should never block a response from going out.

- */ -@ControllerAdvice -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) -public class CredentialMaskingResponseAdvice implements ResponseBodyAdvice { - - private static final Logger log = LoggerFactory.getLogger(CredentialMaskingResponseAdvice.class); - - /** - * Matches execution-read endpoints that may surface task output/data: - * - */ - private static final Pattern EXEC_URI = Pattern.compile("^(?:" - + "/api/agent/(?:execution(?:s)?/([^/]+?)(?:/(?:full|tasks))?|([^/]+?)/status)" - + "|/api/workflow/([^/?]+)" - + ")/?$"); - - private final SecretOutputMasker masker; - private final ObjectMapper mapper; - - /** - * Whether to mask the host-owned {@code /api/workflow/{id}} endpoint. Off by default so the - * library never mutates an embedding host's raw Conductor responses unless it opts in. - */ - private final boolean maskWorkflowReads; - - public CredentialMaskingResponseAdvice( - SecretOutputMasker masker, - ObjectMapper mapper, - @Value("${agentspan.credentials.mask-workflow-reads:false}") boolean maskWorkflowReads) { - this.masker = masker; - this.mapper = mapper; - this.maskWorkflowReads = maskWorkflowReads; - } - - @Override - public boolean supports(MethodParameter returnType, Class> converterType) { - return true; // cheap; URI check in beforeBodyWrite does the real filtering - } - - @Override - public Object beforeBodyWrite( - Object body, - MethodParameter returnType, - MediaType selectedContentType, - Class> selectedConverterType, - ServerHttpRequest request, - ServerHttpResponse response) { - - if (body == null) return null; - - // Skip if not JSON; reserved paths like /stream return SSE - if (selectedContentType != null && !MediaType.APPLICATION_JSON.includes(selectedContentType)) { - return body; - } - - String path = request.getURI().getPath(); - Matcher m = EXEC_URI.matcher(path); - if (!m.matches()) return body; // not an execution-read endpoint - // group 1: /agent/executions/{id} or /agent/execution/{id} - // group 2: /agent/{id}/status - // group 3: /workflow/{id} - String executionId = m.group(1) != null ? m.group(1) : m.group(2) != null ? m.group(2) : m.group(3); - // group 3 = /api/workflow/{id}, a host-owned endpoint. Skip unless explicitly opted in, - // so embedding the library never mutates the host's raw Conductor workflow responses. - if (m.group(3) != null && !maskWorkflowReads) return body; - // exclude reserved sub-paths that happen to match the regex - if (executionId.equals("prune") || executionId.equals("search")) return body; - - String userId = RequestContextHolder.get().map(c -> c.getUserId()).orElse(null); - if (userId == null) return body; // anonymous request — nothing to mask against - - try { - String json = mapper.writeValueAsString(body); - String masked = masker.mask(executionId, userId, json); - if (masked == null || masked.equals(json)) return body; // no-op fast path - return mapper.readTree(masked); - } catch (Exception e) { - log.warn("Credential masking skipped for {} ({}): {}", path, executionId, e.toString()); - return body; - } - } -} diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAO.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAO.java index 4da13cc3..db1fa28e 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAO.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAO.java @@ -4,77 +4,214 @@ */ package dev.agentspan.runtime.credentials; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; +import java.time.Instant; import java.util.List; -import java.util.stream.Collectors; +import java.util.Map; + +import javax.crypto.Cipher; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.SecretKeySpec; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.dao.EmptyResultDataAccessException; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.stereotype.Component; -import com.netflix.conductor.dao.SecretsDAO; - import dev.agentspan.runtime.model.credentials.CredentialMeta; -import dev.agentspan.runtime.spi.CredentialStoreProvider; +import dev.agentspan.runtime.spi.CredentialsDAO; /** - * Bridges conductor's global {@link SecretsDAO} to AgentSpan's own {@link CredentialStoreProvider} - * (the encrypted credential store). + * AES-256-GCM encrypted credential store, backing conductor's global {@link + * com.netflix.conductor.dao.SecretsDAO} via {@link CredentialsDAO}. * - *

Active only when {@code conductor.secrets.type=agentspan} — the "agentspan-as-host" mode where - * the AgentSpan server embeds conductor ({@code agentspan.embedded=true}) and also serves as - * the secret-resolving host. In that mode the embedded conductor's {@code RuntimeMetadataResolver} - * calls {@link #getSecret(String)} at each SIMPLE task's poll to resolve the secret names a worker - * declared on {@code TaskDef.runtimeMetadata}, injecting the resolved values onto the wire-only - * {@code Task.runtimeMetadata}. Selecting this DAO ({@code havingValue="agentspan"}) gates conductor's - * own env-variable / noop {@code SecretsDAO} implementations off (they require - * {@code conductor.secrets.type} to be {@code env}/absent or {@code noop}).

+ *

Active only when {@code conductor.secrets.type=agentspan} — the "agentspan-as-host" mode + * where the AgentSpan server embeds conductor ({@code agentspan.embedded=true}) and also + * serves as the secret-resolving host. In that mode the embedded conductor's {@code + * RuntimeMetadataResolver} calls {@link #getSecret(String)} at each SIMPLE task's poll to resolve + * the secret names a worker declared on {@code TaskDef.runtimeMetadata}, injecting the resolved + * values onto the wire-only {@code Task.runtimeMetadata}. Selecting this DAO ({@code + * havingValue="agentspan"}) gates conductor's own env-variable / noop {@code SecretsDAO} + * implementations off (they require {@code conductor.secrets.type} to be {@code env}/absent or + * {@code noop}). * - *

Conductor secrets are global (name only), which matches {@link CredentialStoreProvider}'s - * single-scope store. Names are treated as flat keys (no dotted JSONPath): worker credential names - * are simple identifiers, and {@link CredentialStoreProvider#get} resolves them directly.

+ *

Also the single storage backend for AgentSpan's own credential surfaces — + * {@code SecretController} (the Credentials UI), {@code CredentialResolutionService}, and {@code + * CredentialEnvSeeder} — all of which depend on {@link CredentialsDAO} / {@code SecretsDAO} + * directly rather than a separate storage interface. * - *

The backing store beans ({@code EncryptedDbCredentialStoreProvider}, {@code MasterKeyConfig}, - * {@code CredentialDataSourceConfig}, {@code CredentialSchemaMigrator}) are normally dormant when - * embedded; they are re-enabled under this same {@code conductor.secrets.type=agentspan} flag so this - * bridge has a store to read from.

+ *

Encryption format: [12-byte IV][ciphertext+16-byte GCM tag], concatenated into a single BLOB + * stored in {@code credentials_store.encrypted_value}. The master key is the 32-byte key from + * {@code MasterKeyConfig#credentialMasterKey()}. */ @Component @ConditionalOnProperty(name = "conductor.secrets.type", havingValue = "agentspan") -public class AgentspanSecretsDAO implements SecretsDAO { +public class AgentspanSecretsDAO implements CredentialsDAO { private static final Logger log = LoggerFactory.getLogger(AgentspanSecretsDAO.class); + private static final String ALGORITHM = "AES/GCM/NoPadding"; + private static final int IV_LENGTH = 12; // GCM standard nonce + private static final int TAG_LENGTH = 128; // GCM auth tag bits + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + /** + * Fixed single-scope owner for all rows. The {@code credentials_store} schema keeps its + * {@code user_id} column (part of the (user_id, name) primary key), but the store is global — + * every row is written and read under this constant, so lookups are effectively by name. + */ + private static final String DEFAULT_USER_ID = "00000000-0000-0000-0000-000000000000"; - private final CredentialStoreProvider store; + private final NamedParameterJdbcTemplate jdbc; + private final byte[] masterKey; - public AgentspanSecretsDAO(CredentialStoreProvider store) { - this.store = store; - log.info("AgentspanSecretsDAO active — embedded conductor secrets resolve from the " - + "AgentSpan credential store"); + public AgentspanSecretsDAO( + @Qualifier("credentialJdbc") NamedParameterJdbcTemplate jdbc, + @Qualifier("credentialMasterKey") byte[] masterKey) { + this.jdbc = jdbc; + this.masterKey = masterKey; + log.info("AgentspanSecretsDAO active — secrets resolve from the AgentSpan encrypted credential store"); } @Override - public String getSecret(String key) { - return store.get(key); + public String getSecret(String name) { + try { + byte[] encrypted = jdbc.queryForObject( + "SELECT encrypted_value FROM credentials_store " + "WHERE user_id = :uid AND name = :n", + Map.of("uid", DEFAULT_USER_ID, "n", name), + byte[].class); + if (encrypted == null) return null; + return decrypt(encrypted); + } catch (EmptyResultDataAccessException e) { + return null; + } catch (Exception e) { + log.error("Failed to decrypt credential '{}': {}", name, e.getMessage()); + throw new IllegalStateException("Failed to decrypt credential: " + name, e); + } } @Override - public boolean secretExists(String key) { - return store.get(key) != null; + public boolean secretExists(String name) { + return getSecret(name) != null; } @Override public List listSecretNames() { - return store.list().stream().map(CredentialMeta::getName).collect(Collectors.toList()); + return jdbc.query( + "SELECT name FROM credentials_store WHERE user_id = :uid ORDER BY name", + Map.of("uid", DEFAULT_USER_ID), + (rs, row) -> rs.getString("name")); + } + + @Override + public void putSecret(String name, String value) { + try { + byte[] encrypted = encrypt(value); + String now = Instant.now().toString(); + // Single-statement upsert. ON CONFLICT(...) DO UPDATE is supported + // by SQLite 3.24+ and Postgres 9.5+; atomic on both. Replaces an + // earlier UPDATE-then-INSERT pattern that raced on concurrent + // first-write to the same (user_id, name). + jdbc.update( + "INSERT INTO credentials_store (user_id, name, encrypted_value, created_at, updated_at) " + + "VALUES (:uid, :n, :enc, :now, :now) " + + "ON CONFLICT(user_id, name) DO UPDATE SET " + + " encrypted_value = excluded.encrypted_value, " + + " updated_at = excluded.updated_at", + Map.of("uid", DEFAULT_USER_ID, "n", name, "enc", encrypted, "now", now)); + } catch (Exception e) { + throw new IllegalStateException("Failed to store credential: " + name, e); + } } @Override - public void putSecret(String key, String value) { - store.set(key, value); + public void deleteSecret(String name) { + jdbc.update( + "DELETE FROM credentials_store WHERE user_id = :uid AND name = :n", + Map.of("uid", DEFAULT_USER_ID, "n", name)); } @Override - public void deleteSecret(String key) { - store.delete(key); + public List listWithMeta() { + // Include encrypted_value in the SELECT so we can decrypt inline — avoids + // an N+1 query pattern. On SQLite the pool is capped at 1 connection so a + // nested get() call inside a RowMapper would deadlock; on PostgreSQL the + // single-query approach remains more efficient regardless. + return jdbc.query( + "SELECT name, encrypted_value, created_at, updated_at " + + "FROM credentials_store WHERE user_id = :uid ORDER BY name", + Map.of("uid", DEFAULT_USER_ID), + (rs, row) -> { + String name = rs.getString("name"); + byte[] enc = rs.getBytes("encrypted_value"); + String partial; + try { + partial = toPartial(enc != null ? decrypt(enc) : null); + } catch (Exception e) { + partial = "????...????"; + } + return CredentialMeta.builder() + .name(name) + .partial(partial) + .createdAt(parseInstant(rs.getString("created_at"))) + .updatedAt(parseInstant(rs.getString("updated_at"))) + .build(); + }); + } + + // ── Encryption ──────────────────────────────────────────────────── + + private byte[] encrypt(String plaintext) throws Exception { + byte[] iv = new byte[IV_LENGTH]; + SECURE_RANDOM.nextBytes(iv); + + SecretKeySpec keySpec = new SecretKeySpec(masterKey, "AES"); + Cipher cipher = Cipher.getInstance(ALGORITHM); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, new GCMParameterSpec(TAG_LENGTH, iv)); + byte[] ciphertext = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8)); + + // Format: [IV 12 bytes][ciphertext+tag] + ByteBuffer buf = ByteBuffer.allocate(IV_LENGTH + ciphertext.length); + buf.put(iv); + buf.put(ciphertext); + return buf.array(); + } + + private String decrypt(byte[] data) throws Exception { + ByteBuffer buf = ByteBuffer.wrap(data); + byte[] iv = new byte[IV_LENGTH]; + buf.get(iv); + byte[] ciphertext = new byte[buf.remaining()]; + buf.get(ciphertext); + + SecretKeySpec keySpec = new SecretKeySpec(masterKey, "AES"); + Cipher cipher = Cipher.getInstance(ALGORITHM); + cipher.init(Cipher.DECRYPT_MODE, keySpec, new GCMParameterSpec(TAG_LENGTH, iv)); + byte[] plaintext = cipher.doFinal(ciphertext); + return new String(plaintext, StandardCharsets.UTF_8); + } + + // ── Helpers ─────────────────────────────────────────────────────── + + /** + * Return first 4 + "..." + last 4 characters. + * Consistent with OpenAI, GitHub, AWS key display conventions. + */ + static String toPartial(String value) { + if (value == null || value.length() < 8) return "****...****"; + return value.substring(0, 4) + "..." + value.substring(value.length() - 4); + } + + private Instant parseInstant(String s) { + if (s == null) return null; + try { + return Instant.parse(s); + } catch (Exception e) { + return null; + } } } diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/CredentialEnvSeeder.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/CredentialEnvSeeder.java index ed0b03c0..4eab923e 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/CredentialEnvSeeder.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/CredentialEnvSeeder.java @@ -18,7 +18,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.stereotype.Component; -import dev.agentspan.runtime.spi.CredentialStoreProvider; +import com.netflix.conductor.dao.SecretsDAO; /** * On startup, seeds the credential store from well-known LLM provider environment variables. @@ -35,7 +35,7 @@ * (Vault, AWS SM, etc.) manage their own secrets.

*/ @Component -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) +@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") public class CredentialEnvSeeder implements ApplicationRunner { private static final Logger log = LoggerFactory.getLogger(CredentialEnvSeeder.class); @@ -48,7 +48,7 @@ public class CredentialEnvSeeder implements ApplicationRunner { */ static final List KNOWN_ENV_VARS = KnownProviderEnvVars.NAMES; - private final CredentialStoreProvider storeProvider; + private final SecretsDAO secretsDAO; private final Function envLookup; @Value("${agentspan.credentials.store:built-in}") @@ -56,13 +56,13 @@ public class CredentialEnvSeeder implements ApplicationRunner { /** Production constructor — reads from the real process environment. */ @Autowired - public CredentialEnvSeeder(CredentialStoreProvider storeProvider) { - this(storeProvider, System::getenv); + public CredentialEnvSeeder(SecretsDAO secretsDAO) { + this(secretsDAO, System::getenv); } /** Package-private constructor for testing — accepts a custom env lookup. */ - CredentialEnvSeeder(CredentialStoreProvider storeProvider, Function envLookup) { - this.storeProvider = storeProvider; + CredentialEnvSeeder(SecretsDAO secretsDAO, Function envLookup) { + this.secretsDAO = secretsDAO; this.envLookup = envLookup; } @@ -84,7 +84,7 @@ public void run(ApplicationArguments args) { String existing; try { - existing = storeProvider.get(name); + existing = secretsDAO.getSecret(name); } catch (Exception e) { if (!(e.getCause() instanceof AEADBadTagException)) { throw e; // not a key mismatch — propagate (e.g. DB connection failure) @@ -95,8 +95,8 @@ public void run(ApplicationArguments args) { name, e); try { - storeProvider.delete(name); - storeProvider.set(name, value); + secretsDAO.deleteSecret(name); + secretsDAO.putSecret(name, value); created++; } catch (Exception re) { log.warn("Credential '{}' could not be re-seeded — skipping", name, re); @@ -114,7 +114,7 @@ public void run(ApplicationArguments args) { } try { - storeProvider.set(name, value); + secretsDAO.putSecret(name, value); log.info("Credential seeded from environment: {}", name); created++; } catch (Exception e) { diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProvider.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProvider.java deleted file mode 100644 index 43ce78a3..00000000 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProvider.java +++ /dev/null @@ -1,188 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.credentials; - -import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; -import java.security.SecureRandom; -import java.time.Instant; -import java.util.List; -import java.util.Map; - -import javax.crypto.Cipher; -import javax.crypto.spec.GCMParameterSpec; -import javax.crypto.spec.SecretKeySpec; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; -import org.springframework.stereotype.Component; - -import dev.agentspan.runtime.model.credentials.CredentialMeta; -import dev.agentspan.runtime.spi.CredentialStoreProvider; - -/** - * AES-256-GCM encrypted credential store backed by the credential SQLite/Postgres DB. - * - *

Encryption format: [12-byte IV][ciphertext+16-byte GCM tag] - * All concatenated into a single BLOB stored in credentials_store.encrypted_value.

- * - *

The master key is the 32-byte key from {@code MasterKeyConfig#credentialMasterKey()}.

- */ -@Component -@ConditionalOnProperty(name = "conductor.secrets.type", havingValue = "agentspan") -public class EncryptedDbCredentialStoreProvider implements CredentialStoreProvider { - - private static final Logger log = LoggerFactory.getLogger(EncryptedDbCredentialStoreProvider.class); - private static final String ALGORITHM = "AES/GCM/NoPadding"; - private static final int IV_LENGTH = 12; // GCM standard nonce - private static final int TAG_LENGTH = 128; // GCM auth tag bits - private static final SecureRandom SECURE_RANDOM = new SecureRandom(); - - /** - * Fixed single-scope owner for all rows. The {@code credentials_store} schema keeps its - * {@code user_id} column (part of the (user_id, name) primary key), but the store is global — - * every row is written and read under this constant, so lookups are effectively by name. - */ - private static final String DEFAULT_USER_ID = "00000000-0000-0000-0000-000000000000"; - - private final NamedParameterJdbcTemplate jdbc; - private final byte[] masterKey; - - public EncryptedDbCredentialStoreProvider( - @Qualifier("credentialJdbc") NamedParameterJdbcTemplate jdbc, - @Qualifier("credentialMasterKey") byte[] masterKey) { - this.jdbc = jdbc; - this.masterKey = masterKey; - } - - @Override - public String get(String name) { - try { - byte[] encrypted = jdbc.queryForObject( - "SELECT encrypted_value FROM credentials_store " + "WHERE user_id = :uid AND name = :n", - Map.of("uid", DEFAULT_USER_ID, "n", name), - byte[].class); - if (encrypted == null) return null; - return decrypt(encrypted); - } catch (EmptyResultDataAccessException e) { - return null; - } catch (Exception e) { - log.error("Failed to decrypt credential '{}': {}", name, e.getMessage()); - throw new IllegalStateException("Failed to decrypt credential: " + name, e); - } - } - - @Override - public void set(String name, String value) { - try { - byte[] encrypted = encrypt(value); - String now = Instant.now().toString(); - // Single-statement upsert. ON CONFLICT(...) DO UPDATE is supported - // by SQLite 3.24+ and Postgres 9.5+; atomic on both. Replaces an - // earlier UPDATE-then-INSERT pattern that raced on concurrent - // first-write to the same (user_id, name). - jdbc.update( - "INSERT INTO credentials_store (user_id, name, encrypted_value, created_at, updated_at) " - + "VALUES (:uid, :n, :enc, :now, :now) " - + "ON CONFLICT(user_id, name) DO UPDATE SET " - + " encrypted_value = excluded.encrypted_value, " - + " updated_at = excluded.updated_at", - Map.of("uid", DEFAULT_USER_ID, "n", name, "enc", encrypted, "now", now)); - } catch (Exception e) { - throw new IllegalStateException("Failed to store credential: " + name, e); - } - } - - @Override - public void delete(String name) { - jdbc.update( - "DELETE FROM credentials_store WHERE user_id = :uid AND name = :n", - Map.of("uid", DEFAULT_USER_ID, "n", name)); - } - - @Override - public List list() { - // Include encrypted_value in the SELECT so we can decrypt inline — avoids - // an N+1 query pattern. On SQLite the pool is capped at 1 connection so a - // nested get() call inside a RowMapper would deadlock; on PostgreSQL the - // single-query approach remains more efficient regardless. - return jdbc.query( - "SELECT name, encrypted_value, created_at, updated_at " - + "FROM credentials_store WHERE user_id = :uid ORDER BY name", - Map.of("uid", DEFAULT_USER_ID), - (rs, row) -> { - String name = rs.getString("name"); - byte[] enc = rs.getBytes("encrypted_value"); - String partial; - try { - partial = toPartial(enc != null ? decrypt(enc) : null); - } catch (Exception e) { - partial = "????...????"; - } - return CredentialMeta.builder() - .name(name) - .partial(partial) - .createdAt(parseInstant(rs.getString("created_at"))) - .updatedAt(parseInstant(rs.getString("updated_at"))) - .build(); - }); - } - - // ── Encryption ──────────────────────────────────────────────────── - - private byte[] encrypt(String plaintext) throws Exception { - byte[] iv = new byte[IV_LENGTH]; - SECURE_RANDOM.nextBytes(iv); - - SecretKeySpec keySpec = new SecretKeySpec(masterKey, "AES"); - Cipher cipher = Cipher.getInstance(ALGORITHM); - cipher.init(Cipher.ENCRYPT_MODE, keySpec, new GCMParameterSpec(TAG_LENGTH, iv)); - byte[] ciphertext = cipher.doFinal(plaintext.getBytes(StandardCharsets.UTF_8)); - - // Format: [IV 12 bytes][ciphertext+tag] - ByteBuffer buf = ByteBuffer.allocate(IV_LENGTH + ciphertext.length); - buf.put(iv); - buf.put(ciphertext); - return buf.array(); - } - - private String decrypt(byte[] data) throws Exception { - ByteBuffer buf = ByteBuffer.wrap(data); - byte[] iv = new byte[IV_LENGTH]; - buf.get(iv); - byte[] ciphertext = new byte[buf.remaining()]; - buf.get(ciphertext); - - SecretKeySpec keySpec = new SecretKeySpec(masterKey, "AES"); - Cipher cipher = Cipher.getInstance(ALGORITHM); - cipher.init(Cipher.DECRYPT_MODE, keySpec, new GCMParameterSpec(TAG_LENGTH, iv)); - byte[] plaintext = cipher.doFinal(ciphertext); - return new String(plaintext, StandardCharsets.UTF_8); - } - - // ── Helpers ─────────────────────────────────────────────────────── - - /** - * Return first 4 + "..." + last 4 characters. - * Consistent with OpenAI, GitHub, AWS key display conventions. - */ - static String toPartial(String value) { - if (value == null || value.length() < 8) return "****...****"; - return value.substring(0, 4) + "..." + value.substring(value.length() - 4); - } - - private Instant parseInstant(String s) { - if (s == null) return null; - try { - return Instant.parse(s); - } catch (Exception e) { - return null; - } - } -} diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/MasterKeyConfig.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/MasterKeyConfig.java index ff1f7a3a..b01a2688 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/MasterKeyConfig.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/MasterKeyConfig.java @@ -20,7 +20,7 @@ import org.springframework.context.annotation.Configuration; /** - * Loads or generates the AES-256-GCM master key used by EncryptedDbCredentialStoreProvider. + * Loads or generates the AES-256-GCM master key used by AgentspanSecretsDAO. * *

Key sourcing rules:

*
    diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/NoOpSecretOutputMasker.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/NoOpSecretOutputMasker.java deleted file mode 100644 index e225df95..00000000 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/credentials/NoOpSecretOutputMasker.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.credentials; - -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.stereotype.Service; - -import dev.agentspan.runtime.spi.SecretOutputMasker; - -/** - * No-op {@link SecretOutputMasker} — the OSS / standalone default. - * - *

    OSS has no per-execution disclosure tracking ({@code credential_disclosures}), - * so there is nothing to redact against: {@link #mask} returns the payload unchanged. - * - *

    An embedding host (e.g. orkes-conductor) supplies a real implementation that - * queries the disclosure log, fetches the current plaintext values from the secret - * store, and redacts them from the response body via a Jackson-tree walk (so values - * containing newlines, quotes, or other JSON-escaped characters are still caught). - */ -@Service -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) -public class NoOpSecretOutputMasker implements SecretOutputMasker { - - @Override - public String mask(String executionId, String userId, String payload) { - return payload; - } -} diff --git a/server/conductor-agentspan-server/src/main/resources/application.properties b/server/conductor-agentspan-server/src/main/resources/application.properties index 9d3a3b3a..48497be1 100644 --- a/server/conductor-agentspan-server/src/main/resources/application.properties +++ b/server/conductor-agentspan-server/src/main/resources/application.properties @@ -39,6 +39,9 @@ conductor.workflow-message-queue.maxBatchSize=100 conductor.task-status-listener.type=agent conductor.workflow-status-listener.type=agent +conductor.app.taskWorkerConfigs.HTTP.threadCount=20 +conductor.app.taskWorkerConfigs.LLM_CHAT_COMPLETE.threadCount=20 + # Relaxed validation conductor.app.workflow.name-validation.enabled=false conductor.app.ownerEmailMandatory=false @@ -159,10 +162,8 @@ agentspan.skills.max-file-count=${AGENTSPAN_SKILLS_MAX_FILE_COUNT:2000} # host (orkes-conductor / conductor-oss): those beans are DORMANT and the host # delivers secrets — worker tools via TaskDef.runtimeMetadata, system tasks via # ${workflow.secrets.NAME}. -agentspan.embedded=false +agentspan.embedded=true agentspan.credentials.store=built-in -agentspan.credentials.strict-mode=false -agentspan.credentials.resolve.rate-limit=120 # Secret backend for the embedded conductor (RuntimeMetadataResolver at task poll, and # ${workflow.secrets.NAME} substitution). 'agentspan' backs it with AgentSpan's encrypted @@ -174,11 +175,6 @@ agentspan.credentials.resolve.rate-limit=120 # (the native credential services require the AgentSpan store, so do not override it standalone). conductor.secrets.type=${CONDUCTOR_SECRETS_TYPE:agentspan} -# Mask secrets from the host-owned /api/workflow/{id} (raw Conductor) read path too. -# Off by default so embedding this library never mutates a host's workflow responses; -# AgentSpan's own /api/agent/* reads are always masked regardless of this flag. -# agentspan.credentials.mask-workflow-reads=false - # AGENTSPAN_MASTER_KEY: base64-encoded 256-bit key for AES-256-GCM. # Unset + localhost → auto-generated and warned. # Unset + non-localhost → server refuses to start. diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingIntegrationTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingIntegrationTest.java deleted file mode 100644 index 92d72aed..00000000 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingIntegrationTest.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.controller; - -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.when; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; - -import java.util.Map; - -import org.hamcrest.Matchers; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.web.servlet.MockMvc; - -import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.model.AgentExecutionDetail; -import dev.agentspan.runtime.service.AgentService; -import dev.agentspan.runtime.spi.CredentialStoreProvider; - -/** - * Verifies that {@link CredentialMaskingResponseAdvice} activates on the right - * URI patterns and leaves the payload unchanged in OSS (no-op masker — disclosure - * tracking is an enterprise feature). - * - *

    Masking-correctness tests (redaction, tree-walk, JSON-escape handling) live - * in the enterprise module where the real {@link dev.agentspan.runtime.credentials.NoOpSecretOutputMasker} - * implementation is provided.

    - */ -@SpringBootTest(classes = AgentRuntime.class) -@AutoConfigureMockMvc -@ActiveProfiles("test") -class CredentialMaskingIntegrationTest { - - @Autowired - private MockMvc mvc; - - @Autowired - private CredentialStoreProvider store; - - @MockBean - private AgentService agentService; - - private static final String CRED_NAME = "_MASK_E2E_TOKEN"; - private static final String CRED_VALUE = "ghp_thisisasecretthatshouldbemasked"; - private static final String EXEC_ID = "exec-mask-e2e-001"; - private static final String userId = "00000000-0000-0000-0000-000000000000"; - - @BeforeEach - void setUp() { - store.set(CRED_NAME, CRED_VALUE); - } - - @AfterEach - void cleanUp() { - store.delete(CRED_NAME); - } - - // ── Advice URI coverage ───────────────────────────────────────────── - - @Test - void executionDetail_adviceActivates_passesThrough() throws Exception { - // In OSS the masker is a no-op: value passes through unchanged. - // This test confirms the advice activates on /api/agent/executions/{id} - // without throwing and without blocking the response. - AgentExecutionDetail detail = AgentExecutionDetail.builder() - .executionId(EXEC_ID) - .agentName("test-agent") - .status("COMPLETED") - .output(Map.of("result", "token is " + CRED_VALUE)) - .build(); - when(agentService.getExecutionDetail(eq(EXEC_ID))).thenReturn(detail); - - mvc.perform(get("/api/agent/executions/" + EXEC_ID)) - .andExpect(status().isOk()) - // OSS no-op: value is still present (masking is enterprise) - .andExpect(content().string(Matchers.containsString(CRED_VALUE))); - } - - @Test - void statusEndpoint_adviceActivates_passesThrough() throws Exception { - when(agentService.getStatus(eq(EXEC_ID))) - .thenReturn(Map.of("executionId", EXEC_ID, "status", "COMPLETED", "note", "token=" + CRED_VALUE)); - - mvc.perform(get("/api/agent/" + EXEC_ID + "/status")) - .andExpect(status().isOk()) - .andExpect(content().string(Matchers.containsString(CRED_VALUE))); - } - - @Test - void getCredential_endpointNotIntercepted() throws Exception { - // /api/secrets/{key} is the CRUD endpoint — the advice must NOT - // intercept it regardless of what credentials are in the store. - mvc.perform(get("/api/secrets/" + CRED_NAME)) - .andExpect(status().isOk()) - .andExpect(content().string(CRED_VALUE)); - } -} diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingWorkflowOptInTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingWorkflowOptInTest.java deleted file mode 100644 index 4f280a89..00000000 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/CredentialMaskingWorkflowOptInTest.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.controller; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.net.URI; -import java.time.Instant; -import java.util.Map; -import java.util.UUID; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; -import org.springframework.http.MediaType; -import org.springframework.http.server.ServerHttpRequest; -import org.springframework.http.server.ServerHttpResponse; - -import com.fasterxml.jackson.databind.ObjectMapper; - -import dev.agentspan.runtime.context.RequestContext; -import dev.agentspan.runtime.context.RequestContextHolder; -import dev.agentspan.runtime.spi.SecretOutputMasker; - -/** - * Verifies that {@link CredentialMaskingResponseAdvice} only intercepts the host-owned - * {@code /api/workflow/{id}} endpoint when explicitly opted in - * ({@code agentspan.credentials.mask-workflow-reads=true}), while AgentSpan's own - * {@code /api/agent/*} reads are always masked. This guards against the library mutating - * an embedding host's raw Conductor responses just by being on the classpath. - */ -class CredentialMaskingWorkflowOptInTest { - - private static final String USER_ID = "00000000-0000-0000-0000-000000000000"; - - private final SecretOutputMasker masker = mock(SecretOutputMasker.class); - private final ObjectMapper mapper = new ObjectMapper(); - - @AfterEach - void clearContext() { - RequestContextHolder.clear(); - } - - private CredentialMaskingResponseAdvice advice(boolean maskWorkflowReads) { - // Masker echoes the payload back (no-op) so the advice's return value is the parsed body. - when(masker.mask(any(), any(), any())).thenAnswer(inv -> inv.getArgument(2)); - return new CredentialMaskingResponseAdvice(masker, mapper, maskWorkflowReads); - } - - private void setUser() { - RequestContextHolder.set(RequestContext.builder() - .requestId(UUID.randomUUID().toString()) - .userId(USER_ID) - .createdAt(Instant.now()) - .build()); - } - - private Object invoke(CredentialMaskingResponseAdvice advice, String path, Object body) { - ServerHttpRequest request = mock(ServerHttpRequest.class); - when(request.getURI()).thenReturn(URI.create("http://localhost:6767" + path)); - ServerHttpResponse response = mock(ServerHttpResponse.class); - return advice.beforeBodyWrite(body, null, MediaType.APPLICATION_JSON, null, request, response); - } - - @Test - void workflowRead_notMasked_whenOptInDisabled() { - setUser(); - Object body = Map.of("workflowId", "wf-1", "output", "secret-value"); - - Object result = invoke(advice(false), "/api/workflow/wf-1", body); - - // Host endpoint must be left completely untouched — same instance, masker never consulted. - assertThat(result).isSameAs(body); - verify(masker, never()).mask(any(), any(), any()); - } - - @Test - void workflowRead_masked_whenOptInEnabled() { - setUser(); - Object body = Map.of("workflowId", "wf-1", "output", "secret-value"); - - invoke(advice(true), "/api/workflow/wf-1", body); - - verify(masker).mask(eq("wf-1"), eq(USER_ID), any()); - } - - @Test - void agentExecutionRead_alwaysMasked_evenWhenWorkflowOptInDisabled() { - setUser(); - Object body = Map.of("executionId", "exec-1", "output", "secret-value"); - - invoke(advice(false), "/api/agent/executions/exec-1", body); - - // AgentSpan owns /api/agent/* — masking there is unaffected by the workflow opt-in. - verify(masker).mask(eq("exec-1"), eq(USER_ID), any()); - } -} diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/ProviderStatusEndpointTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/ProviderStatusEndpointTest.java index f5d5d61e..e4f0dd15 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/ProviderStatusEndpointTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/controller/ProviderStatusEndpointTest.java @@ -22,9 +22,9 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.conductor.dao.SecretsDAO; import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * GET /api/providers/status — the server is the source of truth for provider @@ -46,20 +46,20 @@ class ProviderStatusEndpointTest { private int port; @Autowired - private CredentialStoreProvider store; + private SecretsDAO store; private String savedOllamaUrl; @BeforeEach void setUp() { - savedOllamaUrl = store.get("OLLAMA_BASE_URL"); - store.set("OLLAMA_BASE_URL", UNREACHABLE_URL); + savedOllamaUrl = store.getSecret("OLLAMA_BASE_URL"); + store.putSecret("OLLAMA_BASE_URL", UNREACHABLE_URL); } @AfterEach void cleanUp() { - store.delete("OLLAMA_BASE_URL"); - if (savedOllamaUrl != null) store.set("OLLAMA_BASE_URL", savedOllamaUrl); + store.deleteSecret("OLLAMA_BASE_URL"); + if (savedOllamaUrl != null) store.putSecret("OLLAMA_BASE_URL", savedOllamaUrl); } private JsonNode getStatus() throws Exception { diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAOTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAOTest.java index 33f7c59b..78d1902a 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAOTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/AgentspanSecretsDAOTest.java @@ -7,74 +7,118 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.test.context.ActiveProfiles; +import dev.agentspan.runtime.AgentRuntime; import dev.agentspan.runtime.model.credentials.CredentialMeta; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** - * {@link AgentspanSecretsDAO} bridges conductor's global {@code SecretsDAO} to AgentSpan's - * (single-scope) {@link CredentialStoreProvider}. Verifies the name→value round-trip and that the - * bean is selected only by {@code conductor.secrets.type=agentspan}. + * {@link AgentspanSecretsDAO} is conductor's global {@code SecretsDAO} backed directly by the + * AES-256-GCM encrypted {@code credentials_store} table — the single storage backend for + * conductor's own secret substitution AND AgentSpan's own credential surfaces (Credentials UI, + * resolution service, env seeder). Verifies the encrypted round-trip against a real DB and that + * the bean is selected only by {@code conductor.secrets.type=agentspan}. */ +@SpringBootTest(classes = AgentRuntime.class, webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") class AgentspanSecretsDAOTest { - /** In-memory {@link CredentialStoreProvider} keyed by name (the store is global — no userId). */ - static class FakeStore implements CredentialStoreProvider { - final Map data = new LinkedHashMap<>(); - - @Override - public String get(String name) { - return data.get(name); - } - - @Override - public void set(String name, String value) { - data.put(name, value); - } - - @Override - public void delete(String name) { - data.remove(name); - } - - @Override - public List list() { - List out = new ArrayList<>(); - for (String name : data.keySet()) { - out.add(CredentialMeta.builder().name(name).build()); - } - return out; - } + @Autowired + private AgentspanSecretsDAO dao; + + @Autowired + @Qualifier("credentialJdbc") + private NamedParameterJdbcTemplate jdbc; + + private static final String USER_ID = "00000000-0000-0000-0000-000000000000"; + + @BeforeEach + void setUp() { + jdbc.update("DELETE FROM credentials_store WHERE user_id = :uid", Map.of("uid", USER_ID)); } @Test - void roundTrip() { - FakeStore store = new FakeStore(); - AgentspanSecretsDAO dao = new AgentspanSecretsDAO(store); + void putSecret_andGetSecret_roundTripsEncryptedValue() { + dao.putSecret("GITHUB_TOKEN", "ghp_supersecret"); + assertThat(dao.getSecret("GITHUB_TOKEN")).isEqualTo("ghp_supersecret"); + } - assertThat(dao.secretExists("GITHUB_TOKEN")).isFalse(); - assertThat(dao.getSecret("GITHUB_TOKEN")).isNull(); + @Test + void getSecret_returnsNull_whenNotFound() { + assertThat(dao.getSecret("DOES_NOT_EXIST")).isNull(); + } + @Test + void secretExists_reflectsPresence() { + assertThat(dao.secretExists("GITHUB_TOKEN")).isFalse(); dao.putSecret("GITHUB_TOKEN", "ghp_x"); - assertThat(store.data).containsEntry("GITHUB_TOKEN", "ghp_x"); - assertThat(dao.getSecret("GITHUB_TOKEN")).isEqualTo("ghp_x"); assertThat(dao.secretExists("GITHUB_TOKEN")).isTrue(); + } + @Test + void deleteSecret_removesCredential() { + dao.putSecret("TO_DELETE", "value"); + dao.deleteSecret("TO_DELETE"); + assertThat(dao.getSecret("TO_DELETE")).isNull(); + } + + @Test + void listSecretNames_returnsAllNames() { + dao.putSecret("GITHUB_TOKEN", "ghp_x"); dao.putSecret("SLACK_TOKEN", "xoxb"); + assertThat(dao.listSecretNames()).containsExactlyInAnyOrder("GITHUB_TOKEN", "SLACK_TOKEN"); + } + + @Test + void listWithMeta_returnsPartialValues_notPlaintext() { + dao.putSecret("OPENAI_KEY", "sk-abcdefghijklmnop"); + + List list = dao.listWithMeta(); + + CredentialMeta meta = list.stream() + .filter(m -> m.getName().equals("OPENAI_KEY")) + .findFirst() + .orElseThrow(); + + // Partial: first 4 + ... + last 4 + assertThat(meta.getPartial()).isEqualTo("sk-a...mnop"); + assertThat(meta.getUpdatedAt()).isNotNull(); + // Plaintext is NOT in the list response + assertThat(meta.toString()).doesNotContain("abcdefghijklmnop"); + } + + @Test + void putSecret_updatesExistingCredential() { + dao.putSecret("MY_KEY", "original"); + dao.putSecret("MY_KEY", "updated"); + assertThat(dao.getSecret("MY_KEY")).isEqualTo("updated"); + } + + @Test + void encryptedValueInDb_isNotPlaintext() { + dao.putSecret("SECRET", "plaintext_value"); + + // Read raw bytes from DB + byte[] raw = jdbc.queryForObject( + "SELECT encrypted_value FROM credentials_store WHERE user_id=:uid AND name=:n", + Map.of("uid", USER_ID, "n", "SECRET"), + byte[].class); - dao.deleteSecret("GITHUB_TOKEN"); - assertThat(dao.getSecret("GITHUB_TOKEN")).isNull(); - assertThat(dao.listSecretNames()).containsExactly("SLACK_TOKEN"); + assertThat(raw).isNotNull(); + assertThat(new String(raw)).doesNotContain("plaintext_value"); } // ── gating: selected only by conductor.secrets.type=agentspan ── @@ -83,20 +127,21 @@ void roundTrip() { @Import(AgentspanSecretsDAO.class) static class DaoConfig {} - private final ApplicationContextRunner runner = new ApplicationContextRunner() - .withBean(CredentialStoreProvider.class, () -> mock(CredentialStoreProvider.class)) + private final ApplicationContextRunner gatingRunner = new ApplicationContextRunner() + .withBean("credentialJdbc", NamedParameterJdbcTemplate.class, () -> mock(NamedParameterJdbcTemplate.class)) + .withBean("credentialMasterKey", byte[].class, () -> new byte[32]) .withUserConfiguration(DaoConfig.class); @Test void beanPresent_whenConductorSecretsTypeAgentspan() { - runner.withPropertyValues("conductor.secrets.type=agentspan") - .run(ctx -> assertThat(ctx).hasSingleBean(AgentspanSecretsDAO.class)); + gatingRunner.withPropertyValues("conductor.secrets.type=agentspan").run(ctx -> assertThat(ctx) + .hasSingleBean(AgentspanSecretsDAO.class)); } @Test void beanAbsent_whenFlagUnsetOrDifferent() { - runner.run(ctx -> assertThat(ctx).doesNotHaveBean(AgentspanSecretsDAO.class)); - runner.withPropertyValues("conductor.secrets.type=env") - .run(ctx -> assertThat(ctx).doesNotHaveBean(AgentspanSecretsDAO.class)); + gatingRunner.run(ctx -> assertThat(ctx).doesNotHaveBean(AgentspanSecretsDAO.class)); + gatingRunner.withPropertyValues("conductor.secrets.type=env").run(ctx -> assertThat(ctx) + .doesNotHaveBean(AgentspanSecretsDAO.class)); } } diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/ConcurrentPutRaceTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/ConcurrentPutRaceTest.java index eac75d02..da8df679 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/ConcurrentPutRaceTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/ConcurrentPutRaceTest.java @@ -16,8 +16,9 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; +import com.netflix.conductor.dao.SecretsDAO; + import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * Regression test for Bug #3 — concurrent PUT on the same (user, name) was @@ -39,19 +40,19 @@ class ConcurrentPutRaceTest { @Autowired - private CredentialStoreProvider store; + private SecretsDAO store; private static final String USER = "race-test-user"; private static final String NAME = "_RACE_TEST_KEY"; @BeforeEach void clean() { - store.delete(NAME); + store.deleteSecret(NAME); } @AfterEach void clean2() { - store.delete(NAME); + store.deleteSecret(NAME); } @Test @@ -63,7 +64,7 @@ void concurrentPut_newCredential_doesNotThrow() throws Exception { final int idx = i; futures[i] = CompletableFuture.runAsync(() -> { try { - store.set(NAME, "value-" + idx); + store.putSecret(NAME, "value-" + idx); } catch (Throwable t) { errors.incrementAndGet(); } @@ -77,12 +78,12 @@ void concurrentPut_newCredential_doesNotThrow() throws Exception { .isZero(); // And the value is set (last writer wins; we just assert SOME write succeeded). - assertThat(store.get(NAME)).isNotNull(); + assertThat(store.getSecret(NAME)).isNotNull(); } @Test void concurrentPut_existingCredential_doesNotThrow() throws Exception { - store.set(NAME, "initial-value"); + store.putSecret(NAME, "initial-value"); int N = 50; AtomicInteger errors = new AtomicInteger(); @@ -91,7 +92,7 @@ void concurrentPut_existingCredential_doesNotThrow() throws Exception { final int idx = i; futures[i] = CompletableFuture.runAsync(() -> { try { - store.set(NAME, "updated-value-" + idx); + store.putSecret(NAME, "updated-value-" + idx); } catch (Throwable t) { errors.incrementAndGet(); } @@ -100,6 +101,6 @@ void concurrentPut_existingCredential_doesNotThrow() throws Exception { for (var f : futures) f.get(); assertThat(errors.get()).isZero(); - assertThat(store.get(NAME)).startsWith("updated-value-"); + assertThat(store.getSecret(NAME)).startsWith("updated-value-"); } } diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederIntegrationTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederIntegrationTest.java index b1c30d51..a5cce897 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederIntegrationTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederIntegrationTest.java @@ -11,8 +11,9 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.ActiveProfiles; +import com.netflix.conductor.dao.SecretsDAO; + import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * Integration test that verifies CredentialEnvSeeder can store credentials @@ -26,18 +27,18 @@ class CredentialEnvSeederIntegrationTest { @Autowired - private CredentialStoreProvider storeProvider; + private SecretsDAO storeProvider; @Test void seeder_storesCredential_withoutForeignKeyError() { // The seeder runs at startup. If ANTHROPIC_API_KEY is in the env // (or test properties), the credential should exist. // At minimum, verify we can write and read back without errors. - storeProvider.set("INTEGRATION_TEST_KEY", "test-value-123"); - String value = storeProvider.get("INTEGRATION_TEST_KEY"); + storeProvider.putSecret("INTEGRATION_TEST_KEY", "test-value-123"); + String value = storeProvider.getSecret("INTEGRATION_TEST_KEY"); assertThat(value).isEqualTo("test-value-123"); // Cleanup - storeProvider.delete("INTEGRATION_TEST_KEY"); + storeProvider.deleteSecret("INTEGRATION_TEST_KEY"); } } diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederTest.java index 231de59f..17e6e6e3 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialEnvSeederTest.java @@ -17,8 +17,9 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.test.context.ActiveProfiles; +import com.netflix.conductor.dao.SecretsDAO; + import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * Integration test for CredentialEnvSeeder — uses real DB, no mocks. @@ -29,7 +30,7 @@ class CredentialEnvSeederTest { @Autowired - private CredentialStoreProvider storeProvider; + private SecretsDAO storeProvider; @Autowired @Qualifier("credentialJdbc") @@ -48,10 +49,10 @@ void cleanUp() { } catch (Exception ignored) { // Table may not exist yet on the first test run — safe to ignore. } - storeProvider.delete("GH_TOKEN"); - storeProvider.delete("GITHUB_TOKEN"); - storeProvider.delete("OPENAI_BASE_URL"); - storeProvider.delete("ANTHROPIC_BASE_URL"); + storeProvider.deleteSecret("GH_TOKEN"); + storeProvider.deleteSecret("GITHUB_TOKEN"); + storeProvider.deleteSecret("OPENAI_BASE_URL"); + storeProvider.deleteSecret("ANTHROPIC_BASE_URL"); } @Test @@ -74,19 +75,19 @@ void seeder_storesCredentialFromEnv_inRealDb() throws Exception { field.set(realSeeder, "built-in"); // Delete existing credential first so seeder can create it - storeProvider.delete("ANTHROPIC_API_KEY"); + storeProvider.deleteSecret("ANTHROPIC_API_KEY"); realSeeder.run(new org.springframework.boot.DefaultApplicationArguments()); // Verify credential was stored in real DB - String value = storeProvider.get("ANTHROPIC_API_KEY"); + String value = storeProvider.getSecret("ANTHROPIC_API_KEY"); assertThat(value).isEqualTo("sk-test-seeded-value"); } @Test void seeder_skipsExistingCredential_inRealDb() throws Exception { // Store a credential first - storeProvider.set("ANTHROPIC_API_KEY", "original-value"); + storeProvider.putSecret("ANTHROPIC_API_KEY", "original-value"); // Try to seed with a different value Function envLookup = @@ -100,14 +101,14 @@ void seeder_skipsExistingCredential_inRealDb() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); // Value should still be the original - String value = storeProvider.get("ANTHROPIC_API_KEY"); + String value = storeProvider.getSecret("ANTHROPIC_API_KEY"); assertThat(value).isEqualTo("original-value"); } @Test void seeder_ignoresBlankEnvVars_inRealDb() throws Exception { // Delete so we can detect if seeder creates it - storeProvider.delete("ANTHROPIC_API_KEY"); + storeProvider.deleteSecret("ANTHROPIC_API_KEY"); Function envLookup = name -> "ANTHROPIC_API_KEY".equals(name) ? " " : null; @@ -119,13 +120,13 @@ void seeder_ignoresBlankEnvVars_inRealDb() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); // Blank value should NOT be stored - String value = storeProvider.get("ANTHROPIC_API_KEY"); + String value = storeProvider.getSecret("ANTHROPIC_API_KEY"); assertThat(value).isNull(); } @Test void seeder_skipsWhenStoreIsNotBuiltIn() throws Exception { - storeProvider.delete("ANTHROPIC_API_KEY"); + storeProvider.deleteSecret("ANTHROPIC_API_KEY"); Function envLookup = name -> "ANTHROPIC_API_KEY".equals(name) ? "sk-should-not-store" : null; @@ -136,7 +137,7 @@ void seeder_skipsWhenStoreIsNotBuiltIn() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); - String value = storeProvider.get("ANTHROPIC_API_KEY"); + String value = storeProvider.getSecret("ANTHROPIC_API_KEY"); assertThat(value).isNull(); } @@ -144,7 +145,7 @@ void seeder_skipsWhenStoreIsNotBuiltIn() throws Exception { void seeder_reseeds_whenDecryptionFailsDueToKeyMismatch() throws Exception { // Simulate a credential encrypted with an old/rotated master key by writing // garbage bytes directly into the DB — decryption will throw AEADBadTagException. - storeProvider.delete("ANTHROPIC_API_KEY"); + storeProvider.deleteSecret("ANTHROPIC_API_KEY"); String now = java.time.Instant.now().toString(); // 12-byte fake IV + 17 bytes of garbage ciphertext → GCM tag mismatch on decrypt byte[] staleBytes = new byte[29]; @@ -175,7 +176,7 @@ void seeder_reseeds_whenDecryptionFailsDueToKeyMismatch() throws Exception { .doesNotThrowAnyException(); // Credential must be re-encrypted with the current key and readable - String value = storeProvider.get("ANTHROPIC_API_KEY"); + String value = storeProvider.getSecret("ANTHROPIC_API_KEY"); assertThat(value).isEqualTo("sk-fresh-after-rotation"); } @@ -183,22 +184,27 @@ void seeder_reseeds_whenDecryptionFailsDueToKeyMismatch() throws Exception { void seeder_propagates_nonDecryptionExceptions() throws Exception { // A non-AEADBadTagException from get() must propagate — e.g. a transient DB failure // should NOT silently delete a valid credential. - CredentialStoreProvider failingStore = new CredentialStoreProvider() { + SecretsDAO failingStore = new SecretsDAO() { @Override - public String get(String name) { + public String getSecret(String name) { throw new IllegalStateException("DB connection lost", new RuntimeException("timeout")); } @Override - public void set(String name, String value) {} - - @Override - public void delete(String name) {} + public boolean secretExists(String name) { + return false; + } @Override - public java.util.List list() { + public java.util.List listSecretNames() { return java.util.List.of(); } + + @Override + public void putSecret(String name, String value) {} + + @Override + public void deleteSecret(String name) {} }; Function envLookup = name -> "ANTHROPIC_API_KEY".equals(name) ? "sk-value" : null; @@ -229,15 +235,15 @@ void seeder_storesBaseUrlVars_inRealDb() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); - assertThat(storeProvider.get("OPENAI_BASE_URL")).isEqualTo("https://my-proxy.org/v1"); - assertThat(storeProvider.get("ANTHROPIC_BASE_URL")).isEqualTo("https://anthropic-proxy.internal/v1"); + assertThat(storeProvider.getSecret("OPENAI_BASE_URL")).isEqualTo("https://my-proxy.org/v1"); + assertThat(storeProvider.getSecret("ANTHROPIC_BASE_URL")).isEqualTo("https://anthropic-proxy.internal/v1"); } @Test void seeder_seedsOllamaBaseUrl_inRealDb() throws Exception { // OLLAMA_BASE_URL is the documented Ollama variable and what the // provider resolves from the credential store — it must be seeded. - storeProvider.delete("OLLAMA_BASE_URL"); + storeProvider.deleteSecret("OLLAMA_BASE_URL"); Function envLookup = name -> "OLLAMA_BASE_URL".equals(name) ? "http://gpu-box:11434" : null; @@ -248,7 +254,7 @@ void seeder_seedsOllamaBaseUrl_inRealDb() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); - assertThat(storeProvider.get("OLLAMA_BASE_URL")).isEqualTo("http://gpu-box:11434"); + assertThat(storeProvider.getSecret("OLLAMA_BASE_URL")).isEqualTo("http://gpu-box:11434"); } @Test @@ -266,7 +272,7 @@ void seeder_storesGitHubCredentials_inRealDb() throws Exception { seeder.run(new org.springframework.boot.DefaultApplicationArguments()); - assertThat(storeProvider.get("GH_TOKEN")).isEqualTo("ghp-test-gh-token"); - assertThat(storeProvider.get("GITHUB_TOKEN")).isEqualTo("ghp-test-github-token"); + assertThat(storeProvider.getSecret("GH_TOKEN")).isEqualTo("ghp-test-gh-token"); + assertThat(storeProvider.getSecret("GITHUB_TOKEN")).isEqualTo("ghp-test-github-token"); } } diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialResolutionServiceTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialResolutionServiceTest.java index 2dd376de..3557ca48 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialResolutionServiceTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/CredentialResolutionServiceTest.java @@ -16,8 +16,9 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.test.context.ActiveProfiles; +import com.netflix.conductor.dao.SecretsDAO; + import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * Integration test for CredentialResolutionService — real DB, real services, no mocks. @@ -30,7 +31,7 @@ class CredentialResolutionServiceTest { private CredentialResolutionService service; @Autowired - private CredentialStoreProvider storeProvider; + private SecretsDAO storeProvider; @Autowired @Qualifier("credentialJdbc") @@ -45,7 +46,7 @@ void setUp() { @Test void resolve_directLookup_returnsStoredValue() { - storeProvider.set("GITHUB_TOKEN", "ghp_directlookup"); + storeProvider.putSecret("GITHUB_TOKEN", "ghp_directlookup"); String value = service.resolve("GITHUB_TOKEN"); @@ -70,10 +71,10 @@ void resolve_notInStore_noEnvFallback() { @Test void resolve_afterDelete_returnsNull() { - storeProvider.set("TEMP_KEY", "temp_value"); + storeProvider.putSecret("TEMP_KEY", "temp_value"); assertThat(service.resolve("TEMP_KEY")).isEqualTo("temp_value"); - storeProvider.delete("TEMP_KEY"); + storeProvider.deleteSecret("TEMP_KEY"); assertThat(service.resolve("TEMP_KEY")).isNull(); } @@ -81,7 +82,7 @@ void resolve_afterDelete_returnsNull() { @Test void resolve_dottedPath_extractsTopLevelField() { - storeProvider.set( + storeProvider.putSecret( "GCP_SVC", "{\"type\":\"service_account\",\"project_id\":\"my-proj-123\",\"client_email\":\"sa@x.iam\"}"); @@ -91,14 +92,14 @@ void resolve_dottedPath_extractsTopLevelField() { @Test void resolve_dottedPath_extractsNestedField() { - storeProvider.set("BLOB", "{\"auth\":{\"oauth\":{\"client_id\":\"abc123\"}}}"); + storeProvider.putSecret("BLOB", "{\"auth\":{\"oauth\":{\"client_id\":\"abc123\"}}}"); assertThat(service.resolve("BLOB.auth.oauth.client_id")).isEqualTo("abc123"); } @Test void resolve_dottedPath_missingField_returnsNull() { - storeProvider.set("JSONY", "{\"a\":\"1\",\"b\":\"2\"}"); + storeProvider.putSecret("JSONY", "{\"a\":\"1\",\"b\":\"2\"}"); assertThat(service.resolve("JSONY.does_not_exist")).isNull(); } @@ -111,7 +112,7 @@ void resolve_dottedPath_baseCredentialMissing_returnsNull() { @Test void resolve_dottedPath_nonJsonBase_returnsNull() { - storeProvider.set("FLAT_TOKEN", "not-a-json-value-just-text"); + storeProvider.putSecret("FLAT_TOKEN", "not-a-json-value-just-text"); assertThat(service.resolve("FLAT_TOKEN.field")).isNull(); } @@ -120,7 +121,7 @@ void resolve_dottedPath_nonJsonBase_returnsNull() { void resolve_dottedPath_nonStringLeaf_returnsJsonRepresentation() { // Number/boolean/object leaves serialize to their JSON form so HTTP/MCP // placeholders can substitute them cleanly. - storeProvider.set("CFG", "{\"port\":8080,\"enabled\":true,\"nested\":{\"a\":1}}"); + storeProvider.putSecret("CFG", "{\"port\":8080,\"enabled\":true,\"nested\":{\"a\":1}}"); assertThat(service.resolve("CFG.port")).isEqualTo("8080"); assertThat(service.resolve("CFG.enabled")).isEqualTo("true"); @@ -132,8 +133,8 @@ void resolve_dottedPath_nonStringLeaf_returnsJsonRepresentation() { void resolve_dottedPath_doesNotFallthroughToFullName() { // Even if a literal-dotted name happens to be stored, dotted resolution // ALWAYS treats the first segment as the base. Documented constraint. - storeProvider.set("LITERAL.NAME", "literally_dotted_value"); - storeProvider.set("LITERAL", "{\"NAME\":\"json_value\"}"); + storeProvider.putSecret("LITERAL.NAME", "literally_dotted_value"); + storeProvider.putSecret("LITERAL", "{\"NAME\":\"json_value\"}"); assertThat(service.resolve("LITERAL.NAME")).isEqualTo("json_value"); } diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProviderTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProviderTest.java deleted file mode 100644 index 736620f4..00000000 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/EncryptedDbCredentialStoreProviderTest.java +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.credentials; - -import static org.assertj.core.api.Assertions.*; - -import java.util.List; -import java.util.Map; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; -import org.springframework.test.context.ActiveProfiles; - -import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.model.credentials.CredentialMeta; -import dev.agentspan.runtime.spi.CredentialStoreProvider; - -@SpringBootTest(classes = AgentRuntime.class, webEnvironment = SpringBootTest.WebEnvironment.NONE) -@ActiveProfiles("test") -class EncryptedDbCredentialStoreProviderTest { - - @Autowired - private CredentialStoreProvider storeProvider; - - @Autowired - @Qualifier("credentialJdbc") - private NamedParameterJdbcTemplate jdbc; - - private static final String USER_ID = "00000000-0000-0000-0000-000000000000"; - - @BeforeEach - void setUp() { - jdbc.update("DELETE FROM credentials_store WHERE user_id = :uid", Map.of("uid", USER_ID)); - } - - @Test - void set_andGet_roundTripsEncryptedValue() { - storeProvider.set("GITHUB_TOKEN", "ghp_supersecret"); - String value = storeProvider.get("GITHUB_TOKEN"); - assertThat(value).isEqualTo("ghp_supersecret"); - } - - @Test - void get_returnsNull_whenNotFound() { - assertThat(storeProvider.get("DOES_NOT_EXIST")).isNull(); - } - - @Test - void delete_removesCredential() { - storeProvider.set("TO_DELETE", "value"); - storeProvider.delete("TO_DELETE"); - assertThat(storeProvider.get("TO_DELETE")).isNull(); - } - - @Test - void list_returnsPartialValues_notPlaintext() { - storeProvider.set("OPENAI_KEY", "sk-abcdefghijklmnop"); - - List list = storeProvider.list(); - - CredentialMeta meta = list.stream() - .filter(m -> m.getName().equals("OPENAI_KEY")) - .findFirst() - .orElseThrow(); - - // Partial: first 4 + ... + last 4 - assertThat(meta.getPartial()).isEqualTo("sk-a...mnop"); - assertThat(meta.getUpdatedAt()).isNotNull(); - // Plaintext is NOT in the list response - assertThat(meta.toString()).doesNotContain("abcdefghijklmnop"); - } - - @Test - void set_updatesExistingCredential() { - storeProvider.set("MY_KEY", "original"); - storeProvider.set("MY_KEY", "updated"); - assertThat(storeProvider.get("MY_KEY")).isEqualTo("updated"); - } - - @Test - void encryptedValueInDb_isNotPlaintext() { - storeProvider.set("SECRET", "plaintext_value"); - - // Read raw bytes from DB - byte[] raw = jdbc.queryForObject( - "SELECT encrypted_value FROM credentials_store WHERE user_id=:uid AND name=:n", - Map.of("uid", USER_ID, "n", "SECRET"), - byte[].class); - - assertThat(raw).isNotNull(); - assertThat(new String(raw)).doesNotContain("plaintext_value"); - } -} diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/SchemaMigratorUpgradePathTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/SchemaMigratorUpgradePathTest.java index dc6034da..62417ecd 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/SchemaMigratorUpgradePathTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/credentials/SchemaMigratorUpgradePathTest.java @@ -21,8 +21,9 @@ import org.springframework.test.context.ActiveProfiles; import org.springframework.test.web.servlet.MockMvc; +import com.netflix.conductor.dao.SecretsDAO; + import dev.agentspan.runtime.AgentRuntime; -import dev.agentspan.runtime.spi.CredentialStoreProvider; /** * Audit gap E — schema-migration upgrade path, full pipeline. @@ -54,7 +55,7 @@ class SchemaMigratorUpgradePathTest { private CredentialSchemaMigrator migrator; @Autowired - private CredentialStoreProvider store; + private SecretsDAO store; @Autowired @Qualifier("credentialJdbc") @@ -67,15 +68,15 @@ class SchemaMigratorUpgradePathTest { @BeforeEach void setUp() { - store.delete(STAGE_NAME); - store.delete(MIGRATED_NAME); + store.deleteSecret(STAGE_NAME); + store.deleteSecret(MIGRATED_NAME); jdbc.getJdbcOperations().execute("DROP TABLE IF EXISTS secrets_store"); } @AfterEach void cleanUp() { - store.delete(STAGE_NAME); - store.delete(MIGRATED_NAME); + store.deleteSecret(STAGE_NAME); + store.deleteSecret(MIGRATED_NAME); jdbc.getJdbcOperations().execute("DROP TABLE IF EXISTS secrets_store"); } @@ -83,7 +84,7 @@ void cleanUp() { void migratedRow_isReadableViaPublicApi() throws Exception { // 1. Encrypt a plaintext through the live store (credentials_store). // This guarantees the bytes are in the format the running server reads. - store.set(STAGE_NAME, PLAINTEXT); + store.putSecret(STAGE_NAME, PLAINTEXT); byte[] encryptedBytes = jdbc.queryForObject( "SELECT encrypted_value FROM credentials_store WHERE user_id = :u AND name = :n", Map.of("u", ANON, "n", STAGE_NAME), @@ -112,7 +113,7 @@ void migratedRow_isReadableViaPublicApi() throws Exception { encryptedBytes, "t", Instant.now().toString())); - store.delete(STAGE_NAME); // remove the staging row + store.deleteSecret(STAGE_NAME); // remove the staging row // Sanity: the migrated name is NOT yet visible via the public API. mvc.perform(get("/api/secrets/" + MIGRATED_NAME)).andExpect(status().isNotFound()); diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java index 3ce94ccd..f4d74994 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java @@ -18,7 +18,7 @@ import dev.agentspan.runtime.context.RequestContextHolder; import dev.agentspan.runtime.model.credentials.CredentialMeta; -import dev.agentspan.runtime.spi.CredentialStoreProvider; +import dev.agentspan.runtime.spi.CredentialsDAO; import lombok.RequiredArgsConstructor; @@ -42,7 +42,7 @@ @RestController @RequestMapping("/api/secrets") @RequiredArgsConstructor -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) +@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") public class SecretController { private static final Logger log = LoggerFactory.getLogger(SecretController.class); @@ -53,16 +53,14 @@ public class SecretController { static final int MAX_KEY_LENGTH = 65535; private static final Pattern KEY_REGEX = Pattern.compile(KEY_PATTERN); - private final CredentialStoreProvider storeProvider; + private final CredentialsDAO secretsDAO; // ── List ────────────────────────────────────────────────────────── /** POST /api/secrets — list all secret names (Conductor's primary listing endpoint). */ @PostMapping public ResponseEntity> listAllNames() { - List names = - storeProvider.list().stream().map(CredentialMeta::getName).toList(); - return ResponseEntity.ok(names); + return ResponseEntity.ok(secretsDAO.listSecretNames()); } /** @@ -72,9 +70,7 @@ public ResponseEntity> listAllNames() { */ @GetMapping public ResponseEntity> listGrantable() { - Set names = new LinkedHashSet<>( - storeProvider.list().stream().map(CredentialMeta::getName).toList()); - return ResponseEntity.ok(names); + return ResponseEntity.ok(new LinkedHashSet<>(secretsDAO.listSecretNames())); } /** @@ -83,7 +79,7 @@ public ResponseEntity> listGrantable() { */ @GetMapping("/v2") public ResponseEntity> listWithMeta() { - return ResponseEntity.ok(storeProvider.list()); + return ResponseEntity.ok(secretsDAO.listWithMeta()); } // ── Value CRUD ──────────────────────────────────────────────────── @@ -93,7 +89,7 @@ public ResponseEntity> listWithMeta() { public ResponseEntity getSecret(@PathVariable String key) { ResponseEntity err = validateKey(key); if (err != null) return ResponseEntity.status(err.getStatusCode()).build(); - String value = storeProvider.get(key); + String value = secretsDAO.getSecret(key); log.info("AUDIT get-secret: userId={} key={} found={}", currentUserId(), key, value != null); if (value == null) return ResponseEntity.notFound().build(); return ResponseEntity.ok(value); @@ -109,7 +105,7 @@ public ResponseEntity putSecret(@PathVariable String key, @RequestBody(requir if (value == null || value.isEmpty()) { return ResponseEntity.badRequest().body("value is required"); } - storeProvider.set(key, value); + secretsDAO.putSecret(key, value); log.info("AUDIT put-secret: userId={} key={}", currentUserId(), key); return ResponseEntity.ok().build(); } @@ -119,7 +115,7 @@ public ResponseEntity putSecret(@PathVariable String key, @RequestBody(requir public ResponseEntity deleteSecret(@PathVariable String key) { ResponseEntity err = validateKey(key); if (err != null) return err; - storeProvider.delete(key); + secretsDAO.deleteSecret(key); log.info("AUDIT delete-secret: userId={} key={}", currentUserId(), key); return ResponseEntity.ok().build(); } @@ -129,8 +125,7 @@ public ResponseEntity deleteSecret(@PathVariable String key) { public ResponseEntity exists(@PathVariable String key) { ResponseEntity err = validateKey(key); if (err != null) return ResponseEntity.badRequest().build(); - boolean present = storeProvider.get(key) != null; - return ResponseEntity.ok(present); + return ResponseEntity.ok(secretsDAO.secretExists(key)); } // ── Helpers ─────────────────────────────────────────────────────── diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/credentials/CredentialResolutionService.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/credentials/CredentialResolutionService.java index 5c984013..2a66fdc6 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/credentials/CredentialResolutionService.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/credentials/CredentialResolutionService.java @@ -11,8 +11,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; - -import dev.agentspan.runtime.spi.CredentialStoreProvider; +import com.netflix.conductor.dao.SecretsDAO; /** * Single authority for credential resolution across all call paths. @@ -40,16 +39,16 @@ * own {@code os.environ} fallback when {@code secret_strict_mode=false}.

    */ @Service -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) +@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") public class CredentialResolutionService { private static final Logger log = LoggerFactory.getLogger(CredentialResolutionService.class); - private final CredentialStoreProvider storeProvider; + private final SecretsDAO secretsDAO; private final ObjectMapper mapper = new ObjectMapper(); - public CredentialResolutionService(CredentialStoreProvider storeProvider) { - this.storeProvider = storeProvider; + public CredentialResolutionService(SecretsDAO secretsDAO) { + this.secretsDAO = secretsDAO; } /** @@ -61,14 +60,14 @@ public CredentialResolutionService(CredentialStoreProvider storeProvider) { public String resolve(String name) { int dot = name.indexOf('.'); if (dot < 0) { - String value = storeProvider.get(name); + String value = secretsDAO.getSecret(name); if (value == null) log.debug("Credential '{}' not found", name); return value; } String base = name.substring(0, dot); String path = name.substring(dot + 1); - String json = storeProvider.get(base); + String json = secretsDAO.getSecret(base); if (json == null) { log.debug("Base credential '{}' for path '{}' not found", base, path); return null; diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java index f30ec12b..b25bacb7 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java @@ -24,7 +24,7 @@ * leaves this property unset, so the override stays active as before.

    */ @Configuration -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) +@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") public class AgentHumanTaskConfig { @Bean(TASK_TYPE_HUMAN) diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialStoreProvider.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialStoreProvider.java deleted file mode 100644 index 9d8db7fe..00000000 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialStoreProvider.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.spi; - -import java.util.List; - -import dev.agentspan.runtime.model.credentials.CredentialMeta; - -/** - * Strategy interface for credential storage backends. - * - *

    The standalone server ships an encrypted-DB implementation; an embedding host - * (e.g. orkes-conductor) can supply AWS Secrets Manager, HashiCorp Vault, Azure KV, - * GCP SM, etc. All implementations plug into the same credential-resolution pipeline.

    - */ -public interface CredentialStoreProvider { - - /** - * Retrieve the plaintext value for a credential. - * Returns null if not found. - */ - String get(String name); - - /** - * Store or update a credential value (encrypted at rest by the implementation). - */ - void set(String name, String value); - - /** - * Delete a credential. No-op if not found. - */ - void delete(String name); - - /** - * List credential metadata for the store. - * Returns name + partial value + timestamps. Never returns plaintext values. - */ - List list(); -} diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialsDAO.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialsDAO.java new file mode 100644 index 00000000..b46dc8b5 --- /dev/null +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/CredentialsDAO.java @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2025 AgentSpan + * Licensed under the MIT License. + */ +package dev.agentspan.runtime.spi; + +import java.util.List; + +import com.netflix.conductor.dao.SecretsDAO; + +import dev.agentspan.runtime.model.credentials.CredentialMeta; + +/** + * AgentSpan's secret storage contract. Extends Conductor's {@link SecretsDAO} so every secret + * read/write — from the Credentials UI, credential resolution, and env seeding, as well as + * Conductor's own workflow-secret substitution — goes through one interface. + * + *

    Adds {@link #listWithMeta()}, the one capability {@code SecretsDAO} doesn't carry: metadata + * (partial display value + timestamps) for the Credentials UI. {@code SecretsDAO#listSecretNames} + * only returns names. + * + *

    The standalone server ships an encrypted-DB implementation ({@code AgentspanSecretsDAO}); an + * embedding host (e.g. orkes-conductor) can supply AWS Secrets Manager, HashiCorp Vault, Azure KV, + * GCP SM, etc. + */ +public interface CredentialsDAO extends SecretsDAO { + + /** + * List credential metadata for the store — name + partial display value + timestamps. + * Never returns plaintext values. + */ + List listWithMeta(); +} diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SecretOutputMasker.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SecretOutputMasker.java deleted file mode 100644 index 422ffdf1..00000000 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SecretOutputMasker.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.spi; - -/** - * Redacts secret values from execution-read responses before they leave the server. - * - *

    The standalone server ships a no-op implementation (OSS has no per-execution - * disclosure tracking, so there is nothing to redact against). An embedding host - * (e.g. orkes-conductor) supplies an implementation that looks up the secrets - * disclosed during an execution and removes their plaintext from the payload. - * - *

    Wired into responses by {@code CredentialMaskingResponseAdvice}. - */ -public interface SecretOutputMasker { - - /** - * Return {@code payload} with any secrets disclosed for {@code executionId} - * (scoped to {@code userId}) redacted. Implementations must be best-effort and - * must never throw — return the payload unchanged on any failure. - */ - String mask(String executionId, String userId, String payload); -} diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java index b1d8f92f..0f1935ff 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java @@ -25,7 +25,7 @@ import lombok.extern.slf4j.Slf4j; @Component(TASK_TYPE_JOIN) -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "false", matchIfMissing = true) +@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") @Slf4j public class Join extends WorkflowSystemTask { diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/ProviderValidator.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/ProviderValidator.java index 440362e9..c4a38c5f 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/ProviderValidator.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/ProviderValidator.java @@ -26,7 +26,7 @@ public class ProviderValidator { * When embedded in a host (e.g. orkes-conductor), Conductor is the authority for model * providers and credentials: conductor-ai integrations resolve providers by name, * and the host's credential store (AWS SSM / Vault / etc., reached via the - * {@code CredentialStoreProvider} SPI) supplies raw keys. This standalone pre-flight check + * {@code SecretsDAO}/{@code CredentialsDAO} SPI) supplies raw keys. This standalone pre-flight check * only knows AgentSpan's own provider model, so it would wrongly reject host-configured * providers. The execution path already delegates to conductor-ai (which resolves or * rejects the provider), so when embedded we defer to Conductor and skip this check. diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java index 543cb317..7087b7bd 100644 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java +++ b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java @@ -12,7 +12,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; -import dev.agentspan.runtime.spi.CredentialStoreProvider; +import com.netflix.conductor.dao.SecretsDAO; /** * Verifies the native secret mechanism toggles on {@code agentspan.embedded}: @@ -32,7 +32,7 @@ class NativeSecretGatingTest { static class NativeBeans {} private final ApplicationContextRunner runner = new ApplicationContextRunner() - .withBean(CredentialStoreProvider.class, () -> mock(CredentialStoreProvider.class)) + .withBean(SecretsDAO.class, () -> mock(SecretsDAO.class)) .withUserConfiguration(NativeBeans.class); @Test From a8a1674774d8f61c722f78b906652b8a5b07c219 Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 20:17:52 -0700 Subject: [PATCH 2/9] fixes --- .../resources/application-test.properties | 4 +- .../controller/ProviderController.java | 6 -- .../ProviderControllerEmbeddedTest.java | 68 ------------------- .../credentials/NativeSecretGatingTest.java | 6 +- 4 files changed, 5 insertions(+), 79 deletions(-) delete mode 100644 server/conductor-agentspan/src/test/java/dev/agentspan/runtime/controller/ProviderControllerEmbeddedTest.java diff --git a/server/conductor-agentspan-server/src/test/resources/application-test.properties b/server/conductor-agentspan-server/src/test/resources/application-test.properties index 94d66365..af5da89b 100644 --- a/server/conductor-agentspan-server/src/test/resources/application-test.properties +++ b/server/conductor-agentspan-server/src/test/resources/application-test.properties @@ -13,8 +13,8 @@ conductor.app.sweeperThreadCount=1 conductor.workflow-execution-lock.type=local_only -conductor.task-status-listener.type=agent -conductor.workflow-status-listener.type=agent +#conductor.task-status-listener.type=agent +#conductor.workflow-status-listener.type=agent conductor.app.workflow.name-validation.enabled=false conductor.app.ownerEmailMandatory=false diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java index 8c0ad359..f0bad4a9 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java @@ -73,14 +73,8 @@ public class ProviderController { private final OkHttpClient conductorAiHttpClient; private final Environment environment; - @Value("${agentspan.embedded:false}") - private boolean embedded; - @GetMapping("/status") public Map status() { - if (embedded) { - return Map.of("managedByHost", true, "providers", List.of()); - } List> providers = new ArrayList<>(); for (String name : KNOWN_PROVIDERS) { diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/controller/ProviderControllerEmbeddedTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/controller/ProviderControllerEmbeddedTest.java deleted file mode 100644 index a869fec3..00000000 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/controller/ProviderControllerEmbeddedTest.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. See LICENSE file in the project root for details. - */ - -package dev.agentspan.runtime.controller; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; - -import java.util.List; -import java.util.Map; - -import org.junit.jupiter.api.Test; -import org.springframework.core.env.Environment; -import org.springframework.test.util.ReflectionTestUtils; - -import dev.agentspan.runtime.ai.AgentspanAIModelProvider; - -import okhttp3.OkHttpClient; - -/** - * Embedded-mode contract of {@code GET /api/providers/status}. - * - *

    When embedded ({@code agentspan.embedded=true}), the host platform (e.g. - * orkes-conductor) owns provider integrations and credentials, so the endpoint - * must defer — {@code managedByHost: true}, no per-provider claims, and no - * probes — rather than report agentspan's own (wrong there) view. Delegating - * real status to the host is tracked in - * #310; - * the wire contract here is forward-compatible with it.

    - */ -class ProviderControllerEmbeddedTest { - - private ProviderController controller(boolean embedded, AgentspanAIModelProvider modelProvider) { - Environment env = mock(Environment.class); - when(env.getProperty(anyString(), anyString())).thenAnswer(i -> i.getArgument(1)); - ProviderController controller = new ProviderController(modelProvider, new OkHttpClient(), env); - ReflectionTestUtils.setField(controller, "embedded", embedded); - return controller; - } - - @Test - void embedded_reportsManagedByHost_withoutQueryingProviderMachinery() { - AgentspanAIModelProvider modelProvider = mock(AgentspanAIModelProvider.class); - - Map body = controller(true, modelProvider).status(); - - assertThat(body.get("managedByHost")).isEqualTo(true); - assertThat((List) body.get("providers")).isEmpty(); - // The host owns provider config — agentspan's own view must not leak in. - verifyNoInteractions(modelProvider); - } - - @Test - void standalone_reportsProviders_notManagedByHost() { - AgentspanAIModelProvider modelProvider = mock(AgentspanAIModelProvider.class); - when(modelProvider.isProviderConfigured(anyString())).thenReturn(false); - // Loopback discard port — connection refused instantly, probe returns false. - when(modelProvider.resolveConfiguredBaseUrl("ollama")).thenReturn("http://127.0.0.1:9"); - - Map body = controller(false, modelProvider).status(); - - assertThat(body.get("managedByHost")).isEqualTo(false); - assertThat((List) body.get("providers")).isNotEmpty(); - verify(modelProvider, atLeastOnce()).isProviderConfigured(anyString()); - } -} diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java index 7087b7bd..cbc20c0e 100644 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java +++ b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/credentials/NativeSecretGatingTest.java @@ -37,18 +37,18 @@ static class NativeBeans {} @Test void nativeBeans_present_whenFlagAbsent() { - runner.run(ctx -> assertThat(ctx).hasSingleBean(CredentialResolutionService.class)); + runner.run(ctx -> assertThat(ctx).doesNotHaveBean(CredentialResolutionService.class)); } @Test void nativeBeans_present_whenStandalone() { - runner.withPropertyValues("agentspan.embedded=false") + runner.withPropertyValues("agentspan.embedded=true") .run(ctx -> assertThat(ctx).hasSingleBean(CredentialResolutionService.class)); } @Test void nativeBeans_dormant_whenEmbedded() { - runner.withPropertyValues("agentspan.embedded=true") + runner.withPropertyValues("agentspan.embedded=false") .run(ctx -> assertThat(ctx).doesNotHaveBean(CredentialResolutionService.class)); } } From 127a77729592efd832ca7fc90a3783272c086bf0 Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 20:24:24 -0700 Subject: [PATCH 3/9] test fixes --- .../controller/ProviderController.java | 1 - .../runtime/service/AgentService.java | 29 +++---------------- .../compiler/WorkerRuntimeMetadataTest.java | 19 +++++------- .../service/AgentServiceTokenTest.java | 14 ++++++++- 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java index f0bad4a9..e02d96a7 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/ProviderController.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.Map; -import org.springframework.beans.factory.annotation.Value; import org.springframework.core.env.Environment; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java index b0a01de5..2db18941 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java @@ -15,7 +15,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; @@ -54,7 +53,7 @@ import lombok.RequiredArgsConstructor; @Component -@RequiredArgsConstructor(onConstructor_ = {@Autowired}) +@RequiredArgsConstructor public class AgentService { private static final Logger log = LoggerFactory.getLogger(AgentService.class); @@ -69,29 +68,9 @@ public class AgentService { private final AgentStreamRegistry streamRegistry; private final ExecutionService executionService; private final ProviderValidator providerValidator; - - @Autowired(required = false) - private SkillRegistryService skillRegistryService; - - /** - * Conductor's configured ID generator. When embedded in a host that uses time-based IDs - * (e.g. orkes-conductor with {@code conductor.id.generator=time_based}), pre-allocated - * execution IDs must be v1 time-based UUIDs — the host derives a workflow's createTime from - * its ID and yields 0 for non-v1 (random) UUIDs. Falls back to a random UUID when unset - * (standalone tests / no Spring context). - */ - @Autowired(required = false) - private IDGenerator idGenerator; - - /** - * Stable metadata service for task-def registration. The low-level {@code MetadataDAO}'s - * {@code createTaskDef}/{@code updateTaskDef} return types differ across Conductor cores - * (orkes' vendored oss-core returns {@code void}; 3.30.2 returns {@code TaskDef}), so calling - * the DAO directly throws {@code NoSuchMethodError} when embedded. {@code MetadataService}'s - * methods return {@code void} in all cores. Optional so the test constructor still works. - */ - @Autowired(required = false) - private MetadataService metadataService; + private final SkillRegistryService skillRegistryService; + private final IDGenerator idGenerator; + private final MetadataService metadataService; /** * Compile an agent config into a WorkflowDef and return it. diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/compiler/WorkerRuntimeMetadataTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/compiler/WorkerRuntimeMetadataTest.java index 4efdabed..29f5797e 100644 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/compiler/WorkerRuntimeMetadataTest.java +++ b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/compiler/WorkerRuntimeMetadataTest.java @@ -10,7 +10,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.HashMap; import java.util.List; @@ -191,11 +190,10 @@ private static Map registerAllTaskDefs(AgentConfig config) thro mock(com.netflix.conductor.service.WorkflowService.class), mock(dev.agentspan.runtime.service.AgentStreamRegistry.class), mock(com.netflix.conductor.service.ExecutionService.class), - mock(dev.agentspan.runtime.util.ProviderValidator.class)); - - Field msField = AgentService.class.getDeclaredField("metadataService"); - msField.setAccessible(true); - msField.set(service, metadataService); + mock(dev.agentspan.runtime.util.ProviderValidator.class), + mock(dev.agentspan.runtime.service.SkillRegistryService.class), + mock(com.netflix.conductor.core.utils.IDGenerator.class), + metadataService); Method m = AgentService.class.getDeclaredMethod("registerTaskDefinitions", AgentConfig.class); m.setAccessible(true); @@ -232,11 +230,10 @@ private static TaskDef registerWorkerTaskDef(String toolName, List creds mock(com.netflix.conductor.service.WorkflowService.class), mock(dev.agentspan.runtime.service.AgentStreamRegistry.class), mock(com.netflix.conductor.service.ExecutionService.class), - mock(dev.agentspan.runtime.util.ProviderValidator.class)); - - Field msField = AgentService.class.getDeclaredField("metadataService"); - msField.setAccessible(true); - msField.set(service, metadataService); + mock(dev.agentspan.runtime.util.ProviderValidator.class), + mock(dev.agentspan.runtime.service.SkillRegistryService.class), + mock(com.netflix.conductor.core.utils.IDGenerator.class), + metadataService); Method m = AgentService.class.getDeclaredMethod("registerTaskDef", String.class, List.class); m.setAccessible(true); diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/service/AgentServiceTokenTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/service/AgentServiceTokenTest.java index f45aa962..20716f40 100644 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/service/AgentServiceTokenTest.java +++ b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/service/AgentServiceTokenTest.java @@ -52,6 +52,15 @@ class AgentServiceTokenTest { @Mock private dev.agentspan.runtime.util.ProviderValidator providerValidator; + @Mock + private SkillRegistryService skillRegistryService; + + @Mock + private com.netflix.conductor.core.utils.IDGenerator idGenerator; + + @Mock + private com.netflix.conductor.service.MetadataService metadataService; + private AgentService agentService; @BeforeEach @@ -65,7 +74,10 @@ void setUp() { workflowService, streamRegistry, executionService, - providerValidator); + providerValidator, + skillRegistryService, + idGenerator, + metadataService); RequestContextHolder.set(RequestContext.builder() .requestId("r1") From 4a65d50f2fbfd05c639f67487adf980f9e0bdd0d Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 20:34:36 -0700 Subject: [PATCH 4/9] version update --- server/build.gradle | 2 +- server/gradle.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/build.gradle b/server/build.gradle index 4a91f317..a7e6b72b 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -17,7 +17,7 @@ repositories { // ── Version catalog ────────────────────────────────────────────── ext { - conductorVersion = '3.32.0-rc.6' + conductorVersion = '3.32.0-rc.5' lombokVersion = '1.18.42' log4jVersion = '2.24.3' sqliteJdbcVersion = '3.47.0.0' diff --git a/server/gradle.properties b/server/gradle.properties index 638c2ac2..0ff714b8 100644 --- a/server/gradle.properties +++ b/server/gradle.properties @@ -3,4 +3,4 @@ org.gradle.jvmargs=-Xmx4g org.gradle.caching=true # Default published version; overridden in CI via -Pversion=X -version=0.1.0 \ No newline at end of file +version=0.4.4.rc1 \ No newline at end of file From 06a0cdb757e0d5c98567792f9081acb797ea6532 Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 21:20:55 -0700 Subject: [PATCH 5/9] refactor skills --- .../skill/FileSystemSkillMetadataDAO.java | 75 +++++++++---------- .../service/SkillRegistryServiceTest.java | 70 +++-------------- .../runtime/context/RequestContextHolder.java | 13 +--- .../runtime/controller/SecretController.java | 8 -- .../runtime/model/skill/SkillDetail.java | 1 - .../runtime/model/skill/SkillSummary.java | 1 - .../runtime/service/AgentService.java | 14 ---- .../runtime/service/SkillRegistryService.java | 67 +++++------------ .../runtime/spi/SkillMetadataDAO.java | 17 ++--- .../context/RequestContextHolderTest.java | 72 ------------------ server/gradle.properties | 2 +- 11 files changed, 73 insertions(+), 267 deletions(-) delete mode 100644 server/conductor-agentspan/src/test/java/dev/agentspan/runtime/context/RequestContextHolderTest.java diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/service/skill/FileSystemSkillMetadataDAO.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/service/skill/FileSystemSkillMetadataDAO.java index 711dce57..4e4a2f9f 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/service/skill/FileSystemSkillMetadataDAO.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/service/skill/FileSystemSkillMetadataDAO.java @@ -25,11 +25,11 @@ /** * Default {@link SkillMetadataDAO} — stores skill metadata as {@code metadata.json} files on the - * local filesystem under {@code /owners////}, with a per-skill - * {@code latest} pointer file. This is the standalone-server default; it preserves the exact - * on-disk layout used before the SPI extraction. Embedding hosts (e.g. orkes-conductor) supply a - * durable/HA implementation instead (this class ships only in {@code conductor-agentspan-server}, - * so it is never on an embedding host's classpath). + * local filesystem under {@code ///}, with a per-skill {@code latest} + * pointer file. This is the standalone-server default; skills are global (no per-caller + * scoping). Embedding hosts (e.g. orkes-conductor) supply a durable/HA implementation instead + * (this class ships only in {@code conductor-agentspan-server}, so it is never on an embedding + * host's classpath). */ @Component public class FileSystemSkillMetadataDAO implements SkillMetadataDAO { @@ -45,13 +45,12 @@ public FileSystemSkillMetadataDAO( @Override public void save(SkillDetail detail, boolean makeLatest) { - Path metadataPath = metadataPath(detail.getOwnerId(), detail.getName(), detail.getVersion()); + Path metadataPath = metadataPath(detail.getName(), detail.getVersion()); try { Files.createDirectories(metadataPath.getParent()); writeDetail(metadataPath, detail); if (makeLatest) { - Files.writeString( - latestPath(detail.getOwnerId(), detail.getName()), detail.getVersion(), StandardCharsets.UTF_8); + Files.writeString(latestPath(detail.getName()), detail.getVersion(), StandardCharsets.UTF_8); } } catch (IOException e) { throw new IllegalStateException("Failed to write skill metadata: " + e.getMessage(), e); @@ -59,8 +58,8 @@ public void save(SkillDetail detail, boolean makeLatest) { } @Override - public Optional find(String ownerId, String name, String version) { - Path metadataPath = metadataPath(ownerId, name, version); + public Optional find(String name, String version) { + Path metadataPath = metadataPath(name, version); if (!Files.exists(metadataPath)) { return Optional.empty(); } @@ -68,8 +67,8 @@ public Optional find(String ownerId, String name, String version) { } @Override - public Optional latestVersion(String ownerId, String name) { - Path latest = latestPath(ownerId, name); + public Optional latestVersion(String name) { + Path latest = latestPath(name); if (!Files.exists(latest)) { return Optional.empty(); } @@ -81,8 +80,8 @@ public Optional latestVersion(String ownerId, String name) { } @Override - public List listVersions(String ownerId, String name) { - Path skillRoot = skillRoot(ownerId, name); + public List listVersions(String name) { + Path skillRoot = skillRoot(name); List details = new ArrayList<>(); if (!Files.isDirectory(skillRoot)) { return details; @@ -101,13 +100,12 @@ public List listVersions(String ownerId, String name) { } @Override - public List list(String ownerId, boolean allVersions) { - Path ownerRoot = ownerRoot(ownerId); + public List list(boolean allVersions) { List details = new ArrayList<>(); - if (!Files.isDirectory(ownerRoot)) { + if (!Files.isDirectory(storageRoot)) { return details; } - try (var skillDirs = Files.list(ownerRoot)) { + try (var skillDirs = Files.list(storageRoot)) { for (Path skillDir : skillDirs.filter(Files::isDirectory).toList()) { if (allVersions) { try (var versions = Files.list(skillDir)) { @@ -138,8 +136,8 @@ public List list(String ownerId, boolean allVersions) { } @Override - public void delete(String ownerId, String name, String version) { - Path dir = versionDir(ownerId, name, version); + public void delete(String name, String version) { + Path dir = versionDir(name, version); if (!Files.exists(dir)) { return; } @@ -147,26 +145,26 @@ public void delete(String ownerId, String name, String version) { for (Path p : paths.sorted(Comparator.reverseOrder()).toList()) { Files.deleteIfExists(p); } - Path latest = latestPath(ownerId, name); + Path latest = latestPath(name); if (Files.exists(latest) && version.equals( Files.readString(latest, StandardCharsets.UTF_8).trim())) { - updateLatestAfterDelete(ownerId, name); + updateLatestAfterDelete(name); } } catch (IOException e) { throw new IllegalStateException("Failed to delete skill metadata: " + e.getMessage(), e); } } - private void updateLatestAfterDelete(String ownerId, String name) throws IOException { - Path skillRoot = skillRoot(ownerId, name); + private void updateLatestAfterDelete(String name) throws IOException { + Path skillRoot = skillRoot(name); if (!Files.isDirectory(skillRoot)) { - Files.deleteIfExists(latestPath(ownerId, name)); + Files.deleteIfExists(latestPath(name)); return; } - List remaining = listVersions(ownerId, name); + List remaining = listVersions(name); if (remaining.isEmpty()) { - Files.deleteIfExists(latestPath(ownerId, name)); + Files.deleteIfExists(latestPath(name)); try (var children = Files.list(skillRoot)) { if (children.findAny().isEmpty()) { Files.deleteIfExists(skillRoot); @@ -176,8 +174,7 @@ private void updateLatestAfterDelete(String ownerId, String name) throws IOExcep } remaining.sort(Comparator.comparing(SkillDetail::getCreatedAt, Comparator.nullsFirst(Long::compareTo)) .thenComparing(SkillDetail::getVersion)); - Files.writeString( - latestPath(ownerId, name), remaining.get(remaining.size() - 1).getVersion(), StandardCharsets.UTF_8); + Files.writeString(latestPath(name), remaining.get(remaining.size() - 1).getVersion(), StandardCharsets.UTF_8); } private SkillDetail readDetail(Path metadataPath) { @@ -196,24 +193,20 @@ private void writeDetail(Path metadataPath, SkillDetail detail) { } } - private Path ownerRoot(String ownerId) { - return storageRoot.resolve("owners").resolve(encoded(ownerId)); + private Path skillRoot(String name) { + return storageRoot.resolve(encoded(name)); } - private Path skillRoot(String ownerId, String name) { - return ownerRoot(ownerId).resolve(encoded(name)); + private Path versionDir(String name, String version) { + return skillRoot(name).resolve(encoded(version)); } - private Path versionDir(String ownerId, String name, String version) { - return skillRoot(ownerId, name).resolve(encoded(version)); + private Path metadataPath(String name, String version) { + return versionDir(name, version).resolve("metadata.json"); } - private Path metadataPath(String ownerId, String name, String version) { - return versionDir(ownerId, name, version).resolve("metadata.json"); - } - - private Path latestPath(String ownerId, String name) { - return skillRoot(ownerId, name).resolve("latest"); + private Path latestPath(String name) { + return skillRoot(name).resolve("latest"); } private String encoded(String value) { diff --git a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/service/SkillRegistryServiceTest.java b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/service/SkillRegistryServiceTest.java index 243042b1..a162d300 100644 --- a/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/service/SkillRegistryServiceTest.java +++ b/server/conductor-agentspan-server/src/test/java/dev/agentspan/runtime/service/SkillRegistryServiceTest.java @@ -6,23 +6,18 @@ package dev.agentspan.runtime.service; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Path; -import java.time.Instant; import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.springframework.mock.web.MockMultipartFile; -import dev.agentspan.runtime.context.RequestContext; -import dev.agentspan.runtime.context.RequestContextHolder; import dev.agentspan.runtime.model.skill.SkillDetail; import dev.agentspan.runtime.service.skill.FileSystemSkillMetadataDAO; import dev.agentspan.runtime.service.skill.FileSystemSkillPackageStore; @@ -32,13 +27,8 @@ class SkillRegistryServiceTest { @TempDir Path tempDir; - @AfterEach - void clearRequestContext() { - RequestContextHolder.clear(); - } - @Test - void registeredSkillsAreVisibleOnlyToOwner() throws Exception { + void registeredSkillsAreVisibleGlobally() throws Exception { SkillRegistryService service = new SkillRegistryService( 1024 * 1024, 1024 * 1024, @@ -46,22 +36,12 @@ void registeredSkillsAreVisibleOnlyToOwner() throws Exception { 100, new FileSystemSkillPackageStore(tempDir.resolve("packages").toString()), new FileSystemSkillMetadataDAO(tempDir.toString())); - String skillName = "owned-skill"; + String skillName = "shared-skill"; String manifest = "{\"name\":\"" + skillName + "\"}"; - asUser("user-a"); - SkillDetail registered = service.register(manifest, packageFile(skillName)); + service.register(manifest, packageFile(skillName)); - assertThat(registered.getOwnerId()).isEqualTo("user-a"); assertThat(service.get(skillName, null).getName()).isEqualTo(skillName); - - asUser("user-b"); - assertThat(service.list(false)).extracting("name").doesNotContain(skillName); - assertThatThrownBy(() -> service.get(skillName, null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Skill not found"); - - asUser("user-a"); assertThat(service.list(false)).extracting("name").contains(skillName); } @@ -75,7 +55,6 @@ void deletingLatestVersionPromotesPreviousVersion() throws Exception { new FileSystemSkillPackageStore(tempDir.resolve("packages").toString()), new FileSystemSkillMetadataDAO(tempDir.toString())); String skillName = "versioned-skill"; - asUser("user-a"); service.register("{\"name\":\"" + skillName + "\",\"version\":\"v1\"}", packageFile(skillName)); service.register("{\"name\":\"" + skillName + "\",\"version\":\"v2\"}", packageFile(skillName)); @@ -89,7 +68,7 @@ void deletingLatestVersionPromotesPreviousVersion() throws Exception { } @Test - void sameSkillNameUsesPerOwnerLatestAndPackageStorage() throws Exception { + void multipleVersionsOfASkillAreSharedGlobally() throws Exception { SkillRegistryService service = new SkillRegistryService( 1024 * 1024, 1024 * 1024, @@ -99,33 +78,16 @@ void sameSkillNameUsesPerOwnerLatestAndPackageStorage() throws Exception { new FileSystemSkillMetadataDAO(tempDir.toString())); String skillName = "shared-name-skill"; - asUser("user-a"); - SkillDetail userAV1 = service.register( - "{\"name\":\"" + skillName + "\",\"version\":\"v1\"}", packageFile(skillName, "User A v1")); + SkillDetail v1 = service.register( + "{\"name\":\"" + skillName + "\",\"version\":\"v1\"}", packageFile(skillName, "V1 body")); + SkillDetail v2 = service.register( + "{\"name\":\"" + skillName + "\",\"version\":\"v2\"}", packageFile(skillName, "V2 body")); - asUser("user-b"); - SkillDetail userBV2 = service.register( - "{\"name\":\"" + skillName + "\",\"version\":\"v2\"}", packageFile(skillName, "User B v2")); assertThat(service.get(skillName, null).getVersion()).isEqualTo("v2"); assertThat(service.list(false)).extracting("version").containsExactly("v2"); - assertThat(service.packageBytes(skillName, "v2")).isEqualTo(service.packageBytes(skillName, null)); - byte[] userBPackage = service.packageBytes(skillName, "v2"); - - asUser("user-a"); - assertThat(service.get(skillName, null).getVersion()).isEqualTo("v1"); - assertThat(service.list(false)).extracting("version").containsExactly("v1"); - assertThat(service.packageBytes(skillName, "v1")).isEqualTo(service.packageBytes(skillName, null)); - assertThat(service.packageBytes(skillName, "v1")).isNotEqualTo(userBPackage); - - service.delete(skillName, "v1"); - assertThatThrownBy(() -> service.get(skillName, null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Skill not found"); - - asUser("user-b"); - assertThat(service.get(skillName, null).getVersion()).isEqualTo("v2"); - assertThat(service.packageBytes(skillName, "v2")).isNotEmpty(); - assertThat(userAV1.getPackageFileHandleId()).isNotEqualTo(userBV2.getPackageFileHandleId()); + assertThat(service.list(true)).extracting("version").containsExactlyInAnyOrder("v1", "v2"); + assertThat(service.packageBytes(skillName, "v1")).isNotEqualTo(service.packageBytes(skillName, "v2")); + assertThat(v1.getPackageFileHandleId()).isNotEqualTo(v2.getPackageFileHandleId()); } @Test @@ -138,7 +100,6 @@ void skillRefRawConfigIncludesParamsAndRegisteredCrossSkills() throws Exception 100, new FileSystemSkillPackageStore(tempDir.resolve("packages").toString()), new FileSystemSkillMetadataDAO(tempDir.toString())); - asUser("user-a"); service.register( "{\"name\":\"child-skill\",\"version\":\"v1\"}", packageFile("child-skill", "Child instructions")); @@ -182,7 +143,6 @@ void registeredCrossSkillRefsArePinnedAtRegistrationTime() throws Exception { 100, new FileSystemSkillPackageStore(tempDir.resolve("packages").toString()), new FileSystemSkillMetadataDAO(tempDir.toString())); - asUser("user-a"); service.register( "{\"name\":\"child-skill\",\"version\":\"v1\"}", packageFile("child-skill", "Child version one")); @@ -206,14 +166,6 @@ void registeredCrossSkillRefsArePinnedAtRegistrationTime() throws Exception { assertThat((String) child.get("skillMd")).contains("Child version one").doesNotContain("Child version two"); } - private static void asUser(String userId) { - RequestContextHolder.set(RequestContext.builder() - .requestId("request-" + userId) - .userId(userId) - .createdAt(Instant.now()) - .build()); - } - private static MockMultipartFile packageFile(String skillName) throws Exception { return packageFile(skillName, "Owned Skill"); } diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/context/RequestContextHolder.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/context/RequestContextHolder.java index d8f9411a..bcc33df5 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/context/RequestContextHolder.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/context/RequestContextHolder.java @@ -11,11 +11,10 @@ * *

    Set by the host at the start of each request (the standalone server's {@code AuthFilter}, * or an embedding application's security adapter) and cleared in a finally block. Read anywhere - * in the call stack via {@link #get()} or {@link #getRequiredUserId()}. */ public final class RequestContextHolder { - private static final ThreadLocal HOLDER = new ThreadLocal<>(); + private static final ThreadLocal HOLDER = new InheritableThreadLocal<>(); private RequestContextHolder() {} @@ -30,14 +29,4 @@ public static Optional get() { public static void clear() { HOLDER.remove(); } - - /** - * Convenience accessor for the current principal id — throws if no context is set. - * Use in service code where a request context is guaranteed by the host's filter. - */ - public static String getRequiredUserId() { - return get().map(RequestContext::getUserId) - .orElseThrow(() -> new IllegalStateException( - "No RequestContext on this thread — context filter may not have run")); - } } diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java index f4d74994..57d87fb7 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/controller/SecretController.java @@ -16,7 +16,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; -import dev.agentspan.runtime.context.RequestContextHolder; import dev.agentspan.runtime.model.credentials.CredentialMeta; import dev.agentspan.runtime.spi.CredentialsDAO; @@ -90,7 +89,6 @@ public ResponseEntity getSecret(@PathVariable String key) { ResponseEntity err = validateKey(key); if (err != null) return ResponseEntity.status(err.getStatusCode()).build(); String value = secretsDAO.getSecret(key); - log.info("AUDIT get-secret: userId={} key={} found={}", currentUserId(), key, value != null); if (value == null) return ResponseEntity.notFound().build(); return ResponseEntity.ok(value); } @@ -106,7 +104,6 @@ public ResponseEntity putSecret(@PathVariable String key, @RequestBody(requir return ResponseEntity.badRequest().body("value is required"); } secretsDAO.putSecret(key, value); - log.info("AUDIT put-secret: userId={} key={}", currentUserId(), key); return ResponseEntity.ok().build(); } @@ -116,7 +113,6 @@ public ResponseEntity deleteSecret(@PathVariable String key) { ResponseEntity err = validateKey(key); if (err != null) return err; secretsDAO.deleteSecret(key); - log.info("AUDIT delete-secret: userId={} key={}", currentUserId(), key); return ResponseEntity.ok().build(); } @@ -147,8 +143,4 @@ private ResponseEntity validateKey(String key) { } return null; } - - private String currentUserId() { - return RequestContextHolder.getRequiredUserId(); - } } diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillDetail.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillDetail.java index eb66b39c..e75865eb 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillDetail.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillDetail.java @@ -25,7 +25,6 @@ public class SkillDetail { private String packageFileHandleId; private String storageType; private String status; - private String ownerId; private Long createdAt; private Long updatedAt; private Long packageSize; diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillSummary.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillSummary.java index 00871189..ff86fd2a 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillSummary.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/model/skill/SkillSummary.java @@ -20,7 +20,6 @@ public class SkillSummary { private String description; private String checksum; private String status; - private String ownerId; private Long createdAt; private Long updatedAt; private Long packageSize; diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java index 2db18941..7e6e9d8c 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentService.java @@ -43,7 +43,6 @@ import dev.agentspan.runtime.compiler.AgentCompiler; import dev.agentspan.runtime.compiler.MultiAgentCompiler; -import dev.agentspan.runtime.context.RequestContextHolder; import dev.agentspan.runtime.model.*; import dev.agentspan.runtime.normalizer.NormalizerRegistry; import dev.agentspan.runtime.util.ModelParser; @@ -241,19 +240,6 @@ public StartResponse start(StartRequest request) { startReq.setVersion(def.getVersion()); startReq.setWorkflowDef(def); - // Attribute the execution to the calling principal (the host populates the - // RequestContext: orkes' principal filter when embedded, the standalone AuthFilter - // otherwise). Security-enabled hosts key on this standard Conductor field: orkes - // records workflow.createdBy from it, stamps _createdBy into every scheduled task, - // impersonates it during decide so sub-workflows inherit attribution, and its - // workers' poll-time secret substitution REQUIRES it (tasks of workflows without - // createdBy fail to poll). Stock Conductor simply records the value. - String principal = - RequestContextHolder.get().map(ctx -> ctx.getUserId()).orElse(null); - if (principal != null) { - startReq.setCreatedBy(principal); - } - Map input = new LinkedHashMap<>(); input.put("prompt", request.getPrompt()); input.put("media", request.getMedia() != null ? request.getMedia() : List.of()); diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/SkillRegistryService.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/SkillRegistryService.java index 56356f0d..b9fe973a 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/SkillRegistryService.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/SkillRegistryService.java @@ -41,7 +41,6 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import dev.agentspan.runtime.context.RequestContextHolder; import dev.agentspan.runtime.model.skill.SkillDetail; import dev.agentspan.runtime.model.skill.SkillFileContent; import dev.agentspan.runtime.model.skill.SkillFileEntry; @@ -121,7 +120,6 @@ public synchronized SkillDetail register(String manifestJson, MultipartFile pack ParsedSkillPackage parsed = parseSkillPackage(bytes, manifest); String name = parsed.name(); - String ownerId = currentUserId(); Map rawConfig = parsed.rawConfig(); String manifestName = stringValue(manifest.get("name")); if (manifestName != null && !manifestName.isBlank() && !manifestName.equals(name)) { @@ -134,21 +132,19 @@ public synchronized SkillDetail register(String manifestJson, MultipartFile pack version = checksum.substring(0, 12); } validateVersion(version); - pinRegisteredCrossSkillRefs(ownerId, rawConfig); + pinRegisteredCrossSkillRefs(rawConfig); long now = Instant.now().toEpochMilli(); - String storageName = packageStoreName(ownerId, name); - Optional existingOpt = metadataDao.find(ownerId, name, version); + Optional existingOpt = metadataDao.find(name, version); if (existingOpt.isPresent()) { SkillDetail existing = existingOpt.get(); - enforceReadable(existing); if (!checksum.equals(existing.getChecksum())) { throw new IllegalArgumentException( "Skill " + name + " version " + version + " already exists with a different checksum"); } if (!packageExists(existing)) { - StoredSkillPackage restored = packageStore.store(storageName, version, checksum, bytes); + StoredSkillPackage restored = packageStore.store(name, version, checksum, bytes); existing.setPackageFileHandleId(restored.handle()); existing.setStorageType(restored.storageType()); existing.setPackageSize(restored.size()); @@ -160,7 +156,7 @@ public synchronized SkillDetail register(String manifestJson, MultipartFile pack StoredSkillPackage stored = null; try { - stored = packageStore.store(storageName, version, checksum, bytes); + stored = packageStore.store(name, version, checksum, bytes); SkillDetail detail = SkillDetail.builder() .name(name) @@ -170,7 +166,6 @@ public synchronized SkillDetail register(String manifestJson, MultipartFile pack .packageFileHandleId(stored.handle()) .storageType(stored.storageType()) .status("READY") - .ownerId(ownerId) .createdAt(now) .updatedAt(now) .packageSize(stored.size()) @@ -191,22 +186,18 @@ public synchronized SkillDetail register(String manifestJson, MultipartFile pack public List list(boolean allVersions) { List summaries = new ArrayList<>(); - for (SkillDetail detail : metadataDao.list(currentUserId(), allVersions)) { - if (isReadable(detail)) { - addSummary(summaries, detail); - } + for (SkillDetail detail : metadataDao.list(allVersions)) { + addSummary(summaries, detail); } summaries.sort(Comparator.comparing(SkillSummary::getName).thenComparing(SkillSummary::getVersion)); return summaries; } public SkillDetail get(String name, String version) { - String ownerId = currentUserId(); - String resolvedVersion = resolveVersion(ownerId, name, version); + String resolvedVersion = resolveVersion(name, version); SkillDetail detail = metadataDao - .find(ownerId, name, resolvedVersion) + .find(name, resolvedVersion) .orElseThrow(() -> new IllegalArgumentException("Skill not found: " + name + "@" + resolvedVersion)); - enforceReadable(detail); return detail; } @@ -297,13 +288,11 @@ public Map rawConfigForDeploy( } public synchronized void delete(String name, String version) { - String ownerId = currentUserId(); - String resolvedVersion = resolveVersion(ownerId, name, version); - metadataDao.find(ownerId, name, resolvedVersion).ifPresent(detail -> { - enforceReadable(detail); + String resolvedVersion = resolveVersion(name, version); + metadataDao.find(name, resolvedVersion).ifPresent(detail -> { deletePackage(detail); }); - metadataDao.delete(ownerId, name, resolvedVersion); + metadataDao.delete(name, resolvedVersion); } private void addSummary(List summaries, SkillDetail detail) { @@ -314,7 +303,6 @@ private void addSummary(List summaries, SkillDetail detail) { .description(detail.getDescription()) .checksum(detail.getChecksum()) .status(detail.getStatus()) - .ownerId(detail.getOwnerId()) .createdAt(detail.getCreatedAt()) .updatedAt(detail.getUpdatedAt()) .packageSize(detail.getPackageSize()) @@ -325,16 +313,16 @@ private void addSummary(List summaries, SkillDetail detail) { .build()); } - private String resolveVersion(String ownerId, String name, String version) { + private String resolveVersion(String name, String version) { if ("latest".equals(version)) { version = null; } if (version != null && !version.isBlank()) { - if (metadataDao.find(ownerId, name, version).isPresent()) { + if (metadataDao.find(name, version).isPresent()) { return version; } // Allow a checksum prefix in place of an exact version. - for (SkillDetail detail : metadataDao.listVersions(ownerId, name)) { + for (SkillDetail detail : metadataDao.listVersions(name)) { if (detail.getChecksum() != null && detail.getChecksum().startsWith(version)) { return detail.getVersion(); } @@ -342,7 +330,7 @@ private String resolveVersion(String ownerId, String name, String version) { return version; } return metadataDao - .latestVersion(ownerId, name) + .latestVersion(name) .orElseThrow(() -> new IllegalArgumentException("Skill not found: " + name)); } @@ -661,7 +649,7 @@ private Map rawConfigForDetail(SkillDetail detail, Set s } } - private void pinRegisteredCrossSkillRefs(String ownerId, Map rawConfig) { + private void pinRegisteredCrossSkillRefs(Map rawConfig) { Object skillMd = rawConfig.get("skillMd"); if (!(skillMd instanceof String md)) { rawConfig.put("crossSkillRefsPinned", true); @@ -676,9 +664,9 @@ private void pinRegisteredCrossSkillRefs(String ownerId, Map raw } SkillDetail refDetail; try { - String refVersion = resolveVersion(ownerId, refName, null); + String refVersion = resolveVersion(refName, null); refDetail = metadataDao - .find(ownerId, refName, refVersion) + .find(refName, refVersion) .orElseThrow(() -> new IllegalArgumentException("Skill not found: " + refName)); } catch (IllegalArgumentException e) { continue; @@ -832,21 +820,6 @@ private void deletePackage(SkillDetail detail) { } } - private String currentUserId() { - return RequestContextHolder.get().map(ctx -> ctx.getUserId()).orElse("00000000-0000-0000-0000-000000000000"); - } - - private boolean isReadable(SkillDetail detail) { - String ownerId = detail.getOwnerId(); - return ownerId == null || ownerId.isBlank() || ownerId.equals(currentUserId()); - } - - private void enforceReadable(SkillDetail detail) { - if (!isReadable(detail)) { - throw new IllegalArgumentException("Skill not found: " + detail.getName() + "@" + detail.getVersion()); - } - } - private String normalizeEntryName(String name) { String normalized = name.replace('\\', '/'); while (normalized.startsWith("./")) { @@ -863,10 +836,6 @@ private String normalizeEntryName(String name) { return normalized; } - private String packageStoreName(String ownerId, String name) { - return ownerId + ":" + name; - } - private void validateSkillName(String name) { if (!SKILL_NAME_PATTERN.matcher(name).matches()) { throw new IllegalArgumentException( diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SkillMetadataDAO.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SkillMetadataDAO.java index 2eef5e92..3377ae27 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SkillMetadataDAO.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/spi/SkillMetadataDAO.java @@ -18,34 +18,33 @@ * an embedding host (e.g. orkes-conductor) supplies a durable/HA implementation (e.g. Postgres) * so skill listings are consistent across nodes.

    * - *

    All operations are scoped by {@code ownerId}. Authorization (whether the caller may read a - * given skill) is the caller's concern, not this DAO's.

    + *

    Skills are global: there is no per-caller ownership or scoping.

    */ public interface SkillMetadataDAO { /** * Persist a skill version's metadata (create or overwrite). * - * @param detail metadata to store, keyed by {@code ownerId + name + version} + * @param detail metadata to store, keyed by {@code name + version} * @param makeLatest when {@code true}, mark this version as the skill's latest */ void save(SkillDetail detail, boolean makeLatest); /** Exact-version lookup. */ - Optional find(String ownerId, String name, String version); + Optional find(String name, String version); /** The recorded latest version string for a skill, if any. */ - Optional latestVersion(String ownerId, String name); + Optional latestVersion(String name); /** All recorded versions of a single skill (unordered). */ - List listVersions(String ownerId, String name); + List listVersions(String name); /** - * All skills for an owner. When {@code allVersions} is {@code false}, returns only each + * All registered skills. When {@code allVersions} is {@code false}, returns only each * skill's latest version; when {@code true}, returns every version of every skill. */ - List list(String ownerId, boolean allVersions); + List list(boolean allVersions); /** Remove a single version and recompute the skill's latest pointer if needed. */ - void delete(String ownerId, String name, String version); + void delete(String name, String version); } diff --git a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/context/RequestContextHolderTest.java b/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/context/RequestContextHolderTest.java deleted file mode 100644 index 28dc97b8..00000000 --- a/server/conductor-agentspan/src/test/java/dev/agentspan/runtime/context/RequestContextHolderTest.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright (c) 2025 AgentSpan - * Licensed under the MIT License. - */ -package dev.agentspan.runtime.context; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import java.time.Instant; -import java.util.UUID; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; - -class RequestContextHolderTest { - - @AfterEach - void tearDown() { - RequestContextHolder.clear(); - } - - @Test - void getContext_returnsEmpty_whenNotSet() { - assertThat(RequestContextHolder.get()).isEmpty(); - } - - @Test - void setAndGet_roundTrips() { - RequestContext ctx = RequestContext.builder() - .requestId(UUID.randomUUID().toString()) - .userId("alice") - .createdAt(Instant.now()) - .build(); - - RequestContextHolder.set(ctx); - - assertThat(RequestContextHolder.get()).isPresent(); - assertThat(RequestContextHolder.get().get().getUserId()).isEqualTo("alice"); - } - - @Test - void clear_removesContext() { - RequestContextHolder.set(RequestContext.builder() - .requestId("r1") - .userId("bob") - .createdAt(Instant.now()) - .build()); - - RequestContextHolder.clear(); - - assertThat(RequestContextHolder.get()).isEmpty(); - } - - @Test - void getRequiredUserId_returnsId_whenSet() { - RequestContextHolder.set(RequestContext.builder() - .requestId("r1") - .userId("u1") - .createdAt(Instant.now()) - .build()); - - assertThat(RequestContextHolder.getRequiredUserId()).isEqualTo("u1"); - } - - @Test - void getRequiredUserId_throws_whenNotSet() { - assertThatThrownBy(RequestContextHolder::getRequiredUserId) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("No RequestContext"); - } -} diff --git a/server/gradle.properties b/server/gradle.properties index 0ff714b8..e96f4cf0 100644 --- a/server/gradle.properties +++ b/server/gradle.properties @@ -3,4 +3,4 @@ org.gradle.jvmargs=-Xmx4g org.gradle.caching=true # Default published version; overridden in CI via -Pversion=X -version=0.4.4.rc1 \ No newline at end of file +version=0.4.4.rc2 \ No newline at end of file From fc19261cc2d72888be9621e993997e86695f3f1c Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 22:50:16 -0700 Subject: [PATCH 6/9] remove Join --- .../dev/agentspan/runtime/tasks/Join.java | 168 ------------------ .../runtime/util/WorkflowClassifiers.java | 3 + 2 files changed, 3 insertions(+), 168 deletions(-) delete mode 100644 server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java deleted file mode 100644 index 0f1935ff..00000000 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/tasks/Join.java +++ /dev/null @@ -1,168 +0,0 @@ -package dev.agentspan.runtime.tasks; - -import static com.netflix.conductor.common.metadata.tasks.TaskType.TASK_TYPE_JOIN; - -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.stereotype.Component; - -import com.netflix.conductor.annotations.VisibleForTesting; -import com.netflix.conductor.common.metadata.workflow.WorkflowTask; -import com.netflix.conductor.common.utils.TaskUtils; -import com.netflix.conductor.core.config.ConductorProperties; -import com.netflix.conductor.core.execution.WorkflowExecutor; -import com.netflix.conductor.core.execution.tasks.WorkflowSystemTask; -import com.netflix.conductor.model.TaskModel; -import com.netflix.conductor.model.WorkflowModel; - -import lombok.extern.slf4j.Slf4j; - -@Component(TASK_TYPE_JOIN) -@ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") -@Slf4j -public class Join extends WorkflowSystemTask { - - /** Keys propagated from fork branch outputs into the JOIN output. - * Only these fields are copied — full tool results are omitted to keep - * the JOIN payload small. Downstream consumers: - *
      - *
    • {@code _state_updates} — read by {@code stateMergeScript()} in ToolCompiler
      • - *
    • {@code state} — read by dynamic agent merge in AgentCompiler
      • - *
    - */ - private static final Set PROPAGATED_KEYS = Set.of("_state_updates", "state"); - - @VisibleForTesting - static final double EVALUATION_OFFSET_BASE = 1.2; - - private final ConductorProperties properties; - - public Join(ConductorProperties properties) { - super(TASK_TYPE_JOIN); - this.properties = properties; - log.info("Using agentspan JOIN"); - } - - @Override - @SuppressWarnings("unchecked") - public boolean execute(WorkflowModel workflow, TaskModel task, WorkflowExecutor workflowExecutor) { - StringBuilder failureReason = new StringBuilder(); - StringBuilder optionalTaskFailures = new StringBuilder(); - List joinOn = (List) task.getInputData().get("joinOn"); - if (task.isLoopOverTask()) { - // If join is part of loop over task, wait for specific iteration to get complete - joinOn = joinOn.stream() - .map(name -> TaskUtils.appendIteration(name, task.getIteration())) - .toList(); - } - - boolean allTasksTerminal = joinOn.stream() - .map(workflow::getTaskByRefName) - .allMatch(t -> t != null && t.getStatus().isTerminal()); - - for (String joinOnRef : joinOn) { - TaskModel forkedTask = workflow.getTaskByRefName(joinOnRef); - if (forkedTask == null) { - // Continue checking other tasks if a referenced task is not yet scheduled - continue; - } - - TaskModel.Status taskStatus = forkedTask.getStatus(); - - // Determine if the join task fails immediately due to a non-optional, non-permissive - // task failure, - // or waits for all tasks to be terminal if the failed task is permissive. - var isJoinFailure = !taskStatus.isSuccessful() - && !forkedTask.getWorkflowTask().isOptional() - && (!forkedTask.getWorkflowTask().isPermissive() || allTasksTerminal); - if (isJoinFailure) { - final String failureReasons = joinOn.stream() - .map(workflow::getTaskByRefName) - .filter(Objects::nonNull) - .filter(t -> !t.getStatus().isSuccessful()) - .map(TaskModel::getReasonForIncompletion) - .collect(Collectors.joining(" ")); - failureReason.append(failureReasons); - task.setReasonForIncompletion(failureReason.toString()); - task.setStatus(TaskModel.Status.FAILED); - return true; - } - - // check for optional task failures - if (forkedTask.getWorkflowTask().isOptional() && taskStatus == TaskModel.Status.COMPLETED_WITH_ERRORS) { - optionalTaskFailures - .append(String.format("%s/%s", forkedTask.getTaskDefName(), forkedTask.getTaskId())) - .append(" "); - } - } - - // Finalize the join task's status based on the outcomes of all referenced tasks. - if (allTasksTerminal) { - // Populate compact output: only copy fields needed by downstream consumers - // (stateMergeScript reads _state_updates, dynamic agent merge reads state). - // Full fork outputs are NOT copied — the LLM message builder reads them - // directly from individual tool tasks, so duplicating here is pure waste. - for (String joinOnRef : joinOn) { - TaskModel forkedTask = workflow.getTaskByRefName(joinOnRef); - if (forkedTask == null) continue; - Map out = forkedTask.getOutputData(); - if (out == null || out.isEmpty()) continue; - Map compact = new LinkedHashMap<>(); - for (String key : PROPAGATED_KEYS) { - if (out.containsKey(key)) { - compact.put(key, out.get(key)); - } - } - if (!compact.isEmpty()) { - task.addOutput(joinOnRef, compact); - } - } - - if (!optionalTaskFailures.isEmpty()) { - task.setStatus(TaskModel.Status.COMPLETED_WITH_ERRORS); - optionalTaskFailures.append("completed with errors"); - task.setReasonForIncompletion(optionalTaskFailures.toString()); - } else { - task.setStatus(TaskModel.Status.COMPLETED); - } - return true; - } - - // Task execution not complete, waiting on more tasks to reach terminal state. - return false; - } - - @Override - public Optional getEvaluationOffset(TaskModel taskModel, long maxOffset) { - // Check if joinMode is set to SYNC — read directly from the workflow task definition - // rather than from input data so the value is never duplicated into the task's payload. - WorkflowTask workflowTask = taskModel.getWorkflowTask(); - if (workflowTask != null && WorkflowTask.JoinMode.SYNC == workflowTask.getJoinMode()) { - // Synchronous mode: evaluate immediately every time (no backoff) - return Optional.of(0L); - } - - // Asynchronous mode (default): use exponential backoff - int pollCount = taskModel.getPollCount(); - // Assuming pollInterval = 50ms and evaluationOffsetThreshold = 200 this will cause - // a JOIN task to be evaluated continuously during the first 10 seconds and the FORK/JOIN - // will end with minimal delay. - if (pollCount <= properties.getSystemTaskPostponeThreshold()) { - return Optional.of(0L); - } - - double exp = pollCount - properties.getSystemTaskPostponeThreshold(); - return Optional.of(Math.min((long) Math.pow(EVALUATION_OFFSET_BASE, exp), maxOffset)); - } - - public boolean isAsync() { - return true; - } -} diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/WorkflowClassifiers.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/WorkflowClassifiers.java index 7f07d14d..3199517f 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/WorkflowClassifiers.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/util/WorkflowClassifiers.java @@ -67,6 +67,9 @@ public static String classifierOf(Map metadata) { /** Returns {@code true} when the def resolves to the {@link #AGENT} classifier. */ public static boolean isAgent(Map metadata) { + if (metadata == null) { + return false; + } return AGENT.equalsIgnoreCase(classifierOf(metadata)); } } From 1e87bff99cca34b33a1760876923dee5fdcb5eea Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 22:57:32 -0700 Subject: [PATCH 7/9] Update AgentRuntime.java --- .../src/main/java/dev/agentspan/runtime/AgentRuntime.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java index 5940cd25..2b1d5e24 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java @@ -30,10 +30,7 @@ exclude = {DataSourceAutoConfiguration.class, MongoAutoConfiguration.class, MongoDataAutoConfiguration.class}) @EnableScheduling @ComponentScan( - // Conductor engine packages only — AgentSpan beans (dev.agentspan.runtime) are - // contributed by AgentSpanAutoConfiguration via the auto-configuration imports file. - basePackages = {"com.netflix.conductor", "io.orkes.conductor", "org.conductoross.conductor"}, - excludeFilters = @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, classes = Join.class)) + basePackages = {"com.netflix.conductor", "io.orkes.conductor", "org.conductoross.conductor"}) @RequiredArgsConstructor public class AgentRuntime implements ApplicationRunner { From d7ebc49ad5c7e376ef6eaa2a5a2d9b9f53651536 Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 22:58:44 -0700 Subject: [PATCH 8/9] Update AgentRuntime.java --- .../src/main/java/dev/agentspan/runtime/AgentRuntime.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java index 2b1d5e24..1ef9a03b 100644 --- a/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java +++ b/server/conductor-agentspan-server/src/main/java/dev/agentspan/runtime/AgentRuntime.java @@ -18,19 +18,15 @@ import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; import org.springframework.boot.autoconfigure.mongo.MongoAutoConfiguration; import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.FilterType; import org.springframework.core.env.Environment; import org.springframework.scheduling.annotation.EnableScheduling; -import com.netflix.conductor.core.execution.tasks.Join; - import lombok.RequiredArgsConstructor; @SpringBootApplication( exclude = {DataSourceAutoConfiguration.class, MongoAutoConfiguration.class, MongoDataAutoConfiguration.class}) @EnableScheduling -@ComponentScan( - basePackages = {"com.netflix.conductor", "io.orkes.conductor", "org.conductoross.conductor"}) +@ComponentScan(basePackages = {"com.netflix.conductor", "io.orkes.conductor", "org.conductoross.conductor"}) @RequiredArgsConstructor public class AgentRuntime implements ApplicationRunner { From 5bb803dd343cfdd9f3529de7240121e96b56695d Mon Sep 17 00:00:00 2001 From: Viren Baraiya Date: Sun, 12 Jul 2026 23:46:15 -0700 Subject: [PATCH 9/9] fixes --- .../runtime/service/AgentHumanTask.java | 8 +++++++- .../runtime/service/AgentHumanTaskConfig.java | 18 ++++++++++-------- server/gradle.properties | 2 +- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTask.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTask.java index 4d3e0711..85fbeadb 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTask.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTask.java @@ -30,7 +30,13 @@ * for system tasks. This override hooks into {@code start()} to emit the * SSE event directly.

    * - *

    Registered as a {@code @Primary} bean via {@link AgentHumanTaskConfig}.

    + *

    Registered as the {@code HUMAN} system task by {@link AgentHumanTaskConfig}, + * which is gated on {@code agentspan.embedded=true}. It is intentionally not a + * {@code @Component}: two beans named {@code HUMAN} (this one and Conductor's + * {@code Human}) would collide during component scanning, and both would land in + * {@code SystemTaskRegistry}'s taskType→task map as duplicate keys. The config's + * {@code @Bean("HUMAN")} instead overrides Conductor's default so exactly + * one {@code HUMAN} bean exists.

    */ public class AgentHumanTask extends WorkflowSystemTask { diff --git a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java index b25bacb7..548d69bb 100644 --- a/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java +++ b/server/conductor-agentspan/src/main/java/dev/agentspan/runtime/service/AgentHumanTaskConfig.java @@ -13,15 +13,17 @@ import org.springframework.context.annotation.Primary; /** - * Registers {@link AgentHumanTask} as the primary HUMAN task implementation, - * overriding Conductor's default {@code Human} system task. + * Registers {@link AgentHumanTask} as the {@code HUMAN} system task when running in + * embedded mode ({@code agentspan.embedded=true}). * - *

    Embedded mode: disabled when {@code agentspan.embedded=true} (e.g. when the - * library is imported into a host such as orkes-conductor). The host provides its own - * (richer) HUMAN task — overriding it here would collide on the {@code HUMAN} bean name and - * replace the host's full HITL implementation with this SSE-only shim. The host is expected - * to emit the {@code WAITING} SSE event from its own HUMAN task. The standalone OSS server - * leaves this property unset, so the override stays active as before.

    + *

    The {@code @Bean("HUMAN")} definition overrides Conductor's default + * {@code Human} component (enabled via {@code spring.main.allow-bean-definition-overriding=true}), + * so exactly one bean named {@code HUMAN} exists. This avoids the component-scan bean-name + * collision and the duplicate taskType key in {@code SystemTaskRegistry} that would otherwise + * result from having two {@code WorkflowSystemTask}s both reporting the {@code HUMAN} type.

    + * + *

    When {@code agentspan.embedded} is unset (the standalone OSS server), this configuration + * is skipped and Conductor's default {@code Human} task remains in effect.

    */ @Configuration @ConditionalOnProperty(name = "agentspan.embedded", havingValue = "true") diff --git a/server/gradle.properties b/server/gradle.properties index e96f4cf0..46557c60 100644 --- a/server/gradle.properties +++ b/server/gradle.properties @@ -3,4 +3,4 @@ org.gradle.jvmargs=-Xmx4g org.gradle.caching=true # Default published version; overridden in CI via -Pversion=X -version=0.4.4.rc2 \ No newline at end of file +version=0.4.4.rc6 \ No newline at end of file