Skip to content

Harden SSRF guard for get-image-metadata - #138

Merged
alexmontesg merged 7 commits into
mainfrom
bugfix/GH-136-fixmcp-harden-ssrf-gua
Oct 5, 2026
Merged

alexmontesg merged 7 commits into
mainfrom
bugfix/GH-136-fixmcp-harden-ssrf-gua

Conversation

@alexmontesg

Copy link
Copy Markdown
Contributor

Summary

Closes #136.

get-image-metadata (public MCP tool) accepted destinations the guard did not cover. This PR:

  • Adds an IP classifier (code/src/utils/ssrf.ts) rejecting loopback, RFC 1918, link-local, CGNAT, ULA (fc00::/7), fe80::/10, multicast/reserved, NAT64 and IPv4-mapped/compatible IPv6.
  • Makes assertSafeUrl use it, so IPv6 literals are covered everywhere it is used.
  • Adds safeFetchBuffer: resolves the host, rejects if any address is blocked, and connects only to a validated address (single lookup, no rebinding window). No redirects, 10 MB cap, 10 s timeout. No new dependency.
  • Blocked destinations return the same generic error, without the resolved IP.

Independent of the MCP authorization PR (#137).

Verification

  • npm run lint and npm run build pass.
  • Local harnesses: mapped/ULA/link-local/metadata/localhost literals and hostnames resolving to private or mixed addresses are rejected and the local server is never hit; a public image still returns metadata; redirect, oversize and timeout fail; DNS pinning does a single lookup.

Residual risk

transformMediaUrl (canvas/weave.ts) is literal-only, since the canvas runtime performs that fetch and cannot be validated here.

Reject IPv6 private, link-local, ULA and IPv4-mapped destinations, and
validate the IPs a hostname resolves to. Fetch connects only to the
validated address, without following redirects.

Refs #136

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Comment thread code/src/utils/ssrf.ts Fixed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@alexmontesg
alexmontesg marked this pull request as ready for review October 5, 2026 08:51
@alexmontesg
alexmontesg requested a review from a team as a code owner October 5, 2026 08:51
@alexmontesg alexmontesg added internal Internal changes out of the scope for changelog files and removed internal Internal changes out of the scope for changelog files labels Oct 5, 2026
@alexmontesg alexmontesg self-assigned this Oct 5, 2026
@alexmontesg alexmontesg added the bug Something isn't working label Oct 5, 2026
@alexmontesg
alexmontesg requested a balanced review from Copilot October 5, 2026 08:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Destination filtering remains incomplete, and timeout and rejection paths do not reliably bound resource consumption.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
What changed in this PR

Hardens image fetching for the public get-image-metadata MCP tool to address SSRF issue #136, independently of authorization changes.

Changes:

  • Adds shared IP filtering and DNS-validated connections.
  • Introduces redirect rejection, response-size checks, and timeout handling.
  • Migrates metadata fetching while preserving the existing URL-guard export.
File Description
code/​src/​utils/​ssrf.ts Adds destination validation and guarded fetching.
code/​src/​utils.ts Re-exports the shared URL guard.
code/​src/​mcp/​tools/​get-image-metadata.ts Uses the guarded image fetcher.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread code/src/utils/ssrf.ts
Comment thread code/src/utils/ssrf.ts Outdated
Comment thread code/src/utils/ssrf.ts Outdated
Comment thread code/src/utils/ssrf.ts Outdated
Block site-local, local-use NAT64, IPv4-translated, 6to4 and Teredo
ranges, reject every IPv4-mapped address after canonicalizing it, destroy
the request on early rejections and enforce a total fetch deadline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Confirmed destination-validation and response-handling defects affect security, reliability, and image compatibility.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Decode Content-Encoding Before Image Processing

code/​src/​utils/​ssrf.ts:197

Unlike the previous fetch implementation, http.request does not decode HTTP content encodings. A successful PNG response with Content-Encoding: gzip returns compressed bytes here, which are passed directly to sharp and fail image detection. Decode supported content encodings before returning the image, enforcing maxBytes on the decoded stream as well to prevent decompression bombs.

Comment thread code/src/utils/ssrf.ts Outdated
Comment thread code/src/utils/ssrf.ts
Allow only global unicast IPv6 minus special-purpose ranges, reject protocol
upgrades, decode gzip/deflate/br with the size cap on decoded bytes and
reject (not resolve) when the body exceeds the cap.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@alexmontesg

Copy link
Copy Markdown
Contributor Author

Reply to the 'previously missed' note (Content-Encoding): valid. Node's http.request does not decode, unlike fetch. Fixed in 1e59e12: gzip/deflate/br are decoded, unknown encodings are rejected, and maxBytes applies to the decoded stream (a 50 MB gzip bomb is rejected at the cap). While testing this I also found that exceeding the cap while streaming resolved with an empty buffer instead of rejecting; that is fixed too.

Comment thread code/src/utils/ssrf.ts Fixed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The security-critical fetcher needs human review, and verified decompression work continues after size-limit rejection.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)

Comment thread code/src/utils/ssrf.ts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

@alexmontesg alexmontesg changed the title fix(mcp): harden SSRF guard for get-image-metadata Harden SSRF guard for get-image-metadata Oct 5, 2026
@alexmontesg
alexmontesg merged commit 24fa2a9 into main Oct 5, 2026
11 checks passed
@alexmontesg
alexmontesg deployed to azure-develop October 5, 2026 09:51 — with GitHub Actions Active

This branch was successfully deployed

1 active deployment
azure-develop — cc0c0315 Deployed Oct 5, 2026 by alexmontesg via Deploy to Container Apps #498
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden SSRF guard for get-image-metadata

4 participants