skills/composiohq__composio
1
0
Fork 0
mirror of https://github.com/ComposioHQ/composio.git synced 2026-09-22 11:46:35 +08:00
Code Issues Packages Projects Releases Wiki Activity
5,395 Commits 338 Branches 1,550 Tags
docs/auto-sdk-sync
Commit Graph

7 Commits

Author SHA1 Message Date
Alberto Schiabel 85996c4a1d fix(sdk): harden pusher auth and cross-origin redirect headers (#4406) 
This PR:

- wraps `pysher.Pusher` in `_ComposioPusher`, whose channel-auth POST
carries a `(5, 15)` connect/read timeout and raises
`TriggerSubscriptionAuthError` (a `TriggerSubscriptionError`) on a
transport failure, a non-200, or a response without an `auth` token —
pysher 1.0.8 sent it with no timeout and turned a non-200 into a bare
`AssertionError` on the websocket thread, on every (re)subscribe
- keeps that POST a plain `requests.post(..., timeout=...)` rather than
routing it through `safe_request`: the endpoint is built from the
configured Composio API base URL, a fixed trusted host, not a value from
a response, and the SSRF guard would refuse a local dev base URL
- validates `pusher_cluster` against `^[a-z0-9-]+$` (non-empty, at most
64 chars) before pysher formats it into `ws-{cluster}.pusher.com`,
raising `InvalidPusherClusterError` that names the shape violation
without echoing the value
- replaces the `unittest.mock.MagicMock` stand-in for pysher's
connection logger with a dedicated `logging.Logger` (`NullHandler`,
`propagate=False`, disabled), so `unittest` leaves the runtime import
graph while raw frames stay out of user logs; a test asserts the module
source no longer mentions `unittest`
- strips `Authorization`, `Proxy-Authorization`, and `Cookie` from the
next hop when `ssrfSafeFetch` or `safe_request` follows a redirect to a
different origin; same-origin hops keep them. Manual redirect following
bypasses both `fetch`'s cross-origin rule and `requests`'
`rebuild_auth`, so neither guard applied it before — the gap #4387 left
out
- `@composio/slim` has no mirrored source (its build copies
`core/dist`), so the changeset covers `@composio/core` and
`@composio/slim` as patches

Verified with `pytest tests/test_triggers.py tests/test_url_safety.py
tests/test_path_join_guardrail.py` (192 passed), `ruff check` / `ruff
format --check` on the changed files, `mypy --config-file
config/mypy.ini` on the three changed modules with the noxfile's stub
pins (no issues), `vitest run test/utils/ssrfGuard.test.ts` in
`@composio/core` (42 passed), `pnpm typecheck` at the root (14 tasks
successful), and `oxlint` + `prettier --check` on the changed TypeScript
files.

https://claude.ai/code/session_016ZuBv7JhVdSYTLYcTy2VJr
 2026-09-09 21:29:18 +02:00
Alberto Schiabel ba85f4d183 fix(sdk): honor Fetch redirect semantics in both SSRF guards (#4387) 
This PR:

- builds on top of https://github.com/ComposioHQ/composio/pull/4271,
whose commit it carries unchanged
- applies the Fetch standard's redirect method/body rules in **both**
SSRF guards via `_redirect_rewrite` / `redirectRewrite`: a `303` retries
as a bodiless request, a `301`/`302` does the same for a `POST`, and
`307`/`308` replay both
- narrows `ssrfSafeFetch` to the five statuses the Fetch standard calls
a redirect, so a `304` or `305` carrying a `Location` is returned to the
caller instead of followed — Python already used
`_REDIRECT_STATUS_CODES`
- drops `params` after the first hop in `safe_request`, since `Location`
carries the query for the target it names and re-appending handed a
query-string credential to a target that never asked for one
- purges the union of the Fetch `request-body-header` set and the two
`requests` also drops, identically on both sides
- blocks the IPv6 transition ranges the TypeScript CIDR list missed —
6to4 `2002::/16`, Teredo and the rest of `2001::/23`, local-use NAT64
`64:ff9b:1::/48`, `100::/64`, `2001:db8::/32`, site-local `fec0::/10` —
and the IPv4/IPv6 multicast and `192.88.99.0/24` ranges Python's
`is_global` missed

## Context

Both guards follow redirects by hand so every hop is revalidated against
the address blocklist. That also means neither inherits the method and
body rewriting `fetch` and `requests` would have done, so an upload
answered with a `303` was replayed — payload and all — at a result URL
that expects a GET.

https://github.com/ComposioHQ/composio/pull/4271 landed that rule in
Python only, which left the two SDKs disagreeing on the same wire
behavior. Reviewing for that divergence surfaced the redirect-status
set, the `params` replay, and the address-blocklist gaps above.
`2002:7f00:1::` is 6to4 for `127.0.0.1`, and it passed the TypeScript
guard as a public address.

Verified with `pytest python/tests/test_url_safety.py` (56 passed) and
`vitest run` in `@composio/core` (54 files, 1280 passed), plus `ruff`,
`tsc --noEmit`, `oxlint` and `prettier`. Fail-before confirmed: 10 of
the new TypeScript cases and 5 of the new Python cases fail against the
unmodified guards.

Two known gaps are deliberately left out, each deserving its own change:
neither guard strips `Authorization`/`Cookie` on a cross-origin
redirect, and a non-seekable Python body is re-sent exhausted on a `307`
where TypeScript throws a bare `TypeError` on a consumed
`ReadableStream`.

https://claude.ai/code/session_01SB3ZJdvoqBcRrWb2toWVrX

---------

Co-authored-by: ump45nose <52391318+ump45nose@users.noreply.github.com>
 2026-09-08 20:51:12 +02:00
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.
 2026-08-20 13:34:37 +02:00
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>
 2026-08-18 19:09:02 +02:00
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.
 2026-08-18 13:25:01 +02:00
Alberto Schiabel 026c104fe8 fix(py): validate canonical URL targets (#3962) 
This PR:
- builds on top of https://github.com/ComposioHQ/composio/pull/3823
- carries forward the Requests canonicalization hardening proposed in
https://github.com/ComposioHQ/composio/pull/3810
- resolves and validates the prepared hostname that Requests will
actually connect to
- blocks backslash parser differentials and rejects malformed ports
before DNS resolution
- adds focused URL-safety coverage while preserving both existing upload
entry points

Validation:
- `uv run --frozen pytest tests/test_url_safety.py tests/test_files.py
tests/test_tool_router_session_files.py -q` (177 passed)
- `make chk`
 2026-07-28 14:03:53 +05:30
Alberto Schiabel c4071feab4 fix(py): guard URL uploads and redact telemetry (#3823) 
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 …`
 2026-07-14 14:32:05 +04:00