feat(blueprint): add Entra runtime identity reference - #7686
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Microsoft Entra runtime identity profile, separates token-issuer and credential-delivery trust policies, validates reviewed REST paths, expands provider documentation, and parameterizes live identity E2E coverage across Okta and Entra-shaped scenarios. ChangesRuntime identity contracts and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant E2ETestRunner
participant OpenShell
participant OAuthFixture
E2ETestRunner->>OAuthFixture: Configure scenario token and resource paths
E2ETestRunner->>OpenShell: Plan and attach runtime identity
OpenShell->>OAuthFixture: Refresh token and deliver bearer
E2ETestRunner->>OpenShell: Probe reviewed and denied paths
E2ETestRunner->>OpenShell: Rotate credentials and repeat request
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7686.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit b7bb56e in the TypeScript / code-coverage/cliThe overall coverage in commit b7bb56e in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
nemoclaw/src/blueprint/runtime-identity.test.ts (1)
517-537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDifferentiate the
it.eachcase titles.All four rows share the literal title "rejects a broadened Entra credential-delivery profile", so a CI failure can't tell which boundary broke without inspecting the row index. Case 4 (Line 533-536) also isn't really a "credential-delivery" case — it tests the token-issuer boundary (rejecting
graph.microsoft.comas the refreshtoken_urlhost).♻️ Proposed title parameterization
- ])("rejects a broadened Entra credential-delivery profile", async (profile, message) => { + ])("rejects an Entra profile violating trust boundary: %s", async (profile, message) => {(pass a short case label as a third array element, e.g.
"widened delivery path","delivery host swap","delivery subdomain spoof","issuer/delivery host swap", and reference it in the format string.)🤖 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 `@nemoclaw/src/blueprint/runtime-identity.test.ts` around lines 517 - 537, Differentiate the parameterized test cases in “rejects a broadened Entra credential-delivery profile” by adding a short descriptive label to each row and incorporating it into the test title. Use labels that identify the delivery-path, delivery-host, delivery-subdomain, and issuer/delivery-host boundary cases, with the fourth case explicitly reflecting the token-issuer check.test/e2e/live/runtime-identity-oauth-server.ts (1)
162-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated bearer-token parsing between the
GEThandler and the fallback catchall.Lines 163-172 and 191-198 repeat identical logic for extracting the bearer token and classifying
auth. Consider extracting a small helper (e.g.,classifyBearer(req, accessTokens, currentAccessToken)) to avoid the two copies drifting apart.♻️ Proposed extraction
+function classifyBearer( + req: http.IncomingMessage, + accessTokens: readonly string[], + currentAccessToken: string | undefined, +): { token: string | undefined; accessTokenVersion: number | null; auth: RuntimeIdentityResourceRequest["auth"] } { + const authorization = req.headers.authorization; + const token = + typeof authorization === "string" && authorization.startsWith("Bearer ") + ? authorization.slice("Bearer ".length) + : undefined; + const rawVersion = token ? accessTokens.findIndex((candidate) => candidate === token) + 1 : null; + const accessTokenVersion = rawVersion && rawVersion > 0 ? rawVersion : null; + const auth: RuntimeIdentityResourceRequest["auth"] = + token === currentAccessToken ? "ok" : token ? "invalid" : "missing"; + return { token, accessTokenVersion, auth }; +}🤖 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 `@test/e2e/live/runtime-identity-oauth-server.ts` around lines 162 - 205, Extract the duplicated bearer-token parsing and authentication classification from the GET resource handler and fallback catchall into a shared helper near the request handling code. Have the helper accept the request, accessTokens, and currentAccessToken, and return the token, auth classification, and access-token version needed by both paths; update both handlers to reuse it without changing their existing response behavior.
🤖 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 `@nemoclaw/src/blueprint/runtime-identity.test.ts`:
- Around line 517-537: Differentiate the parameterized test cases in “rejects a
broadened Entra credential-delivery profile” by adding a short descriptive label
to each row and incorporating it into the test title. Use labels that identify
the delivery-path, delivery-host, delivery-subdomain, and issuer/delivery-host
boundary cases, with the fourth case explicitly reflecting the token-issuer
check.
In `@test/e2e/live/runtime-identity-oauth-server.ts`:
- Around line 162-205: Extract the duplicated bearer-token parsing and
authentication classification from the GET resource handler and fallback
catchall into a shared helper near the request handling code. Have the helper
accept the request, accessTokens, and currentAccessToken, and return the token,
auth classification, and access-token version needed by both paths; update both
handlers to reuse it without changing their existing response behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b3fad885-a424-4d4d-afbd-5020ef7aa988
📒 Files selected for processing (7)
.github/workflows/e2e.yamldocs/reference/architecture.mdxnemoclaw-blueprint/provider-profiles/entra-runtime-v1.yamlnemoclaw/src/blueprint/runtime-identity.test.tsnemoclaw/src/blueprint/runtime-identity.tstest/e2e/live/inference-routing.test.tstest/e2e/live/runtime-identity-oauth-server.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…entity/ae Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Exact-head validation evidenceValidated commit Pluggable architecture
Protected E2E
TC-INF-13 proved, through a real OpenShell gateway/provider/sandbox path and a public HTTPS Entra-shaped fixture:
The complete protected matrix also passed, including inference routing, cloud inference/onboarding, network-policy live probes, Hermes, full E2E, and both security-posture jobs. Real Entra acceptanceA separate live-tenant acceptance check silently refreshed an existing delegated token with Boundary: TC-INF-13 intentionally uses deterministic fixture credentials rather than a production tenant secret. Together, the live-tenant Graph check proves the external Entra/Graph contract, while TC-INF-13 proves NemoClaw's complete refresh, policy, injection, rotation, rollback, and secret-safety path. Both PR review advisors and all current exact-head CI/E2E checks are green. GitHub reports the PR mergeable; the remaining branch-protection requirement is human approval. |
ericksoa
added
v0.0.97
feature
|
Maintainer product-scope gate: the technical checks are green, but I cannot advance or approve this supported Entra integration yet. The linked #6871 still has |
|
I rechecked this for the The implementation is still behind NemoClaw's Product Scope Gate: the linked design issue #6871 remains open in Backlog with I am not submitting a Changes Requested review and I am not modifying the contributor branch. To make this reviewable for approval, please first get an accepted maintainer design/product decision for #6871, then refresh onto current |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/reference/architecture.mdx (1)
341-365: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftBlock publication pending the Entra product-scope decision.
This new operational provider reference is a supported integration surface, while
#6871still lacks accepted scope, ownership, lifecycle, compatibility, security, and validation decisions. Keep it out of published product documentation, or route it through Community Solutions, until maintainers accept that scope. As per coding guidelines, “Before implementing or approving a new supported integration… confirm an accepted scope decision and defined ownership, lifecycle, compatibility, security, and validation expectations.”🤖 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 `@docs/reference/architecture.mdx` around lines 341 - 365, Remove the new “Microsoft Entra Reference” operational provider section from published architecture documentation until the Entra product-scope decision is accepted. If retaining the guidance, move it into the Community Solutions documentation path instead of presenting it as a supported integration.Source: Coding guidelines
.github/workflows/e2e.yaml (1)
2067-2069: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winDo not expose secrets to candidate checkouts.
When
inputs.checkout_shais supplied, this job checks out PR-controlled code and then runs it withsecrets.NVIDIA_INFERENCE_API_KEYin the environment. A candidate test can exfiltrate that key. Gate this job to a trusted checkout with nocheckout_sha, or split the secret-bearing live lane from the PR-safe lane.Proposed guard
- if: ${{ (github.event_name != 'workflow_dispatch' || (inputs.jobs == '' && inputs.targets == '')) || contains(format(',{0},', inputs.jobs), ',dashboard-remote-bind,') || contains(format(',{0},', inputs.targets), ',dashboard-remote-bind,') }} + if: ${{ inputs.checkout_sha == '' && ((github.event_name != 'workflow_dispatch' || (inputs.jobs == '' && inputs.targets == '')) || contains(format(',{0},', inputs.jobs), ',dashboard-remote-bind,') || contains(format(',{0},', inputs.targets), ',dashboard-remote-bind,')) }}As per path instructions, trusted automation must not execute PR-controlled code while holding repository secrets or write permissions.
Also applies to: 2083-2099
🤖 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 @.github/workflows/e2e.yaml around lines 2067 - 2069, Update the dashboard-remote-bind job guard and its secret-bearing execution path to prevent running PR-controlled checkouts with NVIDIA_INFERENCE_API_KEY. Allow this job only for trusted checkouts where inputs.checkout_sha is absent, or separate the live secret-using lane from the PR-safe candidate lane while preserving intended job selection behavior.Source: Path instructions
🤖 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 `@docs/reference/architecture.mdx`:
- Around line 379-393: Update the protected-resource acceptance instructions
around the Entra access-token placeholder and curl request to explicitly execute
the command inside the sandbox child, using the child’s environment so proxy
delivery and Graph path rejection are exercised. Clarify that the host shell
must not run the request directly, while preserving the existing request and
secret-handling guidance.
---
Outside diff comments:
In @.github/workflows/e2e.yaml:
- Around line 2067-2069: Update the dashboard-remote-bind job guard and its
secret-bearing execution path to prevent running PR-controlled checkouts with
NVIDIA_INFERENCE_API_KEY. Allow this job only for trusted checkouts where
inputs.checkout_sha is absent, or separate the live secret-using lane from the
PR-safe candidate lane while preserving intended job selection behavior.
In `@docs/reference/architecture.mdx`:
- Around line 341-365: Remove the new “Microsoft Entra Reference” operational
provider section from published architecture documentation until the Entra
product-scope decision is accepted. If retaining the guidance, move it into the
Community Solutions documentation path instead of presenting it as a supported
integration.
🪄 Autofix (Beta)
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: da304032-d139-48ad-b1e9-b54efe370305
📒 Files selected for processing (3)
.github/workflows/e2e.yamldocs/reference/architecture.mdxnemoclaw/src/blueprint/runtime-identity.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- nemoclaw/src/blueprint/runtime-identity.test.ts
|
The Product Scope Gate is now cleared by the accepted maintainer decision in #6871, so I re-reviewed exact head
The rest of the boundary reviewed fail-closed: exact issuer and Graph host matching, exact |
|
Maintainer follow-through is complete on the current exact revision.
The only remaining repository gate is one approving human review on the current head; the author cannot self-approve. |
|
Re-review at exact head
Please bind port 443 in the reviewed Entra credential-delivery policy and reject a copied profile that changes it. This is a plain blocker comment, not a Changes Requested review; I will recheck the next quiet exact head. |
|
Shipping handoff refreshed for the current exact head:
The branch is caught up and the technical gates are green. The remaining branch-protection gate is one approving human review; no reviewer has been requested. |
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical July 30 release entry for `v0.0.99` before the release tag is captured. The entry covers all 37 merged PRs since `v0.0.98` and bounds experimental or dormant work without presenting it as supported behavior. ## Changes - Adds `docs/changelog/2026-07-30.mdx` with the exact `## v0.0.99` heading, parser-safe MDX SPDX comment, summary, detailed release bullets, and published documentation routes. - Records user-visible recovery, snapshot, shared-route, Hermes, readiness, inference, image, documentation, and release E2E changes. - States that the managed-image selection and startup-profile contracts remain dormant and do not activate buildless onboarding. Source summary: - [#7972](#7972) -> `docs/changelog/2026-07-30.mdx`: Records restored managed OpenClaw configuration modes during recovery. - [#7834](#7834) -> `docs/changelog/2026-07-30.mdx`: Records clone-bound pairing verification after snapshot restore. - [#7975](#7975) -> `docs/changelog/2026-07-30.mdx`: Records managed startup recovery coverage. - [#7960](#7960) -> `docs/changelog/2026-07-30.mdx`: Records dormant startup-profile coordination without activating a supported surface. - [#7856](#7856) -> `docs/changelog/2026-07-30.mdx`: Records persistence of the credential-free OpenClaw startup command. - [#7959](#7959) -> `docs/changelog/2026-07-30.mdx`: Records dormant startup-profile construction without changing onboarding. - [#7946](#7946) -> `docs/changelog/2026-07-30.mdx`: Records the internal startup-profile schema and transport contract. - [#7951](#7951) -> `docs/changelog/2026-07-30.mdx`: Records platform-pull cleanup before managed-image validation. - [#7949](#7949) -> `docs/changelog/2026-07-30.mdx`: Records rejection of retained Hermes `uv` build cache metadata. - [#7597](#7597) -> `docs/changelog/2026-07-30.mdx`: Records separate command and agent first-turn latency evidence. - [#7931](#7931) -> `docs/changelog/2026-07-30.mdx`: Records focused E2E replacement evidence for retired selectors. - [#7950](#7950) -> `docs/changelog/2026-07-30.mdx`: Records exclusion of build-only BuildKit telemetry from the Deep Agents Code probe. - [#7665](#7665) -> `docs/changelog/2026-07-30.mdx`: Records consolidated priority 2 E2E coverage. - [#7911](#7911) -> `docs/changelog/2026-07-30.mdx`: Records the corrected NVIDIA DORI installation pin. - [#7934](#7934) -> `docs/changelog/2026-07-30.mdx`: Records the staging image-family wait before Brev Launchable deployment. - [#7772](#7772) -> `docs/changelog/2026-07-30.mdx`: Records dormant managed-image selection contracts without activating buildless onboarding. - [#7941](#7941) -> `docs/changelog/2026-07-30.mdx`: Records corrected agent-specific provider and policy guidance. - [#7819](#7819) -> `docs/changelog/2026-07-30.mdx`: Records removal of empty Deep Agents Code provider-switch sections. - [#7932](#7932) -> `docs/changelog/2026-07-30.mdx`: Records independent credential-generation E2E execution. - [#7840](#7840) -> `docs/changelog/2026-07-30.mdx`: Records shared-route preservation and pre-delete peer validation during upgrades. - [#7874](#7874) -> `docs/changelog/2026-07-30.mdx`: Records the split between pre-tag release entries and post-tag Announcements. - [#7876](#7876) -> `docs/changelog/2026-07-30.mdx`: Records the writable Hermes runtime root within lockdown. - [#7756](#7756) -> `docs/changelog/2026-07-30.mdx`: Records validated multi-platform managed-image publication. - [#7914](#7914) -> `docs/changelog/2026-07-30.mdx`: Records accepted `uv` version metadata in Hermes image validation. - [#7686](#7686) -> `docs/changelog/2026-07-30.mdx`: Records the explicitly experimental Microsoft Entra runtime identity reference. - [#7869](#7869) -> `docs/changelog/2026-07-30.mdx`: Records classified gateway relaunch quarantine and rebuild guidance. - [#7814](#7814) -> `docs/changelog/2026-07-30.mdx`: Records state restore into replacement sandboxes and SQLite write verification. - [#7839](#7839) -> `docs/changelog/2026-07-30.mdx`: Records quieter onboarding test execution without a user-facing behavior claim. - [#7854](#7854) -> `docs/changelog/2026-07-30.mdx`: Records generalized agent-selection guidance. - [#7845](#7845) -> `docs/changelog/2026-07-30.mdx`: Records isolated CDI test evidence without a user-facing behavior claim. - [#7843](#7843) -> `docs/changelog/2026-07-30.mdx`: Records the corrected Omni sub-agent model ID. - [#7908](#7908) -> `docs/changelog/2026-07-30.mdx`: Records reviewed Hermes and Deep Agents Code dependency pins. - [#7887](#7887) -> `docs/changelog/2026-07-30.mdx`: Records rejection of a symlinked DGX Station release marker. - [#7747](#7747) -> `docs/changelog/2026-07-30.mdx`: Records the internal compute-driver separation without a user-facing behavior claim. - [#7660](#7660) -> `docs/changelog/2026-07-30.mdx`: Records atomic publication of rebuild recovery manifests. - [#7661](#7661) -> `docs/changelog/2026-07-30.mdx`: Records bounded local inference health-response retention. - [#7654](#7654) -> `docs/changelog/2026-07-30.mdx`: Records state preservation across supervisor relaunch recovery. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, SPDX comment, version heading, and published routes. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] 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: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-30.mdx`; the documentation-only diff passed review against `WRITING.md`, the controlled word list, and `docs/CONTRIBUTING.md`. The review covered terminology, structure, active voice, release meaning, product-scope boundaries, and link and code presentation. Changelog tests passed 6/6, and the docs build reported 0 errors with 2 pre-existing warnings. - Agent: Codex CLI <!-- docs-review-head-sha: 200940f --> <!-- docs-review-agents-blob-sha: c052d60 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] 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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this documentation-only release entry. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — result: Build passed with 0 errors and 2 pre-existing warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.99 covering snapshot restoration, sandbox recovery, gateway route upgrades, and Hermes security updates. * Documented experimental Microsoft Entra runtime identity support and enhanced readiness checks. * Added details on managed image validation, trusted CI image promotion, and end-to-end release evidence. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Add Microsoft Entra ID as the second pluggable, data-only runtime identity implementation after the Okta reference from #7265. Entra delegated credentials can now refresh through the trusted Entra issuer and reach only the reviewed Microsoft Graph
/v1.0/meresource path.Related Issue
Part of #6871
Changes
entra-runtime-v1provider profile with app-supplied delegated refresh material,User.Read/offline_accessguidance, and no bootstrap application or OBO flow.login.microsoftonline.comand credential delivery toGET graph.microsoft.com/v1.0/me./me, denial of/users, token rotation, secret-safe evidence, and ownership-aware rollback.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/reference/architecture.mdx;nemoclaw-blueprint/provider-profiles/entra-runtime-v1.yamlDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm --prefix nemoclaw test -- src/blueprint/runtime-identity.test.ts src/blueprint/runner-identity.test.ts(129 passed);npx vitest run --project integration test/blueprint-runtime-identity-lifecycle.test.ts test/blueprint-runtime-identity-schema.test.ts(8 passed); CLI typecheck passed; live collection lists TC-INF-12 and TC-INF-13.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — protected exact-head CI and TC-INF-13 pending.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
GET /v1.0/me.entra-runtime-v1) with bearer delivery and refresh configuration.Documentation
Tests / Reliability
method/path.