Section I backend — audit trail + topology source - #32
Merged
Conversation
Part 1 of Section I (platform/audit). Turns the bare `audit_logs` skeleton into an immutable, pseudonymous audit trail and derives an honest system topology from real sources. Audit trail: - Migration 0010: additive columns (actor HMAC token, action_type, target_kind/id, machine_id, origin, detail JSONB, occurred_at) + CHECK constraints + an append-only trigger (blocks UPDATE/DELETE, defense-in-depth; no TRUNCATE trigger so test/reset paths still work). - Writer with two paths: in-session record() for the HITL acknowledge (atomic), best-effort emit() for MCP retrievals on its own session so the MCP read-only invariant stays intact. - Two real writer paths: the drift acknowledge route (origin=dashboard) and the MCP tool wrapper _measured (origin=mcp). MCP actor is a pseudonymous single-consumer label (auth.py has no per-client id; per-client attribution stays [VISION]). - GET /api/v1/audit, filtered + paginated, manager/admin only. Topology source: - Honestly derived nodes only: ingest sources (data_points.source + latest readings activity, simulation marked internal), memory substrate (best-effort live smoke probe), F7 MCP boundary (activity from the new mcp_retrieval audit rows). Status only where measurable (else unbekannt, never green). Named third systems stay in a separate [VISION] list. - GET /api/v1/topology: manager full, shift_lead status-only, others 403. actor is always an HMAC token, never cleartext (section 8); user_id retained but unused. GROUND_TRUTH + WALKTHROUGH updated in the same commit. Tested: mypy strict 0, ruff clean, migration 0010 up/down + trigger, MCP read-only invariant, role guards, honest topology; coverage >=80% on audit/ + topology/ + new routers. 607 backend tests green (local, minus F-PRED) + 31 new Section-I tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughPR führt ein vollständiges Audit-Trail-Subsystem (Schema-Migration mit Append-Only-Trigger, Pydantic-Schemas, Writer-Hilfsfunktionen, Read-Service und Router) sowie einen abgeleiteten Topologie-Read-Endpoint ein. Beide Router werden in FastAPI registriert. MCP-Tools und der Drift-Acknowledge-Endpoint schreiben Audit-Einträge. Migrations-Roundtrip-, API- und Topologie-Integrationstests sowie Dokumentationsaktualisierungen ergänzen die Änderungen. ChangesAudit-Trail & Topologie-Backend (Sektion I)
Sequence Diagram(s)sequenceDiagram
participant Client as API-Client
participant AuditRouter as GET /api/v1/audit
participant TopologyRouter as GET /api/v1/topology
participant AuditService as list_audit()
participant TopologyService as build_topology()
participant DB as PostgreSQL
Client->>AuditRouter: GET /api/v1/audit?action_type=...&machine_id=...
AuditRouter->>AuditRouter: Rollen-Check (nur manager → sonst 403)
AuditRouter->>AuditService: list_audit(session, filter-params, limit, offset)
AuditService->>DB: SELECT * FROM audit_logs WHERE ... ORDER BY occurred_at DESC
DB-->>AuditService: Sequence[AuditLog]
AuditService-->>AuditRouter: Sequence[AuditLog]
AuditRouter-->>Client: list[AuditEntryRead]
Client->>TopologyRouter: GET /api/v1/topology?probe=true&fresh_within_minutes=60
TopologyRouter->>TopologyRouter: Rollen-Check (manager/shift_lead → sonst 403)
TopologyRouter->>TopologyService: build_topology(session, include_audit=True/False, ...)
TopologyService->>DB: SELECT source, max(time) FROM readings GROUP BY source
TopologyService->>DB: SELECT actor, max(occurred_at) FROM audit_logs WHERE action_type=mcp_retrieval
TopologyService-->>TopologyRouter: TopologyView
TopologyRouter-->>Client: TopologyView (nodes + vision + generated_at)
sequenceDiagram
participant Operator as Operator (HITL)
participant DriftRouter as PATCH /drift/alarms/:id/acknowledge
participant Writer as audit/writer.py
participant DB as PostgreSQL
Operator->>DriftRouter: PATCH acknowledge (Bearer token)
DriftRouter->>DriftRouter: Setzt acknowledged_at + acknowledged_by
DriftRouter->>Writer: hitl_acknowledge_entry(pseudo, user_id, role, alarm_id, machine_id, ...)
Writer-->>DriftRouter: AuditEntry (pseudonymisierter actor)
DriftRouter->>Writer: record(session, entry)
Writer->>DB: INSERT INTO audit_logs ... (add + flush, kein Commit)
DriftRouter->>DB: COMMIT (atomar mit Alarm-Update)
DB-->>DriftRouter: OK
DriftRouter-->>Operator: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/foreman/api/routers/audit.py (1)
20-20: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueUngenutzter Import.
AuditLogwird in diesem Modul nicht direkt verwendet –list_auditgibtSequence[AuditLog]zurück, aber FastAPI serialisiert überresponse_model=list[AuditEntryRead]. Der Import kann entfernt werden.♻️ Vorgeschlagene Bereinigung
from foreman.audit.schemas import AuditEntryRead from foreman.audit.service import list_audit -from foreman.db.models import AuditLog from foreman.realtime.authz import ROLE_MANAGER🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/foreman/api/routers/audit.py` at line 20, The import statement for `AuditLog` from `foreman.db.models` is unused in this module. Although `AuditLog` may appear in type hints, FastAPI handles serialization via the `response_model=list[AuditEntryRead]` parameter, making this import unnecessary. Remove the `from foreman.db.models import AuditLog` line from the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@GROUND_TRUTH.md`:
- Around line 127-129: The role naming terminology is inconsistent across the
document. In the endpoints documented for the GET /api/v1/audit and GET
/api/v1/topology sections, the terms "Manager/Admin" are used, but the actual
role model defined in §5 only defines the roles: worker, shift_lead, technician,
and manager. Standardize all role references in these sections to match the
authoritative role model from §5 by replacing "Manager/Admin" with either just
"manager" or by explicitly adding "admin" as an official role throughout the
document if that role is actually supported by the system, ensuring consistency
across all affected sections (the GET /api/v1/audit description, GET
/api/v1/topology description, and any other similar references).
In `@tests/migrations/test_migration_0010.py`:
- Around line 119-140: The test for the append-only trigger behavior is
incomplete. After the DELETE test block and before the CHECK constraint test
(after line 140), add a new test case that verifies TRUNCATE operations are
permitted on the audit_logs table despite the append-only trigger being active.
Unlike the UPDATE and DELETE operations which should raise
asyncpg.exceptions.RaiseError with the "append-only" message, the TRUNCATE
operation should execute successfully without raising an exception. This
verifies that the append-only trigger permits TRUNCATE for test and reset
operations as documented in the PR objectives.
---
Nitpick comments:
In `@src/foreman/api/routers/audit.py`:
- Line 20: The import statement for `AuditLog` from `foreman.db.models` is
unused in this module. Although `AuditLog` may appear in type hints, FastAPI
handles serialization via the `response_model=list[AuditEntryRead]` parameter,
making this import unnecessary. Remove the `from foreman.db.models import
AuditLog` line from the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b2fe210-3961-46a6-965f-0a1cfda43650
📒 Files selected for processing (21)
GROUND_TRUTH.mddocs/WALKTHROUGH.mdmigrations/versions/0010_audit_trail_topology.pysrc/foreman/api/routers/audit.pysrc/foreman/api/routers/topology.pysrc/foreman/audit/__init__.pysrc/foreman/audit/schemas.pysrc/foreman/audit/service.pysrc/foreman/audit/writer.pysrc/foreman/db/models.pysrc/foreman/main.pysrc/foreman/mcp/tools.pysrc/foreman/reasoners/drift/router.pysrc/foreman/topology/__init__.pysrc/foreman/topology/schemas.pysrc/foreman/topology/service.pytests/audit/test_audit_api.pytests/audit/test_mcp_audit.pytests/audit/test_writer.pytests/migrations/test_migration_0010.pytests/topology/test_topology.py
- Role consistency: replace "Manager/Admin" with "Manager" in the GROUND_TRUTH contract + router docs/comments — the role vocabulary (section 5) has no `admin` role; the platform/audit view is enforced for `manager`. Add a study-mapping note (Designstudie §4I says "Manager/Admin" → maps to manager; future admin role would be additive in _AUDIT_ROLES/_FULL_ROLES). - Migration 0010 test: assert TRUNCATE is still permitted (the deliberate "no TRUNCATE trigger" choice so test/reset paths can clear the table) — append-only covers UPDATE/DELETE, not "never emptyable". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
CodeRabbit-Feedback adressiert (Commit 4c69620):
Gates lokal grün nach den Fixes: mypy strict 0, ruff clean + Format clean, Migrationstest (inkl. TRUNCATE) grün. |
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Teil 1 von Sektion I (Plattform/Audit). Macht das nackte
audit_logs-Skelett zu einem unveränderlichen, pseudonymen Audit-Trail (zugleich AI-Act-/Art.-50-Nachweis-Beleg) und leitet eine ehrliche Systemtopologie aus realen Quellen ab. §4I der Designstudie ist [VISION]; dieser PR baut die ehrlich abgeleitete Teilmenge — kein erfundener Knoten, kein erfundener Akteur.Teil A — Audit-Trail
actor(HMAC-Token, nie Klartext),actor_role,action_type/origin(CHECK),target_kind/target_id/machine_id,detail(JSONB),occurred_at; Lese-Indizes; Append-Only-Trigger (PL/pgSQL, weistUPDATE/DELETEab — Defense-in-Depth; bewusst kein TRUNCATE-Trigger, damit Test-/Reset-Pfade leeren können).record()in-session/atomar (HITL),emit_mcp_retrieval()best-effort auf eigener Session (MCP) — die MCP-Read-Only-Invariante bleibt intakt.origin=dashboard, atomar) und der MCP-Tool-Wrapper_measured(origin=mcp, imfinallynach geschlossener Read-Session).GET /api/v1/audit— gefiltert (action_type/target_kind/target_id/actor/machine_id/since/until), paginiert, jüngste zuerst, nur Manager/Admin.Teil B — Topologie-Quelle
data_points.source+ jüngsterreadings-Aktivität (simulationals intern), Gedächtnis-Substrat (Live-Smoke-Probe,?probe=falseabschaltbar), F7-MCP-Grenze (Aktivität aus den neuenmcp_retrieval-Audit-Zeilen). Status nur wo messbar (sonstunbekannt, nie grün); benannte Drittsysteme nur in separater [VISION]-Liste.GET /api/v1/topology— Manager voll · Schichtleiter nur Verbindungsstatus (kein Audit) · Werker/Techniker 403.Ehrliche Abweichungen von der Prompt-Annahme (kein Fake)
mcp/auth.pyhat keine Per-Client-Identität (nur ein geteilter Token) →actor= pseudonymisiertes Single-Consumer-Label; per-Client-Attribution bleibt [VISION].alarms.pyhat keine Ack-Route) — dort hängt der HITL-Write.audit_logs.user_idbleibt erhalten, aber ungenutzt;actor(HMAC) ist die pseudonyme Referenz. §8 nachgezogen.Verifikation
actornie Klartext).audit//topology/+ neuen Routern. 607 Backend-Tests grün (lokal ohne F-PRED) + 31 neue Sektion-I-Tests.Nicht in diesem PR: Frontend §4I (Teil 2). Nicht selbst gemergt — bitte du.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
GET /api/v1/auditmit Filterung und Pagination (rollenbasiert, nur Manager).GET /api/v1/topologymit Live-Status/Frischefenster und rollenbasierter Detailtiefe.Bug Fixes
Documentation