T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/init_nika_skill.py:34
- Finding
- Unrestricted Filesystem Writes and Symbolic-Link Following## Vulnerability Details **File Location**: `scripts/init_nika_skill.py`, lines 34-37 and 157-184 **Vulnerability Type**: Unrestricted filesystem write and symbolic-link traversal **Risk Level**: Medium ### Vulnerable Code ```python def _write_text(path: Path, content: str, *, force: bool) -> None: if path.exists() and not force: _die(f"refusing to overwrite existing file: {path}") path.parent.mkdir(parents=True, exist_ok=True) path.write_text(content, encoding="utf-8") ``` ```python parser.add_argument("--out-dir", default="skills", help="Output base directory (default: skills).") parser.add_argument( "--subdocs", default=None, help="Comma-separated list of sub doc names (default: 步骤,模板,示例).", ) parser.add_argument( "--force", action="store_true", help="Overwrite existing files if they already exist.", ) args = parser.parse_args(argv) skill_name_cn = args.skill_name_cn _validate_name(skill_name_cn) out_dir = Path(args.out_dir) subdocs = _parse_subdocs(args.subdocs) for s in subdocs: _validate_name(s) skill_dir = out_dir / skill_name_cn main_doc = skill_dir / "SKILL.md" refs_dir = skill_dir / "references" refs_dir.mkdir(parents=True, exist_ok=True) _write_text(main_doc, _main_doc_content(skill_name_cn, subdocs), force=args.force) created: list[Path] = [main_doc] for s in subdocs: p = refs_dir / f"{s}.md" _write_text(p, _subdoc_content(s), force=args.force) created.append(p) ``` ### Technical Analysis The `--out-dir` argument is converted directly into a `Path` without canonicalization or verification that the resulting destination remains inside the repository or another approved output root. It can therefore contain an absolute path or traversal components such as `../`. The write operation uses `Path.write_text()`, which follows symbolic links. The code checks whether the destination exists but does no ...[truncated 2482 chars]
- Remediation
- ## Remediation Suggestions 1. Establish an explicit trusted output root, preferably derived from the repository location rather than the current working directory. 2. Resolve both the trusted root and proposed destination with `Path.resolve()` and reject any destination that is not contained within the trusted root. 3. Reject absolute `--out-dir` values and traversal outside the approved root unless external output is an explicitly documented feature protected by a separate opt-in. 4. Inspect every existing destination component with `lstat()` and reject symbolic links, including the destination file and all parent directories. 5. Use exclusive file creation for normal operation, such as mode `x`, so an existing path cannot be replaced accidentally. 6. For intentional overwrite behavior, open files using platform-appropriate no-follow protections such as `O_NOFOLLOW`, verify the opened object's type with `fstat()`, and write through the validated file descriptor. 7. Avoid check-then-write logic because it is vulnerable to time-of-check/time-of-use races. Perform validation and secure opening atomically wherever the operating system permits. 8. Add automated tests covering absolute output paths, `../` traversal, symlinked output directories, symlinked destination files, dangling symlinks, and `--force` behavior.