Add audit status tracking, fix duration/rewritten query, and audit write rejections

- Add `status` ("success" | "error" | "denied") and `error_message` columns to
  `query_audit_log` via two new migrations (017, 018)
- Restructure `PolicyHook::handle_query` with a labeled block so all paths
  (success, error, policy-denied) reach a single audit write site
- Fix `elapsed_ms` to be captured after the labeled block, covering encoding too
- Fix `rewritten_query` to use DataFusion `Unparser` + `BetweenRowsPostgresDialect`
  instead of the fake `/* policy-rewritten */` comment
- Swap hook order to [PolicyHook, ReadOnlyHook] and add `audit_write_rejected()`
  so write statements rejected by ReadOnlyHook are audited with status "denied"
- Extract `is_allowed_statement()` from ReadOnlyHook as shared public helper
- Add `status` filter to `GET /audit/queries` API
- Add Status column + filter dropdown + error_message detail block to QueryAuditPage
- Add `sql-formatter` for pretty-printing SQL in the detail panel
- Add TC-AUDIT-01 through TC-AUDIT-05 integration tests
- Remove resolved roadmap bug entry; add security vectors 21-24

Author: jumpbetweenrows

admin-ui/package-lock.json

@@ -17,7 +17,8 @@
     "axios": "^1.7.9",
     "react": "^19.0.0",
     "react-dom": "^19.0.0",
-    "react-router-dom": "^7.1.5"
+    "react-router-dom": "^7.1.5",
+    "sql-formatter": "^15.7.2"
   },
   "devDependencies": {
     "@tailwindcss/vite": "^4.0.6",

admin-ui/package.json

@@ -14,6 +14,8 @@ export interface AuditLogEntry {
   client_ip: string | null
   client_info: string | null
   created_at: string
+  status: 'success' | 'error' | 'denied'
+  error_message: string | null
 }
 
 export async function listAuditLogs(params?: {
@@ -23,6 +25,7 @@ export async function listAuditLogs(params?: {
   datasource_id?: string
   from?: string
   to?: string
+  status?: string
 }): Promise<PaginatedResponse<AuditLogEntry>> {
   const { data } = await client.get<PaginatedResponse<AuditLogEntry>>('/audit/queries', { params })
   return data

admin-ui/src/api/audit.ts

@@ -1,8 +1,17 @@
 import { useState } from 'react'
 import { useQuery } from '@tanstack/react-query'
+import { format } from 'sql-formatter'
 import { listAuditLogs } from '../api/audit'
 import type { AuditLogEntry } from '../api/audit'
 
+function formatSql(sql: string): string {
+  try {
+    return format(sql, { language: 'postgresql', tabWidth: 2, keywordCase: 'upper' })
+  } catch {
+    return sql
+  }
+}
+
 function formatDate(iso: string) {
   return new Date(iso).toLocaleString()
 }
@@ -11,18 +20,42 @@ function truncate(s: string, n: number) {
   return s.length > n ? s.slice(0, n) + '…' : s
 }
 
+function StatusBadge({ status }: { status: AuditLogEntry['status'] }) {
+  if (status === 'success') {
+    return (
+      <span className="inline-flex items-center rounded-full px-2 py-0.5 text-xs font-medium bg-green-50 text-green-700">
+        success
+      </span>
+    )
+  }
+  if (status === 'denied') {
+    return (
+      <span className="inline-flex items-center rounded-full px-2 py-0.5 text-xs font-medium bg-amber-50 text-amber-700">
+        denied
+      </span>
+    )
+  }
+  return (
+    <span className="inline-flex items-center rounded-full px-2 py-0.5 text-xs font-medium bg-red-50 text-red-700">
+      error
+    </span>
+  )
+}
+
 export function QueryAuditPage() {
   const [page, setPage] = useState(1)
   const [expanded, setExpanded] = useState<Set<string>>(new Set())
   const [filterUser, setFilterUser] = useState('')
   const [filterDs, setFilterDs] = useState('')
   const [filterFrom, setFilterFrom] = useState('')
   const [filterTo, setFilterTo] = useState('')
+  const [filterStatus, setFilterStatus] = useState('')
   const [appliedFilters, setAppliedFilters] = useState<{
     user_id?: string
     datasource_id?: string
     from?: string
     to?: string
+    status?: string
   }>({})
 
   const { data, isLoading, isError } = useQuery({
@@ -42,6 +75,7 @@ export function QueryAuditPage() {
       datasource_id: filterDs || undefined,
       from: filterFrom || undefined,
       to: filterTo || undefined,
+      status: filterStatus || undefined,
     })
     setPage(1)
   }
@@ -51,6 +85,7 @@ export function QueryAuditPage() {
     setFilterDs('')
     setFilterFrom('')
     setFilterTo('')
+    setFilterStatus('')
     setAppliedFilters({})
     setPage(1)
   }
@@ -81,7 +116,7 @@ export function QueryAuditPage() {
 
       {/* Filters */}
       <form onSubmit={handleFilter} className="bg-white rounded-xl border border-gray-200 p-4 mb-4">
-        <div className="grid grid-cols-4 gap-3 mb-3">
+        <div className="grid grid-cols-5 gap-3 mb-3">
           <div>
             <label className="block text-xs font-medium text-gray-600 mb-1">User ID</label>
             <input
@@ -120,6 +155,19 @@ export function QueryAuditPage() {
               className="w-full border border-gray-300 rounded px-2 py-1.5 text-xs focus:outline-none focus:ring-1 focus:ring-blue-500"
             />
           </div>
+          <div>
+            <label className="block text-xs font-medium text-gray-600 mb-1">Status</label>
+            <select
+              value={filterStatus}
+              onChange={(e) => setFilterStatus(e.target.value)}
+              className="w-full border border-gray-300 rounded px-2 py-1.5 text-xs focus:outline-none focus:ring-1 focus:ring-blue-500"
+            >
+              <option value="">All</option>
+              <option value="success">Success</option>
+              <option value="error">Error</option>
+              <option value="denied">Denied</option>
+            </select>
+          </div>
         </div>
         <div className="flex gap-2">
           <button
@@ -156,6 +204,7 @@ export function QueryAuditPage() {
                 <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">User</th>
                 <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">Data Source</th>
                 <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">Query</th>
+                <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">Status</th>
                 <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">Policies</th>
                 <th className="text-left px-4 py-3 font-medium text-gray-600 text-xs">Duration</th>
                 <th className="px-4 py-3" />
@@ -173,6 +222,9 @@ export function QueryAuditPage() {
                     <td className="px-4 py-3 text-xs font-mono text-gray-700 max-w-xs">
                       {truncate(entry.original_query, 80)}
                     </td>
+                    <td className="px-4 py-3 text-xs">
+                      <StatusBadge status={entry.status} />
+                    </td>
                     <td className="px-4 py-3 text-xs text-gray-600">
                       {entry.policies_applied.length > 0 ? (
                         <span className="bg-blue-50 text-blue-700 rounded-full px-2 py-0.5 text-xs">
@@ -196,19 +248,25 @@ export function QueryAuditPage() {
                   </tr>
                   {expanded.has(entry.id) && (
                     <tr key={`${entry.id}-detail`} className="bg-gray-50">
-                      <td colSpan={7} className="px-4 py-4">
+                      <td colSpan={8} className="px-4 py-4">
                         <div className="space-y-3">
+                          {entry.error_message && (
+                            <div className="bg-red-50 border border-red-200 rounded p-3">
+                              <p className="text-xs font-semibold text-red-700 mb-1">Error</p>
+                              <p className="text-xs font-mono text-red-800">{entry.error_message}</p>
+                            </div>
+                          )}
                           <div>
                             <p className="text-xs font-semibold text-gray-600 mb-1">Original query</p>
                             <pre className="text-xs font-mono text-gray-800 bg-white border border-gray-200 rounded p-3 overflow-auto whitespace-pre-wrap">
-                              {entry.original_query}
+                              {formatSql(entry.original_query)}
                             </pre>
                           </div>
                           {entry.rewritten_query && (
                             <div>
                               <p className="text-xs font-semibold text-gray-600 mb-1">Rewritten query</p>
                               <pre className="text-xs font-mono text-gray-800 bg-white border border-gray-200 rounded p-3 overflow-auto whitespace-pre-wrap">
-                                {entry.rewritten_query}
+                                {formatSql(entry.rewritten_query)}
                               </pre>
                             </div>
                           )}

admin-ui/src/pages/QueryAuditPage.tsx

@@ -257,3 +257,52 @@ The `ColumnAccessDef` struct no longer has an `action` field. The API validation
 - Unit: `engine::tests::test_permit_column_access_wildcard_grants_full_visibility_policy_required` — permit policy with `column_access ["*"]` in a `policy_required` aliased datasource → table is visible, `visible_tables` non-empty.
 - Unit: `hooks::policy::tests::test_column_access_deny_no_table_permit` — deny policy with `column_access` in `policy_required` mode → `lit(false)` (deny policy alone does not grant table access).
 - Unit: `admin::policy_handlers::tests::create_policy_column_access_missing_columns_422` — `column_access` definition without `columns` field → `422`.
+
+
+---
+
+### 21. Denied queries leave no audit trail (silent denial)
+
+**Vector**: A user submits a query blocked by a deny policy. If the audit log is only written on the success path, there is no record of the denied access attempt — attackers can probe policy boundaries without leaving evidence.
+
+**Bug**: The `tokio::spawn` audit write in `PolicyHook::handle_query` was placed after all `return Some(Err(...))` paths. Any failed or denied query short-circuited before the audit write.
+
+**Defense**: `handle_query` now uses a labeled block (`'query: { ... }`) that returns `(result, status, error_message, rewritten_query)` on all paths. The audit write follows the block and runs unconditionally for every auditable query. Status values: `"success"`, `"error"`, `"denied"`.
+
+**Test**:
+- Integration: `tc_audit_01_success_audit_status` — successful query → `status: "success"`, `error_message: null`, `execution_time_ms ≥ 0`, `rewritten_query` contains actual SQL (not fake comment).
+- Integration: `tc_audit_02_denied_audit_status` — deny-policy query → `status: "denied"`, `error_message` contains the policy name.
+- Integration: `tc_audit_03_error_audit_status` — query for non-existent table → `status: "error"`, `error_message` populated.
+- Integration: `tc_audit_04_status_filter` — `GET /audit/queries?status=denied` returns only denied entries.
+
+---
+
+### 22. Audit duration excludes encode phase (misleading timing)
+
+**Vector**: `execution_time_ms` was captured after `execute_logical_plan` but before `encode_dataframe`. Since DataFusion returns a lazy `DataFrame`, the actual row-fetching happens during encoding. Timing was systematically under-reported.
+
+**Defense**: `elapsed_ms` is now captured after the labeled block exits, covering planning + policy eval + execution + encoding (full user-perceived latency).
+
+**Test**: Covered by `tc_audit_01_success_audit_status` — `execution_time_ms ≥ 0` (positive timing is asserted).
+
+---
+
+### 23. Rewritten query in audit log was fake (/* policy-rewritten */ comment only)
+
+**Vector**: The audit log's `rewritten_query` field previously just prepended `/* policy-rewritten */` to the original query string. The actual row filters and column masks applied by policies were not visible, defeating the purpose of the audit trail.
+
+**Defense**: DataFusion's `Unparser` with `BetweenRowsPostgresDialect` is used to serialize the final `LogicalPlan` (after all obligation rewrites) back to SQL. If unparsing fails, the fallback is `/* plan-to-sql failed */ {original_query}`.
+
+**Test**: `tc_audit_01_success_audit_status` — `rewritten_query` must not contain `/* policy-rewritten */` and must be non-empty when a row filter was applied.
+
+---
+
+### 24. Write statement rejected by ReadOnlyHook leaves no audit trail
+
+**Vector**: A user submits a write statement (INSERT, UPDATE, DELETE, DROP, SET, etc.). `ReadOnlyHook` rejects it before `PolicyHook` runs, so no audit record is created. An attacker can probe write access without evidence.
+
+**Bug**: Hook execution order was `[ReadOnlyHook, PolicyHook]`. `ReadOnlyHook` returned `Some(Err(...))` and short-circuited the chain, so `PolicyHook` never saw the statement.
+
+**Defense**: Hook order is now `[PolicyHook, ReadOnlyHook]`. `PolicyHook` runs first: for non-`Query` statements that are not on the read-only passthrough list, it calls `audit_write_rejected()` (writes a `"denied"` audit entry with `error_message: "Only read-only queries are allowed"`) then returns `None`. `ReadOnlyHook` then runs and enforces the rejection. A shared `is_allowed_statement()` function in `read_only.rs` is the single source of truth for the allowlist — `PolicyHook` uses it to decide which statements to audit without duplicating the logic.
+
+**Test**: `tc_audit_05_write_rejected_audit_status` — `INSERT` against the proxy → audit entry has `status: "denied"`, `error_message` contains `"read-only"`.