T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/init_nika_skill.py:34- Finding
Unrestricted Filesystem Writes and Symbolic-Link Following
- Content
View full analysis
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: MediumVulnerable 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-dirargument is converted directly into aPathwithout 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
View remediation
Remediation Suggestions
- Establish an explicit trusted output root, preferably derived from the repository location rather than the current working directory.
- Resolve both the trusted root and proposed destination with
Path.resolve()and reject any destination that is not contained within the trusted root. - Reject absolute
--out-dirvalues and traversal outside the approved root unless external output is an explicitly documented feature protected by a separate opt-in. - Inspect every existing destination component with
lstat()and reject symbolic links, including the destination file and all parent directories. - Use exclusive file creation for normal operation, such as mode
x, so an existing path cannot be replaced accidentally. - For intentional overwrite behavior, open files using platform-appropriate no-follow protections such as
O_NOFOLLOW, verify the opened object's type withfstat(), and write through the validated file descriptor. - 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.
- Add automated tests covering absolute output paths,
../traversal, symlinked output directories, symlinked destination files, dangling symlinks, and--forcebehavior.