Skip to content

Commit 80e50af

Browse files
committed
fix(feature_flags): missing context key never satisfies a condition
Conditions read the context with context.get(key), so an absent key was compared as None. Negative actions (NOT_EQUALS, NOT_IN, KEY_NOT_IN_VALUE, VALUE_NOT_IN_KEY) therefore matched requests that carried no such key at all, firing rules meant for one segment on anonymous or partially populated traffic. This is a regression from ab9078c (#2052). Until v2.11.0 _match_by_action returned False for any falsy context value, so a missing key never matched. That guard was removed to allow falsy values such as 0 and "" to be compared, and allowing absent keys to match was an unintended side effect. Restore the original semantics for missing keys by checking key presence before invoking the comparator, while keeping the #2051 behaviour for keys that are present with a falsy or None value. Time based actions are unaffected since they never read the user context. Document the behaviour in the rule actions section. Fixes #8424
1 parent af87f36 commit 80e50af

3 files changed

Lines changed: 79 additions & 2 deletions

File tree

‎aws_lambda_powertools/utilities/feature_flags/feature_flags.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def _evaluate_conditions(
115115
return False
116116

117117
for condition in conditions:
118-
context_value = context.get(condition.get(schema.CONDITION_KEY, ""))
118+
cond_key = condition.get(schema.CONDITION_KEY, "")
119119
cond_action = condition.get(schema.CONDITION_ACTION, "")
120120
cond_value = condition.get(schema.CONDITION_VALUE)
121121

@@ -125,7 +125,17 @@ def _evaluate_conditions(
125125
schema.RuleAction.SCHEDULE_BETWEEN_DATETIME_RANGE.value,
126126
schema.RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value,
127127
):
128-
context_value = condition.get(schema.CONDITION_KEY) # e.g., CURRENT_TIME
128+
context_value = cond_key # e.g., CURRENT_TIME
129+
elif cond_key not in context:
130+
# A key absent from the context never satisfies a condition. Without this guard, `None`
131+
# would be compared as a regular value and negative actions (NOT_EQUALS, NOT_IN, ...) would match.
132+
self.logger.debug(
133+
f"rule did not match, context key not found, rule_name={rule_name}, "
134+
f"rule_value={rule_match_value}, name={feature_name}, key={cond_key}",
135+
)
136+
return False
137+
else:
138+
context_value = context[cond_key]
129139

130140
if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value):
131141
self.logger.debug(

‎docs/utilities/feature_flags.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,8 @@ The `action` configuration can have the following values, where the expressions
444444
???+ info
445445
The `key` and `value` will be compared to the input from the `context` parameter.
446446

447+
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.
448+
447449
???+ "Time based keys"
448450

449451
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‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,71 @@ def test_flags_not_equal_match(mocker, config):
883883
assert toggle == expected_value
884884

885885

886+
@pytest.mark.parametrize(
887+
"action, value",
888+
[
889+
(RuleAction.NOT_EQUALS.value, "premium"),
890+
(RuleAction.NOT_IN.value, ["premium", "enterprise"]),
891+
(RuleAction.KEY_NOT_IN_VALUE.value, ["premium", "enterprise"]),
892+
(RuleAction.VALUE_NOT_IN_KEY.value, "premium"),
893+
],
894+
)
895+
def test_flags_negative_action_no_match_when_context_key_missing(mocker, config, action, value):
896+
# GIVEN a rule with a negative action on key "tier"
897+
mocked_app_config_schema = {
898+
"my_feature": {
899+
"default": False,
900+
"rules": {
901+
"non premium users": {
902+
"when_match": True,
903+
"conditions": [
904+
{
905+
"action": action,
906+
"key": "tier",
907+
"value": value,
908+
},
909+
],
910+
},
911+
},
912+
},
913+
}
914+
feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
915+
916+
# WHEN evaluating with a context that doesn't carry "tier" at all
917+
toggle = feature_flags.evaluate(name="my_feature", context={"username": "a"}, default=False)
918+
919+
# THEN the rule must not match; a missing key never satisfies a condition
920+
assert toggle is False
921+
922+
923+
def test_flags_not_equal_match_when_context_key_is_none(mocker, config):
924+
# GIVEN a NOT_EQUALS rule on key "tier"
925+
mocked_app_config_schema = {
926+
"my_feature": {
927+
"default": False,
928+
"rules": {
929+
"non premium users": {
930+
"when_match": True,
931+
"conditions": [
932+
{
933+
"action": RuleAction.NOT_EQUALS.value,
934+
"key": "tier",
935+
"value": "premium",
936+
},
937+
],
938+
},
939+
},
940+
},
941+
}
942+
feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
943+
944+
# WHEN the key is present but explicitly set to None
945+
toggle = feature_flags.evaluate(name="my_feature", context={"tier": None}, default=False)
946+
947+
# THEN the value is still compared as usual (None != "premium"), so the rule matches
948+
assert toggle is True
949+
950+
886951
# Test less than
887952
def test_flags_less_than_no_match_1(mocker, config):
888953
expected_value = False

0 commit comments

Comments
 (0)