[ricardosrib]

Description

The Agent Gateway tool-invocation path attached a user/system-scoped bearer token as a connection-wide httpx.AsyncClient header, then opened streamable_http_client(tool.url, ...) with no validation of tool.url. If tool.url ever pointed off the gateway origin — whether by a poisoned registry entry or a crafted MCPTool — the live token would be forwarded to that host.

Two vectors are closed:

• Customer flow (_customer.py): ord_id and global_tenant_id from integrationDependencies were interpolated into the MCP URL path without sanitization.
• LoB flow (_lob.py): the MCP destination fragment URL property was used directly with no check against the gateway origin.

Changes:

• New _url.py module: _require_same_origin pins scheme=https + host:port to the trusted gateway origin before any authenticated client is built; _validate_mcp_id applies an ORD-ID-aware path-segment allow-list (^[A-Za-z0-9._:-]+$) to block path traversal on interpolated IDs.
• ClientConfig.allowed_tool_hosts: tuple[str, ...] = () — escape hatch for LoB flows where a fragment legitimately points to a different host; empty by default (strict pin).
• gateway_url threaded from AuthResult down to every call site in agw_client.py — it was already available there but discarded.
• LoB discovery skips + warns on an off-origin fragment rather than aborting the whole list_mcp_tools call, consistent with the existing "no URL property → skip" behavior.

Type of Change

• Bug fix (non-breaking change that fixes an issue)

How to Test

1. Run pytest tests/agentgateway -q — all tests pass; new rejection tests assert AgentGatewaySDKError is raised and httpx.AsyncClient / streamable_http_client are never called on a bad URL.
2. Craft MCPTool(url="https://attacker.example/collect", ...) and call AgentGatewayClient.call_mcp_tool — expect AgentGatewaySDKError before any HTTP call is made.
3. Confirm a legitimate tool URL (same host as gateway_url) passes through normally.
4. To exercise the LoB escape hatch: set ClientConfig(allowed_tool_hosts=("trusted-other.example",)) and verify a fragment pointing to that host is allowed while everything else is still rejected.

Checklist

• I have read the Contributing Guidelines
• I have verified that my changes solve the issue
• I have added/updated automated tests to cover my changes
• All tests pass locally
• I have verified that my code follows the Code Guidelines
• I have updated documentation (if applicable)
• I have added type hints for all public APIs
• My code does not contain sensitive information (credentials, tokens, etc.)
• I have followed Conventional Commits for commit messages
• (Security-sensitive changes only) I have run /security-review and addressed all ❌ findings

Breaking Changes

None. The validation only runs at call time; legitimate callers whose tool.url was always built from gateway_url are unaffected. The allowed_tool_hosts config field is additive with an empty default.

Additional Notes

The assignee's rebuttal (tool.url is not attacker-controlled in BAIP Managed Runtime because it is built by _build_mcp_url at binding time) — this fix is defense-in-depth: the control holds even if an upstream registry or Destination Service fragment is later poisoned, without any overhead on the happy path.

[ricardosrib]

Security Review: fix/agentgateway-pin-tool-url-origin

Branch: fix/agentgateway-pin-tool-url-origin → main
Verdict: ✅ No blocking issues — one ⚠️ worth addressing before merge

---

#	Criterion	Status	Finding
S1	Tenant subdomain validation	✅	_validate_tenant_subdomain called at agw_client.py:165 before any LoB function receives the subdomain
S2	URL construction (structured, not str.replace)	✅	_require_same_origin uses urlparse throughout; no string substitution
S3	Credential emptiness checks	➖	No new credential fields introduced
S4	JWT tokens verified before trust	➖	No JWT decoding in this diff
S5	No hardcoded credentials/secrets	✅	Test fixtures use obvious fakes; no real secrets
S6	Sensitive data not logged	✅	Warning logs include URL host and exception message, never the bearer token
S7	No path traversal from external input	➖	No file path operations
S8	HTTP base URLs validated	✅	Core of this PR — _require_same_origin called before every httpx.AsyncClient build
S9	Multi-tenant cache isolation	➖	No new caches
S10	New dependencies checked for CVEs	➖	Only re + urllib.parse (stdlib)
S11	No verify=False in HTTP calls	➖	Not present
S12	Audit log for mutating tenant operations	➖	Read/invoke path only, no tenant resource mutations

---

⚠️ Non-Blocking

[S8] Port normalization gap in allowed_tool_hosts (_url.py:99)

extra_allowed_hosts values are lowercased but not port-normalised. If a user configures allowed_tool_hosts=("other.example",) and a fragment URL carries an explicit default port (https://other.example:443/mcp), url_host becomes "other.example:443" while the allowed set contains "other.example" — they don't match and the fragment is silently rejected.

This doesn't weaken the security posture (it's stricter than intended, not looser), but it will confuse operators. The docstring already says host or host:port — the implementation just doesn't strip the default port from the URL side.

Suggested fix in _url.py:

_DEFAULT_PORTS = {"https": 443, "http": 80}

def _origin_host(parsed_host: str | None, parsed_port: int | None, scheme: str = "") -> str:
    host = (parsed_host or "").lower()
    # Drop the port if it is the scheme default — keeps comparison consistent
    # with entries in allowed_tool_hosts that omit the default port.
    if parsed_port is not None and parsed_port != _DEFAULT_PORTS.get(scheme):
        return f"{host}:{parsed_port}"
    return host

Then thread parsed.scheme / origin_parsed.scheme into the two _origin_host calls.

---

✅ Things Done Well

• Correct defence placement — _require_same_origin is the first statement in all three invocation functions, before the httpx.AsyncClient context manager. The token is never attached to a request that has not been cleared.
• LoB discovery fail-open with warning — off-origin fragments skip rather than abort, matching the existing "no URL property → skip" pattern. One poisoned fragment can't take down discovery for all other tools.
• Error messages never include the bearer token — confirmed across all raise sites in _url.py.
• _validate_mcp_id blocks all URL metacharacters — the ORD-ID-aware regex ([A-Za-z0-9._:-]+) excludes /, ?, #, %, @, and whitespace, so interpolated IDs can neither add path segments nor alter the origin.
• allowed_tool_hosts defaults to empty — strict pin by default; operators must explicitly opt out per deployment.

---

Unit Test Coverage

All required rejection cases are present in test_url.py and test_lob.py. The port normalization gap identified above has no test yet — a test for https://other.example:443 vs allowed_tool_hosts=("other.example",) would demonstrate the issue and verify the fix.