Description

The gateway tool’s config mutation guard moved from a denylist of protected paths to an allowlist of “agent-tunable” paths. As part of this, agents.list[].id is allowlisted, and the new changed-path collector treats agents.list as a keyed array by id.

Because deletions/renames of keyed entries are represented as per-entry diffs at the synthetic path agents.list[], removing an entry (or changing its id) is reduced to changes on that entry’s leaf fields (e.g., agents.list[].id, agents.list[].model). Those leaf fields are allowlisted, so the guard will not block removal/renaming.

Impact depends on deployment, but in configs where additional agents provide safety/oversight (e.g., monitoring/policy/approval agents) and their entries only contain allowlisted fields, an untrusted model could:

• Delete those agents from agents.list, making them unavailable for routing / enforcement.
• Rename an agent id, causing role/routing confusion (downstream code identifies agents by normalized id).

Relevant code:

const ALLOWED_GATEWAY_CONFIG_PATHS = [
  ​// ...
  "agents.list[].id",
  "agents.list[].model",
  ​// ...
];

​// When a keyed array entry disappears, nextEntries.get(id) is undefined,
​// so collectConfigLeafPaths() records only leaf paths under `${basePath}[]`.
collectChangedConfigPaths(currentEntries.entries.get(id), nextEntries.entries.get(id), `${basePath}[]`, out);

Downstream, agent identity and availability are driven by agents.list ids:

• listAgentIds(cfg) returns ids derived from cfg.agents.list entries; removing entries removes those agent ids from the system.

Recommendation

Fail closed on membership/identity changes to agents.list.

Options:

1. Remove agents.list[].id from the allowlist and require operator control for renames.
2. Forbid deletions of existing agent entries via agent-driven config.patch/config.apply (or allow only additions), by explicitly checking keyed-array id sets.
3. Treat any keyed-array membership change as a change to the parent path (agents.list) and block unless explicitly allowlisted.

Example hardening (conceptual):

if (basePath === "agents.list") {
  const currentIds = new Set(currentEntries.entries.keys());
  const nextIds = new Set(nextEntries.entries.keys());
  if (!setsEqual(currentIds, nextIds)) {
    out.add("agents.list"); // not allowlisted => blocked
    return out;
  }
}

Also consider an explicit invariant that safety/oversight agents must be present and immutable via agent-facing tools.

Description

assertGatewayConfigMutationAllowed computes changed paths by recursively traversing the entire config and performing deep structural comparisons without any explicit bounds.

• raw is parsed as JSON5 with no maximum size/depth limits.
• For config.patch, the mutation is applied (applyMergePatch) and then the entire current and next configs are diffed.
• collectChangedConfigPaths() performs isDeepStrictEqual(currentValue, nextValue) at every recursion level and, when unequal, recurses into objects/arrays and builds Sets of keys/ids.
• Finally changedPaths is expanded into an array and sorted (toSorted()), adding extra CPU/memory pressure.

An agent (not a trusted principal per comments) that can call the owner-only gateway tool could provide a very large/deep raw object to induce high CPU usage and memory allocations, potentially blocking the gateway event loop (denial-of-service).

Vulnerable code:

const nextConfig = ...applyMergePatch(...)
const changedPaths = [...collectChangedConfigPaths(params.currentConfig, nextConfig)].toSorted();
...
function collectChangedConfigPaths(currentValue, nextValue, basePath = "", out = new Set()) {
  if (isDeepStrictEqual(currentValue, nextValue)) return out;
  ... // recursively walks objects/arrays
}

Recommendation

Add explicit resource bounds and early-exit behavior for agent-supplied config mutations.

Suggested mitigations (combine as appropriate):

1. Cap input size for raw (and/or parsed node count/depth) before parsing/diffing.

2. Short-circuit diffing: instead of enumerating all changed paths and sorting them, walk the parsed mutation object (the patch) and validate that only allowlisted paths are being touched; fail fast on the first disallowed path.

3. Bound recursion by tracking depth and total visited nodes/keys and throwing an error if exceeded.

Example guard (illustrative):

const MAX_RAW_BYTES = 50_000;
if (params.raw.length > MAX_RAW_BYTES) {
  throw new Error(`gateway ${params.action} raw too large`);
}

function collectChangedConfigPaths(..., depth = 0, budget = { nodes: 0 }) {
  if (depth > 50) throw new Error("config mutation too deep");
  if (++budget.nodes > 20_000) throw new Error("config mutation too large");
  ...
}

This prevents a model/agent from supplying pathological payloads that monopolize CPU/memory.

Description

isAllowedGatewayConfigPath() treats allowlist entries as prefixes (pattern length <= path length), so any descendant of an allowlisted path is also allowed.

This becomes dangerous in combination with collectChangedConfigPaths(): when a value changes type (e.g., string → object), it records only the base path as changed and does not enumerate the new object's leaf paths. If that base path is allowlisted, an attacker can submit a patch that replaces an allowlisted scalar with an object containing arbitrary nested keys, and the guard will still allow the mutation.

Impact depends on downstream config consumers, but common outcomes include:

• Fail-open / logic bypass if code treats non-boolean values as truthy (e.g., requireMention set to an object)
• Denial of service if code assumes a string/boolean and crashes on object access
• Future bypass risk if additional semantics are later added under an allowlisted key.

Vulnerable code:

​// Prefix match: patternSegments.length > pathSegments.length is the only length check.
if (patternSegments.length > pathSegments.length) {
  return false;
}
...
return true;

Recommendation

Make allowlist matching exact by default (same number of segments), and only allow prefix/descendant matches when explicitly requested (e.g., patterns ending with .*).

Additionally, when types differ in collectChangedConfigPaths, consider collecting leaf paths from the new value too, so nested keys cannot be smuggled under an allowlisted parent.

Example fix (exact match):

function isAllowedGatewayConfigPath(path: string): boolean {
  const pathSegments = path.split(".");
  return ALLOWED_GATEWAY_CONFIG_PATHS.some((pattern) => {
    const patternSegments = pattern.split(".");
    if (patternSegments.length !== pathSegments.length) return false;
    for (let i = 0; i < patternSegments.length; i++) {
      if (!pathSegmentMatches(patternSegments[i], pathSegments[i])) return false;
    }
    return true;
  });
}

Example fix (type-change leaf enumeration):

​// when current/next are different primitive/object kinds
collectConfigLeafPaths(currentValue, basePath, out);
collectConfigLeafPaths(nextValue, basePath, out);

1. src/agents/openclaw-gateway-tool.test.ts, line 1 (link)

   PR description describes a different change than what is implemented

   The PR title says "fix(gateway): fail closed on runtime config edits" but the description is entirely about fixing webhook route secret caching (createTaskFlowWebhookRequestHandler, WeakMap, extensions/webhooks/src/http.test.ts). The actual code changes are in src/agents/tools/gateway-tool.ts and convert the config-mutation guard from a denylist to an allowlist. The description mentions no files that appear in this diff and references test runs of a completely separate extension. Future reviewers and bisect tooling will find it very difficult to correlate this PR with the real change. The description should be rewritten to describe the gateway-tool allowlist migration.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/agents/openclaw-gateway-tool.test.ts
   Line: 1
   
   Comment:
   **PR description describes a different change than what is implemented**
   
   The PR title says "fix(gateway): fail closed on runtime config edits" but the description is entirely about fixing webhook route secret caching (`createTaskFlowWebhookRequestHandler`, `WeakMap`, `extensions/webhooks/src/http.test.ts`). The actual code changes are in `src/agents/tools/gateway-tool.ts` and convert the config-mutation guard from a denylist to an allowlist. The description mentions no files that appear in this diff and references test runs of a completely separate extension. Future reviewers and bisect tooling will find it very difficult to correlate this PR with the real change. The description should be rewritten to describe the gateway-tool allowlist migration.
   
   How can I resolve this? If you propose a fix, please make it concise.