T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/permission_check.py:50
- Finding
- Fail-Open Permission Classification Creates a False Authorization Boundary## Vulnerability Details **File Location**: `scripts/permission_check.py:50-75`; related security claims in `SKILL.md:94-106` and autonomy rules in `SKILL.md:120-176` **Vulnerability Type**: Fail-open authorization logic and ineffective permission enforcement **Risk Level**: High ### Vulnerable Code `scripts/permission_check.py:50-75`: ```python def check_permission(action, config): """ Check if an action requires confirmation. Returns: bool: True if confirmation required, False otherwise """ level = config.get('permission_level', 2) # Combine default and custom rules red_lines = config.get('red_lines', []) + config.get('custom_red_lines', []) yellow_lines = config.get('yellow_lines', []) + config.get('custom_yellow_lines', []) # Level 3: Everything requires confirmation if level == 3: return True # Level 2: Red + Yellow require confirmation if level == 2: return action in red_lines or action in yellow_lines # Level 1: Only Red requires confirmation if level == 1: return action in red_lines return False ``` `SKILL.md:94-106` presents this classifier as automatic permission enforcement: ```python # Example: AI wants to delete a file if permission_check('delete_file', user_permission_level): # Ask user for confirmation else: # Execute directly ``` ### Technical Analysis The permission check uses exact string matching against caller-controlled action names. At permission levels 1 and 2, every action not explicitly listed in `red_lines` or `yellow_lines` is classified as safe to execute without confirmation. Consequently, semantically equivalent but unlisted names such as `remove_file`, `erase_user_data`, `publish_message`, or `change_host_configuration` bypass rules named `delete_file`, `delete_database`, `send_public_message`, or `modify_system_config`. There is no canonical ac ...[truncated 2752 chars]
- Remediation
- ## Remediation Suggestions 1. **Fail closed for unknown actions** - Require confirmation for every action that is not explicitly classified. - Replace the final `return False` with a confirmation-required result or a policy error. 2. **Use a closed action model** - Define a canonical enumeration of supported operations. - Reject arbitrary action strings. - Map low-level operations to security categories such as filesystem deletion, external communication, production mutation, credential access, package installation, and system configuration. 3. **Validate configuration strictly** - Require `permission_level` to be exactly `1`, `2`, or `3`. - Validate that rule fields are lists containing only recognized action identifiers. - Reject malformed, empty, or unexpected YAML structures. - Treat all policy-loading errors as requiring confirmation. 4. **Use one authoritative default policy** - Generate both fallback and on-disk policies from a shared constant or schema. - Add automated tests that verify parity between generated and fallback policies. 5. **Enforce checks at the operation boundary** - Integrate authorization into the actual tool dispatcher or operation wrapper. - Prevent sensitive tool calls from executing until a valid policy decision and, where required, explicit user confirmation are recorded. - Do not rely on Agent instructions to invoke the checker voluntarily. 6. **Bind confirmation to the exact operation** - Include the normalized action, target resource, environment, relevant parameters, and expected side effects in the confirmation request. - Prevent a confirmation for one operation from authorizing a broader or different operation. 7. **Correct the documentation** - Describe the current script as an advisory classifier unless an enforcing integration is implemented. - Clearly state that it cannot itself prevent tool execution. - Avoid p ...[truncated 332 chars]