Reject unrecognized parameters in set_configuration instead of reporting success

[kriszyp]

set_configuration resolved each request key through CONFIG_PARAM_MAP and, on a miss, ended the loop iteration with no error and no log — then returned Configuration successfully set. A typo, or a config block Harper does not expose as a param, was accepted with HTTP 200 and never written. The operation now preflights every request key and rejects the whole request with a 400 naming each unrecognized param, before anything is written locally.

Verified live on 5.2.4 before the fix: a models block and logging_levl both returned success and changed nothing.

For the human reviewer

1. Which layer enforces this. The preflight sits in setConfiguration, not in updateConfigValue's bulk loop, and not as a Joi schema in operationsValidation.js like most operations. The writer must stay permissive: createConfigFile merges installParams carrying install-only keys (INSTALL_PROMPTS.DEFAULTS_MODE, HDB_CONFIG) at utility/install/installer.ts:617, so strictness there aborts install. A Joi schema would also cover future bulk writers and match how every other operation validates — but it would have to duplicate the dynamic alias table and the _package/_port rule outside the config subsystem. Cheap to move later, except that the error text and status become API surface the moment this ships.

2. This is a breaking change; it is milestoned v5.3 (Kris's call, 2026-08-22). set_configuration has tolerated extra keys for years. Any client that sends one — Studio spreads arbitrary ...changes from its config UI — starts failing on upgrade, and the failure is total rather than partial, so it holds for a minor rather than riding a 5.2.x patch.

   The scope question behind that call, for the record: no request that previously succeeded and applied its changes behaves differently. The only changes outside new error responses run the other way — getConfigValue/createConfigFile previously threw a TypeError on an inherited or non-string param name and now return undefined/skip, which removes a crash rather than adding one. Everything else is a 200-that-silently-discarded becoming a 400.

3. Typo-shaped names are still accepted. The preflight reuses the _package/_port escape hatch verbatim, so logging_port or htpp_port passes validation and is written as a component entry. That is arguably the larger half of set_configuration silently discards unrecognized params and returns success (no error on a CONFIG_PARAM_MAP miss) #2266 for a real operator. Tightening it (requiring the prefix to look like a component name) is a follow-up, not a revert.

4. hdb_auth_header is stripped silently, where registryAuth is rejected with a rename message (components/operationsValidation.js:525). I chose stripping because it is the 4.x spelling of an internal auth artifact that clients never typed, so a 400 would break legacy-shaped callers for no benefit. A reviewer may want consistency with the registryAuth precedent instead.

5. Escape-hatch case sensitivity is now pinned by a test. _package/_port match case-sensitively while every mapped param matches case-insensitively, so TYPO_PORT is rejected. That asymmetry is pre-existing behavior I preserved rather than a decision I made — but the test now encodes it, so a reviewer reading it as a bug will find a test asserting it is intended.

Round-3 review added two hardening nits, both applied: the error message now strips control characters and caps the number of names it lists (a key containing a newline could otherwise forge a line in the operations log, and a body with thousands of unknown keys produced an unbounded message), and the atomicity integration case now reads the current value rather than assuming what an earlier test left behind, so it holds under isolation or reordering.

One nit is knowingly carried: the new unit cases sit inside the file's existing Sinon/rewire describe block rather than a real-module seam. The rejection cases need no stub at all — they throw before the writer is reached — but the atomicity assertion has to observe that the writer was not called, and reusing the harness that already exists beat introducing a second one.

Two findings from the review are real, pre-existing, and deliberately not fixed here — both are the same false-success family and are filed: #2269 (a resolved param whose configDoc.setIn throws is caught, logged, and still reported as success) and #2270 (on a replicated: true fan-out the origin overwrites the per-node response with the success string). #2270 matters more because of this PR: a peer that used to silently ignore an unknown name now 400s the whole request, so during a mixed-version window older peers apply neither param where they previously applied the recognized one. That widens divergence, and it is only visible in response.replicated. Worth deciding alongside decision 2 below.

Verification

• Route (a) — extended the existing integration suite. integrationTests/apiTests/configuration.test.mjs gains four HTTP-level cases: single unknown name 400s naming it; several unknown names produce one 400 naming each; a mixed recognized/unrecognized request 400s and a follow-up get_configuration proves the recognized half did not land; a _package entry still writes (asserted via get_configuration, then removed so it does not leak into later tests).
• Unit: npx mocha --conditions=typestrip unitTests/config/configUtils.test.js → 87 passing.
• Fails-on-base: restored config/configUtils.ts from origin/main, deleted dist/config/configUtils.js and the tsbuildinfo, rebuilt, re-ran → 6 failing (the five rejection cases plus the control-field expectation). Three control cases pass on either side. Restored and rebuilt → 87 passing.
• Live: the models and logging_levl payloads from set_configuration silently discards unrecognized params and returns success (no error on a CONFIG_PARAM_MAP miss) #2266 reproduced on a 5.2.4 instance; also confirmed toString/hasOwnProperty/valueOf take the same silent-success path, while constructor is already blocked upstream by a body-property guard — which is why the Object.hasOwn guard is described as defense-in-depth for direct callers rather than a reachable HTTP fix.

Fixes #2266

Complexity: medium

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=6 @ c98b40e}

_{Human-Review-Need: 3 @ c98b40e}

[gemini-code-assist]

Code Review

This pull request introduces robust validation for configuration parameters in config/configUtils.ts by checking for unrecognized parameters before applying any local writes, preventing partial updates, and returning a 400 Bad Request error. It also adds corresponding integration and unit tests. The review feedback suggests enhancing robustness by adding explicit string type checks in lookupConfigParam and isSuffixEscapedParam to prevent runtime TypeError errors. Additionally, it recommends using optional chaining in test assertions to avoid masking server errors, and using assert.strictEqual from the bare node:assert module in accordance with repository standards.