Skip to content

fix(connectors): fetch-and-attach real multipart file parts (#645) - #818

Merged
keysersoft merged 5 commits into
HelpCode-ai:mainfrom
rangarajan19:fix/645-multipart-file-upload
Oct 3, 2026
Merged

keysersoft merged 5 commits into
HelpCode-ai:mainfrom
rangarajan19:fix/645-multipart-file-upload

Conversation

@rangarajan19

Copy link
Copy Markdown
Contributor

Closes #645.

Draft — the download currently uses a plain axios call, isolated in its own fetchFileForUpload function. Per the discussion on #645, I'll swap it to the shared outbound-fetch helper once it's on main, before marking this ready for review.

What this does

  • Adds a __file bodyMapping marker (bodyMapping: { image: { "__file": "$image" } }), following the existing __raw/__spread convention.
  • In the form-data branch, detects that marker and fetches the URL server-side (through the existing assertSafeOutboundUrl SSRF guard, no new surface) instead of stringifying it, then attaches it as a real multipart part.
  • form-urlencoded + __file is a config error — that encoding can't carry a file.
  • A file param type in the custom tool builder, selectable only when target is body and encoding is multipart/form-data. Exposed to the model as { type: 'string', format: 'uri' }, since JSON Schema has no file type.

Following the maintainer's corrections from the issue thread

  • Buffers the download into memory rather than streaming it straight into the multipart part — a FormData built from a live stream can't be read twice, and execute() resends the same axiosConfig on an OAuth2/LOGIN_TOKEN 401 retry. The form is now built via a small factory and rebuilt for that retry, so the file is fetched once but the body can be resent intact.
  • No connector credentials (auth, headers, proxy) on the file download — it's a bare request to a third party unrelated to the connector.
  • Non-2xx from the file URL fails the tool call with a clear message instead of attaching an error page as the file.
  • Filename: Content-Disposition's filename wins when present, otherwise the URL's last path segment; both go through the same sanitizing ([A-Za-z0-9._-], fallback file).
  • Content-Type: trusts the response header, falls back to application/octet-stream.
  • Size: MAX_FILE_UPLOAD_BYTES env var, default 10MB, download aborted as soon as the cap is crossed.
  • 30s timeout on the download; the URL's query string is never logged (presigned URLs carry credentials there).

Tests

Added to rest.engine.spec.ts:

  • fetches the __file URL and attaches it as a real multipart part, with no connector credentials on the download
  • aborts the download once it crosses the size cap, without sending the real request
  • fails the tool call when the file URL answers with a non-2xx status
  • rejects a __file marker when encoding is form-urlencoded
  • refuses to download a __file URL that resolves to a blocked address (SSRF)
  • rebuilds the multipart body with the same file on a 401 retry, fetching the file only once

82/82 tests pass (76 existing + 6 new). Full backend + frontend typecheck clean.

Docs

Short note in docs/tool-definition.md documenting the __file marker.

…-ai#645)

form-data bodies had no way to carry an actual file: appendFormParam
coerced every value to a string, so a tool meant to upload an image
(e.g. Etsy's uploadListingImage) sent the literal URL text as a form
field instead of a real file part.

Add a __file bodyMapping marker (bodyMapping: { image: { __file: "$image" } }),
following the existing __raw/__spread convention. When the form-data
branch sees it, it fetches the URL server-side through the existing
SSRF guard, buffers it (capped by MAX_FILE_UPLOAD_BYTES, default 10MB,
aborted early once crossed), and attaches it as a real multipart part
with filename/content-type. Content-Disposition's filename wins when
present; both it and the URL-derived fallback are sanitized the same
way. Non-2xx responses from the file URL fail the tool call instead of
attaching an error page as the file. The download is a bare request
with none of the connector's own auth/headers/proxy config attached.

form-urlencoded can't carry a file, so the marker is a config error
there instead of being silently stringified.

A form-data body backed by a file Buffer can only be read once; the
OAuth2/LOGIN_TOKEN 401 auto-retry now rebuilds it from a small factory
instead of resending the drained body.

Frontend: a 'file' param type in the custom tool builder, selectable
only when target is body and encoding is multipart/form-data. It
emits the __file marker and is exposed to the model as
{ type: 'string', format: 'uri' }, since JSON Schema has no file type.
Replaces the plain axios call + manual stream-cap in fetchFileForUpload
with fetchOutbound() (HelpCode-ai#821), which already covers the SSRF check on
every redirect hop, the size cap, the timeout and non-2xx handling.
Filename resolution now uses the post-redirect finalUrl rather than the
original URL, so a short link resolves to the real file's name.

Tests mock fetchOutbound directly rather than the raw axios call.
@rangarajan19
rangarajan19 force-pushed the fix/645-multipart-file-upload branch from 30e69e2 to db04693 Compare October 3, 2026 07:21
@rangarajan19
rangarajan19 marked this pull request as ready for review October 3, 2026 07:22
… left out

A $param resolves to the caller's value as-is, objects included, and
__spread lets the caller choose the keys. So a model could make any
form-data tool download a URL of its choosing by sending
{ "__file": "https://..." }. The marker now counts only where the
tool's own bodyMapping writes it; one arriving in the arguments fails the
call. A declared file the caller did not pass is left out of the form.

@keysersoft keysersoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rangarajan19, nice work, and the switch to fetchOutbound is exactly what I hoped for: SSRF checks on every redirect, size cap, no connector credentials on the download, and the rebuilt body on the 401 retry.

One thing I tightened before merging: a $param resolves to the caller's value as is, objects included, and __spread lets the caller choose the keys. So a model could send { "__file": "https://..." } as an ordinary field and make any form-data tool download a URL of its choosing. I pushed a commit that only honours the marker for fields the tool's own bodyMapping declares, fails the call when one arrives in the arguments, and leaves out an optional file the caller didn't pass. Two tests cover it (direct field and __spread).

Also merged main into the branch. Merging once CI is green.

@keysersoft
keysersoft enabled auto-merge (squash) October 3, 2026 16:20
@keysersoft
keysersoft merged commit 33fda82 into HelpCode-ai:main Oct 3, 2026
13 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File upload support (multipart) in custom tool builder

2 participants