Security + performance audit fixes (19 fixed, 2 partial, 3 deferred)

deploy/helm/cheers/templates/frontend.yaml

@@ -16,13 +16,15 @@ data:
       index index.html;
 
       # Security headers (audit M2): clickjacking + MIME-sniff + referrer + a
-      # minimal CSP. CSP is intentionally narrow (no script-src lockdown) to
-      # avoid breaking the Vite SPA; tighten with nonces later. No location here
-      # defines its own add_header, so these inherit to every response.
+      # hardened CSP. The production Vite build emits only external hashed module
+      # scripts (no inline <script>), so script-src 'self' is safe; style-src keeps
+      # 'unsafe-inline' because the SPA injects runtime styles. API + WS are proxied
+      # same-origin (/api, /ws) so connect-src 'self' ws: wss:' covers them. No
+      # location here defines its own add_header, so these inherit to every response.
       add_header X-Content-Type-Options "nosniff" always;
       add_header X-Frame-Options "DENY" always;
       add_header Referrer-Policy "strict-origin-when-cross-origin" always;
-      add_header Content-Security-Policy "frame-ancestors 'none'; object-src 'none'; base-uri 'self'" always;
+      add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob: https:; media-src 'self' blob:; font-src 'self' data:; connect-src 'self' ws: wss:; frame-ancestors 'none'; object-src 'none'; base-uri 'self'" always;
 
       location / {
         try_files $uri $uri/ /index.html;
@@ -40,7 +42,7 @@ data:
         add_header X-Content-Type-Options "nosniff" always;
         add_header X-Frame-Options "DENY" always;
         add_header Referrer-Policy "strict-origin-when-cross-origin" always;
-        add_header Content-Security-Policy "frame-ancestors 'none'; object-src 'none'; base-uri 'self'" always;
+        add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob: https:; media-src 'self' blob:; font-src 'self' data:; connect-src 'self' ws: wss:; frame-ancestors 'none'; object-src 'none'; base-uri 'self'" always;
       }
 
       location /api {

frontend/src/components/MarkdownRenderer.tsx

@@ -1,4 +1,4 @@
-import { useContext, useMemo } from "react";
+import { memo, useContext, useMemo } from "react";
 import ReactMarkdown from "react-markdown";
 import remarkGfm from "remark-gfm";
 import { PathOpenContext, looksLikePath } from "@/features/chat/workspaceLink";
@@ -46,7 +46,7 @@ function CodeBlock({
   );
 }
 
-export function MarkdownRenderer({ content, className }: Props) {
+export const MarkdownRenderer = memo(function MarkdownRenderer({ content, className }: Props) {
   const onPath = useContext(PathOpenContext);
   // react-markdown v10 dropped the `className` prop (passing it throws). Wrap in a styled
   // div instead so "prose" + caller classes still apply.
@@ -92,4 +92,4 @@ export function MarkdownRenderer({ content, className }: Props) {
     </ReactMarkdown>
     </div>
   );
-}
+});

frontend/src/features/chat/ChannelView.tsx

@@ -145,6 +145,17 @@ export function ChannelView({ channel, onBack, sidebarOpen, onToggleSidebar }: P
 
   // Channel file index (id + name) built from loaded messages' attachments — no
   // extra fetch. Powers F3 filename suggestions (draft names a file → offer it).
+  // Attachments never change during streaming, but `flushDeltas` swaps the whole
+  // `messages` array identity every rAF, so keying this off `messages` would rebuild
+  // the nested O(messages × attachments) index on every token delta. Instead key off a
+  // cheap signature of just the messages that HAVE files (id + count) — it only changes
+  // when attachments actually change, keeping both the work AND the value identity stable
+  // across deltas. (Cheap enough to compute inline each render.)
+  let channelFilesKey = `${messages.length}`;
+  for (const m of messages) {
+    const n = m.files?.length ?? 0;
+    if (n) channelFilesKey += `|${m.msg_id}:${n}`;
+  }
   const channelFiles = useMemo(() => {
     const byId = new Map<string, { file_id: string; filename: string }>();
     for (const m of messages) {
@@ -156,7 +167,8 @@ export function ChannelView({ channel, onBack, sidebarOpen, onToggleSidebar }: P
       }
     }
     return Array.from(byId.values());
-  }, [messages]);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [channelFilesKey]);
 
   // A different channel means a different session set — drop any prior target.
   // Also drop any buffered stream deltas + cancel a pending flush so a stale frame
@@ -577,6 +589,20 @@ export function ChannelView({ channel, onBack, sidebarOpen, onToggleSidebar }: P
     setShowConnBanner(rtStatus === "offline");
   }, [rtStatus]);
 
+  // If the realtime connection drops mid-stream, a bot bubble can be left with
+  // `_streaming: true` indefinitely (no done frame arrives), which the streaming
+  // fast-path renders as plain pre-wrap text. Finalize any lingering stream when we
+  // go offline so it falls back to full Markdown; reconnect catch-up then reconciles
+  // it with the persisted server state.
+  useEffect(() => {
+    if (rtStatus !== "offline") return;
+    setMessages((prev) =>
+      prev.some((m) => m._streaming)
+        ? prev.map((m) => (m._streaming ? { ...m, _streaming: false } : m))
+        : prev
+    );
+  }, [rtStatus]);
+
   // Re-flatten the palette when bot labels resolve after the initial fetch.
   useEffect(() => {
     void loadCommands();

frontend/src/features/chat/MessageItem.tsx

@@ -472,14 +472,20 @@ function MessageBody({
     return <p className="text-sm text-red-400 italic">{message.error}</p>;
   }
 
+  // While a bubble is streaming, re-parsing the whole accumulated Markdown +
+  // re-highlighting the growing code block every animation frame is ~O(len^2)
+  // main-thread work. Render the in-flight text as plain whitespace-pre-wrap and
+  // only switch to full Markdown + highlighting once the turn finalizes
+  // (_streaming clears), which leaves completed messages rendered exactly as before.
   const hasMarkdown =
-    content.includes("```") ||
-    content.includes("**") ||
-    content.includes("*") ||
-    content.includes("#") ||
-    content.includes("[") ||
-    content.includes("\n") ||
-    content.includes("`");
+    !message._streaming &&
+    (content.includes("```") ||
+      content.includes("**") ||
+      content.includes("*") ||
+      content.includes("#") ||
+      content.includes("[") ||
+      content.includes("\n") ||
+      content.includes("`"));
 
   return (
     <div className="relative">

packages/cheers-acp-connector-rs/src/acp_adapter.rs

@@ -979,23 +979,31 @@ async fn handle_peer_message(
 async fn handle_peer_notification(
     account_id: &str,
     event_tx: &mpsc::Sender<RuntimeEvent>,
-    value: Value,
+    mut value: Value,
 ) -> anyhow::Result<()> {
     let method = value.get("method").and_then(Value::as_str).unwrap_or("");
     if method != "session/update" {
         tracing::debug!(account = %account_id, method, "ACP peer notification (not session/update; ignored)");
         return Ok(());
     }
-    let params = value.get("params").cloned().unwrap_or(Value::Null);
-    let acp_session_id = params
-        .get("sessionId")
+    // Read only the small sessionId out of the borrowed value...
+    let acp_session_id = value
+        .get("params")
+        .and_then(|params| params.get("sessionId"))
         .and_then(Value::as_str)
         .unwrap_or_default()
         .to_string();
     if acp_session_id.is_empty() {
         return Ok(());
     }
-    let update = params.get("update").cloned().unwrap_or(Value::Null);
+    // ...then MOVE the (potentially large) update subtree out of the owned value
+    // instead of deep-cloning it — session/update is the highest-frequency event
+    // and tool_call_update carries the bulky tool output.
+    let update = value
+        .get_mut("params")
+        .and_then(|params| params.get_mut("update"))
+        .map(Value::take)
+        .unwrap_or(Value::Null);
     tracing::debug!(
         account = %account_id,
         session = %acp_session_id,

packages/cheers-acp-connector-rs/src/bridge_runtime/mod.rs

@@ -1966,8 +1966,14 @@ impl RuntimeContext {
                 )
                 .await?;
                 let (final_text, file_ids) = {
-                    let guard = run.lock().await;
-                    (guard.text.clone(), guard.created_file_ids.clone())
+                    let mut guard = run.lock().await;
+                    // Move the accumulated text out (turn is over; the run is about
+                    // to be dropped from the shared maps below) so the whole streamed
+                    // response isn't deep-cloned just to hand it to the Done frame.
+                    (
+                        std::mem::take(&mut guard.text),
+                        guard.created_file_ids.clone(),
+                    )
                 };
                 let terminal_ack = self
                     .io

packages/cheers-acp-connector-rs/src/loopback.rs

@@ -16,6 +16,13 @@ use uuid::Uuid;
 /// a pooled-but-idle MCP socket can't leak a task/fd for the life of the session.
 const LOOPBACK_IDLE_TIMEOUT: Duration = Duration::from_secs(120);
 
+/// Hard ceiling on a single loopback request body. The only legitimate client is
+/// the local MCP bridge, whose resource calls are small JSON payloads, so a few
+/// MiB is generous. Without this cap a local process could set an arbitrary
+/// `Content-Length` and force the connection task to buffer that many bytes on the
+/// heap before the token is even checked.
+const MAX_LOOPBACK_BODY_BYTES: usize = 8 * 1024 * 1024;
+
 #[derive(Debug, Clone)]
 pub struct LoopbackHandle {
     pub url: String,
@@ -233,6 +240,21 @@ async fn read_http_request(
         .get("content-length")
         .and_then(|value| value.parse::<usize>().ok())
         .unwrap_or(0);
+    // Reject an oversized body from the declared Content-Length BEFORE draining it,
+    // so a local process can't force us to buffer an arbitrary amount of heap. We
+    // still write a proper 413 and then close (returning Err drops the connection).
+    if content_length > MAX_LOOPBACK_BODY_BYTES {
+        write_http_json(
+            stream,
+            413,
+            false,
+            &json!({ "ok": false, "code": "PAYLOAD_TOO_LARGE", "error": "loopback request body too large" }),
+        )
+        .await?;
+        return Err(anyhow!(
+            "loopback request body too large: {content_length} bytes (max {MAX_LOOPBACK_BODY_BYTES})"
+        ));
+    }
     let body_start = header_end + 4;
     let body_end = body_start + content_length;
     while buf.len() < body_end {
@@ -280,6 +302,7 @@ async fn write_http_json(
         200 => "OK",
         401 => "Unauthorized",
         405 => "Method Not Allowed",
+        413 => "Payload Too Large",
         _ => "Error",
     };
     // Content-Length is always present, so the client can frame the body and reuse

packages/cheers-acp-connector-rs/src/self_update.rs

@@ -183,6 +183,12 @@ struct Marker {
     /// can't put the connector into a swap/rollback loop.
     #[serde(default)]
     blocked_versions: Vec<String>,
+    /// Highest version this connector has ever applied. Persisted so a hostile
+    /// gateway can't force a downgrade to a superseded — but still correctly
+    /// signed — historical build that merely happens to be newer than whatever
+    /// is running right now. Monotonic: only ever moves forward.
+    #[serde(default)]
+    highest_applied: Option<String>,
 }
 
 fn marker_path(state_dir: &Path) -> PathBuf {
@@ -485,9 +491,19 @@ impl SelfUpdater {
             .connect_timeout(Duration::from_secs(10))
             .timeout(Duration::from_secs(300))
             .build()?;
-        let manifest_bytes = fetch(&client, &format!("{base}/connector-manifest.json")).await?;
+        let manifest_bytes = fetch(
+            &client,
+            &format!("{base}/connector-manifest.json"),
+            MAX_METADATA_BYTES,
+        )
+        .await?;
         let sig_b64 = String::from_utf8(
-            fetch(&client, &format!("{base}/connector-manifest.json.sig")).await?,
+            fetch(
+                &client,
+                &format!("{base}/connector-manifest.json.sig"),
+                MAX_METADATA_BYTES,
+            )
+            .await?,
         )
         .context("manifest signature is not UTF-8")?;
         let manifest = verify_and_parse_manifest(&manifest_bytes, &sig_b64, self.pubkey_pem())?;
@@ -502,6 +518,22 @@ impl SelfUpdater {
             ));
         }
 
+        // Client-side monotonic anti-downgrade: a manifest may be older than the
+        // highest version we've ever applied yet still newer than what's running
+        // (e.g. a bad manual downgrade, or a rollback that restored an older
+        // binary). The signed manifest can't carry a per-client floor, so we keep
+        // one on disk and refuse anything at or below it. This blocks a hostile
+        // gateway from pinning us to a superseded, correctly-signed build.
+        if let Some(highest) = read_marker(&self.state_dir).and_then(|m| m.highest_applied) {
+            if !version_is_newer(&manifest.version, &highest) {
+                return Err(anyhow!(
+                    "signed manifest is for {} which is not newer than the highest \
+                     version ever applied ({highest}) — refusing (anti-downgrade)",
+                    manifest.version
+                ));
+            }
+        }
+
         let suffix = platform_asset_suffix().expect("checked in disabled_reason");
         let exe = std::env::current_exe()?;
         let exe_dir = exe
@@ -532,7 +564,7 @@ impl SelfUpdater {
                 .get(asset)
                 .ok_or_else(|| anyhow!("manifest has no asset {asset}"))?
                 .sha256;
-            let bytes = fetch(&client, &format!("{base}/{asset}")).await?;
+            let bytes = fetch(&client, &format!("{base}/{asset}"), MAX_BINARY_BYTES).await?;
             let actual = sha256_hex(&bytes);
             if &actual != expected {
                 return Err(anyhow!(
@@ -601,33 +633,75 @@ impl SelfUpdater {
                 previous_exe = Some(old);
             }
         }
+        let prev_marker = read_marker(&self.state_dir);
+        let blocked_versions = prev_marker
+            .as_ref()
+            .map(|m| m.blocked_versions.clone())
+            .unwrap_or_default();
+        // Advance the monotonic floor to the version we're applying (never let it
+        // move backward — read_marker may carry a higher one).
+        let highest_applied = match prev_marker.and_then(|m| m.highest_applied) {
+            Some(prev) if !version_is_newer(version, &prev) => Some(prev),
+            _ => Some(version.to_string()),
+        };
         write_marker(
             &self.state_dir,
             &Marker {
                 version: version.to_string(),
                 previous_exe,
                 attempts: 0,
                 confirmed: false,
-                blocked_versions: read_marker(&self.state_dir)
-                    .map(|m| m.blocked_versions)
-                    .unwrap_or_default(),
+                blocked_versions,
+                highest_applied,
             },
         )?;
         tracing::info!(version = %version, "self-update: binaries swapped, restarting");
         exec_self(&exe)
     }
 }
 
-async fn fetch(client: &reqwest::Client, url: &str) -> anyhow::Result<Vec<u8>> {
-    let resp = client
+/// Download cap for the manifest and its signature — both are tiny JSON/sig.
+const MAX_METADATA_BYTES: usize = 64 * 1024;
+/// Download cap for a single binary asset.
+const MAX_BINARY_BYTES: usize = 128 * 1024 * 1024;
+
+/// GET `url` into memory, refusing to buffer more than `max_bytes`. The cap is
+/// enforced twice: against an advertised `Content-Length` before reading (fast
+/// reject) and against the running byte count while streaming (defends against a
+/// lying or absent `Content-Length` — localhost gateways are plain http, so a
+/// MITM/hostile proxy could otherwise stream a multi-GB body and OOM us). All
+/// signature/sha256 verification still happens on the caller side, after this
+/// bounded download returns.
+async fn fetch(client: &reqwest::Client, url: &str, max_bytes: usize) -> anyhow::Result<Vec<u8>> {
+    let mut resp = client
         .get(url)
         .send()
         .await
         .with_context(|| format!("GET {url}"))?;
     if !resp.status().is_success() {
         return Err(anyhow!("GET {url} returned HTTP {}", resp.status()));
     }
-    Ok(resp.bytes().await?.to_vec())
+    if let Some(len) = resp.content_length() {
+        if len > max_bytes as u64 {
+            return Err(anyhow!(
+                "GET {url}: Content-Length {len} exceeds cap of {max_bytes} bytes"
+            ));
+        }
+    }
+    let mut buf: Vec<u8> = Vec::new();
+    while let Some(chunk) = resp
+        .chunk()
+        .await
+        .with_context(|| format!("read body of {url}"))?
+    {
+        if buf.len() + chunk.len() > max_bytes {
+            return Err(anyhow!(
+                "GET {url}: response body exceeds cap of {max_bytes} bytes"
+            ));
+        }
+        buf.extend_from_slice(&chunk);
+    }
+    Ok(buf)
 }
 
 /// Replace this process with `exe`, keeping argv (daemon metadata matches on
@@ -785,6 +859,7 @@ mod tests {
                 attempts: 0,
                 confirmed: false,
                 blocked_versions: vec![],
+                highest_applied: None,
             },
         )
         .expect("write marker");
@@ -807,6 +882,7 @@ mod tests {
                 attempts: 1,
                 confirmed: false,
                 blocked_versions: vec![],
+                highest_applied: None,
             },
         )
         .expect("write marker");

server/migrations/0049_messages_pending_permission_index.sql

@@ -0,0 +1,9 @@
+-- Fleet inbox pending scan (fleet::find_pending_for_user / find_pending_for_user_all)
+-- selects unresolved `permission` messages `ORDER BY created_at DESC LIMIT 100`. With no
+-- index on `msg_type`, that scan reads the hot `messages` table by created_at and filters
+-- rows post-hoc. A partial index on the small `permission` subset makes the ordered scan
+-- index-driven (permission rows are a tiny fraction of all messages), so the badge/fleet
+-- queries touch only pending cards instead of walking the whole timeline.
+CREATE INDEX IF NOT EXISTS ix_messages_pending_permission
+    ON messages (created_at DESC)
+    WHERE msg_type = 'permission';