fix(telemetry): a credential written as one word is still a credential (#1095)

agentdescent/rsi_telemetry.py

@@ -66,8 +66,21 @@
     "apikey", "accesskey", "privatekey", "secretkey", "clientsecret",
     "apitoken", "accesstoken", "authtoken", "refreshtoken", "bearertoken",
     "password", "passwd", "pwd", "credential", "credentials", "authorization",
+    # A plural compound is still a credential in any position (`db_passwords`).
+    "passwords", "passwds", "secretkeys", "apikeys", "privatekeys",
+    "accesskeys", "clientsecrets",
+))
+_CREDENTIAL_NOUNS = frozenset(("token", "secret", "secrets"))
+#: A credential written as one word (#1095): `PGPASSWORD` is libpq's password
+#: variable and `s3secretkey` / `AWSSecretAccessKey` carry no separator, so a
+#: single-component key is matched by whole-word suffix. `tokens` ends `okens`,
+#: which is why the suffix has to be a whole credential word and not a stem.
+_CREDENTIAL_WORDS = frozenset((
+    "password", "passwd", "secret", "secretkey", "apikey", "privatekey",
+    "accesskey", "clientsecret", "credential",
+    "passwords", "passwds", "secrets", "secretkeys", "apikeys", "privatekeys",
+    "accesskeys", "clientsecrets", "credentials",
 ))
-_CREDENTIAL_NOUNS = frozenset(("token", "secret"))
 #: Two components of one credential spelling (`api` + `key`).
 _CREDENTIAL_PAIRS = frozenset((
     "apikey", "accesskey", "privatekey", "secretkey", "clientsecret",
@@ -81,12 +94,27 @@
     r"Bearer\s+[A-Za-z0-9._-]{10,})")
 
 
+def _singular(word: str) -> Tuple[str, ...]:
+    """``word`` and, when it ends in a plural ``s``, its singular (#1095)."""
+    if len(word) > 1 and word.endswith("s"):
+        return (word, word[:-1])
+    return (word,)
+
+
+def _pair_spells_a_credential(first: str, second: str) -> bool:
+    """`api` + `keys`, `client` + `secrets`: one plural `s` is still the pair."""
+    return any(left + right in _CREDENTIAL_PAIRS
+               for left in _singular(first) for right in _singular(second))
+
+
 def _is_secret_key(key: Any) -> bool:
     """True when a whole component of ``key`` names a credential (#1070).
 
     ``token_count`` and ``secret_free`` measure credentials, they are not one;
-    ``client_secret`` and ``api_key`` are, in any spelling. Values are still
-    filtered by :data:`_SECRET_VALUE` wherever they appear.
+    ``client_secret`` and ``api_key`` are, in any spelling. A credential that is
+    one word (`PGPASSWORD`, `s3secretkey`) has no component to see, so it is
+    matched by whole-word suffix instead (#1095). Values are still filtered by
+    :data:`_SECRET_VALUE` wherever they appear.
     """
     parts = [part for part in _KEY_PARTS.split(str(key).lower()) if part]
     if not parts:
@@ -95,7 +123,10 @@ def _is_secret_key(key: Any) -> bool:
         return True
     if parts[-1] in _CREDENTIAL_NOUNS:
         return True
-    return any(first + second in _CREDENTIAL_PAIRS
+    if len(parts) == 1 and any(parts[0].endswith(word)
+                               for word in _CREDENTIAL_WORDS):
+        return True
+    return any(_pair_spells_a_credential(first, second)
                for first, second in zip(parts, parts[1:]))
 
 

changelog.d/1095-credential-word-suffix.md

@@ -0,0 +1,9 @@
+Fixed
+`_is_secret_key` no longer lets a credential through when it arrives as one
+word: `PGPASSWORD`, `s3secretkey` and `AWSSecretAccessKey` are matched by
+whole-word suffix on a single-component key, and the adjacent-pair lookup
+tolerates one plural `s`, so `client_secrets` and `api_keys` are dropped too.
+The plural spellings (`passwords`, `pgpasswords`, `db_secrets`) match in the
+last position or as a whole component. The `#1070` counters (`tokens`,
+`max_tokens`, `secrets_scanned`) still survive, and `auth` stays a recorded
+non-goal (#1095).

tests/test_span_redaction.py

@@ -177,3 +177,58 @@ def test_a_memory_block_keeps_its_counters_too() -> None:
 
     assert attributes["budget_tokens"] == 500
     assert attributes["token_count"] == 3
+
+
+# --- a credential written as one word is still a credential (#1095) --------
+#
+# `#1070` matches a whole component, so a credential name that arrives as one
+# word falls through all three rules: `PGPASSWORD` is libpq's password variable,
+# `s3secretkey` and `AWSSecretAccessKey` need a separator to split, and
+# `client_secrets` / `api_keys` are the pair with a plural `s`. The amendment
+# also pins the plain plurals (`passwords`, `pgpasswords`, `db_secrets`), which
+# only the last-position noun rule and the plural suffixes reach. `auth` stays a
+# non-goal: it names a mode (`auth_mode`, `auth_ok`) as often as a credential.
+
+CREDENTIAL_WORD_KEYS = (
+    "PGPASSWORD", "pgpassword", "passwords", "pgpasswords", "db_passwords",
+    "db_secrets", "client_secrets", "CLIENT_SECRETS", "api_keys",
+    "secret_keys", "s3secretkey", "AWSSecretAccessKey", "AWS_SECRET_ACCESS_KEY",
+)
+
+#: Names that are not credentials, including the recorded `auth` non-goal.
+NOT_CREDENTIAL_KEYS = (
+    "REDISCLI_AUTH", "auth", "sort_key", "monkey", "max_new_tokens",
+    "total_tokens", "status", "access_level",
+)
+
+
+def _is_secret_key(key: object) -> bool:
+    from agentdescent.rsi_telemetry import _is_secret_key as predicate
+
+    return predicate(key)
+
+
+@pytest.mark.parametrize("key", CREDENTIAL_WORD_KEYS)
+def test_a_credential_key_is_dropped(key: str) -> None:
+    assert _is_secret_key(key) is True
+
+
+@pytest.mark.parametrize("key", tuple(MEASUREMENT_KEYS) + NOT_CREDENTIAL_KEYS)
+def test_the_counters_and_an_auth_mode_are_not_credentials(key: str) -> None:
+    assert _is_secret_key(key) is False
+
+
+def test_a_one_word_credential_never_reaches_the_export() -> None:
+    telemetry = _telemetry()
+    span = telemetry.start_span("cost", attributes={
+        "PGPASSWORD": "hunter2", "MAX_TOKENS": 16, "secrets_scanned": 12})
+
+    assert "PGPASSWORD" not in span.attributes
+    assert span.attributes["MAX_TOKENS"] == 16
+    assert span.attributes["secrets_scanned"] == 12
+
+    (exported,) = telemetry.to_otel()
+    assert "PGPASSWORD" not in exported["attributes"]
+    assert exported["attributes"]["MAX_TOKENS"] == 16
+    assert exported["attributes"]["secrets_scanned"] == 12
+    assert "hunter2" not in str(exported)