T09 · Insecure Skill Coding Practices
- Location
src/core/cleaner.ts:4- Finding
Recursive cleanup deletes unmanaged and expected Skill files
- Content
View full analysis
(); for (const skill of skills) { expectedFiles.add(path.join(skill.name, 'SKILL.md')); } expectedFiles.add('index.md'); await cleanDirectory(targetPath, expectedFiles); } ``` ```ts // src/core/cleaner.ts:4-26 export async function cleanDirectory( targetPath: string, expectedFiles: Set ): Promise { if (!(await pathExistsFn(targetPath))) return; const entries = await readdir(targetPath, { withFileTypes: true }); for (const entry of entries) { const entryPath = path.join(targetPath, entry.name); if (entry.isDirectory()) { await cleanDirectory(entryPath, expectedFiles); const remaining = await readdir(entryPath); if (remaining.length === 0) { await remove(entryPath); } } else { const relative = path.relative(targetPath, entryPath); if (!expectedFiles.has(relative) && !expectedFiles.has(entryPath)) { await remove(entryPath); } } } } ``` ### Technical Analysis The cleanup routine recursively changes `targetPath`, but `expectedFiles` remains relative to the original editor directory. For example, the expected set contains `skill-name/SKILL.md`. After recursion enters `skill-name`, the computed relative path is only `SKILL.md`, so it no longer matches the expected entry and is deleted. The routine also removes every file not present in the current installation set. There is no ownership manifest distinguishing files created by this application from manually installed or third-party Skills. Affected directory presets include: - `~/.claude ...[truncated 1007 chars]- Remediation
View remediation
): Promise { const relative = path.relative(rootPath, entryPath); } ``` 2. Normalize expected and observed paths using one consistent separator and representation. 3. Maintain an application-owned manifest containing only files created by `open-skills`. 4. Delete only files listed in the previous manifest but absent from the new manifest. 5. Never delete unmanaged files automatically. 6. Display a deletion plan and require explicit confirmation before destructive cleanup. 7. Create a backup or move stale files to a recoverable quarantine directory. 8. Add tests for nested Skill directories, unmanaged files, symbolic links, and global installation paths. ]]>
