diff --git a/.sampo/changesets/mask-long-strings-and-dict-keys.md b/.sampo/changesets/mask-long-strings-and-dict-keys.md new file mode 100644 index 000000000..ef666c4bc --- /dev/null +++ b/.sampo/changesets/mask-long-strings-and-dict-keys.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Code variable masking now searches strings of up to 2,048 characters for known credential formats, not only strings of up to 200 characters. A key inside a longer string, such as a SQL query that inlines an access key, is now redacted. Dict keys are now masked too. A key is replaced with a `$$_posthog_redacted_key__$$` placeholder when it matches a mask pattern but is not a plain field name, when it looks like a secret, when a non-string key holds a part that masking redacts or can't check, when its text can't be read, or when it is too long to scan. URL credentials are removed from string keys, and keys that end up with the same text keep separate entries. diff --git a/posthog/exception_utils.py b/posthog/exception_utils.py index 37b0f5f20..546c977a5 100644 --- a/posthog/exception_utils.py +++ b/posthog/exception_utils.py @@ -1144,7 +1144,9 @@ def build( # Well-known credential formats, matched regardless of entropy. High-confidence, # distinctive-prefix patterns adapted from the gitleaks / detect-secrets rule sets. _KNOWN_SECRET_PATTERNS = [ - # AI / LLM providers + # AI / LLM providers. The `sk-` prefix is not anchored to a word boundary, because a + # key often follows a letter or digit, e.g. `%3Dsk-...` in a percent-encoded URL or + # `\nsk-...` in escaped text. This over-redacts words such as `disk-usage-...`. r"sk-ant-[A-Za-z0-9_-]{16,}", # Anthropic r"sk-(?:proj-)?[A-Za-z0-9_-]{20,}", # OpenAI r"hf_[A-Za-z0-9]{34}", # Hugging Face @@ -1185,7 +1187,9 @@ def build( _KNOWN_SECRET_RE = re.compile("|".join(_KNOWN_SECRET_PATTERNS)) _PEM_PRIVATE_KEY_MARKER = "PRIVATE KEY-----" # covers RSA/EC/OpenSSH/PKCS8 -_KNOWN_SECRET_MAX_SCAN_LENGTH = 200 +# Same cap as the other pattern checks, so a key embedded in a longer string (a SQL query +# with credentials, a config dump) is still found. +_KNOWN_SECRET_MAX_SCAN_LENGTH = _MAX_VALUE_LENGTH_FOR_PATTERN_MATCH def _looks_like_path_or_url(value): @@ -1370,18 +1374,98 @@ def _masked_type_members(value, config): return masked +# A mapping can have several redacted keys, so each placeholder carries a number to +# keep the keys unique. +_REDACTED_KEY_TEMPLATE = "$$_posthog_redacted_key_{}_$$" + +# A string key that matches a mask pattern is kept only when it has this shape. Text with +# other characters, such as `password=hunter2` or a SQL query, can hold the value itself. +_FIELD_NAME_RE = re.compile(r"[\w.\-]+") + +_CIRCULAR_REF_VALUE = "" + +# Markers that show a key probe could not vouch for every part of the key. +_KEY_PROBE_MARKERS = ( + CODE_VARIABLES_REDACTED_VALUE, + CODE_VARIABLES_TOO_LONG_VALUE, + _CIRCULAR_REF_VALUE, +) + + +def _redacted_key(result): + """Return a placeholder key that no key already in ``result`` uses.""" + n = 0 + while (candidate := _REDACTED_KEY_TEMPLATE.format(n)) in result: + n += 1 + return candidate + + +def _is_field_name(key, key_is_json_safe): + """True when a key that matches a mask pattern names a field, so it is safe to keep. + A number or None can't hold a credential, and a string must look like an identifier. + Any other key reaches the output as a repr, which can embed field values.""" + if not key_is_json_safe: + return False + if isinstance(key, str): + return _FIELD_NAME_RE.fullmatch(key) is not None + return True + + +class _KeyProbeSeen: + """The ``seen`` set for a key probe. The probe stops at every object that the + traversal or an earlier probe visited, and its visits count against the same node + budget. It records its visits under a separate tag, so an object that a key shares + with a value is still masked in full where the value holds it.""" + + _TAG = "key_probe" + + def __init__(self, seen): + self._seen = seen + + def __contains__(self, obj_id): + return obj_id in self._seen or (self._TAG, obj_id) in self._seen + + def add(self, obj_id): + self._seen.add((self._TAG, obj_id)) + + def __len__(self): + return len(self._seen) + + +def _key_parts_fail_masking(key, config, seen, depth): + """True when masking ``key`` as a value redacts any part of it, reaches an object + that the traversal already visited, or raises. The quotes and brackets of a repr turn + off the entropy check, so the parts are checked one by one. A part that was already + visited is not checked again, so its text in the key's repr can't be vouched for.""" + probe_seen = seen if isinstance(seen, _KeyProbeSeen) else _KeyProbeSeen(seen) + try: + rendered = str(_mask_value(key, config, probe_seen, depth + 1)) + except Exception: + return True + return any(marker in rendered for marker in _KEY_PROBE_MARKERS) + + def _mask_mapping(items, config, seen, depth): """Mask a sequence of ``(key, value)`` pairs into a dict. A key matching the mask redacts its value; surviving values recurse through ``_mask_value``. Keys are kept - JSON-serializable.""" - result = {} + JSON-serializable, and a key whose own text can hold a secret is replaced by a + placeholder, because the key text reaches the output as-is.""" + result: Dict[Any, Any] = {} for key, value in items: if type(key) is str: out_key = key_str = key + key_is_json_safe = True else: - key_str = key if isinstance(key, str) else str(key) + try: + key_str = key if isinstance(key, str) else str(key) + except Exception: + # Without the key text, nothing shows whether the key names a secret, so + # the value is redacted too. + result[_redacted_key(result)] = CODE_VARIABLES_REDACTED_VALUE + continue # json.dumps only accepts str/int/float/bool/None keys; coerce anything else to - # its string form so one exotic key can't make json.dumps fail. + # its string form so one exotic key can't make json.dumps fail. That string form + # is a repr, which can embed field values, not only a name. key_is_json_safe = ( key is None or isinstance(key, (str, int)) # bool is an int subclass @@ -1389,8 +1473,25 @@ def _mask_mapping(items, config, seen, depth): ) out_key = key if key_is_json_safe else key_str if len(key_str) > _MAX_VALUE_LENGTH_FOR_PATTERN_MATCH: - result[out_key] = CODE_VARIABLES_TOO_LONG_VALUE - elif _matcher_matches(key_str, config.mask): + # Too long to scan, so the key text can't be vouched for either. + result[_redacted_key(result)] = CODE_VARIABLES_TOO_LONG_VALUE + continue + key_matches_mask = _matcher_matches(key_str, config.mask) + if key_matches_mask and not _is_field_name(key, key_is_json_safe): + # A name such as `password` is safe to keep, but other text that matches can + # hold the value itself, e.g. `BasicAuth(login='u', password='...')`. + out_key = _redacted_key(result) + elif config.detect_secrets and _looks_like_secret(key_str): + out_key = _redacted_key(result) + elif not key_is_json_safe and _key_parts_fail_masking(key, config, seen, depth): + out_key = _redacted_key(result) + elif config.mask_url_credentials and isinstance(out_key, str): + out_key = _redact_url_credentials(out_key) + if out_key in result: + # Two keys can end up with the same text, for example URLs that differ only in + # their credentials. A placeholder keeps the later entry from overwriting. + out_key = _redacted_key(result) + if key_matches_mask: result[out_key] = CODE_VARIABLES_REDACTED_VALUE else: result[out_key] = _mask_value(value, config, seen, depth + 1) @@ -1439,7 +1540,7 @@ def _mask_value(value, config, seen=None, depth=0): seen = set() obj_id = id(value) if obj_id in seen: - return "" + return _CIRCULAR_REF_VALUE seen.add(obj_id) if len(seen) > _MAX_TOTAL_NODES_TO_MASK: diff --git a/posthog/test/test_code_variables.py b/posthog/test/test_code_variables.py index cb3d3cbe1..3d8abe0fb 100644 --- a/posthog/test/test_code_variables.py +++ b/posthog/test/test_code_variables.py @@ -40,6 +40,41 @@ # --- shared helpers ------------------------------------------------------------------ +# Synthetic, format-correct fakes (no real credentials). Vendor keys are assembled from +# prefix + body so no complete secret literal lives in source (which trips secret scanners). +def _key(prefix, body): + return prefix + body + + +def redacted_key(n): + """The placeholder that replaces the n-th redacted key of one mapping.""" + return f"$$_posthog_redacted_key_{n}_$$" + + +@dataclass(frozen=True) +class _Login: + user: str + code: str + + +class _UnprintableKey: + def __str__(self): + raise RuntimeError("no text") + + +class _BrokenDict(dict): + def __len__(self): + raise RuntimeError("no length") + + +class _KeyWithBrokenField: + def __init__(self): + self.index = _BrokenDict() + + def __repr__(self): + return "KeyWithBrokenField()" + + def make_config( *, patterns=DEFAULT_CODE_VARIABLES_MASK_PATTERNS, ignore=(), mask_urls=True ): @@ -264,9 +299,10 @@ def test_safe_dict_is_unchanged(self): # {"name": "test", "value": 123} -> unchanged assert mask({"name": "test", "value": 123}) == {"name": "test", "value": 123} - def test_dict_key_matching_a_pattern_redacts_its_value(self): + @pytest.mark.parametrize("name", ["password", "Proxy-Authorization", "db.password"]) + def test_dict_key_matching_a_pattern_redacts_its_value(self, name): # {"password": ...} -> value redacted on the strength of the key name alone - assert mask({"password": "anything"}) == {"password": REDACTED} + assert mask({name: "anything"}) == {name: REDACTED} def test_dict_value_matching_a_pattern_is_redacted(self): # {"note": "...password..."} -> value redacted because the value matches @@ -298,12 +334,15 @@ def test_list_of_dicts_is_masked(self): assert out == [{"id": 1, "password": REDACTED}, {"id": 2, "value": "ok"}] def test_overly_long_dict_key_replaces_only_that_entry(self): - # {"short": "ok", : ..., "password": ...} + # {"short": "ok", : ..., "password": ...} -> the long key is too + # long to scan, so the key itself is replaced along with its value long_key = "k" * 20000 out = mask({"short": "ok", long_key: "v", "password": "x"}) - assert out["short"] == "ok" - assert out[long_key] == TOO_LONG - assert out["password"] == REDACTED + assert out == { + "short": "ok", + redacted_key(0): TOO_LONG, + "password": REDACTED, + } @pytest.mark.parametrize( "build", @@ -339,9 +378,105 @@ def test_non_string_dict_key_is_coerced_to_stay_serializable(self): # {(1, 2): "ok"} -> key stringified so the masked dict is always JSON-safe assert mask({(1, 2): "ok"}) == {"(1, 2)": "ok"} - def test_non_string_dict_key_still_redacts_on_its_name(self): - # a key whose text matches a pattern redacts its value, just like a string key - assert mask({("db", "password"): "x"}) == {"('db', 'password')": REDACTED} + def test_non_string_dict_key_matching_a_pattern_is_replaced_with_its_value(self): + # a stringified key is a repr, so a pattern match can mean the key text holds the + # secret itself: the key is replaced, not only its value + assert mask({("db", "password"): "x"}) == {redacted_key(0): REDACTED} + + def test_credentials_inside_a_namedtuple_key_do_not_leak(self): + # a connection pool keyed by a namedtuple that embeds proxy credentials, like + # aiohttp's ConnectionKey(..., proxy_auth=BasicAuth(login, password)) + Auth = collections.namedtuple("Auth", "login password") + PoolKey = collections.namedtuple("PoolKey", "host port proxy_auth") + key = PoolKey("api.example.com", 443, Auth("proxy-user", "hunter2-proxy-pw")) + out = mask({"_conns": {key: ["conn"]}, "_limit": 100}) + assert out == {"_conns": {redacted_key(0): REDACTED}, "_limit": 100} + assert "hunter2-proxy-pw" not in json.dumps(out) + + @pytest.mark.parametrize( + "value, expected", + [ + ( + {("a", "password"): 1, ("b", "password"): 2, "ok": 3}, + {redacted_key(0): REDACTED, redacted_key(1): REDACTED, "ok": 3}, + ), + ( + { + "postgres://alice:p1@db.example.com/prod": "A", + "postgres://bob:p2@db.example.com/prod": "B", + }, + { + f"postgres://{REDACTED}@db.example.com/prod": "A", + redacted_key(0): "B", + }, + ), + ( + {("a", "password"): 1, redacted_key(0): 2}, + {redacted_key(0): REDACTED, redacted_key(1): 2}, + ), + ], + ids=[ + "two-replaced-keys", + "urls-differing-in-credentials", + "literal-placeholder", + ], + ) + def test_keys_that_end_up_with_the_same_text_keep_their_own_entries( + self, value, expected + ): + assert mask(value) == expected + + @pytest.mark.parametrize( + "secret_key, expected_value", + [ + # no pattern matches, so only the key is replaced and the value is kept + (_key("AKIA", "IOSFODNN7EXAMPLE"), "customer-1"), + # `sk_` also matches a pattern, which must not keep the key as a name + (_key("sk_live_", "4eC39HqLyjWDarjtT1zdp7dc"), REDACTED), + # a query cache keyed by SQL: the text matches a pattern but is not a name + ("SELECT * FROM users WHERE auth_token = 'abc123'", REDACTED), + ], + ids=["aws-key-id", "stripe-key", "sql-with-a-token"], + ) + def test_string_key_that_holds_a_secret_is_replaced( + self, secret_key, expected_value + ): + assert mask({secret_key: "customer-1"}) == {redacted_key(0): expected_value} + + @pytest.mark.parametrize( + "build_key", + [ + lambda secret: ("admin", secret), + lambda secret: _Login("admin", secret), + ], + ids=["tuple", "frozen-dataclass"], + ) + def test_non_string_key_holding_an_unnamed_secret_is_replaced(self, build_key): + # the key's repr has quotes and brackets, which turn off the entropy check, so + # the parts of the key are masked one by one + secret = "n8fK2pQ9vX7mL4wR8tY3uZ6bC1dE5gH" + out = mask({build_key(secret): "client"}) + assert out == {redacted_key(0): "client"} + + @pytest.mark.parametrize( + "key, expected_value", + [ + # no key text, so nothing shows whether the key names a secret + (_UnprintableKey(), REDACTED), + # the key text is known, but masking the parts of the key raises + (_KeyWithBrokenField(), "hunter2"), + ], + ids=["str-raises", "key-probe-raises"], + ) + def test_key_that_cannot_be_read_does_not_stop_masking(self, key, expected_value): + # a raise must not abort masking, because the fallback repr of the whole dict + # skips the entropy check for every other entry + out = mask({"note": "n8fK2pQ9vX7mL4wR8tY3uZ6bC1dE5gH", key: "hunter2"}) + assert out == {"note": REDACTED, redacted_key(0): expected_value} + + def test_url_credentials_are_scrubbed_from_a_string_key(self): + out = mask({"postgres://app:hunter2@db.example.com/prod": "pool"}) + assert out == {f"postgres://{REDACTED}@db.example.com/prod": "pool"} def test_non_string_dict_key_does_not_defeat_value_masking(self): # a tuple key used to break json.dumps and fall back to a repr of the *original* @@ -379,6 +514,38 @@ def tree(width, depth): assert TOO_LONG in json.dumps(mask(tree(8, 3))) # ~580 nodes, over the budget assert TOO_LONG not in json.dumps(mask(tree(4, 2))) # ~20 nodes, well under + def test_key_probes_share_the_node_budget(self): + # object keys that point back to their dict must not make each key probe walk + # the whole graph again, which grows exponentially with the number of keys + class Tree: + def __init__(self): + self.children = {} + + class Node: + def __init__(self, parent): + self.parent = parent + + root = Tree() + for i in range(_MAX_COLLECTION_ITEMS_TO_SCAN): + root.children[Node(root)] = i + + out = mask(root) + assert out["children"] == { + redacted_key(i): i for i in range(_MAX_COLLECTION_ITEMS_TO_SCAN) + } + + def test_key_probe_does_not_hide_a_value_the_key_shares(self): + # probing the key must not mark its parts visited for the value traversal, which + # would render the value as a circular ref + login = _Login("admin", "1234") + assert mask({(login,): login}) == { + "(_Login(user='admin', code='1234'),)": { + "user": "admin", + "code": "1234", + "__class__": "_Login", + } + } + # --- 6. object traversal ------------------------------------------------------------- @@ -929,12 +1096,6 @@ def trigger_error(): # --- entropy-based secret detection (last resort) ------------------------------------ -# Synthetic, format-correct fakes (no real credentials). Vendor keys are assembled from -# prefix + body so no complete secret literal lives in source (which trips secret scanners). -def _key(prefix, body): - return prefix + body - - KNOWN_FORMAT_SECRETS = [ _key("sk-proj-", "T3BlbkFJabcd1234efgh5678ijkl9012mnop3456qrst7890wxyz"), # OpenAI _key( @@ -1052,6 +1213,32 @@ def test_pure_hex_is_treated_as_an_id_not_a_secret(self): # -- integration with the masking pipeline -------------------------------------- + @pytest.mark.parametrize("statements", [3, 60], ids=["just-over-200", "near-2048"]) + def test_known_format_inside_a_long_string_is_detected(self, statements): + # a key embedded in a longer string, e.g. a SQL query that inlines credentials + query = "SET max_execution_time = 30; " * statements + ( + "DESCRIBE TABLE s3('https://bucket.example.com/data/', " + f"'{_key('AKIA', 'IOSFODNN7EXAMPLE')}', " + f"'{_key('wJalrXUtnFEMI/K7MDENG', '/bPxRfiCYEXAMPLEKEY')}', 'Parquet')" + ) + assert 200 < len(query) <= _MAX_VALUE_LENGTH_FOR_PATTERN_MATCH + assert _looks_like_secret(query) is True + assert extract(query=query) == {"query": REDACTED} + + @pytest.mark.parametrize( + "text", + [ + "https://example.com/login?next=https%3A%2F%2Fapi.example.com%2Fv1%3Fkey%3D", + 'config dump: {"keys": "old\\n', + ], + ids=["percent-encoded-url", "escaped-newline"], + ) + def test_known_format_after_a_letter_or_digit_is_redacted(self, text): + # `%3D` and `\n` end in a letter or digit, and a URL or a space turns off the + # entropy check, so a word boundary before `sk-` would let the key through + key = _key("sk-proj-", "T3BlbkFJabcd1234efgh5678ijkl9012mnop3456qrst7890wxyz") + assert mask(text + key) == REDACTED + def test_high_entropy_value_in_a_neutral_variable_is_redacted(self): result = extract(api_response="n8fK2pQ9vX7mL4wR8tY3uZ6bC1dE5gH") assert result == {"api_response": REDACTED}