TICKET-pii-bypass-chat-attachments.md

@@ -0,0 +1,78 @@
+# TICKET: PII Detection Bypass in AI Chat
+
+**Branch**: `feature/attachment-db-storage-recall`
+
+---
+
+## Description
+
+Two PII detection bypasses were identified and fixed in the AI triage chat interface.
+
+### Bypass 1 — File Attachments (Critical)
+
+When a user attached a file to a chat message, its content was read via `readTextFile()`, sliced to 8 KB, and embedded directly into the AI message string — bypassing the existing PII pipeline entirely. The message was forwarded to the configured AI provider in plaintext. The audit log recorded the full file content without a SHA-256 hash or redaction marker.
+
+**Root cause**: `handleAttach` stored raw file content in React state. `handleSend` concatenated it into `aiMessage` with no call to `detectPiiCmd` or `applyRedactionsCmd`. The backend `chat_message` command applied no validation.
+
+### Bypass 2 — Typed Chat Messages (High)
+
+Plain typed chat messages were sent to the AI provider without any PII scan. A user typing `How secure is my password: abc123!!` would have the password forwarded to the AI and persisted in the audit log in plaintext.
+
+### Related Fix — Wrong Return Type on `detect_pii`
+
+`detect_pii` was serialising `pii::PiiDetectionResult` (`spans`, `original_text`) while the TypeScript interface expected `db::models::PiiDetectionResult` (`detections`, `total_pii_found`). All frontend code reading `result.detections` received `undefined`, meaning the LogUpload PII review workflow was silently broken. Fixed as part of this work.
+
+---
+
+## Acceptance Criteria
+
+- [x] Attaching a text file containing PII blocks send with a descriptive error naming the file and PII types
+- [x] The user is directed to use Log Analysis to redact before attaching
+- [x] Attaching a clean text file proceeds normally
+- [x] Attaching an image (binary, `content === null`) skips PII scan and proceeds
+- [x] Typing a message containing PII triggers a one-time warning; sending the same message a second time proceeds (explicit acknowledgment)
+- [x] After a successful send the warning state is cleared
+- [x] Backend `chat_message` independently rejects attachment content containing PII, regardless of frontend state
+- [x] Backend attachment parser catches headers both with and without trailing `---`
+- [x] `detectPiiCmd` returns `detections: PiiSpan[]` and `total_pii_found: number` matching the TypeScript contract
+- [x] All Rust and frontend tests pass; zero clippy warnings; tsc clean
+
+---
+
+## Work Implemented
+
+### `src-tauri/src/commands/analysis.rs`
+- Fixed `detect_pii` to return `db::models::PiiDetectionResult` (`detections`, `total_pii_found`) instead of `pii::PiiDetectionResult` (`spans`, `original_text`)
+- Added `scan_text_for_pii(text: String)` command: scans arbitrary text for PII without creating DB records
+
+### `src-tauri/src/commands/ai.rs`
+- Added defence-in-depth PII scan inside `chat_message` before user message is appended to the AI request
+- Extracts body content from all `--- Attached:` blocks; catches headers with and without trailing `---`
+- Returns a hard error if PII spans are found in attachment content
+
+### `src-tauri/src/lib.rs`
+- Registered `scan_text_for_pii` in `generate_handler![]`
+
+### `src/lib/tauriCommands.ts`
+- Added `scanTextForPiiCmd(text: string)` wrapper
+
+### `src/pages/Triage/index.tsx`
+- Updated `PendingFile` type: added `logFileId: string`
+- Stored `logFile.id` when attaching files
+- Added attachment PII gate in `handleSend`: calls `detectPiiCmd` on each text attachment; hard-blocks if PII found
+- Added message PII warning in `handleSend`: calls `scanTextForPiiCmd` on typed message; warns once; second send with same message proceeds
+- Added `piiWarnedMessageRef` to track prior warning state; cleared in `finally` after every send attempt
+
+---
+
+## Testing Needed
+
+1. Attach a file containing `password: secret123` → send is blocked; error names the file and PII type
+2. Attach a clean text file → send proceeds
+3. Attach an image (.png) → no PII scan, send proceeds
+4. Type `My password is abc123!!` in chat → first send shows PII warning
+5. Send the same message again → proceeds (acknowledgment)
+6. Send a different message → prior warning cleared, sends normally
+7. On LogUpload page, upload a file with a known IP/email → confirm PII spans appear in the review UI (was previously broken due to wrong struct)
+8. Directly call `chat_message` IPC with a message containing `--- Attached: test ---\npassword: secret` → backend returns error
+9. `cargo test` → 228/228 pass; `npm run test:run` → 103/103 pass

src-tauri/src/commands/ai.rs

@@ -288,6 +288,44 @@ pub async fn chat_message(
         messages.push(context_message);
     }
 
+    // Defence-in-depth: reject messages containing unredacted file attachment content.
+    // The frontend gates this too, but we enforce it here as a hard stop.
+    if message.contains("--- Attached:") {
+        let mut in_attachment = false;
+        let mut body = String::new();
+        for line in message.lines() {
+            if line.starts_with("--- Attached:") {
+                in_attachment = true;
+            } else if in_attachment {
+                body.push_str(line);
+                body.push('\n');
+            }
+        }
+
+        if !body.is_empty() {
+            let detector = crate::pii::PiiDetector::new();
+            let spans = detector.detect(&body);
+            if !spans.is_empty() {
+                let mut types: Vec<&str> = {
+                    use std::collections::HashSet;
+                    spans
+                        .iter()
+                        .map(|s| s.pii_type.as_str())
+                        .collect::<HashSet<_>>()
+                        .into_iter()
+                        .collect()
+                };
+                types.sort_unstable();
+                return Err(format!(
+                    "PII detected in attached file content ({} items: {}). \
+                     Use Log Analysis to redact before attaching to AI messages.",
+                    spans.len(),
+                    types.join(", ")
+                ));
+            }
+        }
+    }
+
     messages.push(Message {
         role: "user".into(),
         content: message.clone(),

src-tauri/src/commands/analysis.rs

@@ -8,8 +8,8 @@ use std::path::{Path, PathBuf};
 use tauri::State;
 use tracing::warn;
 
-use crate::db::models::{AuditEntry, LogFile, LogFileSummary, PiiSpanRecord};
-use crate::pii::{self, PiiDetectionResult, PiiDetector, RedactedLogFile};
+use crate::db::models::{AuditEntry, LogFile, LogFileSummary, PiiDetectionResult, PiiSpanRecord};
+use crate::pii::{self, PiiDetector, RedactedLogFile};
 use crate::state::AppState;
 
 const MAX_LOG_FILE_BYTES: u64 = 50 * 1024 * 1024;
@@ -440,10 +440,25 @@ pub async fn detect_pii(
         }
     }
 
+    let total_pii_found = spans.len();
     Ok(PiiDetectionResult {
         log_file_id,
-        spans,
-        original_text: content,
+        detections: spans,
+        total_pii_found,
+    })
+}
+
+/// Scan arbitrary text for PII without creating any database records.
+/// Used to warn users before sending typed chat messages to AI providers.
+#[tauri::command]
+pub async fn scan_text_for_pii(text: String) -> Result<PiiDetectionResult, String> {
+    let detector = PiiDetector::new();
+    let spans = detector.detect(&text);
+    let total_pii_found = spans.len();
+    Ok(PiiDetectionResult {
+        log_file_id: String::new(),
+        detections: spans,
+        total_pii_found,
     })
 }
 

src-tauri/src/lib.rs

@@ -85,6 +85,7 @@ pub fn run() {
             commands::analysis::upload_log_file,
             commands::analysis::upload_log_file_by_content,
             commands::analysis::detect_pii,
+            commands::analysis::scan_text_for_pii,
             commands::analysis::apply_redactions,
             commands::analysis::get_log_file_content,
             commands::analysis::list_all_log_files,

src/lib/tauriCommands.ts

@@ -308,6 +308,9 @@ export const deleteImageAttachmentCmd = (attachmentId: string) =>
 export const detectPiiCmd = (logFileId: string) =>
   invoke<PiiDetectionResult>("detect_pii", { logFileId });
 
+export const scanTextForPiiCmd = (text: string) =>
+  invoke<PiiDetectionResult>("scan_text_for_pii", { text });
+
 export const applyRedactionsCmd = (logFileId: string, approvedSpanIds: string[]) =>
   invoke<RedactedLogFile>("apply_redactions", { logFileId, approvedSpanIds });
 

src/pages/Triage/index.tsx

@@ -9,6 +9,8 @@ import { useSessionStore } from "@/stores/sessionStore";
 import { useSettingsStore } from "@/stores/settingsStore";
 import {
   chatMessageCmd,
+  detectPiiCmd,
+  scanTextForPiiCmd,
   getIssueCmd,
   getIssueMessagesCmd,
   uploadLogFileCmd,
@@ -34,7 +36,7 @@ function isCloseIntent(message: string): boolean {
   return CLOSE_PATTERNS.some((p) => lower.includes(p));
 }
 
-type PendingFile = { name: string; content: string | null };
+type PendingFile = { name: string; content: string | null; logFileId: string };
 
 export default function Triage() {
   const { id } = useParams<{ id: string }>();
@@ -45,6 +47,10 @@ export default function Triage() {
   // Track the last user message so we can save it as a resolution step when why level advances
   const lastUserMsgRef = useRef<string>("");
   const initialized = useRef(false);
+  // PII warning state: if the user sends a message that triggers a PII warning,
+  // store it here. Sending the same message a second time bypasses the warning
+  // (explicit acknowledgment). Cleared after each successful send.
+  const piiWarnedMessageRef = useRef<string>("");
 
   const { currentIssue, messages, currentWhyLevel, activeDomain, startSession, addMessage, setWhyLevel, setActiveDomain } =
     useSessionStore();
@@ -108,7 +114,7 @@ export default function Triage() {
         } catch {
           // Binary file (image) — include filename only as context
         }
-        setPendingFiles((prev) => [...prev, { name: logFile.file_name, content }]);
+        setPendingFiles((prev) => [...prev, { name: logFile.file_name, content, logFileId: logFile.id }]);
       }
     } catch (e) {
       setError(`Attachment failed: ${String(e)}`);
@@ -142,6 +148,49 @@ export default function Triage() {
     setIsLoading(true);
     setError(null);
 
+    // PII gate: scan each text attachment before it is sent to an AI provider.
+    // Images (content === null) have no text to scan.
+    for (const f of pendingFiles) {
+      if (f.content === null) continue;
+      try {
+        const result = await detectPiiCmd(f.logFileId);
+        if (result.total_pii_found > 0) {
+          const types = [...new Set(result.detections.map((d) => d.pii_type))].join(", ");
+          setError(
+            `PII detected in "${f.name}" (${result.total_pii_found} item${result.total_pii_found !== 1 ? "s" : ""}: ${types}). ` +
+              `Open Log Analysis to redact before attaching.`
+          );
+          setIsLoading(false);
+          return;
+        }
+      } catch (e) {
+        setError(`PII scan failed for "${f.name}": ${String(e)}`);
+        setIsLoading(false);
+        return;
+      }
+    }
+
+    // PII warning for typed message text. Unlike attachments (hard block), typed
+    // messages only warn once — sending the same text a second time is treated as
+    // explicit acknowledgment and proceeds.
+    if (message.trim() && message !== piiWarnedMessageRef.current) {
+      try {
+        const textResult = await scanTextForPiiCmd(message);
+        if (textResult.total_pii_found > 0) {
+          const types = [...new Set(textResult.detections.map((d) => d.pii_type))].join(", ");
+          piiWarnedMessageRef.current = message;
+          setError(
+            `Your message may contain sensitive data (${types} detected). ` +
+              `Edit your message to remove it, or send again to proceed.`
+          );
+          setIsLoading(false);
+          return;
+        }
+      } catch {
+        // Non-fatal: if the scan fails, do not block the send
+      }
+    }
+
     // Build AI context: user text + file contents; show only text + filenames in chat UI
     const fileContext = pendingFiles
       .map((f) =>
@@ -214,6 +263,7 @@ export default function Triage() {
       setError(String(e));
     } finally {
       setIsLoading(false);
+      piiWarnedMessageRef.current = "";
     }
   };