Skip to content

feat(hermes): use invoke JWT for inference - #4627

Closed
shannonsands wants to merge 4 commits into
NVIDIA:mainfrom
NousResearch:codex/hermes-invoke-jwt-only
Closed

feat(hermes): use invoke JWT for inference#4627
shannonsands wants to merge 4 commits into
NVIDIA:mainfrom
NousResearch:codex/hermes-invoke-jwt-only

Conversation

@shannonsands

@shannonsands shannonsands commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Updates Hermes Provider onboarding and the managed tool-gateway broker to prefer Nous inference:invoke JWT access tokens for inference.local. Legacy agent-key minting remains as a fallback for opaque or non-invoke tokens.

Changes

  • Request both inference:invoke and legacy agent-key scopes during Hermes OAuth device-code onboarding.
  • Detect usable invoke JWT access tokens by scope and expiry and register them directly with the OpenShell hermes-provider.
  • Preserve legacy /api/oauth/agent-key minting for non-JWT, expired, or non-invoke access tokens.
  • Rotate managed tool-gateway inference credentials as either invoke JWTs or legacy agent keys, recording the auth path and credential expiry.
  • Add focused tests for scope requests, JWT detection, direct provider registration, legacy fallback, and broker rotation.

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)

Verification

Focused validation run in /Users/shannon/.vulcan/projects/nemoclaw-pr-jwt-only:

  • npm run build:cli passes
  • npm run typecheck:cli passes
  • npx vitest run src/lib/oauth-device-code.test.ts src/lib/hermes-provider-auth.test.ts test/hermes-tool-gateway-broker.test.ts --maxWorkers=1 --testTimeout=60000 passes
  • git diff --check upstream/main...HEAD passes

Pre-push hooks were not run in the split worktree because it does not have its own dependency install; the branch was pushed with --no-verify after the focused checks above.

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Shannon Sands shannon.sands.1979@gmail.com

Summary by CodeRabbit

  • New Features
    • Upgraded authentication system for inference services that intelligently selects credentials: automatically uses modern JWT-based access tokens when available and suitable, falling back to legacy agent key generation when needed. Optimizes performance while maintaining backward compatibility.
    • Improved credential management with enhanced expiry tracking, lifecycle state management, and automated refresh mechanisms.

Signed-off-by: Shannon Sands <shannon.sands.1979@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 2, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9b764e65-3ac0-4cb4-9b2c-1feeb6a9a26f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@cv cv added v0.0.57 and removed v0.0.57 labels Jun 2, 2026
@wscurran wscurran added area: inference Inference routing, serving, model selection, or outputs feature PR adds or expands user-visible functionality labels Jun 3, 2026
@wscurran

wscurran commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this detailed PR about updating Hermes to prefer invoke JWT access tokens for inference, including changes to onboarding, token detection, and credential rotation. This proposes a new feature that enhances the inference capability and authentication in Hermes.

@wscurran wscurran added the integration: hermes Hermes integration behavior label Jun 3, 2026
@cv cv assigned cv and unassigned cjagwani Jun 3, 2026

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

🧹 Nitpick comments (1)
agents/hermes/host/tool-gateway-broker.ts (1)

243-303: 💤 Low value

Consider extracting shared JWT utilities.

The JWT parsing helpers (decodeBase64UrlJson, decodeJwtClaims, jwtScopes, jwtExpiresAtIso, isUsableInvokeJwt) duplicate logic from src/lib/oauth-device-code.ts. While this duplication is understandable for a standalone broker script, you could consider extracting these to a shared module under agents/hermes/config/ to reduce maintenance burden if the JWT inspection logic evolves.

🤖 Prompt for 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.

In `@agents/hermes/host/tool-gateway-broker.ts` around lines 243 - 303, The JWT
helpers in this file (decodeBase64UrlJson, decodeJwtClaims, jwtScopes,
jwtExpiresAtIso, isUsableInvokeJwt) duplicate logic found in
src/lib/oauth-device-code.ts; extract these utilities into a shared module
(e.g., agents/hermes/config/jwt-utils) and replace the local implementations
with imports from that module. Move the functions decodeBase64UrlJson,
decodeJwtClaims, jwtScopes, jwtExpiresAtIso and isUsableInvokeJwt into the new
module, export them, update callers here to import and use those exports, and
ensure any constants used (e.g., NOUS_INFERENCE_INVOKE_SCOPE,
INFERENCE_CREDENTIAL_REFRESH_SKEW_MS, timestampExpiresSoon) are either imported
into the shared module or passed through so behavior remains identical.
🤖 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.

Nitpick comments:
In `@agents/hermes/host/tool-gateway-broker.ts`:
- Around line 243-303: The JWT helpers in this file (decodeBase64UrlJson,
decodeJwtClaims, jwtScopes, jwtExpiresAtIso, isUsableInvokeJwt) duplicate logic
found in src/lib/oauth-device-code.ts; extract these utilities into a shared
module (e.g., agents/hermes/config/jwt-utils) and replace the local
implementations with imports from that module. Move the functions
decodeBase64UrlJson, decodeJwtClaims, jwtScopes, jwtExpiresAtIso and
isUsableInvokeJwt into the new module, export them, update callers here to
import and use those exports, and ensure any constants used (e.g.,
NOUS_INFERENCE_INVOKE_SCOPE, INFERENCE_CREDENTIAL_REFRESH_SKEW_MS,
timestampExpiresSoon) are either imported into the shared module or passed
through so behavior remains identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b6faa2ce-8210-4e28-94ca-d134dd0c57c6

📥 Commits

Reviewing files that changed from the base of the PR and between dbc6547 and ae2d7a5.

📒 Files selected for processing (6)
  • agents/hermes/host/tool-gateway-broker.ts
  • src/lib/hermes-provider-auth.test.ts
  • src/lib/hermes-provider-auth.ts
  • src/lib/oauth-device-code.test.ts
  • src/lib/oauth-device-code.ts
  • test/hermes-tool-gateway-broker.test.ts

@cv cv added v0.0.59 and removed v0.0.58 labels Jun 4, 2026
@cv cv added v0.0.61 and removed v0.0.60 labels Jun 6, 2026
@jyaunches jyaunches added v0.0.64 and removed v0.0.63 labels Jun 11, 2026
@cv cv added the v0.0.65 label Jun 12, 2026
@cv cv added v0.0.66 and removed v0.0.64 labels Jun 12, 2026
@cv cv added v0.0.67 and removed v0.0.66 labels Jun 23, 2026
@jyaunches jyaunches added v0.0.68 and removed v0.0.67 labels Jun 24, 2026
@cv

cv commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Manual PR Review Advisor result

This PR Review Advisor analysis was run manually via workflow_dispatch, so the workflow did not post its usual sticky comment. Posting the advisor summary here because it produced actionable findings.

Run: https://github.com/NVIDIA/NemoClaw/actions/runs/28206514386


PR Review Advisor

The invoke-JWT path is covered for the happy path, but the credential trust boundary needs tighter base-URL/JWT validation and fallback regression coverage before this is safe.

Required before merge

  • None.

Resolve or justify before merge

  • Source-of-truth review needed: Invoke JWT versus legacy agent-key fallback: The advisor marked localized patch analysis as needs_followup.
    • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Verification hint: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
    • Missing regression test: Opaque fallback is covered in onboarding, but expired, malformed, missing-scope, and broker fallback cases are missing.
    • Evidence: src/lib/hermes-provider-auth.test.ts covers opaque fallback; test/hermes-tool-gateway-broker.test.ts only covers invoke JWT refresh.
  • Source-of-truth review needed: Inference base URL source of truth: The advisor marked localized patch analysis as missing.
    • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Verification hint: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
    • Missing regression test: No test covers untrusted token-response base URLs or verifies onboarding and broker refresh preserve the same endpoint policy.
    • Evidence: src/lib/hermes-provider-auth.ts accepts tokens.inference_base_url, while agents/hermes/host/tool-gateway-broker.ts defaults/allowlists the inference base URL.
  • Source-of-truth review needed: Tolerant JWT claim parsing: The advisor marked localized patch analysis as needs_followup.
    • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Verification hint: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
    • Missing regression test: Malformed-token fallback is not currently tested.
    • Evidence: decodeBase64UrlJson catches parse failures and returns null in both OAuth helper and broker implementations.
  • Constrain token-provided inference base URLs before OpenShell registration (src/lib/hermes-provider-auth.ts:142): The invoke-JWT onboarding path now takes tokens.inference_base_url || baseUrl and passes it through to OPENAI_BASE_URL for the hermes-provider. Unlike the broker refresh path, which normalizes through trustedInferenceBaseUrl, this path accepts the token response endpoint directly. That makes the inference endpoint policy inconsistent across onboarding and broker refresh.
    • Impact: If the OAuth token response path is misconfigured or compromised, NemoClaw can register an OpenShell inference provider that sends future prompts and the invoke JWT to an unexpected endpoint, creating an SSRF/credential-exfiltration class risk. It can also cause behavior drift where onboarding sets a staging/custom URL and the broker later overwrites it with the default trusted URL.
    • Recommendation: Apply a single trusted inference-base-url policy for both onboarding and broker refresh. Prefer an allowlist or explicit user-supplied endpoint precedence, and reject/default token-provided URLs that are not trusted Nous inference origins before calling registerHermesInferenceProvider.
    • Verification hint: Read src/lib/hermes-provider-auth.ts around const inferenceBaseUrl = tokens.inference_base_url || baseUrl and compare it with agents/hermes/host/tool-gateway-broker.ts trustedInferenceBaseUrl usage.
    • Missing regression test: Add a test where the token response returns inference_base_url: "http://127.0.0.1:9/v1" or another untrusted origin and assert ensureHermesProviderOAuthCredentials rejects it or registers the default/explicit trusted base URL instead.
    • Evidence: src/lib/hermes-provider-auth.test.ts currently asserts that https://staging.nous.example/v1 from the token response is registered as OPENAI_BASE_URL, while broker refresh only trusts https://inference-api.nousresearch.com/v1.
  • Do not treat unsigned decoded JWT claims as proof of an invoke credential (src/lib/oauth-device-code.ts:204): isNousInvokeAccessToken and the broker's duplicated isUsableInvokeJwt decode the JWT payload and check scope/scp and exp, but they do not verify issuer, audience, signature, or even a strict three-part JWT structure. The tests create alg: "none" fixtures, which is fine for fixtures but demonstrates that the helper is a claim decoder rather than token validation.
    • Impact: If any untrusted token string can reach this classification boundary, NemoClaw may skip the safer legacy agent-key minting path and store a caller-controlled string as the OpenShell inference credential. Even when the portal endpoint is the intended trust anchor, the current helper name and behavior can mislead future callers into using decoded claims as an authorization check.
    • Recommendation: Either verify the access token's issuer/audience/signature using the portal's trusted JWKS before classifying it as an invoke JWT, or make the trust boundary explicit: restrict this helper to token responses from the trusted portal, rename/document it as a non-verifying hint, and keep legacy fallback for every uncertain case.
    • Verification hint: Inspect decodeJwtClaims, jwtScopeSet, and isNousInvokeAccessToken in src/lib/oauth-device-code.ts, plus decodeJwtClaims and isUsableInvokeJwt in agents/hermes/host/tool-gateway-broker.ts.
    • Missing regression test: Add tests proving malformed JWTs, JWTs with alg: none but missing trusted issuer/audience if validation is added, expired JWTs, and JWTs without inference:invoke all take the legacy agent-key path instead of being registered directly.
    • Evidence: decodeJwtClaims only base64url-decodes parts[1]; isNousInvokeAccessToken accepts any decoded payload with inference:invoke and a future exp.
  • Fallback coverage only exercises opaque tokens, not expired or malformed invoke JWTs (src/lib/hermes-provider-auth.test.ts:178): The PR claims fallback is preserved for non-JWT, expired, or non-invoke tokens, but the changed tests only cover the opaque/non-invoke onboarding case and the broker invoke happy path. The source-of-truth for fallback is the portal token format migration boundary, so the negative cases need to be locked down.
    • Impact: A regression in expiry handling, malformed JWT tolerance, or broker legacy fallback could silently register an unusable access token with OpenShell, breaking inference refresh or exposing a broader credential than intended.
    • Recommendation: Add focused negative-path tests for the changed credential selection logic in both onboarding and the broker. Keep these tests at the existing helper/spawned-broker level; no new framework layer is needed.
    • Verification hint: Compare src/lib/oauth-device-code.test.ts isNousInvokeAccessToken coverage and src/lib/hermes-provider-auth.test.ts falls back to legacy agent-key minting when OAuth returns a non-invoke token with the broker test, which asserts agentKeyRequests length is zero for the invoke path only.
    • Missing regression test: Add ensureHermesProviderOAuthCredentials mints an agent key for an expired invoke JWT, ensureHermesProviderOAuthCredentials mints an agent key for malformed JWT claims, and Hermes managed-tool broker mints and registers a legacy agent key when refresh returns an opaque/non-invoke access_token.
    • Evidence: Existing changed tests cover the positive invoke JWT path and one opaque fallback, but not expired JWTs, missing-scope JWTs, malformed JWT payloads, or broker fallback.
  • Credential-flow comments still describe agent-key-only onboarding (src/lib/oauth-device-code.ts:6): The file header still says onboarding uses ephemeral OAuth tokens to mint short-lived agent keys for OpenShell registration and that OAuth tokens are not exposed through that path. After this PR, a usable inference:invoke access JWT is registered directly as OPENAI_API_KEY.
    • Impact: Future maintainers reviewing credential leakage, sandbox boundaries, or token lifetime assumptions can be misled into thinking OpenShell never stores OAuth access JWTs, which weakens security review and incident response for this high-risk credential path.
    • Recommendation: Update the comments and any nearby user-facing notes to describe the two supported auth paths: direct short-lived invoke JWT registration and legacy agent-key minting fallback, including which credential is stored in OpenShell and when it is refreshed.
    • Verification hint: Read the header comment in src/lib/oauth-device-code.ts and compare it with the useInvokeJwt branch in src/lib/hermes-provider-auth.ts.
    • Missing regression test: Existing tests already prove the behavior drift by asserting providerCalls.some((call) => call.env?.OPENAI_API_KEY === token) in the invoke-JWT path; no additional automated test is needed for the comment update.
    • Evidence: src/lib/hermes-provider-auth.test.ts now expects the access JWT itself to be passed as OPENAI_API_KEY, contradicting the old agent-key-only description.

In-scope improvements

  • Shrink duplicated JWT parsing logic while preserving the trust boundary (src/lib/oauth-device-code.ts:145): The PR adds nearly identical base64url decoding, scope extraction, expiry parsing, and invoke-token classification logic in src/lib/oauth-device-code.ts and agents/hermes/host/tool-gateway-broker.ts. oauth-device-code.ts also grows into a larger hotspot.
    • Impact: Two independent implementations can drift on expiry skew, scope parsing, malformed-token behavior, or future issuer/audience validation, which is especially risky for credential-path selection.
    • Recommendation: If the broker can safely import a shared helper in its runtime context, extract a small shared JWT classification module and use it from both paths. If the broker must remain standalone, add parity tests and reduce the helper surface to the minimum needed in each file.
    • Verification hint: Compare decodeBase64UrlJson, decodeJwtClaims, scope parsing, and expiry helpers in src/lib/oauth-device-code.ts with the corresponding functions in agents/hermes/host/tool-gateway-broker.ts.
    • Missing regression test: If the code stays duplicated, add a parity test table covering scope string, scp array, missing scope, malformed payload, and near-expiry JWTs for both helper implementations.
    • Evidence: The same invoke-JWT classification behavior is implemented in both changed source files, and the drift context flags src/lib/oauth-device-code.ts as growing by more than 100 lines.

Test follow-ups to resolve or justify

  • Runtime validation — isNousInvokeAccessToken returns false for expired invoke JWTs and JWTs missing inference:invoke.. The changed files control runtime credential refresh, OpenShell provider registration, and a host network broker. Existing tests include focused unit tests and a spawned broker integration test for the invoke happy path, but negative and policy tests are still needed.
  • Runtime validation — ensureHermesProviderOAuthCredentials mints an agent key instead of registering an expired invoke JWT.. The changed files control runtime credential refresh, OpenShell provider registration, and a host network broker. Existing tests include focused unit tests and a spawned broker integration test for the invoke happy path, but negative and policy tests are still needed.
  • Runtime validation — ensureHermesProviderOAuthCredentials rejects or defaults token-response inference_base_url values outside trusted Nous inference origins.. The changed files control runtime credential refresh, OpenShell provider registration, and a host network broker. Existing tests include focused unit tests and a spawned broker integration test for the invoke happy path, but negative and policy tests are still needed.
  • Runtime validation — Hermes managed-tool broker mints and registers a legacy agent key when refresh returns an opaque or non-invoke access token.. The changed files control runtime credential refresh, OpenShell provider registration, and a host network broker. Existing tests include focused unit tests and a spawned broker integration test for the invoke happy path, but negative and policy tests are still needed.
  • Runtime validation — Broker refresh and onboarding apply the same inference base URL policy for default, explicit, and token-provided endpoints.. The changed files control runtime credential refresh, OpenShell provider registration, and a host network broker. Existing tests include focused unit tests and a spawned broker integration test for the invoke happy path, but negative and policy tests are still needed.
  • Fallback coverage only exercises opaque tokens, not expired or malformed invoke JWTs — Add focused negative-path tests for the changed credential selection logic in both onboarding and the broker. Keep these tests at the existing helper/spawned-broker level; no new framework layer is needed.
  • Invoke JWT versus legacy agent-key fallback — Opaque fallback is covered in onboarding, but expired, malformed, missing-scope, and broker fallback cases are missing.. src/lib/hermes-provider-auth.test.ts covers opaque fallback; test/hermes-tool-gateway-broker.test.ts only covers invoke JWT refresh.
  • Inference base URL source of truth — No test covers untrusted token-response base URLs or verifies onboarding and broker refresh preserve the same endpoint policy.. src/lib/hermes-provider-auth.ts accepts tokens.inference_base_url, while agents/hermes/host/tool-gateway-broker.ts defaults/allowlists the inference base URL.

What looks good

  • The PR keeps refresh tokens out of durable NemoClaw state and continues to store hashes for broker matching.
  • OpenShell invocations continue to use argument arrays rather than shell string execution.
  • The changed tests cover the direct invoke-JWT happy path, default scope request, header-based refresh-token grant, and broker output redaction.
  • The managed-tool broker still strips inbound secret headers before forwarding requests upstream.

@cv cv removed the v0.0.69 label Jun 28, 2026
@cv cv added v0.0.82 and removed v0.0.82 labels Jul 12, 2026
@cv

cv commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Salvaged onto the NVIDIA-owned draft PR #6885 because maintainer pushes to the contributor fork branch are denied. The replacement preserves Shannon Sands as the commit author and carries forward the existing review context; product-scope approval and the credential-boundary findings remain unresolved. Leaving this PR open until the replacement checks settle.

@cv cv closed this Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs feature PR adds or expands user-visible functionality integration: hermes Hermes integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants