From 55c04691d05426d73dd3da907df785b7b3566fe0 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 5 May 2026 00:33:56 -0700 Subject: [PATCH 1/2] fix(rebuild): preserve custom policy presets Signed-off-by: Test User --- .../customize-network-policy.md | 2 + src/lib/sandbox-rebuild-action.ts | 133 +++++-- src/lib/sandbox-state.ts | 39 +- test/rebuild-policy-presets.test.ts | 347 +++++++++++------- 4 files changed, 350 insertions(+), 171 deletions(-) diff --git a/docs/network-policy/customize-network-policy.md b/docs/network-policy/customize-network-policy.md index efd792e7af..37d5ea83cb 100644 --- a/docs/network-policy/customize-network-policy.md +++ b/docs/network-policy/customize-network-policy.md @@ -299,6 +299,8 @@ Custom presets applied with `--from-file` or `--from-dir` are recorded in the Ne $ nemoclaw my-assistant policy-remove my-internal-api --yes ``` +NemoClaw also uses the stored YAML content when you run `nemoclaw rebuild`, so custom presets survive rebuild even if the original preset file is no longer on disk. + `policy-remove` accepts both built-in and custom preset names. Run `nemoclaw policy-list` to see every preset currently applied to the sandbox. ## Related Topics diff --git a/src/lib/sandbox-rebuild-action.ts b/src/lib/sandbox-rebuild-action.ts index 7ad13c456e..0506a86c00 100644 --- a/src/lib/sandbox-rebuild-action.ts +++ b/src/lib/sandbox-rebuild-action.ts @@ -36,6 +36,108 @@ function _rebuildLog(msg: string) { console.error(` ${D}[rebuild ${new Date().toISOString()}] ${msg}${R}`); } +export interface RestorePolicyPresetsResult { + restoredPresets: string[]; + failedPresets: string[]; + restoredCustomPresets: string[]; + failedCustomPresets: string[]; +} + +interface RestorePolicyPresetsDeps { + applyPreset?: typeof policies.applyPreset; + applyPresetContent?: typeof policies.applyPresetContent; + log?: (msg: string) => void; + stdout?: { log: (...args: unknown[]) => void }; + stderr?: { error: (...args: unknown[]) => void }; +} + +export function restorePolicyPresetsFromManifest( + sandboxName: string, + backupManifest: sandboxState.RebuildManifest, + deps: RestorePolicyPresetsDeps = {}, +): RestorePolicyPresetsResult { + const applyPreset = deps.applyPreset ?? policies.applyPreset; + const applyPresetContent = deps.applyPresetContent ?? policies.applyPresetContent; + const log = deps.log ?? (() => {}); + const stdout = deps.stdout ?? console; + const stderr = deps.stderr ?? console; + + const savedPresets = backupManifest.policyPresets || []; + const savedCustomPresets = backupManifest.customPolicyPresets || []; + const restoredPresets: string[] = []; + const failedPresets: string[] = []; + const restoredCustomPresets: string[] = []; + const failedCustomPresets: string[] = []; + + if (savedPresets.length === 0 && savedCustomPresets.length === 0) { + return { restoredPresets, failedPresets, restoredCustomPresets, failedCustomPresets }; + } + + stdout.log(""); + stdout.log(" Restoring policy presets..."); + + if (savedPresets.length > 0) { + log(`Policy presets to restore: [${savedPresets.join(",")}]`); + } + for (const presetName of savedPresets) { + try { + log(`Applying preset: ${presetName}`); + const applied = applyPreset(sandboxName, presetName); + if (applied) { + restoredPresets.push(presetName); + } else { + failedPresets.push(presetName); + } + } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); + log(`Failed to apply preset '${presetName}': ${errorMessage}`); + failedPresets.push(presetName); + } + } + + if (savedCustomPresets.length > 0) { + log(`Custom policy presets to restore: [${savedCustomPresets.map((p) => p.name).join(",")}]`); + } + for (const preset of savedCustomPresets) { + try { + log(`Applying custom preset: ${preset.name}`); + const applied = applyPresetContent(sandboxName, preset.name, preset.content, { + custom: { sourcePath: preset.sourcePath }, + }); + if (applied) { + restoredCustomPresets.push(preset.name); + } else { + failedCustomPresets.push(preset.name); + } + } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); + log(`Failed to apply custom preset '${preset.name}': ${errorMessage}`); + failedCustomPresets.push(preset.name); + } + } + + if (restoredPresets.length > 0) { + stdout.log(` ${G}\u2713${R} Policy presets restored: ${restoredPresets.join(", ")}`); + } + if (restoredCustomPresets.length > 0) { + stdout.log( + ` ${G}\u2713${R} Custom policy presets restored: ${restoredCustomPresets.join(", ")}`, + ); + } + if (failedPresets.length > 0) { + stderr.error(` ${YW}\u26a0${R} Failed to restore presets: ${failedPresets.join(", ")}`); + stderr.error(` Re-apply manually with: ${CLI_NAME} ${sandboxName} policy-add`); + } + if (failedCustomPresets.length > 0) { + stderr.error( + ` ${YW}\u26a0${R} Failed to restore custom presets: ${failedCustomPresets.join(", ")}`, + ); + stderr.error(` Re-apply manually with: ${CLI_NAME} ${sandboxName} policy-add --from-file`); + } + + return { restoredPresets, failedPresets, restoredCustomPresets, failedCustomPresets }; +} + function getRebuildCredentialEnvFromRegistry(provider: string | null | undefined): string | null { if (!provider || LOCAL_INFERENCE_PROVIDERS.includes(provider)) { return null; @@ -478,36 +580,7 @@ export async function rebuildSandbox( // Policy presets live in the gateway policy engine, not the sandbox filesystem. // They are lost when the sandbox is destroyed and recreated. Re-apply any // presets that were captured in the backup manifest. - const savedPresets = backupManifest.policyPresets || []; - if (savedPresets.length > 0) { - console.log(""); - console.log(" Restoring policy presets..."); - log(`Policy presets to restore: [${savedPresets.join(",")}]`); - const restoredPresets: string[] = []; - const failedPresets: string[] = []; - for (const presetName of savedPresets) { - try { - log(`Applying preset: ${presetName}`); - const applied = policies.applyPreset(sandboxName, presetName); - if (applied) { - restoredPresets.push(presetName); - } else { - failedPresets.push(presetName); - } - } catch (err) { - const errorMessage = err instanceof Error ? err.message : String(err); - log(`Failed to apply preset '${presetName}': ${errorMessage}`); - failedPresets.push(presetName); - } - } - if (restoredPresets.length > 0) { - console.log(` ${G}\u2713${R} Policy presets restored: ${restoredPresets.join(", ")}`); - } - if (failedPresets.length > 0) { - console.error(` ${YW}\u26a0${R} Failed to restore presets: ${failedPresets.join(", ")}`); - console.error(` Re-apply manually with: ${CLI_NAME} ${sandboxName} policy-add`); - } - } + restorePolicyPresetsFromManifest(sandboxName, backupManifest, { log }); // Step 6: Post-restore agent-specific migration const rebuiltAgent = agentRuntime.getSessionAgent(sandboxName); diff --git a/src/lib/sandbox-state.ts b/src/lib/sandbox-state.ts index 6f8aa103f6..165ccbafcc 100644 --- a/src/lib/sandbox-state.ts +++ b/src/lib/sandbox-state.ts @@ -28,6 +28,7 @@ import path from "node:path"; import * as registry from "./registry.js"; import { loadAgent } from "./agent-defs.js"; import type { AgentStateFile } from "./agent-defs.js"; +import type { CustomPolicyEntry, SandboxEntry } from "./registry.js"; import { resolveOpenshell } from "./resolve-openshell.js"; import { captureOpenshellCommand } from "./openshell.js"; import { sanitizeConfigFile, isSensitiveFile } from "./credential-filter.js"; @@ -62,11 +63,19 @@ export interface RebuildManifest { backupPath: string; blueprintDigest: string | null; policyPresets?: string[]; + customPolicyPresets?: RebuildCustomPolicyPreset[]; instances?: InstanceBackup[]; // Optional user-provided label for `snapshot restore `. name?: string; } +export interface RebuildCustomPolicyPreset { + name: string; + content: string; + sourcePath?: string; + appliedAt?: string; +} + // Manifest enriched with a virtual version number computed at list time. // Versions are position-based (v1 = oldest by timestamp) and NOT persisted, // so they can shift if snapshots are deleted. @@ -134,6 +143,16 @@ function isStringArray(value: unknown): value is string[] { return Array.isArray(value) && value.every((entry) => typeof entry === "string"); } +function isRebuildCustomPolicyPreset(value: unknown): value is RebuildCustomPolicyPreset { + return ( + isRecord(value) && + typeof value.name === "string" && + typeof value.content === "string" && + (value.sourcePath === undefined || typeof value.sourcePath === "string") && + (value.appliedAt === undefined || typeof value.appliedAt === "string") + ); +} + function isStateFileSpec(value: unknown): value is StateFileSpec { return ( isRecord(value) && @@ -172,6 +191,9 @@ function isRebuildManifest(value: unknown): value is RebuildManifest { value.blueprintDigest === null || typeof value.blueprintDigest === "string") && (value.policyPresets === undefined || isStringArray(value.policyPresets)) && + (value.customPolicyPresets === undefined || + (Array.isArray(value.customPolicyPresets) && + value.customPolicyPresets.every(isRebuildCustomPolicyPreset))) && (value.instances === undefined || (Array.isArray(value.instances) && value.instances.every((entry) => isInstanceBackup(entry)))) && @@ -179,6 +201,19 @@ function isRebuildManifest(value: unknown): value is RebuildManifest { ); } +export function getPolicyPresetsForManifest(sb: SandboxEntry | null): { + policyPresets: string[]; + customPolicyPresets: RebuildCustomPolicyPreset[]; +} { + return { + policyPresets: sb?.policies && sb.policies.length > 0 ? [...sb.policies] : [], + customPolicyPresets: + sb?.customPolicies && sb.customPolicies.length > 0 + ? sb.customPolicies.map((entry: CustomPolicyEntry) => ({ ...entry })) + : [], + }; +} + // ── Safe tar extraction ────────────────────────────────────────── /** @@ -879,8 +914,9 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = // Capture applied policy presets from the registry so they can be // re-applied after rebuild. Presets live in the gateway policy engine, // not on the sandbox filesystem, so they are lost on destroy/recreate. - const policyPresets: string[] = sb?.policies && sb.policies.length > 0 ? [...sb.policies] : []; + const { policyPresets, customPolicyPresets } = getPolicyPresetsForManifest(sb); _log(`policyPresets from registry: [${policyPresets.join(",")}]`); + _log(`customPolicyPresets from registry: [${customPolicyPresets.map((p) => p.name).join(",")}]`); const manifest: RebuildManifest = { version: MANIFEST_VERSION, @@ -895,6 +931,7 @@ export function backupSandboxState(sandboxName: string, options: BackupOptions = backupPath, blueprintDigest: computeBlueprintDigest(), policyPresets, + customPolicyPresets, ...(providedName !== null ? { name: providedName } : {}), }; diff --git a/test/rebuild-policy-presets.test.ts b/test/rebuild-policy-presets.test.ts index 0c8d49f056..91ac706b6e 100644 --- a/test/rebuild-policy-presets.test.ts +++ b/test/rebuild-policy-presets.test.ts @@ -1,175 +1,242 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 // -// Tests for issue #1952: rebuild should restore policy presets. -// -// Verifies that: -// 1. backupSandboxState() captures applied policy presets in the manifest -// 2. The rebuild flow re-applies presets from the manifest after restore +// Rebuild policy preset tests. Built-in presets can be restored from names, +// while custom presets need their stored YAML content replayed. import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "vitest"; const REPO_ROOT = path.join(import.meta.dirname, ".."); -type ManifestWithOptionalPresets = { - version: number; - sandboxName: string; - timestamp: string; - agentType: string; - agentVersion: string | null; - expectedVersion: string | null; - stateDirs: string[]; - dir: string; - backupPath: string; - blueprintDigest: string | null; - policyPresets?: string[] | null; -}; - -// Import compiled modules from dist/ const sandboxState = await import(path.join(REPO_ROOT, "dist", "lib", "sandbox-state.js")); +const rebuildAction = await import( + path.join(REPO_ROOT, "dist", "lib", "sandbox-rebuild-action.js") +); + +const customPresetYaml = `preset: + name: internal-api + description: "Internal service" +network_policies: + internal-api: + name: internal-api + endpoints: + - host: api.example.internal + port: 443 + protocol: rest + enforcement: enforce + rules: + - allow: { method: GET, path: "/health" } + binaries: + - { path: /usr/local/bin/node } +`; + +function makeManifest(overrides: Record = {}) { + return { + version: 1, + sandboxName: "my-assistant", + timestamp: "2026-05-05T12-00-00-000Z", + agentType: "openclaw", + agentVersion: "1.0.0", + expectedVersion: "1.0.0", + stateDirs: ["workspace"], + dir: "/sandbox/.openclaw", + backupPath: "/tmp/backup", + blueprintDigest: null, + ...overrides, + }; +} + +function collectMockOutput(...mocks: Mock[]) { + return mocks.flatMap((mock) => mock.mock.calls.flat()).join("\n"); +} + +describe("rebuild policy preset restoration", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-rebuild-test-")); + }); -describe("rebuild policy preset restoration (#1952)", () => { - describe("RebuildManifest policyPresets field", () => { - it("manifest interface accepts policyPresets array", () => { - // Verify the manifest structure supports policyPresets - const manifest: ManifestWithOptionalPresets = { - version: 1, - sandboxName: "test", - timestamp: "2026-04-17", - agentType: "openclaw", - agentVersion: "1.0.0", - expectedVersion: "1.0.0", - stateDirs: ["workspace"], - dir: "/sandbox/.openclaw", - backupPath: "/tmp/backup", - blueprintDigest: null, - policyPresets: ["telegram", "npm"], - }; - expect(manifest.policyPresets).toEqual(["telegram", "npm"]); - }); + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); - it("manifest policyPresets defaults to undefined when not set", () => { - const manifest: ManifestWithOptionalPresets = { - version: 1, - sandboxName: "test", - timestamp: "2026-04-17", - agentType: "openclaw", - agentVersion: null, - expectedVersion: null, - stateDirs: [], - dir: "/sandbox/.openclaw", - backupPath: "/tmp/backup", - blueprintDigest: null, - }; - expect(manifest.policyPresets).toBeUndefined(); - }); + it("captures built-in names and custom policy content for the rebuild manifest", () => { + const customPolicy = { + name: "internal-api", + content: customPresetYaml, + sourcePath: "/tmp/internal-api.yaml", + appliedAt: "2026-05-05T12:00:00.000Z", + }; + const sandbox = { + name: "my-assistant", + policies: ["npm", "telegram"], + customPolicies: [customPolicy], + }; + + const result = sandboxState.getPolicyPresetsForManifest(sandbox); + + expect(result.policyPresets).toEqual(["npm", "telegram"]); + expect(result.customPolicyPresets).toEqual([customPolicy]); + + result.policyPresets.push("pypi"); + const capturedCustomPolicy = result.customPolicyPresets[0]; + if (!capturedCustomPolicy) throw new Error("expected custom policy to be captured"); + capturedCustomPolicy.name = "mutated"; + + expect(sandbox.policies).toEqual(["npm", "telegram"]); + expect(sandbox.customPolicies[0]?.name).toBe("internal-api"); + }); - it("manifest policyPresets can be an empty array", () => { - const manifest: ManifestWithOptionalPresets = { - version: 1, - sandboxName: "test", - timestamp: "2026-04-17", - agentType: "openclaw", - agentVersion: null, - expectedVersion: null, - stateDirs: [], - dir: "/sandbox/.openclaw", - backupPath: "/tmp/backup", - blueprintDigest: null, - policyPresets: [], - }; - expect(manifest.policyPresets).toEqual([]); + it("serializes custom policy content in the rebuild manifest", () => { + const customPolicyPresets = [ + { + name: "internal-api", + content: customPresetYaml, + sourcePath: "/tmp/internal-api.yaml", + appliedAt: "2026-05-05T12:00:00.000Z", + }, + ]; + const manifest = makeManifest({ + policyPresets: ["npm"], + customPolicyPresets, }); + const manifestPath = path.join(tmpDir, "rebuild-manifest.json"); + + fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2)); + + const read = JSON.parse(fs.readFileSync(manifestPath, "utf-8")); + expect(read.policyPresets).toEqual(["npm"]); + expect(read.customPolicyPresets).toEqual(customPolicyPresets); }); - describe("manifest serialization round-trip", () => { - let tmpDir: string; + it("replays built-in presets by name and custom presets by stored content", () => { + const applyPreset = vi.fn(() => true); + const applyPresetContent = vi.fn(() => true); + const log = vi.fn(); + const stdout = { log: vi.fn() }; + const stderr = { error: vi.fn() }; + const manifest = makeManifest({ + policyPresets: ["npm"], + customPolicyPresets: [ + { + name: "internal-api", + content: customPresetYaml, + sourcePath: "/tmp/internal-api.yaml", + }, + ], + }); - beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-manifest-test-")); + const result = rebuildAction.restorePolicyPresetsFromManifest("my-assistant", manifest, { + applyPreset, + applyPresetContent, + log, + stdout, + stderr, }); - afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + expect(applyPreset).toHaveBeenCalledWith("my-assistant", "npm"); + expect(applyPresetContent).toHaveBeenCalledWith( + "my-assistant", + "internal-api", + customPresetYaml, + { custom: { sourcePath: "/tmp/internal-api.yaml" } }, + ); + expect(result).toEqual({ + restoredPresets: ["npm"], + failedPresets: [], + restoredCustomPresets: ["internal-api"], + failedCustomPresets: [], }); - it("policyPresets survives JSON write and read", () => { - const manifest: ManifestWithOptionalPresets = { - version: 1, - sandboxName: "test-sandbox", - timestamp: "2026-04-17T10-00-00-000Z", - agentType: "openclaw", - agentVersion: "1.0.0", - expectedVersion: "1.0.0", - stateDirs: ["workspace", "memory"], - dir: "/sandbox/.openclaw", - backupPath: tmpDir, - blueprintDigest: "abc123", - policyPresets: ["telegram", "npm", "pypi"], - }; - - const manifestPath = path.join(tmpDir, "rebuild-manifest.json"); - fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2)); - - const read = JSON.parse(fs.readFileSync(manifestPath, "utf-8")); - expect(read.policyPresets).toEqual(["telegram", "npm", "pypi"]); + const output = collectMockOutput(log, stdout.log, stderr.error); + expect(output).toContain("internal-api"); + expect(output).not.toContain("api.example.internal"); + expect(output).not.toContain(customPresetYaml); + }); + + it("replays multiple custom presets captured from directory application", () => { + const applyPresetContent = vi.fn(() => true); + const manifest = makeManifest({ + customPolicyPresets: [ + { + name: "internal-api", + content: customPresetYaml, + sourcePath: "/tmp/presets/internal-api.yaml", + }, + { + name: "staging-api", + content: customPresetYaml.replaceAll("internal-api", "staging-api"), + sourcePath: "/tmp/presets/staging-api.yaml", + }, + ], }); - it("older manifests without policyPresets read as undefined", () => { - // Simulate a manifest from before the fix - const oldManifest: ManifestWithOptionalPresets = { - version: 1, - sandboxName: "test-sandbox", - timestamp: "2026-04-01T10-00-00-000Z", - agentType: "openclaw", - agentVersion: "1.0.0", - expectedVersion: "1.0.0", - stateDirs: ["workspace"], - dir: "/sandbox/.openclaw", - backupPath: tmpDir, - blueprintDigest: null, - }; - - const manifestPath = path.join(tmpDir, "rebuild-manifest.json"); - fs.writeFileSync(manifestPath, JSON.stringify(oldManifest, null, 2)); - - const read = JSON.parse(fs.readFileSync(manifestPath, "utf-8")); - // The rebuild code uses `backup.manifest.policyPresets || []` - // so undefined falls back to empty array safely - expect(read.policyPresets || []).toEqual([]); + const result = rebuildAction.restorePolicyPresetsFromManifest("my-assistant", manifest, { + applyPreset: vi.fn(() => true), + applyPresetContent, + log: vi.fn(), + stdout: { log: vi.fn() }, + stderr: { error: vi.fn() }, }); + + expect(applyPresetContent).toHaveBeenCalledTimes(2); + expect(result.restoredCustomPresets).toEqual(["internal-api", "staging-api"]); }); - describe("rebuild policy restore logic", () => { - it("empty policyPresets array results in no restore action", () => { - // Simulates the conditional: if (savedPresets.length > 0) - const savedPresets = []; - expect(savedPresets.length).toBe(0); + it("keeps old manifests without customPolicyPresets backward compatible", () => { + const applyPreset = vi.fn(() => true); + const applyPresetContent = vi.fn(() => true); + const manifest = makeManifest({ policyPresets: ["telegram"] }); + + const result = rebuildAction.restorePolicyPresetsFromManifest("my-assistant", manifest, { + applyPreset, + applyPresetContent, + log: vi.fn(), + stdout: { log: vi.fn() }, + stderr: { error: vi.fn() }, }); - it("undefined policyPresets falls back to empty array via || []", () => { - // Simulates: const savedPresets = backup.manifest.policyPresets || []; - const manifest = { policyPresets: undefined }; - const savedPresets = manifest.policyPresets || []; - expect(savedPresets).toEqual([]); - expect(savedPresets.length).toBe(0); - }); + expect(applyPreset).toHaveBeenCalledWith("my-assistant", "telegram"); + expect(applyPresetContent).not.toHaveBeenCalled(); + expect(result.restoredPresets).toEqual(["telegram"]); + expect(result.restoredCustomPresets).toEqual([]); + }); - it("null policyPresets falls back to empty array via || []", () => { - const manifest = { policyPresets: null }; - const savedPresets = manifest.policyPresets || []; - expect(savedPresets).toEqual([]); + it("warns by custom preset name without printing custom YAML when restore fails", () => { + const log = vi.fn(); + const stdout = { log: vi.fn() }; + const stderr = { error: vi.fn() }; + const manifest = makeManifest({ + customPolicyPresets: [ + { + name: "internal-api", + content: customPresetYaml, + sourcePath: "/tmp/internal-api.yaml", + }, + ], }); - it("policyPresets with values triggers restore loop", () => { - const manifest = { policyPresets: ["telegram", "npm"] }; - const savedPresets = manifest.policyPresets || []; - expect(savedPresets.length).toBe(2); - expect(savedPresets).toContain("telegram"); - expect(savedPresets).toContain("npm"); + const result = rebuildAction.restorePolicyPresetsFromManifest("my-assistant", manifest, { + applyPreset: vi.fn(() => true), + applyPresetContent: vi.fn(() => false), + log, + stdout, + stderr, }); + + expect(result.failedCustomPresets).toEqual(["internal-api"]); + expect(stderr.error).toHaveBeenCalledWith( + expect.stringContaining("Failed to restore custom presets: internal-api"), + ); + + const output = collectMockOutput(log, stdout.log, stderr.error); + expect(output).toContain("internal-api"); + expect(output).not.toContain("api.example.internal"); + expect(output).not.toContain(customPresetYaml); }); }); From 596e407ca3b260112cfa080fb6676b77b6d4e914 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 7 May 2026 12:18:06 -0700 Subject: [PATCH 2/2] chore: refresh PR mergeability