T09 · Insecure Skill Coding Practices
- Location
SKILL.md:200- Finding
Destructive MCP operations do not enforce user approval
- Content
View full analysis
Vulnerability Details
File Location:
SKILL.md:200-205; corroborated byreferences/agent-guardrails.md:45-46
Vulnerability Type: Missing authorization gate for privileged state-changing operations
Risk Level: HighVulnerable Snippet
From
SKILL.md:200-205:markdown > **Authorization is not this skill's job**: there is no read-only switch, policy > file, or approval gate. Whether a write runs is the agent's judgement or the > connecting account's permissions — point the tool at an API user without write > scope and writes fail at the server. Every call is still audited. > `FIREWALL_AUDIT_APPROVED_BY` / `FIREWALL_AUDIT_RATIONALE` are optional audit > annotations, recorded when set but never required.The absence of MCP confirmation is explicitly documented in
references/agent-guardrails.md:45-46:markdown | "Confirm before anything destructive" | Every write takes `dry_run=True` for a preview that runs the same guards as the real call. ⚠️ **The double confirmation is a CLI feature, and only `toggle_rule` and `undo apply` have CLI commands** — `apply_changes`, `reconfigure`, `reboot`, `restart_service`, `kill_states`, `add_alias_entry` and `remove_alias_entry` are reachable only over MCP, where nothing prompts. Keep your own confirmation for those. |Technical Analysis
The Skill exposes privileged MCP operations that can commit firewall configuration, restart services, terminate active network states, edit aliases, or reboot the firewall. Although these operations support dry-run behavior and are recorded in an audit database, neither dry-run nor explicit approval is enforced before execution.
The only effective authorization boundary is the permission level of the configured firewall API account. When that account has write access, an agent can directly execute destructive operations without:
- A mandatory prior dry run.
- An interactive user confirmation.
- A required approval identity or rationale.
- A read-on ...[truncated 2075 chars]
- Remediation
View remediation
Remediation Suggestions
- Add a read-only mode and enable it by default. Do not register or expose state-changing MCP tools while this mode is active.
- Require a mandatory, short-lived approval token for every state-changing MCP operation. Bind the token to the target firewall, operation, normalized parameters, approving identity, and expiration time.
- Enforce a two-stage workflow for high-risk operations:
- First call: mandatory dry run producing a stable operation digest.
- Second call: execution requiring explicit approval of that exact digest.
- Reject execution when the proposed parameters differ from the approved dry-run parameters.
- Require
FIREWALL_AUDIT_APPROVED_BYandFIREWALL_AUDIT_RATIONALE, or equivalent structured approval fields, for high-risk writes rather than treating them as optional annotations. - Add explicit confirmation gates for
reboot,kill_states, service restarts, configuration commits, and reconfiguration operations. - Preserve least privilege by documenting and providing separate read-only and write-capable firewall roles. Encourage read-only credentials as the default configuration.
- Keep the existing audit and self-lockout protections as defense-in-depth, but do not treat them as substitutes for authorization.