Skip to content

Commit e47e580

Browse files
galshubeliclaude
andcommitted
fix(usage): address review — log hash not PII, empty-env fallback, track crashes
- usage_tracking: the per-query log line logged the namespaced graph_id ({base64(email)}_{db}); base64 email is reversible, so log a short SHA-256 hash instead — keeps identity out of logs and neutralizes log-injection. - config: ORGANIZATIONS_GRAPH falls back to "Organizations" when the env var is empty (not just unset), so a blank value can't target an empty graph. - routes/graphs: record success=False when run_query/run_confirmed raise before the _Final sentinel (e.g. get_db_description), so pipeline crashes are still counted instead of silently bypassing tracking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 31fa28d commit e47e580

3 files changed

Lines changed: 18 additions & 5 deletions

File tree

api/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
# Central user-management graph holding User/Identity/Token (and UsageEvent)
1818
# nodes. Single source of truth for the name used across auth, tokens, and
1919
# usage tracking — override with the ORGANIZATIONS_GRAPH env var.
20-
ORGANIZATIONS_GRAPH = os.getenv("ORGANIZATIONS_GRAPH", "Organizations")
20+
ORGANIZATIONS_GRAPH = os.getenv("ORGANIZATIONS_GRAPH") or "Organizations"
2121

2222
# Configure litellm logging to prevent sensitive data leakage
2323
def configure_litellm_logging():

api/routes/graphs.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ async def stream():
201201
# Don't leak stack traces (CodeQL: information exposure through
202202
# exception). Log internally; emit a generic error event.
203203
logging.exception("Streaming query failed")
204+
# Pipeline crashed before _Final, so _serialize_pipeline didn't
205+
# record — count this attempt as a failure here.
206+
record_query_usage_background(
207+
request.state.user_id, namespaced, success=False
208+
)
204209
yield json.dumps({
205210
"type": "error",
206211
"final_response": True,
@@ -246,6 +251,10 @@ async def stream():
246251
except Exception: # pylint: disable=broad-exception-caught
247252
# See note on the query endpoint above (CodeQL).
248253
logging.exception("Streaming confirmed-destructive query failed")
254+
# Pipeline crashed before _Final — record the failed attempt here.
255+
record_query_usage_background(
256+
request.state.user_id, namespaced, success=False
257+
)
249258
yield json.dumps({
250259
"type": "error",
251260
"final_response": True,

api/routes/usage_tracking.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import asyncio
2525
import base64
2626
import binascii
27+
import hashlib
2728
import logging
2829
from typing import Optional
2930

@@ -86,11 +87,14 @@ async def _write_usage(email: str, graph_id: str, is_demo: bool, success: bool,
8687
},
8788
)
8889
# Structured-ish log line so usage is visible to log aggregators even
89-
# before any read API exists. Email (PII) is intentionally omitted; the
90-
# user-influenced graph_id has CR/LF stripped to prevent log forging.
90+
# before any read API exists. graph_id is the namespaced name
91+
# ({base64(email)}_{db}) and base64 email is reversible, so log a short
92+
# stable hash instead of the raw value — this keeps user identity out of
93+
# logs and also neutralizes the CodeQL log-injection vector.
94+
graph_ref = hashlib.sha256(graph_id.encode()).hexdigest()[:12]
9195
logging.info(
92-
"usage_event graph_id=%s is_demo=%s success=%s",
93-
graph_id.replace("\r", " ").replace("\n", " "), is_demo, success,
96+
"usage_event graph=%s is_demo=%s success=%s",
97+
graph_ref, is_demo, success,
9498
)
9599

96100

0 commit comments

Comments
 (0)