T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/gate_policy.py:43
- Finding
- Unknown Risk Values Fail Open to Allow## Vulnerability Details **File Location**: `scripts/gate_policy.py`, lines 43–45 and 58–74 **Vulnerability Type**: Fail-open risk classification **Risk Level**: High **Vulnerable Code**: ```python def _norm_risk(r): r = (r or "").lower() return r if r in ("low", "mid", "high") else "low" ``` ```python for t in tools: if not isinstance(t, dict): continue name = t.get("name") if not name: continue risk = _norm_risk(t.get("risk")) sensitive = bool(t.get("sensitive")) if sensitive: bucket = preset["sensitive"] elif risk == "high": bucket = preset["high"] elif risk == "mid": bucket = preset["mid"] else: bucket = "allow" if bucket == "deny": deny.append(name) elif bucket == "review": review.append(name) else: allow.append(name) ``` ### Technical Analysis The `_norm_risk` function converts every missing, misspelled, malformed, or unsupported risk value to `low`. The policy-generation loop then unconditionally maps low-risk tools to `allow`. This is a fail-open design: invalid security metadata produces the least restrictive outcome instead of being rejected or handled conservatively. For example, values such as `critical`, `HIGH `, an integer, or a missing `risk` field can result in unrestricted permission. Non-string values may also trigger an exception because `.lower()` is called without type validation. Tool names are deduplicated independently in each permission list later in the function. Therefore, duplicate manifest records assigning different risks to the same tool can place one tool in multiple, contradictory lists. ### Attack Path 1. An attacker, compromised manifest producer, or configuration error introduces a privileged tool into the input manifest. 2. The tool is assigned an unsupported or misspelled risk value, such as `"critical"`, or its ...[truncated 1042 chars]
- Remediation
- ## Remediation Suggestions - Reject missing, non-string, and unsupported risk values instead of converting them to `low`. - If compatibility requires a default, use the most restrictive result, such as `deny`. - Normalize only explicitly supported formatting differences, such as surrounding whitespace, while still rejecting unknown classifications. - Require every tool entry to follow a strict schema with a non-empty tool name, an enumerated risk value, and a Boolean `sensitive` field. - Enforce uniqueness of tool names before classification. - Reject any policy generation request in which one tool receives conflicting classifications. - Add tests covering missing values, `critical`, mixed case, whitespace, non-string values, duplicate names, and conflicting records. - Consider representing risk levels with an enum and validating the complete manifest before generating any output.