Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
62 changes: 48 additions & 14 deletions src/lib/sandbox-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,50 @@ 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.
* Top-level namespaces that `config set` is allowed to write. The check is
* deliberately a roots allow-list rather than a walk of the loaded config so
* first-time sets under an unset namespace succeed (see #2400). `gateway`
* is blocked separately because it holds auth tokens — use
* `nemoclaw config rotate-token` for that. Other build-time keys
* (integrity hashes, channels baked into the image) are intentionally not
* listed here because modifying them at runtime would desync with the
* startup hash check.
*/
function isRecognizedConfigPath(obj: unknown, dotpath: string): boolean {
const RECOGNIZED_TOP_LEVEL_KEYS: ReadonlySet<string> = new Set([
"provider",
"model",
"agent",
"agents",
"tools",
"mcpServers",
"runtime",
"version",
]);

/**
* Key segments that must never appear in a dotpath — blocking these prevents
* prototype-pollution and accidental traversal into inherited members.
*/
const UNSAFE_KEY_SEGMENTS: ReadonlySet<string> = new Set([
"__proto__",
"constructor",
"prototype",
"toString",
"hasOwnProperty",
]);

/**
* Return true when a dotpath is syntactically valid and its first segment
* is in RECOGNIZED_TOP_LEVEL_KEYS. Does not consult the loaded config, so a
* first-time write under a recognized namespace (e.g.
* `provider.compatible-endpoint.timeoutSeconds` on a fresh install) is
* accepted even before the intermediate objects exist.
*/
function isRecognizedConfigPath(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];
}
return true;
if (keys.some((key) => !key || UNSAFE_KEY_SEGMENTS.has(key))) return false;
return RECOGNIZED_TOP_LEVEL_KEYS.has(keys[0]);
}
Comment thread
laitingsheng marked this conversation as resolved.
Resolved

/**
Expand Down Expand Up @@ -349,8 +378,13 @@ 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.`);
if (!isRecognizedConfigPath(opts.key)) {
console.error(
` Key validation failed: "${opts.key}" is not a recognized ${target.agentName} config path.`,
);
console.error(
` Allowed top-level namespaces: ${[...RECOGNIZED_TOP_LEVEL_KEYS].sort().join(", ")}.`,
);
process.exit(1);
}

Expand Down
47 changes: 21 additions & 26 deletions test/config-set.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,43 +89,38 @@ describe("config set helpers", () => {
});

describe("isRecognizedConfigPath", () => {
it("accepts an existing top-level key", () => {
expect(isRecognizedConfigPath({ version: 1 }, "version")).toBe(true);
it("accepts a recognized top-level key", () => {
expect(isRecognizedConfigPath("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 a deeply nested path under a recognized root", () => {
expect(isRecognizedConfigPath("agents.defaults.model.primary")).toBe(true);
});

it("accepts existing keys whose value is null", () => {
expect(isRecognizedConfigPath({ provider: { endpoint: null } }, "provider.endpoint")).toBe(true);
// Regression: #2400 — first-time writes under an unset namespace used
// to fail the old "walk the loaded config" check even when the key
// path was schema-valid. The roots allow-list must accept them.
it("accepts first-time writes under an unset recognized namespace", () => {
expect(isRecognizedConfigPath("provider.compatible-endpoint.timeoutSeconds")).toBe(true);
expect(isRecognizedConfigPath("mcpServers.my-server.command")).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 an unrecognized top-level key", () => {
expect(isRecognizedConfigPath("inference.endpoint")).toBe(false);
expect(isRecognizedConfigPath("gateway.token")).toBe(false);
});

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

it("rejects prototype-inherited keys", () => {
expect(isRecognizedConfigPath({}, "toString")).toBe(false);
expect(isRecognizedConfigPath({ safe: {} }, "safe.constructor")).toBe(false);
it("rejects prototype-pollution segments anywhere in the path", () => {
expect(isRecognizedConfigPath("toString")).toBe(false);
expect(isRecognizedConfigPath("agents.constructor")).toBe(false);
expect(isRecognizedConfigPath("provider.__proto__.polluted")).toBe(false);
expect(isRecognizedConfigPath("tools.hasOwnProperty")).toBe(false);
});
});

Expand Down
Loading