fix(feature_flags): missing context key never satisfies a condition (#8429)

Author: dreamorosi

aws_lambda_powertools/utilities/feature_flags/feature_flags.py

@@ -85,10 +85,26 @@ def __init__(self, store: StoreProvider, logger: logging.Logger | Logger | None
         # recycled by a different object, which lets us safely skip re-validation on store cache hits.
         self._last_validated_config: dict | None = None
 
-    def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool:
+    def _match_by_action(
+        self,
+        action: str,
+        condition_value: Any,
+        context_value: Any,
+        context_key_present: bool = True,
+    ) -> bool:
         try:
             func = RULE_ACTION_MAPPING.get(action, lambda a, b: False)
-            return func(context_value, condition_value)
+            matched = func(context_value, condition_value)
+
+            if not context_key_present:
+                # A key absent from the context never satisfies a condition. Without this, `None` would be
+                # compared as a regular value and negative actions (NOT_EQUALS, NOT_IN, ...) would match.
+                # We still run the comparator first so that any exception it raises (e.g. ANY_IN_VALUE on a
+                # non-list) reaches registered validation exception handlers exactly as it did before.
+                self.logger.debug(f"context key not present, condition does not match: action={action}")
+                return False
+
+            return matched
         except Exception as exc:
             self.logger.debug(f"caught exception while matching action: action={action}, exception={str(exc)}")
 
@@ -118,19 +134,28 @@ def _evaluate_conditions(
             return False
 
         for condition in conditions:
-            context_value = context.get(condition.get(schema.CONDITION_KEY, ""))
+            cond_key = condition.get(schema.CONDITION_KEY, "")
             cond_action = condition.get(schema.CONDITION_ACTION, "")
             cond_value = condition.get(schema.CONDITION_VALUE)
+            context_key_present = True
 
             # time based rule actions have no user context. the context is the condition key
             if cond_action in (
                 schema.RuleAction.SCHEDULE_BETWEEN_TIME_RANGE.value,
                 schema.RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value,
                 schema.RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value,
             ):
-                context_value = condition.get(schema.CONDITION_KEY)  # e.g., CURRENT_TIME
-
-            if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value):
+                context_value = cond_key  # e.g., CURRENT_TIME
+            else:
+                context_key_present = cond_key in context
+                context_value = context.get(cond_key)
+
+            if not self._match_by_action(
+                action=cond_action,
+                condition_value=cond_value,
+                context_value=context_value,
+                context_key_present=context_key_present,
+            ):
                 self.logger.debug(
                     f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, "
                     f"name={feature_name}, context_value={str(context_value)} ",

docs/utilities/feature_flags.md

@@ -447,6 +447,8 @@ The `action` configuration can have the following values, where the expressions
 ???+ info
     The `key` and `value` will be compared to the input from the `context` parameter.
 
+    If a condition's `key` is not present in `context`, the condition never matches, regardless of the action. For example, a `NOT_EQUALS` rule on `tier` will not match a request that carries no `tier` at all.
+
 ???+ "Time based keys"
 
     For time based keys, we provide a list of predefined keys. These will automatically get converted to the corresponding timestamp on each invocation of your Lambda function.

tests/functional/feature_flags/_boto3/test_feature_flags.py

@@ -883,6 +883,71 @@ def test_flags_not_equal_match(mocker, config):
     assert toggle == expected_value
 
 
+@pytest.mark.parametrize(
+    "action, value",
+    [
+        (RuleAction.NOT_EQUALS.value, "premium"),
+        (RuleAction.NOT_IN.value, ["premium", "enterprise"]),
+        (RuleAction.KEY_NOT_IN_VALUE.value, ["premium", "enterprise"]),
+        (RuleAction.VALUE_NOT_IN_KEY.value, "premium"),
+    ],
+)
+def test_flags_negative_action_no_match_when_context_key_missing(mocker, config, action, value):
+    # GIVEN a rule with a negative action on key "tier"
+    mocked_app_config_schema = {
+        "my_feature": {
+            "default": False,
+            "rules": {
+                "non premium users": {
+                    "when_match": True,
+                    "conditions": [
+                        {
+                            "action": action,
+                            "key": "tier",
+                            "value": value,
+                        },
+                    ],
+                },
+            },
+        },
+    }
+    feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
+
+    # WHEN evaluating with a context that doesn't carry "tier" at all
+    toggle = feature_flags.evaluate(name="my_feature", context={"username": "a"}, default=False)
+
+    # THEN the rule must not match; a missing key never satisfies a condition
+    assert toggle is False
+
+
+def test_flags_not_equal_match_when_context_key_is_none(mocker, config):
+    # GIVEN a NOT_EQUALS rule on key "tier"
+    mocked_app_config_schema = {
+        "my_feature": {
+            "default": False,
+            "rules": {
+                "non premium users": {
+                    "when_match": True,
+                    "conditions": [
+                        {
+                            "action": RuleAction.NOT_EQUALS.value,
+                            "key": "tier",
+                            "value": "premium",
+                        },
+                    ],
+                },
+            },
+        },
+    }
+    feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
+
+    # WHEN the key is present but explicitly set to None
+    toggle = feature_flags.evaluate(name="my_feature", context={"tier": None}, default=False)
+
+    # THEN the value is still compared as usual (None != "premium"), so the rule matches
+    assert toggle is True
+
+
 # Test less than
 def test_flags_less_than_no_match_1(mocker, config):
     expected_value = False
@@ -1704,6 +1769,70 @@ def catch_exception(exc):
         )
 
 
+def test_flags_missing_context_key_still_invokes_validation_exception_handler(mocker, config):
+    # GIVEN an ANY_IN_VALUE rule and a handler registered for the ValueError the comparator raises
+    # when the context value is not a list
+    mocked_app_config_schema = {
+        "my_feature": {
+            "default": False,
+            "rules": {
+                "tenant_id is in allowed list": {
+                    "when_match": True,
+                    "conditions": [
+                        {
+                            "action": RuleAction.ANY_IN_VALUE.value,
+                            "key": "tenant_id",
+                            "value": ["Akua", "John", "Maria", "Pat"],
+                        },
+                    ],
+                },
+            },
+        },
+    }
+    feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
+    handled = []
+
+    @feature_flags.validation_exception_handler(ValueError)
+    def handle_invalid_context(exc):
+        handled.append(exc)
+        return True
+
+    # WHEN the context does not carry the key at all
+    toggle = feature_flags.evaluate(name="my_feature", context={}, default=False)
+
+    # THEN the handler is still called and its result is honoured, as it was before missing keys were guarded
+    assert len(handled) == 1
+    assert toggle is True
+
+
+def test_flags_missing_context_key_no_match_without_handler_for_raising_action(mocker, config):
+    # GIVEN an ANY_IN_VALUE rule and no exception handler registered
+    mocked_app_config_schema = {
+        "my_feature": {
+            "default": False,
+            "rules": {
+                "tenant_id is in allowed list": {
+                    "when_match": True,
+                    "conditions": [
+                        {
+                            "action": RuleAction.ANY_IN_VALUE.value,
+                            "key": "tenant_id",
+                            "value": ["Akua", "John", "Maria", "Pat"],
+                        },
+                    ],
+                },
+            },
+        },
+    }
+    feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
+
+    # WHEN the context does not carry the key at all
+    toggle = feature_flags.evaluate(name="my_feature", context={}, default=False)
+
+    # THEN the rule does not match
+    assert toggle is False
+
+
 # Test schema validation is performed once per fetched document (#8426)
 def test_schema_validated_once_for_cached_document(mocker, config):
     # GIVEN a store that serves the same document object on every call (e.g. a Parameters cache hit)