[iscekic]

Change

Adds one outbound transport for MCP gateway provider traffic (apps/web/src/lib/mcp-gateway/outbound.ts). It resolves the destination in the same call that opens the connection and binds the socket to that address through a custom lookup, while keeping the original hostname for the Host header and SNI. Each request is bounded by a 30s socket timeout, honors init.signal, and advertises and decodes gzip, deflate, and brotli response bodies before the Response is returned.

Routed through it:

• discovery metadata fetches (fetchJson, challenge metadata, authorization-server candidates)
• the remote endpoint probe in discoverRemoteProvider
• the dynamic registration POST in both registerDynamicProviderClient and createDynamicProviderCredentials
• the provider token exchange POST in handleProviderCallback

validatePublicHttpsDestination stays for the early checks that are not followed by a request: config creation (including authMode: none) and stored provider endpoints. The injection option is renamed fetchImpl -> requestImpl; production constructs the services without it, so every real request uses the bound transport.

Verification

• NODE_ENV=test pnpm jest src/lib/mcp-gateway --runInBand — 6 suites, 49 tests pass, including new unit tests that show each request connects to the address resolved for it, a compressed response decoded back to JSON, and an abort while the response is outstanding.
• pnpm --filter web run typecheck — passes.
• pnpm --filter web run lint — 0 errors.
• Live smoke on this VM against the real network: https://example.com/ fetched through the transport returns 200 with TLS and Host preserved; http:// is rejected; createDiscoveryService().discoverRemoteProvider('https://example.com/') completes over the transport; https://httpbin.org/gzip returns parsed JSON with the content-encoding header removed.

Proxy answers

The owner's proxy gave these answers in the owner's place.

1. Question: Transport: Which transport should the shared pinned path use?
   Answer: node:https request with pinned lookup (Recommended)
   Confidence: 0.60 · Precedent: Speed and least work: "do the least amount of work possible, with no drop in quality" (7f3470e0ba6f). Engineering conventions: prefer existing code paths. This matches the two pinned-DNS transports already in apps/web, keeps Host and SNI, and meets the brief's goal without adding undici.
2. Question: Test seam: How should tests keep the existing DI seam?
   Answer: Injectable requestImpl, renamed from fetchImpl (Recommended)
   Confidence: 0.70 · Precedent: Speed and least work: "skip work that doesn't need to be done" (00eb2a1d3e4b). Renaming the seam is far cheaper than rewriting about 15 DB-backed setups, and pinning is still proven against the real default.
3. Question: Precheck scope: Should the standalone validatePublicHttpsDestination calls that are not immediately followed by a request stay?
   Answer: Keep non-fetch prechecks (Recommended)
   Confidence: 0.75 · Precedent: The brief says to remove the precheck only "where the shared path replaces it". Removing all of them would drop validation at creation for authMode:none configs.
4. Question: Proof: Where should the address-binding proof live?
   Answer: New pure unit tests with a fake DNS resolver (Recommended)
   Confidence: 0.75 · Precedent: Proof and verification: "only run it for a targeted subset of files" (df491946927a). The brief asks for unit tests of the bound address. The DB-backed oauth-flow suite still runs for end-to-end proof on the PR.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental changes resolve both previous findings: the abort listener is now removed on request close, and the new test plus mock destroy behavior exercises the mid-flight abort path; no new issues in the changed lines.

Files Reviewed (3 files)

• apps/web/src/lib/mcp-gateway/outbound.ts
• apps/web/src/lib/mcp-gateway/outbound.test.ts
• apps/web/src/tests/helpers/outbound-https-mock.ts

Previous Review Summaries (2 snapshots, latest commit d746956)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit d746956)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The previous timeout and compression findings are fixed; the remaining gaps are the never-removed abort listener on the caller signal and an abort test that never exercises the mid-flight abort path.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
apps/web/src/lib/mcp-gateway/outbound.ts	274	Abort listener is added to init.signal but never removed, retaining completed requests and allowing a late destroy() after settle
apps/web/src/lib/mcp-gateway/outbound.test.ts	117	Abort test aborts before the call, so the addEventListener('abort', ...) path is not covered; the mock destroy emits no error

Files Reviewed (3 files)

• apps/web/src/lib/mcp-gateway/outbound.ts - 1 issue
• apps/web/src/lib/mcp-gateway/outbound.test.ts - 1 issue
• apps/web/src/tests/helpers/outbound-https-mock.ts - 0 issues

Fix these issues in Kilo Cloud

Previous review (commit 743f2e2)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The pinned outbound transport correctly closes the DNS-rebinding gap, but it drops the implicit timeout that fetch provided, so a stalled provider can hang the gateway request indefinitely.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/mcp-gateway/outbound.ts	150	New transport has no timeout/abort path; init.signal is ignored though the type accepts RequestInit

SUGGESTION

File	Line	Issue
apps/web/src/lib/mcp-gateway/outbound.ts	189	Response headers (including content-encoding) are copied verbatim while the body is never decompressed, unlike the previous fetch path

Notes

• Verified the core security intent: resolvePublicDestination resolves every A/AAAA answer, rejects if any is non-public, and requestPublicHttps pins that address via a custom lookup while keeping the original Host header and SNI, so a second lookup cannot swap in a private destination. IPv6 literals, IP-literal fast paths, and the family/servername handling are consistent between resolution and connection.
• Verified all remaining provider-URL requests in mcp-gateway (discovery metadata, challenge metadata, authorization-server candidates, endpoint probe, dynamic registration in both services, and token exchange) now go through the transport; no fetch-based call to a provider URL remains in non-test code, and no import of the moved validatePublicHttpsUrl / validatePublicHttpsDestination symbols is stale.
• The fetchImpl -> requestImpl rename is complete across services.ts, config-service.ts, provider-oauth-service.ts, and oauth-flow.test.ts.

Files Reviewed (10 files)

• apps/web/src/lib/mcp-gateway/outbound.ts - 2 issues
• apps/web/src/lib/mcp-gateway/outbound.test.ts - 0 issues
• apps/web/src/lib/mcp-gateway/discovery-service.ts - 0 issues
• apps/web/src/lib/mcp-gateway/discovery-service.test.ts - 0 issues
• apps/web/src/lib/mcp-gateway/provider-oauth-service.ts - 0 issues
• apps/web/src/lib/mcp-gateway/provider-oauth-token-exchange.test.ts - 0 issues
• apps/web/src/lib/mcp-gateway/config-service.ts - 0 issues
• apps/web/src/lib/mcp-gateway/services.ts - 0 issues
• apps/web/src/lib/mcp-gateway/oauth-flow.test.ts - 0 issues
• apps/web/src/tests/helpers/outbound-https-mock.ts - 0 issues

Fix these issues in Kilo Cloud

---

_{Reviewed by deepseek-v4.1-flash · Input: 39.9K · Output: 6.4K · Cached: 295.9K}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

bot: The whole MCP Gateway feature was removed from main after this branch was opened:

• 0763ba3c3d (chore/remove mcp gateway code, chore/remove mcp gateway code #8156) deleted apps/web/src/lib/mcp-gateway/, services/mcp-gateway/, packages/mcp-gateway/, the web routes and routers, and the specs.
• 2d93713b8a (chore(db): drop MCP Gateway tables, chore(db): drop MCP Gateway tables #8157) dropped the tables and their schema.

Every path this branch changes was deleted by #8156, so the conflict is modify/delete on all of them and can only resolve to an empty diff.

Closing without merging: there is nothing left on main for this branch to change, and no other files in the repository are involved.