fix(provider): isolate unbound static credentials - #2862
Open
prekshivyas wants to merge 1 commit into
Open
Conversation
prekshivyas
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
August 21, 2026 06:19
|
All contributors have signed the DCO ✍️ ✅ |
Author
|
I have read the DCO document and I hereby sign the DCO. |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
prekshivyas
force-pushed
the
codex/fix-profileless-credential-isolation
branch
from
August 21, 2026 06:38
942f8ef to
3596078
Compare
21 tasks
cjagwani
added a commit
to NVIDIA/NemoClaw
that referenced
this pull request
Aug 21, 2026
<!-- markdownlint-disable MD041 --> ## Summary Restore the PR exact OpenClaw MCP qualification path end to end. The job now onboards the exact managed image, preserves a classified MCP credential environment alongside gateway-only inference credentials, and performs authenticated discovery with the live revision-scoped OpenShell placeholder. ## Root cause The same two five-phase qualification jobs were genuinely green before the dependency upgrade. The last clean pre-upgrade pair, [run 32332905722](https://github.com/NVIDIA/NemoClaw/actions/runs/32332905722) at `ab5db717b`, installed OpenShell 0.0.101 and passed without a waiver. [NemoClaw PR #9192](#9192) then merged at 2026-08-19 22:29 PDT and upgraded OpenShell directly from 0.0.101 to 0.0.106. The first exact pair based on 0.0.106, [run 32338412376](https://github.com/NVIDIA/NemoClaw/actions/runs/32338412376), failed phase 3 in both passes with no successful MCP request. The causal OpenShell change is [OpenShell PR #2510](NVIDIA/OpenShell#2510), merged as `0120535ef`, which introduced endpoint-bound static credential snapshots. A profileless inference provider contributes a static environment key without binding or non-secret classification; the binding-capable supervisor rejects that snapshot as `provider environment contains an unclassified credential key` and revokes the otherwise correctly bound MCP credential too. The failures occurred at five successive boundaries: 1. The PR MCP child environment dropped the managed-image catalog and activation inputs. Onboarding therefore built a Dockerfile image instead of qualifying the candidate image. 2. With the exact image active, OpenShell 0.0.106 emitted the legacy `openai` provider credential without selected-profile binding metadata. The supervisor rejected the provider environment as containing an unclassified credential and atomically withheld the MCP static credential too. 3. After classifying the inference provider with an endpointless profile, the discovery runtime still synthesized `openshell:resolve:env:<KEY>`. OpenShell's bound resolver requires the current live revision-scoped value (`openshell:resolve:env:v<revision>_<KEY>`), so discovery received HTTP 500 before any request reached the fake MCP server. 4. Once diagnostic discovery used the live revision and successfully listed `fake_echo` and `fake_status`, the managed mcporter config still persisted the canonical unversioned placeholder. The direct agent-adapter proof therefore received HTTP 500 even though the diagnostic path was green. 5. After both diagnostic discovery and direct mcporter discovery passed, the test entered a separate trusted-private DNS-rebinding fixture. That fixture rewrote `/etc/hosts` on the runner and sandbox, but OpenShell resolves egress in the Docker supervisor namespace. The supervisor never observed the fake hostname mapping, rejected the connection before it reached the server, and the negative-only raw probe had previously passed for the same wrong reason. 6. Once both passes reached the rebuild lifecycle, the 0.0.106 migration's pre-delete `removeGeneratedPolicy()` correctly removed `mcp-bridge-fake`, but the captured policy selection still handed that generated name to inner onboarding and generic policy replay. The first correction normalized the rebuild session, but resumed sandbox creation then overwrote it from the intentionally preserved crash-recovery registry row. Recreate therefore still failed deterministically with `Preset not found: mcp-bridge-fake` before the dedicated MCP restore phase could reattach the provider, generated policy, and adapter. This is the normal host-gateway / one-container-per-sandbox topology. No custom MCP sidecar is involved. ## Changes - Preserve the workflow-owned managed-image catalog, candidate SHA, live qualification flag, and supervisor image across the MCP child-process boundary. - Activate onboarding through `--temp-managed-runtime` and `--temp-managed-runtime-catalog`, then require the sandbox receipt to identify the exact candidate revision. - Import an endpointless, inference-capable `openai` profile before the endpointless MCP profile so OpenShell can classify gateway-only inference credentials without injecting them into workloads. - When `openai` already exists, export it and require the exact gateway-only boundary: `id: openai`, empty credentials/endpoints/binaries, and `inference_capable: true`. Fail closed before MCP policy or provider mutation on export failure, malformed output, or a mismatch. - Make MCP discovery read the fresh process environment and accept only a canonical or revision-scoped OpenShell placeholder for the declared key. Raw, wrong-key, malformed, missing, and injected values fail closed and never enter argv, output, or a request. - Return the bounded live credential revision from the attachment-readiness proof and project that exact revision into managed mcporter configuration. Post-write registration inspection now requires the same readiness-proven revision (`v12` cannot verify as `v11`); canonical status/removal matching remains available only when readiness was canonical. - Qualify every `mcp-bridge-*.ts` change through the PR and main managed-image workflow boundaries so adapter projection changes cannot bypass this live proof. - Scope exact managed-image CI to the topology it actually owns: exact-image onboarding, authenticated public MCP discovery, direct adapter use, endpoint boundaries, credential rotation, restart, and removal. The evidence records `managed-image-discovery`; the job no longer claims trusted-private DNS-rebinding coverage from a runner/sandbox hosts fixture that cannot control the supervisor resolver. Full MCP E2E retains that proof for supervisor-authoritative DNS topologies. - Exclude only the generated policy names already preserved by the MCP rebuild transaction from inner-onboard and generic policy replay. The outer rebuild now carries that normalized selection through an explicit authoritative create intent, so sandbox recreation cannot replace it from the stale source registry row or ambient policy variables. Matching-journal recovery remains a fallback, a journal for another sandbox cannot supply policy state, the crash-recovery registry remains untouched, built-in and operator policy selections remain unchanged, and the dedicated post-rebuild MCP phase remains the sole owner of restoring the provider-bound generated policy and adapter. - Rebuild and pin the reviewed MCP discovery runtime bundle. The `openai` profile is a provisional compatibility path for the pinned 0.0.106 binary, not the intended ownership model. The ownership-free fix is [OpenShell PR #2862](NVIDIA/OpenShell#2862): at the gateway response boundary, remove each static key that lacks binding metadata before sending the snapshot to a binding-capable supervisor. Bound static credentials and valid dynamic credentials remain active, provider resolution stays unchanged, and legacy supervisors retain their existing strip-all behavior. The full 1,415-test server suite passes (1,408 passed, 7 ignored), as do formatting and warning-as-error clippy. After that fix is released and NemoClaw updates its pin, this PR should remove the provisional shared profile and its lifecycle code. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — Senthil explicitly accepted the provisional `openai` profile ownership boundary and approved at `52b2db132`; Ryan's rebuild and exact-revision findings on `75aefaf3b` are addressed by signed commits `a51adb149` and `6c05be2d9`, and the current head awaits re-review. OpenShell PR #2862 remains the ownership-free follow-up. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## 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 pushed commit is signed and DCO-compliant - [x] Normal pre-commit, commit-msg, and pre-push hooks passed - [x] Targeted behavior tests pass for the current change set — the 371-test MCP bridge suite, 217 publication/risk-boundary tests, the current 179-test affected workflow/scope suite, the earlier 149 focused provider, discovery, onboarding, image, build-context, and publication tests, the current 126-test policy/rebuild suite, and the current 98-test adapter/status/crash/restart suite pass; the isolated discovery runtime wire test and typecheck also pass - [x] `npm run typecheck:cli`, `npm run build:cli`, focused Oxlint, formatting, `npm run checks:repository`, and the 32-test growth guard pass - [ ] Applicable broad gate passed — current replacement managed-image run [32468003695](https://github.com/NVIDIA/NemoClaw/actions/runs/32468003695) is pending for signed commit `6c05be2d9`; run [32463784345](https://github.com/NVIDIA/NemoClaw/actions/runs/32463784345) passed the exact-image build and phases 1–3 in both discovery passes, including authenticated `fake_echo`/`fake_status` discovery with `credentialRewriteMatched: true`, then proved that inner sandbox creation still reloaded the stale generated-policy name from the preserved registry; attempt 3 of run [32457422244](https://github.com/NVIDIA/NemoClaw/actions/runs/32457422244) first reproduced that same phase-4 boundary in both passes, run [32454193518](https://github.com/NVIDIA/NemoClaw/actions/runs/32454193518) first exposed rebuild failure, and run [32452343170](https://github.com/NVIDIA/NemoClaw/actions/runs/32452343170) reached live discovery in both passes but was cancelled by a newer push - [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) - [ ] 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: Prekshi Vyas <prekshiv@nvidia.com> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Withhold unbound static credentials at the gateway response boundary instead of sending incomplete binding metadata that makes a binding-capable supervisor reject every static credential in the snapshot.
Related Issue
No issue required: this is a localized fail-closed bug in the static credential binding path. A static credential without a binding cannot be safely projected by a binding-capable supervisor; the bug is that sending that invalid key causes the supervisor to revoke unrelated, correctly bound credentials too.
Design context: OpenShell #2510 states that an endpointless profile without a policy binding contributes non-secret configuration while OpenShell withholds its static credential keys and binding metadata. Its implementation comment explains that sending invalid metadata would let one rejected key revoke unrelated credentials. The released implementation covers the endpointless-profile case but misses the profileless
Nonebranch; this PR applies the same independent-withholding rationale to that missed case without asserting that #2510 already approved this exact implementation.Changes
Testing
mise run pre-commitpasses (miseis unavailable in this environment)Verification performed:
cargo fmt --checkcargo clippy -p openshell-server --features bundled-z3 --lib -- -D warningscargo test -p openshell-server --features bundled-z3 --lib(1,408 passed, 0 failed, 7 ignored)3596078a: Rust passed on bothlinux-amd64-cpu8andlinux-arm64-cpu8; all 21 applicable checks succeeded.OPENAI_API_KEYis withheld while a boundGITHUB_TOKENremains active, and that an unbound static key does not suppress an unrelated dynamic credential.Checklist