T09 · Insecure Skill Coding Practices
Error
- Location
- references/capabilities.md:30
- Finding
- Destructive MCP Operations Execute Without Enforced Approval<![CDATA[ ## Vulnerability Details **File Location**: `references/capabilities.md:30-45` **Supporting Locations**: `SKILL.md:207-211`; `references/agent-guardrails.md:96-106` **Vulnerability Type**: Missing authorization and approval enforcement for high-impact operations **Risk Level**: High ### Vulnerable Code Snippet ```markdown | **MCP** | **None, by design.** A write tool acts on the first call. There is no `confirmed=` handshake, no approval tier, and no read-only switch — the switch existed in v1.8.0–1.8.6 and was removed in v1.8.7 (decision **D-2** of the family security HLD, 2026-07-21) because it was enforced on the MCP path only and any agent with a shell stepped around it. A two-step handshake was considered in the same review and cut: it is neither authorization nor accountability, only a speed-bump that a model intending to act steps over by passing `confirmed=True`. | 7 of the 43 write tools default to a no-write preview (below) | **What actually protects the estate over MCP is the vCenter/ESXi service account.** Writes the account may not perform are refused by vCenter itself, whatever the agent intends, on every surface, with no way around it from inside the skill. To run an agent read-only, give it a read-only vCenter role and point the skill's `.env` at that account — one decision, enforced where it is made. What happened is then recoverable from `~/.vmware/audit.db`, which every write goes through before the caller sees a result. Nothing in this skill will stop `vm_delete` deleting a VM the account is allowed to delete. - **Write tools: 43** — every tool whose description starts `[WRITE]` and whose `readOnlyHint` is `false`. - **Confirm-gated: 7** — `add_host_vmk`, `create_drs_rule`, `create_dvs_portgroup`, `delete_drs_rule`, `remove_host_vmk`, `set_drs_rule_enabled`, `set_vmk_service` <br>These host-networking and DRS authoring tools take a `confirm` argument that defaults to false, in which case they validate everything they can and ...[truncated 3148 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Make the MCP server read-only by default and require an explicit administrative configuration to expose write tools. 2. Implement server-enforced two-phase authorization for destructive and guest-writing operations: - First call validates the request and returns an immutable operation digest. - A separately authenticated approval service issues a short-lived approval token bound to the digest, target, requester, and expiration. - The execution call must present that token and must reject changed parameters. 3. Require external human approval for VM deletion, cluster deletion, snapshot reversion or deletion, forced shutdown, network removal, alarm reset, TTL deletion, guest execution, and guest file upload. 4. Enforce deny-by-default policy rules for production targets rather than relying on operators to create optional deny rules. 5. Separate guest operations into an independently enabled component with distinct credentials and policy controls. 6. Prohibit root or administrator guest credentials by default. Use dedicated guest accounts restricted to the commands, paths, and VMs required for the task. 7. Add command allowlists, argument validation, destination-path restrictions, and execution timeouts for guest operations. 8. Ensure approval and authorization checks occur inside the server and cannot be bypassed through the shell or an alternative client. 9. Retain audit logging, but treat it as a detective control rather than a substitute for preventive authorization. ]]>