Skip to content

fix(provider): isolate unbound static credentials - #2862

Open
prekshivyas wants to merge 1 commit into
NVIDIA:mainfrom
prekshivyas:codex/fix-profileless-credential-isolation
Open

fix(provider): isolate unbound static credentials#2862
prekshivyas wants to merge 1 commit into
NVIDIA:mainfrom
prekshivyas:codex/fix-profileless-credential-isolation

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Aug 21, 2026

Copy link
Copy Markdown

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 None branch; this PR applies the same independent-withholding rationale to that missed case without asserting that #2510 already approved this exact implementation.

Changes

  • Preserve provider resolution behavior and sanitize the completed environment only when the requesting supervisor advertises static-credential-binding support.
  • Remove each unbound static key independently from the environment, expiry metadata, and static-key set before returning the snapshot.
  • Retain bound static credentials and valid dynamic credentials when an unrelated static key is unbound.
  • Preserve the existing legacy-supervisor behavior, which withholds all static credentials when binding metadata is unsupported.

Testing

  • mise run pre-commit passes (mise is unavailable in this environment)
  • Unit tests added/updated
  • E2E tests added/updated (not applicable; this is covered at the provider-environment boundary)

Verification performed:

  • cargo fmt --check
  • cargo clippy -p openshell-server --features bundled-z3 --lib -- -D warnings
  • cargo test -p openshell-server --features bundled-z3 --lib (1,408 passed, 0 failed, 7 ignored)
  • GitHub Branch Checks on 3596078a: Rust passed on both linux-amd64-cpu8 and linux-arm64-cpu8; all 21 applicable checks succeeded.
  • Regression coverage verifies that a profileless OPENAI_API_KEY is withheld while a bound GITHUB_TOKEN remains active, and that an unbound static key does not suppress an unrelated dynamic credential.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable; no public contract or architecture change)

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@prekshivyas

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas
prekshivyas force-pushed the codex/fix-profileless-credential-isolation branch from 942f8ef to 3596078 Compare August 21, 2026 06:38
@prekshivyas prekshivyas changed the title fix(provider): isolate profileless static credentials fix(provider): isolate unbound static credentials Aug 21, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant