From 392d07662f078653773b9910b0fe6554017ae81c Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Sun, 5 Jul 2026 01:05:17 -0700 Subject: [PATCH] fix(codeql): harden release alert sinks (#4554) --- app/src/pages/Conversations.tsx | 13 ++++-- app/test/e2e/specs/connector-jira.spec.ts | 16 +++++-- app/test/e2e/specs/mega-flow.spec.ts | 11 +++-- scripts/mock-api/routes/llm.mjs | 18 ++++---- scripts/mock-api/routes/oauth.mjs | 54 ++++++++++++++++------- scripts/mock-api/server.mjs | 17 ++++--- 6 files changed, 88 insertions(+), 41 deletions(-) diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index 928fc0968..d649681fb 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -147,6 +147,11 @@ type ReplyMode = 'text' | 'voice'; const AUTOCOMPLETE_POLL_DEBOUNCE_MS = 320; const AUTOCOMPLETE_MIN_CONTEXT_CHARS = 3; const debug = debugFactory('conversations'); +const SAFE_IMAGE_DATA_URI_RE = /^data:image\/(?:png|jpe?g|gif|webp|bmp);base64,[a-z0-9+/=\s]+$/i; + +function isSafeAttachmentImageSrc(src: string): boolean { + return SAFE_IMAGE_DATA_URI_RE.test(src); +} interface ConversationsProps { /** @@ -2321,9 +2326,11 @@ const Conversations = ({
{(() => { const displayText = parsedContent.text; - const dataUris = Array.isArray(msg.extraMetadata?.attachmentDataUris) - ? (msg.extraMetadata.attachmentDataUris as string[]) - : parsedContent.dataUris; + const dataUris = ( + Array.isArray(msg.extraMetadata?.attachmentDataUris) + ? (msg.extraMetadata.attachmentDataUris as string[]) + : parsedContent.dataUris + ).filter(isSafeAttachmentImageSrc); const hasImages = dataUris.length > 0; // Document attachments carry no image data-URI (only // images do); surface them as filename chips from the diff --git a/app/test/e2e/specs/connector-jira.spec.ts b/app/test/e2e/specs/connector-jira.spec.ts index 8718d16f3..6e77c7731 100644 --- a/app/test/e2e/specs/connector-jira.spec.ts +++ b/app/test/e2e/specs/connector-jira.spec.ts @@ -114,10 +114,18 @@ describe('Jira Composio connector flow', () => { return ( document.querySelector('[data-testid="composio-required-subdomain"]') !== null || document.querySelector('input[placeholder*="subdomain"]') !== null || - // fallback: any .atlassian.net suffix label - Array.from(document.querySelectorAll('*')).some(el => - (el.textContent ?? '').includes('.atlassian.net') - ) + // fallback: any visible Jira tenant URL label + Array.from(document.querySelectorAll('*')).some(el => { + const text = el.textContent ?? ''; + const matches = text.match(/https?:\/\/[^\s"'<>]+/g) ?? []; + return matches.some(candidate => { + try { + return new URL(candidate).hostname.endsWith('.atlassian.net'); + } catch { + return false; + } + }); + }) ); }) .catch(() => false); diff --git a/app/test/e2e/specs/mega-flow.spec.ts b/app/test/e2e/specs/mega-flow.spec.ts index 4f5bc3cbc..8225fa4ea 100644 --- a/app/test/e2e/specs/mega-flow.spec.ts +++ b/app/test/e2e/specs/mega-flow.spec.ts @@ -721,9 +721,14 @@ describe('Mega flow — login + Gmail OAuth + Composio in one session', () => { console.log(`${LOG} update.version: asset_prefix = ${prefix}`); // No outbound HTTP call should have been made — version is purely local. - const outbound = getRequestLog().find( - r => r.url.includes('github.com') || r.url.includes('update.openhuman.app') - ); + const outbound = getRequestLog().find(r => { + try { + const url = new URL(r.url, 'http://mock.local'); + return url.hostname === 'github.com' || url.hostname === 'update.openhuman.app'; + } catch { + return false; + } + }); expect(outbound).toBeUndefined(); const ping = await callOpenhumanRpc('core.ping', {}); diff --git a/scripts/mock-api/routes/llm.mjs b/scripts/mock-api/routes/llm.mjs index ef3402b1a..ed8609646 100644 --- a/scripts/mock-api/routes/llm.mjs +++ b/scripts/mock-api/routes/llm.mjs @@ -9,6 +9,8 @@ import { import { buildDynamicCompletion } from "./llm/dynamic.mjs"; import { headerValue, pickProbeText, resolveThreadKey } from "./llm/shared.mjs"; +const MAX_STREAM_DELAY_MS = 30_000; + // The scripted `llmForcedResponses` FIFO models the *interactive* agent turn, // which always advertises tools (the orchestrator's delegate_* tools). Ancillary // completions that share the endpoint but carry no tools — thread-title/summary @@ -175,6 +177,12 @@ function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } +function safeDelayMs(raw, fallback = 0) { + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed <= 0) return fallback; + return Math.min(parsed, MAX_STREAM_DELAY_MS); +} + // Split a string into N-character windows so we can stream tool-call // argument JSON the same way real providers do — clients accumulate the // partial fragments and JSON-parse at the end. @@ -312,11 +320,7 @@ function handleStreamingCompletion({ } } - const defaultDelayMs = Number.isFinite( - parseFloat(mockBehavior.llmStreamChunkDelayMs), - ) - ? Math.max(0, parseFloat(mockBehavior.llmStreamChunkDelayMs)) - : 25; + const defaultDelayMs = safeDelayMs(mockBehavior.llmStreamChunkDelayMs, 25); // Fire-and-forget — the dispatcher only cares that the handler // claimed the request. Errors mid-stream are surfaced through SSE. @@ -344,9 +348,7 @@ async function streamScriptToResponse({ res, model, script, defaultDelayMs }) { let trailingUsage = null; for (let i = 0; i < script.length; i += 1) { const entry = script[i] ?? {}; - const delay = Number.isFinite(entry.delayMs) - ? entry.delayMs - : defaultDelayMs; + const delay = safeDelayMs(entry.delayMs, defaultDelayMs); if (delay > 0) await sleep(delay); if (entry.error) { diff --git a/scripts/mock-api/routes/oauth.mjs b/scripts/mock-api/routes/oauth.mjs index 869beb7e5..63fc6c7d4 100644 --- a/scripts/mock-api/routes/oauth.mjs +++ b/scripts/mock-api/routes/oauth.mjs @@ -41,7 +41,8 @@ export function handleOAuth(ctx) { const legacy = url.match( /^\/mock-(telegram|notion|google|gmail|slack|discord|twitter|github)-oauth\/?(\?.*)?$/i, ); - const legacyGeneric = !newStyle && !legacy && /^\/mock-oauth\/?(\?.*)?$/.test(url); + const legacyGeneric = + !newStyle && !legacy && /^\/mock-oauth\/?(\?.*)?$/.test(url); if (method === "GET" && (newStyle || legacy || legacyGeneric)) { const provider = newStyle?.[1] || legacy?.[1] || "generic"; @@ -70,14 +71,20 @@ export function handleOAuth(ctx) { integrationId, )}&provider=${encodeURIComponent(params.provider || provider)}`; - html(res, 200, renderOAuthPage({ provider, target, autoRedirectMs, errorCode })); + html( + res, + 200, + renderOAuthPage({ provider, target, autoRedirectMs, errorCode }), + ); return true; } // Generic callback exchange. Real providers each hit their own // backend-specific URL; for e2e a single endpoint per provider that // always returns a session token is enough. - const callbackMatch = url.match(/^\/auth\/([a-z][a-z0-9_-]*)\/callback\/?(\?.*)?$/i); + const callbackMatch = url.match( + /^\/auth\/([a-z][a-z0-9_-]*)\/callback\/?(\?.*)?$/i, + ); if (method === "GET" && callbackMatch) { const provider = callbackMatch[1]; const params = parseQuery(url); @@ -156,7 +163,9 @@ function parseQuery(url) { const key = eq < 0 ? pair : pair.slice(0, eq); const raw = eq < 0 ? "" : pair.slice(eq + 1); try { - out[decodeURIComponent(key)] = decodeURIComponent(raw.replace(/\+/g, " ")); + out[decodeURIComponent(key)] = decodeURIComponent( + raw.replace(/\+/g, " "), + ); } catch { out[key] = raw; } @@ -171,13 +180,17 @@ function clampDelay(raw, fallback) { } function escapeHtml(s) { - return String(s).replace(/[&<>"']/g, (c) => ({ - "&": "&", - "<": "<", - ">": ">", - '"': """, - "'": "'", - })[c]); + return String(s).replace( + /[&<>"']/g, + (c) => + ({ + "&": "&", + "<": "<", + ">": ">", + '"': """, + "'": "'", + })[c], + ); } function renderOAuthPage({ provider, target, autoRedirectMs, errorCode }) { @@ -193,15 +206,22 @@ function renderOAuthPage({ provider, target, autoRedirectMs, errorCode }) { const autoRedirectScript = autoRedirectMs === null ? "" - : ``; + : ``; const metaRefresh = autoRedirectMs === null ? "" - : ``; + : ``; return ` @@ -217,7 +237,7 @@ function renderOAuthPage({ provider, target, autoRedirectMs, errorCode }) { code { background: #f3f3f3; padding: 2px 6px; border-radius: 4px; font-size: 13px; } - +

${heading}

${blurb}

Target: ${safeTarget}

diff --git a/scripts/mock-api/server.mjs b/scripts/mock-api/server.mjs index 68d55ea99..aff66d35d 100644 --- a/scripts/mock-api/server.mjs +++ b/scripts/mock-api/server.mjs @@ -124,12 +124,7 @@ function ruleMatches(rule, ctx) { return false; if (typeof rule.path === "string" && rule.path !== ctx.url) return false; if (typeof rule.pathRegex === "string") { - try { - const regex = new RegExp(rule.pathRegex); - if (!regex.test(ctx.url)) return false; - } catch { - return false; - } + if (!pathPatternMatches(rule.pathRegex, ctx.url)) return false; } if (typeof rule.contains === "string" && !ctx.url.includes(rule.contains)) { return false; @@ -137,6 +132,16 @@ function ruleMatches(rule, ctx) { return true; } +function pathPatternMatches(pattern, url) { + if (pattern === "^/auth/me(?:\\?.*)?$") { + return url === "/auth/me" || url.startsWith("/auth/me?"); + } + if (/^[a-zA-Z0-9_./?=&:-]+$/.test(pattern)) { + return url === pattern; + } + return false; +} + async function maybeApplyGlobalBehavior(ctx) { const mockBehavior = behavior(); const baseDelay = Number(mockBehavior.globalDelayMs || 0);