fix(llama-cpp): accept public /v1/models with protected /props

[yanyunl1991]

Summary

NemoClaw now accepts a public llama.cpp /v1/models catalog only when anonymous /props remains protected.
This corrects attachment for API-key-protected llama.cpp servers without changing the fixed loopback or native server fingerprint requirements.

Related Issue

Fixes #8302.
The accepted scope comes from #8161 and RFC #7636.

Changes

• Accept HTTP 200 from the anonymous /v1/models probe.
• Require anonymous /props to return HTTP 401 or 403 before creating the authenticated request.
• Reject a public, missing, timed-out, oversized, or incompatible /props response without sending the credential.
• Preserve loopback port 8081, response bounds, model-alias validation, and authenticated native server fingerprint checks.
• Add positive and negative behavior tests for public /v1/models.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• 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:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: Existing documentation already requires an API key, loopback port 8081, bounded probes, consistent native server evidence, and generic-endpoint fallback. Commands, settings, credential lifecycle, ownership, and procedures do not change.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: An independent nine-category review found no findings for PR commit 6321b7fbb292986d2bc82772a55260ddcb49a92d against base SHA 422fe9545ed35dbf7d7976bd719239b5430346aa.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: No waiver is recorded.

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: Reviewed the complete two-file change for PR commit 6321b7fbb292986d2bc82772a55260ddcb49a92d against base SHA 422fe9545ed35dbf7d7976bd719239b5430346aa. Existing documentation remains accurate because API-key authentication, loopback port 8081, bounded probes, native fingerprint evidence, and generic-endpoint fallback remain required. The change does not alter commands, configuration, credential lifecycle, ownership, or procedures.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh does not change.
• Station profile/scenario: Not applicable.
• Result: Not applicable.
• Supporting evidence: Not applicable.

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub — all 19 commits appear as Verified.
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable — normal hooks passed for the change set and current merge commits.
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — the GitHub Actions test jobs passed for the current PR commit. No standalone local test suite ran after the merge.
• Applicable broad gate passed — the required checks, including E2E / PR Gate, passed for the current PR commit.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed — the independent security review passed all nine categories.
• npm run docs builds without warnings (doc changes only) — not applicable; no documentation changes.
• Doc pages follow the style guide (doc changes only) — not applicable; no documentation changes.
• New doc pages include SPDX header and frontmatter (new pages only) — not applicable; no new page.

Sensitive-Path Security Review

Independent review found no security finding for PR commit 6321b7fbb292986d2bc82772a55260ddcb49a92d against base SHA 422fe9545ed35dbf7d7976bd719239b5430346aa.

1. Secrets and credentials: PASS. Anonymous probes carry no credential. Authenticated probes retain the owner-readable temporary curl configuration and finally cleanup.
2. Input validation and data sanitization: PASS. Fixed loopback and port checks, bounded responses, status constraints, native metadata validation, and alias validation remain.
3. Authentication and authorization: PASS. Public catalog access alone is insufficient; anonymous /props must show authentication enforcement before credential use.
4. Dependencies and third-party libraries: PASS. No dependency, image, lockfile, or downloaded artifact changes.
5. Error handling and logging: PASS. New failures contain neither credential values nor response bodies and stop attachment.
6. Cryptography and data protection: PASS. No cryptographic behavior changes. Loopback-only transport, disabled redirects, and disabled ambient proxies remain.
7. Configuration and security headers: PASS. No permission, port, network, header, or debug setting changes.
8. Security testing: PASS. Tests cover public and protected endpoints, incompatible metadata, missing /props, timeout, oversized responses, and stopping after anonymous probes.
9. System security: PASS. Fail-closed selection remains, and the change adds no lifecycle effect, alternate destination, recovery behavior, or ownership claim.

Required checks and maintainer approval remain separate merge requirements.

GitHub Actions

GitHub Actions is authoritative for PR commit 6321b7fbb292986d2bc82772a55260ddcb49a92d against base SHA 422fe9545ed35dbf7d7976bd719239b5430346aa.
All 46 checks in the current GitHub snapshot are successful, skipped as designed, or neutral. The required checks and E2E / PR Gate passed.
No check waiver is recorded.

---

Signed-off-by: Yang Liu yanyunl@nvidia.com

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The llama.cpp attachment probe now accepts public HTTP 200 /v1/models responses. It then checks /props for authentication evidence. Tests cover valid llama.cpp metadata, vLLM rejection, unprotected /props, and missing protected /props.

Changes

llama.cpp detection

Layer / File(s)	Summary
Public catalog probe and validation src/lib/inference/llama-cpp/index.ts	Anonymous /v1/models probes accept HTTP 200 while preserving failure classifications. HTTP 200 responses trigger an anonymous /props probe that requires HTTP 401 or 403 authentication evidence.
Public catalog classification tests src/lib/inference/llama-cpp/index.test.ts	Tests verify successful native attachment, vLLM rejection as not-llama-cpp, authentication-required for unprotected /props, and not-llama-cpp for missing protected /props.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: area: inference, area: providers, bug-fix

Sequence Diagram(s)

sequenceDiagram
  participant probeLlamaCppAttachment
  participant v1_models
  participant props
  probeLlamaCppAttachment->>v1_models: Anonymous GET /v1/models
  v1_models-->>probeLlamaCppAttachment: HTTP 200 model catalog
  probeLlamaCppAttachment->>props: Anonymous GET /props
  props-->>probeLlamaCppAttachment: HTTP 401/403
  probeLlamaCppAttachment-->>probeLlamaCppAttachment: Accept native llama.cpp attachment

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR meets all requirements from #8302: accepts public anonymous /v1/models with HTTP 200, verifies protected endpoints require authentication (/props, /metrics, /v1/chat/completions), rejects vLLM and publicly-exposed protected endpoints, and enables genuine llama.cpp server attachment.
Out of Scope Changes check	✅ Passed	All changes are scoped to the llama.cpp server attachment probe logic; test expansions and implementation adjustments align with the requirement to support public /v1/models catalogs on authenticated servers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: accepting public /v1/models with protected /props for llama.cpp.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/8302-llama-cpp-public-v1-models

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 6321b7f in the fix/8302-llama-cpp-p... branch remains at 96%, unchanged from commit 422fe95 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 6321b7f in the fix/8302-llama-cpp-p... branch remains at 82%, unchanged from commit 422fe95 in the main branch.

Show a code coverage summary of the most impacted files.

File	main 422fe95	fix/8302-llama-cpp-p... 6321b7f	+/-
src/lib/inferen...ma-cpp/index.ts	95%	94%	-1%
src/lib/sandbox...rce-identity.ts	87%	87%	0%
src/lib/tunnel/services.ts	80%	80%	0%

---

_{Updated August 06, 2026 01:40 UTC}

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.

3 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

• authentication evidence at src/lib/inference/llama-cpp/index.ts:251: primary classified it as define; the second opinion classified it as justified.
• protected llama.cpp at src/lib/inference/llama-cpp/index.ts:251: selected only by the second-opinion lane as conflict.
• anonymous /props at src/lib/inference/llama-cpp/index.test.ts:250: selected only by the second-opinion lane as justified.

1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

• llama-cpp-dgx-spark-qualification: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• justified — protected llama.cpp endpoint at src/lib/inference/llama-cpp/index.ts:251: Keep this term. Its modifier distinguishes the endpoint that must require authentication from the permitted public catalog.
• define — authentication evidence at src/lib/inference/llama-cpp/index.ts:251: State the accepted HTTP 401 or 403 responses in the error text or nearby explanatory text.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Cover the accepted anonymous `/props` 403 branch

• Location: src/lib/inference/llama-cpp/index.test.ts:180
• Category: tests
• Problem: The new public-catalog success test supplies HTTP 401 from anonymous `/props`, but no success test supplies HTTP 403.
• Impact: A change that accidentally rejects HTTP 403 would break attachment to a server that correctly denies anonymous access with that status.
• Recommendation: Add a public `/v1/models` success case with anonymous `/props` returning HTTP 403, then verify the authenticated native fingerprint succeeds.
• Verification: Inspect `probeLlamaCppAttachment` at `src/lib/inference/llama-cpp/index.ts:252-253` and run the focused test file after adding the HTTP 403 success case.
• Test coverage: A scripted probe sequence with anonymous `/v1/models` HTTP 200, anonymous `/props` HTTP 403, and valid authenticated fingerprint responses returns `{ ok: true, model }`.
• Evidence: `src/lib/inference/llama-cpp/index.ts:252-253` accepts anonymous `/props` status 401 or 403. `src/lib/inference/llama-cpp/index.test.ts:180-200` has the only added successful public-catalog path and uses status 401. `src/lib/inference/llama-cpp/index.test.ts:284-296` covers 401/403 only for an authenticated model-catalog rejection, not this anonymous `/props` branch.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[apurvvkumaria]

Maintainer security review — PASS\n\nReviewed revision 47bdfc0 with the repository security-review checklist. PRA-1 is addressed: when the model catalog is public, attachment now requires an anonymous /props request to return 401 or 403 before any credential-bearing request is made. A server that leaves /props unprotected is rejected, and focused regression coverage passes 29/29.\n\nNo open security findings remain.\n\n1. Secrets and credentials — PASS: no secrets were added; the credential remains in the protected curl configuration and is not sent during either anonymous probe.\n2. Input validation — PASS: the fixed loopback and port restriction, bounded responses, and safe served-alias validation remain intact.\n3. Authentication and authorization — PASS: the public-catalog path now proves that a protected llama.cpp endpoint enforces the supplied key.\n4. Dependencies — PASS: no dependency changes.\n5. Error handling and logging — PASS: failures are explicit and do not include credential material.\n6. Cryptography and data protection — PASS: no cryptographic behavior changed.\n7. Configuration and security headers — PASS: attachment remains restricted to loopback port 8081.\n8. Security testing — PASS: the new negative test rejects a native-looking server whose protected endpoint is publicly accessible.\n9. Holistic posture — PASS: the supported stock llama.cpp flow is restored without weakening the existing-server authentication guarantee.\n\nFiles reviewed: src/lib/inference/llama-cpp/index.ts and src/lib/inference/llama-cpp/index.test.ts. Commit hooks, pre-push type checking, focused tests, and the documentation build passed. Documentation writer result: no docs needed because no command, setting, credential lifecycle, or user procedure changed.

[coderabbitai]

Actionable comments posted: 2

🤖 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/inference/llama-cpp/index.ts`:
- Around line 252-257: Update the `/props` status handling around the
anonymousProps check to return `not-llama-cpp` for every unrecognized status,
including 404, 405, and 500; only the specific public-success status should
produce `authentication-required`. Add a regression test covering `200
/v1/models` followed by `404 /props` and assert the `not-llama-cpp` result.
- Around line 242-258: Update the authenticated probe flow around
resolveFixedLoopbackBaseUrl and the trusted curl configuration so the bearer
token is never sent over HTTP. Use a protected transport or equivalent local
credential channel for the llama.cpp authentication probe, while preserving the
existing protected-endpoint validation and failure behavior.

🪄 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: 24b82f99-31fc-4110-858d-09c6872c9ef9

📥 Commits

Reviewing files that changed from the base of the PR and between 616ee55 and 47bdfc0.

📒 Files selected for processing (2)

• src/lib/inference/llama-cpp/index.test.ts
• src/lib/inference/llama-cpp/index.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/inference/llama-cpp/index.test.ts

[apurvvkumaria]

Maintainer follow-up:\n\n- The previous cli-test-shards (4) failure was limited to test/mcp-lifecycle-lock.test.ts: the test expected containment-active but observed a mutation-lock timeout. This branch does not change that subsystem, and the focused integration file passes 38/38 locally. The run was superseded by the current revision, so I did not rerun that old workflow.\n- The actionable CodeRabbit classification finding is fixed in 2506429: a missing or otherwise unsuccessful anonymous /props endpoint now returns not-llama-cpp, while a successful public response still returns authentication-required. The new regression is included and the llama.cpp suite passes 30/30.\n- The loopback-transport finding was reviewed and dispositioned in its thread. This PR preserves the established local port 8081 contract, does not follow redirects, keeps the bearer value in an owner-readable temporary curl configuration, and now proves the protected endpoint rejects anonymous access before sending the credential.\n\nBoth review threads are resolved. The follow-up commit is signed and GitHub-verified, pre-push type checking passed, and the documentation writer revalidated the revision as no-docs-needed.

[apurvvkumaria]

Advisor classification for the current branch revision: the Terra lane stopped during its terminology-analysis turn because it produced no required analysis result. The Nemotron lane completed successfully, and the publisher completed normally. This failure occurred inside the advisor workflow and is not associated with the llama.cpp code or regression tests.

I classified it as a transient advisor-runtime failure and restarted only the failed Terra job once. The remaining CI and protected E2E checks are still running. No merge will occur without all required gates and independent approval.

[apurvvkumaria]

E2E failure classification: infrastructure, not caused by this PR.

Both selected targets stopped before their test bodies while restoring the validated CLI artifact. The restored package is missing nemoclaw/dist/shared/sandbox-name.cjs, so the CLI exits with MODULE_NOT_FOUND from dist/lib/name-validation.js.

This is the shared artifact-boundary defect being repaired by #8333. I am not rerunning the unchanged workflow because it would reproduce the same missing-module failure. Once that repair lands, I will refresh this PR’s protected E2E gate and continue monitoring it.

[apurvvkumaria]

CI classification: the required CLI shard timed out after 15 seconds in test/langchain-deepagents-code-nemotron-profile-plugin.test.ts while building an unrelated plugin wheel. This PR changes only the llama.cpp inference implementation and its focused tests, and the other seven CLI shards passed.

The failure is outside the changed code and is consistent with a timing-sensitive test under runner load. I am using the single bounded failed-job rerun for this classification and will continue monitoring. If it repeats, I will stop retrying and document it as a reproducible blocker.

[apurvvkumaria]

The bounded retry passed: the previously timed-out CLI shard is green, the aggregate CLI coverage and required checks are green, and the repository has started a fresh E2E gate evaluation. This confirms the earlier failure was transient and not caused by the llama.cpp change. I am continuing to monitor E2E and review readiness.

[cv]

Protected E2E failure classification: infrastructure, not caused by this PR. The selected network-policy test reached phase 3, then ClawHub returned HTTP 503 with “Rate limit temporarily unavailable” while installing the pinned OpenClaw plugin. This PR changes only the llama.cpp attachment probe and its unit tests; the selected inference-routing lane passed. I restarted only the failed trusted E2E job once and will not waive the gate or retry again without new evidence.

[apurvvkumaria]

E2E classification for the current branch: external service failure, not caused by this PR.

The selected inference-routing target passed. network-policy completed onboarding and policy setup, then failed while installing the unrelated @openclaw/sherpa-onnx-tts package because ClawHub returned HTTP 503: Rate limit temporarily unavailable. The PR changes only the llama.cpp attachment probe and its focused tests.

The protected E2E rollup has not accepted a passing result. I am not manually restarting the controller-owned child run. This revision remains blocked on a fresh repository-authorized E2E result and an independent approval; it will not be merged before both gates pass.

[apurvvkumaria]

Advisor classification for the current branch: transient advisor-runtime failure, not a code finding.

The primary Terra lane completed with no blockers, warnings, or suggestions. The Nemotron lane preserved zero findings but failed its outcome check because the model omitted a required terminology-tool result and emitted text before that tool completed. This occurred inside the advisor protocol after the repository analysis ran; it is unrelated to the llama.cpp implementation or tests.

I am using the single bounded failed-job retry for this advisor run. If the failure repeats, I will stop retrying and document it as an external blocker.

[cv]

The failed-job rerun stopped at the artifact identity check. The provenance record names workflow attempt 1, but the rerun used attempt 2. The check rejected that mismatch before it downloaded or executed PR code.

I will not rerun the child workflow or bypass this trust boundary. The repository must dispatch E2E for a new PR commit after the branch is updated from current main. The previously reported application failure remains the external ClawHub HTTP 503.

[apurvvkumaria]

The bounded advisor retry passed. Both advisor lanes and the publisher are green, with no blocking findings reported. No further advisor retry will be used for this revision.

The PR remains blocked by the required E2E rollup after the unrelated ClawHub HTTP 503 and by the lack of an independent approval. All other reviewed gates remain green.

[apurvvkumaria]

Current branch revalidation after the new test-only commit:

• The added timeout and oversized-response cases are signed, GitHub Verified, and directly close the security review’s prior advisory coverage gap.
• The documentation and nine-category security receipts now cover the current branch.
• All 67 CI checks are green, including the protected E2E gate. Both advisor lanes are also green with zero findings.

The repository merge-gate checker still fails closed on advisor provenance because its validator currently requires a pull_request workflow event, while the trusted advisor workflow records pull_request_target. This is a repository gate-tooling mismatch, not a finding in this PR. The PR also still requires an independent approval. I am not merging until both repository readiness signals are satisfied.

[apurvvkumaria]

Priority follow-up: the current branch revision is fully green across required CI, protected E2E, code scanning, both review-advisor lanes, documentation review, DCO, commit verification, and the independent sensitive-path review. Both prior review threads are resolved. No PR-related technical blocker remains. The remaining gates are independent repository approval and the active merge freeze; no merge will be attempted until the freeze is explicitly lifted.

[apurvvkumaria]

Refreshed this branch onto current main with a conflict-free signed merge. The contributor patch is unchanged, and contributor attribution is preserved.

Validation completed before push:

• focused llama.cpp suite: 32 tests passed
• documentation writer review: no-docs-needed
• sensitive-path review: all nine categories pass; the fixed loopback, bounded anonymous probes, protected /props check, and credential handling remain unchanged
• merge commit: GitHub-verified with DCO sign-off; normal commit and pre-push hooks passed

The PR description now records the current documentation and security review receipts. Fresh CI and automated review checks are running; no check is waived.

[apurvvkumaria]

Priority revalidation is complete for this branch revision. Ordinary CI and code scanning are green; the protected inference-routing and network-policy E2E checks passed; the bounded advisor retry completed with both model lanes green and zero findings; documentation, sensitive-path review, DCO, and GitHub verification are current. The remaining gates are independent approval, reconciliation after the release cut stabilizes main, and the active merge freeze. No merge will be attempted until the freeze is explicitly lifted.

[apurvvkumaria]

Release status update: v0.0.103 has been cut and the merge freeze is lifted. The completed technical, documentation, security, and review-thread evidence remains current. The remaining gate is independent approval; once approved, I will reconcile the branch with current main, run the fresh repository-controlled gates, and use the normal protected merge path.

[cv]

Approved for PR commit 6321b7fbb292986d2bc82772a55260ddcb49a92d against base SHA 422fe9545ed35dbf7d7976bd719239b5430346aa.

• Accepted issue #8161 and RFC #7636 establish the product scope.
• The correctness review and all nine security categories passed.
• The documentation writer review concluded no-docs-needed.
• All 46 current checks, including E2E / PR Gate, passed without a waiver.
• The DCO declaration is present, and all 19 commits appear as Verified.
• Both review threads are resolved.