feat(identity): add host-managed Okta OBO reference - #8155
Conversation
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
📝 WalkthroughWalkthroughThe change adds Okta OBO runtime identity support. It introduces token-exchange contracts, validation, static provider creation, secret-safe credential delivery, sandbox lifecycle updates, documentation, integration tests, and a live end-to-end scenario. ChangesRuntime Identity OBO
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BlueprintRunner
participant RuntimeIdentity
participant OktaTokenEndpoint
participant OpenShellProvider
participant Sandbox
BlueprintRunner->>RuntimeIdentity: prepare OBO identity
RuntimeIdentity->>OktaTokenEndpoint: exchange subject token
OktaTokenEndpoint-->>RuntimeIdentity: return delegated access token
RuntimeIdentity->>OpenShellProvider: create provider with delegated credential
OpenShellProvider->>Sandbox: inject opaque credential placeholder
BlueprintRunner->>Sandbox: validate admitted bearer request
BlueprintRunner->>OpenShellProvider: rollback owned provider
BlueprintRunner->>Sandbox: delete owned sandbox
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 2430fb3 in the TypeScript / code-coverage/cliThe overall coverage in commit 2430fb3 in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-8155.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Alex Fournier <afournier@nvidia.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
wscurran
added
area: integrations
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
…nt/af Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
E2E Target Results — ❌ Some tests failedRun: 30951539467
|
Signed-off-by: Alex Fournier <afournier@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
nemoclaw/src/blueprint/runner.ts (1)
1275-1275: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an exact
sandbox deletenot-found fixture. Mock exit code1with stderrsandbox not foundand assert both cleanup paths complete successfully.🤖 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/runner.ts` at line 1275, Add an exact fixture for the runCmd invocation in the sandbox cleanup flow, matching openshell sandbox delete with exit code 1 and stderr “sandbox not found”; verify that both cleanup paths complete successfully despite this not-found result.test/e2e/live/runtime-identity-oauth-server.ts (1)
170-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
grantTypeOkto state what it records.The enclosing
ifalready proved the grant type is the token-exchange URN.grantTypeOkthen reports whether the fixture was configured withtokenExchange. A run without that option recordsgrantTypeOk: falsefor a request whose grant type was correct.This value reaches the
tc-inf-14-token-exchange-requests.jsonartifact and the scenario assertion, so the misleading name weakens the evidence a reader draws from it. Rename the field toexchangeConfigured, or setgrantTypeOkfrom the grant-type comparison and track fixture configuration separately.🤖 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 170 - 172, Rename grantTypeOk in the token-exchange handling flow to exchangeConfigured, and update all downstream references, including the recorded tc-inf-14-token-exchange-requests.json data and scenario assertion, so the field clearly indicates whether options.tokenExchange is configured rather than whether the grant type is valid.test/blueprint-runtime-identity-lifecycle.test.ts (1)
357-361: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProve that the isolation assertion inspects at least one call.
Array.prototype.everyreturnstruefor an empty array. If the filter ever yields no calls, this assertion passes without checking secret isolation for any call. Assert the inspected set explicitly.As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."♻️ Proposed fix
- expect( - state.calls - .filter(({ args }) => !(args[0] === "provider" && args[1] === "create")) - .every(({ hasClientSecret, hasOboAccessToken }) => !hasClientSecret && !hasOboAccessToken), - ).toBe(true); + const otherCalls = state.calls.filter( + ({ args }) => !(args[0] === "provider" && args[1] === "create"), + ); + expect(otherCalls.length).toBeGreaterThan(0); + expect( + otherCalls.map(({ hasClientSecret, hasOboAccessToken }) => ({ + hasClientSecret, + hasOboAccessToken, + })), + ).toEqual(otherCalls.map(() => ({ hasClientSecret: false, hasOboAccessToken: false })));🤖 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/blueprint-runtime-identity-lifecycle.test.ts` around lines 357 - 361, Update the isolation assertion in the test to first require that the filtered call set contains at least one call, then retain the existing every-based secret isolation check. Ensure an empty filtered result fails rather than passing vacuously.Source: Path instructions
nemoclaw/src/blueprint/runtime-identity.test.ts (1)
503-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the absence of the response detail that these titles claim is suppressed.
Both tests claim non-disclosure, but neither asserts it.
rejects.toThrow("Okta token exchange failed with HTTP 400")andrejects.toThrow(/request failed/)match a substring. A message that also appendedserver echoed a secret,network details, ornot-jsonstill passes.Add a negative assertion on the thrown message so the claim in each title is exercised.
As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."♻️ Proposed fix
- await expect( - exchangeOktaOboToken( - { - tokenUrl: "https://example.okta.com/oauth2/default/v1/token", - clientId: "client-id", - clientSecret: "client-secret", - subjectToken: "user-subject-token", - audience: "api://orders", - scopes: ["orders.read"], - }, - async () => new Response("server echoed a secret", { status: 400 }), - ), - ).rejects.toThrow("Okta token exchange failed with HTTP 400"); + const failure = await exchangeOktaOboToken( + { + tokenUrl: "https://example.okta.com/oauth2/default/v1/token", + clientId: "client-id", + clientSecret: "client-secret", + subjectToken: "user-subject-token", + audience: "api://orders", + scopes: ["orders.read"], + }, + async () => new Response("server echoed a secret", { status: 400 }), + ).catch((error: unknown) => error as Error); + expect(failure.message).toBe("Okta token exchange failed with HTTP 400"); + expect(failure.message).not.toContain("server echoed a secret");Apply the same pattern in the
it.eachblock, asserting that the message excludes the fixture detail for each case.🤖 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 503 - 544, Strengthen the non-disclosure assertions in the HTTP 400 test and the parameterized “rejects an Okta exchange” test around exchangeOktaOboToken. Capture the rejected error, assert its expected message, and additionally verify that the message excludes the fixture-specific response or network detail such as “server echoed a secret,” “network details,” and “not-json.”Source: Path instructions
docs/reference/configure-runtime-identity.mdx (1)
189-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the OBO conformance scenario in this page.
The "Review the Conformance Evidence" section names
TC-INF-12andTC-INF-13as the conformance gates for this trust boundary. This PR adds theTC-INF-14Okta OBO live scenario intest/e2e/live/runtime-identity-obo-scenario.ts, and that scenario is the gate for the OBO path documented in this new section.Add
TC-INF-14to the conformance evidence so the OBO section states how to verify the behavior it documents.As per coding guidelines: "Write clear, accurate, task-oriented documentation that explains what to do, when to do it, and how to verify it."
🤖 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/configure-runtime-identity.mdx` around lines 189 - 192, Update the “Review the Conformance Evidence” section in configure-runtime-identity.mdx to include TC-INF-14 as the verification scenario for the documented Okta OBO path, alongside TC-INF-12 and TC-INF-13. Keep the existing evidence references and explain that TC-INF-14 verifies the behavior described in this OBO section.Source: Coding guidelines
nemoclaw/src/blueprint/runtime-identity.ts (1)
841-888: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse form-encoded client credentials and validate the token-exchange response.
- Encode
clientIdandclientSecretwithapplication/x-www-form-urlencodedbefore constructing the Basic credential.encodeURIComponentis not equivalent because spaces require+and several characters have different escaping rules.- Require
document.issued_token_type === OBO_ACCESS_TOKEN_TYPEbefore returning the token. RFC 8693 Section 2.2.1 also requirestoken_type; reject responses that omit either field. Update the exchange fixtures with these fields.🤖 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.ts` around lines 841 - 888, Update exchangeOktaOboToken to form-encode clientId and clientSecret using URLSearchParams semantics before constructing the Basic authorization credential, preserving correct handling of spaces and reserved characters. Extend the response validation to require non-empty token_type and document.issued_token_type equal to OBO_ACCESS_TOKEN_TYPE before returning access_token, and update the exchange fixtures with both required fields.
🤖 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/configure-runtime-identity.mdx`:
- Around line 172-175: Update the OBO profile documentation in the section
describing the Okta token exchange to state that client_secret_env is required
and must name the environment variable containing the confidential client
secret; keep the existing optionality guidance scoped to the refresh profile so
readers do not apply it to OBO.
In `@nemoclaw/src/blueprint/runner-identity.test.ts`:
- Line 905: Update the migration tests around the rollback command assertions at
lines 905, 922, and 980 to also verify that rollbackCommands does not contain
the legacy “sandbox remove” command. Strengthen the responseQueue fixture or the
exact command-sequence assertion near lines 1069–1075 so unexpected commands
cause the test to fail.
In `@nemoclaw/src/blueprint/runner.ts`:
- Around line 1074-1077: Define and enforce the sandbox image contract in the
blueprint runner flow around the createArgs construction: every supported
sandbox.image must guarantee an executable equivalent to /bin/true before the
headless command is appended and the sandbox is created. Validate this
requirement before creation and reject unsupported images with a clear error, or
update the command to use an equivalently guaranteed executable while preserving
the no-tty, leave-running behavior.
In `@nemoclaw/src/blueprint/runtime-identity.ts`:
- Around line 1040-1054: Validate that the flow derived from the runtime
identity configuration agrees with profilePolicy.flow before loading credential
material in prepareRuntimeIdentity. Reject configurations where the presence of
refresh-token fields versus token-exchange fields implies a different flow,
preventing subject tokens from being ignored; preserve the existing
credential-loading behavior for matching flows.
- Around line 174-191: Bind Okta OBO credential delivery to reviewed hosts:
update runtime-identity.ts lines 174-191 to use hostPolicy "reviewed" with an
approved allowlist, unless an accepted scope decision explicitly permits the
public-https exception. In runtime-identity-obo-scenario.ts lines 277-299, if
public-https remains, remove the override and assert that unreviewed hosts are
rejected before bearer delivery.
---
Nitpick comments:
In `@docs/reference/configure-runtime-identity.mdx`:
- Around line 189-192: Update the “Review the Conformance Evidence” section in
configure-runtime-identity.mdx to include TC-INF-14 as the verification scenario
for the documented Okta OBO path, alongside TC-INF-12 and TC-INF-13. Keep the
existing evidence references and explain that TC-INF-14 verifies the behavior
described in this OBO section.
In `@nemoclaw/src/blueprint/runner.ts`:
- Line 1275: Add an exact fixture for the runCmd invocation in the sandbox
cleanup flow, matching openshell sandbox delete with exit code 1 and stderr
“sandbox not found”; verify that both cleanup paths complete successfully
despite this not-found result.
In `@nemoclaw/src/blueprint/runtime-identity.test.ts`:
- Around line 503-544: Strengthen the non-disclosure assertions in the HTTP 400
test and the parameterized “rejects an Okta exchange” test around
exchangeOktaOboToken. Capture the rejected error, assert its expected message,
and additionally verify that the message excludes the fixture-specific response
or network detail such as “server echoed a secret,” “network details,” and
“not-json.”
In `@nemoclaw/src/blueprint/runtime-identity.ts`:
- Around line 841-888: Update exchangeOktaOboToken to form-encode clientId and
clientSecret using URLSearchParams semantics before constructing the Basic
authorization credential, preserving correct handling of spaces and reserved
characters. Extend the response validation to require non-empty token_type and
document.issued_token_type equal to OBO_ACCESS_TOKEN_TYPE before returning
access_token, and update the exchange fixtures with both required fields.
In `@test/blueprint-runtime-identity-lifecycle.test.ts`:
- Around line 357-361: Update the isolation assertion in the test to first
require that the filtered call set contains at least one call, then retain the
existing every-based secret isolation check. Ensure an empty filtered result
fails rather than passing vacuously.
In `@test/e2e/live/runtime-identity-oauth-server.ts`:
- Around line 170-172: Rename grantTypeOk in the token-exchange handling flow to
exchangeConfigured, and update all downstream references, including the recorded
tc-inf-14-token-exchange-requests.json data and scenario assertion, so the field
clearly indicates whether options.tokenExchange is configured rather than
whether the grant type is valid.
🪄 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: 87ca404f-364b-4964-8380-e27f15e299e2
📒 Files selected for processing (16)
docs/reference/architecture.mdxdocs/reference/configure-runtime-identity.mdxnemoclaw-blueprint/provider-profiles/okta-obo-v1.yamlnemoclaw/src/blueprint/runner-identity.test.tsnemoclaw/src/blueprint/runner-name-validation.test.tsnemoclaw/src/blueprint/runner.test.tsnemoclaw/src/blueprint/runner.tsnemoclaw/src/blueprint/runtime-identity.test.tsnemoclaw/src/blueprint/runtime-identity.tsschemas/blueprint.schema.jsontest/blueprint-runtime-identity-lifecycle.test.tstest/blueprint-runtime-identity-schema.test.tstest/docs-refactor-ownership.test.tstest/e2e/live/inference-routing.test.tstest/e2e/live/runtime-identity-oauth-server.tstest/e2e/live/runtime-identity-obo-scenario.ts
| `OKTA_SUBJECT_TOKEN` must contain a current access token for the signed-in user. | ||
| The client ID and client secret identify the confidential Okta client authorized to perform the exchange. | ||
| The runner sends an RFC 8693 token-exchange request to the configured Okta token endpoint. | ||
| It rejects redirects, URL credentials, non-HTTPS endpoints, private addresses, and token hosts outside `okta.com` or `oktapreview.com`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
State that client_secret_env is required for the OBO profile.
isRuntimeIdentityConfig requires client_secret_env to be a string for the token-exchange flow. See nemoclaw/src/blueprint/runtime-identity.ts Line 711. Line 96 of this page states that client_secret_env is optional, and that statement belongs to the refresh profile. A reader who applies that statement to the OBO profile and omits the field receives only Runtime identity configuration is invalid.
Add the requirement to this section.
📝 Proposed addition
`OKTA_SUBJECT_TOKEN` must contain a current access token for the signed-in user.
The client ID and client secret identify the confidential Okta client authorized to perform the exchange.
+`client_secret_env` is required for this profile, unlike the refresh profile.
The runner sends an RFC 8693 token-exchange request to the configured Okta token endpoint.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `OKTA_SUBJECT_TOKEN` must contain a current access token for the signed-in user. | |
| The client ID and client secret identify the confidential Okta client authorized to perform the exchange. | |
| The runner sends an RFC 8693 token-exchange request to the configured Okta token endpoint. | |
| It rejects redirects, URL credentials, non-HTTPS endpoints, private addresses, and token hosts outside `okta.com` or `oktapreview.com`. | |
| `OKTA_SUBJECT_TOKEN` must contain a current access token for the signed-in user. | |
| The client ID and client secret identify the confidential Okta client authorized to perform the exchange. | |
| `client_secret_env` is required for this profile, unlike the refresh profile. | |
| The runner sends an RFC 8693 token-exchange request to the configured Okta token endpoint. | |
| It rejects redirects, URL credentials, non-HTTPS endpoints, private addresses, and token hosts outside `okta.com` or `oktapreview.com`. |
🤖 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/configure-runtime-identity.mdx` around lines 172 - 175, Update
the OBO profile documentation in the section describing the Okta token exchange
to state that client_secret_env is required and must name the environment
variable containing the confidential client secret; keep the existing
optionality guidance scoped to the refresh profile so readers do not apply it to
OBO.
Source: Coding guidelines
| const rollbackCommands = mockExeca.mock.calls.map(([, args]) => (args ?? []).join(" ")); | ||
| expect(rollbackCommands).not.toContain("sandbox stop test-sandbox"); | ||
| expect(rollbackCommands).not.toContain("sandbox remove test-sandbox"); | ||
| expect(rollbackCommands).not.toContain("sandbox delete test-sandbox"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'sandbox (delete|remove)|rollbackCommands|responseQueue' \
nemoclaw/src/blueprint/runner-identity.test.tsRepository: NVIDIA/NemoClaw
Length of output: 13998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test helper and setup ---'
sed -n '1,150p' nemoclaw/src/blueprint/runner-identity.test.ts
printf '%s\n' '--- rollback-related tests ---'
sed -n '845,1085p' nemoclaw/src/blueprint/runner-identity.test.ts
printf '%s\n' '--- production command construction ---'
rg -n -C 5 'sandbox (delete|remove)|actionRollback|Failed to remove owned sandbox' nemoclaw/srcRepository: NVIDIA/NemoClaw
Length of output: 37956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- actionRollback implementation ---'
sed -n '1365,1435p' nemoclaw/src/blueprint/runner.ts
printf '%s\n' '--- complete compensation fixture and assertions ---'
sed -n '1046,1102p' nemoclaw/src/blueprint/runner-identity.test.ts
printf '%s\n' '--- all sandbox removal command references ---'
rg -n -C 3 'sandbox (delete|remove|stop)' nemoclaw/src/blueprintRepository: NVIDIA/NemoClaw
Length of output: 11681
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- targeted diff for the command migration ---'
git diff --unified=8 -- nemoclaw/src/blueprint/runner.ts nemoclaw/src/blueprint/runner-identity.test.ts \
| rg -n -C 8 'sandbox (delete|remove)|responseQueue|rollbackCommands|actionRollback' || true
printf '%s\n' '--- deterministic fallback probe ---'
python3 - <<'PY'
responses = {
"sandbox delete test-sandbox": [
{"exitCode": 1, "stderr": "sandbox delete denied"},
{"exitCode": 0, "stderr": ""},
],
}
fallbacks = {
"sandbox get test-sandbox": {"exitCode": 0, "stderr": ""},
"settings get --global --json": {"exitCode": 0, "stderr": ""},
}
def response(command):
queued = responses.get(command)
if queued:
return queued.pop(0)
return fallbacks.get(command, {"exitCode": 0, "stderr": ""})
print("sandbox remove test-sandbox ->", response("sandbox remove test-sandbox"))
print("sandbox delete test-sandbox ->", response("sandbox delete test-sandbox"))
PYRepository: NVIDIA/NemoClaw
Length of output: 381
Reject the legacy sandbox remove command in migration tests.
Add negative assertions for sandbox remove at lines 905, 922, and 980. Make responseQueue fail on unexpected commands, or assert the exact command sequence in the fixture at lines 1069–1075.
🤖 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/runner-identity.test.ts` at line 905, Update the
migration tests around the rollback command assertions at lines 905, 922, and
980 to also verify that rollbackCommands does not contain the legacy “sandbox
remove” command. Strengthen the responseQueue fixture or the exact
command-sequence assertion near lines 1069–1075 so unexpected commands cause the
test to fail.
Source: Path instructions
| // The OpenShell CLI defaults to an interactive shell when no command is | ||
| // provided. The blueprint runner is headless, so use a no-op command and | ||
| // leave the created sandbox running after that command exits. | ||
| createArgs.push("--no-tty", "--", "/bin/true"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'sandboxCfg\.image|sandbox.*image|/bin/true' \
--glob '*.ts' \
--glob '*.tsx' \
--glob '*.json' \
--glob '*.yaml' \
--glob '*.md' \
.Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
fd -t f -i 'runner\.ts$|dockerfile|compose|readme|contribut' nemoclaw src .github 2>/dev/null | head -200
printf '%s\n' '--- runner symbols and references ---'
rg -n -C 8 'sandboxCfg|sandboxImage|createArgs|/bin/true|actionApply' nemoclaw/src/blueprint nemoclaw 2>/dev/null | head -500
printf '%s\n' '--- image and command contract references ---'
rg -n -C 4 --glob '*.ts' --glob '*.tsx' --glob '*.md' --glob '*.yaml' --glob '*.yml' --glob 'Dockerfile*' \
'sandboxImage|sandboxCfg\.image|openshell.*(command|image)|sandbox create|--no-tty|OpenShell CLI|/bin/(true|sh)|command contract' \
nemoclaw src .github 2>/dev/null | head -800Repository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- NemoClaw blueprint files ---'
git ls-files nemoclaw | head -300
printf '%s\n' '--- runner imports, types, and apply flow ---'
sed -n '1,180p' nemoclaw/src/blueprint/runner.ts
sed -n '620,780p' nemoclaw/src/blueprint/runner.ts
sed -n '921,1115p' nemoclaw/src/blueprint/runner.ts
printf '%s\n' '--- SandboxConfig and Blueprint declarations ---'
rg -n -C 8 'interface SandboxConfig|type SandboxConfig|interface Blueprint|type Blueprint|components:' nemoclaw/src --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- supported image declarations and blueprint examples ---'
rg -n -C 5 'image:\s*|sandbox:' nemoclaw --glob '*.yaml' --glob '*.yml' --glob '*.json' --glob '*.md' --glob '*.ts' --glob '*.tsx' | head -500Repository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- blueprint schema validation for sandbox.image ---'
rg -n -C 12 'sandboxConfig|sandbox\.image|forward_ports|valid nested component shapes|typeof .*image|image.*string' \
nemoclaw/src/blueprint/runner.ts nemoclaw/src/blueprint/*.test.ts
printf '%s\n' '--- image-specific tests and fixtures ---'
rg -n -C 10 'sandbox.*image|image.*sandbox|custom.*image|minimal.*image|--from|/bin/true' \
nemoclaw/src/blueprint/*.test.ts nemoclaw/src --glob '*.ts' --glob '*.md'
printf '%s\n' '--- blueprint documentation outside source ---'
git ls-files | rg -i 'blueprint|openshell|sandbox' | head -300Repository: NVIDIA/NemoClaw
Length of output: 50371
🌐 Web query:
Official NVIDIA OpenShell documentation for openshell sandbox createcommand execution semantics, including whether the command after-- runs inside the selected image and whether the image must contain that executable
💡 Result:
The NVIDIA OpenShell openshell sandbox create command execution semantics are as follows: Execution Semantics Everything provided after the -- separator in the openshell sandbox create command is passed verbatim to the sandbox container [1][2]. Once the sandbox environment is ready, the OpenShell sandbox supervisor launches this specified command as the restricted, unprivileged sandbox user [3]. Requirement for Executable Yes, the image used for the sandbox must contain the executable or command you intend to run [1]. The command is executed within the context of the container's filesystem and environment [3]. If you specify a command that is not present or not executable within that container image, the command will fail to execute [2][3]. Additional Context - If no command is provided after --, the sandbox defaults to launching an interactive shell [2]. - The -- separator is optional if no additional arguments or flags are needed for the command, but it is standard practice to use it to clearly delineate the command that should run inside the container [1][4][2]. - The sandbox environment is prepared by a supervisor that applies necessary security policies, file access restrictions, and network namespaces before executing the specified command [3]. Top results: [1][2][3]
Citations:
- 1: https://nvidia-openshell.mintlify.app/reference/cli-sandbox
- 2: https://mintlify.wiki/NVIDIA/OpenShell/reference/cli-sandbox
- 3: https://github.com/NVIDIA/OpenShell/blob/abe42fb5/architecture/sandbox.md
- 4: https://github.com/nvidia/openshell/blob/HEAD/.agents/skills/openshell-cli/cli-reference.md
Define the sandbox image contract. sandbox.image accepts any string, but OpenShell executes /bin/true inside that image. Images without this executable cause actionApply to fail. Require /bin/true in every supported image or validate an equivalent guaranteed command before creation.
🤖 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/runner.ts` around lines 1074 - 1077, Define and
enforce the sandbox image contract in the blueprint runner flow around the
createArgs construction: every supported sandbox.image must guarantee an
executable equivalent to /bin/true before the headless command is appended and
the sandbox is created. Validate this requirement before creation and reject
unsupported images with a clear error, or update the command to use an
equivalently guaranteed executable while preserving the no-tty, leave-running
behavior.
| "okta-obo-v1": Object.freeze({ | ||
| providerType: "okta-obo-v1", | ||
| clientIdEnvironmentName: "OKTA_CLIENT_ID", | ||
| flow: "oauth2-token-exchange", | ||
| dnsResolution: "identity-platform-controlled", | ||
| tokenIssuer: Object.freeze({ | ||
| trustedHostnames: Object.freeze([]), | ||
| trustedHostSuffixes: Object.freeze(["okta.com"]), | ||
| trustedHostSuffixes: Object.freeze(["okta.com", "oktapreview.com"]), | ||
| }), | ||
| credentialDelivery: Object.freeze({ | ||
| method: "GET", | ||
| path: "/**", | ||
| hostPolicy: "public-https", | ||
| trustedHostnames: Object.freeze([]), | ||
| trustedHostSuffixes: Object.freeze(["okta.com"]), | ||
| trustedHostSuffixes: Object.freeze([]), | ||
| }), | ||
| trustedBinaries: RUNTIME_IDENTITY_BINARIES, | ||
| }), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- policy definitions and hostname enforcement ---'
sed -n '120,220p' nemoclaw/src/blueprint/runtime-identity.ts
rg -n -C 8 'requireTrustedProfileHostname|hostPolicy|trustedHostnames|credentialDelivery|profilePolicy|runtimeIdentityProfilePolicy' nemoclaw/src/blueprint/runtime-identity.ts
printf '%s\n' '--- live scenario policy override and assertions ---'
sed -n '250,330p' test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -C 10 'TC-INF-14|runtimeIdentityProfilePolicy|unreviewed|bearer|credential delivery|credentialDelivery' test/e2e/live/inference-routing.test.ts test/e2e/live/runtime-identity-obo-scenario.ts
printf '%s\n' '--- comparable provider policies ---'
rg -n -C 14 '"okta-runtime-v1"|"entra-runtime-v1"|"okta-obo-v1"' nemoclaw/src/blueprint/runtime-identity.ts
printf '%s\n' '--- scope and ownership references ---'
rg -n -i -C 3 'okta|obo|runtime identity|scope decision|Community Solutions|ownership|lifecycle|compatibility' --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '120,220p' nemoclaw/src/blueprint/runtime-identity.ts
rg -n -C 8 'requireTrustedProfileHostname|hostPolicy|trustedHostnames|credentialDelivery|profilePolicy|runtimeIdentityProfilePolicy' nemoclaw/src/blueprint/runtime-identity.ts
sed -n '250,330p' test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -C 10 'TC-INF-14|runtimeIdentityProfilePolicy|unreviewed|bearer|credentialDelivery' test/e2e/live/inference-routing.test.ts test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -C 14 '"okta-runtime-v1"|"entra-runtime-v1"|"okta-obo-v1"' nemoclaw/src/blueprint/runtime-identity.ts
rg -n -i -C 3 'okta|obo|runtime identity|scope decision|Community Solutions|ownership|lifecycle|compatibility' --glob '!node_modules' --glob '!dist' --glob '!build' .Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- runtime identity symbols ---'
rg -n 'requireTrustedProfileHostname|profilePolicy|hostPolicy|credentialDelivery|trustedHostnames|runtimeIdentityDeps|prepareRuntimeIdentity|deliver' nemoclaw/src/blueprint/runtime-identity.ts
printf '%s\n' '--- runtime identity implementation slices ---'
for range in '300,430p' '600,760p' '800,930p' '1000,1080p' '1120,1180p'; do
printf '\n--- lines %s ---\n' "$range"
sed -n "$range" nemoclaw/src/blueprint/runtime-identity.ts
done
printf '%s\n' '--- live scenario and gate ---'
rg -n -C 12 'TC-INF-14|runtimeIdentityProfilePolicy|hostPolicy|trustedHostnames|unreviewed|bearer|credential' \
test/e2e/live/runtime-identity-obo-scenario.ts \
test/e2e/live/inference-routing.test.ts
printf '%s\n' '--- likely scope or ownership files ---'
git ls-files | rg -i '(^|/)(scope|decision|contribut|community|okta|identity|runtime).*(md|ya?ml|json|ts)$' | head -200Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
rg -n 'requireTrustedProfileHostname|profilePolicy|hostPolicy|credentialDelivery|trustedHostnames|runtimeIdentityDeps|prepareRuntimeIdentity|deliver' nemoclaw/src/blueprint/runtime-identity.ts
for range in '300,430p' '600,760p' '800,930p' '1000,1080p' '1120,1180p'; do
printf '\n--- lines %s ---\n' "$range"
sed -n "$range" nemoclaw/src/blueprint/runtime-identity.ts
done
rg -n -C 12 'TC-INF-14|runtimeIdentityProfilePolicy|hostPolicy|trustedHostnames|unreviewed|bearer|credential' \
test/e2e/live/runtime-identity-obo-scenario.ts \
test/e2e/live/inference-routing.test.ts
git ls-files | rg -i '(^|/)(scope|decision|contribut|community|okta|identity|runtime).*(md|ya?ml|json|ts)$' | head -200Repository: NVIDIA/NemoClaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact OBO assertions after apply ---'
sed -n '407,510p' test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -C 8 'TC-INF-14|runRuntimeIdentityOboE2EScenario|6871' test/e2e/live/inference-routing.test.ts test/e2e/live/runtime-identity-obo-scenario.ts
printf '%s\n' '--- exact scope and integration references ---'
rg -n -i -C 5 'issue: 6871|`#6871`|okta-obo-v1|okta OBO|scope decision|accepted scope|Community Solutions' \
README.md CONTRIBUTING.md WRITING.md docs nemoclaw test .github \
--glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
--glob '*.md' --glob '*.ts' --glob '*.yml' --glob '*.yaml' --glob '*.json' \
| head -500Repository: NVIDIA/NemoClaw
Length of output: 41202
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '407,510p' test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -C 8 'TC-INF-14|runRuntimeIdentityOboE2EScenario|6871' \
test/e2e/live/inference-routing.test.ts \
test/e2e/live/runtime-identity-obo-scenario.ts
rg -n -i -C 5 'issue: 6871|`#6871`|okta-obo-v1|okta OBO|scope decision|accepted scope|Community Solutions' \
README.md CONTRIBUTING.md WRITING.md docs nemoclaw test .github \
--glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
--glob '*.md' --glob '*.ts' --glob '*.yml' --glob '*.yaml' --glob '*.json' \
| head -500Repository: NVIDIA/NemoClaw
Length of output: 41114
Unbounded Credential-delivery Destination (CWE-522): Insufficiently Protected Credentials
Reachability: Internal · Exploitability: Difficult
Reachability path
● Entry
nemoclaw/src/blueprint/runner.ts:921
actionApply: Code-only conformance-test seam. No CLI flag or environment input populates this policy.
│
▼
● Sink
nemoclaw/src/blueprint/runtime-identity.ts
Bind Okta OBO credential delivery to reviewed hosts. public-https skips requireTrustedProfileHostname, so an okta-obo-v1 profile can send the delegated bearer to any HTTPS endpoint. TC-INF-14 overrides this with a pinned reviewed policy and tests method denial, not host denial.
- Require an accepted scope decision for this exception, or change
nemoclaw/src/blueprint/runtime-identity.ts#L174-L191tohostPolicy: "reviewed"with an approved allowlist. - If
public-httpsremains, remove the override intest/e2e/live/runtime-identity-obo-scenario.ts#L277-L299and assert that an unreviewed host is rejected before bearer delivery.
📍 Affects 2 files
nemoclaw/src/blueprint/runtime-identity.ts#L174-L191(this comment)test/e2e/live/runtime-identity-obo-scenario.ts#L277-L299
🤖 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.ts` around lines 174 - 191, Bind Okta
OBO credential delivery to reviewed hosts: update runtime-identity.ts lines
174-191 to use hostPolicy "reviewed" with an approved allowlist, unless an
accepted scope decision explicitly permits the public-https exception. In
runtime-identity-obo-scenario.ts lines 277-299, if public-https remains, remove
the override and assert that unreviewed hosts are rejected before bearer
delivery.
Source: Coding guidelines
| const isRefreshFlow = profilePolicy.flow === "oauth2-refresh-token"; | ||
| const refreshToken = | ||
| isRefreshFlow && config.refresh_token_env | ||
| ? requiredEnvironmentValue(config, config.refresh_token_env, env) | ||
| : undefined; | ||
| const clientSecret = config.client_secret_env | ||
| ? requiredEnvironmentValue(config, config.client_secret_env, env) | ||
| : undefined; | ||
| const subjectToken = | ||
| !isRefreshFlow && config.subject_token_env | ||
| ? requiredEnvironmentValue(config, config.subject_token_env, env) | ||
| : undefined; | ||
| const tokenExchangeUrl = isRefreshFlow | ||
| ? undefined | ||
| : await requireOktaOBOConfig(config, profilePolicy, deps); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject a configuration whose flow fields disagree with the profile policy flow.
isRuntimeIdentityConfig derives the flow from the configuration fields, but prepareRuntimeIdentity derives isRefreshFlow from profilePolicy.flow. Nothing checks that the two agree.
Trace a blueprint that sets subject_token_env, token_url, audience, and scopes while it selects provider_type: okta-runtime-v1:
isRuntimeIdentityConfigaccepts the configuration as a token-exchange configuration.profilePolicy.flowisoauth2-refresh-token, soisRefreshFlowbecomestrue.refreshTokenstaysundefinedbecauseconfig.refresh_token_envis absent.- No token exchange runs, and the refresh block at Line 1151 is skipped because
config.refresh_token_envis absent. - Apply creates the provider with
--runtime-credentials, reports success, and persists an ownership receipt.
The result is an owned provider that holds no credential and no refresh configuration. The subject token is silently ignored. Add an explicit flow-agreement check before the runner loads any credential material.
🐛 Proposed fix to require flow agreement
const clientId = requiredEnvironmentValue(config, config.client_id_env, env);
const isRefreshFlow = profilePolicy.flow === "oauth2-refresh-token";
+ if (isRefreshFlow !== (config.refresh_token_env !== undefined)) {
+ throw new Error(
+ `Runtime identity configuration flow does not match the reviewed '${profilePolicy.flow}' ` +
+ `flow for provider type '${config.provider_type}'`,
+ );
+ }
const refreshToken =
isRefreshFlow && config.refresh_token_env
? requiredEnvironmentValue(config, config.refresh_token_env, env)
: undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isRefreshFlow = profilePolicy.flow === "oauth2-refresh-token"; | |
| const refreshToken = | |
| isRefreshFlow && config.refresh_token_env | |
| ? requiredEnvironmentValue(config, config.refresh_token_env, env) | |
| : undefined; | |
| const clientSecret = config.client_secret_env | |
| ? requiredEnvironmentValue(config, config.client_secret_env, env) | |
| : undefined; | |
| const subjectToken = | |
| !isRefreshFlow && config.subject_token_env | |
| ? requiredEnvironmentValue(config, config.subject_token_env, env) | |
| : undefined; | |
| const tokenExchangeUrl = isRefreshFlow | |
| ? undefined | |
| : await requireOktaOBOConfig(config, profilePolicy, deps); | |
| const isRefreshFlow = profilePolicy.flow === "oauth2-refresh-token"; | |
| if (isRefreshFlow !== (config.refresh_token_env !== undefined)) { | |
| throw new Error( | |
| `Runtime identity configuration flow does not match the reviewed '${profilePolicy.flow}' ` + | |
| `flow for provider type '${config.provider_type}'`, | |
| ); | |
| } | |
| const refreshToken = | |
| isRefreshFlow && config.refresh_token_env | |
| ? requiredEnvironmentValue(config, config.refresh_token_env, env) | |
| : undefined; | |
| const clientSecret = config.client_secret_env | |
| ? requiredEnvironmentValue(config, config.client_secret_env, env) | |
| : undefined; | |
| const subjectToken = | |
| !isRefreshFlow && config.subject_token_env | |
| ? requiredEnvironmentValue(config, config.subject_token_env, env) | |
| : undefined; | |
| const tokenExchangeUrl = isRefreshFlow | |
| ? undefined | |
| : await requireOktaOBOConfig(config, profilePolicy, deps); |
🤖 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.ts` around lines 1040 - 1054,
Validate that the flow derived from the runtime identity configuration agrees
with profilePolicy.flow before loading credential material in
prepareRuntimeIdentity. Reject configurations where the presence of
refresh-token fields versus token-exchange fields implies a different flow,
preventing subject tokens from being ignored; preserve the existing
credential-loading behavior for matching flows.
|
Thanks for the implementation and validation work. We’re closing this PR because the accepted scope in #6871 explicitly keeps RFC 8693 OBO outside NemoClaw until a generic dynamic identity-source capability exists in OpenShell (NVIDIA/OpenShell#1736). This PR performs an Okta-specific exchange in the NemoClaw host, which is the architectural responsibility we chose not to add here. The current revision also has unresolved security and correctness findings. We can revisit OBO through a new PR after the upstream capability lands and maintainers explicitly approve the expanded scope. |
|
Understood. I’ll preserve the branch and validation artifacts so we can revisit this once OpenShell has the generic dynamic identity-source capability and the NemoClaw scope is reopened. Thanks for clarifying the ownership boundary. |
4 participants
Summary
Adds an experimental Okta on-behalf-of reference to the direct blueprint runner. A user supplies a current Okta subject token on the host; NemoClaw exchanges it once, gives the resulting delegated token to OpenShell for custody, and leaves only an opaque placeholder in the sandbox.
This uses the current OpenShell provider API and does not require an OpenShell token-exchange implementation.
Related Issue
Relates to #6871.
Changes
okta-obo-v1profile and blueprint schema for an RFC 8693 exchange using a host-provided subject token, confidential-client credentials, audience, and scopes.openshell provider createsubprocess; subject tokens and client secrets do not enter sandbox state, command arguments, or run plans.openshell sandbox deletelifecycle command.Type of Change
Quality Gates
npm run test:fasthad 19 unrelated environment failures; no waiver requested yetDocumentation Writer Review
blockeddocs/reference/architecture.mdx;docs/reference/configure-runtime-identity.mdxDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHub — commits are SSH-signed, but GitHub currently reportsunknown_keypre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm --prefix nemoclaw test -- src/blueprint/runtime-identity.test.ts src/blueprint/runner-identity.test.ts(145 passed);npm run test:coverage:plugin(787 passed; 95.36% statements)npm run test:fastran 15,197 tests: 15,139 passed, 39 skipped, and 19 unrelated local environment tests failed or timed out (missing PyYAML/systemd and process timeout fixtures)npm run docsbuilds without warnings (doc changes only) — passed with two pre-existing Fern warningsManual Okta validation
Validated against the existing Okta partner tenant with OpenShell main at
0a3ec7a1:afournier@nvidia.com, audienceapi://default, scoped only toapi:access:read;OKTA_OBO_ACCESS_TOKENplaceholder;/whoamiendpoint from the sandbox and confirmed OpenShell injected the delegated JWT at the proxy boundary;POSTfailed closed with HTTP 403;The smoke used
--no-verifyonly for an unrelated dummy inference route. Runtime identity policy, exchange, attachment, proxy injection, denial, and rollback used the current OpenShell interfaces unchanged.Signed-off-by: Alex Fournier afournier@nvidia.com
Summary by CodeRabbit
New Features
okta-obo-v1provider profile with approved endpoint, method, and tool restrictions.Documentation