fix(security): replace insecure TrustManager with default JVM trust validation (#157)

* fix(security): replace insecure TrustManager with default JVM trust validation

HIGH-severity TLS validation bypass (CWE-295, code-scanning alert #8).

The previous HttpClientFactory honored AxonFlowConfig.insecureSkipVerify(true)
as a single-flag opt-in to a permissive X509TrustManager whose
checkClientTrusted / checkServerTrusted methods returned void and whose
getAcceptedIssuers returned an empty array. With the flag set, the SDK
trusted ANY server certificate over HTTPS, including attacker-presented
certs in MITM scenarios.

Fix: double-gate the insecure path so it activates only when BOTH

  - AxonFlowConfig#insecureSkipVerify(true) is set on the builder, AND
  - AXONFLOW_INSECURE_TLS is set to "true" (case-insensitive) or "1" in
    the runtime environment

are present. Otherwise the JVM's default TrustManager (validating against
the system + JDK trust store) is used. When the builder flag is set but
the env var is not, the SDK logs a warning at client construction time
and keeps verification enabled. When the insecure path actually
activates, a loud *** SECURITY WARNING *** is logged.

Rationale for keeping a development carve-out: AxonFlow demos and
self-hosted local stacks ship with self-signed agent certs in some
configurations; teams need a way to opt in for local dev without a
DIY trust-store override. The double-gate makes accidental production
bypass much harder: a stray builder flag in app code is no longer
sufficient.

Tests: HttpClientFactoryTest gains two assertions. The first verifies
that with insecureSkipVerify(true) but no env var, the resulting
OkHttpClient retains OkHttp's default OkHostnameVerifier (not the
permissive (h, s) -> true verifier). The second verifies the env-var
gate helper returns false when AXONFLOW_INSECURE_TLS is unset, which
is the default in CI and dev shells.

Default behavior in production: standard TLS validation against the
JDK + system trust store. No regressions in the existing 1224-test
suite (mvn test).

Resolves code-scanning alert #8.

* test(security): add positive-path test for insecure TLS double-gate

Code review on PR #157 caught that the existing tests only assert the
SAFE/DEFAULT path and the env-var helper's UNSET behaviour. They never
exercise the positive path where BOTH gates are present and the insecure
TrustManager actually activates — the one combination that ships a
trust-all SSLContext and permissive HostnameVerifier.

This commit adds four tests to close the gap:

- envVarHelperShouldBeTrueWhenSetToTrue / ...WhenSetToOne
  Confirm isInsecureTlsEnvVarEnabled() honours both accepted values.

- shouldActivateInsecurePathWhenBothGatesArePresent
  The positive-path regression. Builds an OkHttpClient with
  insecureSkipVerify(true) AND AXONFLOW_INSECURE_TLS=true, then asserts:
    1. hostnameVerifier class name does NOT contain "OkHostnameVerifier"
       (i.e. the synthetic permissive lambda is installed),
    2. that verifier returns true for an arbitrary hostname,
    3. sslSocketFactory() is NOT the same instance as the one a
       default-path client produces (trust-all SSLContext is wired in).

- shouldKeepDefaultPathWhenOnlyEnvVarIsSet
  Complementary negative path — env var alone, without the builder
  flag, must keep OkHttp's default OkHostnameVerifier. Together with
  the existing "neither flag set" test, this proves the gate is
  symmetric: both must be present to flip behaviour.

Env-var injection is done with junit-pioneer 2.2.0's
@SetEnvironmentVariable, which scopes the variable to a single test
method via reflective access into java.base/java.util. The required
JDK 17+ --add-opens flags are added to the surefire argLine alongside
the existing -Dnet.bytebuddy.experimental=true.

Full suite: 1228/1228 green.

Author: saurabhjain1592

CHANGELOG.md

@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **BREAKING (test API):** Package-private `TelemetryReporter.isEnabled` and `TelemetryReporter.sendPing` overloads no longer accept a `String doNotTrack` parameter. The remaining `String axonflowTelemetry` parameter is the sole opt-out signal accepted by the testability surface.
 
+### Security
+
+- **TLS verification bypass closed (CWE-295).** `HttpClientFactory` previously honored `insecureSkipVerify(true)` on `AxonFlowConfig` as a single-flag opt-in to a permissive `X509TrustManager` that accepted ANY server certificate, including attacker-presented certificates in MITM scenarios. The insecure path is now double-gated: it activates only if both `insecureSkipVerify(true)` is set on the builder AND the `AXONFLOW_INSECURE_TLS` environment variable is set to `true` (or `1`). When the builder flag is set without the env var, the SDK logs a warning and keeps the JVM's default `TrustManager` in place. A loud `*** SECURITY WARNING ***` is logged whenever the insecure path actually activates. Default behavior — and behavior in production environments without the env var — uses standard JDK + system trust-store validation. Resolves code-scanning alert #8.
+
 ### Fixed
 
 - The one-line `DO_NOT_TRACK=1 is deprecated...` `logger.warn` is no longer emitted. Removing the warning eliminates log noise that previously appeared on every telemetry decision when `DO_NOT_TRACK=1` was set.

pom.xml

@@ -57,6 +57,7 @@
 
         <!-- Test Dependency Versions -->
         <junit.version>5.10.2</junit.version>
+        <junit-pioneer.version>2.2.0</junit-pioneer.version>
         <mockito.version>5.11.0</mockito.version>
         <wiremock.version>3.4.2</wiremock.version>
         <assertj.version>3.27.7</assertj.version>
@@ -128,6 +129,13 @@
             <version>${junit.version}</version>
             <scope>test</scope>
         </dependency>
+        <!-- junit-pioneer: provides @SetEnvironmentVariable for in-process env-var injection -->
+        <dependency>
+            <groupId>org.junit-pioneer</groupId>
+            <artifactId>junit-pioneer</artifactId>
+            <version>${junit-pioneer.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
@@ -183,7 +191,8 @@
                 <artifactId>maven-surefire-plugin</artifactId>
                 <version>${maven-surefire-plugin.version}</version>
                 <configuration>
-                    <argLine>@{argLine} -Dnet.bytebuddy.experimental=true</argLine>
+                    <!-- add-opens flags below are required by junit-pioneer's @SetEnvironmentVariable on JDK 17+ -->
+                    <argLine>@{argLine} -Dnet.bytebuddy.experimental=true --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED</argLine>
                     <skipTests>${skipUnitTests}</skipTests>
                     <includes>
                         <include>**/*Test.java</include>

src/main/java/com/getaxonflow/sdk/AxonFlowConfig.java

@@ -463,11 +463,22 @@ public Builder debug(boolean debug) {
     }
 
     /**
-     * Skips SSL certificate verification.
+     * Requests that TLS certificate verification be skipped for outbound HTTPS calls.
      *
-     * <p><strong>Warning:</strong> Only use this in development/testing.
+     * <p><strong>Security:</strong> Setting this flag is <em>not by itself</em> sufficient to
+     * disable TLS verification. To actually disable verification, the environment variable {@code
+     * AXONFLOW_INSECURE_TLS} must <strong>also</strong> be set to {@code "true"} or {@code "1"} in
+     * the runtime environment. This double-gate is intentional defense-in-depth: a stray builder
+     * call in application code cannot silently bypass certificate validation in production.
      *
-     * @param insecureSkipVerify true to skip verification
+     * <p>If the builder flag is set but the environment variable is not, the SDK will log a
+     * warning at client construction time and keep TLS verification enabled.
+     *
+     * <p><strong>Use cases:</strong> local development against self-signed certificates only.
+     * <strong>Never</strong> enable in production.
+     *
+     * @param insecureSkipVerify true to request that verification be skipped (also requires
+     *     {@code AXONFLOW_INSECURE_TLS=true} environment variable)
      * @return this builder
      */
     public Builder insecureSkipVerify(boolean insecureSkipVerify) {

src/main/java/com/getaxonflow/sdk/util/HttpClientFactory.java

@@ -31,13 +31,27 @@ public final class HttpClientFactory {
 
   private static final Logger logger = LoggerFactory.getLogger(HttpClientFactory.class);
 
+  /**
+   * Environment variable that must be explicitly set to {@code "true"} (or {@code "1"}) to allow
+   * disabling TLS certificate verification, in addition to {@code insecureSkipVerify(true)} on the
+   * config builder. Both the builder flag AND this environment variable are required to activate
+   * the insecure path. This is intentional defense-in-depth: a stray builder flag in application
+   * code is not sufficient to silently disable TLS validation in production environments.
+   */
+  static final String INSECURE_TLS_ENV_VAR = "AXONFLOW_INSECURE_TLS";
+
   private HttpClientFactory() {
     // Utility class
   }
 
   /**
    * Creates an OkHttpClient configured according to the SDK configuration.
    *
+   * <p>By default, the client uses the JVM's default {@code TrustManager}, which validates server
+   * certificates against the system + JDK trust store. Certificate validation is only disabled if
+   * {@link AxonFlowConfig#isInsecureSkipVerify()} is {@code true} AND the environment variable
+   * {@value #INSECURE_TLS_ENV_VAR} is set to {@code "true"} or {@code "1"}.
+   *
    * @param config the SDK configuration
    * @return a configured OkHttpClient
    */
@@ -50,7 +64,17 @@ public static OkHttpClient create(AxonFlowConfig config) {
             .callTimeout(config.getTimeout().toMillis() * 2, TimeUnit.MILLISECONDS);
 
     if (config.isInsecureSkipVerify()) {
-      configureInsecureSsl(builder);
+      if (isInsecureTlsEnvVarEnabled()) {
+        configureInsecureSsl(builder);
+      } else {
+        logger.warn(
+            "insecureSkipVerify(true) was set on AxonFlowConfig but environment variable {} is "
+                + "not set to 'true' or '1'. TLS certificate verification REMAINS ENABLED. To "
+                + "disable verification (development/self-signed certs only), export {}=true. "
+                + "This double-gate is intentional to prevent accidental TLS bypass in production.",
+            INSECURE_TLS_ENV_VAR,
+            INSECURE_TLS_ENV_VAR);
+      }
     }
 
     if (config.isDebug()) {
@@ -111,11 +135,26 @@ public X509Certificate[] getAcceptedIssuers() {
           (hostname, session) -> true); // lgtm[java/insecure-hostname-verifier]
 
       logger.warn(
-          "SSL certificate verification is DISABLED (insecureSkipVerify=true). "
-              + "Do NOT use this in production. This is intended only for development environments "
-              + "with self-signed certificates.");
+          "*** SECURITY WARNING *** TLS certificate verification is DISABLED for AxonFlow SDK "
+              + "HTTP client. Both insecureSkipVerify(true) and {}=true were set. All HTTPS calls "
+              + "will accept ANY server certificate, including attacker-presented certificates in "
+              + "MITM scenarios. This MUST NOT be used in production. Intended only for local "
+              + "development against self-signed certificates.",
+          INSECURE_TLS_ENV_VAR);
     } catch (Exception e) {
       logger.error("Failed to configure insecure SSL", e);
     }
   }
+
+  /**
+   * Returns {@code true} if the {@value #INSECURE_TLS_ENV_VAR} environment variable is set to
+   * {@code "true"} (case-insensitive) or {@code "1"}. Package-private to allow test verification.
+   */
+  static boolean isInsecureTlsEnvVarEnabled() {
+    String value = System.getenv(INSECURE_TLS_ENV_VAR);
+    if (value == null) {
+      return false;
+    }
+    return "true".equalsIgnoreCase(value) || "1".equals(value);
+  }
 }

src/test/java/com/getaxonflow/sdk/util/HttpClientFactoryTest.java

@@ -22,6 +22,7 @@
 import okhttp3.OkHttpClient;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetEnvironmentVariable;
 
 @DisplayName("HttpClientFactory")
 class HttpClientFactoryTest {
@@ -70,15 +71,111 @@ void shouldCreateClientWithDebugMode() {
   }
 
   @Test
-  @DisplayName("should create client with insecure skip verify")
-  void shouldCreateClientWithInsecureSkipVerify() {
+  @DisplayName(
+      "should NOT disable TLS verification when insecureSkipVerify=true but env var is unset")
+  void shouldKeepTlsVerificationEnabledWithoutEnvVar() {
+    // Builder flag alone is not enough — AXONFLOW_INSECURE_TLS env var must also be set.
+    // We don't set the env var in this test, so the insecure path must remain inactive.
     AxonFlowConfig config =
         AxonFlowConfig.builder().agentUrl("http://localhost:8080").insecureSkipVerify(true).build();
 
     OkHttpClient client = HttpClientFactory.create(config);
 
     assertThat(client).isNotNull();
-    // Should have a custom hostname verifier that accepts all hosts
-    assertThat(client.hostnameVerifier()).isNotNull();
+    // The default OkHttp hostname verifier (OkHostnameVerifier) should be in place — NOT the
+    // permissive (hostname, session) -> true verifier. We assert the default class name to ensure
+    // we did not silently install the insecure verifier.
+    assertThat(client.hostnameVerifier().getClass().getName())
+        .as("default hostname verifier should be in place when env var is unset")
+        .contains("OkHostnameVerifier");
+  }
+
+  @Test
+  @DisplayName("env-var gate helper should be false when AXONFLOW_INSECURE_TLS is unset")
+  void envVarHelperShouldBeFalseWhenUnset() {
+    // We cannot reliably set env vars in-process across JVMs, but we can assert the helper's
+    // behaviour against the real environment, which is unset in CI and dev shells by default.
+    assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
+        .as(
+            "AXONFLOW_INSECURE_TLS must default to false; if this fails, the test environment "
+                + "has the env var set and the insecure path could activate")
+        .isFalse();
+  }
+
+  @Test
+  @DisplayName("env-var gate helper should be true when AXONFLOW_INSECURE_TLS=true")
+  @SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
+  void envVarHelperShouldBeTrueWhenSetToTrue() {
+    assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
+        .as("helper must return true when env var is set to 'true'")
+        .isTrue();
+  }
+
+  @Test
+  @DisplayName("env-var gate helper should be true when AXONFLOW_INSECURE_TLS=1")
+  @SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "1")
+  void envVarHelperShouldBeTrueWhenSetToOne() {
+    assertThat(HttpClientFactory.isInsecureTlsEnvVarEnabled())
+        .as("helper must return true when env var is set to '1'")
+        .isTrue();
+  }
+
+  @Test
+  @DisplayName(
+      "should activate insecure TLS path when BOTH insecureSkipVerify(true) AND env var are set")
+  @SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
+  void shouldActivateInsecurePathWhenBothGatesArePresent() {
+    // Positive-path regression: this is the single combination where the trust-all path
+    // is intentionally activated. Both gates MUST be required; either alone is insufficient
+    // (covered by sibling tests). If either gate stops being required, this test will still
+    // pass — the gating behaviour is asserted in the negative-path tests.
+    AxonFlowConfig config =
+        AxonFlowConfig.builder().agentUrl("https://localhost:8080").insecureSkipVerify(true).build();
+
+    OkHttpClient client = HttpClientFactory.create(config);
+
+    assertThat(client).isNotNull();
+
+    // 1. Hostname verifier MUST be the permissive lambda, NOT OkHttp's default OkHostnameVerifier.
+    //    The permissive verifier in HttpClientFactory is `(hostname, session) -> true`, which is
+    //    a synthetic lambda class — its name will NOT contain "OkHostnameVerifier".
+    assertThat(client.hostnameVerifier().getClass().getName())
+        .as("permissive hostname verifier must be installed when both gates are set")
+        .doesNotContain("OkHostnameVerifier");
+
+    // 2. Functional check: the verifier must accept any hostname/session pair.
+    assertThat(client.hostnameVerifier().verify("any.host.example", null))
+        .as("permissive hostname verifier must return true for arbitrary hostnames")
+        .isTrue();
+
+    // 3. SSL socket factory must be the trust-all-configured one (i.e. NOT the JVM default
+    //    that OkHttp lazily constructs from the system trust store). We assert non-null and
+    //    that calling sslSocketFactory() does not trigger OkHttp's default construction path
+    //    by comparing against a freshly built default client.
+    AxonFlowConfig defaultConfig =
+        AxonFlowConfig.builder().agentUrl("https://localhost:8080").build();
+    OkHttpClient defaultClient = HttpClientFactory.create(defaultConfig);
+    assertThat(client.sslSocketFactory())
+        .as("insecure-path SSL socket factory must differ from the default-path factory")
+        .isNotSameAs(defaultClient.sslSocketFactory());
+  }
+
+  @Test
+  @DisplayName(
+      "should NOT activate insecure TLS path when env var is set but builder flag is false")
+  @SetEnvironmentVariable(key = "AXONFLOW_INSECURE_TLS", value = "true")
+  void shouldKeepDefaultPathWhenOnlyEnvVarIsSet() {
+    // Complementary negative path: env var alone, without insecureSkipVerify(true), must
+    // keep the default safe TrustManager and OkHostnameVerifier. The double-gate is symmetric:
+    // BOTH must be present — neither alone activates the insecure path.
+    AxonFlowConfig config =
+        AxonFlowConfig.builder().agentUrl("https://localhost:8080").build();
+
+    OkHttpClient client = HttpClientFactory.create(config);
+
+    assertThat(client).isNotNull();
+    assertThat(client.hostnameVerifier().getClass().getName())
+        .as("default hostname verifier must remain when only env var is set")
+        .contains("OkHostnameVerifier");
   }
 }