fix: Escape attribute names reported in redactedAttributes (#505)

Author: jsonbailey

ldclient/impl/events/event_context_formatter.py

@@ -76,11 +76,11 @@ def _format_context_single(self, context: Context, include_kind: bool, redact_an
 
     def _check_whole_attr_private(self, attr: str, all_private: List[AttributeRef], redacted: List[str], redact_all: bool) -> bool:
         if self._all_attributes_private or redact_all:
-            redacted.append(attr)
+            redacted.append(AttributeRef.from_literal(attr).path)
             return True
         for p in all_private:
             if p.depth == 1 and p[0] == attr:
-                redacted.append(attr)
+                redacted.append(AttributeRef.from_literal(attr).path)
                 return True
         return False
 

ldclient/impl/model/attribute_ref.py

@@ -77,7 +77,13 @@ def from_path(path: str) -> AttributeRef:
     def from_literal(name: str) -> AttributeRef:
         if name == '':
             return AttributeRef._from_error(AttributeRef._ERR_EMPTY)
-        return AttributeRef(AttributeRef._escape(name), name, None, None)
+        if name[0] != '/':
+            # A name that does not start with a slash is already a valid
+            # reference to a top-level attribute. It needs no escaping.
+            return AttributeRef(name, name, None, None)
+        # A name that starts with a slash must be escaped. If it is not, a
+        # consumer reads it as a path to a nested property.
+        return AttributeRef('/' + AttributeRef._escape(name), name, None, None)
 
     @staticmethod
     def _from_error(error: str) -> AttributeRef:

ldclient/testing/impl/events/test_event_context_formatter.py

@@ -61,3 +61,21 @@ def test_private_property_in_object():
     f = EventContextFormatter(False, ['/b/prop1', '/c/prop2/sub1'])
     c = Context.builder('a').set('b', {'prop1': True, 'prop2': 3}).set('c', {'prop1': {'sub1': True}, 'prop2': {'sub1': 4, 'sub2': 5}}).build()
     assert f.format_context(c) == {'kind': 'user', 'key': 'a', 'b': {'prop2': 3}, 'c': {'prop1': {'sub1': True}, 'prop2': {'sub2': 5}}, '_meta': {'redactedAttributes': ['/b/prop1', '/c/prop2/sub1']}}
+
+
+def test_all_private_reports_escaped_references():
+    f = EventContextFormatter(True, [])
+    c = Context.builder('a').set('/ssn', '123-45-6789').set('/a~b', 'secret').set('c/d~e', 'plain').build()
+    assert f.format_context(c) == {'kind': 'user', 'key': 'a', '_meta': {'redactedAttributes': ['/~1ssn', '/~1a~0b', 'c/d~e']}}
+
+
+def test_redact_anonymous_reports_escaped_references():
+    f = EventContextFormatter(False, [])
+    c = Context.builder('a').name('b').anonymous(True).set('/ssn', '123-45-6789').build()
+    assert f.format_context_redact_anonymous(c) == {'kind': 'user', 'key': 'a', 'anonymous': True, '_meta': {'redactedAttributes': ['name', '/~1ssn']}}
+
+
+def test_private_slash_prefixed_attribute_reports_escaped_reference():
+    f = EventContextFormatter(False, ['/~1ssn'])
+    c = Context.builder('a').name('b').set('/ssn', '123-45-6789').build()
+    assert f.format_context(c) == {'kind': 'user', 'key': 'a', 'name': 'b', '_meta': {'redactedAttributes': ['/~1ssn']}}

ldclient/testing/impl/test_attribute_ref.py

@@ -42,6 +42,25 @@ def test_literal(self, input: str):
         assert a.depth == 1
         assert a[0] == input
 
+    @pytest.mark.parametrize(
+        "input,expected_path",
+        [
+            ("name", "name"),
+            ("name/with/slashes", "name/with/slashes"),
+            ("a/b~c", "a/b~c"),
+            ("/ssn", "/~1ssn"),
+            ("/a~b", "/~1a~0b"),
+        ],
+    )
+    def test_literal_path_escapes_a_leading_slash(self, input: str, expected_path: str):
+        # A name that starts with a slash must be escaped, so that a consumer
+        # does not read it as a path to a nested property. Any other name is
+        # already a valid reference and stays unchanged.
+        a = AttributeRef.from_literal(input)
+        assert a.path == expected_path
+        assert AttributeRef.from_path(a.path).path == expected_path
+        assert AttributeRef.from_path(a.path)[0] == input
+
     def test_get_component(self):
         a = AttributeRef.from_path("/first/sec~1ond/third")
         assert a.depth == 3