diff --git a/application/api/user/analytics/routes.py b/application/api/user/analytics/routes.py index 0d244c2f..9bf3ba1a 100644 --- a/application/api/user/analytics/routes.py +++ b/application/api/user/analytics/routes.py @@ -12,6 +12,7 @@ from application.api.user.base import ( generate_hourly_range, generate_minute_range, ) +from application.storage.db.redaction import redact_secrets from application.storage.db.repositories.agents import AgentsRepository from application.storage.db.repositories.token_usage import TokenUsageRepository from application.storage.db.session import db_readonly @@ -1059,7 +1060,10 @@ class GetUserLogs(Resource): item.update( { "endpoint": payload.get("endpoint"), - "stacks": payload.get("stacks"), + # Redact on the way out too: rows written + # before write-time redaction still carry the + # reflected provider/user secrets in ``stacks``. + "stacks": redact_secrets(payload.get("stacks")), } ) elif m["event_type"] == "workflow": diff --git a/application/storage/db/redaction.py b/application/storage/db/redaction.py new file mode 100644 index 00000000..cb1e4754 --- /dev/null +++ b/application/storage/db/redaction.py @@ -0,0 +1,64 @@ +"""Secret redaction for reflected ``stack_logs`` data. + +``stacks`` is built by reflecting every public attribute of runtime +objects (the ``llm`` component carries the deployment provider +``api_key`` and the caller's ``user_api_key``). The unified-logs endpoint +returns ``stacks`` to the client, so credentials must be scrubbed — at +write time for new rows, and at read time for rows written before +redaction existed. +""" + +from __future__ import annotations + +REDACTED = "[REDACTED]" + +# Substrings marking a key as a credential. Compound token-count fields +# (``prompt_tokens`` / ``generated_tokens`` / ``token_budget`` / ...) are +# intentionally not matched: secret token forms are spelled out +# (``access_token`` etc.) and a bare ``token`` is handled separately. +_SECRET_SUBSTRINGS = ( + "api_key", + "apikey", + "api_token", + "access_token", + "refresh_token", + "auth_token", + "id_token", + "session_token", + "secret", + "password", + "passwd", + "passphrase", + "private_key", + "credential", + "authorization", + "bearer", +) + + +def is_secret_key(key: str) -> bool: + """True when ``key`` names a credential that must not be persisted/returned.""" + k = key.lower() + if k == "token": + return True + return any(s in k for s in _SECRET_SUBSTRINGS) + + +def redact_secrets(obj): + """Recursively replace secret-keyed values with ``[REDACTED]``. + + Walks dicts and lists; leaf scalars pass through unchanged. ``None`` + returns ``None`` so callers can redact an optional payload directly. + """ + if isinstance(obj, dict): + return { + k: ( + REDACTED + if isinstance(k, str) and is_secret_key(k) + else redact_secrets(v) + ) + for k, v in obj.items() + } + if isinstance(obj, list): + return [redact_secrets(v) for v in obj] + return obj diff --git a/application/storage/db/repositories/stack_logs.py b/application/storage/db/repositories/stack_logs.py index d24ed42f..fc1a6387 100644 --- a/application/storage/db/repositories/stack_logs.py +++ b/application/storage/db/repositories/stack_logs.py @@ -13,36 +13,11 @@ import json from datetime import datetime from typing import Optional +from application.storage.db.redaction import redact_secrets from application.storage.db.serialization import PGNativeJSONEncoder from sqlalchemy import Connection, text -_REDACTED = "[REDACTED]" - - -def _is_secret_key(key: str) -> bool: - k = key.lower() - return k.endswith("api_key") or "secret" in k or "password" in k - - -def _redact_secrets(obj): - """Strip provider/agent keys from ``stacks`` before they persist. - - The ``llm`` stack component is built from every public attribute of - the LLM instance, which includes ``api_key`` (the deployment provider - secret) and ``user_api_key``. The unified-logs endpoint returns - ``stacks`` verbatim, so these must never reach the table. - """ - if isinstance(obj, dict): - return { - k: (_REDACTED if isinstance(k, str) and _is_secret_key(k) - else _redact_secrets(v)) - for k, v in obj.items() - } - if isinstance(obj, list): - return [_redact_secrets(v) for v in obj] - return obj - class StackLogsRepository: """Postgres-backed replacement for Mongo ``stack_logs`` collection.""" @@ -81,7 +56,7 @@ class StackLogsRepository: "api_key": api_key, "query": query, "stacks": json.dumps( - _redact_secrets(stacks or []), cls=PGNativeJSONEncoder + redact_secrets(stacks or []), cls=PGNativeJSONEncoder ), "timestamp": timestamp, }, diff --git a/tests/api/user/test_analytics.py b/tests/api/user/test_analytics.py index 4fb91723..4afd3aec 100644 --- a/tests/api/user/test_analytics.py +++ b/tests/api/user/test_analytics.py @@ -731,6 +731,49 @@ class TestUnifiedLogsBranches: assert response.status_code == 200 assert [log["level"] for log in response.json["logs"]] == ["error"] + def test_system_stacks_redacted_on_read(self, app, pg_conn): + # A row written before write-time redaction existed still holds + # the reflected provider secret in ``stacks``. The endpoint must + # scrub it on the way out, not hand it back to the client. + import json + + from sqlalchemy import text + + pg_conn.execute( + text( + """ + INSERT INTO stack_logs + (activity_id, endpoint, level, user_id, stacks) + VALUES + ('act-leak', 'stream', 'error', 'u-redact', + CAST(:stacks AS jsonb)) + """ + ), + { + "stacks": json.dumps( + [ + { + "component": "llm", + "data": { + "api_key": "sk-deployment-secret", + "user_api_key": "agent-key", + "model": "gpt-x", + }, + } + ] + ) + }, + ) + + response = _post_logs(app, pg_conn, "u-redact", {"event_type": "system"}) + assert response.status_code == 200 + logs = response.json["logs"] + assert len(logs) == 1 + data = logs[0]["stacks"][0]["data"] + assert data["api_key"] == "[REDACTED]" + assert data["user_api_key"] == "[REDACTED]" + assert data["model"] == "gpt-x" + def test_non_numeric_page_returns_400(self, app, pg_conn): response = _post_logs(app, pg_conn, "u", {"page": "abc"}) assert response.status_code == 400 diff --git a/tests/storage/db/repositories/test_stack_logs.py b/tests/storage/db/repositories/test_stack_logs.py index 99527ddc..44325e7d 100644 --- a/tests/storage/db/repositories/test_stack_logs.py +++ b/tests/storage/db/repositories/test_stack_logs.py @@ -83,3 +83,46 @@ class TestInsert: # Non-secret fields (incl. token *counts*) are untouched. assert data["model"] == "gpt-x" assert data["prompt_tokens"] == 42 + + def test_redacts_broadened_secret_keys(self, pg_conn): + # Credentials beyond ``*api_key`` — OAuth tokens, bearer/auth + # headers, private keys — must also be scrubbed, while token + # *count* fields are preserved. + repo = _repo(pg_conn) + repo.insert( + activity_id="act-broad", + level="error", + stacks=[ + { + "component": "tool", + "data": { + "access_token": "ya29.secret", + "api_token": "r8_secret", + "token": "hf_secret", + "Authorization": "Bearer xyz", + "private_key": "-----BEGIN-----", + "client_secret": "cs_secret", + "prompt_tokens": 10, + "generated_tokens": 5, + "token_budget": 1000, + }, + } + ], + ) + row = pg_conn.execute( + text("SELECT stacks FROM stack_logs WHERE activity_id = 'act-broad'") + ).fetchone() + data = dict(row._mapping)["stacks"][0]["data"] + for key in ( + "access_token", + "api_token", + "token", + "Authorization", + "private_key", + "client_secret", + ): + assert data[key] == "[REDACTED]", key + # Token *counts* / budgets are not credentials and must survive. + assert data["prompt_tokens"] == 10 + assert data["generated_tokens"] == 5 + assert data["token_budget"] == 1000