diff --git a/nemoclaw/src/blueprint/snapshot-directory.test.ts b/nemoclaw/src/blueprint/snapshot-directory.test.ts new file mode 100644 index 00000000000..3327ab27210 --- /dev/null +++ b/nemoclaw/src/blueprint/snapshot-directory.test.ts @@ -0,0 +1,66 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, join } from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { + compactUtcTimestamp, + reserveSnapshotDir, + SNAPSHOT_DIR_NAME_RE, +} from "./snapshot-directory.js"; + +// macOS resolves the default TMPDIR through a /var symlink, which the snapshot delete helper +// rejects. Other platforms can use their native temporary directory. +const temporaryRoot = process.platform === "darwin" ? "/private/tmp" : tmpdir(); +const roots: string[] = []; + +function makeSnapshotsDir(): string { + const root = mkdtempSync(join(temporaryRoot, "nemoclaw-snapshot-dir-")); + roots.push(root); + return join(root, "snapshots"); +} + +afterEach(() => { + roots.splice(0).forEach((root) => rmSync(root, { force: true, recursive: true })); +}); + +describe("blueprint snapshot directory reservation", () => { + it("names a reserved directory in the grammar the retention reader accepts", () => { + const reserved = reserveSnapshotDir(makeSnapshotsDir(), Date.parse("2026-08-18T06:43:16.500Z")); + + expect(basename(reserved)).toBe("20260818T064316Z"); + expect(basename(reserved)).toMatch(SNAPSHOT_DIR_NAME_RE); + expect(compactUtcTimestamp(Date.parse("2026-08-18T06:43:16.500Z"))).toBe(basename(reserved)); + }); + + it("gives a same-second reservation the next unused second (#9433)", () => { + const snapshotsDir = makeSnapshotsDir(); + const startedAt = Date.parse("2026-08-18T06:43:16.500Z"); + + const first = reserveSnapshotDir(snapshotsDir, startedAt); + writeFileSync(join(first, "reservation-marker"), "first"); + const second = reserveSnapshotDir(snapshotsDir, startedAt); + + expect(basename(first)).toBe("20260818T064316Z"); + expect(basename(second)).toBe("20260818T064317Z"); + expect(basename(second)).toMatch(SNAPSHOT_DIR_NAME_RE); + // The second reservation owns an empty directory, so it can neither read nor clean up the first. + expect(second).not.toBe(first); + }); + + it("advances past a planted symlink instead of following it", () => { + const snapshotsDir = makeSnapshotsDir(); + const startedAt = Date.parse("2026-08-18T06:43:16.500Z"); + reserveSnapshotDir(snapshotsDir, startedAt); + rmSync(join(snapshotsDir, "20260818T064316Z"), { recursive: true }); + symlinkSync("/etc", join(snapshotsDir, "20260818T064316Z")); + + const reserved = reserveSnapshotDir(snapshotsDir, startedAt); + + expect(basename(reserved)).toBe("20260818T064317Z"); + }); +}); diff --git a/nemoclaw/src/blueprint/snapshot-directory.ts b/nemoclaw/src/blueprint/snapshot-directory.ts new file mode 100644 index 00000000000..a55b52f593d --- /dev/null +++ b/nemoclaw/src/blueprint/snapshot-directory.ts @@ -0,0 +1,38 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { mkdirSync } from "node:fs"; +import { join } from "node:path"; + +/** The snapshot directory grammar that the retention commands accept. */ +export const SNAPSHOT_DIR_NAME_RE = /^\d{8}T\d{6}Z$/; + +/** A UTC instant in the snapshot directory grammar: 20260818T064316Z. */ +export function compactUtcTimestamp(at: number = Date.now()): string { + return new Date(at).toISOString().replace(/[-:]|\.\d+(?=Z)/g, ""); +} + +/** + * Reserve one snapshot directory for the calling operation alone. + * + * The leaf mkdir is non-recursive, so the reservation is a single atomic syscall: EEXIST means + * some other snapshot already owns that second, and the caller never writes into, or cleans up, a + * directory it did not create. The grammar above is second-resolution, so a taken second advances + * to the next second rather than taking a suffix the retention reader would reject. Each attempt + * names a later second than the last, so the loop ends at the first unused one. + * + * A non-directory entry planted at a candidate name, including a symlink, also fails with EEXIST, + * so reservation advances past it instead of following it. + */ +export function reserveSnapshotDir(snapshotsDir: string, startedAt: number = Date.now()): string { + mkdirSync(snapshotsDir, { recursive: true }); + for (let at = startedAt; ; at += 1000) { + const candidate = join(snapshotsDir, compactUtcTimestamp(at)); + try { + mkdirSync(candidate); + return candidate; + } catch (error: unknown) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; + } + } +} diff --git a/nemoclaw/src/blueprint/snapshot-management.ts b/nemoclaw/src/blueprint/snapshot-management.ts index 979b21602f3..af1b7865b5c 100644 --- a/nemoclaw/src/blueprint/snapshot-management.ts +++ b/nemoclaw/src/blueprint/snapshot-management.ts @@ -7,8 +7,7 @@ import { homedir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; import { deleteSnapshotDirectory, snapshotDeletionSupported } from "./snapshot-delete-helper.js"; - -const SNAPSHOT_DIR_NAME_RE = /^\d{8}T\d{6}Z$/; +import { SNAPSHOT_DIR_NAME_RE } from "./snapshot-directory.js"; export { snapshotDeletionSupported }; diff --git a/nemoclaw/src/blueprint/snapshot.ts b/nemoclaw/src/blueprint/snapshot.ts index 1e1e52d4ecf..47d0e82cf52 100644 --- a/nemoclaw/src/blueprint/snapshot.ts +++ b/nemoclaw/src/blueprint/snapshot.ts @@ -15,7 +15,6 @@ import { cpSync, existsSync, lstatSync, - mkdirSync, readdirSync, readlinkSync, renameSync, @@ -23,11 +22,12 @@ import { writeFileSync, } from "node:fs"; import { homedir } from "node:os"; -import { dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; import { execa } from "execa"; import * as importedSandboxName from "../shared/sandbox-name.cjs"; +import { compactUtcTimestamp, reserveSnapshotDir } from "./snapshot-directory.js"; const HOME = homedir(); const OPENCLAW_DIR = join(HOME, ".openclaw"); @@ -40,13 +40,6 @@ const sourceOrGeneratedSandboxName = importedSandboxName as typeof importedSandb }; const { assertValidName } = sourceOrGeneratedSandboxName.default ?? sourceOrGeneratedSandboxName; -function compactTimestamp(): string { - return new Date() - .toISOString() - .replace(/[-:]/g, "") - .replace(/\.\d+Z$/, "Z"); -} - /** * Reject a path if it — or any ancestor up to $HOME — is a symlink. * Prevents an attacker from planting a symlink at the target path to @@ -111,13 +104,11 @@ export function createSnapshot(): string | null { // sensitive files into the snapshot. rejectSymlinksOnPath(OPENCLAW_DIR); - const timestamp = compactTimestamp(); - const snapshotDir = join(SNAPSHOTS_DIR, timestamp); - - // SECURITY: Verify snapshot destination ancestors are not symlinks. - rejectSymlinksOnPath(snapshotDir); + // SECURITY: Verify snapshot destination ancestors are not symlinks, before reserving. + rejectSymlinksOnPath(SNAPSHOTS_DIR); - mkdirSync(snapshotDir, { recursive: true }); + const snapshotDir = reserveSnapshotDir(SNAPSHOTS_DIR); + const timestamp = basename(snapshotDir); const dest = join(snapshotDir, "openclaw"); cpSync(OPENCLAW_DIR, dest, { recursive: true }); @@ -245,7 +236,7 @@ export function cutoverHost(): boolean { return true; } - const archivePath = join(HOME, `.openclaw.pre-nemoclaw.${compactTimestamp()}`); + const archivePath = join(HOME, `.openclaw.pre-nemoclaw.${compactUtcTimestamp()}`); try { moveSync(OPENCLAW_DIR, archivePath); return true; @@ -261,7 +252,7 @@ export function rollbackFromSnapshot(snapshotDir: string): boolean { } const archivePath = existsSync(OPENCLAW_DIR) - ? join(HOME, `.openclaw.nemoclaw-archived.${compactTimestamp()}`) + ? join(HOME, `.openclaw.nemoclaw-archived.${compactUtcTimestamp()}`) : null; try { diff --git a/nemoclaw/src/commands/migration-state-security.test.ts b/nemoclaw/src/commands/migration-state-security.test.ts index 4aab12776dc..d03ec137646 100644 --- a/nemoclaw/src/commands/migration-state-security.test.ts +++ b/nemoclaw/src/commands/migration-state-security.test.ts @@ -18,6 +18,7 @@ import { import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { listSnapshots, pruneSnapshots } from "../blueprint/snapshot-management.js"; import type { PluginLogger } from "../index.js"; import * as credentialFilter from "../security/credential-filter.js"; import * as snapshotSanitizer from "../security/snapshot-sanitizer.js"; @@ -96,12 +97,63 @@ function expectSnapshotFailure( } afterEach(() => { + vi.useRealTimers(); vi.restoreAllMocks(); for (const root of roots.splice(0)) { rmSync(root, { force: true, recursive: true }); } }); +describe("migration-state snapshot directory reservation", () => { + it("takes its snapshot directory from the shared reservation (#9433)", () => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2026-08-18T06:43:16.500Z")); + const { home, configPath, logger } = makeMinimalHostSnapshot(); + const hostState = makeHostState(home, configPath); + + const first = createSnapshotBundle(hostState, logger, { persist: true }); + expectSnapshotBundle(first); + const second = createSnapshotBundle(hostState, logger, { persist: true }); + expectSnapshotBundle(second); + + // The clock has not moved, so an unreserved leaf would be the first snapshot's directory. + // Reservation grammar and same-second advance are owned by snapshot-directory.test.ts. + expect(second.snapshotDir).not.toBe(first.snapshotDir); + expect(path.basename(first.snapshotDir)).toBe(first.manifest.timestamp); + expect(path.basename(second.snapshotDir)).toBe(second.manifest.timestamp); + }); + + it("publishes persisted migration snapshots to the retention reader (#9433)", () => { + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(new Date("2026-08-18T06:43:16.500Z")); + const { home, configPath, logger } = makeMinimalHostSnapshot(); + const bundle = createSnapshotBundle(makeHostState(home, configPath), logger, { + persist: true, + }); + expectSnapshotBundle(bundle); + const snapshotsDir = path.join(home, ".nemoclaw", "snapshots"); + + expect(listSnapshots({ snapshotsDir })).toEqual([ + expect.objectContaining({ + path: bundle.snapshotDir, + timestamp: bundle.manifest.timestamp, + }), + ]); + + const result = pruneSnapshots(0, { + snapshotsDir, + deleteDirectory: (root, name) => { + expect(root).toBe(snapshotsDir); + expect(name).toBe(bundle.manifest.timestamp); + rmSync(path.join(root, name), { force: true, recursive: true }); + return true; + }, + }); + expect(result).toEqual({ deleted: [bundle.snapshotDir], failed: [], kept: [] }); + expect(listSnapshots({ snapshotsDir })).toEqual([]); + }); +}); + describe("migration-state prepared config security", () => { it("installs a mode-0600 config after scrubbing contextual secrets in memory", () => { const home = makeHome(); diff --git a/nemoclaw/src/commands/migration-state.test.ts b/nemoclaw/src/commands/migration-state.test.ts index 360b4831a95..784b7af5b38 100644 --- a/nemoclaw/src/commands/migration-state.test.ts +++ b/nemoclaw/src/commands/migration-state.test.ts @@ -519,13 +519,13 @@ describe("commands/migration-state", () => { const hostState = makeHostOpenClawState(); const bundle = createSnapshotBundle(hostState, logger, { persist: true }); - if (bundle === null) { - expect.unreachable("bundle should not be null"); - return; - } - expect(bundle.manifest.version).toBe(3); - expect(bundle.manifest.homeDir).toBe("/home/user"); + if (bundle === null) expect.unreachable("bundle should not be null"); + expect(bundle.manifest).toMatchObject({ version: 3, homeDir: "/home/user" }); expect(bundle.temporary).toBe(false); + // The retention reader accepts only this directory grammar and requires the + // manifest to name the same identity (blueprint/snapshot-management.ts). + expect(bundle.snapshotDir).toMatch(/^\/home\/user\/\.nemoclaw\/snapshots\/\d{8}T\d{6}Z$/); + expect(bundle.snapshotDir.endsWith(`/${String(bundle.manifest.timestamp)}`)).toBe(true); }); it("snapshots external config when hasExternalConfig", () => { diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index 6b03c0783f9..0415aa2bfd8 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -20,6 +20,7 @@ import path from "node:path"; import JSON5 from "json5"; import { create as createTar } from "tar"; import type { PluginLogger } from "../index.js"; +import { reserveSnapshotDir } from "../blueprint/snapshot-directory.js"; import { CREDENTIAL_SENSITIVE_BASENAMES, isSensitiveFile, @@ -75,6 +76,7 @@ export interface HostOpenClawState { export interface SnapshotManifest { version: number; + timestamp?: string; createdAt: string; homeDir: string; stateDir: string; @@ -577,6 +579,7 @@ function isSnapshotManifest(value: unknown): value is SnapshotManifest { return ( isObjectRecord(value) && typeof value.version === "number" && + (value.timestamp === undefined || typeof value.timestamp === "string") && typeof value.createdAt === "string" && typeof value.homeDir === "string" && typeof value.stateDir === "string" && @@ -762,16 +765,17 @@ export function createSnapshotBundle( return null; } - const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); - const parentDir = path.join( + const snapshotsDir = path.join( hostState.homeDir, ".nemoclaw", options.persist ? "snapshots" : "staging", - timestamp, ); + // Empty until this operation owns a directory, so failure cleanup can never remove another one. + let parentDir = ""; try { - mkdirSync(parentDir, { recursive: true }); + parentDir = reserveSnapshotDir(snapshotsDir, Date.now()); + const timestamp = path.basename(parentDir); const snapshotStateDir = path.join(parentDir, "openclaw"); copyDirectory(hostState.stateDir, snapshotStateDir, { stripCredentials: true }); sanitizeMigrationDirectory(snapshotStateDir); @@ -808,6 +812,7 @@ export function createSnapshotBundle( const manifest: SnapshotManifest = { version: SNAPSHOT_VERSION, + timestamp, createdAt: new Date().toISOString(), homeDir: hostState.homeDir, stateDir: hostState.stateDir,