diff --git a/agentdescent/rsi_telemetry.py b/agentdescent/rsi_telemetry.py index 7714348f..f3345438 100644 --- a/agentdescent/rsi_telemetry.py +++ b/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:])) diff --git a/changelog.d/1095-credential-word-suffix.md b/changelog.d/1095-credential-word-suffix.md new file mode 100644 index 00000000..892191da --- /dev/null +++ b/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). diff --git a/tests/test_span_redaction.py b/tests/test_span_redaction.py index cb4be62a..e0455543 100644 --- a/tests/test_span_redaction.py +++ b/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)