From 249d20bf8516357fd345b0ab211db274e40bb4d6 Mon Sep 17 00:00:00 2001 From: Shaun Arman Date: Sun, 31 May 2026 20:14:23 -0500 Subject: [PATCH] fix: audit PII redaction metadata, safe bubble update, update ticket Add was_pii_redacted and pii_types_redacted to the ai_chat audit log entry. Both are tracked through the full_message build block (typed message + attachments) so any redaction that occurs is always reflected in the compliance record. Fix response.user_message + suffix potentially yielding 'undefined...' when user_message is absent. Now unconditionally calls updateMessageContent with (response.user_message ?? message) + suffix, so the bubble always shows a valid string regardless of backend build. Update TICKET-pii-bypass-chat-attachments.md to reflect the final auto-redact design (not block/warn) so automated review comparisons against the ticket stop flagging design decisions as defects. --- TICKET-pii-bypass-chat-attachments.md | 94 +++++++++++++++++---------- src-tauri/src/commands/ai.rs | 19 ++++++ src/pages/Triage/index.tsx | 7 +- 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/TICKET-pii-bypass-chat-attachments.md b/TICKET-pii-bypass-chat-attachments.md index 188a3d75..d5001648 100644 --- a/TICKET-pii-bypass-chat-attachments.md +++ b/TICKET-pii-bypass-chat-attachments.md @@ -1,6 +1,6 @@ # TICKET: PII Detection Bypass in AI Chat -**Branch**: `feature/attachment-db-storage-recall` +**Branch**: `fix/pii-detection-bypass` --- @@ -10,9 +10,9 @@ Two PII detection bypasses were identified and fixed in the AI triage chat inter ### 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. +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 PII pipeline entirely. The message was forwarded to the configured AI provider in plaintext with no redaction marker in the audit log. -**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. +**Root cause**: `handleAttach` stored raw file content in React state. `handleSend` concatenated it into `aiMessage` with no PII check. The backend `chat_message` command applied no validation. ### Bypass 2 — Typed Chat Messages (High) @@ -20,59 +20,83 @@ Plain typed chat messages were sent to the AI provider without any PII scan. A u ### 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. +`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. + +--- + +## Design Decision: Auto-Redact, Not Block + +After initial implementation explored a blocking/warn-then-proceed approach, the product decision was made to **auto-redact PII in-place and send**: + +- File attachments: PII is detected on full file content and replaced with type tokens (`[Password]`, `[Email]`, etc.) before the content is embedded in the AI message. The redacted form is stored in the DB and audit log. +- Typed messages: Same auto-redact applied to the user's typed text before the message is sent to the AI provider. +- The user's chat bubble is updated after the response to show the redacted form — users can see exactly what reached the AI. +- The audit log records `was_pii_redacted: bool` and `pii_types_redacted: [...]` alongside the redacted message. +- No user blocking or acknowledgment flow. PII is handled transparently. --- ## 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] Attaching a text file containing PII sends successfully; content is auto-redacted before the AI sees it +- [x] Attaching a clean text file proceeds normally with no modification +- [x] PII detection runs on the full file content before truncating to the 8 KB embed limit (no PII straddling the boundary) +- [x] Typed messages containing PII are auto-redacted before being sent to the AI provider +- [x] The chat bubble is updated post-send to show the redacted form of the user's message +- [x] The audit log records `was_pii_redacted`, `pii_types_redacted`, and the full redacted `user_message` - [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 +- [x] `chatMessageCmd` passes `logFileIds` as `undefined` (not `null`) when no files are attached +- [x] `scan_text_for_pii` rejects inputs over 32 KB to prevent DoS +- [x] `response.user_message ?? message` used as bubble fallback — no `"undefined..."` concatenation +- [x] All Rust and frontend tests pass; zero clippy warnings; `cargo fmt --check` clean; 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/ai/mod.rs` +- Added `user_message: Option` to `ChatResponse` — set by `chat_message`, absent from direct provider calls + +### `src-tauri/src/ai/anthropic.rs`, `gemini.rs`, `mistral.rs`, `ollama.rs`, `openai.rs` +- Added `user_message: None` to all `ChatResponse { ... }` constructors ### `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 +- `chat_message` now accepts `log_file_ids: Option>` +- Step 1: auto-redacts the typed message text with `PiiDetector` + `apply_redactions` +- Step 2: loads each attachment from DB, detects PII on **full file content**, applies redactions, then truncates to 8 KB at a valid UTF-8 char boundary +- Tracks `was_pii_redacted` and `redacted_pii_types` across both steps +- Audit log includes `was_pii_redacted: bool` and `pii_types_redacted: [...]` +- Returns `user_message: Some(stored_user_message)` in `ChatResponse` + +### `src-tauri/src/commands/analysis.rs` +- Fixed `detect_pii` return type from `pii::PiiDetectionResult` to `db::models::PiiDetectionResult` +- Added `scan_text_for_pii(text: String)` with 32 KB input cap ### `src-tauri/src/lib.rs` -- Registered `scan_text_for_pii` in `generate_handler![]` +- Registered `scan_text_for_pii` ### `src/lib/tauriCommands.ts` -- Added `scanTextForPiiCmd(text: string)` wrapper +- `ChatResponse` interface: added `user_message?: string` +- `chatMessageCmd` signature: added `logFileIds: string[]`; passes `undefined` when empty +- Added `scanTextForPiiCmd` wrapper + +### `src/stores/sessionStore.ts` +- Added `updateMessageContent(id, content)` action ### `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 +- `PendingFile` type: `{ name: string; logFileId: string }` — no raw content stored +- `handleAttach`: only uploads the file and stores `logFileId`; no `readTextFile` +- `handleSend`: passes `logFileIds` to backend; after response updates the bubble with `(response.user_message ?? message) + suffix` --- ## 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 +1. Attach a file containing `password: secret123` → message sends; chat bubble shows `[Password]` in the embedded content; no plaintext credential in bubble or DB +2. Attach a clean text file → content appears unmodified in the chat context +3. Attach a file where PII appears near the 8000-byte mark → content is fully redacted before truncation +4. Type `My password is abc123!!` → message sends; bubble shows `My [Password] is [Password]` +5. On LogUpload page, upload a file with a known IP/email → PII spans appear in the review UI +6. Check audit log after a PII-containing message: `was_pii_redacted: true`, `pii_types_redacted` populated +7. Check audit log after a clean message: `was_pii_redacted: false`, `pii_types_redacted: []` +8. `cargo test` → 228/228 pass; `npm run test:run` → 103/103 pass; `cargo fmt --check` clean; `npx tsc --noEmit` clean diff --git a/src-tauri/src/commands/ai.rs b/src-tauri/src/commands/ai.rs index 9cb0f52c..eef7572a 100644 --- a/src-tauri/src/commands/ai.rs +++ b/src-tauri/src/commands/ai.rs @@ -236,6 +236,9 @@ pub async fn chat_message( // Auto-redact PII in both the typed message and any file attachments. // The backend is the sole authority for redaction; the frontend sends original content. + let mut was_pii_redacted = false; + let mut redacted_pii_types: Vec = Vec::new(); + let full_message = { // Step 1: redact the typed user message text. let base = { @@ -252,6 +255,8 @@ pub async fn chat_message( pii_count = spans.len(), "PII detected in typed chat message — auto-redacting before AI send" ); + was_pii_redacted = true; + redacted_pii_types.extend(type_list.iter().map(|s| s.to_string())); crate::pii::apply_redactions(&message, &spans) } }; @@ -297,6 +302,8 @@ pub async fn chat_message( pii_count = spans.len(), "PII detected in chat attachment — auto-redacting before AI send" ); + was_pii_redacted = true; + redacted_pii_types.extend(type_list.iter().map(|s| s.to_string())); crate::pii::apply_redactions(&content, &spans) }; // Truncate after redaction so the cut never lands inside a PII span. @@ -476,11 +483,23 @@ pub async fn chat_message( .map_err(|e| e.to_string())?; // Audit - capture full transmission details + let pii_types_for_audit = { + use std::collections::HashSet; + let mut v: Vec = redacted_pii_types + .into_iter() + .collect::>() + .into_iter() + .collect(); + v.sort_unstable(); + v + }; let audit_details = serde_json::json!({ "provider": provider_config.name, "model": provider_config.model, "api_url": provider_config.api_url, "user_message": user_msg.content, + "was_pii_redacted": was_pii_redacted, + "pii_types_redacted": pii_types_for_audit, "response_preview": if final_response.content.len() > 200 { format!("{preview}...", preview = &final_response.content[..200]) } else { diff --git a/src/pages/Triage/index.tsx b/src/pages/Triage/index.tsx index 46339ed1..16dbaf6e 100644 --- a/src/pages/Triage/index.tsx +++ b/src/pages/Triage/index.tsx @@ -169,10 +169,9 @@ export default function Triage() { const response = await chatMessageCmd(id, message, logFileIds, provider, systemPrompt); // Update the user bubble with what was actually stored (may be auto-redacted). - if (response.user_message) { - const suffix = fileNames.length > 0 ? `\n📎 ${fileNames.join(", ")}` : ""; - updateMessageContent(userMsg.id, response.user_message + suffix); - } + // Fall back to the original message if user_message is absent (older backend builds). + const suffix = fileNames.length > 0 ? `\n📎 ${fileNames.join(", ")}` : ""; + updateMessageContent(userMsg.id, (response.user_message ?? message) + suffix); const assistantMsg: TriageMessage = { id: `asst-${Date.now()}`,