fix(onboard): treat an unreachable custom endpoint as a transport failure - #6867
Conversation
…lure Choosing "Other OpenAI-compatible endpoint" and entering a DNS-unreachable base URL failed the SSRF preflight and silently looped back to provider selection: no probe guidance, no retry/back/exit prompt, and no clean exit. The credential-failure and other transport-failure paths already offer that recovery, but the preflight built a synthetic failure with curlStatus 0, so it classified as "unknown" and fell through to the silent selection loop. When the preflight fails because the host does not resolve (an unreachable or non-existent endpoint), mark the synthetic failure with curl's "could not resolve host" status (6). It then routes through the existing transport recovery — a DNS/VPN/endpoint-URL hint plus a retry/back/exit prompt with a clean exit — matching the credential path. A private-IP SSRF block keeps status 0 (it resolved fine; the address is just refused), so its behavior is unchanged. Fixes #6854 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCustom endpoint preflight failures now distinguish DNS-unresolved hosts from other blocked endpoints, routing unresolved hosts to transport recovery. A Vitest case verifies recovery classification and confirms endpoint probing is skipped. ChangesCustom endpoint recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 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 remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. 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. |
…ypecheck The #6854 regression test read `promptValidationRecovery.mock.calls[0][1]`, which fails typecheck:cli (tsconfig.cli.json includes tests): the zero-arg mock's call tuple has no index 1. Capture the recovery argument through a typed closure instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
cjagwani
left a comment
There was a problem hiding this comment.
Maintainer review: approved. The change preserves the SSRF fail-closed boundary, classifies only unresolved-host preflight failures as transport recovery, keeps private/internal address rejection unchanged, and adds regression coverage proving no endpoint probe occurs. Required CI and the exact-head E2E gate are green; DCO and commit verification pass; no actionable automated-review findings remain.
## Summary Add v0.0.83 release notes to `docs/about/release-notes.mdx` for pre-tag release prep. ## Source Summary - #6773 -> `docs/about/release-notes.mdx`: Shared inference route changes are explicit and fail-safe; status shows recorded route, live route, and drift. - #6875 -> `docs/about/release-notes.mdx`: DGX Station GB300 express setup restored; vLLM storage preflight narrowed. - #6770 -> `docs/about/release-notes.mdx`: Risky Spark vLLM server warning during onboarding. - #6856 -> `docs/about/release-notes.mdx`: Re-onboard reuse preserves tier-default brave/tavily presets. - #6867 -> `docs/about/release-notes.mdx`: Unreachable custom endpoint routed through transport-recovery path. - #6860 -> `docs/about/release-notes.mdx`: Rebuild preflight uses model-aware token field for o-series/GPT-5. - #6845 -> `docs/about/release-notes.mdx`: Corporate CA anchored for image build TLS. - #6833 -> `docs/about/release-notes.mdx`: SSH ControlMaster-delegated forwards recognized in fallback. - #6837 -> `docs/about/release-notes.mdx`: Hermes light skin writes via stdin on macOS. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release notes - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed - [ ] Non-success, skipped, or missing CI check accepted by maintainer ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub - [x] Normal pre-commit, commit-msg, and pre-push hooks passed - [x] `npm run docs` passes with 0 errors Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com> Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
Summary
During
nemoclaw onboard, choosing "Other OpenAI-compatible endpoint" and entering a DNS-unreachable base URL (https://example.invalid/v1) failed the SSRF preflight and silently looped back to provider selection — no probe guidance, no retry/back/exit prompt, no clean exit. This PR routes an unreachable endpoint through the existing transport-recovery path (DNS/VPN/URL hint + retry/back/exit + exit), matching the credential-failure path. The safety property is unchanged: no sandbox is created.Closes #6854.
Reproduction
Exercised the real compiled recovery path on our DGX Spark aarch64 test host (matching the reporter's DGX Spark aarch64), driving
validateCustomOpenAiLikeSelectionagainst a host that does not resolve and capturing the recovery classification handed to the prompt.Environment
main(v0.0.82)Observed on
main(before fix)Observed on
fix/...(after fix)Analysis
preflightCustomEndpointOrFail(src/lib/onboard/inference-selection-validation.ts) builds a synthetic probe failure for an SSRF-preflight failure withcurlStatus: 0andhttpStatus: 0.classifyValidationFailurehas no status to key on, so it returnskind: "unknown"(retry: "selection"), andpromptValidationRecoverytakes its final else-branch — printing "Please choose a provider/model again" and returning to the menu with no guidance and no exit option. The credential path and every real transport failure (curl exit 6/7/28, HTTP 429/5xx) instead getgetTransportRecoveryMessage+ a retry/back/exit prompt. A DNS-unreachable endpoint is genuinely a transport failure, but the synthetic failure never carried that signal.Fix
When the preflight reason is a host-resolution failure (
cannot resolve endpoint host …/did not resolve to any address), mark the synthetic failure with curl's "could not resolve host" status (6). It then classifies astransportand routes through the existing transport recovery — which already emits the "Check DNS, VPN, or the endpoint URL" hint (curl 6 branch) and the retry/back/exit prompt with a clean exit. A private-IP SSRF block keeps status 0 (it resolved fine; the address is just refused), so that path is unchanged. No new recovery UI is added — the fix reuses the existing transport contract.Changes
src/lib/onboard/inference-selection-validation.ts: classify an unresolved-host SSRF-preflight failure as a transport failure.src/lib/onboard/inference-selection-validation.test.ts: regression test that an unreachable endpoint yields a transport recovery.Type of Change
Verification
npm testpasses (touched files)AI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Summary by CodeRabbit