Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,19 @@ $ nemoclaw <sandbox> channels remove <telegram|discord|slack>
`channels add` stores credentials under `~/.nemoclaw/credentials.json` and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set.
In non-interactive mode (`NEMOCLAW_NON_INTERACTIVE=1`), the commands stage the change and leave the rebuild to a follow-up `nemoclaw <sandbox> rebuild`.

### `nemoclaw <sandbox> config set` refuses a key that does not currently exist

This is intentional.
The host-side `config set` does not maintain a copy of OpenClaw's config schema, so it cannot tell a typo'd key path apart from a schema-valid path that has not been written yet.
To make typos visible without blocking documented first-time writes (such as `provider.compatible-endpoint.timeoutSeconds`), it asks before creating a brand-new key.

In an interactive terminal, accept the prompt to proceed.

In non-interactive mode (CI or `NEMOCLAW_NON_INTERACTIVE=1`), pass `--config-accept-new-path` or set `NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1`.
Modifying a key that already exists in the config never triggers this gate.

Numeric path segments are also refused because `config set` only writes plain objects and would silently overwrite array elements; replace the array as a whole with `--value '[...]'` instead.

### `openclaw config set` or `unset` is blocked inside the sandbox

This is expected.
Expand Down
13 changes: 13 additions & 0 deletions docs/reference/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,19 @@ $ nemoclaw <sandbox> channels remove <telegram|discord|slack>
`channels add` stores credentials under `~/.nemoclaw/credentials.json` and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set.
In non-interactive mode (`NEMOCLAW_NON_INTERACTIVE=1`), the commands stage the change and leave the rebuild to a follow-up `nemoclaw <sandbox> rebuild`.

### `nemoclaw <sandbox> config set` refuses a key that does not currently exist

This is intentional.
The host-side `config set` does not maintain a copy of OpenClaw's config schema, so it cannot tell a typo'd key path apart from a schema-valid path that has not been written yet.
To make typos visible without blocking documented first-time writes (such as `provider.compatible-endpoint.timeoutSeconds`), it asks before creating a brand-new key.

In an interactive terminal, accept the prompt to proceed.

In non-interactive mode (CI or `NEMOCLAW_NON_INTERACTIVE=1`), pass `--config-accept-new-path` or set `NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1`.
Modifying a key that already exists in the config never triggers this gate.

Numeric path segments are also refused because `config set` only writes plain objects and would silently overwrite array elements; replace the array as a whole with `--value '[...]'` instead.

### `openclaw config set` or `unset` is blocked inside the sandbox

This is expected.
Expand Down
224 changes: 193 additions & 31 deletions src/lib/sandbox-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// config set: Host-initiated config mutation with validation.
// config rotate-token: Credential rotation via stdin or env var.

const readline = require("readline");
const fs = require("fs");
const os = require("os");
const path = require("path");
Expand Down Expand Up @@ -137,21 +138,125 @@ function setDotpath(obj: ConfigObject, dotpath: string, value: ConfigValue): voi
}

/**
* Return true when every segment in a dotpath is an own property on the
* current config object, which keeps config set constrained to recognized keys.
* Key segments that must never appear in a dotpath — blocking these prevents
* prototype-pollution and accidental traversal into inherited members.
*/
function isRecognizedConfigPath(obj: ConfigValue, dotpath: string): boolean {
if (!dotpath || typeof dotpath !== "string") return false;
const UNSAFE_KEY_SEGMENTS: ReadonlySet<string> = new Set([
"__proto__",
"constructor",
"prototype",
"toString",
"hasOwnProperty",
]);

type DotpathValidation = { ok: true } | { ok: false; reason: string };

/**
* Validate the syntax of a config dotpath: non-empty, no empty segments, no
* prototype-pollution / inherited-member segments. Schema validity is not
* checked here — `configSet` handles unknown paths via an interactive
* confirm or a `--config-accept-new-path` opt-in so first-time writes
* under unset namespaces stay possible (see #2400).
*/
function validateConfigDotpath(dotpath: string): DotpathValidation {
if (!dotpath || typeof dotpath !== "string") {
return { ok: false, reason: "key is empty" };
}
const keys = dotpath.split(".");
for (const key of keys) {
if (!key) return { ok: false, reason: "key contains an empty segment" };
if (UNSAFE_KEY_SEGMENTS.has(key)) {
return { ok: false, reason: `segment '${key}' is reserved` };
}
}
return { ok: true };
}

/**
* Walk a dotpath and report the first reason `configSet` should refuse it:
*
* - Numeric segment: would target an array index, but `setDotpath` always
* materialises plain objects, so allowing this would either clobber an
* existing array or create a confusingly object-shaped "array".
* - Non-object ancestor: an existing intermediate value (string, number,
* null, array, …) would be silently overwritten by `setDotpath` on its
* way to the leaf.
*
* Missing ancestors are fine — they get materialised on write. Returns
* `null` when no refusal reason applies.
*/
function findClobberingAncestor(
obj: ConfigValue,
dotpath: string,
): { segment: string; reason: string } | null {
const keys = dotpath.split(".");
if (keys.some((key) => !key)) return false;

for (let i = 0; i < keys.length; i++) {
if (/^\d+$/.test(keys[i])) {
return {
segment: keys.slice(0, i + 1).join("."),
reason: "is a numeric segment, but 'config set' does not support array editing",
};
}
}

if (keys.length <= 1) return null;

let current: ConfigValue = obj;
for (const key of keys) {
if (!isConfigObject(current)) return false;
if (!Object.prototype.hasOwnProperty.call(current, key)) return false;
current = current[key];
for (let i = 0; i < keys.length - 1; i++) {
if (!isConfigObject(current)) {
return {
segment: keys.slice(0, i).join(".") || "(root)",
reason: `is ${describeNonConfigValue(current)}, not a config object`,
};
}
const key = keys[i];
if (!Object.prototype.hasOwnProperty.call(current, key)) {
return null;
}
const next = current[key];
if (!isConfigObject(next)) {
return {
segment: keys.slice(0, i + 1).join("."),
reason: `is ${describeNonConfigValue(next)}, not a config object`,
};
}
current = next;
}
return null;
}

function describeNonConfigValue(value: ConfigValue): string {
if (value === null) return "null";
if (Array.isArray(value)) return "an array";
return `a ${typeof value}`;
}

/**
* Decide what to do when `config set` targets a key that does not yet exist.
* Returns `accept` if an explicit override (CLI flag or env) is in effect,
* `prompt` if the caller should ask the user interactively, and `refuse`
* otherwise. Inputs are passed in so the gate can be tested without
* touching `process.env` or `process.stdin`.
*/
type NewKeyGate = { mode: "accept" } | { mode: "prompt" } | { mode: "refuse" };

interface NewKeyGateInputs {
acceptNewPath?: boolean;
acceptEnv?: string;
isTTY?: boolean;
nonInteractiveEnv?: string;
}

function classifyNewKeyGate(inputs: NewKeyGateInputs): NewKeyGate {
if (inputs.acceptNewPath === true || inputs.acceptEnv === "1") {
return { mode: "accept" };
}
return true;
const interactive = !!inputs.isTTY && inputs.nonInteractiveEnv !== "1";
if (!interactive) {
return { mode: "refuse" };
}
return { mode: "prompt" };
}

/**
Expand Down Expand Up @@ -299,9 +404,10 @@ interface ConfigSetOpts {
key?: string | null;
value?: string | null;
restart?: boolean;
acceptNewPath?: boolean;
}

function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
async function configSet(sandboxName: string, opts: ConfigSetOpts = {}): Promise<void> {
validateName(sandboxName, "sandbox name");

if (!opts.key) {
Expand All @@ -316,16 +422,22 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
process.exit(1);
}

const dotpathCheck = validateConfigDotpath(opts.key);
if (!dotpathCheck.ok) {
console.error(` Invalid config key '${opts.key}': ${dotpathCheck.reason}.`);
process.exit(1);
}

const target = resolveAgentConfig(sandboxName);

// 1. Read current config
// Read current config
console.log(` Reading ${target.agentName} config...`);
const config = readSandboxConfig(sandboxName, target);

// 2. Parse and validate value
// Parse and validate value
const parsedValue = parseCliConfigValue(opts.value);

// 3. Validate URLs for SSRF. validateUrlValue no-ops on non-URL input,
// Validate URLs for SSRF. validateUrlValue no-ops on non-URL input,
// so run it for every string to avoid bypasses via mixed-case schemes
// ("HTTP://127.0.0.1") or leading whitespace.
if (typeof parsedValue === "string") {
Expand All @@ -338,36 +450,70 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
}
}

// 4. Check that we're not modifying the gateway section (contains auth tokens)
// Check that we're not modifying the gateway section (contains auth tokens)
if (opts.key.startsWith("gateway.") || opts.key === "gateway") {
console.error(" Cannot modify the gateway section directly.");
console.error(" Use `nemoclaw config rotate-token` for credential changes.");
process.exit(1);
}

if (!isRecognizedConfigPath(config, opts.key)) {
console.error(
` Key validation failed: "${opts.key}" is not a recognized ${target.agentName} config path.`,
);
process.exit(1);
}

// 5. Show what will change
// 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
// Refuse outright if writing this path would silently overwrite an
// existing scalar ancestor or target an array index — setDotpath would
// either replace the scalar with a fresh empty object or clobber the
// array on its way to the leaf.
const refusal = findClobberingAncestor(config, opts.key);
if (refusal) {
console.error(
` Cannot set '${opts.key}' in ${target.agentName} config: '${refusal.segment}' ${refusal.reason}.`,
);
process.exit(1);
}

// First-time writes go through a confirmation gate so users get a
// signal when they are creating a brand-new key (which may be a typo)
// without coupling the validator to OpenClaw's evolving config schema
// (see #2400).
if (oldValue === undefined) {
const gate = classifyNewKeyGate({
acceptNewPath: opts.acceptNewPath,
acceptEnv: process.env.NEMOCLAW_CONFIG_ACCEPT_NEW_PATH,
isTTY: process.stdin.isTTY,
nonInteractiveEnv: process.env.NEMOCLAW_NON_INTERACTIVE,
});
if (gate.mode === "refuse") {
console.error(
` Key '${opts.key}' does not currently exist in the ${target.agentName} config.`,
);
console.error(
" Re-run interactively, pass --config-accept-new-path, or set NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1.",
);
process.exit(1);
}
if (gate.mode === "prompt") {
const confirmed = await confirmYesNo(" Write this new key? [y/N] ");
if (!confirmed) {
console.error(" Aborted.");
process.exit(1);
}
}
}

// Apply change
setDotpath(config, opts.key, parsedValue);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 7. Write to temp file in the agent's native format
// 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)
// 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(
Expand All @@ -392,7 +538,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
{ input: content, stdio: ["pipe", "pipe", "pipe"], timeout: 15000 },
);

// 9. Fix ownership via kubectl exec (bypasses Landlock)
// Fix ownership via kubectl exec (bypasses Landlock)
try {
execFileSync(
"docker",
Expand All @@ -417,15 +563,15 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
// Best effort — chown failure is non-fatal
}

// 10. Cleanup temp
// Cleanup temp
try {
fs.unlinkSync(tmpFile);
fs.rmdirSync(tmpDir);
} catch {
// Best effort
}

// 11. Audit log
// Audit log
appendAuditEntry({
action: "shields_down",
sandbox: sandboxName,
Expand All @@ -435,7 +581,7 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {

console.log(` ${target.agentName} config updated.`);

// 12. Restart if requested
// Restart if requested
if (opts.restart) {
console.log(" Restarting sandbox agent process...");
const restartBinary = getOpenshellBinary();
Expand Down Expand Up @@ -601,6 +747,20 @@ function readStdin(): Promise<string> {
});
}

/**
* Ask a yes/no question on stderr. Returns true only when the answer matches
* /^y(es)?$/i — empty, "no", or unparseable input is treated as no.
*/
function confirmYesNo(prompt: string): Promise<boolean> {
return new Promise((resolve) => {
const rl = readline.createInterface({ input: process.stdin, output: process.stderr });
rl.question(prompt, (answer: string) => {
rl.close();
resolve(/^y(es)?$/i.test(answer.trim()));
});
});
}

// ---------------------------------------------------------------------------
// Exports
// ---------------------------------------------------------------------------
Expand All @@ -612,7 +772,9 @@ export {
resolveAgentConfig,
extractDotpath,
setDotpath,
isRecognizedConfigPath,
validateConfigDotpath,
findClobberingAncestor,
classifyNewKeyGate,
validateUrlValue,
readStdin,
};
Loading
Loading