## Summary
- preserve boolean, empty, null, type-array, enum, const, and
scalar-constraint semantics across every Python conversion entry point
- intersect Zod enum and const values with declared types and
constraints, including compound JSON values
- default unversioned exact validation to Draft 7 and apply inclusive
and numeric exclusive bounds independently
- run one byte-identical corpus through Python, Zod, and Effect so
accepted and rejected inputs stay aligned
- keep exact JSON Schema acceptance separate from Pydantic default
materialization
## Review follow-up (second push)
- Python: exact Draft 7 acceptance now wraps all three entry points
(`json_schema_to_pydantic_type`, `json_schema_to_model`,
`pydantic_model_from_param_schema`), so they can no longer disagree
- Python: draft-4 boolean `exclusiveMinimum`/`exclusiveMaximum` (OpenAPI
3.0 style) no longer crash conversion — exact validation falls back to
Draft 4, and the library input is translated to the numeric spelling
- Python: ECMA-only regex patterns (look-around) no longer crash
pydantic model builds — Rust-incompatible patterns fall back to Python
`re`
- Python: type arrays with sibling constraints no longer raise
`TypeError` on valid input — constraints are scoped per member before
the library sees them
- Python: integral floats satisfy `integer`, `const` intersects `enum`,
annotation-only schemas accept anything, and an optional property with
an empty `enum` tolerates absence
- Zod: typeless scalar constraints apply per instance type, and string
lengths count Unicode code points instead of UTF-16 code units
- Effect: draft-4 boolean exclusive bounds are enforced instead of
silently ignored
- `multipleOf` uses decimal scaling in all three converters (declared
`divergesFromJsonSchema` on the corpus case)
- shared corpus grows by 13 primitive cases; new property-based tests
check acceptance against real Draft 7 oracles (hypothesis + `jsonschema`
in Python, fast-check + Ajv in TypeScript)
## Verification
- Python `make chk` (ruff + mypy)
- Python pytest: 1,572 passed (5 langchain-extra tests need an env this
sandbox lacks; unchanged from base)
- `@composio/json-schema-to-zod`: 187 passed incl. 300-run fast-check
property test; typecheck + build
- `@composio/json-schema-to-effect-schema`: 133 passed; typecheck
- `@composio/core` corpus ingress tests: 61 passed
- shared Python/TypeScript corpus files are byte-identical
(shasum-verified)
- `git diff --check`
## Contributor context
This replaces four narrow proposals after independent local
reproduction:
- [#4301](https://github.com/ComposioHQ/composio/pull/4301) ·
[Glen](https://app.tryglen.com/ComposioHQ/composio/pull/4301)
- [#4302](https://github.com/ComposioHQ/composio/pull/4302) ·
[Glen](https://app.tryglen.com/ComposioHQ/composio/pull/4302)
- [#4303](https://github.com/ComposioHQ/composio/pull/4303) ·
[Glen](https://app.tryglen.com/ComposioHQ/composio/pull/4303)
- [#4307](https://github.com/ComposioHQ/composio/pull/4307) ·
[Glen](https://app.tryglen.com/ComposioHQ/composio/pull/4307)
---------
Co-authored-by: simpleqt <89645338+simpleqt@users.noreply.github.com>
## 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
Review follow-up on the download size cap.
`requests.exceptions.RequestException` subclasses `OSError`, so the two
handlers added for the write loop were byte-identical and the second already
subsumed the first. Collapse them into one `except OSError` and say why in a
comment, so the next reader does not re-add the redundant clause.
Route partial-file cleanup through `_discard_partial_download`, which
suppresses cleanup failures: an `OSError` from `unlink` would otherwise
replace the `ResponseTooLargeError` or transport error the caller needs.
Cover the two error paths that had no tests: a transport failure mid-stream
and a failing write both raise `ErrorDownloadingFile` and leave no partial
file behind. Without the handler the write failure escapes as a raw
`OSError(28)` — the defect these pin.
Claude-Session: https://claude.ai/code/session_01K1hH9PMmd6KPKdkACX553z
`FileDownloadable.download` streamed the response straight to disk with no
byte accounting, so an untrusted `s3url` could fill the disk. Add the same
`Content-Length` pre-check plus authoritative streamed-byte counter the
sibling `_fetch_file_from_url` already uses, capped at `_MAX_RESPONSE_SIZE`
and overridable per call via `max_size`.
Also close two gaps the write loop left open: an `OSError` from `fd.write`
(disk full, permissions) escaped the documented `ErrorDownloadingFile`
contract, and any failure left a partial file on disk that no caller was
told about. Every failure path now unlinks the partial file;
`ResponseTooLargeError` still propagates uncaught so callers see the limit.
Claude-Session: https://claude.ai/code/session_01K1hH9PMmd6KPKdkACX553z
Both SDKs forwarded "" for a file_uploadable parameter (e.g. Gmail
attachment) verbatim to the backend, which rejected it with a Pydantic
validation error. Python only dropped it inside the opt-in auto-upload
walker; TypeScript never did, and with auto-upload on it tried to upload
the empty string.
Run a schema-aware, upload-free pass on the default execute path that
omits empty file values, and reuse the same walker for staging when
auto-upload is enabled.
Closes#4233
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.
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.
This PR:
- centralizes filesystem path construction for API-provided slugs and
filenames in `composio.utils.safe_path`
- rejects traversal, Windows-invalid names, invalid Unicode, and
overlong encoded filenames before creating directories or writing files
- normalizes trusted roots consistently and routes both Python download
paths through the shared helpers
- adds a fail-closed AST guard for new dynamic path construction,
including direct `Path(...)` calls
- isolates provider initialization from the real home directory and
makes the home-write guard report changes without deleting them
- removes the obsolete download filename wrapper
## Verification
- `pytest -q`: 1,248 passed, 47 skipped
- repository-configured Ruff checks and formatting passed for every
changed Python file
- targeted mypy checks passed for the changed helpers and tests
This PR:
- ports the TypeScript SDK's public-network guard to Python URL file
uploads
- rejects non-HTTP(S), private, loopback, link-local, reserved, and
mixed DNS targets before a request is made
- applies the guard to both `FileUploadable.from_url()` and Tool Router
session-file URL uploads
- redacts URL queries, Authorization credentials, and secret-like
key/value pairs before error telemetry leaves the process
- adds focused SSRF and redaction regressions alongside the existing
file-upload coverage
Validation:
- `uv run --frozen pytest tests/test_url_safety.py
tests/test_redaction.py tests/test_files.py -q`
- `uv run --frozen ruff check …`
- `uv run --frozen ruff format --check …`
This PR:
- fixes#3746 / GHSA-hp3h-89pf-5q58 — the CLI's tool file-upload path
bypassed the sensitive-file denylist
- **root cause:** the denylist was enforced at the *caller* layer, so
`@composio/cli`'s duplicate upload path (`readFileFromDisk`) read and
uploaded any local path a tool argument pointed at — `~/.ssh/id_rsa`,
`~/.aws/credentials`, `.env`, etc. — enabling credential exfiltration
via `composio execute` / `composio run` (incl. LLM-driven agents hit by
prompt injection)
- **unified fix:** one canonical guard, exported from `@composio/core`
and enforced at the read primitive in every SDK
- `core`: the guard now routes fs/path access through the internal
`#platform` abstraction (adds a `realpathSync` method), so it carries
**no static `node:*` imports** and is exported from the package root:
`assertSafeFileUploadPath`, `isBlockedSensitiveFileUploadPath`,
`BUILTIN_FILE_UPLOAD_PATH_DENY_SEGMENTS`
- `cli`: `readFileFromDisk` calls the shared `assertSafeFileUploadPath`
before `fs.readFile`; URLs and `File` objects are unaffected (matches
core)
- `python`: already routes all uploads through the guarded
`FileUploadable.from_path` — no functional change; adds regression tests
for parity
- supersedes #3755, which exported the then-node-only guard from core's
**main** `index.ts` and would have injected `node:fs` into the shared
`dist/index.mjs` (the workerd/edge-light entry), likely breaking
Cloudflare Workers / Vercel Edge
- changesets: `@composio/core` minor (new public exports),
`@composio/cli` patch (security fix)
## Verification
- core: full suite passes (997), incl. new root-export +
realpath-symlink cases
- **edge bundle stays clean**: built `dist/index.mjs` has no static
`node:*` imports and reaches the platform only via `#platform`
- cli: new regression test drives the real `uploadToolInputFiles →
readFileFromDisk → guard` chain — `~/.ssh/id_rsa` and `.env` throw
`ComposioSensitiveFilePathBlockedError` before any `createPresignedURL`
call; a normal file still uploads
- python: `test_files.py` + sensitive-path suite pass (145), incl. 3 new
`from_path` guard tests
- `tsgo` typecheck + eslint clean for core and cli
## Problem
Tools whose parameter schemas express their file fields through a
`$ref`/`$defs` indirection lose auto file handling. With
`dangerously_allow_auto_upload_download_files=True`, a
`file_downloadable` output behind a `$ref` is never downloaded (the raw
`{name, mimetype, s3url}` object is passed through), and a
`file_uploadable` input behind a `$ref` is never staged — silently, with
no error.
`GMAIL_GET_ATTACHMENT` is the canonical shape — the flag sits two `$ref`
hops deep:
```
data → $ref → #/$defs/GetAttachmentResponse → file → $ref → #/$defs/FileDownloadable (file_downloadable: true)
```
**Root cause:** the hand-rolled walkers in
`composio/core/models/_files.py` (`_has_file_property`,
`_substitute_file_upload_value`, `_substitute_file_download_value`, …)
recurse through `properties`/`anyOf`/`oneOf`/`allOf`/`items` but **never
dereference `$ref`**, so a flagged node reachable only through a
reference is invisible to them.
Fixes https://github.com/ComposioHQ/composio/issues/3506
## Approach
Rather than teach each walker to dereference — which means threading a
root schema through ~10 methods, re-deriving `$ref` handling in each,
and risking unbounded recursion on cyclic schemas — this introduces a
single **`dereference_json_schema`** utility and inlines references
**once at the `FileHelper` boundary** (`process_file_uploadable_schema`,
`substitute_file_uploads`, `substitute_file_downloads`). The walkers
stay reference-agnostic.
This mirrors the TypeScript SDK's `dereferenceJsonSchema` (the
counterpart fix in #3566 for the TS twin of this bug, #3307) so both
SDKs share one behavioral contract. Resolving once at the boundary also
gives every walker `$ref` support for free — including keywords they
never special-cased (`additionalProperties`, `patternProperties`,
`prefixItems`, `not`), because the resolver walks containers
reflectively.
## What's in the utility (`composio/utils/json_schema.py`)
Faithful port of the TS guards:
- **Cycle safety** — breaks both `$ref` cycles and live-object identity
cycles with a permissive `{type: object, additionalProperties: true}`
sentinel. (A per-node lazy resolver without threaded cycle tracking
infinite-loops on a self-referential schema; there's a regression test
for exactly this.)
- **Depth caps** — `MAX_REF_CHAIN_DEPTH=100` / `MAX_NODE_DEPTH=512`
raise a typed `JSONSchemaRefResolutionError` instead of exhausting the
interpreter (with a `RecursionError` backstop converted to the same
typed error).
- **Sibling-keyword merge** — Draft 2020-12 semantics (siblings win on
collision), so a `description`/`default` next to a `$ref` survives.
- **External-ref passthrough** — `http(s)://` refs are left untouched
and logged once for audit.
- **`sentinel` mode** — a dangling `$ref` (an API schema that emits
`$ref` with no `$defs` block,
https://github.com/ComposioHQ/composio/issues/3307) degrades to the
permissive sentinel + an LLM-visible hint, instead of aborting the tool
call. The `FileHelper` uses this mode for API-sourced schemas.
- **Non-mutating** — returns a new schema; `$defs`/`definitions` are
stripped from the inlined root.
## Tests
- `python/tests/test_json_schema.py` — 30 cases mirroring the TS
`jsonSchema.test.ts` contract (chains, reflective container walk, legacy
`definitions`, sibling merge, cycles, depth caps, pointer-escape
decoding, array-index pointers, full `sentinel`-mode suite).
- `python/tests/test_files.py` — end-to-end regressions through the
public methods: download/upload behind `$ref`/`$defs` (the #3506 repro),
the LLM-facing input transform, graceful degradation of a dangling
`$ref`, and a **self-referential schema that must not recurse
infinitely**.
- `nox -s chk` (ruff + mypy) green; `nox -s tst` for the file/execution
suites green.
## Notes
Supersedes #3545 (third-party PR that fixed the same bug by threading
`root_schema` through each walker). That diagnosis was correct; this
takes the centralized route consistent with the TS SDK and adds the
cycle/depth guards. No changeset — Python-package change.
## 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#3560Fixes#3561Fixes#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>
Stacked on #3397 · refs
https://github.com/ComposioHQ/composio/issues/3354
## Summary
The TS fix in #3397 handled the reported case (`output_parameters: {}`
from MCP toolkits) by normalizing empty schemas at `transformToolCases`.
While verifying the Python SDK didn't have the equivalent Zod check, I
found an **adjacent** crash on the input side.
The Python SDK does not have the same Pydantic check** —
`composio_client` types `input_parameters` / `output_parameters` as
`Dict[str, Optional[object]]`, which accepts `{}` trivially.
However, `Tools._get()` pipes every fetched tool's `input_parameters`
unconditionally through `FileHelper.enhance_schema_descriptions`
(`python/composio/core/models/tools.py:361-363`), which does:
```python
for _param, _schema in schema["properties"].items():
```
That crashes with `KeyError: 'properties'` on `schema={}`. Not reachable
for the specific `granola_mcp` tools in the original report (they all
have populated `input_parameters`), but it's the same API contract — any
MCP tool with no required inputs would hit this through the public
`Composio.tools.get(...)` call.
The sibling `FileHelper.process_file_uploadable_schema`
(`_files.py:760`) already guards this with `if "properties" not in
schema: return schema`. This applies the same guard to
`enhance_schema_descriptions`.
## Fix
```diff
def enhance_schema_descriptions(self, schema: t.Dict) -> t.Dict:
...
+ if "properties" not in schema:
+ return schema
required = schema.get("required") or []
for _param, _schema in schema["properties"].items():
```
## Testing
Red → green confirmed locally. New
`TestEnhanceSchemaDescriptionsEmptySchema` class in
`tests/test_files.py` covers:
1. `schema={}` → returns `{}` (the bug repro).
2. Schema with metadata but no `properties` key → returned unchanged
(mirrors the sibling method).
3. Schema with `properties: {}` (empty dict) → already works today;
pinning behavior.
4. Populated schema → still enhanced (regression guard for the type-hint
/ required-marker enhancement).
Cases 1–2 **fail on `next`** with `KeyError: 'properties'`. All 4 pass
with the guard. Full `tests/test_files.py` stays green (116 tests).
```bash
cd python
.venv/bin/python -m pytest tests/test_files.py::TestEnhanceSchemaDescriptionsEmptySchema -v # → 4 passed
.venv/bin/python -m pytest tests/test_files.py -q # → 116 passed
```
## Stacked PR notes
- Base: `jkomyno/fix-mcp-empty-output-parameters` (#3397).
- This PR's diff against that base shows **only** the Python changes
(`_files.py` + `tests/test_files.py`).
- After #3397 merges, this should be retargeted to `next` (or rebased
automatically by GitHub).
---------
Co-authored-by: jkomyno <12381818+jkomyno@users.noreply.github.com>
## Summary
This updates the Python SDK's automatic file upload/download
substitution so it can handle tool schemas that accept either a single
file or a list of files.
Tools can expose file inputs as a union such as:
```json
{
"anyOf": [
{ "type": "object", "file_uploadable": true },
{ "type": "array", "items": { "type": "object", "file_uploadable": true } }
]
}
```
When auto-upload is enabled, the SDK presents those file-uploadable
objects to models as file path strings. If the model supplies a list of
local paths, the old resolver selected the first file-bearing union
branch, usually the single-file branch, and attempted to upload the
entire Python list as one path. That fails before the backend ever
receives the intended list of staged file descriptors.
## Changes
- Refactors upload substitution into a value/schema recursive walker via
`_substitute_file_upload_value`.
- Adds the same value/schema traversal shape for downloads via
`_substitute_file_download_value`.
- Selects composed-schema variants by runtime shape when possible, so
list values choose array branches and string/dict values keep the
single-value behavior.
- Preserves the existing fallback behavior: if no runtime shape matches,
use the first file-bearing variant.
- Treats `anyOf`, `oneOf`, and `allOf` consistently with the SDK's
existing composed-schema convention.
- Preserves root request/response dict mutation behavior for existing
call sites.
- Adds regression coverage for single-file vs multi-file unions, nested
array-item unions, download parity, and first-match fallback.
## Why
This is needed for file-capable tool schemas that are backward
compatible at the API level by accepting both a single file and multiple
files. The Python SDK should transform a list of local paths into a list
of staged `{name, mimetype, s3key}` descriptors, just as it already does
for direct array schemas.
## Verification
- `python -m ruff check python/tests/test_files.py
python/composio/core/models/_files.py`
- `python -m pytest python/tests/test_files.py -q` -> 119 passed
- `python -m pytest python/tests/test_files.py
python/tests/test_auto_upload_download_files.py
python/tests/test_upload_dir_allowlist.py
python/tests/test_tool_router_session_files.py -q` -> 166 passed
- Manual playground verification with a Gmail send using two local
attachments: both files were staged and sent successfully as an
attachment array.
Note: a full local `python/tests` run still has three unrelated failures
because `composio_langchain` is not installed in this environment; the
affected file-upload suites pass.
---------
Co-authored-by: Zen <zen@composio.dev>
Two regression tests for `FileDownloadable.download()`:
- `test_basename_collapse_through_subdir_is_rejected`: `name="foo/.."`
exercises the case where `Path(name).name` collapses to `..`. The
plain `name=".."` test already covers the same `is_relative_to()`
rejection, but the through-subdir form makes the basename
normalization explicit. Also asserts (per the P3.1 reorder) that
`outdir` is not created as a side effect of a rejected payload.
- `test_empty_name_safe_fails_at_write_time`: `name=""` makes
`outfile == outdir`, which passes the containment check (a path is
relative to itself). The download then safe-fails at
`outfile.open("wb")` with `IsADirectoryError`. Pinning this
documented behavior locks in the safe-fail and stops a future
change from silently weakening the check without a test failure.
Apply `ruff format --config config/ruff.toml` to fix `with patch(...)`
multi-line indentation that the CI Lint and Format Check rejected.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Srujan A <srujan@composio.dev>
The Composio Python SDK's `FileDownloadable.download()` constructed the
output path as `outdir / self.name`, where `self.name` is taken directly
from the Composio API response without sanitization. A compromised or
man-in-the-middle'd API server could return a filename like
`../../../../PWNED.sh` and write arbitrary files outside the configured
output directory on the user's machine.
Fix mirrors the equivalent change made in mercury's legacy
FileDownloadable (commit e3915b82c3 + c3d675dbcc):
- Strip directory components from `self.name` via `Path(...).name`,
collapsing traversal sequences to a basename.
- As a second-line defense, verify the resolved output path stays
within `outdir` using `Path.is_relative_to()` (correct directory
containment semantics, vs. naive `str.startswith()` which is
vulnerable to sibling-prefix attacks).
Adds 4 unit tests in `TestFileDownloadablePathTraversal` covering
relative traversal, absolute paths, dotdot-only names, and the safe
pass-through case.
Reference: https://cwe.mitre.org/data/definitions/22.html
CVSS: 8.2 (CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:H/A:N)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Srujan A <srujan@composio.dev>
## Summary
Automatic tool file upload/download for `file_uploadable` fields is
**off by default** in TypeScript and Python. Callers must explicitly opt
in, and uploads from local paths are constrained by a fail-closed
allowlist.
## Changes
- **Removed (breaking):** `autoUploadDownloadFiles` (TS) /
`auto_upload_download_files` (Python) — the legacy default-on flag is
gone, not just deprecated.
- **New opt-in:** `dangerouslyAllowAutoUploadDownloadFiles` (TS) /
`dangerously_allow_auto_upload_download_files` (Python). When `true`,
`tools.get(...)` collapses `file_uploadable` schemas to `{ type:
'string', format: 'path' }` and the SDK stages local paths/URLs at
execute time.
- **New:** `fileUploadDirs?: string[] | false` — fail-closed allowlist
for local upload paths. `undefined` → `[<home>/.composio/temp]`; `false`
→ reject all local paths (URLs / `File` objects unaffected); explicit
`string[]` replaces the default. Components are matched on a path
boundary after `realpath`.
- **New:** `fileDownloadDir?: string` — directory where
`file_downloadable` results are staged.
- **New:** `beforeFileUpload` hook receives `source: 'path' | 'url' |
'file'` (TS) / `'path' | 'url'` (Python) so it can branch on input type.
- **New (TS):** when auto-upload is **off** and an LLM-driven
`tools.execute` is called against a tool with `file_uploadable` inputs,
the SDK emits a one-shot warning per tool slug pointing at
`composio.files.upload()` for manual staging.
## Migration
To restore previous behavior:
```ts
new Composio({
apiKey: process.env.COMPOSIO_API_KEY!,
dangerouslyAllowAutoUploadDownloadFiles: true,
// Optional: tighten the allowlist beyond the default ~/.composio/temp
fileUploadDirs: ['/srv/uploads'],
});
```
```python
Composio(api_key="...", dangerously_allow_auto_upload_download_files=True)
```
If you previously passed the legacy flag, remove it. There is no
transitional warning — TS and Python both reject the unknown property at
the type/keyword-arg level.
## Versioning
| Package | Bump |
| ------- | ---- |
| `@composio/core` | minor |
| `composio` (Python) | minor |
| Other `@composio/*` packages | patch (via changesets
`updateInternalDependencies: "patch"`) |
See
`docs/content/changelog/04-24-26-legacy-auto-upload-config-removal.mdx`
for the full migration writeup.
Addresses review feedback from cursorbot:
- MIME type lookup now uses .lower() to be case-insensitive per RFC 2045
- URL paths are now decoded using urllib.parse.unquote before extracting filenames
This fixes issues where:
- Servers returning Content-Type headers like 'Image/JPEG' would fail lookup
- URLs like 'My%20Document.pdf' would result in percent-encoded filenames
Addresses review feedback from cursorbot:
- MIME type lookup now uses .lower() to be case-insensitive per RFC 2045
- URL paths are now decoded using urllib.parse.unquote before extracting filenames
This fixes issues where:
- Servers returning Content-Type headers like 'Image/JPEG' would fail lookup
- URLs like 'My%20Document.pdf' would result in percent-encoded filenames
Fixes issue where filenames without extensions were replaced with
timestamped names. Now a URL like https://example.com/invoice with
mimetype application/pdf becomes invoice.pdf instead of
file_20260126_abc12345.pdf.