From 6b22741f549d041f25c41db8fee9532fabfca396 Mon Sep 17 00:00:00 2001
From: Cog <cog@tod.local>
Date: Wed, 11 Mar 2026 23:02:36 -0500
Subject: [PATCH] fix: audit logs never persisted + firmware-cache permission
 denied

Two bugs fixed:

1. audit_service.py: log_action() inserted into audit_logs using the
   caller's DB session but never committed. Any router that called
   db.commit() before log_action() (firmware, devices, config_editor,
   alerts, certificates) had its audit rows silently rolled back when
   the request session closed.
   Fix: log_action now opens its own AdminAsyncSessionLocal and self-
   commits, making audit persistence independent of the caller's
   transaction. The 'db' parameter is kept for backward compat but
   unused. Affects 5 routers (firmware, devices, config_editor,
   alerts, certificates).

2. docker-compose.override.yml: /data/firmware-cache had no volume
   mount so the directory didn't exist in the container, causing
   firmware downloads to fail with Permission denied.
   Fix: bind-mount docker-data/firmware-cache:/data/firmware-cache
   so firmware images survive container restarts.
---
 backend/app/services/audit_service.py | 62 ++++++++++++++++-----------
 docker-compose.override.yml           |  1 +
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/backend/app/services/audit_service.py b/backend/app/services/audit_service.py
index ae6a65d..05c9f26 100644
--- a/backend/app/services/audit_service.py
+++ b/backend/app/services/audit_service.py
@@ -10,6 +10,11 @@ Phase 30: When details are non-empty, they are encrypted via OpenBao Transit
 (per-tenant data key) and stored in encrypted_details. The plaintext details
 column is set to '{}' for column compatibility. If Transit encryption fails
 (e.g., OpenBao unavailable), details are stored in plaintext as a fallback.
+
+IMPORTANT: log_action always opens its own AdminAsyncSessionLocal session and
+commits internally. This guarantees the audit INSERT is persisted regardless of
+whether the caller's DB session has already been committed or rolled back.  The
+``db`` parameter is kept for backward compatibility but is intentionally unused.
 """
 
 import uuid
@@ -23,7 +28,7 @@ logger = structlog.get_logger("audit")
 
 
 async def log_action(
-    db: AsyncSession,
+    db: AsyncSession,  # kept for backward compat — not used internally
     tenant_id: uuid.UUID,
     user_id: uuid.UUID,
     action: str,
@@ -33,9 +38,14 @@ async def log_action(
     details: Optional[dict[str, Any]] = None,
     ip_address: Optional[str] = None,
 ) -> None:
-    """Insert a row into audit_logs.  Swallows all exceptions on failure."""
+    """Insert a row into audit_logs using a dedicated session.
+
+    Always self-commits so the INSERT is never dependent on the caller's
+    transaction lifecycle.  Swallows all exceptions on failure.
+    """
     try:
         import json as _json
+        from app.database import AdminAsyncSessionLocal
 
         details_dict = details or {}
         details_json = _json.dumps(details_dict)
@@ -59,30 +69,34 @@ async def log_action(
                     tenant_id=str(tenant_id),
                     exc_info=True,
                 )
-                # Keep details_json as-is (plaintext fallback)
                 encrypted_details = None
 
-        await db.execute(
-            text(
-                "INSERT INTO audit_logs "
-                "(tenant_id, user_id, action, resource_type, resource_id, "
-                "device_id, details, encrypted_details, ip_address) "
-                "VALUES (:tenant_id, :user_id, :action, :resource_type, "
-                ":resource_id, :device_id, CAST(:details AS jsonb), "
-                ":encrypted_details, :ip_address)"
-            ),
-            {
-                "tenant_id": str(tenant_id),
-                "user_id": str(user_id),
-                "action": action,
-                "resource_type": resource_type,
-                "resource_id": resource_id,
-                "device_id": str(device_id) if device_id else None,
-                "details": details_json,
-                "encrypted_details": encrypted_details,
-                "ip_address": ip_address,
-            },
-        )
+        # Use a dedicated session so this commit is independent of the
+        # caller's transaction (fixes dropped audit logs in routers that
+        # call log_action after their own db.commit()).
+        async with AdminAsyncSessionLocal() as audit_db:
+            await audit_db.execute(
+                text(
+                    "INSERT INTO audit_logs "
+                    "(tenant_id, user_id, action, resource_type, resource_id, "
+                    "device_id, details, encrypted_details, ip_address) "
+                    "VALUES (:tenant_id, :user_id, :action, :resource_type, "
+                    ":resource_id, :device_id, CAST(:details AS jsonb), "
+                    ":encrypted_details, :ip_address)"
+                ),
+                {
+                    "tenant_id": str(tenant_id),
+                    "user_id": str(user_id),
+                    "action": action,
+                    "resource_type": resource_type,
+                    "resource_id": resource_id,
+                    "device_id": str(device_id) if device_id else None,
+                    "details": details_json,
+                    "encrypted_details": encrypted_details,
+                    "ip_address": ip_address,
+                },
+            )
+            await audit_db.commit()
     except Exception:
         logger.warning(
             "audit_log_insert_failed",
diff --git a/docker-compose.override.yml b/docker-compose.override.yml
index 023141e..16934fa 100644
--- a/docker-compose.override.yml
+++ b/docker-compose.override.yml
@@ -47,6 +47,7 @@ services:
     volumes:
       - ./backend:/app
       - ./docker-data/git-store:/data/git-store
+      - ./docker-data/firmware-cache:/data/firmware-cache
       - ./docker-data/wireguard:/data/wireguard
     depends_on:
       postgres: