T09 · Insecure Skill Coding Practices
- Location
scripts/create-skills-pager-map.js:73- Finding
Path Traversal Enables Filesystem Writes Outside the Intended Skill Index Directory
- Content
View full analysis
Vulnerability Details
File Location:
scripts/create-skills-pager-map.js, lines 17–18, 32–34, 63, and 73–75
Vulnerability Type: Unvalidated path component / directory traversal
Risk Level: MediumVulnerable Code
js if (value === "--skill-id") { args.skillId = argv[++i]; }js if (!args.skillId) { fail("Missing required argument: --skill-id"); }js function writeText(filePath, content) { fs.mkdirSync(path.dirname(filePath), { recursive: true }); fs.writeFileSync(filePath, content, "utf8"); }js const indexRoot = path.join(workspaceRoot, ".skill-index"); const skillRoot = path.join(indexRoot, "skills", args.skillId); const registryPath = path.join(indexRoot, "registry.json"); const indexPath = path.join(skillRoot, "index.md");Technical Analysis
The
--skill-idargument is used directly as a filesystem path component without validation.path.join()normalizes path traversal sequences rather than rejecting them. Consequently, a value containing../segments can causeskillRootandindexPathto resolve outside the intended.skill-index/skills/directory.The resulting path is passed to
writeText(), which recursively creates missing directories and overwrites the destination withfs.writeFileSync(). No canonical-path containment check, safe identifier allowlist, symbolic-link check, or overwrite confirmation protects the operation.Although the destination filename is fixed as
index.md, an attacker can select an arbitrary writable directory relative to the current workspace and cause that directory'sindex.mdto be created or replaced.Attack Path
-
An attacker influences the script invocation or supplies an untrusted skill identifier.
-
The attacker provides a traversal value, for example:
bash node scripts/create-skills-pager-map.js \ --skill-id ../../../target-directory \ --source target/SKILL.md \ --page setup -
`path.join(indexRoot, " ...[truncated 1221 chars]
-
- Remediation
View remediation
Remediation Suggestions
-
Apply a strict identifier allowlist
Accept only a single safe path component:
js function validateSkillId(skillId) { if (!/^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(skillId)) { fail("Invalid --skill-id: use letters, numbers, dots, underscores, or hyphens only"); } if (skillId === "." || skillId === "..") { fail("Invalid --skill-id"); } return skillId; } -
Enforce canonical path containment
Resolve both the permitted root and destination, then verify that the destination remains beneath the permitted root:
js const skillsRoot = path.resolve(indexRoot, "skills"); const skillId = validateSkillId(args.skillId); const skillRoot = path.resolve(skillsRoot, skillId); if ( skillRoot === skillsRoot || !skillRoot.startsWith(`${skillsRoot}${path.sep}`) ) { fail("Resolved skill path escapes .skill-index/skills"); } const indexPath = path.join(skillRoot, "index.md"); -
Reject path syntax explicitly
Reject absolute paths,
/,\, null bytes, and traversal components even if an allowlist is also used:js if ( path.isAbsolute(args.skillId) || args.skillId.includes("/") || args.skillId.includes("\\") || args.skillId.includes("\0") ) { fail("Invalid --skill-id"); } -
Protect against unintended overwrites
Use exclusive creation when appropriate or require an explicit overwrite option:
js fs.writeFileSync(indexPath, content, { encoding: "utf8", flag: args.force ? "w" : "wx", }); -
Use atomic writes
Write to a temporary file in the validated destination directory and rename it atomically after successful completion. Ensure the temporary filename is generated by the program rather than derived from user input.
-
Add regression tests
Tests should verify rejection of:
../target../../../../tmp/target
...[truncated 181 chars]
-