settingsSet writes return {"result":"saved"} but silently write nothing (params.data/item wrapper never unwrapped)

[cyworxs-prez]

Summary

Every settingsSet-style write through the MCP returns {"result":"saved"} with HTTP 200 — and changes nothing on the firewall. The write is silently discarded.

This is the inverse symptom of #4. That issue reports writes returning {"result":"failed"}, which is loud and self-evident. This one reports success. Nothing in the response, the HTTP status, or the logs indicates anything went wrong. The only way to detect it is to read the value back after every write.

Filing separately from #4 because the symptom, the affected method family, and the searchability are all different:

	#4	this
Response	{"result":"failed"}	{"result":"saved"}
Methods	addItem / setItem / addRule (take uuid)	settingsSet family (no uuid)
Detectability	immediate	silent — requires read-back

Anyone searching "returns saved but nothing changed" will not find #4.

Environment

• @richard-stovall/opnsense-mcp-server 0.5.3 (current dist-tags.latest)
• @richard-stovall/opnsense-typescript-client (bundled)
• OPNsense 26.1.x
• Node 22, macOS

Reproduction

ids.LogPayload is a harmless boolean — good for demonstrating this.

1. Read the current value:

{ "tool": "ids_manage", "arguments": { "method": "settingsGet" } }

→ ids.general.LogPayload is "0"

2. Write the documented shape — the tool schema itself advertises a data property:

{
  "tool": "ids_manage",
  "arguments": {
    "method": "settingsSet",
    "params": { "data": { "ids": { "general": { "LogPayload": "1" } } } }
  }
}

→ {"data":{"result":"saved"},"status":200,"statusText":"OK"}

3. Read it back:

{ "tool": "ids_manage", "arguments": { "method": "settingsGet" } }

→ ids.general.LogPayload is still "0"

Same result with params.item instead of params.data.

Root cause

callModularTool in index.js passes the entire params bag through as the request body:

const { method: _, params = {}, ...otherArgs } = args;
const callParams = { ...params, ...otherArgs };
...
return await method.call(moduleObj, callParams);   // callParams = {data: {ids: {...}}}

The client method is positional and treats its first argument as the body:

async settingsSet(data, config) {
    return this.http.post(`/api/ids/settings/set`, data, config);
}

So the posted body is:

{"data": {"ids": {"general": {"LogPayload": "1"}}}}

when OPNsense expects:

{"ids": {"general": {"LogPayload": "1"}}}

OPNsense's setBase reads the top-level model key (ids) from the POST body. It isn't there, so setNodes receives nothing, no fields are written — and the controller still returns {"result":"saved"}, because from its perspective an empty update succeeded.

The tool schema declares a data wrapper property that the dispatcher never unwraps.

Impact

Silent write failure is materially worse than a loud one. An agent driving this MCP will report a successful configuration change to the user, and the firewall will be unchanged. In our case this masked a real config change for some time — we only caught it because we read every write back.

Worth noting the blast radius: this affects the settingsSet method of every module that has one (ids, cron, dnsmasq, captiveportal, dhcrelay, and others — same generated shape throughout).

Fix

Already fixed by #11, which is open and unmerged. Its dispatcher rewrite handles this case correctly:

const { uuid, item, data } = callParams;
const body = item !== undefined ? item : data;
if (uuid !== undefined) { ... }
if (body !== undefined) {
  return await method.call(moduleObj, body);   // settingsSet({ids:{...}}) — correct
}

I verified this independently — see my comment on #11.

For anyone blocked before #11 lands, the minimal standalone change is to unwrap a body consisting solely of a single data or item key, immediately before the existing method.call(moduleObj, callParams):

const unwrapBody = (o) => {
  if (!o || typeof o !== 'object' || Array.isArray(o)) return o;
  const k = Object.keys(o);
  if (k.length === 1 && (k[0] === 'data' || k[0] === 'item')) {
    const inner = o[k[0]];
    if (inner && typeof inner === 'object' && !Array.isArray(inner)) return inner;
  }
  return o;
};
return await method.call(moduleObj, unwrapBody(callParams));

Deliberately narrow — only a sole data/item key is unwrapped, so {searchPhrase, rowCount}, bare bodies like {test:{...}}, and multi-key params are untouched. #11's approach is broader and better; this is just a stopgap.

Verification

Driven against a patched server over JSON-RPC stdio, independent of any MCP client:

LogPayload before   : '0'
writing LogPayload  : '1' via {data:{ids:{general:{...}}}}
settingsSet returned: {'result': 'saved'}
LogPayload after    : '1'
>>> RESULT: WRITE WORKS
restored to         : '0'

Before the patch, step 3 read back '0' while step 2 still reported saved.