Skip to content

fix: restrict remote media domains#4734

Open
CUHKSZzxy wants to merge 7 commits into
InternLM:mainfrom
CUHKSZzxy:fix/allowed-media-domains-ssrf
Open

fix: restrict remote media domains#4734
CUHKSZzxy wants to merge 7 commits into
InternLM:mainfrom
CUHKSZzxy:fix/allowed-media-domains-ssrf

Conversation

@CUHKSZzxy

Copy link
Copy Markdown
Collaborator

Summary

  • Add an allowed_media_domains option for multimodal media URL fetching.
  • Enforce exact HTTP(S) media host allowlisting while preserving existing non-global IP blocking.
  • Revalidate manual redirects before fetching redirected media URLs.
  • Wire the setting through the API server, pipeline, async engines, and multimodal processor.
  • Add focused coverage for allowlist matches, rejected domains, unsafe IPs, host confusion, redirects, and multimodal parser forwarding.

Validation

  • Focused pytest coverage for media URL safety and multimodal content parsing passed.
  • Syntax checks for touched runtime modules passed.
  • Diff whitespace check passed.
  • Real API-server smoke tests passed for default media URL fetching and allowlisted media URL fetching; non-allowlisted media was rejected during prompt processing.

Assistance

Assisted with Codex + GPT-5.5 xHigh Fast, reviewed manually

Copilot AI review requested due to automatic review settings July 6, 2026 04:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an exact-host allowlist (allowed_media_domains) for HTTP(S) multimodal media fetching to reduce SSRF risk, and wires that setting from CLI/API entrypoints down into the multimodal processor and URL loader, including manual redirect handling with revalidation.

Changes:

  • Add allowed_media_domains plumbed through api_server/engines/pipeline and into MultimodalProcessor + load_from_url.
  • Enforce per-request redirect handling (allow_redirects=False) with explicit redirect loops and revalidation on each hop.
  • Add/extend tests for allowlist behavior, host confusion attempts, redirects, and forwarding of the allowlist through multimodal parsing; document the CLI flag (zh-cn).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_lmdeploy/test_vl/test_safe_url.py Adds/updates URL safety tests covering allowlist + redirects + host confusion.
tests/test_lmdeploy/test_content_merge.py Updates multimodal parsing tests and adds coverage for allowlist forwarding to URL loaders.
lmdeploy/vl/media/connection.py Implements allowed_media_domains enforcement and manual redirect revalidation for HTTP fetches.
lmdeploy/serve/processors/multimodal.py Stores/pass-through of allowed_media_domains into load_from_url for all modalities.
lmdeploy/serve/openai/api_server.py Adds allowed_media_domains parameter to server entrypoint and forwards to engine.
lmdeploy/serve/core/vl_async_engine.py Wires allowlist into VL async engine + prompt processor.
lmdeploy/serve/core/async_engine.py Wires allowlist into the default async engine prompt processor.
lmdeploy/pipeline.py Exposes allowlist on pipeline construction and forwards it to engine.
lmdeploy/cli/serve.py Adds --allowed-media-domains CLI flag and forwards it to server launch/serve.
lmdeploy/api.py Exposes allowlist on the public pipeline() API and forwards to Pipeline.
docs/zh_cn/multi_modal/multimodal_inputs.md Documents the new CLI option and exact-host matching semantics (zh-cn).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lmdeploy/vl/media/connection.py Outdated
Comment thread tests/test_lmdeploy/test_content_merge.py Outdated
Comment thread tests/test_lmdeploy/test_content_merge.py Outdated
Comment thread tests/test_lmdeploy/test_content_merge.py Outdated

## 本地文件与 Base64

当 API 服务面向不可信客户端时,建议在启动服务时使用 `--allowed-media-domains` 限制 HTTP(S) 媒体 URL 可访问的域名,降低 SSRF 风险。该参数按精确 hostname 匹配;例如 `--allowed-media-domains example.com` 只允许 `https://example.com/...`,不允许 `https://cdn.example.com/...`。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing the english document

Comment on lines 343 to 345
if isinstance(image, str):
image = load_image(image)
item = {'type': 'image_data', 'image_data': {'data': image}}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we use "allowed_media_domains" here?



def load_from_url(url: str, media_io: MediaIO[_M]) -> _M:
def load_from_url(url: str, media_io: MediaIO[_M], allowed_media_domains: list[str] | None = None) -> _M:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agent suggests:

Normalize allowed_media_domains at the enforcement point

parse_url() already lowercases the URL host, but allowlist entries are compared as-is (parsed.hostname not in allowed_media_domains). A config like --allowed-media-domains Example.COM would incorrectly reject https://example.com/....

# Conflicts:
#	lmdeploy/cli/serve.py
#	lmdeploy/serve/openai/api_server.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants