Files
DocsGPT/docsgpt/api/audit.py
T
arc53-machine 25c82003d7 fix(admin): second review pass
Correctness
- /api/remote never recorded source.created, so URL, GitHub and connector
  sources had a source.deleted with no matching creation. All three creation
  paths now go through one _audit_source_created helper.
- The prompt-cache rate divided cached tokens by a whole bucket's prompt
  tokens. A bucket is a day and mixes calls whose provider reports a cache
  breakdown with calls whose provider does not, so filtering buckets in the
  client could not separate them and the rate was understated by however much
  traffic ran on a non-reporting provider. The denominator is now computed in
  SQL over the reporting rows.
- The outcome pill matched values nothing writes. Guardrails emit triggered /
  not_evaluated and the device feed emits dispatched; the map had blocked /
  denied / allowed, so a guardrail that fired rendered neutral grey -- the one
  signal the merged feed exists to surface. Fixtures were seeding the
  fictional values, so the tests passed on it too.
- Stream duration_ms timed the consumer. stream_token_usage is a generator,
  so start-to-exhaustion includes the agent loop's tool handling and the SSE
  client's pace; a slow browser recorded ~30s for a sub-second call. It now
  accumulates only the time spent inside next().

Safety
- Activity filters failed open: an unknown facet or unparseable timestamp was
  dropped, and no filter means every row, so a typo widened an audit view and
  on the export streamed the full history. Both are now a 400.
- The search term was interpolated into an ILIKE pattern, so "100%" matched
  everything and "q1_report" matched more than it should. Escaped.
- 0034 set actor_id NOT NULL with no default. A previous-release process
  inserting mid-rollout would raise, and in admin/routes.py that insert shares
  the request transaction, so a role grant beside it would roll back too.

Noise and dead code
- The per-user panel is a security panel: data-plane events file under the
  actor, so an active account's routine deletes pushed a denied login out of
  the 20-row window. It now excludes them; the Activity tab shows everything.
- device_audit_log had no created_at-leading index, so the merged feed
  sequentially scanned that branch every page (migration 0036).
- conversation.deleted_all no longer records when nothing was deleted, and
  agent.updated no longer records an empty field list.
- Dropped by_model from /admin/usage (no consumer; an extra aggregate per page
  load), the duplicate filter surface on AuthEventsRepository that nothing
  called, and the unreachable FLOW_LABELS.schedule entry.
- Type hints on record_event's conn and the remaining unannotated helpers.
2026-09-22 12:47:40 +01:00

75 lines
3.1 KiB
Python

"""Shared audit-trail helper for recording data-plane and identity events.
Identity and access events (login, role grants, provisioning) have been
audited since ``auth_events`` was introduced. Data-plane actions — creating and
deleting sources, agents, agent keys and conversations — were not, which left
the trail unable to answer "who deleted that source". :func:`record_event` is
the one-line hook those routes use.
Two properties matter and are enforced here rather than at every call site:
1. **An audit write can never fail the action it records.** The insert runs in
a SAVEPOINT so a rejected row rolls back alone, and any exception is
swallowed with a log line.
2. **Request context is optional.** Celery tasks (ingestion finishing, a
scheduled run) have no Flask request; the row simply records without an IP
or user agent instead of raising.
The event → category taxonomy the admin activity feed filters on lives in
``docsgpt/audit_events.py``, which the storage layer needs too.
"""
from __future__ import annotations
import logging
from typing import Any, Optional
from flask import has_request_context, request
from sqlalchemy import Connection
from docsgpt.storage.db.repositories.auth_events import AuthEventsRepository
logger = logging.getLogger(__name__)
def record_event(
conn: Connection,
event: str,
*,
actor: Optional[str],
target: Optional[str] = None,
**metadata: Any,
) -> None:
"""Append one audit event, best-effort, inside the caller's transaction.
Args:
conn: Open SQLAlchemy ``Connection`` -- not a Session -- inside the
action's transaction, so the audit row commits atomically with the
change it describes and ``begin_nested`` gives it a real SAVEPOINT.
event: Dotted event name (``source.deleted``, ``agent.created``).
actor: Who performed the action; ``"unknown"`` when unauthenticated.
target: The user acted upon, when the event is about a user. Data-plane
events act on a resource, not an account, so they leave this None.
**metadata: Free-form detail for the audit drill-down. ``None`` values
are dropped so the stored object only carries what was known.
"""
detail = {key: value for key, value in metadata.items() if value is not None}
ip = request.remote_addr if has_request_context() else None
user_agent = request.headers.get("User-Agent") if has_request_context() else None
resolved_actor = actor or "unknown"
try:
with conn.begin_nested():
AuthEventsRepository(conn).insert(
# ``user_id`` is the per-user feed key: the target when the
# event is about someone, else the actor's own trail.
user_id=target or resolved_actor,
event=event,
ip=ip,
user_agent=user_agent,
metadata=detail,
actor_id=resolved_actor,
target_id=target,
)
except Exception:
logger.warning("audit insert failed for event=%s", event, exc_info=True)