fix(openfeature): isolate malformed flag configuration

dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy

@@ -206,7 +206,10 @@ class OpenFeatureProviderSmokeTest extends AbstractServerSmokeTest {
   private static Map<String, Boolean> buildLoggedAllocations(final Map<String, Object> config) {
     final logged = [:]
     (config.flags as Map<String, Object>).each { flag, definition ->
-      (definition.allocations ?: []).each { allocation ->
+      if (!(definition instanceof Map) || !(definition.allocations instanceof List)) {
+        return
+      }
+      definition.allocations.findAll { it instanceof Map }.each { allocation ->
         logged["${flag}\u0000${allocation.key}"] = allocation.doLog == true
       }
     }

products/feature-flagging/feature-flagging-api/build.gradle.kts

@@ -49,6 +49,7 @@ dependencies {
   compileOnly("io.opentelemetry:opentelemetry-api:1.47.0")
 
   testImplementation(project(":products:feature-flagging:feature-flagging-bootstrap"))
+  testImplementation(project(":products:feature-flagging:feature-flagging-lib"))
   testImplementation(project(":utils:config-utils"))
   testImplementation("io.opentelemetry:opentelemetry-api:1.47.0")
   testImplementation(libs.bundles.junit5)

products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java

@@ -9,7 +9,10 @@
 import datadog.trace.api.featureflag.ufc.v1.ConditionConfiguration;
 import datadog.trace.api.featureflag.ufc.v1.ConditionOperator;
 import datadog.trace.api.featureflag.ufc.v1.Flag;
+import datadog.trace.api.featureflag.ufc.v1.FlagMap;
+import datadog.trace.api.featureflag.ufc.v1.FlagValidator;
 import datadog.trace.api.featureflag.ufc.v1.Rule;
+import datadog.trace.api.featureflag.ufc.v1.SemanticVersion;
 import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration;
 import datadog.trace.api.featureflag.ufc.v1.Shard;
 import datadog.trace.api.featureflag.ufc.v1.ShardRange;
@@ -111,6 +114,10 @@ public <T> ProviderEvaluation<T> evaluate(
 
       final Flag flag = config.flags.get(key);
       if (flag == null) {
+        if (config.flags instanceof FlagMap && ((FlagMap) config.flags).isRejected(key)) {
+          return error(
+              defaultValue, ErrorCode.PARSE_ERROR, "Invalid configuration for flag " + key);
+        }
         return error(defaultValue, ErrorCode.FLAG_NOT_FOUND);
       }
 
@@ -275,6 +282,13 @@ private static boolean evaluateCondition(
         return compareNumber(attributeValue, condition.value, (a, b) -> a <= b);
       case LT:
         return compareNumber(attributeValue, condition.value, (a, b) -> a < b);
+      case SEMVER_EQ:
+      case SEMVER_NEQ:
+      case SEMVER_LT:
+      case SEMVER_LTE:
+      case SEMVER_GT:
+      case SEMVER_GTE:
+        return matchesSemanticVersion(condition.operator, attributeValue, condition.value);
       default:
         return false;
     }
@@ -283,7 +297,7 @@ private static boolean evaluateCondition(
   private static boolean matchesRegex(final Object attributeValue, final Object conditionValue) {
     // PatternSyntaxException is intentionally not caught here so it propagates to evaluate(),
     // which maps it to ErrorCode.PARSE_ERROR.
-    final Pattern pattern = Pattern.compile(String.valueOf(conditionValue));
+    final Pattern pattern = FlagValidator.compileRegex(String.valueOf(conditionValue));
     return pattern.matcher(String.valueOf(attributeValue)).find();
   }
 
@@ -318,6 +332,33 @@ private static boolean compareNumber(
     return comparator.compare(a, b);
   }
 
+  private static boolean matchesSemanticVersion(
+      final ConditionOperator operator, final Object attributeValue, final Object conditionValue) {
+    final int comparison;
+    try {
+      comparison =
+          SemanticVersion.parse(attributeValue).compareTo(SemanticVersion.parse(conditionValue));
+    } catch (IllegalArgumentException ignored) {
+      return false;
+    }
+    switch (operator) {
+      case SEMVER_EQ:
+        return comparison == 0;
+      case SEMVER_NEQ:
+        return comparison != 0;
+      case SEMVER_LT:
+        return comparison < 0;
+      case SEMVER_LTE:
+        return comparison <= 0;
+      case SEMVER_GT:
+        return comparison > 0;
+      case SEMVER_GTE:
+        return comparison >= 0;
+      default:
+        return false;
+    }
+  }
+
   private static boolean matchesShard(final Shard shard, final String targetingKey) {
     final int assignedShard = getShard(shard.salt, targetingKey, shard.totalShards);
     for (final ShardRange range : shard.ranges) {
@@ -417,10 +458,7 @@ private static <T> ProviderEvaluation<T> resolveVariant(
     final ProviderEvaluation<T> result =
         ProviderEvaluation.<T>builder()
             .value(mappedValue)
-            .reason(
-                !isEmpty(allocation.rules)
-                    ? Reason.TARGETING_MATCH.name()
-                    : !isEmpty(split.shards) ? Reason.SPLIT.name() : Reason.STATIC.name())
+            .reason(assignmentReason(allocation, split))
             .variant(variant.key)
             .flagMetadata(metadataBuilder.build())
             .build();
@@ -431,6 +469,20 @@ private static <T> ProviderEvaluation<T> resolveVariant(
     return result;
   }
 
+  private static String assignmentReason(final Allocation allocation, final Split split) {
+    if (!isEmpty(allocation.rules)) {
+      return Reason.TARGETING_MATCH.name();
+    }
+    if ((allocation.startAtInstant() != null || allocation.endAtInstant() != null)
+        && allocation.splits.size() == 1
+        && isEmpty(split.shards)) {
+      return Reason.DEFAULT.name();
+    }
+    return allocation.splits.size() == 1 && isEmpty(split.shards)
+        ? Reason.STATIC.name()
+        : Reason.SPLIT.name();
+  }
+
   private static Object resolveAttribute(final String name, final EvaluationContext context) {
     // Special handling for "id" attribute: if not explicitly provided, use targeting key
     if ("id".equals(name) && !context.keySet().contains(name)) {

products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java

@@ -17,15 +17,15 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+import com.datadog.featureflag.UniversalFlagConfigParser;
 import com.squareup.moshi.JsonAdapter;
 import com.squareup.moshi.JsonDataException;
-import com.squareup.moshi.JsonReader;
-import com.squareup.moshi.JsonWriter;
 import com.squareup.moshi.Moshi;
 import com.squareup.moshi.Types;
 import datadog.trace.api.featureflag.FeatureFlaggingGateway;
 import datadog.trace.api.featureflag.ufc.v1.Allocation;
 import datadog.trace.api.featureflag.ufc.v1.Flag;
+import datadog.trace.api.featureflag.ufc.v1.FlagMap;
 import datadog.trace.api.featureflag.ufc.v1.ServerConfiguration;
 import dev.openfeature.sdk.ErrorCode;
 import dev.openfeature.sdk.EvaluationContext;
@@ -39,7 +39,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Instant;
-import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
@@ -59,9 +58,7 @@ public class DDEvaluatorTest {
 
   private static final String CANONICAL_FIXTURE_PATH =
       "dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data";
-  private static final Moshi MOSHI = new Moshi.Builder().add(Date.class, new DateAdapter()).build();
-  private static final JsonAdapter<ServerConfiguration> CONFIG_ADAPTER =
-      MOSHI.adapter(ServerConfiguration.class);
+  private static final Moshi MOSHI = new Moshi.Builder().build();
   private static final Type FIXTURE_LIST_TYPE =
       Types.newParameterizedType(List.class, FixtureCase.class);
   private static final JsonAdapter<List<FixtureCase>> FIXTURE_LIST_ADAPTER =
@@ -214,6 +211,24 @@ public void testNoAllocations() {
     assertThat(details.getErrorCode(), nullValue());
   }
 
+  @Test
+  public void testRejectedFlagsAreScopedToCurrentConfiguration() {
+    final FlagMap rejectedFlags = new FlagMap();
+    rejectedFlags.reject("target");
+    final DDEvaluator evaluator = new DDEvaluator(mock(Runnable.class));
+    final EvaluationContext context = new MutableContext("target");
+
+    evaluator.accept(new ServerConfiguration("", "", null, rejectedFlags));
+    ProviderEvaluation<?> details = evaluator.evaluate(String.class, "target", "default", context);
+    assertThat(details.getReason(), equalTo(ERROR.name()));
+    assertThat(details.getErrorCode(), equalTo(ErrorCode.PARSE_ERROR));
+
+    evaluator.accept(new ServerConfiguration("", "", null, new FlagMap()));
+    details = evaluator.evaluate(String.class, "target", "default", context);
+    assertThat(details.getReason(), equalTo(ERROR.name()));
+    assertThat(details.getErrorCode(), equalTo(ErrorCode.FLAG_NOT_FOUND));
+  }
+
   @Test
   public void testAllocationDateAbiAndInstantAccessors() throws Exception {
     final Date startAt = Date.from(Instant.parse("2024-01-01T00:00:00Z"));
@@ -319,7 +334,8 @@ private static ProviderEvaluation<?> evaluate(
   }
 
   private static ServerConfiguration loadCanonicalConfiguration() throws IOException {
-    return CONFIG_ADAPTER.fromJson(read(fixtureRoot().resolve("ufc-config.json")));
+    return UniversalFlagConfigParser.INSTANCE.deserialize(
+        read(fixtureRoot().resolve("ufc-config.json")).getBytes(StandardCharsets.UTF_8));
   }
 
   private static List<FixtureCase> canonicalTestCases() throws IOException {
@@ -433,28 +449,4 @@ private static final class FixtureResult {
     String variant;
     Map<String, Object> flagMetadata = emptyMap();
   }
-
-  private static final class DateAdapter extends JsonAdapter<Date> {
-    @Override
-    public Date fromJson(final JsonReader reader) throws IOException {
-      if (reader.peek() == JsonReader.Token.NULL) {
-        return reader.nextNull();
-      }
-      try {
-        return Date.from(
-            DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(reader.nextString(), Instant::from));
-      } catch (final Exception ignored) {
-        return null;
-      }
-    }
-
-    @Override
-    public void toJson(final JsonWriter writer, final Date value) throws IOException {
-      if (value == null) {
-        writer.nullValue();
-        return;
-      }
-      writer.value(value.toInstant().toString());
-    }
-  }
 }

products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/ConditionOperator.java

@@ -9,5 +9,11 @@ public enum ConditionOperator {
   NOT_MATCHES,
   ONE_OF,
   NOT_ONE_OF,
-  IS_NULL
+  IS_NULL,
+  SEMVER_EQ,
+  SEMVER_NEQ,
+  SEMVER_LT,
+  SEMVER_LTE,
+  SEMVER_GT,
+  SEMVER_GTE
 }

products/feature-flagging/feature-flagging-bootstrap/src/main/java/datadog/trace/api/featureflag/ufc/v1/FlagMap.java

@@ -0,0 +1,17 @@
+package datadog.trace.api.featureflag.ufc.v1;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Set;
+
+public final class FlagMap extends HashMap<String, Flag> {
+  private final Set<String> rejected = new HashSet<>();
+
+  public void reject(final String key) {
+    rejected.add(key);
+  }
+
+  public boolean isRejected(final String key) {
+    return rejected.contains(key);
+  }
+}