T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/learning_coordinator.py:91
- Finding
- Unrestricted Filesystem Read and File Creation Through Configurable Rules Path## Vulnerability Details **File Location**: `scripts/learning_coordinator.py`, lines 91–94, 137–148, and 218–229 **Vulnerability Type**: Unrestricted filesystem path access **Risk Level**: Medium ### Vulnerable Code ```python self.learning_rules_file = self.config.get( 'learning_rules_file', os.path.expanduser('~/self-improving/learning.md') ) ``` ```python if not os.path.exists(self.learning_rules_file): if self.auto_create: self._ensure_file_exists() else: logger.warning(f"Learning rules file not found: {self.learning_rules_file}") return try: with open(self.learning_rules_file, 'r', encoding='utf-8') as f: content = f.read() ``` ```python parent_dir = os.path.dirname(self.learning_rules_file) if parent_dir and not os.path.exists(parent_dir): os.makedirs(parent_dir, exist_ok=True) content = """# Learning Mechanics ... """ with open(self.learning_rules_file, 'w', encoding='utf-8') as f: f.write(content) ``` ### Technical Analysis The public `LearningCoordinator` constructor accepts the `learning_rules_file` configuration value without validating or constraining it. The path is subsequently passed directly to filesystem APIs. If the supplied path exists, `_load_rules()` reads the file and places parsed content in `self._rules`. The public `search()` method can then return matching excerpts from that content. If the path does not exist and `auto_create` is enabled, `_ensure_file_exists()` recursively creates parent directories and writes a new file at the selected location. The implementation performs no canonical path resolution, trusted-root enforcement, path traversal rejection, symbolic-link protection, file-type validation, or verification that configuration came from a trusted source. Consequently, a caller capable of controlling coordinator configuration can direct the process toward arbitrary paths accessible under the process account. This issue does not provide unrestricted control ...[truncated 1955 chars]
- Remediation
- ## Remediation Suggestions 1. **Restrict files to a trusted root** - Define an explicit data root such as `~/self-improving/`. - Resolve both the trusted root and requested path with `Path.resolve()`. - Reject paths that are not descendants of the trusted root. 2. **Reject unsafe path forms** - Reject traversal components and unexpected absolute paths before use. - Allow only expected filenames or extensions, such as `learning.md`. - Reject device files, sockets, FIFOs, and other non-regular files. 3. **Protect against symbolic-link attacks** - Check each relevant path component for symbolic links. - Where supported, open files using flags such as `O_NOFOLLOW`. - Use atomic creation with exclusive semantics when creating a new file. 4. **Separate reading from creation** - Default `auto_create` to `False` when configuration may be externally influenced. - Require explicit authorization before creating directories or files. - Avoid recursively creating arbitrary parent directory trees. 5. **Constrain information returned by `search()`** - Ensure only the designated learning-rules file can be indexed. - Avoid returning raw excerpts from files whose provenance has not been validated. - Apply authorization controls if search results can be exposed to untrusted users. 6. **Validate configuration at the trust boundary** - Use a schema that rejects unknown or unsafe path values. - Treat caller-supplied paths as untrusted unless the caller is explicitly authorized to select local files. A suitable containment check should follow this pattern: ```python from pathlib import Path trusted_root = Path("~/self-improving").expanduser().resolve() requested = Path(configured_path).expanduser().resolve() if requested != trusted_root / "learning.md": raise ValueError("learning_rules_file must be the approved learning rules file") ``` If multiple filenames must be supported, verify containment with `requested.is_relative_ ...[truncated 73 chars]