fix(ssh): make execution timeout configurable and remove internal 300s cutoff

README.md

@@ -172,6 +172,32 @@ Key environment variables for self-hosted deployments. See [`.env.example`](apps
 | `API_ENCRYPTION_KEY` | Yes | Encrypts API keys (`openssl rand -hex 32`) |
 | `COPILOT_API_KEY` | No | API key from sim.ai for Copilot features |
 
+### SSH Block Timeout (10 minutes)
+
+For SSH blocks, there are two timeout layers to keep aligned:
+
+1. Workflow sync execution timeout (`EXECUTION_TIMEOUT_FREE`) controls the max duration in seconds.
+2. Internal tool HTTP calls must respect per-tool timeout values (the SSH timeout path in `apps/sim/tools/index.ts`).
+
+Local Docker Compose (default remains 5 minutes):
+
+```bash
+# in docker-compose.prod.yml
+EXECUTION_TIMEOUT_FREE=${EXECUTION_TIMEOUT_FREE:-300}
+```
+
+To run SSH up to 10 minutes:
+
+```bash
+EXECUTION_TIMEOUT_FREE=600
+```
+
+Verification inside the running app container:
+
+```bash
+docker compose -f docker-compose.prod.yml exec -T simstudio sh -lc 'printenv | grep ^EXECUTION_TIMEOUT_FREE='
+```
+
 ## Tech Stack
 
 - **Framework**: [Next.js](https://nextjs.org/) (App Router)

apps/sim/tools/index.test.ts

@@ -14,6 +14,8 @@ import {
   type MockFetchResponse,
 } from '@sim/testing'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { DEFAULT_EXECUTION_TIMEOUT_MS } from '@/lib/core/execution-limits'
+import * as securityValidation from '@/lib/core/security/input-validation.server'
 
 // Hoisted mock state - these are available to vi.mock factories
 const { mockIsHosted, mockEnv, mockGetBYOKKey, mockLogFixedUsage, mockRateLimiterFns } = vi.hoisted(
@@ -280,6 +282,109 @@ function setupEnvVars(variables: Record<string, string>) {
   }
 }
 
+function responseHeadersToRecord(headers: unknown): Record<string, string> {
+  const record: Record<string, string> = {}
+  if (!headers || typeof headers !== 'object') return record
+
+  if (headers instanceof Headers) {
+    headers.forEach((value, key) => {
+      record[key] = value
+    })
+    return record
+  }
+
+  if ('forEach' in headers && typeof (headers as any).forEach === 'function') {
+    ;(headers as any).forEach((value: string, key: string) => {
+      record[key] = value
+    })
+    return record
+  }
+
+  for (const [key, value] of Object.entries(headers as Record<string, unknown>)) {
+    if (typeof value === 'string') record[key] = value
+  }
+  return record
+}
+
+function setupSecurityFetchMocks() {
+  vi.spyOn(securityValidation, 'validateUrlWithDNS').mockResolvedValue({
+    isValid: true,
+    resolvedIP: '127.0.0.1',
+    originalHostname: 'localhost',
+  })
+
+  vi.spyOn(securityValidation, 'secureFetchWithPinnedIP').mockImplementation(
+    async (url, _resolvedIP, options = {}) => {
+      let fetchResponse: any
+      try {
+        fetchResponse = await global.fetch(url, {
+          method: options.method,
+          headers: options.headers as HeadersInit,
+          body: options.body as BodyInit | null | undefined,
+        })
+      } catch (error) {
+        // Keep parity with secure fetch timeout behavior expected by retry logic tests.
+        if (error instanceof Error && error.name === 'AbortError') {
+          throw new Error('Request timed out')
+        }
+        throw error
+      }
+
+      if (!fetchResponse) {
+        throw new Error('Mock fetch returned no response')
+      }
+
+      const headersRecord = responseHeadersToRecord(fetchResponse.headers)
+      const status = fetchResponse.status ?? 200
+      const statusText = (fetchResponse as any).statusText ?? (status >= 200 ? 'OK' : 'Error')
+      const ok = fetchResponse.ok ?? (status >= 200 && status < 300)
+      let cachedBodyText: string | null = null
+
+      const getBodyText = async (): Promise<string> => {
+        if (cachedBodyText !== null) return cachedBodyText
+
+        if (typeof fetchResponse.text === 'function') {
+          cachedBodyText = await fetchResponse.text()
+          return cachedBodyText
+        }
+
+        if (typeof fetchResponse.json === 'function') {
+          const jsonData = await fetchResponse.json()
+          cachedBodyText = typeof jsonData === 'string' ? jsonData : JSON.stringify(jsonData)
+          return cachedBodyText
+        }
+
+        if (typeof fetchResponse.arrayBuffer === 'function') {
+          const arr = await fetchResponse.arrayBuffer()
+          cachedBodyText = new TextDecoder().decode(arr)
+          return cachedBodyText
+        }
+
+        cachedBodyText = ''
+        return cachedBodyText
+      }
+
+      return {
+        ok,
+        status,
+        statusText,
+        headers: new securityValidation.SecureFetchHeaders(headersRecord),
+        text: async () => {
+          return getBodyText()
+        },
+        json: async () => {
+          const rawText = await getBodyText()
+          return rawText ? JSON.parse(rawText) : {}
+        },
+        arrayBuffer: async () => {
+          const rawText = await getBodyText()
+          return new TextEncoder().encode(rawText).buffer
+        },
+      }
+    }
+  )
+}
+
 describe('Tools Registry', () => {
   it('should include all expected built-in tools', () => {
     expect(tools.http_request).toBeDefined()
@@ -334,6 +439,7 @@ describe('executeTool Function', () => {
       status: 200,
       headers: { 'content-type': 'application/json' },
     })
+    setupSecurityFetchMocks()
 
     process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
     cleanupEnvVars = setupEnvVars({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000' })
@@ -437,10 +543,194 @@ describe('executeTool Function', () => {
   })
 })
 
+describe('Internal Tool Timeout Behavior', () => {
+  let cleanupEnvVars: () => void
+
+  beforeEach(() => {
+    setupSecurityFetchMocks()
+    process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
+    cleanupEnvVars = setupEnvVars({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000' })
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+    cleanupEnvVars()
+  })
+
+  it('should pass explicit timeout to secureFetchWithPinnedIP for internal routes', async () => {
+    const expectedTimeout = 600000
+
+    const secureFetchSpy = vi
+      .spyOn(securityValidation, 'secureFetchWithPinnedIP')
+      .mockResolvedValue({
+        ok: true,
+        status: 200,
+        statusText: 'OK',
+        headers: new securityValidation.SecureFetchHeaders({ 'content-type': 'application/json' }),
+        text: async () => JSON.stringify({ success: true }),
+        json: async () => ({ success: true }),
+        arrayBuffer: async () => new TextEncoder().encode(JSON.stringify({ success: true })).buffer,
+      })
+
+    const originalFunctionTool = { ...tools.function_execute }
+    tools.function_execute = {
+      ...tools.function_execute,
+      transformResponse: vi.fn().mockResolvedValue({
+        success: true,
+        output: { result: 'executed' },
+      }),
+    }
+
+    try {
+      const result = await executeTool(
+        'function_execute',
+        {
+          code: 'return 1',
+          timeout: expectedTimeout,
+        },
+        true
+      )
+
+      expect(result.success).toBe(true)
+      expect(secureFetchSpy).toHaveBeenCalled()
+      expect(secureFetchSpy).toHaveBeenCalledWith(
+        expect.stringContaining('/api/function/execute'),
+        '127.0.0.1',
+        expect.objectContaining({
+          timeout: expectedTimeout,
+        })
+      )
+    } finally {
+      tools.function_execute = originalFunctionTool
+    }
+  })
+
+  it('should use DEFAULT_EXECUTION_TIMEOUT_MS when timeout is not provided', async () => {
+    const secureFetchSpy = vi
+      .spyOn(securityValidation, 'secureFetchWithPinnedIP')
+      .mockResolvedValue({
+        ok: true,
+        status: 200,
+        statusText: 'OK',
+        headers: new securityValidation.SecureFetchHeaders({ 'content-type': 'application/json' }),
+        text: async () => JSON.stringify({ success: true }),
+        json: async () => ({ success: true }),
+        arrayBuffer: async () => new TextEncoder().encode(JSON.stringify({ success: true })).buffer,
+      })
+
+    const originalFunctionTool = { ...tools.function_execute }
+    tools.function_execute = {
+      ...tools.function_execute,
+      transformResponse: vi.fn().mockResolvedValue({
+        success: true,
+        output: { result: 'executed' },
+      }),
+    }
+
+    try {
+      const result = await executeTool(
+        'function_execute',
+        {
+          code: 'return 1',
+        },
+        true
+      )
+
+      expect(result.success).toBe(true)
+      expect(secureFetchSpy).toHaveBeenCalled()
+      expect(secureFetchSpy).toHaveBeenCalledWith(
+        expect.stringContaining('/api/function/execute'),
+        '127.0.0.1',
+        expect.objectContaining({
+          timeout: DEFAULT_EXECUTION_TIMEOUT_MS,
+        })
+      )
+    } finally {
+      tools.function_execute = originalFunctionTool
+    }
+  })
+
+  it('should preserve plain text error bodies for internal secure fetch responses', async () => {
+    vi.spyOn(securityValidation, 'secureFetchWithPinnedIP').mockResolvedValue({
+      ok: false,
+      status: 401,
+      statusText: 'Unauthorized',
+      headers: new securityValidation.SecureFetchHeaders({ 'content-type': 'text/plain' }),
+      text: async () => 'Invalid access token',
+      json: async () => {
+        throw new Error('Invalid JSON')
+      },
+      arrayBuffer: async () => new TextEncoder().encode('Invalid access token').buffer,
+    })
+
+    const result = await executeTool(
+      'function_execute',
+      {
+        code: 'return 1',
+      },
+      true
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toBe('Invalid access token')
+  })
+
+  it('should not read a body for internal 204 responses', async () => {
+    const arrayBufferSpy = vi.fn(async () => new ArrayBuffer(0))
+    const secureFetchSpy = vi
+      .spyOn(securityValidation, 'secureFetchWithPinnedIP')
+      .mockResolvedValue({
+        ok: true,
+        status: 204,
+        statusText: 'No Content',
+        headers: new securityValidation.SecureFetchHeaders({}),
+        text: async () => '',
+        json: async () => {
+          throw new Error('No response body')
+        },
+        arrayBuffer: arrayBufferSpy,
+      })
+
+    const originalTool = (tools as any).test_internal_no_content
+    ;(tools as any).test_internal_no_content = {
+      id: 'test_internal_no_content',
+      name: 'Test Internal No Content',
+      description: 'A test tool for internal 204 responses',
+      version: '1.0.0',
+      params: {},
+      request: {
+        url: '/api/test/no-content',
+        method: 'POST',
+        headers: () => ({ 'Content-Type': 'application/json' }),
+      },
+    }
+
+    try {
+      const result = await executeTool('test_internal_no_content', {}, true)
+
+      expect(result.success).toBe(true)
+      expect(result.output).toEqual({ status: 204 })
+      expect(secureFetchSpy).toHaveBeenCalledWith(
+        expect.stringContaining('/api/test/no-content'),
+        '127.0.0.1',
+        expect.any(Object)
+      )
+      expect(arrayBufferSpy).not.toHaveBeenCalled()
+    } finally {
+      if (originalTool) {
+        ;(tools as any).test_internal_no_content = originalTool
+      } else {
+        delete (tools as any).test_internal_no_content
+      }
+    }
+  })
+})
+
 describe('Automatic Internal Route Detection', () => {
   let cleanupEnvVars: () => void
 
   beforeEach(() => {
+    setupSecurityFetchMocks()
     process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
     cleanupEnvVars = setupEnvVars({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000' })
   })
@@ -696,6 +986,7 @@ describe('Centralized Error Handling', () => {
   let cleanupEnvVars: () => void
 
   beforeEach(() => {
+    setupSecurityFetchMocks()
     process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
     cleanupEnvVars = setupEnvVars({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000' })
   })
@@ -832,7 +1123,6 @@ describe('Centralized Error Handling', () => {
     )
 
     expect(result.success).toBe(false)
-    // Should extract the text error message, not the JSON parsing error
     expect(result.error).toBe('Invalid access token')
   })
 
@@ -891,8 +1181,7 @@ describe('Centralized Error Handling', () => {
     )
 
     expect(result.success).toBe(false)
-    // Should fall back to HTTP status text when both parsing methods fail
-    expect(result.error).toBe('Internal Server Error')
+    expect(result.error).toBe('Cannot read response')
   })
 
   it('should handle complex nested error objects', async () => {
@@ -925,6 +1214,7 @@ describe('MCP Tool Execution', () => {
   let cleanupEnvVars: () => void
 
   beforeEach(() => {
+    setupSecurityFetchMocks()
     process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
     cleanupEnvVars = setupEnvVars({ NEXT_PUBLIC_APP_URL: 'http://localhost:3000' })
   })