Skip to content

[Security] Restrict EC NIXL reads to configured producer peers - #65024

Open
jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:codex/validate-ec-producer-peers
Open

jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:codex/validate-ec-producer-peers

Conversation

@jperezdealgaba

@jperezdealgaba jperezdealgaba commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Require CPU EC NIXL consumers to authorize producer endpoints before allocating cache entries or opening connections. ec_allowed_peers is a server-side exact host:port allowlist and defaults to denying remote reads.

Claims

  • A request cannot introduce a producer outside the operator's configured allowlist, even with the correct image hash and embedding shape.
  • Unapproved sources retain existing failure behavior: local encoding when media is available, request failure for metadata-only inputs.
  • Approved producer transfers and local CPU offload remain supported. Existing NIXL deployments must configure their producer addresses.

Validation

  • Changed-file pre-commit checks: passed, including Ruff, mypy, Markdown, and Buildkite test tethering.
  • git diff --check: passed.
  • Local scheduler pytest execution was blocked on macOS by the Linux /dev/shm dependency; the normal test cleanup also crashed in PyTorch's empty_host_cache. No passing runtime results are claimed.
  • At the maintainer's request, Linux/GPU validation will run in CI. The two-GPU EC job installs the KV connector dependencies and runs:
pytest -v -s \
  v1/ec_connector/unit/test_scheduler_nixl_consumer.py \
  v1/ec_connector/integration/test_nixl_failure.py

The new HTTP regression uses the producer's actual image hash without a client UUID. Metadata-only input must fail and pixel input must fall back locally, with neither contacting the unapproved control peer. A later request uses a different salt without transfer parameters. Existing integration cases verify failures from an approved peer and successful subsequent native NIXL transfers.

GPU regression results, before/after failure evidence, and model-output evaluation are pending CI. This PR should not merge until that validation and human line-by-line review are complete.

Details

Request-selected EC coordinates previously flowed from API input to _start_xfer, which allocated an entry under the image hash and connected to that producer. The guard now runs before both side effects. Every frontend using this CPU EC consumer reaches the same scheduler guard: Python chat/completions/Responses/Anthropic, scale-out generate, Rust frontend, SageMaker/batch forwarding, and direct engine clients. Streaming and non-streaming share this path. Other connectors are unchanged.

The allowlist establishes operator authorization of the endpoint, not transport authentication or verification of embedding contents. Allowed producers, their DNS, and the inter-node network remain trusted. Producer compromise is outside this control.

Duplicate-work check: reviewed the diffs of #58853, #56814, #54324, #55290, #59875, #57333, #61764, #50353, #64721, and #65018. They address ack shape/error containment, extra-argument typing, serialization, resource cleanup, metrics, port allocation, request copying, or forwarded/rendered features. None restricts producer selection before CPU EC reads. Current main retains the unrestricted sink.

AI assistance was used to develop this PR.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: The control is confined to the opt-in CPU EC connector.

  • Testing and Validation: Linux/GPU regression and model-output validation pending CI; local environment limitations described above.

  • Code Quality and Style: Changed-file checks pass; feature documentation includes the default and migration requirement.

  • Pull Request Contents: Includes root cause, coverage, duplicate checks, tradeoffs, and validation limitations.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
@jperezdealgaba
jperezdealgaba requested a review from a team November 1, 2026 20:54

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify

mergify Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--65024.org.readthedocs.build/en/65024/

@mergify mergify Bot added documentation Improvements or additions to documentation ci/build cpu Related to CPU backends kv-connector nixl NIXL KV connector / multi-tiered KV offloading (KVCR) labels Nov 1, 2026
@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #99020 for commit db8807c67351.

@mergify

mergify Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jperezdealgaba.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Nov 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build cpu Related to CPU backends documentation Improvements or additions to documentation kv-connector needs-rebase nixl NIXL KV connector / multi-tiered KV offloading (KVCR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant