T09 · Insecure Skill Coding Practices
Error
- Location
- novelforge_engine/settlement.py:143
- Finding
- Project Directory Symlinks Permit Filesystem Access Outside the Project Root<![CDATA[ ## Vulnerability Details **File Location**: `novelforge_engine/settlement.py:143-152, 189-225`; `novelforge_engine/wiki.py:67-114, 125-126, 244-245` **Vulnerability Type**: Insufficient symlink validation and project-root path confinement **Risk Level**: High ### Vulnerable Code ```python # novelforge_engine/settlement.py:143-152 wiki_file = root / folder / (fact["id"] + ".md") if not wiki_file.is_file(): wiki_file = root / "wiki" / folder / (fact["id"] + ".md") if wiki_file.is_file(): try: meta, _ = read_frontmatter(wiki_file) if meta.get("_source_chapter") == chapter: wiki_file.unlink() except (OSError, ValueError): pass ``` ```python # novelforge_engine/settlement.py:189-225 for fact in canonical["facts"]: folder = _FOLDER_BY_TYPE[fact["type"]] folder_path = root / folder folder_path.mkdir(parents=True, exist_ok=True) wiki_path = folder_path / (fact["id"] + ".md") existing_meta = {} existing_body = "" if wiki_path.is_file(): try: existing_meta, existing_body = read_frontmatter(wiki_path) except Exception: existing_meta, existing_body = {}, "" elif (root / "wiki" / folder / (fact["id"] + ".md")).is_file(): try: existing_meta, existing_body = read_frontmatter( root / "wiki" / folder / (fact["id"] + ".md") ) except Exception: existing_meta, existing_body = {}, "" meta = dict(existing_meta) meta.update(fact) meta["_source_chapter"] = chapter meta["_source_body_hash"] = canonical["source_hash"] body = existing_body if not body.strip(): name = meta.get("name", meta.get("id", "Untitled")) lines = [f"# {name}\n"] for key in sorted(meta): if key in ( "id", "name", "type", "_source_chapter", "_source_body_hash", "p ...[truncated 3830 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Reject symbolic links for the project root and every managed directory before any operation: - `.novelforge` - `wiki` - `characters` - `locations` - `items` - `factions` - `hooks` - `relations` - `timeline` - `entities` - `blueprints` - `chapters` 2. Resolve every candidate path and verify confinement immediately before reading, writing, replacing, or deleting: ```python def require_project_path(root, candidate): root = Path(root).resolve(strict=True) candidate = Path(candidate) if candidate.is_symlink(): raise ValueError("symbolic links are not permitted") resolved_parent = candidate.parent.resolve(strict=True) resolved_parent.relative_to(root) return resolved_parent / candidate.name ``` 3. Check every intermediate path component, not only the final file. Reject the operation if any component is a symlink. 4. For deletion, verify that the resolved target remains beneath the resolved project root and is a regular file before calling `unlink()`. 5. Where supported, use directory file descriptors and no-follow flags such as `O_NOFOLLOW` to reduce time-of-check/time-of-use races. 6. Do not recursively scan a directory until its resolved location has been confirmed to be inside the project root. 7. Add tests covering: - A symlinked standard Wiki directory. - A symlinked legacy `wiki` directory. - Settlement writes through a symlink. - Stale-settlement deletion through a symlink. - Wiki scanning through a symlink. - Symlink replacement between validation and mutation. ]]>