fix(sidecar): close Composio execution and lifecycle governance gaps - #441
Open
veeso wants to merge 6 commits into
Open
fix(sidecar): close Composio execution and lifecycle governance gaps#441veeso wants to merge 6 commits into
veeso wants to merge 6 commits into
Conversation
`decode_backend` is reached only for POST, but its final match arm reused
`is_recognized_non_execution_path` as an unconditional catch-all. That
helper enumerates the *read* route shapes for the GET/HEAD/OPTIONS branch
in `decode_protected`, so treating it as a POST allowlist admitted writes
to `toolkits`, `tools`, `tools/{slug}`, and the Tool Router session
sub-collections as ungoverned passthrough: no capability validation, no
Cedar evaluation, no logical action in the audit trail.
Only two POST passthroughs were ever intended, both Tool Router session
shapes that `decode_lifecycle_write` deliberately declines to govern:
session creation on the bare collection, and a write to an existing
`session/{id}`. Spell those two out as explicit match arms and let
everything else fall to the existing `unsupported_route` denial, so the
decoder fails closed on any POST it cannot classify.
Lifecycle writes are unaffected: `connected_accounts`, `auth_configs`,
and `session/{id}/link` POSTs are classified by `decode_lifecycle_write`
before the match and never reach the catch-all. The GET/HEAD/OPTIONS and
MCP DELETE branch still uses the read-route recognizer unchanged.
Only direct execution required a `version` field. Session, `execute_meta`, meta-tool, multi-execute and hosted MCP calls passed the field through to `action_for_tool`, which checked it only when present, so omitting it downgraded the pinned-version guarantee to a bare slug allowlist on those routes: Composio would run whatever its server currently serves while the action class still came from the local snapshot. Consolidate the gate in `action_for_tool` and deny an absent version with the same `unpinned_tool` code direct execution already used. Hosted MCP JSON-RPC has no version slot, so `tools/call` now reads the pin from the tool arguments the way it already reads the account selector; an MCP client that cannot attach one can no longer reach a governed tool. Also stop dropping a nonstandard port from the governed envelope. The connector rebuilds the outbound URL from that resource host, so a request that arrived on `backend.composio.dev:8443` was evaluated correctly and then dispatched to 443. `canonical_host` keeps stripping ports for the host-identity checks; the envelope now uses the shared normalization instead.
`GET`/`HEAD`/`OPTIONS` on the `connected_accounts` and `auth_configs` routes matched the generic passthrough recognizer at any path depth, so they reached Composio with no capability check, no Cedar evaluation, and ahead of any operator-authored mapping rule on those hosts. Those responses list which integrations exist and how they authenticate, which is credential disclosure, not discovery. Add `decode_lifecycle_read`, mirroring `decode_lifecycle_write` over the same `classify_lifecycle_route` shapes, and classify those reads as `credential.read` with a synthetic `COMPOSIO_LIST_CONNECTED_ACCOUNT` style slug. `lifecycle_action` now takes the action class so writes stay `account.permission.change`. Session, `tools`, and `toolkits` reads and MCP streams keep their current passthrough treatment. Being governed actions, these listings inherit the existing query-string rule: a paginated read is denied rather than dispatched with a filter the policy never saw.
Requiring a pinned version on every execution route made hosted MCP unreachable in practice. JSON-RPC `tools/call` has no native version field, so demanding one denied every stock MCP client rather than closing a hole an attacker could use: omitting a version does not let a caller reach anything the slug allowlist does not already admit, it only leaves the executed version up to Composio's server. Introduce `VersionPolicy` and thread it through the decode chain. Direct execution, Tool Router `execute`, and `execute_meta` pass `Required` and keep denying `unpinned_tool`, which is still stronger than before. Hosted MCP passes `Optional`. The exemption covers absence only: a version a client does attach as a tool argument is still checked against the pin and still denies `version_mismatch`. The MCP test fixtures go back to their version-free form, which is now the coverage proving the exemption holds.
Governing `connected_accounts` and `auth_configs` reads pulled them under the query-string denial, which broke pagination: `GET /api/v3/connected_accounts?cursor=…` started failing closed. That rule exists to stop unevaluated input riding along on a write or a semantically specific action, but a cursor selects a page of the same listing rather than changing which action is classified, so the denial was a side effect nobody wanted. Exempt exactly that family. `is_lifecycle_read` requires both the synthetic lifecycle toolkit and `credential.read`, because a catalog tool can also map to `credential.read` (`GMAIL_LIST_CSE_KEYPAIRS` does) and must keep denying. Every other governed shape, lifecycle writes included, is unchanged. Exempting the decode alone would have been worse than the denial: the logical envelope deliberately carries no query, so the connector rebuilds the URL from the query-free resource and would have silently returned page one. `hydrate_dispatch_http_fields` now restores the query onto the dispatch clone alongside the headers and body it already restored, which is reachable only for this family since every other governed shape denies a query during decoding. The logical resource stays query-free, so the cursor never enters a policy resource or an audit record.
…ng pack `config/mappings/composio.toml` claimed to keep the transport protected if generic normalization is reached, but only covered hosted MCP on the host and path shape each is usually seen on. The decoder recognizes both shapes on both hosts, so add the four missing rows. Document the route list the decoder actually accepts: the `v3.1` variants of every execution path, `execute_meta`, and both hosted MCP path shapes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
OpenFirma governs the calls an AI agent makes through Composio, a gateway that connects agents to outside services. The gateway support already on
main(#370) left several openings: some routes accepted a tool call without proving which version of the tool catalog it belonged to, reading back account and credential settings passed through without any policy check, and a write dressed up as a read could slip past the checks meant for read-only routes. Each of those is a way for a call to reach a service without being classified first.What Changed
Risks / Notes
AI Assistance
Split out and verified with Claude Code (Claude Opus 5).