fix: audit PII redaction metadata, safe bubble update, update ticket
All checks were successful
Test / rust-fmt-check (pull_request) Successful in 1m54s
Test / frontend-typecheck (pull_request) Successful in 2m6s
Test / frontend-tests (pull_request) Successful in 2m5s
Test / rust-clippy (pull_request) Successful in 3m59s
PR Review Automation / review (pull_request) Successful in 4m10s
Test / rust-tests (pull_request) Successful in 5m15s
All checks were successful
Test / rust-fmt-check (pull_request) Successful in 1m54s
Test / frontend-typecheck (pull_request) Successful in 2m6s
Test / frontend-tests (pull_request) Successful in 2m5s
Test / rust-clippy (pull_request) Successful in 3m59s
PR Review Automation / review (pull_request) Successful in 4m10s
Test / rust-tests (pull_request) Successful in 5m15s
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.
This commit is contained in:
parent
631221dbf1
commit
249d20bf85
@ -1,6 +1,6 @@
|
|||||||
# TICKET: PII Detection Bypass in AI Chat
|
# 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)
|
### 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)
|
### 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`
|
### 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
|
## Acceptance Criteria
|
||||||
|
|
||||||
- [x] Attaching a text file containing PII blocks send with a descriptive error naming the file and PII types
|
- [x] Attaching a text file containing PII sends successfully; content is auto-redacted before the AI sees it
|
||||||
- [x] The user is directed to use Log Analysis to redact before attaching
|
- [x] Attaching a clean text file proceeds normally with no modification
|
||||||
- [x] Attaching a clean text file proceeds normally
|
- [x] PII detection runs on the full file content before truncating to the 8 KB embed limit (no PII straddling the boundary)
|
||||||
- [x] Attaching an image (binary, `content === null`) skips PII scan and proceeds
|
- [x] Typed messages containing PII are auto-redacted before being sent to the AI provider
|
||||||
- [x] Typing a message containing PII triggers a one-time warning; sending the same message a second time proceeds (explicit acknowledgment)
|
- [x] The chat bubble is updated post-send to show the redacted form of the user's message
|
||||||
- [x] After a successful send the warning state is cleared
|
- [x] The audit log records `was_pii_redacted`, `pii_types_redacted`, and the full redacted `user_message`
|
||||||
- [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] `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
|
## Work Implemented
|
||||||
|
|
||||||
### `src-tauri/src/commands/analysis.rs`
|
### `src-tauri/src/ai/mod.rs`
|
||||||
- Fixed `detect_pii` to return `db::models::PiiDetectionResult` (`detections`, `total_pii_found`) instead of `pii::PiiDetectionResult` (`spans`, `original_text`)
|
- Added `user_message: Option<String>` to `ChatResponse` — set by `chat_message`, absent from direct provider calls
|
||||||
- Added `scan_text_for_pii(text: String)` command: scans arbitrary text for PII without creating DB records
|
|
||||||
|
### `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`
|
### `src-tauri/src/commands/ai.rs`
|
||||||
- Added defence-in-depth PII scan inside `chat_message` before user message is appended to the AI request
|
- `chat_message` now accepts `log_file_ids: Option<Vec<String>>`
|
||||||
- Extracts body content from all `--- Attached:` blocks; catches headers with and without trailing `---`
|
- Step 1: auto-redacts the typed message text with `PiiDetector` + `apply_redactions`
|
||||||
- Returns a hard error if PII spans are found in attachment content
|
- 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`
|
### `src-tauri/src/lib.rs`
|
||||||
- Registered `scan_text_for_pii` in `generate_handler![]`
|
- Registered `scan_text_for_pii`
|
||||||
|
|
||||||
### `src/lib/tauriCommands.ts`
|
### `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`
|
### `src/pages/Triage/index.tsx`
|
||||||
- Updated `PendingFile` type: added `logFileId: string`
|
- `PendingFile` type: `{ name: string; logFileId: string }` — no raw content stored
|
||||||
- Stored `logFile.id` when attaching files
|
- `handleAttach`: only uploads the file and stores `logFileId`; no `readTextFile`
|
||||||
- Added attachment PII gate in `handleSend`: calls `detectPiiCmd` on each text attachment; hard-blocks if PII found
|
- `handleSend`: passes `logFileIds` to backend; after response updates the bubble with `(response.user_message ?? message) + suffix`
|
||||||
- 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
|
## Testing Needed
|
||||||
|
|
||||||
1. Attach a file containing `password: secret123` → send is blocked; error names the file and PII type
|
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 → send proceeds
|
2. Attach a clean text file → content appears unmodified in the chat context
|
||||||
3. Attach an image (.png) → no PII scan, send proceeds
|
3. Attach a file where PII appears near the 8000-byte mark → content is fully redacted before truncation
|
||||||
4. Type `My password is abc123!!` in chat → first send shows PII warning
|
4. Type `My password is abc123!!` → message sends; bubble shows `My [Password] is [Password]`
|
||||||
5. Send the same message again → proceeds (acknowledgment)
|
5. On LogUpload page, upload a file with a known IP/email → PII spans appear in the review UI
|
||||||
6. Send a different message → prior warning cleared, sends normally
|
6. Check audit log after a PII-containing message: `was_pii_redacted: true`, `pii_types_redacted` populated
|
||||||
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)
|
7. Check audit log after a clean message: `was_pii_redacted: false`, `pii_types_redacted: []`
|
||||||
8. Directly call `chat_message` IPC with a message containing `--- Attached: test ---\npassword: secret` → backend returns error
|
8. `cargo test` → 228/228 pass; `npm run test:run` → 103/103 pass; `cargo fmt --check` clean; `npx tsc --noEmit` clean
|
||||||
9. `cargo test` → 228/228 pass; `npm run test:run` → 103/103 pass
|
|
||||||
|
|||||||
@ -236,6 +236,9 @@ pub async fn chat_message(
|
|||||||
|
|
||||||
// Auto-redact PII in both the typed message and any file attachments.
|
// 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.
|
// 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<String> = Vec::new();
|
||||||
|
|
||||||
let full_message = {
|
let full_message = {
|
||||||
// Step 1: redact the typed user message text.
|
// Step 1: redact the typed user message text.
|
||||||
let base = {
|
let base = {
|
||||||
@ -252,6 +255,8 @@ pub async fn chat_message(
|
|||||||
pii_count = spans.len(),
|
pii_count = spans.len(),
|
||||||
"PII detected in typed chat message — auto-redacting before AI send"
|
"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)
|
crate::pii::apply_redactions(&message, &spans)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@ -297,6 +302,8 @@ pub async fn chat_message(
|
|||||||
pii_count = spans.len(),
|
pii_count = spans.len(),
|
||||||
"PII detected in chat attachment — auto-redacting before AI send"
|
"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)
|
crate::pii::apply_redactions(&content, &spans)
|
||||||
};
|
};
|
||||||
// Truncate after redaction so the cut never lands inside a PII span.
|
// 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())?;
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
// Audit - capture full transmission details
|
// Audit - capture full transmission details
|
||||||
|
let pii_types_for_audit = {
|
||||||
|
use std::collections::HashSet;
|
||||||
|
let mut v: Vec<String> = redacted_pii_types
|
||||||
|
.into_iter()
|
||||||
|
.collect::<HashSet<_>>()
|
||||||
|
.into_iter()
|
||||||
|
.collect();
|
||||||
|
v.sort_unstable();
|
||||||
|
v
|
||||||
|
};
|
||||||
let audit_details = serde_json::json!({
|
let audit_details = serde_json::json!({
|
||||||
"provider": provider_config.name,
|
"provider": provider_config.name,
|
||||||
"model": provider_config.model,
|
"model": provider_config.model,
|
||||||
"api_url": provider_config.api_url,
|
"api_url": provider_config.api_url,
|
||||||
"user_message": user_msg.content,
|
"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 {
|
"response_preview": if final_response.content.len() > 200 {
|
||||||
format!("{preview}...", preview = &final_response.content[..200])
|
format!("{preview}...", preview = &final_response.content[..200])
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@ -169,10 +169,9 @@ export default function Triage() {
|
|||||||
const response = await chatMessageCmd(id, message, logFileIds, provider, systemPrompt);
|
const response = await chatMessageCmd(id, message, logFileIds, provider, systemPrompt);
|
||||||
|
|
||||||
// Update the user bubble with what was actually stored (may be auto-redacted).
|
// Update the user bubble with what was actually stored (may be auto-redacted).
|
||||||
if (response.user_message) {
|
// Fall back to the original message if user_message is absent (older backend builds).
|
||||||
const suffix = fileNames.length > 0 ? `\n📎 ${fileNames.join(", ")}` : "";
|
const suffix = fileNames.length > 0 ? `\n📎 ${fileNames.join(", ")}` : "";
|
||||||
updateMessageContent(userMsg.id, response.user_message + suffix);
|
updateMessageContent(userMsg.id, (response.user_message ?? message) + suffix);
|
||||||
}
|
|
||||||
|
|
||||||
const assistantMsg: TriageMessage = {
|
const assistantMsg: TriageMessage = {
|
||||||
id: `asst-${Date.now()}`,
|
id: `asst-${Date.now()}`,
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user