Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/lib/actions/sandbox/process-recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1278,11 +1278,41 @@ function checkAndRecoverSandboxProcessesWithoutHostLock(
if (relaunch) {
try {
const completion = relaunch.finalize(true);
if (completion.stateRestored === false || completion.rolledBack) {
if (!quiet) {
console.error(
completion.rolledBack
? " Sandbox recovery did not complete; the previous container was restored."
: " Sandbox recovery failed and the previous container could not be restored automatically.",
);
if (completion.rolledBack && completion.stateBackupRemoved === false) {
console.error(" Warning: the temporary sandbox state backup could not be removed.");
}
if (!completion.rolledBack) {
printHostManagedGatewayRecoveryHints(
sandboxName,
recoveryAgent,
managedRecoveryFailureLayer,
);
}
}
return {
checked: true,
wasRunning: false,
recovered: false,
forwardRecovered: false,
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (!completion.backupRemoved && !quiet) {
console.error(
" Warning: the recovered sandbox is healthy, but its previous container backup could not be removed.",
);
}
if (completion.stateBackupRemoved === false && !quiet) {
console.error(
" Warning: the recovered sandbox is healthy, but its temporary state backup could not be removed.",
);
}
} catch {
if (!quiet) {
console.error(
Expand Down
130 changes: 127 additions & 3 deletions src/lib/actions/sandbox/supervisor-relaunch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,32 @@ function baseDeps(overrides: ManagedSupervisorRelaunchDeps = {}) {
}) as never,
),
resolveDashboardPort: vi.fn(() => 18789),
resolveContainer: vi.fn(() => "old-container-id"),
resolveContainer: vi
.fn()
.mockReturnValueOnce("old-container-id")
.mockReturnValue("new-container-id"),
inspectContainer: vi.fn(() => ({
Config: { Env: ["OPENSHELL_SANDBOX_COMMAND=sleep infinity"] },
})),
confirmMissingSupervisor: vi.fn(() => true),
backupState: vi.fn(() => ({
success: true,
manifest: {
backupPath: "/tmp/rebuild-backups/alpha/recovery",
},
backedUpDirs: ["workspace"],
failedDirs: [],
backedUpFiles: [],
failedFiles: [],
})) as never,
restoreState: vi.fn(() => ({
success: true,
restoredDirs: ["workspace"],
failedDirs: [],
restoredFiles: [],
failedFiles: [],
})),
removeBackup: vi.fn(() => true),
recreate: vi.fn(() => patchResult()),
finalize: vi.fn(({ supervisorReady }) =>
supervisorReady
Expand Down Expand Up @@ -123,7 +144,14 @@ describe("relaunchManagedSupervisorSession", () => {
expect(serialized).not.toContain("CUSTOM_PROVIDER_CREDENTIAL");
expect(serialized).not.toContain("proxypass");

expect(relaunch?.finalize(true)).toEqual({ backupRemoved: true, rolledBack: false });
expect(relaunch?.finalize(true)).toEqual({
backupRemoved: true,
rolledBack: false,
stateRestored: true,
stateBackupRemoved: true,
});
expect(deps.restoreState).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery");
expect(deps.removeBackup).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery");
expect(deps.finalize).toHaveBeenCalledWith({
result: expect.objectContaining({ newContainerId: "new-container-id" }),
supervisorReady: true,
Expand All @@ -134,13 +162,109 @@ describe("relaunchManagedSupervisorSession", () => {
const deps = baseDeps();
const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps });

expect(relaunch?.finalize(false)).toEqual({ backupRemoved: false, rolledBack: true });
expect(relaunch?.finalize(false)).toEqual({
backupRemoved: false,
rolledBack: true,
stateRestored: false,
stateBackupRemoved: true,
});
expect(deps.restoreState).not.toHaveBeenCalled();
expect(deps.removeBackup).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery");
expect(deps.finalize).toHaveBeenCalledWith({
result: expect.objectContaining({ backupContainerName: expect.any(String) }),
supervisorReady: false,
});
});

it("refuses recreation when sandbox state cannot be fully backed up", () => {
const deps = baseDeps({
backupState: vi.fn(() => ({
success: false,
backedUpDirs: [],
failedDirs: ["workspace"],
backedUpFiles: [],
failedFiles: [],
})),
});

expect(relaunchManagedSupervisorSession("alpha", { quiet: true, deps })).toBeNull();
expect(deps.recreate).not.toHaveBeenCalled();
});

it("rolls back the container transaction when state restore fails", () => {
const deps = baseDeps({
restoreState: vi.fn(() => ({
success: false,
restoredDirs: [],
failedDirs: ["workspace"],
restoredFiles: [],
failedFiles: [],
})),
});
const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps });

expect(relaunch?.finalize(true)).toEqual({
backupRemoved: false,
rolledBack: true,
stateRestored: false,
stateBackupRemoved: true,
});
expect(deps.removeBackup).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery");
expect(deps.finalize).toHaveBeenCalledWith({
result: expect.objectContaining({ backupContainerName: expect.any(String) }),
supervisorReady: false,
});
});

it("rolls back before restore when the replacement container identity changes", () => {
const deps = baseDeps({
resolveContainer: vi
.fn()
.mockReturnValueOnce("old-container-id")
.mockReturnValue("different-container-id"),
});
const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps });

expect(relaunch?.finalize(true)).toEqual({
backupRemoved: false,
rolledBack: true,
stateRestored: false,
stateBackupRemoved: true,
});
expect(deps.restoreState).not.toHaveBeenCalled();
expect(deps.removeBackup).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery");
expect(deps.finalize).toHaveBeenCalledWith({
result: expect.objectContaining({ backupContainerName: expect.any(String) }),
supervisorReady: false,
});
});

it("retains the state backup when rollback fails", () => {
const deps = baseDeps({
finalize: vi.fn(() => ({ backupRemoved: false, rolledBack: false })),
});
const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps });

expect(relaunch?.finalize(false)).toEqual({
backupRemoved: false,
rolledBack: false,
stateRestored: false,
});
expect(deps.removeBackup).not.toHaveBeenCalled();
});

it("reports best-effort state-backup cleanup failure after a successful restore", () => {
const deps = baseDeps({ removeBackup: vi.fn(() => false) });
const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps });

expect(relaunch?.finalize(true)).toEqual({
backupRemoved: true,
rolledBack: false,
stateRestored: true,
stateBackupRemoved: false,
});
});

it("returns null when the pinned recreation fails", () => {
const deps = baseDeps({
recreate: vi.fn(() => {
Expand Down
96 changes: 92 additions & 4 deletions src/lib/actions/sandbox/supervisor-relaunch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type DockerContainerInspect,
parseDockerInspectJson,
} from "../../onboard/docker-gpu-patch";
import { sameContainerId } from "../../onboard/docker-gpu-patch-clone";
import {
type DockerGpuPatchFinalizeOutcome,
finalizeDockerGpuPatchBackup,
Expand All @@ -17,6 +18,7 @@ import { buildSandboxRuntimeEnvArgs } from "../../onboard/sandbox-create-launch"
import { resolveDirectSandboxContainer } from "../../sandbox/privileged-exec";
import { redact, redactFull } from "../../security/redact";
import * as registry from "../../state/registry";
import * as sandboxState from "../../state/sandbox";
import { resolveSandboxDashboardPort } from "./forward-recovery";

/**
Expand All @@ -33,7 +35,10 @@ const DOCKER_INSPECT_TIMEOUT_MS = 15000;

export type ManagedSupervisorRelaunch = {
containerId: string;
finalize(supervisorReady: boolean): DockerGpuPatchFinalizeOutcome;
finalize(supervisorReady: boolean): DockerGpuPatchFinalizeOutcome & {
stateRestored?: boolean;
stateBackupRemoved?: boolean;
};
};

export type ManagedSupervisorRelaunchDeps = {
Expand All @@ -43,6 +48,9 @@ export type ManagedSupervisorRelaunchDeps = {
resolveContainer?: typeof resolveDirectSandboxContainer;
inspectContainer?: (containerId: string) => DockerContainerInspect;
confirmMissingSupervisor?: (containerId: string) => boolean;
backupState?: typeof sandboxState.backupSandboxState;
restoreState?: typeof sandboxState.restoreSandboxState;
removeBackup?: typeof sandboxState.removeSandboxStateBackup;
recreate?: typeof recreateOpenShellDockerSandboxWithStartupCommand;
finalize?: typeof finalizeDockerGpuPatchBackup;
};
Expand Down Expand Up @@ -128,12 +136,31 @@ export function relaunchManagedSupervisorSession(
const resolveContainer = deps.resolveContainer ?? resolveDirectSandboxContainer;
const inspect = deps.inspectContainer ?? inspectContainer;
const confirmMissingSupervisor = deps.confirmMissingSupervisor;
const backupState = deps.backupState ?? sandboxState.backupSandboxState;
const restoreState = deps.restoreState ?? sandboxState.restoreSandboxState;
const removeBackup = deps.removeBackup ?? sandboxState.removeSandboxStateBackup;
const recreate = deps.recreate ?? recreateOpenShellDockerSandboxWithStartupCommand;
const finalize = deps.finalize ?? finalizeDockerGpuPatchBackup;
try {
const containerId = resolveContainer(sandboxName, driver);
if (!hasLegacyKeepaliveStartup(inspect(containerId))) return null;
if (!confirmMissingSupervisor?.(containerId)) return null;
const backup = backupState(sandboxName);
if (
!backup.success ||
!backup.manifest ||
backup.failedDirs.length > 0 ||
backup.failedFiles.length > 0
) {
if (!quiet) {
console.error(
" Trusted container recovery stopped before recreation because sandbox state could not be fully backed up.",
);
console.error(" The existing sandbox container was left unchanged.");
}
return null;
}
const backupManifest = backup.manifest;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (!quiet) {
console.log(" Recreating the sandbox container with its managed startup command...");
}
Expand All @@ -143,8 +170,20 @@ export function relaunchManagedSupervisorSession(
expectedOldContainerId: containerId,
waitForSupervisor: false,
});
let completed: { supervisorReady: boolean; outcome: DockerGpuPatchFinalizeOutcome } | null =
null;
let completed: {
supervisorReady: boolean;
outcome: DockerGpuPatchFinalizeOutcome & {
stateRestored?: boolean;
stateBackupRemoved?: boolean;
};
} | null = null;
const removeSettledStateBackup = (): boolean => {
try {
return removeBackup(sandboxName, backupManifest.backupPath);
} catch {
return false;
}
};
return {
containerId: result.newContainerId,
finalize(supervisorReady) {
Expand All @@ -156,7 +195,56 @@ export function relaunchManagedSupervisorSession(
}
return completed.outcome;
}
const outcome = finalize({ result, supervisorReady });
if (!supervisorReady) {
const finalized = finalize({ result, supervisorReady: false });
const outcome = {
...finalized,
stateRestored: false,
...(finalized.rolledBack ? { stateBackupRemoved: removeSettledStateBackup() } : {}),
};
completed = { supervisorReady, outcome };
return outcome;
}
let replacementOwned = false;
try {
replacementOwned = sameContainerId(
resolveContainer(sandboxName, driver),
result.newContainerId,
);
} catch {
replacementOwned = false;
}
if (!replacementOwned) {
const finalized = finalize({ result, supervisorReady: false });
const outcome = {
...finalized,
stateRestored: false,
...(finalized.rolledBack ? { stateBackupRemoved: removeSettledStateBackup() } : {}),
};
completed = { supervisorReady, outcome };
return outcome;
}
let stateRestored = false;
try {
stateRestored = restoreState(sandboxName, backupManifest.backupPath).success;
} catch {
stateRestored = false;
}
if (!stateRestored) {
const finalized = finalize({ result, supervisorReady: false });
const outcome = {
...finalized,
stateRestored: false,
...(finalized.rolledBack ? { stateBackupRemoved: removeSettledStateBackup() } : {}),
};
completed = { supervisorReady, outcome };
return outcome;
}
const outcome = {
...finalize({ result, supervisorReady: true }),
stateRestored: true,
stateBackupRemoved: removeSettledStateBackup(),
};
completed = { supervisorReady, outcome };
return outcome;
},
Expand Down
26 changes: 26 additions & 0 deletions src/lib/state/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1838,6 +1838,32 @@ export type RebuildRecoveryManifestValidation =
| { ok: true; manifest: RebuildManifest }
| { ok: false; reason: string };

/**
* Remove one completed rebuild backup without allowing a caller-controlled
* path to escape the sandbox's timestamped backup directory.
*/
export function removeSandboxStateBackup(sandboxName: string, backupPath: string): boolean {
const rebuildBackupsRoot = path.resolve(REBUILD_BACKUPS_DIR);
const sandboxBackupRoot = path.resolve(rebuildBackupsRoot, sandboxName);
const candidateBackupPath = path.resolve(backupPath);

if (
sandboxBackupRoot === rebuildBackupsRoot ||
!isWithinRoot(sandboxBackupRoot, rebuildBackupsRoot) ||
normalizeHostPath(path.dirname(candidateBackupPath)) !== normalizeHostPath(sandboxBackupRoot)
) {
return false;
}

try {
rejectSymlinksOnPath(candidateBackupPath);
rmSync(candidateBackupPath, { recursive: true, force: true });
return !existsSync(candidateBackupPath);
} catch {
return false;
}
}

/**
* Re-read and validate a prepared rebuild backup before a destructive recovery.
*
Expand Down
Loading
Loading