diff --git a/src/cli/commands/channels.ts b/src/cli/commands/channels.ts index 5b23dc3..bc06f2d 100644 --- a/src/cli/commands/channels.ts +++ b/src/cli/commands/channels.ts @@ -6,9 +6,16 @@ import { listChannelNotificationSubscriptions, markChannelNotificationsRead, sub import { closeDb } from "../../lib/db.js"; import { resolveIdentity } from "../../lib/identity.js"; import { previewText, windowItems } from "../../lib/compact-output.js"; +import { assertNoSensitiveContent } from "../../lib/content-safety.js"; import { getCliWindow, pageFromQuery, printCompactFooter, queryLimitFor } from "../compact.js"; import { printMessageEntry } from "../message-output.js"; +function failCommand(error: unknown, fallback: string): never { + console.error(chalk.red(error instanceof Error ? error.message : fallback)); + closeDb(); + process.exit(1); +} + export function registerChannelCommands(program: Command): void { const channel = program .command("channel") @@ -257,20 +264,32 @@ export function registerChannelCommands(program: Command): void { process.exit(1); } + try { + assertNoSensitiveContent(channelArg, "Message channel"); + } catch (error) { + return failCommand(error, "Failed to send channel message."); + } + const sp = getChannel(channelArg); if (!sp) { - console.error(chalk.red(`Channel #${channelArg} not found.`)); + console.error(chalk.red("Channel not found.")); process.exit(1); } - const msg = sendMessage({ - from, - to: channelArg, - content, - channel: channelArg, - session_id: `channel:${channelArg}`, - priority: opts.priority, - }); + const msg = (() => { + try { + return sendMessage({ + from, + to: channelArg, + content, + channel: channelArg, + session_id: `channel:${channelArg}`, + priority: opts.priority, + }); + } catch (error) { + return failCommand(error, "Failed to send channel message."); + } + })(); if (opts.json) { console.log(JSON.stringify(msg, null, 2)); diff --git a/src/cli/commands/messaging.ts b/src/cli/commands/messaging.ts index ebf0b5d..473df89 100644 --- a/src/cli/commands/messaging.ts +++ b/src/cli/commands/messaging.ts @@ -28,6 +28,12 @@ export function formatDigestContinuationCommand(result: Pick { + try { + return sendMessage({ + from, + to: to || from, + channel: channel || undefined, + content, + session_id: session, + priority: opts.priority, + working_dir: opts.workingDir, + repository: opts.repository, + branch: opts.branch, + metadata, + blocking: opts.blocking, + }); + } catch (error) { + return failCommand(error, "Failed to send message."); + } + })(); if (opts.json) { console.log(JSON.stringify(msg, null, 2)); @@ -401,14 +413,20 @@ export function registerMessagingCommands(program: Command): void { const to = channel ? channel : (original.from_agent === from ? original.to_agent : original.from_agent); - const msg = sendMessage({ - from, - to, - content, - session_id: original.session_id, - priority: opts.priority, - channel, - }); + const msg = (() => { + try { + return sendMessage({ + from, + to, + content, + session_id: original.session_id, + priority: opts.priority, + channel, + }); + } catch (error) { + return failCommand(error, "Failed to send reply."); + } + })(); if (opts.json) { console.log(JSON.stringify(msg, null, 2)); @@ -497,7 +515,13 @@ export function registerMessagingCommands(program: Command): void { process.exit(1); } - const msg = editMessage(id, agent, content); + const msg = (() => { + try { + return editMessage(id, agent, content); + } catch (error) { + return failCommand(error, "Failed to edit message."); + } + })(); if (opts.json) { console.log(JSON.stringify(msg, null, 2)); diff --git a/src/cli/compact-output.e2e.test.ts b/src/cli/compact-output.e2e.test.ts index 9128427..35af8ad 100644 --- a/src/cli/compact-output.e2e.test.ts +++ b/src/cli/compact-output.e2e.test.ts @@ -26,6 +26,10 @@ function runCli(args: string[], agent: string) { }; } +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "cli_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + describe("compact CLI output", () => { afterAll(() => { try { unlinkSync(TEST_DB); } catch {} @@ -67,4 +71,53 @@ describe("compact CLI output", () => { expect(messages).toHaveLength(1); expect(messages[0].content).toBe("second page message"); }); + + test("send exits nonzero for sensitive content without echoing the value", () => { + const blocked = syntheticDatabaseUrl(); + const send = runCli(["send", `blocked ${blocked}`, "--to", "blocked-target"], "alice"); + + expect(send.exitCode).not.toBe(0); + expect(send.stderr).toContain("sensitive content detected"); + expect(send.stderr).not.toContain(blocked); + + const read = runCli(["read", "--to", "blocked-target", "--json"], "blocked-target"); + expect(read.exitCode).toBe(0); + expect(JSON.parse(read.stdout)).toHaveLength(0); + }); + + test("send exits nonzero for sensitive metadata without echoing the value", () => { + const blocked = syntheticDatabaseUrl(); + const send = runCli([ + "send", + "metadata should be checked", + "--to", + "metadata-blocked", + "--metadata", + JSON.stringify({ dsn: blocked }), + ], "alice"); + + expect(send.exitCode).not.toBe(0); + expect(send.stderr).toContain("sensitive content detected"); + expect(send.stderr).not.toContain(blocked); + + const read = runCli(["read", "--to", "metadata-blocked", "--json"], "metadata-blocked"); + expect(read.exitCode).toBe(0); + expect(JSON.parse(read.stdout)).toHaveLength(0); + }); + + test("channel send exits nonzero for sensitive channel input without echoing the value", () => { + const blocked = syntheticDatabaseUrl(); + const send = runCli(["channel", "send", blocked, "channel should be checked"], "alice"); + + expect(send.exitCode).not.toBe(0); + expect(send.stderr).toContain("sensitive content detected"); + expect(send.stderr).not.toContain(blocked); + }); + + test("send exits nonzero for truncated metadata JSON", () => { + const send = runCli(["send", "metadata parse check", "--to", "metadata-target", "--metadata", "{\"broken\":"], "alice"); + + expect(send.exitCode).not.toBe(0); + expect(send.stderr).toContain("Invalid --metadata JSON."); + }); }); diff --git a/src/cli/components/ChatView.test.tsx b/src/cli/components/ChatView.test.tsx new file mode 100644 index 0000000..00780d6 --- /dev/null +++ b/src/cli/components/ChatView.test.tsx @@ -0,0 +1,55 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { unlinkSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { readMessages } from "../../lib/messages.js"; +import { closeDb } from "../../lib/db.js"; +import { submitChatViewMessage } from "./ChatView.js"; + +const TEST_DB = join(tmpdir(), `conversations-chat-view-${Date.now()}.db`); + +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "tui_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + +beforeEach(() => { + process.env.CONVERSATIONS_DB_PATH = TEST_DB; + closeDb(); +}); + +afterEach(() => { + closeDb(); + try { unlinkSync(TEST_DB); } catch {} + try { unlinkSync(`${TEST_DB}-wal`); } catch {} + try { unlinkSync(`${TEST_DB}-shm`); } catch {} +}); + +describe("submitChatViewMessage", () => { + test("blocks sensitive content without throwing, echoing, or persisting", () => { + const blocked = syntheticDatabaseUrl(); + const result = submitChatViewMessage( + { agent: "tui-sender", recipient: "tui-recipient" }, + `blocked ${blocked}` + ); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("blocked"); + expect(result.error).not.toContain(blocked); + } + expect(readMessages({ to: "tui-recipient" })).toHaveLength(0); + }); + + test("sends safe content", () => { + const result = submitChatViewMessage( + { agent: "tui-sender", recipient: "tui-recipient" }, + "safe chat message" + ); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.message.content).toBe("safe chat message"); + } + expect(readMessages({ to: "tui-recipient" })).toHaveLength(1); + }); +}); diff --git a/src/cli/components/ChatView.tsx b/src/cli/components/ChatView.tsx index 4bc2a06..4e0866b 100644 --- a/src/cli/components/ChatView.tsx +++ b/src/cli/components/ChatView.tsx @@ -2,6 +2,7 @@ import React, { useState, useEffect, useRef } from "react"; import { Box, Text, useInput } from "ink"; import TextInput from "ink-text-input"; import { readMessages, sendMessage, markSessionRead, markChannelRead } from "../../lib/messages.js"; +import { SensitiveContentError } from "../../lib/content-safety.js"; import { startPolling } from "../../lib/poll.js"; import { MessageBubble } from "./MessageBubble.js"; import type { Message } from "../../types.js"; @@ -16,9 +17,63 @@ interface ChatViewProps { channelName?: string; } +interface ChatViewSubmitOptions { + agent: string; + sessionId?: string; + recipient?: string; + channelName?: string; +} + +export type ChatViewSubmitResult = + | { ok: true; message: Message } + | { ok: false; error: string }; + +function chatViewSendError(error: unknown): string { + if (error instanceof SensitiveContentError) { + return "Message blocked by sensitive-content controls."; + } + return "Unable to send message."; +} + +export function submitChatViewMessage( + { agent, sessionId, recipient, channelName }: ChatViewSubmitOptions, + value: string +): ChatViewSubmitResult { + const content = value.trim(); + if (!content) return { ok: false, error: "" }; + + try { + if (channelName) { + return { + ok: true, + message: sendMessage({ + from: agent, + to: channelName, + content, + channel: channelName, + session_id: `channel:${channelName}`, + }), + }; + } + + return { + ok: true, + message: sendMessage({ + from: agent, + to: recipient || agent, + content, + session_id: sessionId, + }), + }; + } catch (error) { + return { ok: false, error: chatViewSendError(error) }; + } +} + export function ChatView({ agent, onBack, sessionId: initialSessionId, recipient, channelName }: ChatViewProps) { const [messages, setMessages] = useState([]); const [input, setInput] = useState(""); + const [sendError, setSendError] = useState(null); const [sessionId, setSessionId] = useState(initialSessionId); const isChannel = !!channelName; const seenIds = useRef>(new Set()); @@ -84,30 +139,20 @@ export function ChatView({ agent, onBack, sessionId: initialSessionId, recipient const handleSubmit = (value: string) => { if (!value.trim()) return; - if (isChannel && channelName) { - const msg = sendMessage({ - from: agent, - to: channelName, - content: value.trim(), - channel: channelName, - session_id: `channel:${channelName}`, - }); - seenIds.current.add(msg.id); - setMessages((prev) => [...prev, msg]); - } else { - const to = recipient || agent; - const msg = sendMessage({ - from: agent, - to, - content: value.trim(), - session_id: sessionId, - }); - seenIds.current.add(msg.id); - setMessages((prev) => [...prev, msg]); - // For new conversations, capture the real session ID from the first message - if (!sessionId) { - setSessionId(msg.session_id); - } + const result = submitChatViewMessage({ agent, sessionId, recipient, channelName }, value); + if (!result.ok) { + setSendError(result.error || "Unable to send message."); + setInput(""); + return; + } + + const msg = result.message; + seenIds.current.add(msg.id); + setMessages((prev) => [...prev, msg]); + setSendError(null); + // For new conversations, capture the real session ID from the first message + if (!isChannel && !sessionId) { + setSessionId(msg.session_id); } setInput(""); @@ -142,6 +187,12 @@ export function ChatView({ agent, onBack, sessionId: initialSessionId, recipient )} + {sendError ? ( + + {sendError} + + ) : null} + {prompt}: { process.env.CONVERSATIONS_DB_PATH = TEST_DB; closeDb(); @@ -98,6 +110,20 @@ describe("channel notifications", () => { expect(notifications[0].unread).toBe(true); }); + test("redacts legacy sensitive content from notification previews", () => { + const blocked = syntheticDatabaseUrl(); + createChannel("ops", "creator"); + subscribeToChannelNotifications("ops", "agent-a", { preview_chars: 120 }); + + const id = insertLegacyChannelMessage("ops", `legacy DSN ${blocked}`); + const notifications = readChannelNotifications({ agent: "agent-a" }); + + expect(notifications).toHaveLength(1); + expect(notifications[0].message_id).toBe(id); + expect(notifications[0].preview).toContain("[REDACTED:DATABASE URL]"); + expect(notifications[0].preview).not.toContain(blocked); + }); + test("marks notifications read by ids and all", () => { createChannel("ops", "creator"); subscribeToChannelNotifications("ops", "agent-a"); diff --git a/src/lib/channel-notifications.ts b/src/lib/channel-notifications.ts index f645c52..93d4e27 100644 --- a/src/lib/channel-notifications.ts +++ b/src/lib/channel-notifications.ts @@ -1,11 +1,12 @@ import { getDb } from "./db.js"; import type { ChannelNotification, ChannelNotificationSubscription } from "../types.js"; import { normalizeChannelName } from "./channel-names.js"; +import { redactSensitiveText } from "./content-safety.js"; const DEFAULT_PREVIEW_CHARS = 140; export function buildMessagePreview(content: string, maxChars = DEFAULT_PREVIEW_CHARS): string { - const normalized = content + const normalized = redactSensitiveText(content) .replace(/[*#`~_>\-]/g, " ") .replace(/\s+/g, " ") .trim(); diff --git a/src/lib/content-safety.ts b/src/lib/content-safety.ts new file mode 100644 index 0000000..88072ec --- /dev/null +++ b/src/lib/content-safety.ts @@ -0,0 +1,340 @@ +export type SensitiveContentKind = + | "private_key" + | "cloud_key" + | "bearer_token" + | "personal_access_token" + | "database_url" + | "multiline_env_dump"; + +export interface SensitiveContentFinding { + kind: SensitiveContentKind; + label: string; + line: number; +} + +interface PatternRule { + kind: SensitiveContentKind; + label: string; + pattern: RegExp; + redaction: string; +} + +const KIND_REDACTIONS: Record = { + private_key: "[REDACTED:PRIVATE_KEY]", + cloud_key: "[REDACTED:CLOUD_KEY]", + bearer_token: "[REDACTED:BEARER_TOKEN]", + personal_access_token: "[REDACTED:PAT]", + database_url: "[REDACTED:DATABASE_URL]", + multiline_env_dump: "[REDACTED:ENV_DUMP]", +}; + +const AUTH_CHARS = "[A-Za-z0-9._~+/@=-]"; +const ASSIGNED_VALUE_PATTERN = String.raw`\s*[:=]\s*["']?[A-Za-z0-9._~+/@=-]{16,}["']?`; + +function envKey(...parts: string[]): string { + return parts.join("_"); +} + +const CLOUD_ENV_NAMES = [ + envKey("AWS", "SECRET", "ACCESS", "KEY"), + envKey("AWS", "ACCESS", "KEY", "ID"), + envKey("GOOGLE", "API", "KEY"), + envKey("OPENAI", "API", "KEY"), + envKey("ANTHROPIC", "API", "KEY"), + envKey("CLOUDFLARE", "API", "TOKEN"), + envKey("STRIPE", "SECRET", "KEY"), +]; + +const VCS_ENV_NAMES = [ + envKey("PERSONAL", "ACCESS", "TOKEN"), + envKey("GITHUB", "TOKEN"), + envKey("GITLAB", "TOKEN"), + envKey("GH", "TOKEN"), + "PAT", +]; + +const DATABASE_ENV_NAMES = [ + envKey("DATABASE", "URL"), + envKey("DATABASE", "URI"), + envKey("DB", "URL"), + envKey("POSTGRES", "URL"), + envKey("POSTGRESQL", "URL"), + envKey("MYSQL", "URL"), + envKey("MONGODB", "URI"), + envKey("REDIS", "URL"), +]; + +const SENSITIVE_PATTERNS: PatternRule[] = [ + { + kind: "private_key", + label: "private key", + pattern: /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?(?:-----END [A-Z0-9 ]*PRIVATE KEY-----|$)/gi, + redaction: KIND_REDACTIONS.private_key, + }, + { + kind: "private_key", + label: "private key", + pattern: /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----/gi, + redaction: KIND_REDACTIONS.private_key, + }, + { + kind: "cloud_key", + label: "cloud key", + pattern: /\b(?:AKIA|ASIA|AGPA|AIDA|AROA)[A-Z0-9]{16}\b/g, + redaction: KIND_REDACTIONS.cloud_key, + }, + { + kind: "cloud_key", + label: "cloud key", + pattern: /\bAIza[0-9A-Za-z_-]{35}\b/g, + redaction: KIND_REDACTIONS.cloud_key, + }, + { + kind: "cloud_key", + label: "cloud key", + pattern: /\b(?:sk-(?:proj-)?[A-Za-z0-9_-]{24,}|xox[baprs]-[A-Za-z0-9-]{20,}|sk_(?:live|test)_[A-Za-z0-9]{16,})\b/g, + redaction: KIND_REDACTIONS.cloud_key, + }, + { + kind: "cloud_key", + label: "cloud key", + pattern: new RegExp(String.raw`\b(?:${CLOUD_ENV_NAMES.join("|")})${ASSIGNED_VALUE_PATTERN}`, "gi"), + redaction: KIND_REDACTIONS.cloud_key, + }, + { + kind: "bearer_token", + label: "bearer token", + pattern: new RegExp(`\\bBearer\\s+${AUTH_CHARS}{20,}`, "gi"), + redaction: KIND_REDACTIONS.bearer_token, + }, + { + kind: "personal_access_token", + label: "personal access token", + pattern: /\b(?:ghp|gho|ghu|ghs|ghr)_[A-Za-z0-9_]{20,}\b|github_pat_[A-Za-z0-9_]{20,}\b|glpat-[A-Za-z0-9_-]{20,}\b/g, + redaction: KIND_REDACTIONS.personal_access_token, + }, + { + kind: "personal_access_token", + label: "personal access token", + pattern: new RegExp(String.raw`\b(?:${VCS_ENV_NAMES.join("|")})${ASSIGNED_VALUE_PATTERN}`, "gi"), + redaction: KIND_REDACTIONS.personal_access_token, + }, + { + kind: "database_url", + label: "database URL", + pattern: /\b(?:postgres(?:ql)?|mysql|mariadb|mongodb(?:\+srv)?|redis|rediss|mssql):\/\/[^\s"'<>`]+/gi, + redaction: KIND_REDACTIONS.database_url, + }, +]; + +const ENV_ASSIGNMENT = /^\s*(?:export\s+)?([A-Z][A-Z0-9_]{2,})\s*=\s*(.+?)\s*$/; +const MIN_ENV_DUMP_LINES = 3; + +function lineNumberAt(text: string, index: number): number { + let line = 1; + for (let i = 0; i < index && i < text.length; i++) { + if (text.charCodeAt(i) === 10) line++; + } + return line; +} + +function dedupeFindings(findings: SensitiveContentFinding[]): SensitiveContentFinding[] { + const seen = new Set(); + const deduped: SensitiveContentFinding[] = []; + for (const finding of findings) { + const key = `${finding.kind}:${finding.line}`; + if (seen.has(key)) continue; + seen.add(key); + deduped.push(finding); + } + return deduped; +} + +function envDumpRanges(text: string): Array<{ start: number; end: number; line: number }> { + const ranges: Array<{ start: number; end: number; line: number }> = []; + const lines = text.matchAll(/[^\n]*(?:\n|$)/g); + let current: { start: number; end: number; count: number; line: number } | null = null; + let line = 1; + + for (const match of lines) { + const rawLine = match[0]; + if (rawLine === "") break; + const start = match.index ?? 0; + const end = start + rawLine.length; + const trimmed = rawLine.trim(); + const isEnvLine = trimmed.length > 0 && !trimmed.startsWith("#") && ENV_ASSIGNMENT.test(rawLine); + + if (isEnvLine) { + if (!current) current = { start, end, count: 1, line }; + else { + current.end = end; + current.count++; + } + } else { + if (current && current.count >= MIN_ENV_DUMP_LINES) { + ranges.push({ start: current.start, end: current.end, line: current.line }); + } + current = null; + } + line++; + } + + if (current && current.count >= MIN_ENV_DUMP_LINES) { + ranges.push({ start: current.start, end: current.end, line: current.line }); + } + + return ranges; +} + +export class SensitiveContentError extends Error { + readonly findings: SensitiveContentFinding[]; + + constructor(context: string, findings: SensitiveContentFinding[]) { + const labels = [...new Set(findings.map((finding) => finding.label))].join(", "); + super(`${context} blocked: sensitive content detected (${labels}). Remove secrets before sending.`); + this.name = "SensitiveContentError"; + this.findings = findings; + } +} + +export function scanSensitiveContent(text: string): SensitiveContentFinding[] { + if (!text) return []; + const findings: SensitiveContentFinding[] = []; + + for (const rule of SENSITIVE_PATTERNS) { + for (const match of text.matchAll(rule.pattern)) { + findings.push({ + kind: rule.kind, + label: rule.label, + line: lineNumberAt(text, match.index ?? 0), + }); + } + } + + for (const range of envDumpRanges(text)) { + findings.push({ + kind: "multiline_env_dump", + label: "multiline env dump", + line: range.line, + }); + } + + return dedupeFindings(findings).sort((a, b) => a.line - b.line || a.kind.localeCompare(b.kind)); +} + +export function assertNoSensitiveContent(text: string, context = "Message content"): void { + const findings = scanSensitiveContent(text); + if (findings.length > 0) { + throw new SensitiveContentError(context, findings); + } +} + +function scalarKeyValueText(key: string, value: unknown): string | null { + if (typeof value === "string" || typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { + return `${key}=${String(value)}`; + } + return null; +} + +function normalizeMetadataKey(key: string): string { + return key.trim().toUpperCase().replace(/[^A-Z0-9]+/g, "_").replace(/^_+|_+$/g, ""); +} + +function metadataKeyFindings(key: string): SensitiveContentFinding[] { + const normalized = normalizeMetadataKey(key); + const findings: SensitiveContentFinding[] = []; + + if (CLOUD_ENV_NAMES.includes(normalized)) { + findings.push({ kind: "cloud_key", label: "cloud key", line: 1 }); + } + if (VCS_ENV_NAMES.includes(normalized)) { + findings.push({ kind: "personal_access_token", label: "personal access token", line: 1 }); + } + if (DATABASE_ENV_NAMES.includes(normalized)) { + findings.push({ kind: "database_url", label: "database URL", line: 1 }); + } + if (normalized.includes("PRIVATE_KEY")) { + findings.push({ kind: "private_key", label: "private key", line: 1 }); + } + if (normalized === "AUTHORIZATION" || normalized === "BEARER_TOKEN") { + findings.push({ kind: "bearer_token", label: "bearer token", line: 1 }); + } + + return dedupeFindings(findings); +} + +function scanSensitiveValue(value: unknown, seen = new WeakSet()): SensitiveContentFinding[] { + const findings: SensitiveContentFinding[] = []; + + if (typeof value === "string") { + findings.push(...scanSensitiveContent(value)); + return findings; + } + + if (!value || typeof value !== "object") return findings; + if (seen.has(value)) return findings; + seen.add(value); + + if (Array.isArray(value)) { + for (const item of value) { + findings.push(...scanSensitiveValue(item, seen)); + } + return dedupeFindings(findings); + } + + for (const [key, nested] of Object.entries(value as Record)) { + const keyFindings = [...scanSensitiveContent(key), ...metadataKeyFindings(key)]; + findings.push(...keyFindings); + const keyValueText = scalarKeyValueText(key, nested); + if (keyValueText) findings.push(...scanSensitiveContent(keyValueText)); + findings.push(...scanSensitiveValue(nested, seen)); + } + + return dedupeFindings(findings); +} + +export function assertNoSensitiveValue(value: unknown, context = "Message metadata"): void { + const findings = scanSensitiveValue(value); + if (findings.length > 0) { + throw new SensitiveContentError(context, findings); + } +} + +function redactionForFindings(findings: SensitiveContentFinding[]): string { + return [...new Set(findings.map((finding) => KIND_REDACTIONS[finding.kind]))].join(" "); +} + +export function redactSensitiveText(text: string): string { + if (!text) return text; + let redacted = text; + + for (const range of envDumpRanges(redacted).sort((a, b) => b.start - a.start)) { + redacted = `${redacted.slice(0, range.start)}[REDACTED:ENV_DUMP]\n${redacted.slice(range.end)}`; + } + + for (const rule of SENSITIVE_PATTERNS) { + redacted = redacted.replace(rule.pattern, rule.redaction); + } + + return redacted; +} + +export function redactSensitiveValue(value: T): T { + if (typeof value === "string") return redactSensitiveText(value) as T; + if (Array.isArray(value)) return value.map((item) => redactSensitiveValue(item)) as T; + if (value && typeof value === "object") { + const result: Record = {}; + for (const [key, nested] of Object.entries(value)) { + const redactedKey = redactSensitiveText(key); + const keyFindings = metadataKeyFindings(key); + const keyValueText = scalarKeyValueText(key, nested); + const contextualFindings = keyValueText ? scanSensitiveContent(keyValueText) : []; + const findings = [...keyFindings, ...contextualFindings]; + result[redactedKey] = findings.length > 0 + ? redactionForFindings(findings) + : redactSensitiveValue(nested); + } + return result as T; + } + return value; +} diff --git a/src/lib/messages.test.ts b/src/lib/messages.test.ts index a16f977..294cb3b 100644 --- a/src/lib/messages.test.ts +++ b/src/lib/messages.test.ts @@ -3,12 +3,67 @@ import { sendMessage, readMessages, readDigest, markRead, markReadByIds, markSes import { createChannel, joinChannel } from "./channels"; import { readChannelNotifications, subscribeToChannelNotifications } from "./channel-notifications"; import { closeDb, getDb } from "./db"; +import { redactSensitiveText } from "./content-safety"; import { unlinkSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; const TEST_DB = join(tmpdir(), `conversations-test-msg-${Date.now()}.db`); +function syntheticPrivateKey(): string { + return [ + "-----BEGIN " + "PRIVATE KEY-----", + "not-a-real-key-material", + "-----END " + "PRIVATE KEY-----", + ].join("\n"); +} + +function syntheticUnterminatedPrivateKey(): string { + return [ + "-----BEGIN " + "PRIVATE KEY-----", + "not-real-unterminated-key-material", + ].join("\n"); +} + +function syntheticCloudKey(): string { + return "AKIA" + "A".repeat(16); +} + +function syntheticBearerToken(): string { + return "Bearer " + "b".repeat(32); +} + +function syntheticPat(): string { + return "gh" + "p_" + "c".repeat(36); +} + +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "app_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + +function syntheticCloudSecretValue(): string { + return "s".repeat(32); +} + +function cloudSecretKeyName(): string { + return ["AWS", "SECRET", "ACCESS", "KEY"].join("_"); +} + +function syntheticEnvDump(): string { + return [ + "APP_MODE=development", + "SERVICE_HOST=localhost", + "FEATURE_FLAG=enabled", + ].join("\n"); +} + +function insertLegacyMessage(content: string, metadata?: Record): void { + getDb().prepare(` + INSERT INTO messages (session_id, from_agent, to_agent, content, metadata) + VALUES (?, ?, ?, ?, ?) + `).run("legacy-session", "legacy-from", "legacy-to", content, metadata ? JSON.stringify(metadata) : null); +} + beforeEach(() => { process.env.CONVERSATIONS_DB_PATH = TEST_DB; closeDb(); @@ -94,6 +149,77 @@ describe("sendMessage", () => { expect(msg.branch).toBe("main"); }); + for (const [label, fixture] of [ + ["private keys", syntheticPrivateKey], + ["cloud keys", syntheticCloudKey], + ["bearer tokens", syntheticBearerToken], + ["personal access tokens", syntheticPat], + ["database URLs", syntheticDatabaseUrl], + ["multiline env dumps", syntheticEnvDump], + ] as const) { + test(`blocks ${label} before persistence`, () => { + expect(() => sendMessage({ from: "alice", to: "bob", content: `please do not send\n${fixture()}` })) + .toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + } + + test("blocks sensitive metadata before persistence", () => { + const blocked = syntheticDatabaseUrl(); + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "metadata should be checked too", + metadata: { nested: { dsn: blocked } }, + })).toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + + test("blocks sensitive serialized metadata before persistence", () => { + const blocked = syntheticDatabaseUrl(); + const metadata = { + toJSON: () => ({ dsn: blocked }), + } as unknown as Record; + + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "serialized metadata should be checked too", + metadata, + })).toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + + test("blocks label-based metadata secrets before persistence", () => { + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "metadata labels should be checked too", + metadata: { [cloudSecretKeyName()]: syntheticCloudSecretValue() }, + })).toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + + test("blocks nested label-based metadata secrets before persistence", () => { + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "nested metadata labels should be checked too", + metadata: { [cloudSecretKeyName()]: { value: syntheticCloudSecretValue() } }, + })).toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + + test("blocks sensitive persisted context fields before persistence", () => { + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "context fields should be checked too", + branch: syntheticPat(), + })).toThrow(/sensitive content detected/); + expect(readMessages()).toHaveLength(0); + }); + test("null metadata when not provided", () => { const msg = sendMessage({ from: "a", to: "b", content: "hi" }); expect(msg.metadata).toBeNull(); @@ -184,6 +310,25 @@ describe("readMessages", () => { const msgs = readMessages({ since }); expect(msgs.length).toBeGreaterThanOrEqual(0); // Timing-dependent }); + + test("redacts legacy sensitive content from read, show, search, and digest paths", () => { + const blocked = syntheticDatabaseUrl(); + insertLegacyMessage(`legacy ${blocked}`, { nested: { dsn: blocked } }); + + const read = readMessages({ to: "legacy-to" }); + expect(read).toHaveLength(1); + expect(JSON.stringify(read)).not.toContain(blocked); + expect(read[0].content).toContain("[REDACTED:DATABASE_URL]"); + + const shown = getMessageById(read[0].id); + expect(JSON.stringify(shown)).not.toContain(blocked); + + const searched = searchMessages({ query: "legacy" }); + expect(JSON.stringify(searched)).not.toContain(blocked); + + const digest = readDigest({ to: "legacy-to" }); + expect(JSON.stringify(digest)).not.toContain(blocked); + }); }); describe("markRead", () => { @@ -309,6 +454,36 @@ describe("exportMessages", () => { expect(parsed[1].content).toBe("world"); }); + test("redacts sensitive content in JSON exports", () => { + const legacyContent = `legacy DSN ${syntheticDatabaseUrl()}`; + const legacyMetadata = { authorization: syntheticBearerToken(), [cloudSecretKeyName()]: { value: syntheticCloudSecretValue() } }; + insertLegacyMessage(legacyContent, legacyMetadata); + + const result = exportMessages(); + const parsed = JSON.parse(result); + const serialized = JSON.stringify(parsed); + + expect(parsed[0].content).toContain("[REDACTED:DATABASE_URL]"); + expect(serialized).toContain("[REDACTED:BEARER_TOKEN]"); + expect(serialized).toContain("[REDACTED:CLOUD_KEY]"); + expect(serialized).not.toContain(syntheticDatabaseUrl()); + expect(serialized).not.toContain(syntheticBearerToken()); + expect(serialized).not.toContain(syntheticCloudSecretValue()); + }); + + test("redacts unterminated private key blocks through EOF", () => { + const blocked = syntheticUnterminatedPrivateKey(); + const material = "not-real-unterminated-key-material"; + insertLegacyMessage(`legacy ${blocked}`); + + expect(redactSensitiveText(blocked)).toContain("[REDACTED:PRIVATE_KEY]"); + expect(redactSensitiveText(blocked)).not.toContain(material); + + const exported = exportMessages(); + expect(exported).toContain("[REDACTED:PRIVATE_KEY]"); + expect(exported).not.toContain(material); + }); + test("returns CSV with headers", () => { sendMessage({ from: "alice", to: "bob", content: "hello" }); const result = exportMessages({ format: "csv" }); @@ -320,6 +495,15 @@ describe("exportMessages", () => { expect(lines[1]).toContain("hello"); }); + test("redacts sensitive content in CSV exports", () => { + insertLegacyMessage(`legacy token ${syntheticPat()}`); + + const result = exportMessages({ format: "csv" }); + + expect(result).toContain("[REDACTED:PAT]"); + expect(result).not.toContain(syntheticPat()); + }); + test("filters by channel", () => { sendMessage({ from: "a", to: "general", content: "in-channel", channel: "general" }); sendMessage({ from: "a", to: "b", content: "no-channel" }); @@ -391,6 +575,14 @@ describe("deleteMessage", () => { }); describe("editMessage", () => { + test("blocks sensitive content before updating", () => { + const msg = sendMessage({ from: "alice", to: "bob", content: "original" }); + + expect(() => editMessage(msg.id, "alice", `new ${syntheticBearerToken()}`)) + .toThrow(/sensitive content detected/); + expect(getMessageById(msg.id)?.content).toBe("original"); + }); + test("edits own message and sets edited_at", () => { const msg = sendMessage({ from: "alice", to: "bob", content: "original" }); const edited = editMessage(msg.id, "alice", "updated"); @@ -774,11 +966,40 @@ describe("attachments", () => { expect(msg.attachments![1].mime_type).toBe("image/png"); }); + test("sends large safe attachment with chunked scanning", () => { + const srcFile = join(TEMP_DIR, "large.txt"); + writeFileSync(srcFile, "safe attachment content\n".repeat(5000)); + + const msg = sendMessage({ + from: "alice", + to: "bob", + content: "large file", + attachments: [{ name: "large.txt", source_path: srcFile }], + }); + + expect(msg.attachments).toBeTruthy(); + expect(msg.attachments![0].size).toBeGreaterThan(64 * 1024); + }); + test("message without attachments has null attachments field", () => { const msg = sendMessage({ from: "alice", to: "bob", content: "no files" }); expect(msg.attachments).toBeNull(); }); + test("does not persist message when attachment content is sensitive", () => { + const srcFile = join(TEMP_DIR, "leaky.txt"); + writeFileSync(srcFile, `attachment ${syntheticDatabaseUrl()}`); + + expect(() => sendMessage({ + from: "alice", + to: "bob", + content: "bad attachment content", + attachments: [{ name: "leaky.txt", source_path: srcFile }], + })).toThrow(/sensitive content detected/); + + expect(readMessages()).toHaveLength(0); + }); + test("does not persist message when attachment source is missing", () => { expect(() => sendMessage({ from: "alice", diff --git a/src/lib/messages.ts b/src/lib/messages.ts index 2fbb7ec..091a313 100644 --- a/src/lib/messages.ts +++ b/src/lib/messages.ts @@ -1,11 +1,12 @@ import { getDb, getDataDir } from "./db.js"; import type { Message, Attachment, SendMessageOptions, ReadMessagesOptions, SearchMessagesOptions, SearchResult } from "../types.js"; import { createHash, randomUUID } from "crypto"; -import { mkdirSync, copyFileSync, statSync, existsSync, realpathSync } from "fs"; +import { mkdirSync, copyFileSync, closeSync, openSync, readSync, statSync, existsSync, realpathSync } from "fs"; import { join, basename, resolve } from "path"; import { fireWebhooks } from "./webhooks.js"; import { normalizeChannelName } from "./channel-names.js"; import { markChannelNotificationsRead } from "./channel-notifications.js"; +import { assertNoSensitiveContent, assertNoSensitiveValue, redactSensitiveText, redactSensitiveValue } from "./content-safety.js"; /** Strip null/undefined fields from a message for compact output. */ export function compactMessage(msg: Message): Partial { @@ -38,13 +39,24 @@ function parseMessage(row: Record): Message { } } - return { + return redactMessage({ ...row, metadata, attachments, blocking: !!(row.blocking as number), reply_to: (row.reply_to as number) || null, - } as Message; + } as Message); +} + +function redactMessage(message: T): T { + return redactSensitiveValue(message); +} + +function buildSearchSnippet(message: Message): string | null { + if (!message.content) return null; + const normalized = message.content.replace(/\s+/g, " ").trim(); + if (!normalized) return null; + return normalized.length > 320 ? `${normalized.slice(0, 317)}...` : normalized; } function getAttachmentsDir(): string { @@ -52,6 +64,27 @@ function getAttachmentsDir(): string { return join(getDataDir(), "attachments"); } +const ATTACHMENT_SCAN_CHUNK_BYTES = 64 * 1024; +const ATTACHMENT_SCAN_CARRY_CHARS = 8192; + +function assertAttachmentContentSafe(path: string): void { + const fd = openSync(path, "r"); + const buffer = Buffer.allocUnsafe(ATTACHMENT_SCAN_CHUNK_BYTES); + let carry = ""; + + try { + while (true) { + const bytesRead = readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead <= 0) break; + const text = carry + buffer.subarray(0, bytesRead).toString("utf8"); + assertNoSensitiveContent(text, "Message attachment content"); + carry = text.slice(-ATTACHMENT_SCAN_CARRY_CHARS); + } + } finally { + closeSync(fd); + } +} + /** Validate attachment source path and name to prevent arbitrary file read and path traversal. */ function validateAttachment(sourcePath: string, name: string): { safeSource: string; safeName: string } { // Resolve to absolute and verify the file exists and is a regular file @@ -69,6 +102,7 @@ function validateAttachment(sourcePath: string, name: string): { safeSource: str if (!safeName || safeName.startsWith(".")) { throw new Error(`Invalid attachment name: ${name}`); } + assertAttachmentContentSafe(real); return { safeSource: real, safeName }; } @@ -118,8 +152,37 @@ function checkRateLimit(agentId: string): void { } } +function assertNoSensitiveSendFields(opts: SendMessageOptions, serializedMetadata: string | null): void { + assertNoSensitiveContent(opts.content, "Message content"); + + const persistedStrings: Array<[string, string | undefined]> = [ + ["Message sender", opts.from], + ["Message recipient", opts.to], + ["Message session", opts.session_id], + ["Message channel", opts.channel], + ["Message project", opts.project_id], + ["Message working directory", opts.working_dir], + ["Message repository", opts.repository], + ["Message branch", opts.branch], + ]; + + for (const [context, value] of persistedStrings) { + if (value) assertNoSensitiveContent(value, context); + } + + if (opts.metadata) assertNoSensitiveValue(opts.metadata, "Message metadata"); + if (serializedMetadata) assertNoSensitiveContent(serializedMetadata, "Message metadata"); + + for (const attachment of opts.attachments ?? []) { + assertNoSensitiveContent(attachment.name, "Message attachment name"); + assertNoSensitiveContent(attachment.source_path, "Message attachment path"); + } +} + export function sendMessage(opts: SendMessageOptions): Message { assertMessageSize(opts.content); + const metadata = opts.metadata ? JSON.stringify(opts.metadata) ?? null : null; + assertNoSensitiveSendFields(opts, metadata); checkRateLimit(opts.from); @@ -134,7 +197,6 @@ export function sendMessage(opts: SendMessageOptions): Message { ? `channel:${channelName}` : explicitSession ?? `${[opts.from, opts.to].sort().join("-")}-${randomUUID().slice(0, 8)}`; const toAgent = channelName ?? opts.to; - const metadata = opts.metadata ? JSON.stringify(opts.metadata) : null; const normalizedPriority = (opts.priority === "low" || opts.priority === "normal" || opts.priority === "high" || opts.priority === "urgent") ? opts.priority : "normal"; @@ -875,7 +937,7 @@ export function exportMessages(opts?: ExportMessagesOptions): string { escapeCsvField(m.from_agent), escapeCsvField(m.to_agent), escapeCsvField(m.channel), - escapeCsvField(m.content), + escapeCsvField(redactSensitiveText(m.content)), escapeCsvField(m.priority), escapeCsvField(m.created_at), escapeCsvField(m.read_at), @@ -896,6 +958,7 @@ export function deleteMessage(id: number, agent: string): boolean { export function editMessage(id: number, agent: string, newContent: string): Message | null { assertMessageSize(newContent); + assertNoSensitiveContent(newContent, "Message content"); const db = getDb(); const stmt = db.prepare( @@ -1047,7 +1110,7 @@ export function searchMessages(opts: SearchMessagesOptions): SearchResult[] { const pinnedBoost = msg.pinned_at ? 20 : 0; const blockingBoost = msg.blocking ? 15 : 0; const relevance_score = Math.round((ftsScore * priorityBoost + pinnedBoost + blockingBoost) * 100) / 100; - return { ...msg, snippet: (row.snippet as string) || null, relevance_score }; + return { ...msg, snippet: buildSearchSnippet(msg), relevance_score }; }); } catch { // Fallback to LIKE if FTS not available @@ -1071,7 +1134,7 @@ export function searchMessages(opts: SearchMessagesOptions): SearchResult[] { return rows.map((row) => { const msg = parseMessage(row); - return { ...msg, snippet: null, relevance_score: 0 }; + return { ...msg, snippet: buildSearchSnippet(msg), relevance_score: 0 }; }); } diff --git a/src/lib/summary.test.ts b/src/lib/summary.test.ts index fe8d485..a9a019f 100644 --- a/src/lib/summary.test.ts +++ b/src/lib/summary.test.ts @@ -3,13 +3,34 @@ import { getConversationSummary } from "./summary"; import { sendMessage, pinMessage } from "./messages"; import { addReaction } from "./reactions"; import { createChannel } from "./channels"; -import { closeDb } from "./db"; +import { closeDb, getDb } from "./db"; import { unlinkSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; const TEST_DB = join(tmpdir(), `conversations-test-summary-${Date.now()}.db`); +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "summary_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + +function insertLegacyMessage(content: string, opts?: { session_id?: string; channel?: string; priority?: string; blocking?: boolean; pinned_at?: string }): number { + const result = getDb().prepare(` + INSERT INTO messages (session_id, from_agent, to_agent, channel, content, priority, blocking, pinned_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + `).run( + opts?.session_id ?? "legacy-summary", + "legacy-from", + opts?.channel ?? "legacy-to", + opts?.channel ?? null, + content, + opts?.priority ?? "normal", + opts?.blocking ? 1 : 0, + opts?.pinned_at ?? null, + ); + return Number(result.lastInsertRowid); +} + beforeEach(() => { process.env.CONVERSATIONS_DB_PATH = TEST_DB; closeDb(); @@ -84,4 +105,17 @@ describe("getConversationSummary", () => { expect(summary).toBeTruthy(); expect(summary!.message_count).toBe(2); }); + + test("redacts legacy sensitive content in key messages and blockers", () => { + const blocked = syntheticDatabaseUrl(); + insertLegacyMessage(`urgent ${blocked}`, { session_id: "legacy-summary", priority: "urgent" }); + insertLegacyMessage(`blocked ${blocked}`, { session_id: "legacy-summary", blocking: true }); + insertLegacyMessage(`pinned ${blocked}`, { session_id: "legacy-summary", pinned_at: new Date().toISOString() }); + + const summary = getConversationSummary("legacy-summary"); + const serialized = JSON.stringify(summary); + + expect(serialized).toContain("[REDACTED:DATABASE_URL]"); + expect(serialized).not.toContain(blocked); + }); }); diff --git a/src/lib/summary.ts b/src/lib/summary.ts index dcdaca2..73a43b3 100644 --- a/src/lib/summary.ts +++ b/src/lib/summary.ts @@ -1,6 +1,7 @@ import { getDb } from "./db.js"; import type { Message } from "../types.js"; import { extractTopics, type TopicWeight } from "./topics.js"; +import { redactSensitiveText } from "./content-safety.js"; export interface ConversationSummary { session_id: string; @@ -21,6 +22,10 @@ export interface SummaryOptions { limit?: number; } +function redactedSnippet(content: unknown, maxChars: number): string { + return redactSensitiveText(String(content ?? "")).slice(0, maxChars); +} + /** * Generate a structured summary of a conversation (session or channel). * Uses message metadata — no LLM needed. @@ -52,7 +57,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO const dateRange = { first: dates[0], last: dates[dates.length - 1] }; // Topics - const allContent = messages.map((m) => m.content as string).join("\n"); + const allContent = messages.map((m) => redactSensitiveText(m.content as string)).join("\n"); const topics = extractTopics(allContent, 10); // Key messages: high priority, pinned, most reactions, most replies @@ -65,7 +70,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO keyMessages.push({ id: m.id as number, from: m.from_agent as string, - content: (m.content as string).slice(0, 200), + content: redactedSnippet(m.content, 200), reason: `${priority} priority`, }); } @@ -73,7 +78,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO keyMessages.push({ id: m.id as number, from: m.from_agent as string, - content: (m.content as string).slice(0, 200), + content: redactedSnippet(m.content, 200), reason: "blocking message", }); } @@ -85,7 +90,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO keyMessages.push({ id: m.id as number, from: m.from_agent as string, - content: (m.content as string).slice(0, 200), + content: redactedSnippet(m.content, 200), reason: "pinned", }); } @@ -105,7 +110,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO keyMessages.push({ id: r.message_id, from: m.from_agent as string, - content: (m.content as string).slice(0, 200), + content: redactedSnippet(m.content, 200), reason: `${r.c} reaction(s)`, }); } @@ -126,7 +131,7 @@ export function getConversationSummary(sessionOrChannel: string, opts?: SummaryO .map((m) => ({ id: m.id as number, from: m.from_agent as string, - content: (m.content as string).slice(0, 200), + content: redactedSnippet(m.content, 200), created_at: m.created_at as string, })); diff --git a/src/mcp/channel.test.ts b/src/mcp/channel.test.ts index d2d468e..d8cfe3a 100644 --- a/src/mcp/channel.test.ts +++ b/src/mcp/channel.test.ts @@ -3,7 +3,7 @@ import { unlinkSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import { closeDb } from "../lib/db.js"; +import { closeDb, getDb } from "../lib/db.js"; import { readMessages, sendMessage } from "../lib/messages.js"; import { createChannel } from "../lib/channels.js"; import { readChannelNotifications, subscribeToChannelNotifications } from "../lib/channel-notifications.js"; @@ -14,6 +14,18 @@ function createTestDbPath(): string { return join(tmpdir(), `conversations-channel-${Date.now()}-${Math.random().toString(16).slice(2)}.db`); } +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "bridge_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + +function insertLegacyChannelMessage(channel: string, content: string): number { + const result = getDb().prepare(` + INSERT INTO messages (session_id, from_agent, to_agent, channel, content) + VALUES (?, ?, ?, ?, ?) + `).run(`channel:${channel}`, "legacy-sender", channel, channel, content); + return Number(result.lastInsertRowid); +} + async function waitFor(check: () => boolean, timeoutMs = 1500): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { @@ -87,6 +99,39 @@ describe("channel bridge delivery", () => { } }); + test("channel blurbs redact legacy sensitive notification previews", async () => { + const blocked = syntheticDatabaseUrl(); + process.env.CONVERSATIONS_DB_PATH = createTestDbPath(); + closeDb(); + + createChannel("notify-bridge-redact", "creator"); + subscribeToChannelNotifications("notify-bridge-redact", "watcher", { preview_chars: 120 }); + setSessionAgent("watcher", "claude-session-channel-redact"); + + const delivered: Array<{ method: string; params: any }> = []; + const stop = registerChannelBridge({ + server: { + registerCapabilities() {}, + async notification(payload: { method: string; params: any }) { + delivered.push(payload); + }, + }, + } as any, { + pollIntervalMs: 20, + startDelayMs: 0, + }); + + try { + insertLegacyChannelMessage("notify-bridge-redact", `legacy ${blocked}`); + await waitFor(() => delivered.length === 1); + + expect(delivered[0].params.content).toContain("[REDACTED:DATABASE URL]"); + expect(delivered[0].params.content).not.toContain(blocked); + } finally { + stop(); + } + }); + test("retries direct session deliveries after a transient transport failure", async () => { process.env.CONVERSATIONS_DB_PATH = createTestDbPath(); closeDb(); diff --git a/src/mcp/index.test.ts b/src/mcp/index.test.ts index 88265fe..9763071 100644 --- a/src/mcp/index.test.ts +++ b/src/mcp/index.test.ts @@ -2,7 +2,7 @@ import { describe, test, expect, beforeAll, afterAll } from "bun:test"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { server } from "./index.js"; -import { closeDb } from "../lib/db.js"; +import { closeDb, getDb } from "../lib/db.js"; import { sendMessage, readMessages } from "../lib/messages.js"; import { createChannel } from "../lib/channels.js"; import { resolveIdentity } from "../lib/identity.js"; @@ -14,6 +14,18 @@ import { join } from "path"; const TEST_DB = join(tmpdir(), `conversations-test-mcp-${Date.now()}.db`); let client: Client; +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "mcp_user:synthetic-password", "@db.example.invalid/app"].join(""); +} + +function insertLegacyChannelMessage(channel: string, content: string, opts?: { priority?: string; blocking?: boolean }): number { + const result = getDb().prepare(` + INSERT INTO messages (session_id, from_agent, to_agent, channel, content, priority, blocking) + VALUES (?, ?, ?, ?, ?, ?, ?) + `).run(`channel:${channel}`, "legacy-from", channel, channel, content, opts?.priority ?? "normal", opts?.blocking ? 1 : 0); + return Number(result.lastInsertRowid); +} + beforeAll(async () => { process.env.CONVERSATIONS_DB_PATH = TEST_DB; delete process.env.CONVERSATIONS_AGENT_ID; @@ -89,6 +101,35 @@ describe("send_message from parameter", () => { expect(msg.from_agent).toBe("explicit-agent"); delete process.env.CONVERSATIONS_AGENT_ID; }); + + test("blocks sensitive content without echoing the value", async () => { + const blocked = syntheticDatabaseUrl(); + const result = await client.callTool({ + name: "send_message", + arguments: { from: "agent-alpha", to: "agent-beta", content: `blocked ${blocked}` }, + }) as any; + const text = (result.content[0] as { type: string; text: string }).text; + + expect(result.isError).toBe(true); + expect(text).toContain("sensitive content detected"); + expect(text).not.toContain(blocked); + expect(readMessages({ to: "agent-beta" }).some((message) => message.content.includes(blocked))).toBe(false); + }); + + test("blocks sensitive session-target metadata without echoing the value", async () => { + const blocked = syntheticDatabaseUrl(); + const result = await client.callTool({ + name: "send_to_session", + arguments: { from: "agent-alpha", target_session_id: blocked, content: "metadata route should be checked" }, + }) as any; + const text = (result.content[0] as { type: string; text: string }).text; + const persisted = JSON.stringify(readMessages()); + + expect(result.isError).toBe(true); + expect(text).toContain("sensitive content detected"); + expect(text).not.toContain(blocked); + expect(persisted).not.toContain(blocked); + }); }); // ---- reply ---- @@ -190,6 +231,33 @@ describe("send_to_channel from parameter", () => { const msg = parseResult(result as any) as any; expect(msg.from_agent).toBe(autoName); }); + + test("blocks sensitive channel input without echoing the value", async () => { + const blocked = syntheticDatabaseUrl(); + const result = await client.callTool({ + name: "send_to_channel", + arguments: { from: "channel-sender", channel: blocked, content: "channel should be checked" }, + }) as any; + const text = (result.content[0] as { type: string; text: string }).text; + + expect(result.isError).toBe(true); + expect(text).toContain("sensitive content detected"); + expect(text).not.toContain(blocked); + }); + + test("broadcast blocks sensitive channel input without echoing the value", async () => { + const blocked = syntheticDatabaseUrl(); + const result = await client.callTool({ + name: "broadcast", + arguments: { from: "channel-sender", channels: [blocked], content: "broadcast should be checked" }, + }) as any; + const text = (result.content[0] as { type: string; text: string }).text; + const data = JSON.parse(text); + + expect(text).not.toContain(blocked); + expect(data.sent).toHaveLength(0); + expect(data.errors[0]).toContain("sensitive content detected"); + }); }); describe("channel notification tools", () => { @@ -264,6 +332,25 @@ describe("channel notification tools", () => { expect(message.id).toBe(sent.id); expect(message.content).toBe(fullContent); }); + + test("read channel notifications redacts legacy sensitive preview content", async () => { + const blocked = syntheticDatabaseUrl(); + createChannel("notify-channel-redact", "creator"); + await client.callTool({ + name: "subscribe_channel_notifications", + arguments: { from: "notify-agent-redact", channel: "notify-channel-redact", preview_chars: 120 }, + }); + + insertLegacyChannelMessage("notify-channel-redact", `legacy ${blocked}`); + const result = parseResult(await client.callTool({ + name: "read_channel_notifications", + arguments: { from: "notify-agent-redact", unread_only: true }, + }) as any) as any; + const serialized = JSON.stringify(result); + + expect(serialized).toContain("[REDACTED:DATABASE URL]"); + expect(serialized).not.toContain(blocked); + }); }); // ---- join_channel ---- @@ -352,6 +439,24 @@ describe("channel notification subscription tools", () => { }); }); +describe("channel review tools", () => { + test("summarize_channel redacts legacy sensitive blocker and priority content", async () => { + const blocked = syntheticDatabaseUrl(); + createChannel("review-redact", "creator"); + insertLegacyChannelMessage("review-redact", `blocking ${blocked}`, { blocking: true }); + insertLegacyChannelMessage("review-redact", `urgent ${blocked}`, { priority: "urgent" }); + + const result = await client.callTool({ + name: "summarize_channel", + arguments: { channel: "review-redact", since: "1970-01-01T00:00:00.000Z", limit: 10 }, + }) as any; + const text = (result.content[0] as { type: string; text: string }).text; + + expect(text).toContain("[REDACTED:DATABASE_URL]"); + expect(text).not.toContain(blocked); + }); +}); + // ---- leave_channel ---- describe("leave_channel from parameter", () => { diff --git a/src/mcp/tools/channels.ts b/src/mcp/tools/channels.ts index 60a91b1..a7ce68b 100644 --- a/src/mcp/tools/channels.ts +++ b/src/mcp/tools/channels.ts @@ -14,8 +14,16 @@ import { listChannelNotificationSubscriptions, markAllChannelNotificationsRead, import { resolveIdentity } from "../../lib/identity.js"; import { recordReadReceiptsBatch } from "../../lib/messages.js"; import { getConversationSummary } from "../../lib/summary.js"; +import { assertNoSensitiveContent, redactSensitiveText } from "../../lib/content-safety.js"; import { compactQueriedMessages, compactWindowedChannels, jsonText, resolveMcpWindow } from "../compact.js"; +function toolError(error: unknown, fallback: string) { + return { + content: [{ type: "text" as const, text: error instanceof Error ? error.message : fallback }], + isError: true, + }; +} + export function registerChannelTools(server: McpServer): void { server.registerTool("create_channel", { @@ -93,23 +101,34 @@ export function registerChannelTools(server: McpServer): void { const { from: fromParam, channel, content, priority, blocking } = args; const from = resolveIdentity(fromParam); + try { + assertNoSensitiveContent(channel, "Message channel"); + } catch (error) { + return toolError(error, "Failed to send channel message."); + } + const sp = getChannel(channel); if (!sp) { return { - content: [{ type: "text", text: `channel "${channel}" not found` }], + content: [{ type: "text", text: "channel not found" }], isError: true, }; } - const msg = sendMessage({ - from, - to: channel, - content, - channel, - session_id: `channel:${channel}`, - priority, - blocking, - }); + let msg; + try { + msg = sendMessage({ + from, + to: channel, + content, + channel, + session_id: `channel:${channel}`, + priority, + blocking, + }); + } catch (error) { + return toolError(error, "Failed to send channel message."); + } return { content: [{ type: "text", text: JSON.stringify(msg) }], @@ -437,13 +456,14 @@ export function registerChannelTools(server: McpServer): void { for (const m of rows) { const from = m.from_agent as string; + const content = redactSensitiveText(m.content as string); agents.add(from); agentCounts[from] = (agentCounts[from] ?? 0) + 1; if (m.blocking) { - blockers.push({ id: m.id as number, from, content: (m.content as string).slice(0, 150), created_at: m.created_at as string }); + blockers.push({ id: m.id as number, from, content: content.slice(0, 150), created_at: m.created_at as string }); } // Count @mentions - const mentionedAgents = (m.content as string).match(/@([a-zA-Z0-9_-]+)/g) ?? []; + const mentionedAgents = content.match(/@([a-zA-Z0-9_-]+)/g) ?? []; for (const mention of mentionedAgents) { const a = mention.slice(1).toLowerCase(); mentions[a] = (mentions[a] ?? 0) + 1; @@ -472,7 +492,7 @@ export function registerChannelTools(server: McpServer): void { if (highPri.length > 0) { parts.push(`\n\ud83d\udd34 High priority (${highPri.length}):`); for (const m of highPri) { - parts.push(` \u2022 [${m.priority}] ${m.from_agent}: ${(m.content as string).slice(0, 100)}`); + parts.push(` \u2022 [${m.priority}] ${m.from_agent}: ${redactSensitiveText(m.content as string).slice(0, 100)}`); } } diff --git a/src/mcp/tools/messaging.ts b/src/mcp/tools/messaging.ts index 003892e..2edfff1 100644 --- a/src/mcp/tools/messaging.ts +++ b/src/mcp/tools/messaging.ts @@ -12,6 +12,13 @@ import { listSessions } from "../../lib/sessions.js"; import { resolveIdentity } from "../../lib/identity.js"; import { compactQueriedMessages, compactQueriedSearchMessages, compactWindowedSessions, jsonText, resolveMcpWindow } from "../compact.js"; +function toolError(error: unknown, fallback: string) { + return { + content: [{ type: "text" as const, text: error instanceof Error ? error.message : fallback }], + isError: true, + }; +} + export function registerMessagingTools( server: McpServer, resolveProjectId: (explicitProjectId: string | undefined, agentId: string) => string | undefined, @@ -36,15 +43,20 @@ export function registerMessagingTools( const to = toParam || to_agent; // Accept both "to" and "to_agent" const from = resolveIdentity(fromParam); - const msg = sendMessage({ - from, - to: target_session_id ? `session:${target_session_id}` : to, - content, - priority, - blocking, - project_id, - metadata: target_session_id ? { target_session_id } : undefined, - }); + let msg; + try { + msg = sendMessage({ + from, + to: target_session_id ? `session:${target_session_id}` : to, + content, + priority, + blocking, + project_id, + metadata: target_session_id ? { target_session_id } : undefined, + }); + } catch (error) { + return toolError(error, "Failed to send message."); + } return { content: [{ type: "text", text: JSON.stringify(msg) }], @@ -83,13 +95,18 @@ export function registerMessagingTools( } // Use session: as the to field and store the real target in metadata - const msg = sendMessage({ - from, - to: `session:${target_session_id}`, - content, - priority, - metadata: { target_session_id }, - }); + let msg; + try { + msg = sendMessage({ + from, + to: `session:${target_session_id}`, + content, + priority, + metadata: { target_session_id }, + }); + } catch (error) { + return toolError(error, "Failed to send session message."); + } return { content: [{ type: "text", text: JSON.stringify(msg) }], @@ -200,14 +217,19 @@ export function registerMessagingTools( const channel = original.channel || (original.session_id?.startsWith("channel:") ? original.session_id.slice(6) : undefined); - const msg = sendMessage({ - from, - to: channel ?? (original.from_agent === from ? original.to_agent : original.from_agent), - content, - session_id: original.session_id, - channel, - reply_to: message_id, // thread linkage - }); + let msg; + try { + msg = sendMessage({ + from, + to: channel ?? (original.from_agent === from ? original.to_agent : original.from_agent), + content, + session_id: original.session_id, + channel, + reply_to: message_id, // thread linkage + }); + } catch (error) { + return toolError(error, "Failed to send reply."); + } return { content: [{ type: "text", text: JSON.stringify(msg) }], @@ -411,7 +433,12 @@ export function registerMessagingTools( }, async (args: Record) => { const { from: fromParam, id, content } = args; const agent = resolveIdentity(fromParam); - const msg = editMessage(id, agent, content); + let msg; + try { + msg = editMessage(id, agent, content); + } catch (error) { + return toolError(error, "Failed to edit message."); + } if (!msg) { return { @@ -510,7 +537,7 @@ export function registerMessagingTools( const msg = sendMessage({ from, to: channel, content, channel, priority }); results.push({ channel, id: msg.id }); } catch (e) { - errors.push(`${channel}: ${e instanceof Error ? e.message : String(e)}`); + errors.push(e instanceof Error ? e.message : "Failed to send broadcast message."); } } diff --git a/src/server/api.test.ts b/src/server/api.test.ts index f00f6f1..23ec60e 100644 --- a/src/server/api.test.ts +++ b/src/server/api.test.ts @@ -27,6 +27,9 @@ function makeFakeClient() { if (/SELECT name FROM channels WHERE name/i.test(sql)) { return channels[(p as any[])[0]] ? { name: (p as any[])[0] } : null; } + if (/SELECT \* FROM messages WHERE id/i.test(sql)) { + return messages.find((row) => row.id === (p as any[])[0]) ?? null; + } if (/SELECT \* FROM channels WHERE name/i.test(sql) || /SELECT name, description/i.test(sql)) { return channels[(p as any[])[0]] ?? null; } @@ -44,9 +47,15 @@ function makeFakeClient() { } const SIGNING = "test-signing-secret-0123456789"; +let activeFakeClient: ReturnType | null = null; + +function syntheticDatabaseUrl(): string { + return ["postgres", "://", "api_user:synthetic-password", "@db.example.invalid/app"].join(""); +} function makeDeps(): ApiServerDeps { const client = makeFakeClient(); + activeFakeClient = client; const keys = new ApiKeyStore(client as any); const verifier = verifyApiKey({ app: "conversations", signingSecret: SIGNING, isRevoked: async () => false }); return { client: client as any, keys, verifier }; @@ -141,6 +150,58 @@ describe("conversations-serve", () => { expect(r.status).toBe(400); }); + test("POST /v1/messages blocks sensitive content without echoing it", async () => { + const blocked = syntheticDatabaseUrl(); + const r = await fetch(`${base}/v1/messages`, { + method: "POST", + headers: { "x-api-key": rwKey, "content-type": "application/json" }, + body: JSON.stringify({ from: "a", to: "b", content: `blocked ${blocked}` }), + }); + const text = await r.text(); + + expect(r.status).toBe(400); + expect(text).toContain("sensitive content detected"); + expect(text).not.toContain(blocked); + }); + + test("POST /v1/messages blocks sensitive persisted routing fields without echoing them", async () => { + const blocked = syntheticDatabaseUrl(); + const r = await fetch(`${base}/v1/messages`, { + method: "POST", + headers: { "x-api-key": rwKey, "content-type": "application/json" }, + body: JSON.stringify({ from: "a", to: blocked, content: "safe body" }), + }); + const text = await r.text(); + + expect(r.status).toBe(400); + expect(text).toContain("sensitive content detected"); + expect(text).not.toContain(blocked); + }); + + test("GET /v1/messages redacts sensitive legacy content", async () => { + const blocked = syntheticDatabaseUrl(); + const sent = await fetch(`${base}/v1/messages`, { + method: "POST", + headers: { "x-api-key": rwKey, "content-type": "application/json" }, + body: JSON.stringify({ from: "a", to: "b", content: "safe before legacy mutation" }), + }); + const created = await sent.json() as any; + // Mutate the fake backing store through the route's own insert path shape. + await activeFakeClient!.get( + `INSERT INTO messages (session_id, from_agent, to_agent, channel, project_id, content, priority, blocking) + VALUES ($1,$2,$3,$4,$5,$6,$7,$8) + RETURNING id`, + ["legacy", "legacy", "b", null, null, `legacy ${blocked}`, "normal", false], + ); + + const listText = await (await fetch(`${base}/v1/messages`, { headers: { "x-api-key": rwKey } })).text(); + const getText = await (await fetch(`${base}/v1/messages/${created.message.id}`, { headers: { "x-api-key": rwKey } })).text(); + + expect(listText).toContain("[REDACTED:DATABASE_URL]"); + expect(listText).not.toContain(blocked); + expect(getText).not.toContain(blocked); + }); + test("GET /v1/openapi.json is served for SDK discovery", async () => { const b = await (await fetch(`${base}/v1/openapi.json`)).json(); expect(b.openapi).toBeTruthy(); diff --git a/src/server/api.ts b/src/server/api.ts index 1624648..e2cb192 100644 --- a/src/server/api.ts +++ b/src/server/api.ts @@ -23,6 +23,7 @@ import { verifyApiKey, ApiKeyStore } from "@hasna/contracts/auth"; import type { ApiKeyVerifier } from "@hasna/contracts/auth"; import { version as pkgVersion } from "../../package.json"; import { openapiSpec } from "./openapi.js"; +import { assertNoSensitiveContent, redactSensitiveValue } from "../lib/content-safety.js"; export const APP = "conversations"; const SCOPE_READ = `${APP}:read`; @@ -42,6 +43,14 @@ function json(data: unknown, status = 200, extra?: Record): Resp }); } +function redactResponse(data: T): T { + return redactSensitiveValue(data); +} + +function assertNoSensitiveOptionalText(value: string | undefined, context: string): void { + if (value) assertNoSensitiveContent(value, context); +} + function signingSecret(): string { const secret = process.env.HASNA_CONVERSATIONS_API_SIGNING_KEY || @@ -161,7 +170,7 @@ export function startApiServer(options: StartApiServerOptions = {}) { "WWW-Authenticate": "Bearer", }); } - return handleV1(path, method, req, url, deps, decision.principal.agent); + return await handleV1(path, method, req, url, deps, decision.principal.agent); } return json({ error: "Not found" }, 404); @@ -212,7 +221,7 @@ async function handleV1( FROM messages ${where} ORDER BY id DESC LIMIT $${params.length}`, params, ); - return json({ messages: rows }); + return json({ messages: redactResponse(rows) }); } if (sub === "messages" && method === "POST") { @@ -221,11 +230,17 @@ async function handleV1( const to = str(body.to); const content = str(body.content); if (!from || !to || !content) return json({ error: "from, to, and content are required" }, 400); + assertNoSensitiveContent(content, "Message content"); const channel = str(body.channel); const projectId = str(body.project_id); const sessionId = str(body.session_id) ?? `api:${from}`; let priority = str(body.priority)?.toLowerCase() ?? "normal"; if (!VALID_PRIORITIES.includes(priority)) return json({ error: "Invalid priority" }, 400); + assertNoSensitiveContent(from, "Message sender"); + assertNoSensitiveContent(to, "Message recipient"); + assertNoSensitiveOptionalText(channel, "Message channel"); + assertNoSensitiveOptionalText(projectId, "Message project"); + assertNoSensitiveContent(sessionId, "Message session"); const blocking = body.blocking === true; const row = await client.get( `INSERT INTO messages (session_id, from_agent, to_agent, channel, project_id, content, priority, blocking) @@ -233,7 +248,7 @@ async function handleV1( RETURNING id, uuid, session_id, from_agent, to_agent, channel, project_id, content, priority, blocking, created_at`, [sessionId, from, to, channel ?? null, projectId ?? null, content, priority, blocking], ); - return json({ message: row }, 201); + return json({ message: redactResponse(row) }, 201); } const msgIdMatch = sub.match(/^messages\/(\d+)$/); @@ -242,7 +257,7 @@ async function handleV1( if (method === "GET") { const row = await client.get(`SELECT * FROM messages WHERE id = $1`, [id]); if (!row) return json({ error: "Message not found" }, 404); - return json({ message: row }); + return json({ message: redactResponse(row) }); } if (method === "DELETE") { const from = str(url.searchParams.get("from")) ?? agent ?? undefined;