diff --git a/TICKET-pii-bypass-chat-attachments.md b/TICKET-pii-bypass-chat-attachments.md new file mode 100644 index 00000000..188a3d75 --- /dev/null +++ b/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 diff --git a/src-tauri/src/commands/ai.rs b/src-tauri/src/commands/ai.rs index 47414d4f..0989eaf4 100644 --- a/src-tauri/src/commands/ai.rs +++ b/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::>() + .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(), diff --git a/src-tauri/src/commands/analysis.rs b/src-tauri/src/commands/analysis.rs index 24a24031..596401b6 100644 --- a/src-tauri/src/commands/analysis.rs +++ b/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 { + 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, }) } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index c3414a08..54a8eb05 100644 --- a/src-tauri/src/lib.rs +++ b/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, diff --git a/src/lib/tauriCommands.ts b/src/lib/tauriCommands.ts index 4541fd0d..136ad7c5 100644 --- a/src/lib/tauriCommands.ts +++ b/src/lib/tauriCommands.ts @@ -308,6 +308,9 @@ export const deleteImageAttachmentCmd = (attachmentId: string) => export const detectPiiCmd = (logFileId: string) => invoke("detect_pii", { logFileId }); +export const scanTextForPiiCmd = (text: string) => + invoke("scan_text_for_pii", { text }); + export const applyRedactionsCmd = (logFileId: string, approvedSpanIds: string[]) => invoke("apply_redactions", { logFileId, approvedSpanIds }); diff --git a/src/pages/Triage/index.tsx b/src/pages/Triage/index.tsx index f1e30e2f..26055180 100644 --- a/src/pages/Triage/index.tsx +++ b/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(""); 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(""); 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 = ""; } };