fix(security): address PR review — move attachment handling to backend, auto-redact PII
Some checks failed
Test / rust-fmt-check (pull_request) Failing after 1m25s
Test / frontend-typecheck (pull_request) Successful in 1m37s
Test / frontend-tests (pull_request) Successful in 1m36s
Test / rust-clippy (pull_request) Failing after 3m18s
PR Review Automation / review (pull_request) Successful in 4m19s
Test / rust-tests (pull_request) Successful in 4m30s

Resolves all four findings from the automated review:

[BLOCKER 1] Attachment PII scan error path left pendingFiles intact,
allowing retry with stale file references. Fix: file content is no
longer held in frontend state at all — PendingFile drops the content
field entirely. logFileIds are captured before setPendingFiles([]) and
passed directly to the backend.

[BLOCKER 2] Raw file content stored in PendingFile.content created a
UI-visible PII surface and a data-residency risk. Fix: frontend never
reads or stores file content. The backend loads file data from disk,
auto-redacts PII in-memory using pii::apply_redactions(), and embeds
the clean text into the AI message. No PII ever touches the frontend.

[WARNING 1] String-based attachment header parsing was fragile and
bypassable. Fix: parsing is gone — backend identifies attachments by
log_file_id, reads them directly from the DB/disk path, and applies
redaction at that level.

[WARNING 2] Error message disclosed PII type list to the caller. Fix:
PII types are logged via tracing::warn only; no type details in the
user-facing error or API response.

Additionally: typed chat messages are now auto-redacted rather than
blocked. scanTextForPiiCmd runs on the typed text; detected spans are
replaced in reverse-offset order before the message is sent to the AI
and stored in the DB. The user sees the redacted form in their chat
bubble.

Architecture:
- chat_message now accepts log_file_ids: Option<Vec<String>>
- Backend reads file → detects PII → redacts in memory → embeds
- Frontend: no readTextFile, no content field, no frontend PII gate
This commit is contained in:
Shaun Arman 2026-05-31 19:20:46 -05:00
parent cd26801a39
commit f05b954250
3 changed files with 85 additions and 108 deletions

View File

@ -165,6 +165,7 @@ fn extract_list(text: &str, header: &str) -> Vec<String> {
pub async fn chat_message( pub async fn chat_message(
issue_id: String, issue_id: String,
message: String, message: String,
log_file_ids: Option<Vec<String>>,
provider_config: ProviderConfig, provider_config: ProviderConfig,
system_prompt: Option<String>, system_prompt: Option<String>,
app_handle: tauri::AppHandle, app_handle: tauri::AppHandle,
@ -233,6 +234,54 @@ pub async fn chat_message(
.collect() .collect()
}; };
// Load attachment files from DB, scan for PII, and embed clean content into the message.
// File content never passes through the frontend — the backend is the single source of truth.
let full_message = {
let files: Vec<(String, String)> = if let Some(ref ids) = log_file_ids {
let db = state.db.lock().map_err(|e| e.to_string())?;
let mut v = Vec::new();
for file_id in ids {
if let Ok((name, path)) = db
.prepare("SELECT file_name, file_path FROM log_files WHERE id = ?1")
.and_then(|mut s| {
s.query_row([file_id], |row| {
Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?))
})
})
{
v.push((name, path));
}
}
v
} else {
vec![]
};
let mut msg = message.clone();
for (file_name, file_path) in &files {
let content = std::fs::read_to_string(file_path).unwrap_or_default();
let preview = &content[..content.len().min(8000)];
let spans = crate::pii::PiiDetector::new().detect(preview);
let body = if spans.is_empty() {
preview.to_string()
} else {
let types: std::collections::HashSet<&str> =
spans.iter().map(|s| s.pii_type.as_str()).collect();
let mut type_list: Vec<&str> = types.into_iter().collect();
type_list.sort_unstable();
warn!(
file_name = %file_name,
pii_types = ?type_list,
pii_count = spans.len(),
"PII detected in chat attachment — auto-redacting before AI send"
);
crate::pii::apply_redactions(preview, &spans)
};
msg.push_str(&format!("\n\n--- Attached: {} ---\n{}", file_name, body));
}
msg
};
let provider = create_provider(&provider_config); let provider = create_provider(&provider_config);
// Search integration sources for relevant context // Search integration sources for relevant context
@ -288,47 +337,9 @@ pub async fn chat_message(
messages.push(context_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 { messages.push(Message {
role: "user".into(), role: "user".into(),
content: message.clone(), content: full_message.clone(),
tool_call_id: None, tool_call_id: None,
tool_calls: None, tool_calls: None,
}); });
@ -400,7 +411,7 @@ pub async fn chat_message(
// Save both user message and response to DB // Save both user message and response to DB
{ {
let db = state.db.lock().map_err(|e| e.to_string())?; let db = state.db.lock().map_err(|e| e.to_string())?;
let user_msg = AiMessage::new(conversation_id.clone(), "user".to_string(), message); let user_msg = AiMessage::new(conversation_id.clone(), "user".to_string(), full_message);
let asst_msg = AiMessage::new( let asst_msg = AiMessage::new(
conversation_id, conversation_id,
"assistant".to_string(), "assistant".to_string(),

View File

@ -271,8 +271,20 @@ export interface TriageMessage {
export const analyzeLogsCmd = (issueId: string, logFileIds: string[], providerConfig: ProviderConfig) => export const analyzeLogsCmd = (issueId: string, logFileIds: string[], providerConfig: ProviderConfig) =>
invoke<AnalysisResult>("analyze_logs", { issueId, logFileIds, providerConfig }); invoke<AnalysisResult>("analyze_logs", { issueId, logFileIds, providerConfig });
export const chatMessageCmd = (issueId: string, message: string, providerConfig: ProviderConfig, systemPrompt?: string) => export const chatMessageCmd = (
invoke<ChatResponse>("chat_message", { issueId, message, providerConfig, systemPrompt: systemPrompt ?? null }); issueId: string,
message: string,
logFileIds: string[],
providerConfig: ProviderConfig,
systemPrompt?: string
) =>
invoke<ChatResponse>("chat_message", {
issueId,
message,
logFileIds: logFileIds.length > 0 ? logFileIds : null,
providerConfig,
systemPrompt: systemPrompt ?? null,
});
export const listProvidersCmd = () => invoke<ProviderInfo[]>("list_providers"); export const listProvidersCmd = () => invoke<ProviderInfo[]>("list_providers");

View File

@ -2,14 +2,12 @@ import { useEffect, useRef, useState } from "react";
import { useParams, useNavigate } from "react-router-dom"; import { useParams, useNavigate } from "react-router-dom";
import { CheckCircle, ChevronRight } from "lucide-react"; import { CheckCircle, ChevronRight } from "lucide-react";
import { open } from "@tauri-apps/plugin-dialog"; import { open } from "@tauri-apps/plugin-dialog";
import { readTextFile } from "@tauri-apps/plugin-fs";
import { ChatWindow } from "@/components/ChatWindow"; import { ChatWindow } from "@/components/ChatWindow";
import { TriageProgress } from "@/components/TriageProgress"; import { TriageProgress } from "@/components/TriageProgress";
import { useSessionStore } from "@/stores/sessionStore"; import { useSessionStore } from "@/stores/sessionStore";
import { useSettingsStore } from "@/stores/settingsStore"; import { useSettingsStore } from "@/stores/settingsStore";
import { import {
chatMessageCmd, chatMessageCmd,
detectPiiCmd,
scanTextForPiiCmd, scanTextForPiiCmd,
getIssueCmd, getIssueCmd,
getIssueMessagesCmd, getIssueMessagesCmd,
@ -36,7 +34,7 @@ function isCloseIntent(message: string): boolean {
return CLOSE_PATTERNS.some((p) => lower.includes(p)); return CLOSE_PATTERNS.some((p) => lower.includes(p));
} }
type PendingFile = { name: string; content: string | null; logFileId: string }; type PendingFile = { name: string; logFileId: string };
export default function Triage() { export default function Triage() {
const { id } = useParams<{ id: string }>(); const { id } = useParams<{ id: string }>();
@ -47,10 +45,6 @@ export default function Triage() {
// Track the last user message so we can save it as a resolution step when why level advances // Track the last user message so we can save it as a resolution step when why level advances
const lastUserMsgRef = useRef<string>(""); const lastUserMsgRef = useRef<string>("");
const initialized = useRef(false); 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 } = const { currentIssue, messages, currentWhyLevel, activeDomain, startSession, addMessage, setWhyLevel, setActiveDomain } =
useSessionStore(); useSessionStore();
@ -107,14 +101,7 @@ export default function Triage() {
const paths = Array.isArray(selected) ? selected : [selected]; const paths = Array.isArray(selected) ? selected : [selected];
for (const filePath of paths) { for (const filePath of paths) {
const logFile = await uploadLogFileCmd(id, filePath); const logFile = await uploadLogFileCmd(id, filePath);
let content: string | null = null; setPendingFiles((prev) => [...prev, { name: logFile.file_name, logFileId: logFile.id }]);
try {
const raw = await readTextFile(filePath);
content = raw.slice(0, 8000); // cap at 8 KB to keep context manageable
} catch {
// Binary file (image) — include filename only as context
}
setPendingFiles((prev) => [...prev, { name: logFile.file_name, content, logFileId: logFile.id }]);
} }
} catch (e) { } catch (e) {
setError(`Attachment failed: ${String(e)}`); setError(`Attachment failed: ${String(e)}`);
@ -148,62 +135,29 @@ export default function Triage() {
setIsLoading(true); setIsLoading(true);
setError(null); setError(null);
// PII gate: scan each text attachment before it is sent to an AI provider. // Auto-redact PII in typed message text before sending to AI.
// Images (content === null) have no text to scan. // Spans are replaced in reverse-start-offset order to preserve byte positions.
for (const f of pendingFiles) { let outMessage = message;
if (f.content === null) continue; if (message.trim()) {
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 { try {
const textResult = await scanTextForPiiCmd(message); const textResult = await scanTextForPiiCmd(message);
if (textResult.total_pii_found > 0) { if (textResult.total_pii_found > 0) {
const types = [...new Set(textResult.detections.map((d) => d.pii_type))].join(", "); const sorted = [...textResult.detections].sort((a, b) => b.start - a.start);
piiWarnedMessageRef.current = message; let redacted = message;
setError( for (const span of sorted) {
`Your message may contain sensitive data (${types} detected). ` + redacted = redacted.slice(0, span.start) + span.replacement + redacted.slice(span.end);
`Edit your message to remove it, or send again to proceed.` }
); outMessage = redacted;
setIsLoading(false);
return;
} }
} catch { } catch {
// Non-fatal: if the scan fails, do not block the send // Non-fatal: if the scan fails, send original
} }
} }
// Build AI context: user text + file contents; show only text + filenames in chat UI
const fileContext = pendingFiles
.map((f) =>
f.content
? `\n\n--- Attached: ${f.name} ---\n${f.content}`
: `\n\n[Image attached: ${f.name} — describe what you see if relevant]`
)
.join("");
const aiMessage = pendingFiles.length > 0 ? `${message}${fileContext}` : message;
const displayContent = const displayContent =
pendingFiles.length > 0 pendingFiles.length > 0
? `${message}${message ? "\n" : ""}📎 ${pendingFiles.map((f) => f.name).join(", ")}` ? `${outMessage}${outMessage ? "\n" : ""}📎 ${pendingFiles.map((f) => f.name).join(", ")}`
: message; : outMessage;
const userMsg: TriageMessage = { const userMsg: TriageMessage = {
id: `user-${Date.now()}`, id: `user-${Date.now()}`,
@ -213,23 +167,24 @@ export default function Triage() {
why_level: currentWhyLevel, why_level: currentWhyLevel,
created_at: Date.now(), created_at: Date.now(),
}; };
lastUserMsgRef.current = message; lastUserMsgRef.current = outMessage;
addMessage(userMsg); addMessage(userMsg);
const logFileIds = pendingFiles.map((f) => f.logFileId);
setPendingFiles([]); setPendingFiles([]);
try { try {
// Detect domain from conversation messages // Detect domain from conversation messages
const messageContents = messages.map((m) => m.content); const messageContents = messages.map((m) => m.content);
const detectedDomain = detectDomain(messageContents); const detectedDomain = detectDomain(messageContents);
// Update active domain if it has changed // Update active domain if it has changed
if (detectedDomain !== activeDomain && detectedDomain !== "general") { if (detectedDomain !== activeDomain && detectedDomain !== "general") {
setActiveDomain(detectedDomain); setActiveDomain(detectedDomain);
} }
// Use the active domain for the system prompt // Use the active domain for the system prompt
const systemPrompt = activeDomain ? getDomainPrompt(activeDomain) : undefined; const systemPrompt = activeDomain ? getDomainPrompt(activeDomain) : undefined;
const response = await chatMessageCmd(id, aiMessage, provider, systemPrompt); const response = await chatMessageCmd(id, outMessage, logFileIds, provider, systemPrompt);
const assistantMsg: TriageMessage = { const assistantMsg: TriageMessage = {
id: `asst-${Date.now()}`, id: `asst-${Date.now()}`,
issue_id: id, issue_id: id,
@ -263,7 +218,6 @@ export default function Triage() {
setError(String(e)); setError(String(e));
} finally { } finally {
setIsLoading(false); setIsLoading(false);
piiWarnedMessageRef.current = "";
} }
}; };