-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(sandbox): restore protected stopped sandboxes #8683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cf19198
7b6c98b
ec9a52f
d66690e
33c7501
92490c3
f9cd443
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { cliName } from "../../onboard/branding"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CURRENT_RUNTIME_PROVIDER_BUNDLES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RuntimeProviderBundleRegistry, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -19,9 +20,11 @@ function verifyGateway(sandboxName: string): Promise<void> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return connectSandbox(sandboxName, { probeOnly: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function restoreProcessState(sandboxName: string): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type SandboxStartupRecoveryResult = import("./connect").SandboxStartupRecoveryResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function restoreProcessState(sandboxName: string): SandboxStartupRecoveryResult { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { restoreSandboxStartupState } = require("./connect") as typeof import("./connect"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreSandboxStartupState(sandboxName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return restoreSandboxStartupState(sandboxName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function restoreLockedStartupAccess(sandboxName: string): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -33,28 +36,58 @@ function restoreLockedStartupAccess(sandboxName: string): void { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface SandboxStartupStateDeps { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| agent?: SandboxEntry["agent"]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreLockedStartupAccess?: (sandboxName: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreProcessState?: (sandboxName: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreProcessState?: (sandboxName: string) => SandboxStartupRecoveryResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function restoreStoppedSandboxStartupState( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sandboxName: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deps: SandboxStartupStateDeps = {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): SandboxStartupRecoveryResult { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ((deps.agent ?? "openclaw") === "openclaw") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (deps.restoreLockedStartupAccess ?? restoreLockedStartupAccess)(sandboxName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (deps.restoreProcessState ?? restoreProcessState)(sandboxName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return (deps.restoreProcessState ?? restoreProcessState)(sandboxName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface SandboxStartDeps { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| environment?: NodeJS.ProcessEnv; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getSandbox?: typeof registry.getSandbox; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runtimeProviders?: RuntimeProviderBundleRegistry; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreStartupState?: (sandboxName: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreStartupState?: (sandboxName: string) => SandboxStartupRecoveryResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| verifyGateway?: (sandboxName: string) => Promise<void>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log?: (message: string) => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function startupRecoveryFailure(check: SandboxStartupRecoveryResult): string | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!check.checked) return "managed agent gateway inspection did not complete"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ("runtime" in check && check.runtime === "terminal") return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ("secretBoundaryRefused" in check && check.secretBoundaryRefused) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `secret-boundary refusal: ${String(check.secretBoundaryReason)}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ("mcpReconciliationRefused" in check && check.mcpReconciliationRefused) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `MCP reconciliation refusal: ${String(check.mcpReconciliationReason)}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ("forwardRecoveryFailed" in check && check.forwardRecoveryFailed) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return String(check.forwardRecoveryFailureDetail); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (check.wasRunning || check.recovered) return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const layer = check.recoveryFailureLayer ?? "managed agent gateway recovery"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return check.recoveryFailureDetail | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? `${layer}: ${check.recoveryFailureDetail}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : `${layer}: the agent gateway did not recover`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function preservedSandboxRecoveryError(sandboxName: string, detail: unknown): Error { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { sanitizeSandboxStartupRecoveryDetail } = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require("./connect") as typeof import("./connect"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rawDetail = detail instanceof Error && detail.message ? detail.message : String(detail); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const safeDetail = sanitizeSandboxStartupRecoveryDetail(rawDetail); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Sandbox '${sandboxName}' started, but startup recovery failed: ${safeDetail}. ` + | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `The existing sandbox was preserved. Run \`${cliName()} ${sandboxName} recover\`, then retry \`${cliName()} ${sandboxName} start\`.`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Restart a stopped sandbox through the lifecycle facet bound to its durable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * provider identity, then restore startup state before verifying readiness and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -93,7 +126,14 @@ export async function startSandbox( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreStoppedSandboxStartupState(sandboxNameToRestore, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| agent: resolved.sandbox.agent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restoreStartupState(name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let recovery: SandboxStartupRecoveryResult; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| recovery = restoreStartupState(name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw preservedSandboxRecoveryError(name, error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Import the auto-restore sentinel instead of redeclaring the literal. Line 16 declares Export the constant from the Shields module and import it here so one definition governs both sides. ♻️ Proposed changeIn -const SHIELDS_STARTUP_AUTO_RESTORE_REQUIRED = "NEMOCLAW_SHIELDS_AUTO_RESTORE_REQUIRED";
+export const SHIELDS_STARTUP_AUTO_RESTORE_REQUIRED = "NEMOCLAW_SHIELDS_AUTO_RESTORE_REQUIRED";In -const SHIELDS_STARTUP_AUTO_RESTORE_REQUIRED = "NEMOCLAW_SHIELDS_AUTO_RESTORE_REQUIRED"; if (
error &&
typeof error === "object" &&
"code" in error &&
- error.code === SHIELDS_STARTUP_AUTO_RESTORE_REQUIRED
+ error.code ===
+ sandboxStartDependencies.loadShields().SHIELDS_STARTUP_AUTO_RESTORE_REQUIRED
) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const failure = startupRecoveryFailure(recovery); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (failure) throw preservedSandboxRecoveryError(name, failure); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log(" Checking gateway health and host forwards…"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await (deps.verifyGateway ?? verifyGateway)(name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that sanitized failure details are retained.
Both fixtures now contain
REDACTED_TOKEN, so the token assertion checks the redaction path. The test can still pass if the implementation dropsrecoveryFailureDetailandforwardRecoveryFailureDetailinstead of sanitizing them. Add a stable, non-secret marker from each injected detail and assert that it remains in the user-facing error.As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 Prompt for AI Agents
Source: Path instructions