From cd26801a39c966982fca134c797d02eede30c2e4 Mon Sep 17 00:00:00 2001 From: Shaun Arman Date: Sun, 31 May 2026 19:05:51 -0500 Subject: [PATCH] fix(security): block PII in chat attachments and typed messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File attachments were embedded into AI messages without any PII scanning, allowing credentials, tokens, and other sensitive data to be forwarded to AI providers in plaintext. Typed chat messages had the same gap: a user could type a password or API key directly and it would be sent unscanned. Changes: - chat_message (Rust): defence-in-depth scan of all attachment body content (between --- Attached: markers); hard rejects if PII found - detect_pii (Rust): fix return type from pii::PiiDetectionResult (spans/original_text) to db::models::PiiDetectionResult (detections/total_pii_found) to match the TypeScript contract; the LogUpload PII review workflow was receiving undefined for detections - scan_text_for_pii (Rust): new command — scans arbitrary text for PII without creating DB records; used for typed message warnings - Triage/index.tsx: PendingFile now carries logFileId; handleSend gates each text attachment through detectPiiCmd (hard block on PII found); typed message text scanned via scanTextForPiiCmd with a one-time warning — second send of same message proceeds as acknowledgment --- TICKET-pii-bypass-chat-attachments.md | 78 +++++++++++++++++++++++++++ src-tauri/src/commands/ai.rs | 38 +++++++++++++ src-tauri/src/commands/analysis.rs | 23 ++++++-- src-tauri/src/lib.rs | 1 + src/lib/tauriCommands.ts | 3 ++ src/pages/Triage/index.tsx | 54 ++++++++++++++++++- 6 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 TICKET-pii-bypass-chat-attachments.md 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 = ""; } };