fix(onboard): authorize managed loopback vLLM endpoints - #8548
Conversation
The managed host-local runtime always publishes on 127.0.0.1, and the shared SSRF preflight admits an explicit loopback host without minting a trusted-private capability, so the unconditional capability demand could never pass and every managed vLLM onboard exited before creating a sandbox. A loopback endpoint now satisfies the check on its own, while any other host still requires a provenance-checked capability. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
📝 WalkthroughWalkthroughManaged vLLM authorization now permits loopback endpoints without trusted-private capability. Non-loopback endpoints still require trusted-private capability. Authorization runs before model discovery and validation. ChangesManaged vLLM authorization
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 57bc91b in the TypeScript / code-coverage/cliThe overall coverage in commit 57bc91b in the Show a code coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard/setup-nim-vllm.ts`:
- Around line 82-88: In src/lib/onboard/setup-nim-vllm.ts:82-88, move the
managed endpoint authorization using preflight and managedVllmValidationOptions
before queryVllmModels, and exit immediately when authorization fails so no
API-key-bearing request occurs. In
src/lib/onboard/setup-nim-vllm.test.ts:169-185, spy on queryVllmModels and
assert it is not called when authorization fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c12f7381-b556-4731-ae16-015f27ff2620
📒 Files selected for processing (2)
src/lib/onboard/setup-nim-vllm.test.tssrc/lib/onboard/setup-nim-vllm.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer follow-up is complete on branch revision
Fresh required checks and independent human review are still pending. No gate is waived. |
Summary
The managed host-local vLLM runtime always publishes on
127.0.0.1, and the shared SSRF preflight admits an explicit loopback host without minting a trusted-private capability, so onboarding's unconditional capability demand could never pass and every managed vLLM install exited before creating a sandbox. A loopback managed endpoint now satisfies the check on its own, while any other host still requires a provenance-checked capability.Related Issue
Fixes #8539
Changes
managedVllmValidationOptionsinsrc/lib/onboard/setup-nim-vllm.tssplits one condition into two. A preflight rejection still fails closed. The trusted-private capability is now demanded only when the endpoint host is not loopback, becauseassertEndpointResolvesPublicreturns early for an explicit loopback host and issues no capability for it — a capability records which private addresses may appear in a curl--resolvepin, and loopback needs no pin. The truthiness test is replaced byisTrustedPrivateEndpointCapability, so a forged object no longer satisfies the check.pinnedAddresseskeeps passingpreflight.addresses ?? []. The empty array is load-bearing:src/lib/adapters/http/probe.tstreats a defined value, including[], as proof that the preflight ran and strips every proxy environment variable, so the credentialed loopback probe is not routed through an ambient proxy.src/lib/onboard/setup-nim-vllm.test.ts. The first drives a loopback managed binding through the handler and asserts the validation options it forwards; it fails against the unfixed code. The second holds the other side of the gate by asserting that a managed endpoint which is neither loopback nor operator-trusted private still fails closed. Neither test resolves DNS.trustedPrivateEndpointflag. Gating on that flag would also have admitted a managed endpoint on a public address, which the previous code rejected. There are three managed-binding producers, and the managed-cluster one derives its base URL from a discovered fabric interface address that is not constrained to private space, so that widening would have been a real behaviour change rather than a theoretical one. Scoping to loopback fixes this defect and widens nothing.10.40.0.1.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededdocs/CONTRIBUTING.mdand found none met: no flag is added, removed, or renamed, no default or configuration changes, and the pages describe the flow this fix restores rather than describing it incorrectly. It found the error string absent fromdocs/, with the only near hit indocs/reference/troubleshooting.mdxbeing an unrelated WSL message, and no vLLM onboarding-failure entry to stay consistent with. It confirmed that the managed-endpoint address statements indocs/inference/choose-local-inference-server.mdxanddocs/inference/set-up-vllm.mdxdescribe where the server is published rather than an authorization rule, and that the trusted-private posture indocs/inference/custom-endpoint-security.mdxis scoped to custom endpoints and disclaims managed provider defaults, so no published claim is contradicted. It also checkeddocs/reference/commands.mdx, the remainingdocs/inference/vLLM pages, all pages underdocs/security/, anddocs/get-started/, and confirmed that neitherCONTRIBUTING.mdnorAGENTS.mddocuments the preflight capability pattern, so nothing there is stale. The review additionally rejected an earlier version of this change that gated ontrustedPrivateEndpoint, and its finding produced the loopback-scoped gate in this commit.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project cli src/lib/onboard/setup-nim-vllm.test.ts— 34 passed.npx vitest run --project cliacross the eight further suites reaching this handler, includingsetup-nim-flow,vllm-menu,provider-selection,inference-providers/vllm-local, bothprovider-inferencehandlers, andrebuild-local-provider-recreate— 123 passed.npx vitest run --project integration test/onboard-selection-vllm.test.ts— 7 passed.npx vitest run --project installer-integration test/install-express-prompt.test.ts— 84 passed.npm run typecheck:cliandnpm run build:cli— clean.test/vllm-docker-storage.test.tsskips without Docker and was not run.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable. This changes one private function in a single onboarding module and adds no harness mechanics — no Vitest config, setup file, project glob, or registration topology.npm run checks:repositorypassed, including the layer import boundary, source architecture budget with zero cycles, and Vitest project membership.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit