Skip to content

fix(onboard): reuse the gateway web-search credential on rebuild recreate - #8774

Merged
apurvvkumaria merged 5 commits into
mainfrom
fix/8717-rebuild-websearch-credential-reuse
Aug 11, 2026
Merged

fix(onboard): reuse the gateway web-search credential on rebuild recreate#8774
apurvvkumaria merged 5 commits into
mainfrom
fix/8717-rebuild-websearch-credential-reuse

Conversation

@Dongni-Yang

@Dongni-Yang Dongni-Yang commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Problem

nemoclaw <name> rebuild --yes destroys the sandbox during preflight and then fails to recreate it:

Brave Search requires BRAVE_API_KEY or a saved Brave Search credential in non-interactive mode.
Recreate failed after sandbox was destroyed.

Exit code 1, sandbox left destroyed. Reported on Ubuntu 24.04 / v0.0.105; the code is platform-independent.

Root cause

resolveWebSearchForCreation required two proofs before reusing the credential already registered with the sandbox's OpenShell gateway provider: a session-scoped staged receipt and an exact live gateway binding match.

The first is structurally unobtainable on rebuild:

  • runRebuildRecreatePhase overwrites the session with createSession({...}) and rebuilds the checkpoint from deriveCheckpointFromSession before calling onboard --resume (rebuild-recreate-phase.ts:151-210), so stagedCredentialProviders is [] and the checkpoint bindings are empty.
  • The only writer of that array runs in registerCompletedCredentialProviders at sandbox.ts:1930/1943after the read at :1892.

So the reuse branch was unreachable on every rebuild, and control fell through to ensureValidatedWebSearchCredential, which throws under nonInteractive (web-search-flow.ts:283-288) for a host key rebuild can never read.

This is the residual half of #7097. That change taught the preflight to reuse the gateway binding (canReuseGatewayWebSearchCredential) on the stated premise that "recreate reuses that gateway binding … so the preflight accepts the same binding instead of demanding a host key the recreate will never read." That premise holds for the rest of recreate but not for this one gate — which is why preflight destroys the sandbox and recreate then refuses.

It also contradicts the shipped contract in recover-rebuild-sandboxes.mdx:216, which promises reuse on provider name + type + credential key with no receipt clause, and matches messaging-prep.ts:101-110, which already accepts the live binding alone for this same provider.

Fix

Accept the recreate journal the replacement driver hands off as the equivalent durable-ownership proof, in a new ownsGatewayWebSearchProvider: the run must have been handed the journal (recreateJournalTargetIntentFingerprint, set only by rebuild-recreate-phase.ts:271), and that journal must name this sandbox, carry the same target-intent fingerprint, and be past the delete boundary.

A journal merely resident in the session is deliberately not accepted — nothing binds it to this run: one survives a failed attempt, and beginSandboxRecreateTransaction opens one straight at deleted when the sandbox is already missing.

The live gateway binding check is unchanged and still ANDed, so neither proof can reuse a provider bound to anything but this sandbox on this gateway. A host credential, when present, still takes precedence over both.

Nothing downstream needed to change: preflightRequiredCredentialProviderBindings accepts the surviving matching provider with no planned token, the token-less def is filtered before upsert, and allRequiredBindingsLive is satisfied by the live binding.

Scope

  • src/lib/onboard/machine/handlers/sandbox.ts — the gate and its rationale.
  • src/lib/onboard/lifecycle-contracts.md — record the alternative ownership proof and why a resident journal is not one.
  • src/lib/onboard/machine/handlers/sandbox-rebuild-web-search-reuse.test.ts — new focused file, 10 cases.

Not in scope: the same shape exists on the messaging side, in the credentialNeedsValidation predicate in selectionFromCompletedMessagingCheckpoint (sandbox-messaging.ts:353). It is not reachable on the rebuild pass itself — messagingDecision gates on sandboxPromptProgress.messaging, which rebuild's session reset leaves false, so reconcileSandboxMessaging returns the registry-authoritative plan first. But that pass persists the state that detonates later: checkpointMessaging (sandbox.ts:1286) sets messaging = true unconditionally while nothing is staged without a host token, so a rebuild that fails after that point leaves messaging=completed with stagedCredentialProviders=[], and the recovery command rebuild prints (onboard --resume --name <sb>, rebuild-mcp-phase.ts:145) then throws at messaging-channel-setup.ts:140-146.

That is deliberately left out of this PR: open PR #7853 already rewrites that predicate and disarms it on exactly those routes, so fixing it here would collide. Noted in a comment on #7853 so the behaviour survives if that PR is narrowed. Traced by code inspection, not reproduced at runtime.

Maintainer Reviews

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: docs/manage-sandboxes/recover-rebuild-sandboxes.mdx already documents matching gateway-provider credential reuse and failure remediation; npm run docs:sync-agent-variants passed; documentation build completed with 0 errors and 2 existing Fern warnings
  • Agent: Codex Desktop

Verification

  • New test file: the [Ubuntu 24.04][Sandbox] nemoclaw rebuild fails with missing BRAVE_API_KEY after sandbox destruction despite registered gateway credential #8717 regression case fails on unpatched main and passes with the fix; the other 9 pass on both.
  • Negative cases pin every conjunct: journal not past deleted, journal naming another sandbox, journal not handed to this run, handed journal with a stale target intent, no journal and no receipt, live binding mismatch (fails closed), host key present. The pre-existing staged-receipt path is pinned unchanged.
  • src/lib/onboard/machine/handlers/ — 33 files, 376 tests, all pass. tsc -p tsconfig.src.json clean. npm run lint, checks:repository, test-size:check, test:titles:check, test:imports:check all pass. Biome clean.
  • 51 failures in local-model-profile, setup-nim-flow, command, created-sandbox-finalization, docker-llama-cpp-managed-lifecycle and gateway-restart-hermes-drift are pre-existing — verified byte-identical on unpatched main.

Verified by unit test and by tracing the full recreate path; I have not run a live rebuild against a Brave-backed sandbox, so QA re-verification on hardware is worth having before this is treated as closed.

Closes #8717

Signed-off-by: Dongni Yang dongniy@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved credential reuse during sandbox rebuilds.
    • Reuses web-search credentials only when they are verified against the correct sandbox and provider connection.
    • Revalidates credentials when recovery information is missing, incomplete, mismatched, or invalid.
    • Preserves the correct precedence for host credentials and previously staged credentials.
  • Documentation

    • Clarified recovery and credential-handling behavior for failed or already-completed sandbox rebuilds.

…eate

`nemoclaw <name> rebuild --yes` destroyed the sandbox during preflight and then
failed to recreate it with "Brave Search requires BRAVE_API_KEY ... in
non-interactive mode", leaving no running instance.

Rebuild resets the onboard session and derives a fresh checkpoint before it
calls `onboard --resume`, so `stagedCredentialProviders` is always empty by the
time recreate resolves web search. `resolveWebSearchForCreation` required that
receipt before it would reuse the credential already registered with the
sandbox's OpenShell gateway provider, so its reuse branch was unreachable on
every rebuild and control fell through to non-interactive revalidation of a
host key that rebuild can never read (#7097).

Accept the recreate journal the replacement driver hands off as the equivalent
durable-ownership proof: same sandbox name, same target-intent fingerprint, and
past the delete boundary. A journal merely resident in the session is not that
proof, because nothing binds it to this run. Both proofs stay paired with the
unchanged exact live gateway binding check, so neither can reuse a provider
bound to anything but this sandbox.

This makes the recreate gate agree with the rebuild preflight
(`canReuseGatewayWebSearchCredential`) and `messaging-prep`, which already
accept the live binding alone, and with the documented contract in
docs/manage-sandboxes/recover-rebuild-sandboxes.mdx.

Closes #8717

Signed-off-by: Dongni Yang <dongniy@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Sandbox recreation now validates gateway web-search credential ownership through staged providers or eligible recreate journals. The change documents the contract and adds tests for matching, mismatched, incomplete, absent, and unowned journal conditions.

Changes

Sandbox web-search credential reuse

Layer / File(s) Summary
Recreate-journal ownership validation
src/lib/onboard/lifecycle-contracts.md, src/lib/onboard/machine/handlers/sandbox.ts
The contract documents driver-bound journals, delete-boundary checks, exact gateway bindings, and rejection of unbound session journals. Sandbox recreation uses this validation when reusing gateway credentials.
Credential reuse behavior tests
src/lib/onboard/machine/handlers/sandbox-rebuild-web-search-reuse.test.ts
Tests cover valid reuse, phase and identity mismatches, missing ownership, gateway-binding failures, host-credential precedence, and staged receipt reuse.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#8655: Both changes modify sandbox recreation journaling in sandbox.ts, but address different behaviors.
  • NVIDIA/NemoClaw#8738: This change consumes recreate-journal ownership guarantees introduced by the related credential-reuse work.

Suggested labels: area: onboarding, area: sandbox, bug-fix, v0.0.106

Suggested reviewers: cv, ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #8717 by enabling journal-based credential reuse while preserving gateway binding and host-key precedence checks.
Out of Scope Changes check ✅ Passed The code, regression tests, and contract documentation directly support the linked issue and PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reusing the gateway web-search credential during sandbox rebuild recreation.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/8717-rebuild-websearch-credential-reuse

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

@github-code-quality

github-code-quality Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 27f3c92 in the fix/8717-rebuild-web... branch remains at 96%, unchanged from commit 0cddc38 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 27f3c92 in the fix/8717-rebuild-web... branch remains at 82%, unchanged from commit 0cddc38 in the main branch.

Show a code coverage summary of the most impacted files.
File main 0cddc38 fix/8717-rebuild-web... 27f3c92 +/-
src/lib/inferen...ollama/proxy.ts 36% 33% -3%
src/lib/state/config-io.ts 95% 93% -2%
src/lib/adapter...ateway-drift.ts 61% 60% -1%
src/lib/onboard...-transaction.ts 94% 94% 0%
src/lib/onboard...lers/sandbox.ts 97% 97% 0%
src/lib/credentials/store.ts 57% 57% 0%
src/lib/onboard...eway-process.ts 90% 90% 0%
src/lib/sandbox...rce-identity.ts 86% 86% 0%
src/lib/actions...ence-gateway.ts 82% 91% +9%
src/lib/actions...ocker-health.ts 61% 78% +17%

Updated August 11, 2026 07:51 UTC

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Failed

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • define — durable-ownership evidence at src/lib/onboard/machine/handlers/sandbox.ts:1111: Define this phrase at first use as the matching handed-off recreate journal evidence required for credential reuse.
  • established — recreate journal at src/lib/onboard/lifecycle-contracts.md:243: Keep the established term and state the handoff, matching identity, and delete-phase conditions where journal authority matters.
  • define — target-intent fingerprint at src/lib/onboard/lifecycle-contracts.md:243: Define the term where it first appears as the journal value that must match the replacement driver's handoff value.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

Manual-only E2E: cloud-onboard, onboard-repair, onboard-resume, openshell-gateway-upgrade, ubuntu-repo-cloud-langchain-deepagents-code
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

2 optional E2E recommendations
  • rebuild-openclaw
  • brave-search

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Security review

Verdict: PASS — no findings.

I reviewed branch revision 9c95da725 and the full three-file change.

  1. Secrets and credentials — PASS. The recreate path reuses only the registered gateway binding; it does not read, copy, or log the Brave credential value.
  2. Input validation and sanitization — PASS. Journal reuse requires the same sandbox name, the same target-intent fingerprint, and a phase at or beyond the delete boundary. A merely resident or stale journal fails closed.
  3. Authentication and authorization — PASS. The ownership proof remains paired with the live provider name, provider type, credential environment key, sandbox, and gateway binding checks.
  4. Dependencies and third-party libraries — PASS. No dependency, image, registry, or lockfile changes.
  5. Error handling and logging — PASS. Host credentials still take precedence, mismatches continue through normal validation, and no credential contents enter diagnostics.
  6. Cryptography and data protection — PASS. No cryptographic or data-storage format changes. Existing provider secrets remain gateway-owned.
  7. Configuration and security headers — PASS. No network policy, endpoint, environment contract, or security-header change.
  8. Security testing — PASS. Ten focused cases cover valid handoff plus stale phase, wrong sandbox, missing handoff, stale target intent, absent proof, binding mismatch, and host-key precedence. All 376 handler tests and CLI type-checking pass.
  9. System security — PASS. Reuse is authorized only after the replacement driver hands off durable recreate state for the same destructive transaction; unrelated or replayed state cannot authorize another sandbox.

The change is moderate at 279 additions and 4 deletions, so no large-LOC flag applies. The contributor commit is signed off and GitHub marks it Verified.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

CI disposition for branch revision 9c95da725:

The PR-owned validation remains green: 10 focused regression cases, all 376 onboarding-handler tests, CLI type-checking, formatting, and documentation review pass.

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rebuild recreate path now reuses a web-search credential only when the driver-provided journal matches the sandbox and target intent, has reached deletion, and the live gateway binding matches exactly. Host credentials still take precedence. Focused tests cover the allowed path and each new rejection condition.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Security review: PASS

Reviewed the complete PR diff through branch revision 27f3c92c0 against current main (0cddc38c8). The security-sensitive change adds a second durable-ownership proof for reusing an existing web-search gateway credential during sandbox replacement.

Review checklist

  1. Authentication and authorization: Reuse remains limited to the same sandbox-derived provider name and requires both durable ownership evidence and the existing live gateway binding check. A host credential, when available, still takes precedence.
  2. Input validation and injection: The handoff fingerprint must be present, the journal must name the selected sandbox, and its target-intent fingerprint must match the driver handoff. No new command, shell, template, or query construction is introduced.
  3. Secrets and credentials: The journal and checkpoint carry identifiers and fingerprints, not credential values. The change reuses a credential already held by the matching gateway provider and does not expose or copy the secret to the host session.
  4. Cryptography and integrity: The target-intent fingerprint comparison binds the handoff to the intended replacement. A mismatched or stale fingerprint fails closed.
  5. Network and SSRF: No endpoint, URL, allowlist, DNS, proxy, or SSRF-validation change. The existing provider/type/credential-key live binding check remains required.
  6. Filesystem and state safety: A journal merely resident in the session is insufficient. The driver must hand the journal to this run, it must match the sandbox, and it must have reached the delete boundary before reuse is allowed.
  7. Sandbox and process safety: The alternative proof applies only during resume/recreate after deletion. Missing handoff, wrong sandbox, wrong phase, wrong intent, missing receipt, or live-binding mismatch all stop reuse.
  8. Supply chain and dependencies: No dependency, workflow, image, action, lockfile, or build-provenance changes.
  9. Privacy, logging, and denial of service: No new user data, telemetry, logging, retry loop, or unbounded work is introduced.

Verification

  • Focused rebuild web-search reuse suite: 10/10 passed on the current branch revision.
  • The focused cases cover every new conjunct plus host-key precedence and the existing staged-receipt path.
  • Formatting and CLI type-check passed locally.
  • GitHub CI, all 12 CLI shards, managed-runtime activation, CodeQL, GPT advisor, CodeRabbit, DCO, and commit verification passed. The only non-success is an advisor-analysis infrastructure failure being retried once.
  • All five commits are signed and appear as Verified in GitHub.
  • Diff size: 3 files, +279/-4. This is not a large LOC increase.

No security findings or security-review blocker remain for this branch revision.

@apurvvkumaria
apurvvkumaria merged commit cd80922 into main Aug 11, 2026
80 of 82 checks passed
@apurvvkumaria
apurvvkumaria deleted the fix/8717-rebuild-websearch-credential-reuse branch August 11, 2026 09:14
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.

[Ubuntu 24.04][Sandbox] nemoclaw rebuild fails with missing BRAVE_API_KEY after sandbox destruction despite registered gateway credential

3 participants