Skip to content

Commit 3455d44

Browse files
committed
fix(integrations): correct claims that overstated or invented provider behavior
- restore firecrawl's conditional country default; the server applies 'us' only when location is unset, and flattening it to the OpenAPI's bare default was the regression - declare firecrawl's six undeclared subBlocks in inputs - agentmail's justification claimed no trash exists; it does. The operative point stands: the endpoint takes no deletion-mode parameter - drop apify's synthesized status; the sync 201 declares no status field, matching the runId removal this branch already made - daytona documents no timeout default, and reserves 'combined output' for session commands, not these two - elasticsearch's scheme guard is belt-and-braces: the transport rejects a scheme-less external URL before it resolves. Both the TSDoc and the user-facing error said otherwise.
1 parent 177bcd4 commit 3455d44

12 files changed

Lines changed: 102 additions & 49 deletions

File tree

apps/sim/blocks/blocks/agentmail.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,14 @@ export const AgentMailBlock: BlockConfig = {
336336

337337
// Delete Thread fields
338338
/**
339-
* AgentMail's DELETE thread endpoint takes no deletion-mode parameter and its own
340-
* description reads "Permanently deletes a thread and all of its messages." There is no
341-
* trash to move a thread to, so this id — which used to render a dropdown defaulting to
342-
* "No (move to trash)" — is now a read-only notice. Keeping the id claimed means the
339+
* AgentMail's DELETE thread endpoint declares no deletion-mode parameter — its only
340+
* documented inputs are the path ids and the `Authorization` header — and its own
341+
* description reads "Permanently deletes a thread and all of its messages." So the choice
342+
* this id used to render — a dropdown defaulting to "No (move to trash)" — could not
343+
* change the request Sim sends, and named an outcome the endpoint does not document. It is
344+
* now a read-only notice. (AgentMail does have a trash label — `include_trash` filters
345+
* list results, `label_trash_read` is an API-key scope — but nothing in the delete
346+
* endpoint's contract routes a thread there.) Keeping the id claimed means the
343347
* warning appears exactly where the misleading choice used to sit, and no future subBlock
344348
* inherits the orphaned 'true'/'false' value still stored in existing workflows.
345349
*/

apps/sim/blocks/blocks/daytona.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export const DaytonaBlock: BlockConfig = {
285285
id: 'timeout',
286286
title: 'Timeout (seconds)',
287287
type: 'short-input',
288-
placeholder: 'Timeout in seconds (defaults to 10)',
288+
placeholder: 'Timeout in seconds (0 disables the server-side limit)',
289289
mode: 'advanced',
290290
condition: { field: 'operation', value: ['execute_command', 'run_code'] },
291291
},
@@ -600,7 +600,7 @@ export const DaytonaBlock: BlockConfig = {
600600
},
601601
result: {
602602
type: 'string',
603-
description: 'Combined stdout/stderr output (execute command and run code operations)',
603+
description: 'Standard output (execute command and run code operations)',
604604
},
605605
artifacts: { type: 'json', description: 'Run artifacts such as charts (run code operation)' },
606606
uploadedPath: {

apps/sim/blocks/blocks/firecrawl.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,13 @@ Example 2 - Product Data:
833833
urls: { type: 'json', description: 'Array of URLs for extraction or batch scraping' },
834834
jobId: { type: 'string', description: 'Job ID for status/cancel operations' },
835835
query: { type: 'string', description: 'Search query terms' },
836+
sources: { type: 'json', description: 'Search result sources (web, news, images)' },
837+
categories: {
838+
type: 'json',
839+
description: 'Search result categories to restrict web results to',
840+
},
841+
location: { type: 'string', description: 'Location to search from' },
842+
country: { type: 'string', description: 'ISO country code for geo-targeting search results' },
836843
prompt: { type: 'string', description: 'Extraction prompt' },
837844
limit: { type: 'string', description: 'Result/page limit' },
838845
formats: { type: 'json', description: 'Output formats array' },
@@ -876,6 +883,8 @@ Example 2 - Product Data:
876883
removeBase64Images: { type: 'boolean', description: 'Remove base64 images, keep alt text' },
877884
blockAds: { type: 'boolean', description: 'Block ads and popups during parsing' },
878885
proxy: { type: 'string', description: 'Proxy mode (basic or auto)' },
886+
maxConcurrency: { type: 'string', description: 'Maximum number of concurrent scrapes' },
887+
ignoreInvalidURLs: { type: 'boolean', description: 'Skip URLs that are invalid for Firecrawl' },
879888
zeroDataRetention: { type: 'boolean', description: 'Enable zero data retention' },
880889
},
881890
outputs: {

apps/sim/tools/apify/apify.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ describe('apify run_actor_sync response contract', () => {
125125
expect(apifyRunActorSyncTool.outputs?.runId).toBeUndefined()
126126
})
127127

128+
it('emits no fabricated run status — the sync body carries no status field', async () => {
129+
for (const tool of [apifyRunActorSyncTool, apifyRunTaskTool]) {
130+
const result = await tool.transformResponse!(jsonResponse([{ a: 1 }]))
131+
expect(Object.hasOwn(result.output, 'status')).toBe(false)
132+
expect(tool.outputs?.status).toBeUndefined()
133+
}
134+
})
135+
128136
it('guards a non-array response body', async () => {
129137
const result = await apifyRunActorSyncTool.transformResponse!(jsonResponse({ error: 'nope' }))
130138
expect(result.output.items).toEqual([])

apps/sim/tools/apify/run_actor_sync.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export const apifyRunActorSyncTool: ToolConfig<RunActorParams, RunActorResult> =
9292
const errorText = await response.text()
9393
return {
9494
success: false,
95-
output: { success: false, status: 'ERROR', items: [] },
95+
output: { success: false, items: [] },
9696
error: `APIFY API error: ${errorText}`,
9797
}
9898
}
@@ -102,21 +102,25 @@ export const apifyRunActorSyncTool: ToolConfig<RunActorParams, RunActorResult> =
102102
success: true,
103103
output: {
104104
success: true,
105-
status: 'SUCCEEDED',
106105
items: Array.isArray(items) ? items : [],
107106
},
108107
}
109108
},
110109

111110
/**
112-
* The sync endpoint answers with a bare dataset-items array and documents no run
113-
* identifier — neither in the body nor in a response header (only the
114-
* `X-Apify-Pagination-*` family is returned) — so no `runId` is emitted. Wiring a
115-
* fabricated id into `apify_get_run` would 404 every time.
111+
* The sync endpoint answers with a bare dataset-items array and documents neither a run
112+
* identifier nor a run status — not in the body, and not in a response header (only the
113+
* `X-Apify-Pagination-*` family is returned). So neither `runId` nor `status` is emitted:
114+
* a fabricated id wired into `apify_get_run` would 404 every time, and a hardcoded
115+
* `'SUCCEEDED'` would claim a terminal state nothing in the response reports. Whether a
116+
* failed run can still answer 201 is unknown — the spec's 201 carries an empty description
117+
* — so there is no honest replacement value to substitute.
116118
*/
117119
outputs: {
118-
success: { type: 'boolean', description: 'Whether the actor run succeeded' },
119-
status: { type: 'string', description: 'Run status (SUCCEEDED, FAILED, etc.)' },
120+
success: {
121+
type: 'boolean',
122+
description: "Whether the request returned dataset items (not the run's own terminal status)",
123+
},
120124
items: { type: 'array', description: 'Dataset items produced by the run' },
121125
},
122126
}

apps/sim/tools/apify/run_task.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export const apifyRunTaskTool: ToolConfig<RunTaskParams, RunTaskResult> = {
100100
const errorText = await response.text()
101101
return {
102102
success: false,
103-
output: { success: false, status: 'ERROR', items: [] },
103+
output: { success: false, items: [] },
104104
error: `APIFY API error: ${errorText}`,
105105
}
106106
}
@@ -110,15 +110,20 @@ export const apifyRunTaskTool: ToolConfig<RunTaskParams, RunTaskResult> = {
110110
success: true,
111111
output: {
112112
success: true,
113-
status: 'SUCCEEDED',
114113
items: Array.isArray(items) ? items : [],
115114
},
116115
}
117116
},
118117

118+
/**
119+
* Same contract as `apify_run_actor_sync`: the task sync endpoint returns dataset items
120+
* with no run identifier and no run status, so neither is synthesized here.
121+
*/
119122
outputs: {
120-
success: { type: 'boolean', description: 'Whether the task run succeeded' },
121-
status: { type: 'string', description: 'Run status (SUCCEEDED, FAILED, etc.)' },
123+
success: {
124+
type: 'boolean',
125+
description: "Whether the request returned dataset items (not the run's own terminal status)",
126+
},
122127
items: { type: 'array', description: 'Dataset items produced by the run' },
123128
},
124129
}

apps/sim/tools/apify/types.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import type { ToolResponse } from '@/tools/types'
55
* endpoints.
66
*
77
* Apify documents that "If the Actor run exceeds 300 seconds, the HTTP response
8-
* will return the 408 status code (Request Timeout)" on both the actor and the
9-
* actor-task variant. Sim's own transport falls back to `options.timeout || 300000`
8+
* will return the 408 status code (Request Timeout)" on the actor variant. The
9+
* actor-task variant's prose is weaker — "The run must finish in 300 seconds
10+
* otherwise the HTTP request fails with a timeout error" — and only its
11+
* response-code list names 408. Sim's own transport falls back to `options.timeout || 300000`
1012
* in `lib/core/security/input-validation.server.ts` — the same number — so a run
1113
* that lands on the boundary is a race between Apify's structured 408 and a
1214
* generic `Request timed out after 300000ms` that names neither the actor nor the
@@ -59,7 +61,8 @@ export interface RunActorResult extends ToolResponse {
5961
success: boolean
6062
/** Absent for the sync endpoint, whose response carries no run identifier. */
6163
runId?: string
62-
status: string
64+
/** Absent for the sync endpoint, whose response carries no run status. */
65+
status?: string
6366
datasetId?: string
6467
items?: unknown[]
6568
}
@@ -78,7 +81,6 @@ export interface RunTaskParams {
7881
export interface RunTaskResult extends ToolResponse {
7982
output: {
8083
success: boolean
81-
status: string
8284
items: unknown[]
8385
}
8486
}

apps/sim/tools/daytona/execute_command.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ export const daytonaExecuteCommandTool: ToolConfig<
5050
type: 'number',
5151
required: false,
5252
visibility: 'user-or-llm',
53-
description: 'Timeout in seconds (defaults to 10 seconds)',
53+
description:
54+
'Maximum time in seconds to wait for the command to complete; 0 disables the server-side limit. Daytona documents no default, so leaving this blank uses whatever the toolbox daemon applies.',
5455
},
5556
},
5657

@@ -93,6 +94,6 @@ export const daytonaExecuteCommandTool: ToolConfig<
9394
type: 'number',
9495
description: 'Exit code of the command (-1 if missing from the response)',
9596
},
96-
result: { type: 'string', description: 'Combined stdout/stderr output of the command' },
97+
result: { type: 'string', description: 'Standard output from the command' },
9798
},
9899
}

apps/sim/tools/daytona/run_code.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export const daytonaRunCodeTool: ToolConfig<DaytonaRunCodeParams, DaytonaRunCode
4444
type: 'number',
4545
required: false,
4646
visibility: 'user-or-llm',
47-
description: 'Timeout in seconds (defaults to 10 seconds)',
47+
description:
48+
'Maximum time in seconds to wait for the code run to complete; 0 disables the server-side limit. Daytona documents no default, so leaving this blank uses whatever the toolbox daemon applies.',
4849
},
4950
},
5051

@@ -88,7 +89,10 @@ export const daytonaRunCodeTool: ToolConfig<DaytonaRunCodeParams, DaytonaRunCode
8889
type: 'number',
8990
description: 'Exit code of the code run (-1 if missing from the response)',
9091
},
91-
result: { type: 'string', description: 'Combined stdout/stderr output of the code run' },
92+
result: {
93+
type: 'string',
94+
description: 'Standard output from the code (same as artifacts.stdout)',
95+
},
9296
artifacts: {
9397
type: 'json',
9498
description: 'Artifacts produced by the run (e.g., matplotlib charts)',

apps/sim/tools/elasticsearch/utils.ts

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -299,27 +299,27 @@ const SELF_HOSTED_SCHEME_PATTERN = /^https?:\/\//i
299299
/**
300300
* Normalizes and validates a self-hosted host into an absolute origin.
301301
*
302-
* The scheme requirement is a credential-exposure control, not tidiness. A
303-
* host without one stays a **relative** URL, and the executor resolves every
304-
* tool URL as `new URL(endpointUrl, getBaseUrl())` — with Sim's own origin as
305-
* the base:
306-
*
307-
* ```
308-
* new URL('es.internal/products/_search', 'https://sim.ai')
309-
* // => https://sim.ai/es.internal/products/_search
310-
* new URL('//evil.example.com/x', 'https://sim.ai')
311-
* // => https://evil.example.com/x (protocol-relative, inherits the scheme)
312-
* new URL('localhost:9200/products/_search', 'https://sim.ai')
313-
* // => protocol "localhost:", origin null
314-
* ```
315-
*
316-
* {@link buildAuthHeaders} attaches `Authorization: ApiKey …` or `Basic …`
317-
* regardless, so the caller's Elasticsearch credential is sent to Sim's public
318-
* origin — or, for the protocol-relative form, to an attacker's. The SSRF
319-
* guard does not catch it either, because the resolved host really is Sim.
320-
* Only a scheme makes the value absolute, so it is required rather than
321-
* guessed at: prefixing `https://` ourselves would silently redirect a
322-
* plaintext-only cluster and turn a typo into a different host.
302+
* The scheme requirement is belt-and-braces, not the load-bearing control. A
303+
* scheme-less host would be a **relative** URL, and the executor does resolve
304+
* tool URLs against Sim's own origin (`new URL(endpointUrl, baseUrl)` in
305+
* `tools/index.ts`) — which would send the credential {@link buildAuthHeaders}
306+
* attaches to `https://sim.ai/es.internal/...`, or, for a protocol-relative
307+
* `//evil.example.com/...`, to an attacker. That resolution is real but
308+
* unreachable from here: `assertRequestUrlMatchesTrust` in
309+
* `tools/request-transport.ts` runs first, parses every external tool URL with
310+
* `new URL(url)` and **no base**, and pins the protocol to `http:`/`https:`.
311+
* Both scheme-less forms throw on that parse, and `localhost:9200/...` parses
312+
* but fails the protocol pin.
313+
*
314+
* What this check buys is the error the user sees. It fires at configuration
315+
* time, names the field, and says what to type, instead of letting the request
316+
* die later against the transport's generic "invalid external URL" rejection.
317+
* Keep it for the message, and do not weaken the transport guard on the
318+
* assumption that this one is holding the line — it is the other way round.
319+
*
320+
* The scheme is required rather than guessed at: prefixing `https://`
321+
* ourselves would silently redirect a plaintext-only cluster and turn a typo
322+
* into a different host.
323323
*
324324
* The parse that follows catches a scheme-ful value the URL parser still
325325
* cannot resolve (`https://`, `http://[`). The original string — not the
@@ -341,7 +341,7 @@ function normalizeSelfHostedHost(rawHost: unknown): string {
341341
if (!SELF_HOSTED_SCHEME_PATTERN.test(host)) {
342342
throw new Error(
343343
`Host must start with "http://" or "https://" (received "${host}"). ` +
344-
'A host without a scheme is a relative URL and would send your Elasticsearch credential to Sim instead of your cluster.'
344+
'A host without a scheme is not an absolute URL, so the request cannot be addressed to your cluster.'
345345
)
346346
}
347347

0 commit comments

Comments
 (0)