fix(security): harden untrusted-input sinks (find file-write, source links, deep links) (#4158)

app/src/lib/notificationRouter.test.ts

@@ -66,6 +66,22 @@ describe('resolveIntegrationRoute', () => {
     const n = makeIntegration({ provider: 'slack', deep_link: '/skills' });
     expect(resolveIntegrationRoute(n)).toBe('/skills');
   });
+
+  it.each([
+    'javascript:alert(1)',
+    '//evil.example.com',
+    'https://evil.example.com/x',
+    'http://evil.example.com',
+    'data:text/html,x',
+    'mailto:x@y.z',
+    'relative/no/leading/slash',
+    '\\evil',
+  ])('ignores unsafe deep_link %s and uses the provider default', deep_link => {
+    // `deep_link` derives from untrusted inbound provider content, so a value
+    // that is not a relative in-app path must be dropped, not navigated to.
+    const n = makeIntegration({ provider: 'slack', deep_link });
+    expect(resolveIntegrationRoute(n)).toBe('/chat');
+  });
 });
 
 // ─────────────────────────────────────────────────────────────────────────────
@@ -110,4 +126,16 @@ describe('resolveSystemRoute', () => {
     const item = makeSystem({ category: 'messages', deepLink: '/notifications' });
     expect(resolveSystemRoute(item)).toBe('/notifications');
   });
+
+  it.each([
+    'javascript:alert(1)',
+    '//evil.example.com',
+    'https://evil.example.com/x',
+    'data:text/html,x',
+    'relative/no/leading/slash',
+    '\\evil',
+  ])('ignores unsafe deepLink %s and uses the category default', deepLink => {
+    const item = makeSystem({ category: 'messages', deepLink });
+    expect(resolveSystemRoute(item)).toBe('/chat');
+  });
 });

app/src/lib/notificationRouter.ts

@@ -33,6 +33,23 @@ const MESSAGE_PROVIDERS = new Set([
   'twitter',
 ]);
 
+/**
+ * A `deep_link` / `deepLink` is set by the core triage pipeline, which derives
+ * it from untrusted inbound provider content. Callers feed the result straight
+ * to react-router `navigate()`, so accept it only when it is a relative in-app
+ * path: a single leading `/`, no protocol-relative `//`, no scheme, no
+ * backslash. This blocks `javascript:` / `http(s)://evil` / `//host` from
+ * reaching the router (HashRouter already contains most of these, but the
+ * allowlist keeps navigation strictly in-app). Unsafe values fall through to
+ * the provider/category default.
+ */
+function isSafeInAppPath(path: string): boolean {
+  // A single leading slash, no protocol-relative `//`, no backslash. Keeps the
+  // value a relative in-app route, so `javascript:` / `http(s)://evil` / `//host`
+  // can never reach react-router `navigate()`.
+  return path.startsWith('/') && path[1] !== '/' && path[1] !== '\\' && !path.includes('\\');
+}
+
 // ─────────────────────────────────────────────────────────────────────────────
 // Route resolvers
 // ─────────────────────────────────────────────────────────────────────────────
@@ -46,10 +63,13 @@ const MESSAGE_PROVIDERS = new Set([
  *   3. `/notifications` fallback.
  */
 export function resolveIntegrationRoute(n: IntegrationNotification): string {
-  if (n.deep_link) {
+  if (n.deep_link && isSafeInAppPath(n.deep_link)) {
     log('[notification-router] integration id=%s explicit deep_link=%s', n.id, n.deep_link);
     return n.deep_link;
   }
+  if (n.deep_link) {
+    log('[notification-router] integration id=%s ignored unsafe deep_link=%s', n.id, n.deep_link);
+  }
 
   if (MESSAGE_PROVIDERS.has(n.provider)) {
     log('[notification-router] integration id=%s provider=%s → /chat', n.id, n.provider);
@@ -73,10 +93,13 @@ export function resolveIntegrationRoute(n: IntegrationNotification): string {
  *   3. `/notifications` fallback.
  */
 export function resolveSystemRoute(item: NotificationItem): string {
-  if (item.deepLink) {
+  if (item.deepLink && isSafeInAppPath(item.deepLink)) {
     log('[notification-router] system id=%s explicit deepLink=%s', item.id, item.deepLink);
     return item.deepLink;
   }
+  if (item.deepLink) {
+    log('[notification-router] system id=%s ignored unsafe deepLink=%s', item.id, item.deepLink);
+  }
 
   switch (item.category) {
     case 'messages':

app/src/utils/__tests__/toolTimelineFormatting.test.ts

@@ -5,6 +5,7 @@ import type { PersistedTranscriptItem } from '../../types/turnState';
 import {
   buildProcessingBlocks,
   categorizeTool,
+  extractAgentSources,
   formatTimelineEntry,
   formatToolName,
   isKnownClientTool,
@@ -373,3 +374,38 @@ describe('formatter null-safety (malformed / legacy snapshot guard)', () => {
     expect(stripToolCallEnvelopes(null)).toBe('');
   });
 });
+
+describe('extractAgentSources', () => {
+  const source = (id: string, url: string): ToolTimelineEntry =>
+    entry({ id, name: 'web_fetch', argsBuffer: JSON.stringify({ url }) });
+
+  it('surfaces http(s) sources with hostname titles, deduped in first-seen order', () => {
+    const out = extractAgentSources([
+      source('a', 'https://example.com/docs'),
+      source('b', 'http://blog.example.org/post'),
+      source('c', 'https://example.com/docs'), // dup URL — dropped
+    ]);
+    expect(out).toEqual([
+      { id: 'a', title: 'example.com', url: 'https://example.com/docs' },
+      { id: 'b', title: 'blog.example.org', url: 'http://blog.example.org/post' },
+    ]);
+  });
+
+  it('drops non-http(s) URLs so a hostile scheme never reaches an anchor href', () => {
+    const out = extractAgentSources([
+      source('js', 'javascript:alert(1)'),
+      source('data', 'data:text/html,<script>alert(1)</script>'),
+      source('file', 'file:///etc/passwd'),
+      source('ok', 'https://safe.example.com'),
+    ]);
+    expect(out).toEqual([{ id: 'ok', title: 'safe.example.com', url: 'https://safe.example.com' }]);
+  });
+
+  it('ignores entries from non-URL tools', () => {
+    expect(
+      extractAgentSources([
+        entry({ id: 'g', name: 'grep', argsBuffer: JSON.stringify({ url: 'https://x.com' }) }),
+      ])
+    ).toEqual([]);
+  });
+});

app/src/utils/toolTimelineFormatting.ts

@@ -420,7 +420,11 @@ export function extractAgentSources(entries: ToolTimelineEntry[]): AgentSource[]
     const baseName = entry.name.replace(/^subagent:/, '');
     if (!URL_SOURCE_TOOLS.has(baseName)) continue;
     const url = parseToolArgs(entry.argsBuffer)?.url?.trim();
-    if (!url || seen.has(url)) continue;
+    // `url` is the raw tool-call argument the model emitted — it is
+    // prompt-injection-influenceable and not guaranteed to be a real web
+    // address. Only surface http(s) sources as clickable links so a
+    // `javascript:` / `data:` / `file:` value can never reach an `<a href>`.
+    if (!url || seen.has(url) || !isHttpUrl(url)) continue;
     seen.add(url);
     sources.push({ id: entry.id, title: hostnameFromUrl(url) ?? url, url });
   }
@@ -443,6 +447,16 @@ function hostnameFromUrl(url: string): string | undefined {
   }
 }
 
+/** True only for well-formed http(s) URLs — the schemes safe to link out to. */
+function isHttpUrl(url: string): boolean {
+  try {
+    const { protocol } = new URL(url);
+    return protocol === 'http:' || protocol === 'https:';
+  } catch {
+    return false;
+  }
+}
+
 function shortenPath(filePath: string): string {
   const parts = filePath.split('/');
   if (parts.length <= 3) return filePath;

src/openhuman/security/policy/policy_command.rs

@@ -779,12 +779,29 @@ pub(super) fn classify_segment(base: &str, args: &[String], joined: &str) -> Com
     if is_command_executor(base) {
         return CommandClass::Write;
     }
-    // `find` is read-only unless it executes commands or deletes files.
+    // `find` is read-only unless it executes commands, deletes, or writes
+    // files. -fprintf / -fprint / -fprint0 / -fls write their output to a
+    // named file rather than stdout — an arbitrary-path write that side-steps
+    // the gated file-write tools (and their workspace confinement), so they
+    // must be classified Write (approval-gated) alongside -delete.
+    //
+    // `args` is built with `split_whitespace()`, which keeps shell quotes that
+    // the shell itself strips before `find` runs — `find . '-fprint' out`
+    // arrives as the literal `'-fprint'`. Trim surrounding single/double quotes
+    // before matching so a quoted predicate cannot slip the write past the gate.
     if base == "find" {
         if args.iter().any(|a| {
             matches!(
-                a.as_str(),
-                "-exec" | "-execdir" | "-ok" | "-okdir" | "-delete"
+                a.trim_matches(|c| c == '\'' || c == '"'),
+                "-exec"
+                    | "-execdir"
+                    | "-ok"
+                    | "-okdir"
+                    | "-delete"
+                    | "-fprintf"
+                    | "-fprint"
+                    | "-fprint0"
+                    | "-fls"
             )
         }) {
             return CommandClass::Write;

src/openhuman/security/policy/policy_tests.rs

@@ -398,6 +398,37 @@ fn classify_writes_are_write() {
     }
 }
 
+#[test]
+fn classify_find_file_write_actions_are_write() {
+    let p = default_policy();
+    // -fprintf / -fprint / -fprint0 / -fls write find's output to a named
+    // file (an arbitrary-path write side-channel), so they must be gated as
+    // Write rather than slipping through as a read-only search.
+    for c in [
+        "find . -maxdepth 0 -fprintf /tmp/out.txt '%p\\n'",
+        "find . -name '*.rs' -fprint /tmp/list.txt",
+        "find . -fprint0 /tmp/list0.txt",
+        "find . -fls /tmp/ls.txt",
+        "find . -delete",
+    ] {
+        assert_eq!(p.classify_command(c), CommandClass::Write, "{c}");
+    }
+    // Quoting the predicate must not slip the write past the gate — the shell
+    // strips the quotes before find runs, so `'-fprint'` is the same write.
+    for c in [
+        "find . '-fprint' /tmp/list.txt",
+        "find . \"-fprintf\" /tmp/out.txt '%p\\n'",
+        "find . '-delete'",
+    ] {
+        assert_eq!(p.classify_command(c), CommandClass::Write, "{c}");
+    }
+    // A plain search stays read-only.
+    assert_eq!(
+        p.classify_command("find . -name '*.rs' -print"),
+        CommandClass::Read
+    );
+}
+
 #[test]
 fn classify_network_is_network() {
     let p = default_policy();