feat(feedback): deliver shared diagnostic reports to Chatwoot support inbox (#88)

* feat(feedback): deliver shared diagnostic reports to Chatwoot support inbox Wire shareReport() to the Chatwoot public client API (/public/api/v1/inboxes/{inbox_identifier}) so user-shared diagnostic reports also reach the OpenCode Mobile Feedback inbox. - New src/lib/chatwoot.ts: dependency-injected, node-testable client — anonymous contact -> conversation -> message. Ships only the inbox identifier (EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER); never an account api_access_token. Contact source_id persisted via SecureStore for conversation continuity; stale id recreated on 404. - Delivery is gated on the same telemetry consent flag as Sentry/PostHog and is best-effort (share sheet never blocks on it). - Reports are scrubbed before leaving the device: all URLs and every occurrence of the target host redacted (new redactHostAndUrls in scrub.ts). - CI: pass EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER in build and Play-publish workflows. Deliberately NOT added to the F-Droid workflow to avoid widening reproducible-build divergence (#86). - Consent modal copy discloses support-inbox delivery. Closes #85 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(feedback): close host-leak gaps in support-report scrubbing Security review findings on the Chatwoot delivery path: - Log-buffer lines record server hosts without a scheme, which the URL regex never matches, and crash reports carry no host of their own — so bare hostnames could reach the support inbox. Track every host probed this session and redact them all in the support copy. - Redact bare IPv4 addresses as a catch-all for hosts never parsed. - Resolve telemetry consent from SecureStore when a report is shared before startup finished loading it, instead of silently dropping. - Move redactHostAndUrls tests to scrub.test.ts alongside the module. Refs #85 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-16 23:08:40 -07:00
parent 004fa13795
commit 0bedad366b
8 changed files with 369 additions and 4 deletions
--- a/src/lib/chatwoot.test.ts
+++ b/src/lib/chatwoot.test.ts
@@ -0,0 +1,131 @@
+import { test, beforeEach, afterEach } from "node:test"
+import assert from "node:assert/strict"
+import { chatwootConfigured, sendSupportReport } from "./chatwoot.ts"
+
+const INBOX = "inbox-token-abc"
+const BASE = "https://support.example.com"
+
+const savedEnv = { ...process.env }
+
+beforeEach(() => {
+  process.env.EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER = INBOX
+  process.env.EXPO_PUBLIC_CHATWOOT_BASE_URL = BASE
+})
+
+afterEach(() => {
+  process.env = { ...savedEnv }
+})
+
+interface Call {
+  url: string
+  body: Record<string, unknown>
+}
+
+// Sequenced fetch mock: each entry answers one call, in order.
+function mockFetch(responses: Array<{ status: number; json?: unknown }>) {
+  const calls: Call[] = []
+  const fetchFn = (async (url: RequestInfo | URL, init?: RequestInit) => {
+    calls.push({ url: String(url), body: JSON.parse(String(init?.body ?? "{}")) })
+    const next = responses[calls.length - 1] ?? { status: 500 }
+    return {
+      status: next.status,
+      json: async () => {
+        if (next.json === undefined) throw new Error("no body")
+        return next.json
+      },
+    } as Response
+  }) as typeof fetch
+  return { fetchFn, calls }
+}
+
+test("chatwootConfigured false without inbox identifier", () => {
+  delete process.env.EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER
+  assert.equal(chatwootConfigured(), false)
+})
+
+test("sendSupportReport returns false when unconfigured, makes no calls", async () => {
+  delete process.env.EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER
+  const { fetchFn, calls } = mockFetch([])
+  assert.equal(await sendSupportReport("hello", { fetchFn }), false)
+  assert.equal(calls.length, 0)
+})
+
+test("happy path: contact -> conversation -> message, source_id saved", async () => {
+  const { fetchFn, calls } = mockFetch([
+    { status: 200, json: { source_id: "src-1" } },
+    { status: 200, json: { id: 42 } },
+    { status: 200, json: {} },
+  ])
+  let saved: string | null = null
+  const ok = await sendSupportReport("report body", {
+    fetchFn,
+    loadSourceId: async () => null,
+    saveSourceId: async (id) => {
+      saved = id
+    },
+  })
+  assert.equal(ok, true)
+  assert.equal(saved, "src-1")
+  assert.deepEqual(
+    calls.map((c) => c.url),
+    [
+      `${BASE}/public/api/v1/inboxes/${INBOX}/contacts`,
+      `${BASE}/public/api/v1/inboxes/${INBOX}/contacts/src-1/conversations`,
+      `${BASE}/public/api/v1/inboxes/${INBOX}/contacts/src-1/conversations/42/messages`,
+    ],
+  )
+  assert.equal(calls[2].body.content, "report body")
+})
+
+test("stored source_id skips contact creation", async () => {
+  const { fetchFn, calls } = mockFetch([
+    { status: 200, json: { id: 7 } },
+    { status: 200, json: {} },
+  ])
+  const ok = await sendSupportReport("hi", { fetchFn, loadSourceId: async () => "src-old" })
+  assert.equal(ok, true)
+  assert.equal(calls.length, 2)
+  assert.match(calls[0].url, /contacts\/src-old\/conversations$/)
+})
+
+test("stale source_id (404) recreates contact once and retries", async () => {
+  const { fetchFn, calls } = mockFetch([
+    { status: 404, json: { error: "resource could not be found" } },
+    { status: 200, json: { source_id: "src-new" } },
+    { status: 200, json: { id: 9 } },
+    { status: 200, json: {} },
+  ])
+  let saved: string | null = null
+  const ok = await sendSupportReport("hi", {
+    fetchFn,
+    loadSourceId: async () => "src-stale",
+    saveSourceId: async (id) => {
+      saved = id
+    },
+  })
+  assert.equal(ok, true)
+  assert.equal(saved, "src-new")
+  assert.equal(calls.length, 4)
+  assert.match(calls[3].url, /contacts\/src-new\/conversations\/9\/messages$/)
+})
+
+test("contact create failure throws", async () => {
+  const { fetchFn } = mockFetch([{ status: 500, json: { error: "boom" } }])
+  await assert.rejects(() => sendSupportReport("hi", { fetchFn, loadSourceId: async () => null }), /http 500/)
+})
+
+test("non-404 conversation failure does not recreate contact", async () => {
+  const { fetchFn, calls } = mockFetch([{ status: 500 }])
+  await assert.rejects(() => sendSupportReport("hi", { fetchFn, loadSourceId: async () => "src-x" }), /http 500/)
+  assert.equal(calls.length, 1)
+})
+
+test("base url trailing slash is normalised", async () => {
+  process.env.EXPO_PUBLIC_CHATWOOT_BASE_URL = `${BASE}///`
+  const { fetchFn, calls } = mockFetch([
+    { status: 200, json: { id: 1 } },
+    { status: 200, json: {} },
+  ])
+  await sendSupportReport("hi", { fetchFn, loadSourceId: async () => "s" })
+  assert.equal(calls[0].url, `${BASE}/public/api/v1/inboxes/${INBOX}/contacts/s/conversations`)
+})
--- a/src/lib/chatwoot.ts
+++ b/src/lib/chatwoot.ts
@@ -0,0 +1,143 @@
+// Chatwoot support-feedback client using the PUBLIC client API
+// (/public/api/v1/inboxes/{inbox_identifier}/...), which is designed for
+// untrusted clients: the only value shipped in the binary is the inbox
+// identifier. Never put an account-level api_access_token in this app —
+// that token can read/write the whole support account.
+//
+// Contacts are created anonymously (no identifier), so Chatwoot's HMAC
+// identifier-validation does not apply. The returned contact source_id is
+// persisted via injected storage hooks so repeat reports from the same
+// install land on the same contact.
+//
+// This module is dependency-injected and imports no React Native / Expo
+// packages so it runs under `node --test`.
+
+const REQUEST_TIMEOUT_MS = 15_000
+
+export interface ChatwootDeps {
+  fetchFn?: typeof fetch
+  loadSourceId?: () => Promise<string | null>
+  saveSourceId?: (id: string) => Promise<void>
+}
+
+interface ChatwootConfig {
+  baseUrl: string
+  inboxIdentifier: string
+}
+
+export function getChatwootConfig(): ChatwootConfig | null {
+  const inboxIdentifier = process.env.EXPO_PUBLIC_CHATWOOT_INBOX_IDENTIFIER
+  if (!inboxIdentifier) return null
+  const baseUrl = (process.env.EXPO_PUBLIC_CHATWOOT_BASE_URL || "https://support.agentlabs.cc").replace(/\/+$/, "")
+  return { baseUrl, inboxIdentifier }
+}
+
+export function chatwootConfigured(): boolean {
+  return getChatwootConfig() !== null
+}
+
+async function post(
+  fetchFn: typeof fetch,
+  url: string,
+  body: Record<string, unknown>,
+): Promise<{ status: number; json: Record<string, unknown> | null }> {
+  const controller = new AbortController()
+  const timer = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS)
+  try {
+    const res = await fetchFn(url, {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify(body),
+      signal: controller.signal,
+    })
+    let json: Record<string, unknown> | null = null
+    try {
+      json = (await res.json()) as Record<string, unknown>
+    } catch {
+      // non-JSON error body
+    }
+    return { status: res.status, json }
+  } finally {
+    clearTimeout(timer)
+  }
+}
+
+async function createContact(fetchFn: typeof fetch, cfg: ChatwootConfig): Promise<string> {
+  const { status, json } = await post(fetchFn, `${cfg.baseUrl}/public/api/v1/inboxes/${cfg.inboxIdentifier}/contacts`, {
+    name: "OpenCode Mobile user",
+  })
+  const sourceId = json?.source_id
+  if (status >= 400 || typeof sourceId !== "string" || !sourceId) {
+    throw new Error(`chatwoot contact create failed (http ${status})`)
+  }
+  return sourceId
+}
+
+async function createConversation(fetchFn: typeof fetch, cfg: ChatwootConfig, sourceId: string): Promise<number> {
+  const { status, json } = await post(
+    fetchFn,
+    `${cfg.baseUrl}/public/api/v1/inboxes/${cfg.inboxIdentifier}/contacts/${encodeURIComponent(sourceId)}/conversations`,
+    {},
+  )
+  const id = json?.id
+  if (status >= 400 || typeof id !== "number") {
+    const err = new Error(`chatwoot conversation create failed (http ${status})`)
+    ;(err as Error & { status?: number }).status = status
+    throw err
+  }
+  return id
+}
+
+async function postMessage(
+  fetchFn: typeof fetch,
+  cfg: ChatwootConfig,
+  sourceId: string,
+  conversationId: number,
+  content: string,
+): Promise<void> {
+  const { status } = await post(
+    fetchFn,
+    `${cfg.baseUrl}/public/api/v1/inboxes/${cfg.inboxIdentifier}/contacts/${encodeURIComponent(sourceId)}/conversations/${conversationId}/messages`,
+    { content },
+  )
+  if (status >= 400) throw new Error(`chatwoot message post failed (http ${status})`)
+}
+
+/**
+ * Send an already-scrubbed report to the support inbox as a new conversation.
+ * Returns true on success, false when Chatwoot is not configured.
+ * Throws on network/API failure — callers decide whether that is fatal
+ * (the in-app share path treats it as best-effort).
+ *
+ * Consent is NOT checked here; this module has no access to the consent
+ * store. Callers MUST gate on telemetry consent before invoking.
+ */
+export async function sendSupportReport(content: string, deps: ChatwootDeps = {}): Promise<boolean> {
+  const cfg = getChatwootConfig()
+  if (!cfg) return false
+  const fetchFn = deps.fetchFn ?? fetch
+
+  let sourceId = (await deps.loadSourceId?.().catch(() => null)) ?? null
+  let created = false
+  if (!sourceId) {
+    sourceId = await createContact(fetchFn, cfg)
+    created = true
+  }
+
+  let conversationId: number
+  try {
+    conversationId = await createConversation(fetchFn, cfg, sourceId)
+  } catch (error) {
+    const status = (error as { status?: number }).status
+    // A persisted source_id can go stale (contact deleted server-side).
+    // Recreate the contact once and retry.
+    if (created || status !== 404) throw error
+    sourceId = await createContact(fetchFn, cfg)
+    created = true
+    conversationId = await createConversation(fetchFn, cfg, sourceId)
+  }
+
+  await postMessage(fetchFn, cfg, sourceId, conversationId, content)
+  if (created) await deps.saveSourceId?.(sourceId).catch(() => undefined)
+  return true
+}
--- a/src/lib/diagnostics.ts
+++ b/src/lib/diagnostics.ts
@@ -4,9 +4,13 @@
 import { Platform, Share } from "react-native"
 import * as Clipboard from "expo-clipboard"
 import * as Device from "expo-device"
+import * as SecureStore from "expo-secure-store"
 import appJson from "../../app.json"
 import { log, formatLogLines } from "./logbuffer"
 import { type Classification, type ProbeAttempt, type ParsedUrl, parseUrl, classify } from "./diagnostics-classify"
+import { chatwootConfigured, sendSupportReport } from "./chatwoot"
+import { hasTelemetryConsent, loadTelemetryConsent } from "./telemetry"
+import { redactHostAndUrls } from "./scrub"

 export type { Classification, ProbeAttempt } from "./diagnostics-classify"

@@ -55,8 +59,14 @@ async function timedFetch(name: string, target: string, init?: RequestInit): Pro
  }
 }

+// Every host probed this session. Log lines mention hosts without a scheme
+// (so URL-based scrubbing misses them), and a crash report has no host of its
+// own — this set lets formatReportForSupport redact them all regardless.
+const seenHosts = new Set<string>()
+
 export async function probeConnection(url: string, auth?: { username: string; password: string }): Promise<DiagnosticReport> {
  const parsed = parseUrl(url)
+  if (parsed.host) seenHosts.add(parsed.host)
  log.info("diag", "probe start", url, "parsed", JSON.stringify(parsed))

  const headers: Record<string, string> = {}
@@ -160,10 +170,42 @@ export function buildCrashReport(error: unknown, source: "react-boundary" | "glo
  }
 }

+// Variant of formatReport for sending off-device: the user's server address
+// must never leave the phone, so every URL and every occurrence of the target
+// host is redacted. Classification, probe outcomes, device info and (URL-
+// scrubbed) logs survive — that is what support needs.
+export function formatReportForSupport(report: DiagnosticReport): string {
+  return redactHostAndUrls(formatReport(report), [report.host, ...seenHosts])
+}
+
+const CHATWOOT_SOURCE_KEY = "opencode_chatwoot_source_id"
+
+// Best-effort delivery of the scrubbed report to the Chatwoot support inbox.
+// Only runs when the user has granted telemetry consent (same flag that
+// gates Sentry/analytics) and the inbox is configured for this build.
+async function sendReportToSupport(report: DiagnosticReport): Promise<void> {
+  // Consent may not be loaded yet if a crash happens very early in startup —
+  // resolve it from the store rather than silently dropping the report.
+  if (hasTelemetryConsent() === null) await loadTelemetryConsent()
+  if (hasTelemetryConsent() !== true || !chatwootConfigured()) return
+  try {
+    await sendSupportReport(formatReportForSupport(report), {
+      loadSourceId: () => SecureStore.getItemAsync(CHATWOOT_SOURCE_KEY),
+      saveSourceId: (id) => SecureStore.setItemAsync(CHATWOOT_SOURCE_KEY, id),
+    })
+    log.info("diag", "report delivered to support inbox")
+  } catch (e) {
+    log.warn("diag", "support delivery failed", String(e))
+  }
+}
+
 // Copy the report to the clipboard and open the native share sheet.
-// Works fully offline (unlike the Sentry auto-upload).
+// Works fully offline (unlike the Sentry auto-upload). When telemetry
+// consent is granted, a scrubbed copy is also delivered to the support
+// inbox so reports reach us even if the user cancels the share sheet.
 export async function shareReport(report: DiagnosticReport): Promise<void> {
  const text = formatReport(report)
+  void sendReportToSupport(report)
  try {
    await Clipboard.setStringAsync(text)
  } catch {
--- a/src/lib/scrub.test.ts
+++ b/src/lib/scrub.test.ts
@@ -1,6 +1,6 @@
 import { test } from "node:test"
 import assert from "node:assert/strict"
-import { scrubUrl, scrubString, scrubObject } from "./scrub.ts"
+import { scrubUrl, scrubString, scrubObject, redactHostAndUrls } from "./scrub.ts"

 // scrubUrl ----------------------------------------------------------------

@@ -103,3 +103,36 @@ test("scrubObject: preserves non-string, non-object values as-is", () => {
  assert.deepEqual(result.items, [1, 2, 3])
  assert.equal(result.nothing, null)
 })
+
+// redactHostAndUrls -------------------------------------------------------
+
+test("redactHostAndUrls: strips URLs, bare host occurrences, credentials", () => {
+  const host = "my-dev-box.tail1234.ts.net"
+  const text = [
+    `Target URL:  https://user:pw@${host}:4096/api`,
+    `  scheme=https host=${host} port=4096 hostname=true`,
+    `probe start http://${host}:4096/global/health`,
+    "internet https://www.gstatic.com/generate_204 OK",
+  ].join("\n")
+  const out = redactHostAndUrls(text, [host])
+  assert.ok(!out.includes(host), "host must not survive")
+  assert.ok(!out.includes("user:pw"), "credentials must not survive")
+  assert.ok(out.includes("<redacted-url>"))
+  assert.ok(out.includes("host=<redacted-host>"))
+})
+
+test("redactHostAndUrls: redacts every session host, not just the report's own", () => {
+  // Crash-report case: report.host is undefined but earlier failed-connect
+  // log lines mention hosts without a scheme.
+  const text = ['{"host":"box-a.tailnet.ts.net","port":"4096"}', "server unreachable box-b.local:8080"].join("\n")
+  const out = redactHostAndUrls(text, [undefined, "box-a.tailnet.ts.net", "box-b.local"])
+  assert.ok(!out.includes("box-a.tailnet.ts.net"))
+  assert.ok(!out.includes("box-b.local"))
+})
+
+test("redactHostAndUrls: blanks bare IPv4 addresses even when unknown", () => {
+  const out = redactHostAndUrls("connect failed 192.168.1.50:4096 via 10.0.0.1", [])
+  assert.ok(!out.includes("192.168.1.50"))
+  assert.ok(!out.includes("10.0.0.1"))
+  assert.ok(out.includes("<redacted-ip>"))
+})
--- a/src/lib/scrub.ts
+++ b/src/lib/scrub.ts
@@ -15,6 +15,19 @@ export function scrubString(s: string): string {
  return s.replace(/https?:\/\/\S+/g, (m) => scrubUrl(m))
 }

+// Harder redaction for text that leaves the device (support inbox): drop
+// every URL wholesale, erase every known server host (bare `host=…` fragments
+// and log lines carry hosts without a scheme, which the URL regex misses),
+// and blank bare IPv4 addresses as a catch-all for hosts we never parsed.
+export function redactHostAndUrls(text: string, hosts?: Array<string | undefined>): string {
+  let out = text.replace(/https?:\/\/[^\s)\]}"']+/gi, "<redacted-url>")
+  for (const host of hosts ?? []) {
+    if (host) out = out.split(host).join("<redacted-host>")
+  }
+  out = out.replace(/\b(?:\d{1,3}\.){3}\d{1,3}\b/g, "<redacted-ip>")
+  return out
+}
+
 export function scrubObject(obj: Record<string, unknown>): Record<string, unknown> {
  const out: Record<string, unknown> = {}
  for (const [k, v] of Object.entries(obj)) {