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
24 changes: 24 additions & 0 deletions src/lib/sandbox-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@ function setDotpath(obj: Record<string, unknown>, dotpath: string, value: unknow
current[keys[keys.length - 1]] = value;
}

/**
* 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.
*/
function isRecognizedConfigPath(obj: unknown, dotpath: string): boolean {
if (!dotpath || typeof dotpath !== "string") return false;
const keys = dotpath.split(".");
if (keys.some((key) => !key)) return false;

let current: unknown = obj;
for (const key of keys) {
if (current == null || typeof current !== "object" || Array.isArray(current)) return false;
if (!Object.prototype.hasOwnProperty.call(current as Record<string, unknown>, key)) return false;
current = (current as Record<string, unknown>)[key];
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return true;
}

/**
* Parse a config file's raw text according to its format.
*/
Expand Down Expand Up @@ -308,6 +326,11 @@ function configSet(sandboxName: string, opts: ConfigSetOpts = {}): void {
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
const oldValue = extractDotpath(config, opts.key);
console.log(` Agent: ${target.agentName}`);
Expand Down Expand Up @@ -522,6 +545,7 @@ export {
resolveAgentConfig,
extractDotpath,
setDotpath,
isRecognizedConfigPath,
validateUrlValue,
readStdin,
};
51 changes: 50 additions & 1 deletion test/config-set.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { createRequire } from "node:module";
import { describe, it, expect } from "vitest";

// Build must run before these tests (imports from dist/)
const { extractDotpath, setDotpath, validateUrlValue, resolveAgentConfig } = require("../dist/lib/sandbox-config");
const require = createRequire(import.meta.url);
const {
extractDotpath,
isRecognizedConfigPath,
setDotpath,
validateUrlValue,
resolveAgentConfig,
} = require("../dist/lib/sandbox-config");
Comment thread
coderabbitai[bot] marked this conversation as resolved.

describe("resolveAgentConfig", () => {
it("returns openclaw defaults for unknown sandbox", () => {
Expand Down Expand Up @@ -80,6 +88,47 @@ describe("config set helpers", () => {
});
});

describe("isRecognizedConfigPath", () => {
it("accepts an existing top-level key", () => {
expect(isRecognizedConfigPath({ version: 1 }, "version")).toBe(true);
});

it("accepts an existing nested key path", () => {
expect(
isRecognizedConfigPath(
{ agents: { defaults: { model: { primary: "gpt-5.4" } } } },
"agents.defaults.model.primary",
),
).toBe(true);
});

it("accepts existing keys whose value is null", () => {
expect(isRecognizedConfigPath({ provider: { endpoint: null } }, "provider.endpoint")).toBe(true);
});

it("rejects an unknown top-level key", () => {
expect(isRecognizedConfigPath({ version: 1 }, "inference.endpoint")).toBe(false);
});

it("rejects an unknown nested key", () => {
expect(
isRecognizedConfigPath(
{ agents: { defaults: { model: { primary: "gpt-5.4" } } } },
"agents.defaults.model.secondary",
),
).toBe(false);
});

it("rejects malformed dotpaths", () => {
expect(isRecognizedConfigPath({ version: 1 }, "agents..defaults")).toBe(false);
});

it("rejects prototype-inherited keys", () => {
expect(isRecognizedConfigPath({}, "toString")).toBe(false);
expect(isRecognizedConfigPath({ safe: {} }, "safe.constructor")).toBe(false);
});
});

describe("validateUrlValue", () => {
it("accepts public https URLs", () => {
expect(() => validateUrlValue("https://api.nvidia.com/v1")).not.toThrow();
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/test-shields-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ section "Phase 5: config set"

# Set a test key
CONFIG_SET_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" config set \
--key "nemoclaw_e2e_test" --value '"shields-config-e2e"' 2>&1)
--key "agents.defaults.model.primary" --value '"shields-config-e2e"' 2>&1)
echo "$CONFIG_SET_OUTPUT"

if echo "$CONFIG_SET_OUTPUT" | grep -q "Config updated\|config updated"; then
Expand All @@ -280,7 +280,7 @@ else
fi

# Verify the change is visible via config get
VERIFY_SET=$(nemoclaw "${SANDBOX_NAME}" config get --key nemoclaw_e2e_test 2>&1)
VERIFY_SET=$(nemoclaw "${SANDBOX_NAME}" config get --key agents.defaults.model.primary 2>&1)
if echo "$VERIFY_SET" | grep -q "shields-config-e2e"; then
pass "config set change visible in config get"
else
Expand All @@ -298,7 +298,7 @@ fi

# Verify SSRF validation on URLs
SSRF_SET=$(nemoclaw "${SANDBOX_NAME}" config set \
--key "test_url" --value '"http://127.0.0.1:8080/steal"' 2>&1 || true)
--key "agents.defaults.model.primary" --value '"http://127.0.0.1:8080/steal"' 2>&1 || true)
if echo "$SSRF_SET" | grep -qi "private\|validation failed"; then
pass "config set blocks private IP URLs (SSRF)"
else
Expand Down Expand Up @@ -377,7 +377,7 @@ fi
# ══════════════════════════════════════════════════════════════════
section "Phase 8: Config changes persist"

PERSIST_CHECK=$(nemoclaw "${SANDBOX_NAME}" config get --key nemoclaw_e2e_test 2>&1)
PERSIST_CHECK=$(nemoclaw "${SANDBOX_NAME}" config get --key agents.defaults.model.primary 2>&1)
if echo "$PERSIST_CHECK" | grep -q "shields-config-e2e"; then
pass "Config changes survived shields up (persisted)"
else
Expand Down