mirror of
https://github.com/ComposioHQ/composio.git
synced 2026-09-22 11:46:35 +08:00
docs/auto-update-data
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Alberto Schiabel
|
0d28befb14 |
fix(sdk): map streamed file transport failures (#4321)
## Summary - map Python file-fetch failures that occur after response headers into the documented upload and download errors - map TypeScript RemoteFile connection and streamed-body failures into RemoteFileDownloadError while preserving blocked-URL errors - close or cancel response bodies on every exit and apply the shared 100 MiB response limit to TypeScript RemoteFile downloads This supersedes the Python-only proposal in #4305 and carries the same failure category across both SDKs. ## Independent reproduction A response double returned one chunk and then raised a connection-reset error. On current next: - Python _fetch_file_from_url leaked ConnectionError, although it did close the response - Python Tool Router URL fetch leaked ConnectionError and left the response open - TypeScript RemoteFile leaked the native fetch/body TypeError instead of RemoteFileDownloadError ## Verification - Python make chk - Python make tst: 1,490 passed - TypeScript core typecheck - TypeScript core tests: 1,245 passed, 2 expected failures - TypeScript package build: 19 packages - focused Python regression tests: 3 passed - focused TypeScript RemoteFile tests: 17 passed |
||
|
Alberto Schiabel
|
d544006a25 |
fix(sdk): pin the validated address when fetching URLs (SSRF DNS rebinding) (#4172)
Fixes #4151. ## The problem Both SDKs validated a URL by resolving its hostname, and then handed the *hostname* to the HTTP client, which resolved it again when it opened the socket. Two lookups, two answers: a short-TTL record under an attacker's control answers publicly for the check and with `169.254.169.254`, `127.0.0.1`, or RFC 1918 space for the connect. The guard passes and the connection lands inside the network — classic TOCTOU DNS rebinding, documented in both modules until now as a known residual. ```mermaid sequenceDiagram participant SDK participant DNS as Attacker DNS participant Meta as 169.254.169.254 Note over SDK,Meta: before SDK->>DNS: resolve evil.example.com (validate) DNS-->>SDK: 93.184.216.34 — passes the guard SDK->>DNS: resolve evil.example.com (connect) DNS-->>SDK: 169.254.169.254 SDK->>Meta: GET /latest/meta-data/… Meta-->>SDK: credentials ``` ## The fix Resolve once, validate every answer, then connect to the address that was validated. There is no second lookup left to rebind. - **Python** — `safe_get` / `safe_request` mount a transport adapter that swaps the connect target for the duration of the socket connect only. The `Host` header and TLS SNI keep the hostname, so certificate verification is unchanged; rewriting `conn._dns_host` for the whole connection would have sent `Host: <ip>` and offered the IP as SNI, failing against every real origin. Every fetch call site now goes through those two helpers, so no `requests.get` sits next to a bare check any more: - `_files.py::_fetch_file_from_url`, `_files.py::FileDownloadable.download` - `tool_router_session_files.py::_fetch_url_bytes` - `safe_request`, per redirect hop - **TypeScript** — `assertSafeFetchTarget` returns the validated address and `ssrfSafeFetch` hands `fetch` a dispatcher pinned to it, re-pinned per redirect hop. The dispatcher goes to the runtime's own `fetch`, so callers that stub `globalThis.fetch` keep working. The pinned `lookup` answers both shapes Node calls it with — the address *list* it uses for Happy Eyeballs, and the single `(address, family)` it uses when `autoSelectFamily` is off — since answering in the wrong shape is rejected as an invalid address. - A fail-closed peer assertion runs on the Python side before a byte is written to the socket — redundant while pinning works, and a tripwire if a urllib3 upgrade ever breaks it. - `workerd` is unchanged: it already fails closed for user-supplied URLs. Redirect *validation* already existed in both SDKs (`safe_request` / `ssrfSafeFetch`); what was missing was re-pinning each hop. ## Tests The existing suites could not express this bug: they mock both the resolver and the HTTP client, so check and use are the same mock. The new tests use real sockets. - `python/tests/test_url_safety_pinning.py` — two loopback servers and a resolver that answers the first lookup with one endpoint and every later one with another, which is what a short-TTL rebinding record does. Asserts the rebound endpoint receives **zero** connections, and that `Host` still carries the hostname. Both tests fail on `next` and pass here. - `ts/packages/core/test/utils/pinnedDispatcher.node.test.ts` — a real server plus a hostname under `.invalid`, which RFC 2606 guarantees never resolves. A request that arrives proves the connect used the pinned address and never consulted DNS. The third case shows the contrast: unpinned, the same fetch cannot resolve at all. - `ssrfGuard.test.ts` gains assertions that each hop is pinned to that hop's own validated address. - `pinnedDispatcher.node.test.ts` also pins with `setDefaultAutoSelectFamily(false)`, which is the branch Node takes for the single-address callback. ## Notes - Supersedes #4157, which diagnosed this correctly. Its post-response peer check turned out not to hold: with an HTTP/1.0 or `Connection: close` server, urllib3 detaches the socket (`conn.sock is None`) while `r.content` still returns the full body, so the check fails open exactly where exfiltration succeeds. That is why the assertion here runs at connect time instead. - The Python package now declares `urllib3>=2` directly. `url_safety` imports it for `NameResolutionError`, which only exists from 2.0, and the pinning adapter reaches into 2.x connection internals; `requests` alone allows 1.x, where `import composio` would have failed outright. - `@composio/core` gains an `undici` dependency, pinned to `^7`: undici 8 dispatchers are rejected by the `fetch` in every Node version this package supports (22/24/25, verified). The real-socket test runs on the full CI matrix, so a future incompatibility fails loudly instead of silently un-pinning. - `undici` is imported on first pinned request rather than at module load: importing it installs a process-wide global dispatcher, which would have handed the host application's own unrelated `fetch` calls this package's undici merely because it imported `@composio/core`. - Residuals, now documented in the modules: - Requests routed through an environment proxy keep the pre-flight check only. The proxy resolves the hostname itself and the SDK cannot see or pin that resolution. - A process that does perform a pinned fetch still ends up on this package's `Agent` if nothing had claimed the global dispatcher slot yet. undici defines that slot non-configurable, so it cannot be handed back — assigning `undefined` leaves the runtime's own `fetch` asserting on a missing dispatcher. |
||
|
Alberto Schiabel
|
b1c6671b20 |
fix(py): send Content-Type on S3 PUTs and parse Content-Length (#4168)
This PR: - fixes #4153 - supersedes https://github.com/ComposioHQ/composio/pull/4154 (fork PR authored against a pre-#4146 tree; its tests patch `requests.put`, which `upload()` no longer calls after #4146, so they fail on merge) - adds `parse_content_length()` in `composio.utils.url_safety`, shared by both URL fetch helpers (`_files.py::_fetch_file_from_url` and `tool_router_session_files.py::_fetch_url_bytes`), so a malformed/negative remote `Content-Length` degrades to unknown size under the streamed byte count instead of raising a raw `ValueError` - converges the file and bytes upload paths onto `_upload_to_presigned_url()`: one PUT that sends the `Content-Type` the presign request was signed with and raises `ErrorUploadingFile` carrying the HTTP status (a 403 no longer collapses into a path-only error) - single-sources the presign wire shape via `_request_presigned_upload()`; `from_path()` forwards the exact mimetype it minted, so signed and sent content types cannot drift - mirrors the TypeScript SDK: `uploadFileToS3` funnels path/URL/File inputs through one uploader that always sends `Content-Type` and throws on non-2xx, and `readResponseBodyWithLimit` trusts `Content-Length` only as a hint - adds tests against the `safe_request` seam (`test_file_upload_robustness.py`, plus one malformed-header case for `RemoteFile.buffer()`) ## Context Both defects in #4153 were symptoms of two parallel upload paths drifting: the bytes path sent `Content-Type` and raised with the status, the file path did neither. Patching the symptom in place (as #4154 proposed) would have left three presigned PUT sites and two error contracts in the tree; this PR deletes the drift vector instead. Full suite green (1320 passed), `make chk` and `make snt` clean; Python-only change, so no changeset per `AGENTS.md`. --------- Co-authored-by: Mustaqeem66 <265153888+Mustaqeem66@users.noreply.github.com> |
||
|
Alberto Schiabel
|
6ba9179b48 |
fix(files): validate URLs from API responses before fetching them (#4146)
This PR: - builds on top of https://github.com/ComposioHQ/composio/pull/4144 - routes every fetch whose URL comes from an API response through the SSRF guards that already existed — ten sinks, five per SDK: the tool-execution download, both S3 presigned uploads, `RemoteFile.buffer()`/`blob()`, and the Tool Router session file upload - adds `safe_request()` (Python), which follows redirects itself and re-validates each hop, so a validated URL cannot 302 into private space and an S3 307 region redirect still works - makes `RemoteFile.buffer()` (Python) share `_fetch_url_bytes` with the user-supplied-URL path instead of duplicating it — it previously read `response.content` with no size cap, no redirect control, and no target validation - adds `ssrfSafeFetchWhereSupported` (TypeScript): the full guard on Node, a plain `fetch` on workerd, so Tool Router session file transfers keep working in edge runtimes rather than failing closed - documents DNS rebinding as a known residual in both guards - adds tests that assert the guard *runs* — blocked URL, sink never called, nothing written — rather than that a transfer succeeds ## Context `python/AGENTS.md` states the trust boundary: every field of an API response is untrusted input, because the backend may be compromised or the connection MITM'd. Both SDKs enforced that for URLs a *user* passes in (`composio.utils.url_safety`, `ssrfGuard.node.ts`) and left the URLs an API *response* supplies unguarded — backwards relative to the stated model. A response naming an internal address turned the SDK into a request proxy for it, and the fetched bytes were written to disk or returned to the caller, typically into an LLM context. `RemoteFile.buffer()` was the weakest of the ten: four lines above it, `_fetch_from_url` validated its target, refused redirects, and streamed against a 100 MiB cap; `buffer()` did none of the three. The only difference between them was which side of the trust boundary the URL arrived from. Three decisions worth review: - **The guard is unconditional.** Presigned URLs are public, so nothing legitimate should resolve to private space. There is no escape hatch, and no new configuration surface. - **The tool-execution download stays uncapped.** It streams straight to disk, so `_MAX_RESPONSE_SIZE` — a memory-exhaustion bound — does not apply, and tool attachments legitimately exceed it. `RemoteFile.buffer()` *is* capped, because it buffers in memory. - **Uploads follow redirects with re-validation rather than refusing them**, since S3 can answer a PUT with a 307 region redirect. Downloads keep `allow_redirects=False`, matching the existing fetch paths. Python now matches the TypeScript guard's per-hop re-validation, which was the better of the two implementations. Verified locally: `make chk` and `make tst` clean on the Python side; `pnpm -C packages/core test` 1095 passed, typecheck and lint clean. |
||
|
Xiaoju
|
dd40a9bb6f |
fix(py): bound file transfer requests with timeouts (#3563)
## Summary - add bounded `(connect, read)` timeouts to Python SDK session file downloads and uploads - add the same timeout coverage to presigned S3 upload/download paths used by file helpers - convert timeout/request failures into the SDK's existing file errors and cover them with tests Fixes #3560 Fixes #3561 Fixes #3562 ## Changes - `RemoteFile.buffer()` now uses the existing file transfer timeout constants and preserves file context on request failures - `ToolRouterSessionFilesMount.upload()` and S3 upload helpers now bound presigned `PUT` requests - `FileDownloadable.download()` now bounds streaming `GET` requests, wraps streaming read failures, and closes the response - added regression tests for request timeouts and timeout arguments - added a changeset for the published SDK change ## Type of change - [x] Bug fix - [ ] New feature - [ ] Refactor/Chore - [ ] Documentation - [ ] Breaking change ## How Has This Been Tested? - [x] `D:\anaconda3\envs\yolov12\python.exe -m ruff check python\composio\core\models\tool_router_session_files.py python\composio\core\models\_files.py python\tests\test_tool_router_session_files.py python\tests\test_files.py` - [x] `D:\anaconda3\envs\yolov12\python.exe -m ruff format --check python\composio\core\models\tool_router_session_files.py python\composio\core\models\_files.py python\tests\test_tool_router_session_files.py python\tests\test_files.py` - [x] `COMPOSIO_CACHE_DIR=<repo>\.tmp-composio-cache D:\anaconda3\envs\yolov12\python.exe -m pytest python\tests\test_tool_router_session_files.py python\tests\test_files.py::TestUploadBytesToS3 python\tests\test_files.py::TestFileDownloadablePathTraversal::test_safe_filename_passes_through python\tests\test_files.py::TestFileDownloadablePathTraversal::test_download_uses_timeout python\tests\test_files.py::TestFileDownloadablePathTraversal::test_download_timeout_raises_error python\tests\test_files.py::TestFileDownloadablePathTraversal::test_download_stream_timeout_raises_error` Note: I also ran the full `python\tests\test_tool_router_session_files.py python\tests\test_files.py` pair locally. The new and related tests passed, but the full run has one pre-existing Windows-specific failure in `TestFileDownloadablePathTraversal::test_empty_name_safe_fails_at_write_time`: opening a directory for writing raises `PermissionError` on Windows instead of the test's expected `IsADirectoryError`. ## Screenshots (if applicable) N/A ## Checklist - [x] I have read the Code of Conduct and this PR adheres to it - [x] I ran linters/tests locally and they passed - [ ] I updated documentation as needed - [x] I added tests or explain why not applicable - [x] I added a changeset if this change affects published packages ## Additional context This keeps the existing `(5s connect, 60s read)` timeout convention already used by the Python SDK's URL fetch helpers. --------- Co-authored-by: jkomyno <alberto@composio.dev> |
||
|
jkomyno
|
7a3d30a37c |
fix(security): apply SEC-316 defense-in-depth to RemoteFile.save()
`RemoteFile.save()` in `tool_router_session_files.py` shares the same
trust-boundary shape as the `FileDownloadable.download()` path traversal
fixed in SEC-316: the default save location is built as
`~/.composio/files/<self.filename>`, where `self.filename` is
`Path(mount_relative_path).name` derived from the (potentially
compromised or MITM'd) tool-router API response.
Today the basename strip alone makes this safe-by-OS-error — a server
returning `mount_relative_path == ".."` produces `filename == ".."`,
which collapses the resolved path to `~/.composio/` and `write_bytes()`
fails with `IsADirectoryError`. That's a non-security bug surface, but
it leaks intent through a confusing error and relies on a happy
accident of `Path` semantics.
Mirror the SEC-316 pattern: after building the default `save_path`,
verify `save_path.resolve().is_relative_to(default_dir.resolve())` and
raise `ValidationError` on mismatch. The check only runs on the
default-location branch — a caller-supplied `path` is fully trusted
and writes wherever the SDK user chooses.
Adds `test_save_default_location_rejects_dotdot_filename` covering the
`mount_relative_path="foo/.."` case (where `Path("foo/..").name == ".."`)
and asserting nothing is written under the cache dir.
|
||
|
Musthaq Ahamad
|
2bccee8d55 | feat(python): add file upload/download support and ToolRouterSession refactor (#2865) |