diff --git a/src/lib/onboard/managed-bootstrap/README.md b/src/lib/onboard/managed-bootstrap/README.md index 9e4af7401b1..22d288ecd40 100644 --- a/src/lib/onboard/managed-bootstrap/README.md +++ b/src/lib/onboard/managed-bootstrap/README.md @@ -97,12 +97,15 @@ the provider and sandbox identities, plan and profile fingerprints, exact original and replacement IDs, rollback target, and phase. Exact commit and cleanup receipts are durable terminal records, so adapter recreation does not depend on process-local transaction sets or tombstone maps. -Rollback retains an `owner-cleanup-required` phase only after image-owned shared -state is restored and the exact replacement is absent. That phase keeps the -restored original quiescent and preserves the journal without a terminal -receipt until the owning sandbox service removes the exact runtime and the -provider proves its absence. Unknown runtime presence is a retryable durable -cleanup failure, never evidence of absence. +Rollback retains an `owner-cleanup-required` phase after image-owned shared +state is restored and the exact replacement is absent. If +`DockerManagedStartupSharedStateRestoreError` reports a restoration failure, +rollback can retain the same phase only after it removes the exact replacement. +This path restores the exact original to its canonical name and keeps it +stopped. In both cases, the durable journal remains without a terminal receipt. +The owning sandbox service remains responsible for `destroy`, and the provider +must prove the exact runtime is absent. Unknown runtime presence is a retryable +durable cleanup failure, never evidence of absence. The dormant Podman candidate keeps the same provider-neutral coordinator boundary but owns its engine-specific authority internally. It binds one diff --git a/src/lib/onboard/managed-bootstrap/docker-shared-state.test.ts b/src/lib/onboard/managed-bootstrap/docker-shared-state.test.ts index 691de0c5f21..e9c4e01aad1 100644 --- a/src/lib/onboard/managed-bootstrap/docker-shared-state.test.ts +++ b/src/lib/onboard/managed-bootstrap/docker-shared-state.test.ts @@ -147,6 +147,39 @@ describe("Docker managed-bootstrap shared-state helper environment", () => { helpers.forEach(expectCleanRunNodeHelper); }); + it("grants only the capabilities needed to restore exact Hermes root metadata (#9486)", () => { + const fake = fixture({ sharedState: "pending" }); + finalizeDockerManagedStartupSharedState( + { + transaction: sharedStateTransaction(), + retainContainerAfterRollback: true, + supervisorReady: false, + }, + fake.deps, + ); + + const rollbackHelper = nodeHelperCalls(fake.deps).find((args) => + args.includes("--rollback-shared-state-transaction"), + ); + expect(rollbackHelper).toBeDefined(); + const capabilities = rollbackHelper!.flatMap((value, index, args) => + value === "--cap-add" ? [args[index + 1]] : [], + ); + expect(capabilities).toEqual(["CHOWN", "DAC_OVERRIDE", "FOWNER", "FSETID"]); + expect(rollbackHelper).toEqual( + expect.arrayContaining([ + "--cap-drop", + "ALL", + "--network", + "none", + "--read-only", + "--volumes-from", + NEW_ID, + ]), + ); + expect(rollbackHelper).not.toContain("--privileged"); + }); + it("clears arbitrary container environment before the durable receipt-clear helper", () => { const fake = fixture({ sharedState: "committed" }); clearDockerManagedStartupSharedStateCommitReceipt(sharedStateTransaction(), fake.deps); diff --git a/src/lib/onboard/managed-bootstrap/docker-shared-state.ts b/src/lib/onboard/managed-bootstrap/docker-shared-state.ts index 77e145d82c0..58438aa1601 100644 --- a/src/lib/onboard/managed-bootstrap/docker-shared-state.ts +++ b/src/lib/onboard/managed-bootstrap/docker-shared-state.ts @@ -89,6 +89,13 @@ export class DockerManagedStartupSharedStateCommitIndeterminateError extends Err } } +export class DockerManagedStartupSharedStateRestoreError extends Error { + constructor(detail: string) { + super(detail); + this.name = "DockerManagedStartupSharedStateRestoreError"; + } +} + export function probeDockerManagedStartupSharedState( input: { readonly transaction: DockerManagedBootstrapSharedStateTransaction; @@ -511,6 +518,10 @@ function rollbackManagedStartupSharedState( "DAC_OVERRIDE", "--cap-add", "FOWNER", + // Hermes keeps its shared state root setgid. FSETID lets this isolated + // helper restore that bit after CHOWN changes the directory group. + "--cap-add", + "FSETID", ...NEUTRALIZED_PRE_ENTRYPOINT_ENV, "--volumes-from", transaction.containerId, @@ -526,7 +537,7 @@ function rollbackManagedStartupSharedState( DOCKER_MUTATION_OPTIONS, ); if (!hasZeroDockerExitStatus(helper)) { - throw new Error( + throw new DockerManagedStartupSharedStateRestoreError( `Immutable managed-startup helper could not restore and verify shared state: ${commandDetail(helper)}. ` + `Protected receipt retained at ${receiptPath}`, ); diff --git a/src/lib/onboard/managed-bootstrap/docker-test-fixture.ts b/src/lib/onboard/managed-bootstrap/docker-test-fixture.ts index 56fc0b3dce9..06adfb8e598 100644 --- a/src/lib/onboard/managed-bootstrap/docker-test-fixture.ts +++ b/src/lib/onboard/managed-bootstrap/docker-test-fixture.ts @@ -85,6 +85,7 @@ export type DockerFixtureOptions = { readonly replacementEnvironment?: (environment: readonly string[]) => readonly string[]; readonly sharedState?: "committed" | "none" | "pending"; readonly sharedStateCommitResult?: FixtureCommandResult; + readonly sharedStateRollbackResult?: FixtureCommandResult; readonly sharedReceiptClearFailures?: readonly Error[]; }; @@ -456,10 +457,12 @@ export function fixture(options: DockerFixtureOptions = {}) { switch (true) { case args.includes("--shared-state-transaction-status"): return ok(`${sharedState}\n`); - case args.includes("--rollback-shared-state-transaction"): - sharedState = "none"; + case args.includes("--rollback-shared-state-transaction"): { + const result = options.sharedStateRollbackResult ?? ok(); + if (result.status === 0) sharedState = "none"; events.push("shared:rollback"); - return ok(); + return result; + } } break; case "exec": diff --git a/src/lib/onboard/managed-bootstrap/docker.test.ts b/src/lib/onboard/managed-bootstrap/docker.test.ts index ebb3d149c0c..27f28465ed1 100644 --- a/src/lib/onboard/managed-bootstrap/docker.test.ts +++ b/src/lib/onboard/managed-bootstrap/docker.test.ts @@ -682,6 +682,222 @@ describe("Docker managed bootstrap adapter", () => { expect(fake.events).not.toContain("shared:rollback"); }); + it("removes only the exact replacement after Hermes metadata restoration fails (#9486)", async () => { + const metadataFailure = + "Managed startup shared-state transaction failed: managed directory metadata was not restored exactly: /sandbox/.hermes"; + const fake = fixture({ + sharedState: "pending", + sharedStateCommitResult: { status: 1, stderr: "injected commit failure" }, + sharedStateRollbackResult: { status: 1, stderr: metadataFailure }, + }); + const adapter = createDockerManagedBootstrapAdapter(fake.deps); + const { handle, request, snapshot } = authority("hermes"); + const prepared = await adapter.prepareBootstrapReplacement({ + handle, + snapshot, + request, + replacementOptions: { values: {} }, + }); + const durable = durablePreparation(handle, snapshot, prepared); + const replacement = await adapter.activateBootstrapReplacement({ + handle, + snapshot, + prepared, + durablePreparation: durable, + }); + const commitReceipt = await adapter.awaitBootstrap({ + handle, + snapshot, + replacement, + timeoutSecs: 1, + }); + + const failure = (await adapter + .finalizeBootstrap({ + outcome: "commit", + handle, + snapshot, + prepared, + durablePreparation: durable, + replacement, + completion: commitReceipt, + }) + .catch((error: unknown) => error)) as Error & { + managedBootstrapRollbackError?: unknown; + }; + + expect(failure).toBeInstanceOf(Error); + expect(failure.message).toContain(metadataFailure); + expect(failure.managedBootstrapRollbackError).toBeInstanceOf( + ManagedBootstrapOwnerCleanupRequiredError, + ); + expect(failure.managedBootstrapRollbackError).toMatchObject({ runtimeId: OLD_ID }); + expect(vi.mocked(fake.deps.dockerRm!)).toHaveBeenCalledTimes(1); + expect(vi.mocked(fake.deps.dockerRm!)).toHaveBeenCalledWith(NEW_ID, expect.any(Object)); + expect(fake.replacement).toBeNull(); + expect(fake.original).toMatchObject({ + Id: OLD_ID, + Name: "/openshell-alpha", + State: { Running: false }, + }); + expect(fake.events).not.toContain(`rm:${OLD_ID}`); + expect(fake.events).not.toContain(`start:${OLD_ID}`); + expectEventBefore(fake.events, "journal:rollback-authorized", `rm:${NEW_ID}`); + expectEventBefore(fake.events, `rm:${NEW_ID}`, `rename:${OLD_ID}:openshell-alpha`); + expectEventBefore( + fake.events, + `rename:${OLD_ID}:openshell-alpha`, + "journal:owner-cleanup-required", + ); + expect(fake.journal).toMatchObject({ + phase: "owner-cleanup-required", + originalRuntimeId: OLD_ID, + replacementRuntimeId: NEW_ID, + }); + expect(fake.finalization).toBeNull(); + }); + + it("reports a retryable restart recovery failure after exact Hermes restoration-failure cleanup (#9486)", async () => { + const metadataFailure = + "Managed startup shared-state transaction failed: managed directory metadata was not restored exactly: /sandbox/.hermes"; + const fake = fixture({ + agent: "hermes", + sharedState: "pending", + sharedStateRollbackResult: { status: 1, stderr: metadataFailure }, + }); + const first = createDockerManagedBootstrapAdapter(fake.deps); + const { handle, request, snapshot } = authority("hermes"); + const prepared = await first.prepareBootstrapReplacement({ + handle, + snapshot, + request, + replacementOptions: { values: {} }, + }); + const durable = durablePreparation(handle, snapshot, prepared); + await first.activateBootstrapReplacement({ + handle, + snapshot, + prepared, + durablePreparation: durable, + }); + + const recovered = await createDockerManagedBootstrapAdapter( + fake.deps, + ).recoverUnfinishedTransactions(); + + expect(recovered.receipts).toEqual([]); + expect(recovered.failures).toEqual([ + expect.objectContaining({ + bootstrapIdentity: IDENTITY, + sourcePhase: "owner-cleanup-required", + code: "provider-recovery-failed", + retryable: true, + detail: expect.stringContaining(metadataFailure), + }), + ]); + expect(vi.mocked(fake.deps.dockerRm!)).toHaveBeenCalledTimes(1); + expect(vi.mocked(fake.deps.dockerRm!)).toHaveBeenCalledWith(NEW_ID, expect.any(Object)); + expect(fake.replacement).toBeNull(); + expect(fake.original).toMatchObject({ + Id: OLD_ID, + Name: "/openshell-alpha", + State: { Running: false }, + }); + expect(fake.events).not.toContain(`rm:${OLD_ID}`); + expect(fake.events).not.toContain(`start:${OLD_ID}`); + expectEventBefore(fake.events, "journal:rollback-authorized", "shared:rollback"); + expectEventBefore(fake.events, "shared:rollback", `rm:${NEW_ID}`); + expectEventBefore(fake.events, `rm:${NEW_ID}`, `rename:${OLD_ID}:openshell-alpha`); + expectEventBefore( + fake.events, + `rename:${OLD_ID}:openshell-alpha`, + "journal:owner-cleanup-required", + ); + expect(fake.journal).toMatchObject({ + phase: "owner-cleanup-required", + originalRuntimeId: OLD_ID, + replacementRuntimeId: NEW_ID, + }); + expect(fake.finalization).toBeNull(); + }); + + it.each([ + { + driftedRuntime: "original", + drift: (fake: ReturnType) => { + fake.original!.Config!.Env = [...(fake.original!.Config!.Env ?? []), "DRIFTED=1"]; + }, + }, + { + driftedRuntime: "replacement", + drift: (fake: ReturnType) => { + fake.replacement!.Name = "/unowned-replacement"; + }, + }, + ])( + "retains both containers when the exact $driftedRuntime identity changes before restoration-failure cleanup (#9486)", + async ({ drift }) => { + const fake = fixture({ + sharedState: "pending", + sharedStateCommitResult: { status: 1, stderr: "injected commit failure" }, + sharedStateRollbackResult: { + status: 1, + stderr: "managed directory metadata was not restored exactly: /sandbox/.hermes", + }, + }); + const adapter = createDockerManagedBootstrapAdapter(fake.deps); + const { handle, request, snapshot } = authority("hermes"); + const prepared = await adapter.prepareBootstrapReplacement({ + handle, + snapshot, + request, + replacementOptions: { values: {} }, + }); + const durable = durablePreparation(handle, snapshot, prepared); + const replacement = await adapter.activateBootstrapReplacement({ + handle, + snapshot, + prepared, + durablePreparation: durable, + }); + const commitReceipt = await adapter.awaitBootstrap({ + handle, + snapshot, + replacement, + timeoutSecs: 1, + }); + drift(fake); + + const failure = (await adapter + .finalizeBootstrap({ + outcome: "commit", + handle, + snapshot, + prepared, + durablePreparation: durable, + replacement, + completion: commitReceipt, + }) + .catch((error: unknown) => error)) as Error & { + managedBootstrapRollbackError?: Error; + }; + + expect(failure.message).toContain( + "managed directory metadata was not restored exactly: /sandbox/.hermes", + ); + expect(failure.managedBootstrapRollbackError?.message).toMatch( + /exact original launch spec changed|replacement container has an unexpected transaction name/u, + ); + expect(fake.journal?.phase).toBe("rollback-authorized"); + expect(fake.original).not.toBeNull(); + expect(fake.replacement).not.toBeNull(); + expect(fake.events).not.toContain(`rm:${OLD_ID}`); + expect(fake.events).not.toContain(`rm:${NEW_ID}`); + expect(fake.events).not.toContain(`rename:${OLD_ID}:openshell-alpha`); + expect(fake.events).not.toContain(`start:${OLD_ID}`); + }, + ); + it("publishes durable rollback authority before deleting the replacement after restart", async () => { const fake = fixture(); const secret = "post-start-provider-secret"; @@ -690,26 +906,24 @@ describe("Docker managed bootstrap adapter", () => { expect(options).toEqual({ tail: 120, timeout: 2_000 }); return `startup failed with NVIDIA_API_KEY=${secret}`; }); - const dockerStarts: Record< - string, - () => { status: number; stdout?: string; stderr: string } - > = { - [NEW_ID]: () => { - assert(fake.replacement?.State); - Object.assign(fake.replacement.State, { - Status: "exited", - Running: false, - ExitCode: 31, - FinishedAt: "2026-08-18T12:30:00.000Z", - }); - return { status: 1, stderr: "injected start failure" }; - }, - [OLD_ID]: () => { - assert(fake.original?.State); - Object.assign(fake.original.State, { Running: true }); - return { status: 0, stdout: "", stderr: "" }; - }, - }; + const dockerStarts: Record { status: number; stdout?: string; stderr: string }> = + { + [NEW_ID]: () => { + assert(fake.replacement?.State); + Object.assign(fake.replacement.State, { + Status: "exited", + Running: false, + ExitCode: 31, + FinishedAt: "2026-08-18T12:30:00.000Z", + }); + return { status: 1, stderr: "injected start failure" }; + }, + [OLD_ID]: () => { + assert(fake.original?.State); + Object.assign(fake.original.State, { Running: true }); + return { status: 0, stdout: "", stderr: "" }; + }, + }; fake.deps.dockerStart = vi.fn((id) => dockerStarts[id]!()); const first = createDockerManagedBootstrapAdapter(fake.deps); const { handle, request: rootRequest, snapshot } = authority(); @@ -901,53 +1115,54 @@ describe("Docker managed bootstrap adapter", () => { expect(fake.events).not.toContain("journal:staged"); }); - it.each( - SUPPORTED_AGENTS, - )("prepares, activates, and exactly rolls back the %s agent without a central switch", async (agent) => { - const fake = fixture({ agent, sharedState: "pending" }); - const adapter = createDockerManagedBootstrapAdapter(fake.deps); - const { handle, request: rootRequest, snapshot } = authority(agent); - const prepared = await adapter.prepareBootstrapReplacement({ - handle, - snapshot, - request: rootRequest, - replacementOptions: { values: {} }, - }); - const durable = durablePreparation(handle, snapshot, prepared); - const replacement = await adapter.activateBootstrapReplacement({ - handle, - snapshot, - prepared, - durablePreparation: durable, - }); - await expect( - adapter.finalizeBootstrap({ - outcome: "rollback", + it.each(SUPPORTED_AGENTS)( + "prepares, activates, and exactly rolls back the %s agent without a central switch", + async (agent) => { + const fake = fixture({ agent, sharedState: "pending" }); + const adapter = createDockerManagedBootstrapAdapter(fake.deps); + const { handle, request: rootRequest, snapshot } = authority(agent); + const prepared = await adapter.prepareBootstrapReplacement({ + handle, + snapshot, + request: rootRequest, + replacementOptions: { values: {} }, + }); + const durable = durablePreparation(handle, snapshot, prepared); + const replacement = await adapter.activateBootstrapReplacement({ handle, snapshot, prepared, durablePreparation: durable, - replacement, - completion: null, - }), - ).rejects.toBeInstanceOf(ManagedBootstrapOwnerCleanupRequiredError); - expect(fake.journal?.phase).toBe("owner-cleanup-required"); - expect(fake.finalization).toBeNull(); - expect(fake.replacement).toBeNull(); - expect(fake.original?.State?.Running).toBe(false); - expectEventBefore(fake.events, "shared:rollback", `rm:${NEW_ID}`); - expectEventBefore(fake.events, `rm:${NEW_ID}`, "journal:owner-cleanup-required"); - expect( - vi.mocked(fake.deps.dockerRun!).mock.calls.some(([args]) => { - const agentIndex = args.indexOf("--agent"); - return ( - args.includes("--shared-state-transaction-status") && - agentIndex >= 0 && - args[agentIndex + 1] === agent - ); - }), - ).toBe(true); - }); + }); + await expect( + adapter.finalizeBootstrap({ + outcome: "rollback", + handle, + snapshot, + prepared, + durablePreparation: durable, + replacement, + completion: null, + }), + ).rejects.toBeInstanceOf(ManagedBootstrapOwnerCleanupRequiredError); + expect(fake.journal?.phase).toBe("owner-cleanup-required"); + expect(fake.finalization).toBeNull(); + expect(fake.replacement).toBeNull(); + expect(fake.original?.State?.Running).toBe(false); + expectEventBefore(fake.events, "shared:rollback", `rm:${NEW_ID}`); + expectEventBefore(fake.events, `rm:${NEW_ID}`, "journal:owner-cleanup-required"); + expect( + vi.mocked(fake.deps.dockerRun!).mock.calls.some(([args]) => { + const agentIndex = args.indexOf("--agent"); + return ( + args.includes("--shared-state-transaction-status") && + agentIndex >= 0 && + args[agentIndex + 1] === agent + ); + }), + ).toBe(true); + }, + ); it("exactly rolls back an identity-bound replacement in Docker's restart loop", async () => { const fake = fixture({ sharedState: "pending" }); @@ -1061,35 +1276,33 @@ describe("Docker managed bootstrap adapter", () => { expect(fake.events).not.toContain(`stop:${OLD_ID}`); }); - it.each([ - "NODE_OPTIONS", - "NODE_PATH", - "LD_PRELOAD", - "BASH_ENV", - ])("rejects hostile %s from the launch snapshot before replacement creation", async (key) => { - const fake = fixture(); - const adapter = createDockerManagedBootstrapAdapter(fake.deps); - const { handle, request, snapshot } = authority(); - const parsed = parseDockerManagedBootstrapLaunchSpec(snapshot.specCanonicalJson); - const hostileInspect = structuredClone(parsed.inspect); - hostileInspect.Config!.Env = [...(hostileInspect.Config!.Env ?? []), `${key}=/tmp/hostile`]; - const hostileSpec = normalizeDockerManagedBootstrapLaunchSpec(hostileInspect); + it.each(["NODE_OPTIONS", "NODE_PATH", "LD_PRELOAD", "BASH_ENV"])( + "rejects hostile %s from the launch snapshot before replacement creation", + async (key) => { + const fake = fixture(); + const adapter = createDockerManagedBootstrapAdapter(fake.deps); + const { handle, request, snapshot } = authority(); + const parsed = parseDockerManagedBootstrapLaunchSpec(snapshot.specCanonicalJson); + const hostileInspect = structuredClone(parsed.inspect); + hostileInspect.Config!.Env = [...(hostileInspect.Config!.Env ?? []), `${key}=/tmp/hostile`]; + const hostileSpec = normalizeDockerManagedBootstrapLaunchSpec(hostileInspect); - await expect( - adapter.prepareBootstrapReplacement({ - handle, - snapshot: { - ...snapshot, - specHash: hostileSpec.hash, - specCanonicalJson: hostileSpec.canonicalJson, - }, - request, - replacementOptions: { values: {} }, - }), - ).rejects.toThrow(`Managed bootstrap refuses root-process injection environment '${key}'.`); - expect(fake.events).not.toContain("create:replacement"); - expect(fake.replacement).toBeNull(); - }); + await expect( + adapter.prepareBootstrapReplacement({ + handle, + snapshot: { + ...snapshot, + specHash: hostileSpec.hash, + specCanonicalJson: hostileSpec.canonicalJson, + }, + request, + replacementOptions: { values: {} }, + }), + ).rejects.toThrow(`Managed bootstrap refuses root-process injection environment '${key}'.`); + expect(fake.events).not.toContain("create:replacement"); + expect(fake.replacement).toBeNull(); + }, + ); it("quiesces and retains an exact incomplete create when its mutable name is reused", async () => { const fake = fixture({ ownerId: "sandbox-alpha-recreated" }); diff --git a/src/lib/onboard/managed-bootstrap/docker.ts b/src/lib/onboard/managed-bootstrap/docker.ts index 2f5c9c72c5f..f790a507acf 100644 --- a/src/lib/onboard/managed-bootstrap/docker.ts +++ b/src/lib/onboard/managed-bootstrap/docker.ts @@ -98,6 +98,7 @@ import { import { clearDockerManagedStartupSharedStateCommitReceipt, DockerManagedStartupSharedStateCommitIndeterminateError, + DockerManagedStartupSharedStateRestoreError, finalizeDockerManagedStartupSharedState, probeDockerManagedStartupSharedState, } from "./docker-shared-state"; @@ -1276,6 +1277,46 @@ function removeExactReplacement( } } +function restoreExactOriginalName( + transaction: DockerBootstrapTransaction, + original: DockerContainerInspect, + deps: ResolvedDeps, +): DockerContainerInspect { + assertTransactionOriginal(transaction, original); + const currentName = dockerContainerName(original); + if (currentName !== transaction.originalName) { + if (currentName !== transaction.backupName) { + throw new Error("Managed bootstrap original container has an unexpected rollback name."); + } + const renamed = deps.dockerRename(transaction.originalRuntimeId, transaction.originalName, { + ignoreError: true, + suppressOutput: true, + timeout: DOCKER_GPU_PATCH_TIMEOUT_MS, + }); + if (!hasZeroDockerExitStatus(renamed)) { + const afterRename = inspectTransactionRuntime( + transaction, + transaction.originalRuntimeId, + deps, + ); + if (!afterRename || dockerContainerName(afterRename) !== transaction.originalName) { + throw new Error( + `Managed bootstrap could not restore the original container name: ${ + commandDetail(renamed) || "Docker rename failed" + }`, + ); + } + assertTransactionOriginal(transaction, afterRename); + } + } + const restored = inspectExact(transaction.originalRuntimeId, deps); + assertTransactionOriginal(transaction, restored); + if (dockerContainerName(restored) !== transaction.originalName) { + throw new Error("Managed bootstrap rollback did not restore the authoritative container name."); + } + return restored; +} + function restoreOriginal(transaction: DockerBootstrapTransaction, deps: ResolvedDeps): void { const options = { ignoreError: true, @@ -1303,35 +1344,11 @@ function restoreOriginal(transaction: DockerBootstrapTransaction, deps: Resolved if (replacement) { removeExactReplacement(transaction, replacement, deps); } - const original = inspectExact(transaction.originalRuntimeId, deps); - assertTransactionOriginal(transaction, original); - const currentName = dockerContainerName(original); - if (currentName !== transaction.originalName) { - if (currentName !== transaction.backupName) { - throw new Error("Managed bootstrap original container has an unexpected rollback name."); - } - const renamed = deps.dockerRename( - transaction.originalRuntimeId, - transaction.originalName, - options, - ); - if (!hasZeroDockerExitStatus(renamed)) { - const afterRename = inspectTransactionRuntime( - transaction, - transaction.originalRuntimeId, - deps, - ); - if (!afterRename || dockerContainerName(afterRename) !== transaction.originalName) { - throw new Error( - `Managed bootstrap could not restore the original container name: ${ - commandDetail(renamed) || "Docker rename failed" - }`, - ); - } - assertTransactionOriginal(transaction, afterRename); - } - } - const restoredBeforeStart = inspectExact(transaction.originalRuntimeId, deps); + const restoredBeforeStart = restoreExactOriginalName( + transaction, + inspectExact(transaction.originalRuntimeId, deps), + deps, + ); if (restoredBeforeStart.State?.Running !== true) { const started = deps.dockerStart(transaction.originalRuntimeId, options); if (!hasZeroDockerExitStatus(started)) { @@ -2076,6 +2093,121 @@ export function createDockerManagedBootstrapAdapter( removeDockerBootstrapJournalDurably(journal, deps); return finalization; }; + const failAfterSharedStateRestoreError = ( + journal: DockerBootstrapTransaction, + failure: DockerManagedStartupSharedStateRestoreError, + ): never => { + try { + let activeJournal = journal; + if (journal.phase === "cutover") { + const current = deps.journalStore.load(journal.bootstrapIdentity); + if (!current || !sameDockerBootstrapJournal(current, journal)) { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: journal.bootstrapIdentity, + runtimeId: journal.replacementRuntimeId, + detail: "durable authority changed before failed shared-state restoration cleanup", + }); + } + activeJournal = transitionDockerBootstrapJournalDurably( + journal, + "rollback-authorized", + deps, + ); + } + if (activeJournal.phase !== "rollback-authorized") { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: activeJournal.bootstrapIdentity, + runtimeId: activeJournal.replacementRuntimeId, + detail: `failed shared-state restoration cleanup is forbidden from durable phase ${activeJournal.phase}`, + }); + } + + const current = deps.journalStore.load(activeJournal.bootstrapIdentity); + if (!current || !sameDockerBootstrapJournal(current, activeJournal)) { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: activeJournal.bootstrapIdentity, + runtimeId: activeJournal.replacementRuntimeId, + detail: "rollback authority changed before failed shared-state restoration cleanup", + }); + } + const original = inspectTransactionRuntime( + activeJournal, + activeJournal.originalRuntimeId, + deps, + ); + const replacement = inspectTransactionRuntime( + activeJournal, + activeJournal.replacementRuntimeId, + deps, + ); + if (!original || !replacement) { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: activeJournal.bootstrapIdentity, + runtimeId: original + ? activeJournal.replacementRuntimeId + : activeJournal.originalRuntimeId, + detail: + "failed shared-state restoration cleanup requires both exact transaction runtimes", + }); + } + assertTransactionOriginal(activeJournal, original); + assertTransactionReplacement(activeJournal, replacement); + if ( + dockerContainerName(original) !== activeJournal.backupName || + !isExplicitlyStopped(original) || + dockerContainerName(replacement) !== activeJournal.originalName || + !isExplicitlyStopped(replacement) + ) { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: activeJournal.bootstrapIdentity, + runtimeId: activeJournal.replacementRuntimeId, + detail: + "failed shared-state restoration cleanup runtimes do not match exact cutover authority", + }); + } + + removeExactReplacement(activeJournal, replacement, deps); + const restored = restoreExactOriginalName(activeJournal, original, deps); + if ( + !isExplicitlyStopped(restored) || + normalizeDockerManagedBootstrapLaunchSpec(restored).hash !== activeJournal.originalSpecHash + ) { + throw new ManagedBootstrapCommitStateIndeterminateError({ + bootstrapIdentity: activeJournal.bootstrapIdentity, + runtimeId: activeJournal.originalRuntimeId, + detail: + "failed shared-state restoration cleanup did not retain the exact original container in the stopped state", + }); + } + requireExactOwnerCleanup(activeJournal); + throw new Error( + "Managed bootstrap owner cleanup was not retained after restoration failure.", + ); + } catch (cleanupError) { + attachManagedBootstrapRollbackError(failure, cleanupError); + } + throw failure; + }; + const finalizePendingSharedStateRollback = ( + journal: DockerBootstrapTransaction, + transaction: ReturnType, + ): void => { + try { + finalizeDockerManagedStartupSharedState( + { + transaction, + supervisorReady: false, + retainContainerAfterRollback: true, + }, + deps, + ); + } catch (error) { + if (error instanceof DockerManagedStartupSharedStateRestoreError) { + failAfterSharedStateRestoreError(journal, error); + } + throw error; + } + }; const completedCommit = ( handle: ManagedBootstrapHeldWorkloadHandle, commitReceipt: ManagedBootstrapCompletionReceipt, @@ -2406,14 +2538,7 @@ export function createDockerManagedBootstrapAdapter( ); } if (sharedStatus === "pending") { - finalizeDockerManagedStartupSharedState( - { - transaction: sharedTransaction, - supervisorReady: false, - retainContainerAfterRollback: true, - }, - deps, - ); + finalizePendingSharedStateRollback(activeJournal, sharedTransaction); } } else if (journal.phase === "cutover") { activeJournal = transitionDockerBootstrapJournalDurably(journal, "rollback-authorized", deps); @@ -2705,14 +2830,7 @@ export function createDockerManagedBootstrapAdapter( } activeJournal = transitionDockerBootstrapJournalDurably(journal, "rollback-authorized", deps); if (!sharedStateAlreadyRolledBack && sharedStatus === "pending") { - finalizeDockerManagedStartupSharedState( - { - transaction: sharedTransaction, - supervisorReady: false, - retainContainerAfterRollback: true, - }, - deps, - ); + finalizePendingSharedStateRollback(activeJournal, sharedTransaction); } } else { if (!originalAtTargetRecoverable && !originalAtBackupRecoverable) { @@ -2763,14 +2881,7 @@ export function createDockerManagedBootstrapAdapter( }); } if (sharedStatus === "pending") { - finalizeDockerManagedStartupSharedState( - { - transaction: sharedTransaction, - supervisorReady: false, - retainContainerAfterRollback: true, - }, - deps, - ); + finalizePendingSharedStateRollback(activeJournal, sharedTransaction); } } } @@ -3038,6 +3149,9 @@ export function createDockerManagedBootstrapAdapter( detail: error.message, }); } + if (error instanceof DockerManagedStartupSharedStateRestoreError) { + failAfterSharedStateRestoreError(journal, error); + } throw error; } if (!outcome.supervisorReady) { diff --git a/src/lib/onboard/managed-startup-shared-state-transaction.test.ts b/src/lib/onboard/managed-startup-shared-state-transaction.test.ts index d25e3108f7e..7cc42889bc6 100644 --- a/src/lib/onboard/managed-startup-shared-state-transaction.test.ts +++ b/src/lib/onboard/managed-startup-shared-state-transaction.test.ts @@ -102,14 +102,48 @@ describe("managed startup shared-state transaction", () => { Object.defineProperty(stat, "dev", { configurable: true, value: - typeof stat.dev === "bigint" - ? stat.dev + BigInt(deviceOffset) - : stat.dev + deviceOffset, + typeof stat.dev === "bigint" ? stat.dev + BigInt(deviceOffset) : stat.dev + deviceOffset, }); return stat; }) as typeof fs.lstatSync); } + function simulateLinuxDirectoryMode(root: string, initialMode: number): void { + const resolvedRoot = path.resolve(root); + let exactMode = initialMode; + const originalChmodSync = fs.chmodSync.bind(fs); + vi.spyOn(fs, "chmodSync").mockImplementation((target, targetMode) => { + originalChmodSync(target, targetMode); + [path.resolve(String(target))] + .filter((resolvedTarget) => resolvedTarget === resolvedRoot) + .forEach(() => { + exactMode = Number(targetMode); + }); + }); + const originalLstatSync = fs.lstatSync.bind(fs); + vi.spyOn(fs, "lstatSync").mockImplementation((( + target: fs.PathLike, + statOptions?: { readonly bigint?: boolean }, + ) => { + const stat = + statOptions?.bigint === true + ? originalLstatSync(target, { bigint: true }) + : originalLstatSync(target); + [path.resolve(String(target))] + .filter((resolvedTarget) => resolvedTarget === resolvedRoot) + .forEach(() => { + Object.defineProperty(stat, "mode", { + configurable: true, + value: + typeof stat.mode === "bigint" + ? (stat.mode & ~0o7777n) | BigInt(exactMode) + : (stat.mode & ~0o7777) | exactMode, + }); + }); + return stat; + }) as typeof fs.lstatSync); + } + function rewriteManifest( rewrite: (manifest: Record) => Record = (manifest) => manifest, @@ -182,8 +216,11 @@ describe("managed startup shared-state transaction", () => { "langchain-deepagents-code": [".state", "skills"], pi: ["agent", path.join("agent", "models.json")], }; - expect(absentManagedPaths[agent].every((relativePath) => - Object.is(fs.existsSync(path.join(root, relativePath)), false))).toBe(true); + expect( + absentManagedPaths[agent].every((relativePath) => + Object.is(fs.existsSync(path.join(root, relativePath)), false), + ), + ).toBe(true); expect(fs.existsSync(transactionDirectory)).toBe(false); }, ); @@ -208,6 +245,24 @@ describe("managed startup shared-state transaction", () => { expect(fs.readFileSync(env, "utf8")).toBe("TOKEN=before\n"); }); + it("restores the setgid bit on the exact Hermes state root (#9486)", () => { + const root = agentRoot("hermes"); + fs.mkdirSync(root, { mode: 0o770 }); + simulateLinuxDirectoryMode(root, 0o3770); + const config = path.join(root, "config.yaml"); + fs.writeFileSync(config, "before: true\n"); + + expect( + beginManagedStartupSharedStateTransaction(managedStartupE2eProfile("hermes"), options), + ).toBe(true); + fs.chmodSync(root, 0o770); + fs.writeFileSync(config, "after: true\n"); + + expect(rollbackManagedStartupSharedStateTransaction("hermes", options)).toBe(true); + expect(mode(root)).toBe(0o3770); + expect(fs.readFileSync(config, "utf8")).toBe("before: true\n"); + }); + it("rejects a nested mount below the exact Hermes named-volume root", () => { const root = agentRoot("hermes"); const nestedOutputDirectory = path.join(root, "channels");