Skip to content

PUT /api/v1/admin/agents/{id} silently wipes an agent when fields are omitted — no validation, no partial-update alternative #74

Description

@JohnV-AAAA

Summary

PUT /api/v1/admin/agents/{id} performs a full replacement of the stored agent, but accepts a body with missing or empty core fields and persists the result. A request that omits name, llm and mcpServers leaves a syntactically valid but functionally dead agent in store.json, with no error returned and no warning logged. There is no PATCH endpoint, so any caller wanting to change one field must reconstruct the entire object correctly or destroy the agent.

Impact

This turns a routine API probe into silent production data loss. In our deployment, a single exploratory PUT against a live production agent (a paid service) wiped its name, its LLM backend binding and all five of its attached MCP servers in one call. The API returned success. The agent kept answering the admin listing endpoint — with an empty name — so the breakage was not visible from the API surface; it only surfaced when the agent stopped producing output. Recovery required restoring store.json from a filesystem backup.

Reproduction

  1. Create an agent with a name, an llm block (backend + model) and one or more mcpServers.
  2. Issue PUT /api/v1/admin/agents/{id} with a body that omits name, llm and mcpServers (for example, a body carrying only one unrelated field a caller intended to update).
  3. Observe: HTTP success, no validation error.
  4. Inspect the persisted store. Observed state after step 2:
{
  "id": "<agent-uuid>",
  "name": "",
  "description": null,
  "llm": { "config": {} },
  "mcpServers": null,
  "skills": null,
  "instruction": null
}

The pre-existing llm.backend, llm.model and the five mcpServers entries are gone. Nothing in the response or the server log indicates a destructive change occurred.

Expected behaviour

Either of the following would have prevented this, and we would be happy with either:

Option A — strict validation on the existing replace semantics. Keep PUT as a full replacement, but reject incomplete bodies with 400 Bad Request when any of name (non-empty string), llm.backend, llm.model is missing or empty, and when mcpServers is null rather than an explicit []. The distinction between "omitted" and "explicitly cleared" matters: an explicit empty array is an intentional clear, an absent key is almost always a caller mistake. This makes the destructive case impossible to reach by accident.

Option B — add a real partial update. Introduce PATCH /api/v1/admin/agents/{id} with merge semantics (only supplied keys are modified, absent keys are preserved), and leave PUT as full replacement with the strict validation from Option A. Most callers actually want PATCH; today they are forced to use the destructive verb.

Additional suggestions (lower priority)

  • Log a WARN whenever a PUT reduces a non-empty mcpServers list to empty/null, or clears a previously non-empty name.
  • Consider returning the resulting object in the response body so callers can detect an unintended wipe immediately.
  • An agent whose llm.backend is unset cannot serve any request; treating that state as invalid at write time rather than at run time would fail fast.

Environment

Magec deployed as a standalone binary (no Docker), JSON file store (data/store.json), single-node. Happy to provide the redacted before/after object pair above in any other format if useful — no credentials or tokens are involved in this report.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions