diff --git a/agents/openclaw/manifest.yaml b/agents/openclaw/manifest.yaml index 300b54cda8..a64145517a 100644 --- a/agents/openclaw/manifest.yaml +++ b/agents/openclaw/manifest.yaml @@ -38,6 +38,24 @@ config: writable_dir: /sandbox/.openclaw-data config_file: openclaw.json # relative to immutable_dir format: json + # Known top-level sections in openclaw.json. Used by `nemoclaw config set` + # to reject obvious typos (e.g. `inferenc.endpoint`) without coupling + # NemoClaw to OpenClaw's evolving config schema. Agents that omit this + # field skip validation entirely (see #2400). + known_sections: + - llm + - tts + - stt + - mcp + - sandbox + - extensions + - personas + - customization + - inference + - plugins + - skills + - ui + - logging # ── State directories ────────────────────────────────────────── state_dirs: diff --git a/src/lib/agent-defs.ts b/src/lib/agent-defs.ts index 7e64a5a199..75ec1b818c 100644 --- a/src/lib/agent-defs.ts +++ b/src/lib/agent-defs.ts @@ -58,6 +58,7 @@ export interface AgentDefinition { readonly healthProbe: AgentHealthProbe; readonly forwardPort: number; readonly configPaths: AgentConfigPaths; + readonly knownConfigSections: string[] | null; readonly stateDirs: string[]; readonly versionCommand: string; readonly expectedVersion: string | null; @@ -148,6 +149,22 @@ export function loadAgent(name: string): AgentDefinition { }; }, + /** + * Optional list of known top-level config sections, declared by the + * agent in `manifest.yaml > config.known_sections`. Used by + * `nemoclaw config set` to give an early error on obvious typos + * (e.g. `inferenc.endpoint`) without coupling NemoClaw to any one + * agent's evolving config schema. When an agent does NOT declare + * this list, validation is skipped — agents stay free to add new + * top-level keys without a NemoClaw release. + */ + get knownConfigSections(): string[] | null { + const cfg = (raw.config as Record) || {}; + const sections = cfg.known_sections; + if (!Array.isArray(sections) || sections.length === 0) return null; + return sections.filter((s): s is string => typeof s === "string"); + }, + get stateDirs(): string[] { return (raw.state_dirs as string[]) || []; }, diff --git a/src/lib/sandbox-config.ts b/src/lib/sandbox-config.ts index b9681d3519..b7d9f38b99 100644 --- a/src/lib/sandbox-config.ts +++ b/src/lib/sandbox-config.ts @@ -49,6 +49,14 @@ interface AgentConfigTarget { format: string; /** Config file basename */ configFile: string; + /** + * Optional list of known top-level config sections from the agent's + * manifest. When provided, `nemoclaw config set` rejects writes whose + * top-level key isn't in this set. When null, validation is skipped + * (agents are free to evolve their config schema without a NemoClaw + * release). + */ + knownSections: string[] | null; } const DEFAULT_AGENT_CONFIG: AgentConfigTarget = { @@ -57,6 +65,7 @@ const DEFAULT_AGENT_CONFIG: AgentConfigTarget = { configDir: "/sandbox/.openclaw", format: "json", configFile: "openclaw.json", + knownSections: null, }; function resolveAgentConfig(sandboxName: string): AgentConfigTarget { @@ -75,6 +84,7 @@ function resolveAgentConfig(sandboxName: string): AgentConfigTarget { configDir: cfg.immutableDir, format: cfg.format || "json", configFile: cfg.configFile, + knownSections: agent.knownConfigSections, }; } catch { // Registry or agent-defs unavailable (e.g., during tests) — fall back @@ -308,22 +318,43 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void { process.exit(1); } - // 5. Show what will change + // 5. Validate the top-level config section against the agent's manifest. + // The agent's manifest.yaml may declare `config.known_sections`. When it + // does, we reject obvious typos here for a fast, friendly error. When it + // doesn't (e.g. for the Hermes agent, whose schema is intentionally open), + // validation is skipped — NemoClaw stays decoupled from each agent's + // evolving config schema (per #2400). + const topLevelKey = opts.key.split(".")[0]; + if (target.knownSections && target.knownSections.length > 0) { + if (!target.knownSections.includes(topLevelKey)) { + console.error(` Unknown config section: '${topLevelKey}'`); + console.error( + ` Valid top-level keys for agent '${target.agentName}': ${[...target.knownSections].sort().join(", ")}`, + ); + console.error( + ` (Allowlist is declared in agents/${target.agentName}/manifest.yaml → config.known_sections.\n` + + ` If '${topLevelKey}' is a real new section, add it there in a follow-up PR.)`, + ); + process.exit(1); + } + } + + // 6. Show what will change const oldValue = extractDotpath(config, opts.key); console.log(` Agent: ${target.agentName}`); console.log(` Key: ${opts.key}`); console.log(` Old value: ${oldValue !== undefined ? JSON.stringify(oldValue) : "(not set)"}`); console.log(` New value: ${JSON.stringify(parsedValue)}`); - // 6. Apply change + // 7. Apply change setDotpath(config, opts.key, parsedValue); - // 7. Write to temp file in the agent's native format + // 8. Write to temp file in the agent's native format const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-config-")); const tmpFile = path.join(tmpDir, target.configFile); fs.writeFileSync(tmpFile, serializeConfig(config, target.format), { mode: 0o600 }); - // 8. Write config to sandbox via kubectl exec (bypasses Landlock) + // 9. Write config to sandbox via kubectl exec (bypasses Landlock) console.log(` Writing config to sandbox (${target.configPath})...`); const content = fs.readFileSync(tmpFile, "utf-8"); execFileSync("docker", [ @@ -332,7 +363,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void { "sh", "-c", `cat > ${target.configPath}`, ], { input: content, stdio: ["pipe", "pipe", "pipe"], timeout: 15000 }); - // 9. Fix ownership via kubectl exec (bypasses Landlock) + // 10. Fix ownership via kubectl exec (bypasses Landlock) try { execFileSync("docker", [ "exec", K3S_CONTAINER, @@ -343,7 +374,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void { // Best effort — chown failure is non-fatal } - // 10. Cleanup temp + // 11. Cleanup temp try { fs.unlinkSync(tmpFile); fs.rmdirSync(tmpDir); @@ -351,7 +382,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void { // Best effort } - // 11. Audit log + // 12. Audit log appendAuditEntry({ action: "shields_down", sandbox: sandboxName, @@ -361,7 +392,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void { console.log(` ${target.agentName} config updated.`); - // 12. Restart if requested + // 13. Restart if requested if (opts.restart) { console.log(" Restarting sandbox agent process..."); const restartBinary = getOpenshellBinary(); diff --git a/test/config-set.test.ts b/test/config-set.test.ts index 25489a6f15..37ef931859 100644 --- a/test/config-set.test.ts +++ b/test/config-set.test.ts @@ -2,6 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, it, expect } from "vitest"; +import assert from "node:assert"; +import fs from "node:fs"; +import path from "node:path"; +// eslint-disable-next-line @typescript-eslint/no-require-imports +const yaml = require("js-yaml") as { load: (s: string) => unknown }; // Build must run before these tests (imports from dist/) const { extractDotpath, setDotpath, validateUrlValue, resolveAgentConfig } = require("../dist/lib/sandbox-config"); @@ -23,6 +28,61 @@ describe("resolveAgentConfig", () => { const target = resolveAgentConfig("any-sandbox"); expect(target.configPath.endsWith(target.configFile)).toBe(true); }); + + it("defaults knownSections to null when manifest is absent (e.g. fallback)", () => { + const target = resolveAgentConfig("nonexistent-sandbox"); + expect(target.knownSections).toBe(null); + }); +}); + +describe("config set top-level section validation (manifest-aware)", () => { + // Tracks the design discussion on #2071 / #2400: the validator must NOT + // hardcode an OpenClaw-specific allowlist. It must read `config.known_sections` + // from the agent's manifest, and skip validation entirely when the agent + // doesn't declare one (so agents like Hermes with their own schema aren't + // blocked). + + const sandboxConfigSource = fs.readFileSync( + path.join(import.meta.dirname, "..", "src", "lib", "sandbox-config.ts"), + "utf-8", + ); + + it("does not contain a hardcoded KNOWN_TOP_LEVEL_KEYS allowlist", () => { + assert.doesNotMatch(sandboxConfigSource, /KNOWN_TOP_LEVEL_KEYS\s*=\s*new Set/); + }); + + it("reads the allowlist from the agent's manifest via target.knownSections", () => { + assert.match(sandboxConfigSource, /target\.knownSections/); + assert.match( + sandboxConfigSource, + /if \(target\.knownSections && target\.knownSections\.length > 0\)/, + ); + }); + + it("declares known_sections in the openclaw agent manifest", () => { + const manifest = yaml.load( + fs.readFileSync( + path.join(import.meta.dirname, "..", "agents", "openclaw", "manifest.yaml"), + "utf-8", + ), + ) as { config?: { known_sections?: string[] } }; + expect(Array.isArray(manifest.config?.known_sections)).toBe(true); + expect(manifest.config!.known_sections!.length).toBeGreaterThan(0); + // Spot-check a few essentials. + for (const key of ["llm", "sandbox", "extensions"]) { + expect(manifest.config!.known_sections).toContain(key); + } + }); + + it("does NOT declare known_sections in the hermes agent manifest (intentionally permissive)", () => { + const manifest = yaml.load( + fs.readFileSync( + path.join(import.meta.dirname, "..", "agents", "hermes", "manifest.yaml"), + "utf-8", + ), + ) as { config?: { known_sections?: string[] } }; + expect(manifest.config?.known_sections).toBeUndefined(); + }); }); describe("config set helpers", () => {