fix: reject unrecognized config paths in config set (Fixes #2019)

[deepujain]

Fixes #2019

Summary

nemoclaw <name> config set was accepting arbitrary dotpaths and writing them straight into openclaw.json. That made it easy to create config keys that OpenClaw does not recognize, which then breaks agent startup.

This change makes config set reject unrecognized config paths before it writes anything.

Changes

• src/lib/sandbox-config.ts: added isRecognizedConfigPath() and made configSet() fail fast when the requested dotpath is not already present in the current agent config.
• test/config-set.test.ts: added regression coverage for recognized and unrecognized config paths.
• test/e2e/test-shields-config.sh: updated the config-set smoke path to mutate a real existing config field instead of relying on creating a brand-new top-level key.

Testing

• npm run build:cli
• npm run typecheck:cli
• npm test -- test/config-set.test.ts test/config-rotate-token.test.ts

Evidence It Works

• config set --key inference.endpoint ... now fails with a key validation error instead of corrupting openclaw.json.
• The focused config helper tests pass, including the new path-recognition regression coverage.

Notes

• I also ran npm test. In this worktree it still reproduces unrelated existing failures in test/legacy-path-guard.test.ts, src/lib/preflight.test.ts, src/lib/sandbox-version.test.ts, and test/install-preflight.test.ts.

Signed-off-by: Deepak Jain [email protected]

Summary by CodeRabbit

• New Features

  • Configuration commands now validate that dot-path keys are well-formed and recognized; invalid keys produce a clear error and terminate the operation.

• Tests

  • Added unit tests for valid/invalid configuration paths and updated end-to-end tests to verify the new validation and persistence behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: add401f0-e377-4c71-9967-c40aaee5ec3a

📥 Commits

Reviewing files that changed from the base of the PR and between 359c8bf and 0a29bf0.

📒 Files selected for processing (3)

• src/lib/sandbox-config.ts
• test/config-set.test.ts
• test/e2e/test-shields-config.sh

🚧 Files skipped from review as they are similar to previous changes (3)

• src/lib/sandbox-config.ts
• test/e2e/test-shields-config.sh
• test/config-set.test.ts

---

📝 Walkthrough

Walkthrough

Adds and exports isRecognizedConfigPath(obj, dotpath) and integrates it into the config-set flow so dotpath keys are validated against the loaded agent config; failures log "Key validation failed" and exit with status code 1 before writing changes.

Changes

Cohort / File(s)	Summary
Config Path Validation src/lib/sandbox-config.ts	Introduces exported isRecognizedConfigPath(obj: unknown, dotpath: string): boolean; configSet() now calls it after gateway-section guard and before reading/applying values, logging an error and exiting (code 1) on validation failure.
Unit Tests test/config-set.test.ts	Imports isRecognizedConfigPath and adds tests asserting true for valid top-level/nested paths (including null leafs) and false for unknown keys, malformed dotpaths, and prototype-inherited properties.
E2E Script test/e2e/test-shields-config.sh	Updates Phase 5/8 test key from nemoclaw_e2e_test to agents.defaults.model.primary and adjusts SSRF-phase config set/config get checks accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibbled through dots, each segment I sought,
I checked every branch for the keys that were taught.
No stray phantom paths may now creep in the night,
Configs stay tidy, and launches take flight. 🎈

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main purpose of the PR: rejecting unrecognized config paths in config set, and references the related issue.
Linked Issues check	✅ Passed	The PR implementation matches all key objectives from #2019: validates config paths exist in current agent config [src/lib/sandbox-config.ts], rejects unrecognized paths with error logging [src/lib/sandbox-config.ts], and includes comprehensive regression tests [test/config-set.test.ts].
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing #2019: the validator implementation, validation tests, and e2e test alignment to use existing config paths are all within scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/sandbox-config.ts`:
- Around line 116-125: The function isRecognizedConfigPath currently uses the
"in" operator which allows prototype-inherited keys (e.g., "toString") to pass
validation; update the check inside isRecognizedConfigPath to use
Object.prototype.hasOwnProperty.call(current as Record<string, unknown>, key)
instead of "(key in (current as Record<string, unknown>))" so only own
properties are accepted, and add unit tests exercising paths like "toString" and
"constructor" to assert they are rejected to prevent prototype-chain bypasses.

In `@test/config-set.test.ts`:
- Around line 7-13: Replace the CommonJS require at module level by converting
the top-level import to an ESM-compatible pattern: use Node's
createRequire(import.meta.url) to load the compiled CommonJS module and then
destructure the exported symbols (extractDotpath, isRecognizedConfigPath,
setDotpath, validateUrlValue, resolveAgentConfig) from that require result;
update the test file imports to follow the same pattern used in
test/registry.test.ts and test/onboard.test.ts so the test-module convention is
respected.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6952bcf4-8470-4da7-8339-6528a728466d

📥 Commits

Reviewing files that changed from the base of the PR and between 9222cae and 334c167.

📒 Files selected for processing (3)

• src/lib/sandbox-config.ts
• test/config-set.test.ts
• test/e2e/test-shields-config.sh

[deepujain]

Rebased on current main and addressed the CodeRabbit follow-up. The config path check now only accepts own properties, the test import uses createRequire, and the focused config tests pass locally.

[wscurran]

✨ Thanks for submitting this PR that proposes a fix to reject unrecognized config paths in config set, which could help improve the getting started process and prevent config corruption.

---

Possibly related open issues:

• #2019 [NemoClaw][All Platform] nemoclaw config set missing key validation allows writing invalid config path

[deepujain]

Rebased on current main and reran the focused config checks. npm run build:cli and npm test -- test/config-set.test.ts test/config-rotate-token.test.ts both pass locally.

[ericksoa]

Looks good. The validation logic is solid — handles empty segments, null intermediates, arrays, and prototype-chain keys correctly. Test coverage is thorough and the E2E update is clean.

One minor observation for a future follow-up: the validation allows setting intermediate (non-leaf) nodes. E.g., config set --key agents.defaults --value '"foo"' would pass validation since agents.defaults is a recognized own property, but it would overwrite the entire defaults object with a string. Not introduced by this PR and not a blocker — just something to consider adding a leaf-only or type-mismatch guard for down the road.